| Message ID | 20190227062155.19732-1-yuyufen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: add a new flag BLK_MQ_POLL_HYBRID_INVALID for hybrid poll | expand |
Hello Yufen, On 2/27/19 3:21 AM, Yufen Yu wrote: > For q->poll_nsec == -1, means using classic poll, not hybrid poll. > We introduce a new flag BLK_POLL_DELAY_INVALID to replace -1, which > may make code much easier to read. In the commit message you say BLK_POLL_DELAY_INVALID, but the code and the commit title use BLK_MQ_POLL_HYBRID_INVALID. Is this a typo? Thanks. > > Signed-off-by: Yufen Yu <yuyufen@huawei.com> > --- > block/blk-mq.c | 4 ++-- > block/blk-sysfs.c | 4 ++-- > include/linux/blkdev.h | 3 +++ > 3 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 9437a5eb07cf..8df9f6e97d2e 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -2857,7 +2857,7 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set, > /* > * Default to classic polling > */ > - q->poll_nsec = -1; > + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; > > blk_mq_init_cpu_queues(q, set->nr_hw_queues); > blk_mq_add_queue_tag_set(set, q); > @@ -3389,7 +3389,7 @@ static bool blk_mq_poll_hybrid(struct request_queue *q, > { > struct request *rq; > > - if (q->poll_nsec == -1) > + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) > return false; > > if (!blk_qc_t_is_internal(cookie)) > diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c > index 590d1ef2f961..8775b898f96d 100644 > --- a/block/blk-sysfs.c > +++ b/block/blk-sysfs.c > @@ -360,7 +360,7 @@ static ssize_t queue_poll_delay_show(struct request_queue *q, char *page) > { > int val; > > - if (q->poll_nsec == -1) > + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) > val = -1; > else > val = q->poll_nsec / 1000; > @@ -381,7 +381,7 @@ static ssize_t queue_poll_delay_store(struct request_queue *q, const char *page, > return err; > > if (val == -1) > - q->poll_nsec = -1; > + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; > else > q->poll_nsec = val * 1000; > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index 338604dff7d0..2102447b67d9 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -50,6 +50,9 @@ struct blk_stat_callback; > /* Must be consistent with blk_mq_poll_stats_bkt() */ > #define BLK_MQ_POLL_STATS_BKTS 16 > > +/* Using default classic polling */ > +#define BLK_MQ_POLL_HYBRID_INVALID -1 > + > /* > * Maximum number of blkcg policies allowed to be registered concurrently. > * Defined here to simplify include dependency. >
On 2019/2/27 18:27, Marcos Paulo de Souza wrote: > Hello Yufen, > > On 2/27/19 3:21 AM, Yufen Yu wrote: >> For q->poll_nsec == -1, means using classic poll, not hybrid poll. >> We introduce a new flag BLK_POLL_DELAY_INVALID to replace -1, which >> may make code much easier to read. > > In the commit message you say BLK_POLL_DELAY_INVALID, but the code and > the commit title use BLK_MQ_POLL_HYBRID_INVALID. Is this a typo? > Thanks a lot for pointing out this fault. What I want to say is BLK_MQ_POLL_HYBRID_INVALID, not BLK_POLL_DELAY_INVALID. Yufen Thanks > Thanks. > >> >> Signed-off-by: Yufen Yu <yuyufen@huawei.com> >> --- >> block/blk-mq.c | 4 ++-- >> block/blk-sysfs.c | 4 ++-- >> include/linux/blkdev.h | 3 +++ >> 3 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/block/blk-mq.c b/block/blk-mq.c >> index 9437a5eb07cf..8df9f6e97d2e 100644 >> --- a/block/blk-mq.c >> +++ b/block/blk-mq.c >> @@ -2857,7 +2857,7 @@ struct request_queue >> *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set, >> /* >> * Default to classic polling >> */ >> - q->poll_nsec = -1; >> + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; >> blk_mq_init_cpu_queues(q, set->nr_hw_queues); >> blk_mq_add_queue_tag_set(set, q); >> @@ -3389,7 +3389,7 @@ static bool blk_mq_poll_hybrid(struct >> request_queue *q, >> { >> struct request *rq; >> - if (q->poll_nsec == -1) >> + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) >> return false; >> if (!blk_qc_t_is_internal(cookie)) >> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c >> index 590d1ef2f961..8775b898f96d 100644 >> --- a/block/blk-sysfs.c >> +++ b/block/blk-sysfs.c >> @@ -360,7 +360,7 @@ static ssize_t queue_poll_delay_show(struct >> request_queue *q, char *page) >> { >> int val; >> - if (q->poll_nsec == -1) >> + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) >> val = -1; >> else >> val = q->poll_nsec / 1000; >> @@ -381,7 +381,7 @@ static ssize_t queue_poll_delay_store(struct >> request_queue *q, const char *page, >> return err; >> if (val == -1) >> - q->poll_nsec = -1; >> + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; >> else >> q->poll_nsec = val * 1000; >> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h >> index 338604dff7d0..2102447b67d9 100644 >> --- a/include/linux/blkdev.h >> +++ b/include/linux/blkdev.h >> @@ -50,6 +50,9 @@ struct blk_stat_callback; >> /* Must be consistent with blk_mq_poll_stats_bkt() */ >> #define BLK_MQ_POLL_STATS_BKTS 16 >> +/* Using default classic polling */ >> +#define BLK_MQ_POLL_HYBRID_INVALID -1 >> + >> /* >> * Maximum number of blkcg policies allowed to be registered >> concurrently. >> * Defined here to simplify include dependency. >> > > . >
On 2019/02/27 5:38, yuyufen wrote: > > On 2019/2/27 18:27, Marcos Paulo de Souza wrote: >> Hello Yufen, >> >> On 2/27/19 3:21 AM, Yufen Yu wrote: >>> For q->poll_nsec == -1, means using classic poll, not hybrid poll. >>> We introduce a new flag BLK_POLL_DELAY_INVALID to replace -1, which >>> may make code much easier to read. >> >> In the commit message you say BLK_POLL_DELAY_INVALID, but the code and >> the commit title use BLK_MQ_POLL_HYBRID_INVALID. Is this a typo? >> > > Thanks a lot for pointing out this fault. What I want to > say is BLK_MQ_POLL_HYBRID_INVALID, not BLK_POLL_DELAY_INVALID. > > Yufen > Thanks > >> Thanks. >> >>> >>> Signed-off-by: Yufen Yu <yuyufen@huawei.com> >>> --- >>> block/blk-mq.c | 4 ++-- >>> block/blk-sysfs.c | 4 ++-- >>> include/linux/blkdev.h | 3 +++ >>> 3 files changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/block/blk-mq.c b/block/blk-mq.c >>> index 9437a5eb07cf..8df9f6e97d2e 100644 >>> --- a/block/blk-mq.c >>> +++ b/block/blk-mq.c >>> @@ -2857,7 +2857,7 @@ struct request_queue >>> *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set, >>> /* >>> * Default to classic polling >>> */ >>> - q->poll_nsec = -1; >>> + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; >>> blk_mq_init_cpu_queues(q, set->nr_hw_queues); >>> blk_mq_add_queue_tag_set(set, q); >>> @@ -3389,7 +3389,7 @@ static bool blk_mq_poll_hybrid(struct >>> request_queue *q, >>> { >>> struct request *rq; >>> - if (q->poll_nsec == -1) >>> + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) >>> return false; >>> if (!blk_qc_t_is_internal(cookie)) >>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c >>> index 590d1ef2f961..8775b898f96d 100644 >>> --- a/block/blk-sysfs.c >>> +++ b/block/blk-sysfs.c >>> @@ -360,7 +360,7 @@ static ssize_t queue_poll_delay_show(struct >>> request_queue *q, char *page) >>> { >>> int val; >>> - if (q->poll_nsec == -1) >>> + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) >>> val = -1; >>> else >>> val = q->poll_nsec / 1000; >>> @@ -381,7 +381,7 @@ static ssize_t queue_poll_delay_store(struct >>> request_queue *q, const char *page, >>> return err; >>> if (val == -1) >>> - q->poll_nsec = -1; >>> + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; >>> else >>> q->poll_nsec = val * 1000; >>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h >>> index 338604dff7d0..2102447b67d9 100644 >>> --- a/include/linux/blkdev.h >>> +++ b/include/linux/blkdev.h >>> @@ -50,6 +50,9 @@ struct blk_stat_callback; >>> /* Must be consistent with blk_mq_poll_stats_bkt() */ >>> #define BLK_MQ_POLL_STATS_BKTS 16 >>> +/* Using default classic polling */ >>> +#define BLK_MQ_POLL_HYBRID_INVALID -1 May be simply renaming this as BLK_MQ_POLL_CLASSIC would be even simpler ? That matches the comment and also the actual use of the macro, i.e.: if (q->poll_nsec == -1) means "do classic polling", which would nicely fits with: if (q->poll_nsec == BLK_MQ_POLL_CLASSIC) A lot easier to understand in my opinion. Using BLK_MQ_POLL_HYBRID_INVALID as a name is confusing with potential error cases where we actually want to do hybrid polling but the poll sleep time is invalid. >>> + >>> /* >>> * Maximum number of blkcg policies allowed to be registered >>> concurrently. >>> * Defined here to simplify include dependency. >>> >> >> . >> > > >
On 2019/2/27 20:45, Damien Le Moal wrote: > On 2019/02/27 5:38, yuyufen wrote: >> On 2019/2/27 18:27, Marcos Paulo de Souza wrote: >>> Hello Yufen, >>> >>> On 2/27/19 3:21 AM, Yufen Yu wrote: >>>> For q->poll_nsec == -1, means using classic poll, not hybrid poll. >>>> We introduce a new flag BLK_POLL_DELAY_INVALID to replace -1, which >>>> may make code much easier to read. >>> In the commit message you say BLK_POLL_DELAY_INVALID, but the code and >>> the commit title use BLK_MQ_POLL_HYBRID_INVALID. Is this a typo? >>> >> Thanks a lot for pointing out this fault. What I want to >> say is BLK_MQ_POLL_HYBRID_INVALID, not BLK_POLL_DELAY_INVALID. >> >> Yufen >> Thanks >> >>> Thanks. >>> >>>> Signed-off-by: Yufen Yu <yuyufen@huawei.com> >>>> --- >>>> block/blk-mq.c | 4 ++-- >>>> block/blk-sysfs.c | 4 ++-- >>>> include/linux/blkdev.h | 3 +++ >>>> 3 files changed, 7 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/block/blk-mq.c b/block/blk-mq.c >>>> index 9437a5eb07cf..8df9f6e97d2e 100644 >>>> --- a/block/blk-mq.c >>>> +++ b/block/blk-mq.c >>>> @@ -2857,7 +2857,7 @@ struct request_queue >>>> *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set, >>>> /* >>>> * Default to classic polling >>>> */ >>>> - q->poll_nsec = -1; >>>> + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; >>>> blk_mq_init_cpu_queues(q, set->nr_hw_queues); >>>> blk_mq_add_queue_tag_set(set, q); >>>> @@ -3389,7 +3389,7 @@ static bool blk_mq_poll_hybrid(struct >>>> request_queue *q, >>>> { >>>> struct request *rq; >>>> - if (q->poll_nsec == -1) >>>> + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) >>>> return false; >>>> if (!blk_qc_t_is_internal(cookie)) >>>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c >>>> index 590d1ef2f961..8775b898f96d 100644 >>>> --- a/block/blk-sysfs.c >>>> +++ b/block/blk-sysfs.c >>>> @@ -360,7 +360,7 @@ static ssize_t queue_poll_delay_show(struct >>>> request_queue *q, char *page) >>>> { >>>> int val; >>>> - if (q->poll_nsec == -1) >>>> + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) >>>> val = -1; >>>> else >>>> val = q->poll_nsec / 1000; >>>> @@ -381,7 +381,7 @@ static ssize_t queue_poll_delay_store(struct >>>> request_queue *q, const char *page, >>>> return err; >>>> if (val == -1) >>>> - q->poll_nsec = -1; >>>> + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; >>>> else >>>> q->poll_nsec = val * 1000; >>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h >>>> index 338604dff7d0..2102447b67d9 100644 >>>> --- a/include/linux/blkdev.h >>>> +++ b/include/linux/blkdev.h >>>> @@ -50,6 +50,9 @@ struct blk_stat_callback; >>>> /* Must be consistent with blk_mq_poll_stats_bkt() */ >>>> #define BLK_MQ_POLL_STATS_BKTS 16 >>>> +/* Using default classic polling */ >>>> +#define BLK_MQ_POLL_HYBRID_INVALID -1 > May be simply renaming this as BLK_MQ_POLL_CLASSIC would be even simpler ? > That matches the comment and also the actual use of the macro, i.e.: > > if (q->poll_nsec == -1) > > means "do classic polling", which would nicely fits with: > > if (q->poll_nsec == BLK_MQ_POLL_CLASSIC) > > A lot easier to understand in my opinion. Using BLK_MQ_POLL_HYBRID_INVALID as a > name is confusing with potential error cases where we actually want to do hybrid > polling but the poll sleep time is invalid. I think your suggestion is good. Thanks a lot. Yufen thanks >>>> + >>>> /* >>>> * Maximum number of blkcg policies allowed to be registered >>>> concurrently. >>>> * Defined here to simplify include dependency. >>>> >>> . >>> >> >> >
diff --git a/block/blk-mq.c b/block/blk-mq.c index 9437a5eb07cf..8df9f6e97d2e 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2857,7 +2857,7 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set, /* * Default to classic polling */ - q->poll_nsec = -1; + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; blk_mq_init_cpu_queues(q, set->nr_hw_queues); blk_mq_add_queue_tag_set(set, q); @@ -3389,7 +3389,7 @@ static bool blk_mq_poll_hybrid(struct request_queue *q, { struct request *rq; - if (q->poll_nsec == -1) + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) return false; if (!blk_qc_t_is_internal(cookie)) diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 590d1ef2f961..8775b898f96d 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -360,7 +360,7 @@ static ssize_t queue_poll_delay_show(struct request_queue *q, char *page) { int val; - if (q->poll_nsec == -1) + if (q->poll_nsec == BLK_MQ_POLL_HYBRID_INVALID) val = -1; else val = q->poll_nsec / 1000; @@ -381,7 +381,7 @@ static ssize_t queue_poll_delay_store(struct request_queue *q, const char *page, return err; if (val == -1) - q->poll_nsec = -1; + q->poll_nsec = BLK_MQ_POLL_HYBRID_INVALID; else q->poll_nsec = val * 1000; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 338604dff7d0..2102447b67d9 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -50,6 +50,9 @@ struct blk_stat_callback; /* Must be consistent with blk_mq_poll_stats_bkt() */ #define BLK_MQ_POLL_STATS_BKTS 16 +/* Using default classic polling */ +#define BLK_MQ_POLL_HYBRID_INVALID -1 + /* * Maximum number of blkcg policies allowed to be registered concurrently. * Defined here to simplify include dependency.
For q->poll_nsec == -1, means using classic poll, not hybrid poll. We introduce a new flag BLK_POLL_DELAY_INVALID to replace -1, which may make code much easier to read. Signed-off-by: Yufen Yu <yuyufen@huawei.com> --- block/blk-mq.c | 4 ++-- block/blk-sysfs.c | 4 ++-- include/linux/blkdev.h | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-)