diff mbox

dm-zoned: remove test for impossible conditions

Message ID alpine.LRH.2.02.1707211150200.31337@file01.intranet.prod.int.rdu2.redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Mike Snitzer
Headers show

Commit Message

Mikulas Patocka July 21, 2017, 3:56 p.m. UTC 
Hi

Here I send a patch that removes some impossible code from dm-zoned. The 
target also contains large number of GFP_KERNEL allocations, they should 
be changed to GFP_NOIO, if I/O processing could be delayed because of the 
allocation (i.e. anywhere except initialization you should use GFP_NOIO).

Regarding this "if (!nr_sectors && bio_op(bio) != REQ_OP_WRITE)" - could 
this be removed too? Does it really process some zero-sized requests that 
are not flushes? It doesn't seem so.

Mikulas


From: Mikulas Patocka <mpatocka@redhat.com>

The value REQ_OP_FLUSH is used by the block code in the request structure. 
It is not used in the bio structure.

This patch removes the tests for REQ_OP_FLUSH from dm-zoned-target.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-zoned-target.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Comments

Damien Le Moal July 24, 2017, 6:35 a.m. UTC | #1 
On 7/22/17 00:56, Mikulas Patocka wrote:
> Hi
> 
> Here I send a patch that removes some impossible code from dm-zoned. The 
> target also contains large number of GFP_KERNEL allocations, they should 
> be changed to GFP_NOIO, if I/O processing could be delayed because of the 
> allocation (i.e. anywhere except initialization you should use GFP_NOIO).
> 
> Regarding this "if (!nr_sectors && bio_op(bio) != REQ_OP_WRITE)" - could 
> this be removed too? Does it really process some zero-sized requests that 
> are not flushes? It doesn't seem so.
> 
> Mikulas
> 
> 
> From: Mikulas Patocka <mpatocka@redhat.com>
> 
> The value REQ_OP_FLUSH is used by the block code in the request structure. 
> It is not used in the bio structure.
> 
> This patch removes the tests for REQ_OP_FLUSH from dm-zoned-target.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  drivers/md/dm-zoned-target.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/md/dm-zoned-target.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-zoned-target.c
> +++ linux-2.6/drivers/md/dm-zoned-target.c
> @@ -588,7 +588,7 @@ static int dmz_map(struct dm_target *ti,
>  
>  	bio->bi_bdev = dev->bdev;
>  
> -	if (!nr_sectors && (bio_op(bio) != REQ_OP_FLUSH) && (bio_op(bio) != REQ_OP_WRITE))
> +	if (!nr_sectors && bio_op(bio) != REQ_OP_WRITE)
>  		return DM_MAPIO_REMAPPED;
>  
>  	/* The BIO should be block aligned */
> @@ -603,7 +603,7 @@ static int dmz_map(struct dm_target *ti,
>  	bioctx->status = BLK_STS_OK;
>  
>  	/* Set the BIO pending in the flush list */
> -	if (bio_op(bio) == REQ_OP_FLUSH || (!nr_sectors && bio_op(bio) == REQ_OP_WRITE)) {
> +	if (!nr_sectors && bio_op(bio) == REQ_OP_WRITE) {
>  		spin_lock(&dmz->flush_lock);
>  		bio_list_add(&dmz->flush_list, bio);
>  		spin_unlock(&dmz->flush_lock);
> 

Good catch !

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
diff mbox

Patch

Index: linux-2.6/drivers/md/dm-zoned-target.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-zoned-target.c
+++ linux-2.6/drivers/md/dm-zoned-target.c
@@ -588,7 +588,7 @@  static int dmz_map(struct dm_target *ti,
 
 	bio->bi_bdev = dev->bdev;
 
-	if (!nr_sectors && (bio_op(bio) != REQ_OP_FLUSH) && (bio_op(bio) != REQ_OP_WRITE))
+	if (!nr_sectors && bio_op(bio) != REQ_OP_WRITE)
 		return DM_MAPIO_REMAPPED;
 
 	/* The BIO should be block aligned */
@@ -603,7 +603,7 @@  static int dmz_map(struct dm_target *ti,
 	bioctx->status = BLK_STS_OK;
 
 	/* Set the BIO pending in the flush list */
-	if (bio_op(bio) == REQ_OP_FLUSH || (!nr_sectors && bio_op(bio) == REQ_OP_WRITE)) {
+	if (!nr_sectors && bio_op(bio) == REQ_OP_WRITE) {
 		spin_lock(&dmz->flush_lock);
 		bio_list_add(&dmz->flush_list, bio);
 		spin_unlock(&dmz->flush_lock);