* [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations @ 2023-12-11 23:14 Bart Van Assche 2023-12-11 23:31 ` Damien Le Moal 0 siblings, 1 reply; 5+ messages in thread From: Bart Van Assche @ 2023-12-11 23:14 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, Bart Van Assche, Damien Le Moal, Christoph Hellwig If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone originate from different cgroups that could result in different priorities being assigned to these operations. Do not modify the I/O priority of these write operations to prevent that these would be executed in the wrong order when using the mq-deadline I/O scheduler. Cc: Damien Le Moal <dlemoal@kernel.org> Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/blk-ioprio.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c index 4051fada01f1..09ce083a0e3a 100644 --- a/block/blk-ioprio.c +++ b/block/blk-ioprio.c @@ -192,6 +192,17 @@ void blkcg_set_ioprio(struct bio *bio) if (!blkcg || blkcg->prio_policy == POLICY_NO_CHANGE) return; + /* + * If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone + * originate from different cgroups that could result in different + * priorities being assigned to these operations. Do not modify the I/O + * priority of these write operations to prevent that these would be + * executed in the wrong order when using the mq-deadline I/O + * scheduler. + */ + if (bdev_op_is_zoned_write(bio->bi_bdev, bio_op(bio))) + return; + if (blkcg->prio_policy == POLICY_PROMOTE_TO_RT || blkcg->prio_policy == POLICY_NONE_TO_RT) { /* ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations 2023-12-11 23:14 [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations Bart Van Assche @ 2023-12-11 23:31 ` Damien Le Moal 2023-12-12 0:11 ` Bart Van Assche 0 siblings, 1 reply; 5+ messages in thread From: Damien Le Moal @ 2023-12-11 23:31 UTC (permalink / raw) To: Bart Van Assche, Jens Axboe; +Cc: linux-block, Christoph Hellwig On 12/12/23 08:14, Bart Van Assche wrote: > If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone > originate from different cgroups that could result in different > priorities being assigned to these operations. Do not modify the I/O > priority of these write operations to prevent that these would be > executed in the wrong order when using the mq-deadline I/O ...to prevent them from being executed in the wrong... > scheduler. > > Cc: Damien Le Moal <dlemoal@kernel.org> > Cc: Christoph Hellwig <hch@lst.de> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/blk-ioprio.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c > index 4051fada01f1..09ce083a0e3a 100644 > --- a/block/blk-ioprio.c > +++ b/block/blk-ioprio.c > @@ -192,6 +192,17 @@ void blkcg_set_ioprio(struct bio *bio) > if (!blkcg || blkcg->prio_policy == POLICY_NO_CHANGE) > return; > > + /* > + * If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone > + * originate from different cgroups that could result in different > + * priorities being assigned to these operations. Do not modify the I/O > + * priority of these write operations to prevent that these would be > + * executed in the wrong order when using the mq-deadline I/O > + * scheduler. > + */ > + if (bdev_op_is_zoned_write(bio->bi_bdev, bio_op(bio))) Ideally, we want the bio equivalent of blk_rq_is_seq_zoned_write() here so that writes to conventional zones are not affected (these can be reordered). > + return; > + > if (blkcg->prio_policy == POLICY_PROMOTE_TO_RT || > blkcg->prio_policy == POLICY_NONE_TO_RT) { > /* -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations 2023-12-11 23:31 ` Damien Le Moal @ 2023-12-12 0:11 ` Bart Van Assche 2023-12-12 10:08 ` Damien Le Moal 2023-12-12 21:26 ` kernel test robot 0 siblings, 2 replies; 5+ messages in thread From: Bart Van Assche @ 2023-12-12 0:11 UTC (permalink / raw) To: Damien Le Moal, Jens Axboe; +Cc: linux-block, Christoph Hellwig On 12/11/23 15:31, Damien Le Moal wrote: > On 12/12/23 08:14, Bart Van Assche wrote: >> + /* >> + * If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone >> + * originate from different cgroups that could result in different >> + * priorities being assigned to these operations. Do not modify the I/O >> + * priority of these write operations to prevent that these would be >> + * executed in the wrong order when using the mq-deadline I/O >> + * scheduler. >> + */ >> + if (bdev_op_is_zoned_write(bio->bi_bdev, bio_op(bio))) > > Ideally, we want the bio equivalent of blk_rq_is_seq_zoned_write() here so that > writes to conventional zones are not affected (these can be reordered). How about the patch below? Thanks, Bart. [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone originate from different cgroups that could result in different priorities being assigned to these operations. Do not modify the I/O priority of these write operations to prevent them from being executed in the wrong order when using the mq-deadline I/O scheduler. Cc: Damien Le Moal <dlemoal@kernel.org> Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/blk-ioprio.c | 11 +++++++++++ include/linux/blk-mq.h | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c index 4051fada01f1..96b46d34e3d6 100644 --- a/block/blk-ioprio.c +++ b/block/blk-ioprio.c @@ -192,6 +192,17 @@ void blkcg_set_ioprio(struct bio *bio) if (!blkcg || blkcg->prio_policy == POLICY_NO_CHANGE) return; + /* + * If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone + * originate from different cgroups that could result in different + * priorities being assigned to these operations. Do not modify the I/O + * priority of these write operations to prevent that these would be + * executed in the wrong order when using the mq-deadline I/O + * scheduler. + */ + if (blk_bio_is_seq_zoned_write(bio)) + return; + if (blkcg->prio_policy == POLICY_PROMOTE_TO_RT || blkcg->prio_policy == POLICY_NONE_TO_RT) { /* diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index 1ab3081c82ed..90907d9001c0 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -1149,6 +1149,18 @@ static inline unsigned int blk_rq_zone_no(struct request *rq) return disk_zone_no(rq->q->disk, blk_rq_pos(rq)); } +/** + * blk_bio_is_seq_zoned_write() - Check if @bio requires write serialization. + * @bio: Bio to examine. + * + * Note: REQ_OP_ZONE_APPEND bios do not require serialization. + */ +static inline bool blk_bio_is_seq_zoned_write(struct bio *bio) +{ + return op_needs_zoned_write_locking(bio_op(bio)) && + disk_zone_is_seq(bio->bi_disk, bio.bi_iter.bi_sector); +} + static inline unsigned int blk_rq_zone_is_seq(struct request *rq) { return disk_zone_is_seq(rq->q->disk, blk_rq_pos(rq)); @@ -1196,6 +1208,11 @@ static inline bool blk_req_can_dispatch_to_zone(struct request *rq) return !blk_req_zone_is_write_locked(rq); } #else /* CONFIG_BLK_DEV_ZONED */ +static inline bool blk_bio_is_seq_zoned_write(struct bio *bio) +{ + return false; +} + static inline bool blk_rq_is_seq_zoned_write(struct request *rq) { return false; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations 2023-12-12 0:11 ` Bart Van Assche @ 2023-12-12 10:08 ` Damien Le Moal 2023-12-12 21:26 ` kernel test robot 1 sibling, 0 replies; 5+ messages in thread From: Damien Le Moal @ 2023-12-12 10:08 UTC (permalink / raw) To: Bart Van Assche, Jens Axboe; +Cc: linux-block, Christoph Hellwig On 12/12/23 09:11, Bart Van Assche wrote: > On 12/11/23 15:31, Damien Le Moal wrote: >> On 12/12/23 08:14, Bart Van Assche wrote: >>> + /* >>> + * If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone >>> + * originate from different cgroups that could result in different >>> + * priorities being assigned to these operations. Do not modify the I/O >>> + * priority of these write operations to prevent that these would be >>> + * executed in the wrong order when using the mq-deadline I/O >>> + * scheduler. >>> + */ >>> + if (bdev_op_is_zoned_write(bio->bi_bdev, bio_op(bio))) >> >> Ideally, we want the bio equivalent of blk_rq_is_seq_zoned_write() here so that >> writes to conventional zones are not affected (these can be reordered). > How about the patch below? > > Thanks, > > Bart. > > > [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations > > If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone > originate from different cgroups that could result in different priorities > being assigned to these operations. Do not modify the I/O priority of > these write operations to prevent them from being executed in the wrong > order when using the mq-deadline I/O scheduler. > > Cc: Damien Le Moal <dlemoal@kernel.org> > Cc: Christoph Hellwig <hch@lst.de> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/blk-ioprio.c | 11 +++++++++++ > include/linux/blk-mq.h | 17 +++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c > index 4051fada01f1..96b46d34e3d6 100644 > --- a/block/blk-ioprio.c > +++ b/block/blk-ioprio.c > @@ -192,6 +192,17 @@ void blkcg_set_ioprio(struct bio *bio) > if (!blkcg || blkcg->prio_policy == POLICY_NO_CHANGE) > return; > > + /* > + * If REQ_OP_WRITE or REQ_OP_WRITE_ZEROES operations for the same zone > + * originate from different cgroups that could result in different > + * priorities being assigned to these operations. Do not modify the I/O > + * priority of these write operations to prevent that these would be > + * executed in the wrong order when using the mq-deadline I/O > + * scheduler. > + */ > + if (blk_bio_is_seq_zoned_write(bio)) > + return; > + > if (blkcg->prio_policy == POLICY_PROMOTE_TO_RT || > blkcg->prio_policy == POLICY_NONE_TO_RT) { > /* > diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h > index 1ab3081c82ed..90907d9001c0 100644 > --- a/include/linux/blk-mq.h > +++ b/include/linux/blk-mq.h > @@ -1149,6 +1149,18 @@ static inline unsigned int blk_rq_zone_no(struct request *rq) > return disk_zone_no(rq->q->disk, blk_rq_pos(rq)); > } > > +/** > + * blk_bio_is_seq_zoned_write() - Check if @bio requires write serialization. > + * @bio: Bio to examine. > + * > + * Note: REQ_OP_ZONE_APPEND bios do not require serialization. > + */ > +static inline bool blk_bio_is_seq_zoned_write(struct bio *bio) > +{ > + return op_needs_zoned_write_locking(bio_op(bio)) && > + disk_zone_is_seq(bio->bi_disk, bio.bi_iter.bi_sector); Given that disk_zone_is_seq() always return false for regular devices, I think reversing the test order is better: return disk_zone_is_seq(bio->bi_disk, bio.bi_iter.bi_sector) && op_needs_zoned_write_locking(bio_op(bio)); > +} > + > static inline unsigned int blk_rq_zone_is_seq(struct request *rq) > { > return disk_zone_is_seq(rq->q->disk, blk_rq_pos(rq)); > @@ -1196,6 +1208,11 @@ static inline bool blk_req_can_dispatch_to_zone(struct request *rq) > return !blk_req_zone_is_write_locked(rq); > } > #else /* CONFIG_BLK_DEV_ZONED */ > +static inline bool blk_bio_is_seq_zoned_write(struct bio *bio) > +{ > + return false; > +} > + > static inline bool blk_rq_is_seq_zoned_write(struct request *rq) > { > return false; > -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations 2023-12-12 0:11 ` Bart Van Assche 2023-12-12 10:08 ` Damien Le Moal @ 2023-12-12 21:26 ` kernel test robot 1 sibling, 0 replies; 5+ messages in thread From: kernel test robot @ 2023-12-12 21:26 UTC (permalink / raw) To: Bart Van Assche; +Cc: oe-kbuild-all Hi Bart, kernel test robot noticed the following build errors: [auto build test ERROR on axboe-block/for-next] [also build test ERROR on linus/master v6.7-rc5 next-20231212] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Bart-Van-Assche/Re-PATCH-block-blk-ioprio-Skip-zoned-writes-that-are-not-append-operations/20231212-081223 base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next patch link: https://lore.kernel.org/r/d660cc31-a5be-47f2-9fdf-ba4bf5106226%40acm.org patch subject: Re: [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations config: x86_64-rhel-8.3-func (https://download.01.org/0day-ci/archive/20231213/202312130548.cusrlw1s-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231213/202312130548.cusrlw1s-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312130548.cusrlw1s-lkp@intel.com/ All errors (new ones prefixed by >>): In file included from include/linux/t10-pi.h:6, from include/scsi/scsi_cmnd.h:7, from drivers/scsi/scsi.c:61: include/linux/blk-mq.h: In function 'blk_bio_is_seq_zoned_write': >> include/linux/blk-mq.h:1161:37: error: 'struct bio' has no member named 'bi_disk' 1161 | disk_zone_is_seq(bio->bi_disk, bio.bi_iter.bi_sector); | ^~ >> include/linux/blk-mq.h:1161:51: error: 'bio' is a pointer; did you mean to use '->'? 1161 | disk_zone_is_seq(bio->bi_disk, bio.bi_iter.bi_sector); | ^ | -> vim +1161 include/linux/blk-mq.h 1151 1152 /** 1153 * blk_bio_is_seq_zoned_write() - Check if @bio requires write serialization. 1154 * @bio: Bio to examine. 1155 * 1156 * Note: REQ_OP_ZONE_APPEND bios do not require serialization. 1157 */ 1158 static inline bool blk_bio_is_seq_zoned_write(struct bio *bio) 1159 { 1160 return op_needs_zoned_write_locking(bio_op(bio)) && > 1161 disk_zone_is_seq(bio->bi_disk, bio.bi_iter.bi_sector); 1162 } 1163 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-12 21:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-11 23:14 [PATCH] block/blk-ioprio: Skip zoned writes that are not append operations Bart Van Assche 2023-12-11 23:31 ` Damien Le Moal 2023-12-12 0:11 ` Bart Van Assche 2023-12-12 10:08 ` Damien Le Moal 2023-12-12 21:26 ` kernel test robot
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.