All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@sandisk.com>
To: "paolo.valente@linaro.org" <paolo.valente@linaro.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"fchecconi@gmail.com" <fchecconi@gmail.com>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"avanzini.arianna@gmail.com" <avanzini.arianna@gmail.com>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"tj@kernel.org" <tj@kernel.org>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>
Subject: Re: [PATCH RFC 01/14] block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler
Date: Sat, 18 Mar 2017 15:24:34 +0000	[thread overview]
Message-ID: <1489850658.2339.1.camel@sandisk.com> (raw)
In-Reply-To: <0B1F1CFD-C8AB-442D-B5C6-426D19B1FBA3@linaro.org>

T24gU2F0LCAyMDE3LTAzLTE4IGF0IDA4OjA4IC0wNDAwLCBQYW9sbyBWYWxlbnRlIHdyb3RlOg0K
PiA+IElsIGdpb3JubyAwNiBtYXIgMjAxNywgYWxsZSBvcmUgMTQ6NDAsIEJhcnQgVmFuIEFzc2No
ZSA8YmFydC52YW5hc3NjaGVAc2FuZGlzay5jb20+IGhhIHNjcml0dG86DQo+ID4gPiArI2RlZmlu
ZSBCRlFfQkZRUV9GTlMobmFtZSkJCQkJCQlcDQo+ID4gPiArc3RhdGljIHZvaWQgYmZxX21hcmtf
YmZxcV8jI25hbWUoc3RydWN0IGJmcV9xdWV1ZSAqYmZxcSkJCVwNCj4gPiA+ICt7CQkJCQkJCQkJ
XA0KPiA+ID4gKwkoYmZxcSktPmZsYWdzIHw9ICgxIDw8IEJGUV9CRlFRX0ZMQUdfIyNuYW1lKTsJ
CQlcDQo+ID4gPiArfQkJCQkJCQkJCVwNCj4gPiA+ICtzdGF0aWMgdm9pZCBiZnFfY2xlYXJfYmZx
cV8jI25hbWUoc3RydWN0IGJmcV9xdWV1ZSAqYmZxcSkJCVwNCj4gPiA+ICt7CQkJCQkJCQkJXA0K
PiA+ID4gKwkoYmZxcSktPmZsYWdzICY9IH4oMSA8PCBCRlFfQkZRUV9GTEFHXyMjbmFtZSk7CQkJ
XA0KPiA+ID4gK30JCQkJCQkJCQlcDQo+ID4gPiArc3RhdGljIGludCBiZnFfYmZxcV8jI25hbWUo
Y29uc3Qgc3RydWN0IGJmcV9xdWV1ZSAqYmZxcSkJCVwNCj4gPiA+ICt7CQkJCQkJCQkJXA0KPiA+
ID4gKwlyZXR1cm4gKChiZnFxKS0+ZmxhZ3MgJiAoMSA8PCBCRlFfQkZRUV9GTEFHXyMjbmFtZSkp
ICE9IDA7CVwNCj4gPiA+ICt9DQo+ID4gDQo+ID4gQXJlIHRoZSBib2RpZXMgb2YgdGhlIGFib3Zl
IGZ1bmN0aW9ucyBkdXBsaWNhdGVzIG9mIF9fc2V0X2JpdCgpLA0KPiA+IF9fY2xlYXJfYml0KCkg
YW5kIHRlc3RfYml0KCk/DQo+IA0KPiBZZXMuICBXZSB3cmFwcGVkIHRoZW0gaW50byBmdW5jdGlv
bnMsIGJlY2F1c2Ugd3JpdGluZyBtYXJrX2ZsYWdfbmFtZQ0KPiBzZWVtZWQgbW9yZSByZWFkYWJs
ZSB0aGFuIHdyaXRpbmcgdGhlIGltcGxlbWVudGF0aW9uIG9mIHRoZSBtYXJraW5nIG9mIHRoZQ0K
PiBmbGFnLg0KDQpQbGVhc2UgZG8gbm90IG9wZW4tY29kZSBfX3NldF9iaXQoKSwgX19jbGVhcl9i
aXQoKSBhbmQgdGVzdF9iaXQoKSBidXQgdXNlDQp0aGVzZSBtYWNyb3MgaW5zdGVhZC4NCg0KPiA+
ID4gKwl9IGVsc2UNCj4gPiA+ICsJCS8qDQo+ID4gPiArCQkgKiBBc3luYyBxdWV1ZXMgZ2V0IGFs
d2F5cyB0aGUgbWF4aW11bSBwb3NzaWJsZQ0KPiA+ID4gKwkJICogYnVkZ2V0LCBhcyBmb3IgdGhl
bSB3ZSBkbyBub3QgY2FyZSBhYm91dCBsYXRlbmN5DQo+ID4gPiArCQkgKiAoaW4gYWRkaXRpb24s
IHRoZWlyIGFiaWxpdHkgdG8gZGlzcGF0Y2ggaXMgbGltaXRlZA0KPiA+ID4gKwkJICogYnkgdGhl
IGNoYXJnaW5nIGZhY3RvcikuDQo+ID4gPiArCQkgKi8NCj4gPiA+ICsJCWJ1ZGdldCA9IGJmcWQt
PmJmcV9tYXhfYnVkZ2V0Ow0KPiA+ID4gKw0KPiA+IA0KPiA+IFBsZWFzZSBiYWxhbmNlIGJyYWNl
cy4gQ2hlY2twYXRjaCBzaG91bGQgaGF2ZSB3YXJuZWQgYWJvdXQgdGhlIHVzZSBvZiAifQ0KPiA+
IGVsc2UiIGluc3RlYWQgb2YgIn0gZWxzZSB7Ii4NCj4gDQo+IE5vIHdhcm5pbmcsIEkgZ3Vlc3Mg
YmVjYXVzZSB0aGUgYm9keSBvZiB0aGUgZWxzZSBjb250YWlucyBvbmx5IGENCj4gc2ltcGxlIGlu
c3RydWN0aW9uLiAgSnVzdCB0byBsZWFybiBmb3IgdGhlIGZ1dHVyZTogd2hhdCdzIHRoZQ0KPiBy
YXRpb25hbGUgZm9yIGFkZGluZyBicmFjZXMgaGVyZSwgYnV0IG5vdCBpbXBvc2luZyBicmFjZXMg
ZXZlcnl3aGVyZQ0KPiBmb3Igc2luZ2xlLWluc3RydWN0aW9uIGJvZGllcz8NCg0KSXQncyBhIGdl
bmVyYWwgc3R5bGUgcmVjb21tZW5kYXRpb24gZm9yIGFsbCBrZXJuZWwgY29kZTogaWYgYnJhY2Vz
IGFyZSB1c2VkDQpmb3Igb25lIHNpZGUgb2YgYW4gaWYtc3RhdGVtZW50LCBhbHNvIHVzZSBicmFj
ZXMgZm9yIHRoZSBvdGhlciBzaWRlLCBhbmQNCmRlZmluaXRlbHkgaWYgdGhhdCBvdGhlciBzaWRl
IGNvbnNpc3RzIG9mIG11bHRpcGxlIGxpbmVzIGR1ZSB0byBhIGNvbW1lbnQuDQoNCkJhcnQu

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@sandisk.com>
To: "paolo.valente@linaro.org" <paolo.valente@linaro.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"fchecconi@gmail.com" <fchecconi@gmail.com>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"avanzini.arianna@gmail.com" <avanzini.arianna@gmail.com>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"tj@kernel.org" <tj@kernel.org>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>
Subject: Re: [PATCH RFC 01/14] block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler
Date: Sat, 18 Mar 2017 15:24:34 +0000	[thread overview]
Message-ID: <1489850658.2339.1.camel@sandisk.com> (raw)
In-Reply-To: <0B1F1CFD-C8AB-442D-B5C6-426D19B1FBA3@linaro.org>

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.

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

  reply	other threads:[~2017-03-18 15:24 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-04 16:01 [PATCH RFC 00/14] Add the BFQ I/O Scheduler to blk-mq Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 01/14] block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler Paolo Valente
2017-03-05 15:16   ` Jens Axboe
2017-03-05 16:02     ` Paolo Valente
2017-03-05 16:02       ` Paolo Valente
2017-03-06 20:46       ` Jens Axboe
2017-03-14 11:28         ` Paolo Valente
2017-03-14 11:28           ` Paolo Valente
2017-03-06 19:40   ` Bart Van Assche
2017-03-06 19:40     ` Bart Van Assche
2017-03-14 14:18     ` Paolo Valente
2017-03-14 14:18       ` Paolo Valente
2017-03-18 12:08     ` Paolo Valente
2017-03-18 12:08       ` Paolo Valente
2017-03-18 15:24       ` Bart Van Assche [this message]
2017-03-18 15:24         ` Bart Van Assche
2017-03-19 11:45         ` Paolo Valente
2017-03-19 11:45           ` Paolo Valente
2017-03-07 23:22   ` Jens Axboe
2017-03-18 12:41     ` Paolo Valente
2017-03-18 12:41       ` Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 02/14] block, bfq: add full hierarchical scheduling and cgroups support Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 03/14] block, bfq: improve throughput boosting Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 04/14] block, bfq: modify the peak-rate estimator Paolo Valente
2017-03-07  0:47   ` Bart Van Assche
2017-03-07  0:47     ` Bart Van Assche
2017-03-04 16:01 ` [PATCH RFC 05/14] block, bfq: add more fairness with writes and slow processes Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 06/14] block, bfq: improve responsiveness Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 07/14] block, bfq: reduce I/O latency for soft real-time applications Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 08/14] block, bfq: preserve a low latency also with NCQ-capable drives Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 09/14] block, bfq: reduce latency during request-pool saturation Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 10/14] block, bfq: add Early Queue Merge (EQM) Paolo Valente
2017-03-07 17:44   ` Jens Axboe
2017-03-15 12:01     ` Paolo Valente
2017-03-15 12:01       ` Paolo Valente
2017-03-15 15:47       ` Jens Axboe
2017-03-15 16:30         ` Jens Axboe
2017-03-15 16:59           ` Paolo Valente
2017-03-15 16:59             ` Paolo Valente
2017-03-15 21:00             ` Jens Axboe
2017-03-18 10:33               ` Paolo Valente
2017-03-18 10:33                 ` Paolo Valente
2017-03-15 16:56   ` Jens Axboe
2017-03-15 17:02     ` Paolo Valente
2017-03-15 17:02       ` Paolo Valente
2017-03-15 21:01       ` Jens Axboe
2017-03-04 16:01 ` [PATCH RFC 11/14] block, bfq: reduce idling only in symmetric scenarios Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 12/14] block, bfq: boost the throughput on NCQ-capable flash-based devices Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 13/14] block, bfq: boost the throughput with random I/O on NCQ-capable HDDs Paolo Valente
2017-03-07  0:54   ` Bart Van Assche
2017-03-07  0:54     ` Bart Van Assche
2017-03-14 14:12     ` Paolo Valente
2017-03-14 14:12       ` Paolo Valente
2017-03-04 16:01 ` [PATCH RFC 14/14] block, bfq: handle bursts of queue activations Paolo Valente
2017-03-06  7:43 ` [PATCH RFC 00/14] Add the BFQ I/O Scheduler to blk-mq Markus Trippelsdorf
2017-03-31 13:27   ` Paolo Valente
2017-03-31 13:27     ` Paolo Valente
2017-03-07  0:22 ` Bart Van Assche
2017-03-07  0:22   ` Bart Van Assche
2017-03-14 14:12   ` Paolo Valente
2017-03-14 14:12     ` Paolo Valente
2017-03-07  1:00 ` Bart Van Assche
2017-03-07  1:00   ` Bart Van Assche
2017-03-14 15:35   ` Paolo Valente
2017-03-14 15:35     ` Paolo Valente
2017-03-14 15:42     ` Jens Axboe
2017-03-14 16:32     ` Bart Van Assche
2017-03-14 16:32       ` Bart Van Assche
2017-03-18 10:52       ` Paolo Valente
2017-03-18 10:52         ` Paolo Valente
2017-03-18 17:09         ` Linus Walleij
2017-03-18 17:46           ` Bart Van Assche
2017-03-18 17:46             ` Bart Van Assche
2017-03-18 20:46             ` Linus Walleij
2017-03-19 12:14             ` Paolo Valente
2017-03-19 12:14               ` Paolo Valente
2017-03-20 18:40             ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1489850658.2339.1.camel@sandisk.com \
    --to=bart.vanassche@sandisk.com \
    --cc=avanzini.arianna@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=broonie@kernel.org \
    --cc=fchecconi@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paolo.valente@linaro.org \
    --cc=tj@kernel.org \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.