Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751597AbdCSLyH (ORCPT ); Sun, 19 Mar 2017 07:54:07 -0400 Received: from mail-qk0-f179.google.com ([209.85.220.179]:36357 "EHLO mail-qk0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751284AbdCSLyG (ORCPT ); Sun, 19 Mar 2017 07:54:06 -0400 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Subject: Re: [PATCH RFC 01/14] block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler From: Paolo Valente In-Reply-To: <1489850658.2339.1.camel@sandisk.com> Date: Sun, 19 Mar 2017 07:45:30 -0400 Cc: "linux-kernel@vger.kernel.org" , "linux-block@vger.kernel.org" , "fchecconi@gmail.com" , "linus.walleij@linaro.org" , "axboe@kernel.dk" , Arianna Avanzini , "broonie@kernel.org" , "tj@kernel.org" , "ulf.hansson@linaro.org" Message-Id: References: <20170304160131.57366-1-paolo.valente@linaro.org> <20170304160131.57366-2-paolo.valente@linaro.org> <1D08B61A9CF0974AA09887BE32D889DA0DA145@ULS-OP-MBXIP03.sdcorp.global.sandisk.com> <0B1F1CFD-C8AB-442D-B5C6-426D19B1FBA3@linaro.org> <1489850658.2339.1.camel@sandisk.com> To: Bart Van Assche X-Mailer: Apple Mail (2.3124) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v2JBsIuM021637 Content-Length: 2242 Lines: 63 > Il giorno 18 mar 2017, alle ore 11:24, Bart Van Assche 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 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.