| Message ID | 2358a1f93c2c2f9f7564eb77334a7ea679453deb.1602062387.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: soft limit zone-append sectors as well | expand |
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>
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
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>
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>
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 :(
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?
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 --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)
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(-)