Changeset c0c388d2 in mainline

Timestamp:

2019-08-03T07:36:48Z (5 years ago)

Author:

Matthieu Riolo <matthieu.riolo@…>

Children:

4fe7fcb

Parents:

694253c

git-author:

Michal Koutný <xm.koutny+hos@…> (2015-03-16 19:31:17)

git-committer:

Matthieu Riolo <matthieu.riolo@…> (2019-08-03 07:36:48)

Message:

Sysman jobs memory management (refcnt)

Location:

uspace/srv/sysman

Files:

• job.c (modified) (8 diffs)
• job.h (modified) (4 diffs)
• sysman.c (modified) (3 diffs)

uspace/srv/sysman/job.c

@@ -14 +14 @@
 static fibril_condvar_t job_queue_cv;
 
+static void job_destroy(job_t **);
+
+
 static int job_run_start(job_t *job)
 {
@@ -72 +75 @@
         fibril_condvar_broadcast(&job->state_cv);
         fibril_mutex_unlock(&job->state_mtx);
+
+        job_del_ref(&job);
 
         return EOK;
@@ -91 +96 @@
                  * Note that possible use of fibril pool must hold invariant
                  * that job is started asynchronously. In the case there exists
-                 * circular dependency between jobs, it may attain a deadlock.
+                 * circular dependency between jobs, it may result in a deadlock.
                  */
                 job_t *job = list_get_instance(link, job_t, link);
-                fid_t runner_fibril = fibril_create(job_runner, job); // TODO cast to void*?
+                fid_t runner_fibril = fibril_create(job_runner, job);
                 fibril_add_ready(runner_fibril);
         }
@@ -135 +140 @@
         }
 
+        /* Only job dispatcher waits, it's correct to notify one only. */
         fibril_condvar_signal(&job_queue_cv);
         fibril_mutex_unlock(&job_queue_mtx);
@@ -160 +166 @@
 }
 
+void job_add_ref(job_t *job)
+{
+        atomic_inc(&job->refcnt);
+}
+
+void job_del_ref(job_t **job_ptr)
+{
+        job_t *job = *job_ptr;
+        if (job == NULL) {
+                return;
+        }
+
+        assert(atomic_get(&job->refcnt) > 0);
+        if (atomic_predec(&job->refcnt) == 0) {
+                job_destroy(job_ptr);
+        }
+}
+
 static void job_init(job_t *job, job_type_t type)
 {
@@ -166 +190 @@
         link_initialize(&job->link);
         list_initialize(&job->blocking_jobs);
+
+        /* Start with one reference for the creator */
+        atomic_set(&job->refcnt, 1);
 
         job->type = type;
@@ -185 +212 @@
 }
 
-void job_destroy(job_t **job_ptr)
+static void job_destroy(job_t **job_ptr)
 {
         job_t *job = *job_ptr;
@@ -195 +222 @@
                 job_link_t *jl = list_get_instance(cur_link, job_link_t, link);
                 list_remove(cur_link);
+                job_del_ref(&jl->job);
                 free(jl);
         }

uspace/srv/sysman/job.h

@@ -3 +3 @@
 
 #include <adt/list.h>
+#include <atomic.h>
 
 #include "unit.h"
@@ -25 +26 @@
 
 struct job {
+        /** Link to queue job is in */
         link_t link;
 
+        /** List of jobs (job_link_t ) that are blocking the job. */
         list_t blocking_jobs;
+
+        /** Reference counter for the job structure. */
+        atomic_t refcnt;
 
         job_type_t type;
@@ -36 +42 @@
         fibril_condvar_t state_cv;
 
+        /** Return value of the job, defined only when state == JOB_FINISHED */
         int retval;
 };
@@ -44 +51 @@
 extern int job_wait(job_t *);
 
+extern void job_add_ref(job_t *);
+extern void job_del_ref(job_t **);
+
 extern job_t *job_create(job_type_t type);
-extern void job_destroy(job_t **);
 
 #endif

uspace/srv/sysman/sysman.c

@@ -27 +27 @@
         }
 
+        /* Job is passed to the accumulator, i.e. no add_ref. */
         list_append(&job->link, accumulator);
 
@@ -35 +36 @@
 
 fail:
-        job_destroy(&job);
+        job_del_ref(&job);
         return rc;
 }
@@ -50 +51 @@
         }
 
+        // TODO handle errors when adding job accumulator
         job_queue_jobs(&new_jobs);