Il giorno 07 mar 2017, alle ore 18:22, Jens Axboe [off-list ref] ha =

scritto:

=20

quoted

+/**
+ * bfq_entity_of - get an entity from a node.
+ * @node: the node field of the entity.
+ *
+ * Convert a node pointer to the relative entity.  This is used only
+ * to simplify the logic of some functions and not as the generic
+ * conversion mechanism because, e.g., in the tree walking =

functions,

quoted

+ * the check for a %NULL value would be redundant.
+ */
+static struct bfq_entity *bfq_entity_of(struct rb_node *node)
+{
+	struct bfq_entity *entity =3D NULL;
+
+	if (node)
+		entity =3D rb_entry(node, struct bfq_entity, rb_node);
+
+	return entity;
+}

=20
Get rid of pointless wrappers like this, just use rb_entry() in the
caller. It's harmful to the readability of the code to have to lookup
things like this, if it's a list_entry or rb_entry() in the caller you
know exactly what is going on immediately.
=20

Ok.  Just a quick request for help about this.  The code seems to become
quite ugly with a verbatim replacement of this function with its body
(and there are several occurrences of it), so there is probably =
something I'm
missing.  For example, given the following loop:

	for (; entity ; entity =3D bfq_entity_of(rb_first(active)))
		bfq_reparent_leaf_entity(bfqd, entity);

the only non-cryptic, but heavier solution I see is:

	for (; entity ; ) {
		bfq_reparent_leaf_entity(bfqd, entity);
		if (rb_first(active))
			entity =3D rb_entry(rb_first(active), struct =
bfq_entity, rb_node);
		else
			entity =3D NULL;
	}

Am I missing something?  Or are these extra lines of code a reasonable
price to pay for increased transparency?

Thanks,
Paolo

--=20
Jens Axboe
=20