Il giorno 18 mar 2017, alle ore 11:24, Bart Van Assche =

[off-list ref] ha scritto:

=20
On Sat, 2017-03-18 at 08:08 -0400, Paolo Valente wrote:

quoted

quoted

Il giorno 06 mar 2017, alle ore 14:40, Bart Van Assche =

[off-list ref] ha scritto:

quoted

quoted

quoted

+#define BFQ_BFQQ_FNS(name)						=

\

quoted

quoted

quoted

+static void bfq_mark_bfqq_##name(struct bfq_queue *bfqq)		=

\

quoted

quoted

quoted

+{									=

\

quoted

quoted

quoted

+	(bfqq)->flags |=3D (1 << BFQ_BFQQ_FLAG_##name);			=

\

quoted

quoted

quoted

+}									=

\

quoted

quoted

quoted

+static void bfq_clear_bfqq_##name(struct bfq_queue *bfqq)		=

\

quoted

quoted

quoted

+{									=

\

quoted

quoted

quoted

+	(bfqq)->flags &=3D ~(1 << BFQ_BFQQ_FLAG_##name);			=

\

quoted

quoted

quoted

+}									=

\

quoted

quoted

quoted

+static int bfq_bfqq_##name(const struct bfq_queue *bfqq)		=

\

quoted

quoted

quoted

+{									=

\

quoted

quoted

quoted

+	return ((bfqq)->flags & (1 << BFQ_BFQQ_FLAG_##name)) !=3D 0;	=

\

quoted

quoted

quoted

+}

=20
Are the bodies of the above functions duplicates of __set_bit(),
__clear_bit() and test_bit()?

=20
Yes.  We wrapped them into functions, because writing mark_flag_name
seemed more readable than writing the implementation of the marking =

of the

quoted

flag.

=20
Please do not open-code __set_bit(), __clear_bit() and test_bit() but =

use

these macros instead.
=20

ok, as usual, I misunderstood, and thought you wanted me to remove
those macros altogether.  I'll fix their bodies, sorry.

quoted

quoted

quoted

+	} else
+		/*
+		 * Async queues get always the maximum possible
+		 * budget, as for them we do not care about latency
+		 * (in addition, their ability to dispatch is limited
+		 * by the charging factor).
+		 */
+		budget =3D bfqd->bfq_max_budget;
+

=20
Please balance braces. Checkpatch should have warned about the use =

of "}

quoted

quoted

else" instead of "} else {".

=20
No warning, I guess because the body of the else contains only a
simple instruction.  Just to learn for the future: what's the
rationale for adding braces here, but not imposing braces everywhere
for single-instruction bodies?

=20
It's a general style recommendation for all kernel code: if braces are =

used

for one side of an if-statement, also use braces for the other side, =

and

definitely if that other side consists of multiple lines due to a =

comment.

=20

Ok, thanks for repeating this rule for me.

Thanks,
Paolo

Bart.