On Sat, 2017-03-18 at 08:08 -0400, Paolo Valente wrote:
> > Il giorno 06 mar 2017, alle ore 14:40, Bart Van Assche
> > <[email protected]> ha scritto:
> > > +#define BFQ_BFQQ_FNS(name)
> > > \
> > > +static void bfq_mark_bfqq_##name(struct bfq_queue *bfqq) \
> > > +{
> > > \
> > > + (bfqq)->flags |= (1 << BFQ_BFQQ_FLAG_##name); \
> > > +}
> > > \
> > > +static void bfq_clear_bfqq_##name(struct bfq_queue *bfqq)
> > > \
> > > +{
> > > \
> > > + (bfqq)->flags &= ~(1 << BFQ_BFQQ_FLAG_##name); \
> > > +}
> > > \
> > > +static int bfq_bfqq_##name(const struct bfq_queue *bfqq) \
> > > +{
> > > \
> > > + return ((bfqq)->flags & (1 << BFQ_BFQQ_FLAG_##name)) != 0; \
> > > +}
> >
> > Are the bodies of the above functions duplicates of __set_bit(),
> > __clear_bit() and test_bit()?
>
> Yes. We wrapped them into functions, because writing mark_flag_name
> seemed more readable than writing the implementation of the marking of the
> flag.
Please do not open-code __set_bit(), __clear_bit() and test_bit() but use
these macros instead.
> > > + } 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 = bfqd->bfq_max_budget;
> > > +
> >
> > Please balance braces. Checkpatch should have warned about the use of "}
> > else" instead of "} else {".
>
> 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?
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.
Bart.
