lkml.org 
[lkml]   [2017]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RFC 01/14] block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler
From
Date

> Il giorno 18 mar 2017, alle ore 11:24, Bart Van Assche <bart.vanassche@sandisk.com> ha scritto:
>
> On Sat, 2017-03-18 at 08:08 -0400, Paolo Valente wrote:
>>> Il giorno 06 mar 2017, alle ore 14:40, Bart Van Assche <bart.vanassche@sandisk.com> 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.
>

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

>>>> + } 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.
>

Ok, thanks for repeating this rule for me.

Thanks,
Paolo

> Bart.

\
 
 \ /
  Last update: 2017-03-19 12:54    [W:0.583 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site