[PATCH] block: soft limit zone-append sectors as well

[PATCH] block: soft limit zone-append sectors as well

[Johannes Thumshirn]

Martin rightfully noted that for normal filesystem IO we have soft limits
in place, to prevent them from getting too big and not lead to
unpredictable latencies. For zone append we only have the hardware limit
in place.

Cap the max sectors we submit via zone-append to the maximal number of
sectors if the second limit is lower.

Link: https://lore.kernel.org/linux-btrfs/yq1k0w8g3rw.fsf@ca-mkp.ca.oracle.com
Reported-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 include/linux/blkdev.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index cf80e61b4c5e..967cd76f16d4 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1406,7 +1406,10 @@ static inline unsigned int queue_max_segment_size(const struct request_queue *q)
 
 static inline unsigned int queue_max_zone_append_sectors(const struct request_queue *q)
 {
-	return q->limits.max_zone_append_sectors;
+
+	struct queue_limits *l = q->limits;
+
+	return min(l->max_zone_append_sectors, l->max_sectors);
 }
 
 static inline unsigned queue_logical_block_size(const struct request_queue *q)
-- 
2.26.2

Re: [PATCH] block: soft limit zone-append sectors as well

[Damien Le Moal]

On 2020/10/07 18:20, Johannes Thumshirn wrote:
> Martin rightfully noted that for normal filesystem IO we have soft limits
> in place, to prevent them from getting too big and not lead to
> unpredictable latencies. For zone append we only have the hardware limit
> in place.
> 
> Cap the max sectors we submit via zone-append to the maximal number of
> sectors if the second limit is lower.
> 
> Link: https://lore.kernel.org/linux-btrfs/yq1k0w8g3rw.fsf@ca-mkp.ca.oracle.com
> Reported-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  include/linux/blkdev.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index cf80e61b4c5e..967cd76f16d4 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1406,7 +1406,10 @@ static inline unsigned int queue_max_segment_size(const struct request_queue *q)
>  
>  static inline unsigned int queue_max_zone_append_sectors(const struct request_queue *q)
>  {
> -	return q->limits.max_zone_append_sectors;
> +
> +	struct queue_limits *l = q->limits;
> +
> +	return min(l->max_zone_append_sectors, l->max_sectors);
>  }
>  
>  static inline unsigned queue_logical_block_size(const struct request_queue *q)
> 

Looks good.

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

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] block: soft limit zone-append sectors as well

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2742 bytes --]

Hi Johannes,

I love your patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on v5.9-rc8 next-20201007]
[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]

url:    https://github.com/0day-ci/linux/commits/Johannes-Thumshirn/block-soft-limit-zone-append-sectors-as-well/20201007-172135
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/27105719d2526e828458cd8c3de408860f6cbd7f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Johannes-Thumshirn/block-soft-limit-zone-append-sectors-as-well/20201007-172135
        git checkout 27105719d2526e828458cd8c3de408860f6cbd7f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/blk-cgroup.h:23,
                    from include/linux/writeback.h:14,
                    from include/linux/memcontrol.h:22,
                    from include/linux/swap.h:9,
                    from include/linux/suspend.h:5,
                    from arch/powerpc/kernel/asm-offsets.c:23:
   include/linux/blkdev.h: In function 'queue_max_zone_append_sectors':
>> include/linux/blkdev.h:1413:27: error: incompatible types when initializing type 'struct queue_limits *' using type 'const struct queue_limits'
    1413 |  struct queue_limits *l = q->limits;
         |                           ^
   make[2]: *** [scripts/Makefile.build:117: arch/powerpc/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1198: prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

vim +1413 include/linux/blkdev.h

  1412	
> 1413		struct queue_limits *l = q->limits;
  1414	
  1415		return min(l->max_zone_append_sectors, l->max_sectors);
  1416	}
  1417	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70374 bytes --]

Re: [PATCH] block: soft limit zone-append sectors as well

[Christoph Hellwig]

On Wed, Oct 07, 2020 at 06:20:05PM +0900, Johannes Thumshirn wrote:
> Martin rightfully noted that for normal filesystem IO we have soft limits
> in place, to prevent them from getting too big and not lead to
> unpredictable latencies. For zone append we only have the hardware limit
> in place.
> 
> Cap the max sectors we submit via zone-append to the maximal number of
> sectors if the second limit is lower.
> 
> Link: https://lore.kernel.org/linux-btrfs/yq1k0w8g3rw.fsf@ca-mkp.ca.oracle.com
> Reported-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] block: soft limit zone-append sectors as well

[Martin K. Petersen]

Johannes,

> Cap the max sectors we submit via zone-append to the maximal number of
> sectors if the second limit is lower.

Yep, that's good. Nice and simple.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] block: soft limit zone-append sectors as well

[Johannes Thumshirn]

On 07/10/2020 14:25, Martin K. Petersen wrote:
> Yep, that's good. Nice and simple.

Yeah no, forgot to git commit --amend, sorry :(

Re: [PATCH] block: soft limit zone-append sectors as well

[Jens Axboe]

On 10/7/20 3:20 AM, Johannes Thumshirn wrote:
> Martin rightfully noted that for normal filesystem IO we have soft limits
> in place, to prevent them from getting too big and not lead to
> unpredictable latencies. For zone append we only have the hardware limit
> in place.
> 
> Cap the max sectors we submit via zone-append to the maximal number of
> sectors if the second limit is lower.
> 
> Link: https://lore.kernel.org/linux-btrfs/yq1k0w8g3rw.fsf@ca-mkp.ca.oracle.com
> Reported-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  include/linux/blkdev.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index cf80e61b4c5e..967cd76f16d4 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1406,7 +1406,10 @@ static inline unsigned int queue_max_segment_size(const struct request_queue *q)
>  
>  static inline unsigned int queue_max_zone_append_sectors(const struct request_queue *q)
>  {
> -	return q->limits.max_zone_append_sectors;
> +
> +	struct queue_limits *l = q->limits;
> +
> +	return min(l->max_zone_append_sectors, l->max_sectors);

As the test robot points out, this won't even compile... How much
testing did you do with this?

-- 
Jens Axboe

Re: [PATCH] block: soft limit zone-append sectors as well

[Johannes Thumshirn]

On 07/10/2020 15:58, Jens Axboe wrote:
> As the test robot points out, this won't even compile... How much
> testing did you do with this?
> 

Please see the v2 and my answer on Martin's mail, I forgot to commit --amend 
the fix before sending.

As for testing it passed the zonefs testsuite on a null_blk device with a default of 127
max sectors and a run with a limit of 64 sectors.