linux-kernel - Re: [PATCH RFC 01/14] block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <C37CED4C-5A92-422B-B6C9-4ECCD7E596A3@linaro.org>
Date:   Sun, 19 Mar 2017 07:45:30 -0400
From:   Paolo Valente <paolo.valente@...aro.org>
To:     Bart Van Assche <bart.vanassche@...disk.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "fchecconi@...il.com" <fchecconi@...il.com>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        Arianna Avanzini <avanzini.arianna@...il.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "tj@...nel.org" <tj@...nel.org>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>
Subject: Re: [PATCH RFC 01/14] block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler


> Il giorno 18 mar 2017, alle ore 11:24, Bart Van Assche <bart.vanassche@...disk.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@...disk.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.