on top of for-5.16/block

bdev = &BDEV_I(file->f_mapping->host)->bdev

Getting struct block_device from a file requires 2 memory dereferences as illustrated above, that takes a toll on performance, so cache it in yet unused file->private_data. That gives a noticeable peak performance improvement.

Signed-off-by Pavel Begunkov <[email protected]>

---
 block/fops.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

block/fops.c +0 -0

index 7bb9581a146c..c71e91cd6bcb 100644

Hi Pavel,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.15-rc5]

[cannot apply to axboe-block/for-next next-20211013]
[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/Pavel-Begunkov/on-top-of-for-5-16-block/20211013-165953
base:    64570fbc14f8d7cb3fe3995f20e26bc25ce4b2cc
config: hexagon-randconfig-r045-20211013 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a8c695542b2987eb9a203d5663a0740cb4725f)

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/9e538de87cc869bd26a3ca78da49d1437ec8688c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pavel-Begunkov/on-top-of-for-5-16-block/20211013-165953 git checkout 9e538de87cc869bd26a3ca78da49d1437ec8688c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

Quoted 1 line

Quoted 1 line

block/fops.c:460:37: error: implicit declaration of function 'bdev_file_inode' [-Werror,-Wimplicit-function-declaration]

            struct block_device *bdev = I_BDEV(bdev_file_inode(file));
                                               ^
    block/fops.c:460:37: note: did you mean 'file_inode'?
    include/linux/fs.h:1348:29: note: 'file_inode' declared here
    static inline struct inode *file_inode(const struct file *f)
                                ^

Quoted 1 line

Quoted 1 line

block/fops.c:460:37: warning: incompatible integer to pointer conversion passing 'int' to parameter of type 'struct inode *' [-Wint-conversion]

struct block_device *bdev = I_BDEV(bdev_file_inode(file));
^~~~~~~~~~~~~~~~~~~~~
include/linux/blkdev.h:1990:57: note: passing argument to parameter 'inode' here static inline struct block_device *I_BDEV(struct inode *inode)
^
1 warning and 1 error generated.

vim +/bdev_file_inode +460 block/fops.c

cd82cca7ebfe9c Christoph Hellwig 2021-09-07  457
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  458  static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  459  {
cd82cca7ebfe9c Christoph Hellwig 2021-09-07 @460  	struct block_device *bdev = I_BDEV(bdev_file_inode(file));
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  461  	fmode_t mode = file->f_mode;
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  462
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  463  	/*
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  464  	 * O_NDELAY can be altered using fcntl(.., F_SETFL, ..), so we have
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  465  	 * to updated it before every ioctl.
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  466  	 */
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  467  	if (file->f_flags & O_NDELAY)
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  468  		mode |= FMODE_NDELAY;
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  469  	else
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  470  		mode &= ~FMODE_NDELAY;
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  471
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  472  	return blkdev_ioctl(bdev, mode, cmd, arg);
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  473  }
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  474

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

Quoted 5 lines · kernel test robot

Hi Pavel,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.15-rc5]

It's against for-5.16/block and there is no more block_ioctl() in fops.c, should be fine

Quoted 89 lines

[cannot apply to axboe-block/for-next next-20211013]
[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/Pavel-Begunkov/on-top-of-for-5-16-block/20211013-165953

base:    64570fbc14f8d7cb3fe3995f20e26bc25ce4b2cc
config: hexagon-randconfig-r045-20211013 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
b6a8c695542b2987eb9a203d5663a0740cb4725f)
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/9e538de87cc869bd26a3ca78da49d1437ec8688c

         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review Pavel-Begunkov/on-top-of-for-5-16-block/20211013-165953 git checkout 9e538de87cc869bd26a3ca78da49d1437ec8688c

         # save the attached .config to linux build tree
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross 
W=1 ARCH=hexagon

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

Quoted 2 lines

Quoted 2 lines

block/fops.c:460:37: error: implicit declaration of function 'bdev_file_inode' [-Werror,-Wimplicit-function-declaration]

            struct block_device *bdev = I_BDEV(bdev_file_inode(file)); ^ block/fops.c:460:37: note: did you mean 'file_inode'? include/linux/fs.h:1348:29: note: 'file_inode' declared here static inline struct inode *file_inode(const struct file *f) ^

Quoted 3 lines

Quoted 3 lines

block/fops.c:460:37: warning: incompatible integer to pointer conversion passing 'int' to parameter of type 'struct inode *' [-Wint-conversion]

            struct block_device *bdev = I_BDEV(bdev_file_inode(file)); ^~~~~~~~~~~~~~~~~~~~~ include/linux/blkdev.h:1990:57: note: passing argument to parameter 'inode' here
    static inline struct block_device *I_BDEV(struct inode *inode) ^ 1 warning and 1 error generated.

vim +/bdev_file_inode +460 block/fops.c

cd82cca7ebfe9c Christoph Hellwig 2021-09-07  457 cd82cca7ebfe9c Christoph Hellwig 2021-09-07  458  static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg) cd82cca7ebfe9c Christoph Hellwig 2021-09-07  459  { cd82cca7ebfe9c Christoph Hellwig 2021-09-07 @460      struct block_device *bdev = I_BDEV(bdev_file_inode(file)); cd82cca7ebfe9c Christoph Hellwig 2021-09-07  461      fmode_t mode = file->f_mode;
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  462 cd82cca7ebfe9c Christoph Hellwig 2021-09-07  463      /* cd82cca7ebfe9c Christoph Hellwig 2021-09-07  464       * O_NDELAY can be altered using fcntl(.., F_SETFL, ..), so we have cd82cca7ebfe9c Christoph Hellwig 2021-09-07  465       * to updated it before every ioctl.
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  466       */ cd82cca7ebfe9c Christoph Hellwig 2021-09-07  467      if (file->f_flags & O_NDELAY)
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  468          mode |= FMODE_NDELAY;
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  469      else cd82cca7ebfe9c Christoph Hellwig 2021-09-07  470          mode &= ~FMODE_NDELAY;
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  471 cd82cca7ebfe9c Christoph Hellwig 2021-09-07  472      return blkdev_ioctl(bdev, mode, cmd, arg);
cd82cca7ebfe9c Christoph Hellwig 2021-09-07  473  } cd82cca7ebfe9c Christoph Hellwig 2021-09-07  474

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

Instead of tricks with struct bdev_inode, just openly place the inode inside struct block_device. First, it allows us to inline I_BDEV, which is simple but non-inline nature of it impacts performance. Also, make it possible to get rid of ->bd_inode pointer and hooping with extra indirection, the amount of which became a noticeable problem for the block layer.

Quoted 6 lines · Pavel Begunkov

Instead of tricks with struct bdev_inode, just openly place the inode inside struct block_device. First, it allows us to inline I_BDEV, which is simple but non-inline nature of it impacts performance. Also, make it possible to get rid of ->bd_inode pointer and hooping with extra indirection, the amount of which became a noticeable problem for the block layer.

What fast path outside of bdev.c cares about an inlined I_BDEV?

I have dusted off patches to reduce (and hopefully eventually kill) accesses to bd_inode outside of bdev.c, so this goes into the wrong direction.

If needed I'd rather figure out a way to fix any smoking gun without this change.

Mildly hot in io_uring w/o fixed files, but that's not peak perf, but would also be great to get rid of bdev->bd_inode dereference, e.g. lots of in fops.c.

Are you going to just hid the dereference in helpers or kill it with some offseting magic?

Quoted 6 lines · Pavel Begunkov

Mildly hot in io_uring w/o fixed files, but that's not peak perf, but would also be great to get rid of bdev->bd_inode dereference, e.g. lots of in fops.c.

Are you going to just hid the dereference in helpers or kill it with some offseting magic?

The only real hot path uses I found is the size (which you and Jens already seem to have moved to use something out of the inode for the fast path), and maybe the blkbits for which we could do the same.

So basically the idea is to not touch the inode in the hot path, and use accessors helpers in bdev.c that could do the offsetof trick.

We already have a blk_mq_need_time_stamp() check in __blk_mq_end_request() to get a timestamp, hide all the statistics accounting under it. It cuts some cycles for requests that don't need stats, and is free otherwise.

./io_uring -d32 -s32 -c32 -b512 -p1 /dev/nullb0 ~3.3 MIOPS vs 3.5 MIOPS, so gives around extra ~4-5%.

The main part is caching struct block_device + some inlining.

v2: without applied patches, merge previous 6/6 into the second patch
get rid of helpers (Jens, Christoph)
kill bdev_inode and move inode into bdev (Christoph)

Pavel Begunkov (3):
block: cache bdev in struct file for raw bdev IO
block: don't hide inode from block_device users
blk-mq: optimise *end_request non-stat path

  block/bdev.c              | 44 ++++++++++-----------------------------
  block/blk-mq.c            | 18 +++++++---------
  block/fops.c              | 43 ++++++++++++++++----------------------
  include/linux/blk_types.h |  1 +
  include/linux/blkdev.h    |  8 +++++--
  5 files changed, 44 insertions(+), 70 deletions(-)

./io_uring -d32 -s32 -c32 -b512 -p1 /dev/nullb0 ~3.3 MIOPS vs 3.5 MIOPS, so gives around extra ~4-5%.

The main part is caching struct block_device + some inlining.

v2: without applied patches, merge previous 6/6 into the second patch
get rid of helpers (Jens, Christoph)
kill bdev_inode and move inode into bdev (Christoph)

[...]