diff mbox series

block: soft limit zone-append sectors as well

Message ID 2358a1f93c2c2f9f7564eb77334a7ea679453deb.1602062387.git.johannes.thumshirn@wdc.com (mailing list archive)
State New
Headers show
Series block: soft limit zone-append sectors as well | expand

Commit Message

Johannes Thumshirn Oct. 7, 2020, 9:20 a.m. UTC
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(-)

Comments

Damien Le Moal Oct. 7, 2020, 11:21 a.m. UTC | #1
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>
kernel test robot Oct. 7, 2020, 11:40 a.m. UTC | #2
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
Christoph Hellwig Oct. 7, 2020, 12:14 p.m. UTC | #3
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>
Martin K. Petersen Oct. 7, 2020, 12:25 p.m. UTC | #4
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>
Johannes Thumshirn Oct. 7, 2020, 12:28 p.m. UTC | #5
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 :(
Jens Axboe Oct. 7, 2020, 1:58 p.m. UTC | #6
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?
Johannes Thumshirn Oct. 7, 2020, 2:19 p.m. UTC | #7
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.
diff mbox series

Patch

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)