[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 7/9] xen/sched: switch scheduling to bool where appropriate

On 18.12.19 12:54, Dario Faggioli wrote:
On Wed, 2019-12-18 at 08:48 +0100, Juergen Gross wrote:
Scheduling code has several places using int or bool_t instead of
Switch those.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

I'm fine with pretty much everything in this patch. Just two comments:

diff --git a/xen/common/sched/sched_rt.c
index 264a753116..8646d77343 100644
--- a/xen/common/sched/sched_rt.c
+++ b/xen/common/sched/sched_rt.c
@@ -490,10 +490,10 @@ rt_update_deadline(s_time_t now, struct rt_unit
  static inline bool
  deadline_queue_remove(struct list_head *queue, struct list_head
-    int pos = 0;
+    bool pos = false;
if ( queue->next != elem )
-        pos = 1;
+        pos = true;
      return !pos;

IIRC, this code was "inspired" by similar functions in Credit2, where
we store in 'pos' the actual position of the entity in the runq (only
for tracing purposes, these days, but that's another story).

In here, it is indeed used only as a flag so it must be bool, and it
can also have a better name like, for instance, 'first' or 'head' (I
probably like 'first' better).

I'm fine with that.

@@ -505,14 +505,14 @@ deadline_queue_insert(struct rt_unit *
(*qelem)(struct list_head *),
                        struct list_head *queue)
      struct list_head *iter;
-    int pos = 0;
+    bool pos = false;
list_for_each ( iter, queue )
          struct rt_unit * iter_svc = (*qelem)(iter);
          if ( compare_unit_priority(svc, iter_svc) > 0 )
-        pos++;
+        pos = true;
      list_add_tail(elem, iter);
      return !pos;

And this is similar, but the logic is inverted.

I think the best solution for this hunk, without subverting the code
too much, would be to also rename 'pos' into 'fist' and initialize it
to true.

Then, in the loop, we set it to false. So that it will still be true,
if we exit immediately, false if we do at least one step.

And finally we return 'first' and not its negation.


Yes, will do that.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.