diff mbox series

[v16,06/26] blk-zoned: Fix requeuing of zoned writes

Message ID 20241119002815.600608-7-bvanassche@acm.org (mailing list archive)
State New
Headers show
Series Improve write performance for zoned UFS devices | expand

Commit Message

Bart Van Assche Nov. 19, 2024, 12:27 a.m. UTC
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(-)

Comments

Damien Le Moal Nov. 19, 2024, 3 a.m. UTC | #1
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)) {
Bart Van Assche Nov. 19, 2024, 9:06 p.m. UTC | #2
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 mbox series

Patch

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)) {