All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] block: soft limit zone-append sectors as well
@ 2020-10-07  9:20 Johannes Thumshirn
  2020-10-07 11:21 ` Damien Le Moal
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Johannes Thumshirn @ 2020-10-07  9:20 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-fsdevel, linux-block, Damien Le Moal, Christoph Hellwig,
	Johannes Thumshirn, Martin K . Petersen

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] block: soft limit zone-append sectors as well
  2020-10-07  9:20 [PATCH] block: soft limit zone-append sectors as well Johannes Thumshirn
@ 2020-10-07 11:21 ` Damien Le Moal
  2020-10-07 11:40   ` kernel test robot
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Damien Le Moal @ 2020-10-07 11:21 UTC (permalink / raw)
  To: Johannes Thumshirn, Jens Axboe
  Cc: linux-fsdevel, linux-block, Christoph Hellwig, Martin K . Petersen

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] block: soft limit zone-append sectors as well
  2020-10-07  9:20 [PATCH] block: soft limit zone-append sectors as well Johannes Thumshirn
@ 2020-10-07 11:40   ` kernel test robot
  2020-10-07 11:40   ` kernel test robot
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-10-07 11:40 UTC (permalink / raw)
  To: Johannes Thumshirn, Jens Axboe
  Cc: kbuild-all, linux-fsdevel, linux-block, Damien Le Moal,
	Christoph Hellwig, Johannes Thumshirn, Martin K . Petersen

[-- 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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] block: soft limit zone-append sectors as well
@ 2020-10-07 11:40   ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-10-07 11:40 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2802 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(a)lists.01.org

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] block: soft limit zone-append sectors as well
  2020-10-07  9:20 [PATCH] block: soft limit zone-append sectors as well Johannes Thumshirn
  2020-10-07 11:21 ` Damien Le Moal
  2020-10-07 11:40   ` kernel test robot
@ 2020-10-07 12:14 ` Christoph Hellwig
  2020-10-07 12:25 ` Martin K. Petersen
  2020-10-07 13:58 ` Jens Axboe
  4 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2020-10-07 12:14 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Jens Axboe, linux-fsdevel, linux-block, Damien Le Moal,
	Christoph Hellwig, Martin K . Petersen

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>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] block: soft limit zone-append sectors as well
  2020-10-07  9:20 [PATCH] block: soft limit zone-append sectors as well Johannes Thumshirn
                   ` (2 preceding siblings ...)
  2020-10-07 12:14 ` Christoph Hellwig
@ 2020-10-07 12:25 ` Martin K. Petersen
  2020-10-07 12:28   ` Johannes Thumshirn
  2020-10-07 13:58 ` Jens Axboe
  4 siblings, 1 reply; 9+ messages in thread
From: Martin K. Petersen @ 2020-10-07 12:25 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Jens Axboe, linux-fsdevel, linux-block, Damien Le Moal,
	Christoph Hellwig, 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] block: soft limit zone-append sectors as well
  2020-10-07 12:25 ` Martin K. Petersen
@ 2020-10-07 12:28   ` Johannes Thumshirn
  0 siblings, 0 replies; 9+ messages in thread
From: Johannes Thumshirn @ 2020-10-07 12:28 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: Jens Axboe, linux-fsdevel, linux-block, Damien Le Moal,
	Christoph Hellwig

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 :(

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] block: soft limit zone-append sectors as well
  2020-10-07  9:20 [PATCH] block: soft limit zone-append sectors as well Johannes Thumshirn
                   ` (3 preceding siblings ...)
  2020-10-07 12:25 ` Martin K. Petersen
@ 2020-10-07 13:58 ` Jens Axboe
  2020-10-07 14:19   ` Johannes Thumshirn
  4 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2020-10-07 13:58 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: linux-fsdevel, linux-block, Damien Le Moal, Christoph Hellwig,
	Martin K . Petersen

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] block: soft limit zone-append sectors as well
  2020-10-07 13:58 ` Jens Axboe
@ 2020-10-07 14:19   ` Johannes Thumshirn
  0 siblings, 0 replies; 9+ messages in thread
From: Johannes Thumshirn @ 2020-10-07 14:19 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-fsdevel, linux-block, Damien Le Moal, Christoph Hellwig,
	Martin K . Petersen

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-10-07 14:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07  9:20 [PATCH] block: soft limit zone-append sectors as well Johannes Thumshirn
2020-10-07 11:21 ` Damien Le Moal
2020-10-07 11:40 ` kernel test robot
2020-10-07 11:40   ` kernel test robot
2020-10-07 12:14 ` Christoph Hellwig
2020-10-07 12:25 ` Martin K. Petersen
2020-10-07 12:28   ` Johannes Thumshirn
2020-10-07 13:58 ` Jens Axboe
2020-10-07 14:19   ` Johannes Thumshirn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.