[PATCH V2 0/2] block: allow zone_mgmt_ops to bail out on SIGKILL

[PATCH V2 0/2] block: allow zone_mgmt_ops to bail out on SIGKILL

[Chaitanya Kulkarni]

Hi Jens,

This is a small patch-series which allows zone-mgmt ops to handle
SIGKILL. This patch series formatted on the top of following patch:-
https://marc.info/?l=linux-block&m=157321402002207&w=2.

* Changes from V1:-

1. Export blk_should abort() to avoid code duplication.
2. Reuse blk_should_abort() and return -EINTR when zone-mgmt
   ops is interrupted when last submit_bio_wait() is successful.

In case you want me to add original patch posted by Tetsuo Honda,
I'll be happy to respin this series so we can have everything
in one patch-series.

Regards,
Chaitanya

In case someone is interested :-
Without this patch :-

# blkzone reset -o 0 -c 1000 /dev/nullb0 
^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C

[  174.115065] null_blk: null_zone_mgmt 163 zoneid 993
[  174.125071] null_blk: null_zone_mgmt 163 zoneid 994
[  174.135076] null_blk: null_zone_mgmt 163 zoneid 995
[  174.145082] null_blk: null_zone_mgmt 163 zoneid 996
[  174.155087] null_blk: null_zone_mgmt 163 zoneid 997
[  174.165091] null_blk: null_zone_mgmt 163 zoneid 998
[  174.175096] null_blk: null_zone_mgmt 163 zoneid 999

With this patch :-
# blkzone reset -o 0 -c 1000 /dev/nullb0
^C

[  211.889379] null_blk: null_zone_mgmt 163 zoneid 191
[  211.899420] null_blk: null_zone_mgmt 163 zoneid 192
[  211.909424] null_blk: null_zone_mgmt 163 zoneid 193

Chaitanya Kulkarni (2):
  block: export blk_should_abort()
  block: allow zone_mgmt_ops to bail out on SIGKILL

 block/blk-lib.c   | 3 ++-
 block/blk-zoned.c | 5 ++++-
 block/blk.h       | 2 ++
 3 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.22.1

[PATCH V2 1/2] block: export blk_should_abort()

[Chaitanya Kulkarni]

This patch exports blk_should_abort() function to avoid dulicate code.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/blk-lib.c | 3 ++-
 block/blk.h     | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 6ca7cae62876..c0afddb2a67b 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -11,7 +11,7 @@
 
 #include "blk.h"
 
-static int blk_should_abort(struct bio *bio)
+int blk_should_abort(struct bio *bio)
 {
 	int ret;
 
@@ -22,6 +22,7 @@ static int blk_should_abort(struct bio *bio)
 	bio_put(bio);
 	return ret ? ret : -EINTR;
 }
+EXPORT_SYMBOL(blk_should_abort);
 
 struct bio *blk_next_bio(struct bio *bio, unsigned int nr_pages, gfp_t gfp)
 {
diff --git a/block/blk.h b/block/blk.h
index 2bea40180b6f..63fa4694d333 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -297,6 +297,8 @@ static inline struct io_context *create_io_context(gfp_t gfp_mask, int node)
 	return current->io_context;
 }
 
+int blk_should_abort(struct bio *bio);
+
 /*
  * Internal throttling interface
  */
-- 
2.22.1

[PATCH V2 2/2] block: allow zone_mgmt_ops to bail out on SIGKILL

[Chaitanya Kulkarni]

This patch is on the similar concept which is posted earlier:-
https://marc.info/?l=linux-block&m=157321402002207&w=2.

This allows zone-mgmt ops to handle SIGKILL.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/blk-zoned.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 481eaf7d04d4..8f0f740d89e8 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -286,12 +286,15 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
 		sector += zone_sectors;
 
 		/* This may take a while, so be nice to others */
-		cond_resched();
+		ret = blk_should_abort(bio);
+		if (ret)
+			goto out;
 	}
 
 	ret = submit_bio_wait(bio);
 	bio_put(bio);
 
+out:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(blkdev_zone_mgmt);
-- 
2.22.1

Re: [PATCH V2 1/2] block: export blk_should_abort()

[Damien Le Moal]

On 2019/11/12 6:18, Chaitanya Kulkarni wrote:
> This patch exports blk_should_abort() function to avoid dulicate code.

s/dulicate/duplicate

And why export this symbol ? It is not used in kernel modules so I do
not see the need for it.

In any case, the export should be EXPORT_SYMBOL_GPL().

> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  block/blk-lib.c | 3 ++-
>  block/blk.h     | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 6ca7cae62876..c0afddb2a67b 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -11,7 +11,7 @@
>  
>  #include "blk.h"
>  
> -static int blk_should_abort(struct bio *bio)
> +int blk_should_abort(struct bio *bio)
>  {
>  	int ret;
>  
> @@ -22,6 +22,7 @@ static int blk_should_abort(struct bio *bio)
>  	bio_put(bio);
>  	return ret ? ret : -EINTR;
>  }
> +EXPORT_SYMBOL(blk_should_abort);
>  
>  struct bio *blk_next_bio(struct bio *bio, unsigned int nr_pages, gfp_t gfp)
>  {
> diff --git a/block/blk.h b/block/blk.h
> index 2bea40180b6f..63fa4694d333 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -297,6 +297,8 @@ static inline struct io_context *create_io_context(gfp_t gfp_mask, int node)
>  	return current->io_context;
>  }
>  
> +int blk_should_abort(struct bio *bio);
> +
>  /*
>   * Internal throttling interface
>   */
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 2/2] block: allow zone_mgmt_ops to bail out on SIGKILL

[Damien Le Moal]

On 2019/11/12 6:18, Chaitanya Kulkarni wrote:
> This patch is on the similar concept which is posted earlier:-
> https://marc.info/?l=linux-block&m=157321402002207&w=2.

May be reference a commit ID here instead of an URL ?

> 
> This allows zone-mgmt ops to handle SIGKILL.
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  block/blk-zoned.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 481eaf7d04d4..8f0f740d89e8 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -286,12 +286,15 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
>  		sector += zone_sectors;
>  
>  		/* This may take a while, so be nice to others */
> -		cond_resched();
> +		ret = blk_should_abort(bio);
> +		if (ret)
> +			goto out;

There is no need for the goto here. You can return directly.

>  	}
>  
>  	ret = submit_bio_wait(bio);
>  	bio_put(bio);
>  
> +out:
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(blkdev_zone_mgmt);
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 2/2] block: allow zone_mgmt_ops to bail out on SIGKILL

[Chaitanya Kulkarni]

On 11/11/2019 04:04 PM, Damien Le Moal wrote:
> On 2019/11/12 6:18, Chaitanya Kulkarni wrote:
>> This patch is on the similar concept which is posted earlier:-
>> https://marc.info/?l=linux-block&m=157321402002207&w=2.
>
> May be reference a commit ID here instead of an URL ?
>
With reference to the cover-letter  I couldn't find this commit in
the repo (on for-next branch), waiting for Jens to respond if he wants
me to bundle these patches with the original in one series.

>>
>> This allows zone-mgmt ops to handle SIGKILL.
>>
>> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
>> ---
>>   block/blk-zoned.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
>> index 481eaf7d04d4..8f0f740d89e8 100644
>> --- a/block/blk-zoned.c
>> +++ b/block/blk-zoned.c
>> @@ -286,12 +286,15 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
>>   		sector += zone_sectors;
>>
>>   		/* This may take a while, so be nice to others */
>> -		cond_resched();
>> +		ret = blk_should_abort(bio);
>> +		if (ret)
>> +			goto out;
>
> There is no need for the goto here. You can return directly.
>
Okay.
>>   	}
>>
>>   	ret = submit_bio_wait(bio);
>>   	bio_put(bio);
>>
>> +out:
>>   	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(blkdev_zone_mgmt);
>>
>
>

Re: [PATCH V2 1/2] block: export blk_should_abort()

[Chaitanya Kulkarni]

On 11/11/2019 04:01 PM, Damien Le Moal wrote:
> On 2019/11/12 6:18, Chaitanya Kulkarni wrote:
>> >This patch exports blk_should_abort() function to avoid dulicate code.
> s/dulicate/duplicate
>

> And why export this symbol ? It is not used in kernel modules so I do
> not see the need for it.
Yes we don't have any user for this function outside of block layer yet.
I'll remove this in next version.
>
> In any case, the export should be EXPORT_SYMBOL_GPL().
blk-lib.c has EXPORT_SYMBOL() so I kept the same pattern.
I'll update this in the next version.
>

Re: [PATCH V2 2/2] block: allow zone_mgmt_ops to bail out on SIGKILL

[Jens Axboe]

On 11/11/19 8:53 PM, Chaitanya Kulkarni wrote:
> On 11/11/2019 04:04 PM, Damien Le Moal wrote:
>> On 2019/11/12 6:18, Chaitanya Kulkarni wrote:
>>> This patch is on the similar concept which is posted earlier:-
>>> https://marc.info/?l=linux-block&m=157321402002207&w=2.
>>
>> May be reference a commit ID here instead of an URL ?
>>
> With reference to the cover-letter  I couldn't find this commit in
> the repo (on for-next branch), waiting for Jens to respond if he wants
> me to bundle these patches with the original in one series.

It's not added yet, there was still discussion around it. Someone needs
to followup on that one.

But in general, don't make commit messages just a link, that's not
a good commit message.

-- 
Jens Axboe