| Message ID | 20241119002815.600608-7-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Improve write performance for zoned UFS devices | expand |
On 11/19/24 9:27 AM, Bart Van Assche wrote: > Make sure that unaligned writes are sent to the block driver. This > allows the block driver to limit the number of retries of unaligned > writes. Remove disk_zone_wplug_abort_unaligned() because it only examines > the bio plug list. Pending writes can occur either on the bio plug list > or on the request queue requeue list. There can be only one write in flight, so at most one write in the requeue list... And if that write was requeued, it means that it was not even started so is not failed yet. So not sure what this is all about. Am I missing something ? > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/blk-zoned.c | 36 ------------------------------------ > 1 file changed, 36 deletions(-) > > diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index 284820b29285..ded38fa9ae3d 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -577,33 +577,6 @@ static void disk_zone_wplug_abort(struct blk_zone_wplug *zwplug) > blk_zone_wplug_bio_io_error(zwplug, bio); > } > > -/* > - * Abort (fail) all plugged BIOs of a zone write plug that are not aligned > - * with the assumed write pointer location of the zone when the BIO will > - * be unplugged. > - */ > -static void disk_zone_wplug_abort_unaligned(struct gendisk *disk, > - struct blk_zone_wplug *zwplug) > -{ > - unsigned int wp_offset = zwplug->wp_offset; > - struct bio_list bl = BIO_EMPTY_LIST; > - struct bio *bio; > - > - while ((bio = bio_list_pop(&zwplug->bio_list))) { > - if (disk_zone_is_full(disk, zwplug->zone_no, wp_offset) || > - (bio_op(bio) != REQ_OP_ZONE_APPEND && > - bio_offset_from_zone_start(bio) != wp_offset)) { > - blk_zone_wplug_bio_io_error(zwplug, bio); > - continue; > - } > - > - wp_offset += bio_sectors(bio); > - bio_list_add(&bl, bio); > - } > - > - bio_list_merge(&zwplug->bio_list, &bl); > -} > - > static inline void disk_zone_wplug_set_error(struct gendisk *disk, > struct blk_zone_wplug *zwplug) > { > @@ -982,14 +955,6 @@ static bool blk_zone_wplug_prepare_bio(struct blk_zone_wplug *zwplug, > * so that we can restore its operation code on completion. > */ > bio_set_flag(bio, BIO_EMULATES_ZONE_APPEND); > - } else { > - /* > - * Check for non-sequential writes early because we avoid a > - * whole lot of error handling trouble if we don't send it off > - * to the driver. > - */ > - if (bio_offset_from_zone_start(bio) != zwplug->wp_offset) > - goto err; > } > > /* Advance the zone write pointer offset. */ > @@ -1425,7 +1390,6 @@ static void disk_zone_wplug_handle_error(struct gendisk *disk, > > /* Update the zone write pointer offset. */ > zwplug->wp_offset = zwplug->wp_offset_compl; > - disk_zone_wplug_abort_unaligned(disk, zwplug); > > /* Restart BIO submission if we still have any BIO left. */ > if (!bio_list_empty(&zwplug->bio_list)) {
On 11/18/24 7:00 PM, Damien Le Moal wrote: > On 11/19/24 9:27 AM, Bart Van Assche wrote: >> Make sure that unaligned writes are sent to the block driver. This >> allows the block driver to limit the number of retries of unaligned >> writes. Remove disk_zone_wplug_abort_unaligned() because it only examines >> the bio plug list. Pending writes can occur either on the bio plug list >> or on the request queue requeue list. > > There can be only one write in flight, so at most one write in the requeue > list... And if that write was requeued, it means that it was not even started > so is not failed yet. So not sure what this is all about. > > Am I missing something ? Patch 20/26 in this series enables support for multiple in-flight zoned writes per zone. Thanks, Bart.
diff --git a/block/blk-zoned.c b/block/blk-zoned.c index 284820b29285..ded38fa9ae3d 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -577,33 +577,6 @@ static void disk_zone_wplug_abort(struct blk_zone_wplug *zwplug) blk_zone_wplug_bio_io_error(zwplug, bio); } -/* - * Abort (fail) all plugged BIOs of a zone write plug that are not aligned - * with the assumed write pointer location of the zone when the BIO will - * be unplugged. - */ -static void disk_zone_wplug_abort_unaligned(struct gendisk *disk, - struct blk_zone_wplug *zwplug) -{ - unsigned int wp_offset = zwplug->wp_offset; - struct bio_list bl = BIO_EMPTY_LIST; - struct bio *bio; - - while ((bio = bio_list_pop(&zwplug->bio_list))) { - if (disk_zone_is_full(disk, zwplug->zone_no, wp_offset) || - (bio_op(bio) != REQ_OP_ZONE_APPEND && - bio_offset_from_zone_start(bio) != wp_offset)) { - blk_zone_wplug_bio_io_error(zwplug, bio); - continue; - } - - wp_offset += bio_sectors(bio); - bio_list_add(&bl, bio); - } - - bio_list_merge(&zwplug->bio_list, &bl); -} - static inline void disk_zone_wplug_set_error(struct gendisk *disk, struct blk_zone_wplug *zwplug) { @@ -982,14 +955,6 @@ static bool blk_zone_wplug_prepare_bio(struct blk_zone_wplug *zwplug, * so that we can restore its operation code on completion. */ bio_set_flag(bio, BIO_EMULATES_ZONE_APPEND); - } else { - /* - * Check for non-sequential writes early because we avoid a - * whole lot of error handling trouble if we don't send it off - * to the driver. - */ - if (bio_offset_from_zone_start(bio) != zwplug->wp_offset) - goto err; } /* Advance the zone write pointer offset. */ @@ -1425,7 +1390,6 @@ static void disk_zone_wplug_handle_error(struct gendisk *disk, /* Update the zone write pointer offset. */ zwplug->wp_offset = zwplug->wp_offset_compl; - disk_zone_wplug_abort_unaligned(disk, zwplug); /* Restart BIO submission if we still have any BIO left. */ if (!bio_list_empty(&zwplug->bio_list)) {
Make sure that unaligned writes are sent to the block driver. This allows the block driver to limit the number of retries of unaligned writes. Remove disk_zone_wplug_abort_unaligned() because it only examines the bio plug list. Pending writes can occur either on the bio plug list or on the request queue requeue list. Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/blk-zoned.c | 36 ------------------------------------ 1 file changed, 36 deletions(-)