| Message ID | 20231121101156.378105-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: move .bd_inode into 1st cacheline of block_device | expand |
Hi, 在 2023/11/21 18:11, Ming Lei 写道: > The .bd_inode field of block_device is used in IO fast path of > blkdev_write_iter() and blkdev_llseek(), so it is more efficient to keep > it into the 1st cacheline. > > .bd_openers is only touched in open()/close(), and .bd_size_lock is only > for updating bdev capacity, which is in slow path too. > > So swap .bd_inode layout with .bd_openers & .bd_size_lock to move > .bd_inode into the 1st cache line. This patch looks good, do you want me do take it for a v3 for the other patchset? And by the way, can we also move 'int bd_writers' to near 'atomic_t bd_fsfreeze_count' to save 8 bytes(int 64bit platform)? diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index 07abd0165226..a47ab9249bdd 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -63,11 +63,11 @@ struct block_device { int bd_holders; struct kobject *bd_holder_dir; + int bd_writers; atomic_t bd_fsfreeze_count; /* number of freeze requests */ struct mutex bd_fsfreeze_mutex; /* serialize freeze/thaw */ struct partition_meta_info *bd_meta_info; - int bd_writers; Thanks, Kuai > > Cc: Yu Kuai <yukuai3@huawei.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > include/linux/blk_types.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index d5c5e59ddbd2..f7d40692dd94 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -49,9 +49,10 @@ struct block_device { > bool bd_write_holder; > bool bd_has_submit_bio; > dev_t bd_dev; > + struct inode *bd_inode; /* will die */ > + > atomic_t bd_openers; > spinlock_t bd_size_lock; /* for bd_inode->i_size updates */ > - struct inode * bd_inode; /* will die */ > void * bd_claiming; > void * bd_holder; > const struct blk_holder_ops *bd_holder_ops; >
On Tue, Nov 21, 2023 at 07:12:44PM +0800, Yu Kuai wrote: > Hi, > > 在 2023/11/21 18:11, Ming Lei 写道: > > The .bd_inode field of block_device is used in IO fast path of > > blkdev_write_iter() and blkdev_llseek(), so it is more efficient to keep > > it into the 1st cacheline. > > > > .bd_openers is only touched in open()/close(), and .bd_size_lock is only > > for updating bdev capacity, which is in slow path too. > > > > So swap .bd_inode layout with .bd_openers & .bd_size_lock to move > > .bd_inode into the 1st cache line. > > This patch looks good, do you want me do take it for a v3 for the > other patchset? Yeah, please take it. > > And by the way, can we also move 'int bd_writers' to near 'atomic_t > bd_fsfreeze_count' to save 8 bytes(int 64bit platform)? > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index 07abd0165226..a47ab9249bdd 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -63,11 +63,11 @@ struct block_device { > int bd_holders; > struct kobject *bd_holder_dir; > > + int bd_writers; > atomic_t bd_fsfreeze_count; /* number of freeze > requests */ > struct mutex bd_fsfreeze_mutex; /* serialize freeze/thaw > */ > > struct partition_meta_info *bd_meta_info; > - int bd_writers; Which tree are you talking about? I don't see 'bd_writers' in both linus tree and block-6.7, and for-6.8/block isn't open yet. Thanks, Ming
Hi, 在 2023/11/21 19:21, Ming Lei 写道: > On Tue, Nov 21, 2023 at 07:12:44PM +0800, Yu Kuai wrote: >> Hi, >> >> 在 2023/11/21 18:11, Ming Lei 写道: >>> The .bd_inode field of block_device is used in IO fast path of >>> blkdev_write_iter() and blkdev_llseek(), so it is more efficient to keep >>> it into the 1st cacheline. >>> >>> .bd_openers is only touched in open()/close(), and .bd_size_lock is only >>> for updating bdev capacity, which is in slow path too. >>> >>> So swap .bd_inode layout with .bd_openers & .bd_size_lock to move >>> .bd_inode into the 1st cache line. >> >> This patch looks good, do you want me do take it for a v3 for the >> other patchset? > > Yeah, please take it. Ok > >> >> And by the way, can we also move 'int bd_writers' to near 'atomic_t >> bd_fsfreeze_count' to save 8 bytes(int 64bit platform)? >> >> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h >> index 07abd0165226..a47ab9249bdd 100644 >> --- a/include/linux/blk_types.h >> +++ b/include/linux/blk_types.h >> @@ -63,11 +63,11 @@ struct block_device { >> int bd_holders; >> struct kobject *bd_holder_dir; >> >> + int bd_writers; >> atomic_t bd_fsfreeze_count; /* number of freeze >> requests */ >> struct mutex bd_fsfreeze_mutex; /* serialize freeze/thaw >> */ >> >> struct partition_meta_info *bd_meta_info; >> - int bd_writers; > > Which tree are you talking about? I don't see 'bd_writers' in both > linus tree and block-6.7, and for-6.8/block isn't open yet. This is introduced from commit dc85fbc92365 ("block: Add config option to not allow writing to mounted devices") from linux-next by Jan. Thanks, Kuai > > Thanks, > Ming > > . >
On Tue, Nov 21, 2023 at 07:36:34PM +0800, Yu Kuai wrote: > Hi, > > 在 2023/11/21 19:21, Ming Lei 写道: > > On Tue, Nov 21, 2023 at 07:12:44PM +0800, Yu Kuai wrote: > > > Hi, > > > > > > 在 2023/11/21 18:11, Ming Lei 写道: > > > > The .bd_inode field of block_device is used in IO fast path of > > > > blkdev_write_iter() and blkdev_llseek(), so it is more efficient to keep > > > > it into the 1st cacheline. > > > > > > > > .bd_openers is only touched in open()/close(), and .bd_size_lock is only > > > > for updating bdev capacity, which is in slow path too. > > > > > > > > So swap .bd_inode layout with .bd_openers & .bd_size_lock to move > > > > .bd_inode into the 1st cache line. > > > > > > This patch looks good, do you want me do take it for a v3 for the > > > other patchset? > > > > Yeah, please take it. > > Ok > > > > > > > > And by the way, can we also move 'int bd_writers' to near 'atomic_t > > > bd_fsfreeze_count' to save 8 bytes(int 64bit platform)? > > > > > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > > > index 07abd0165226..a47ab9249bdd 100644 > > > --- a/include/linux/blk_types.h > > > +++ b/include/linux/blk_types.h > > > @@ -63,11 +63,11 @@ struct block_device { > > > int bd_holders; > > > struct kobject *bd_holder_dir; > > > > > > + int bd_writers; > > > atomic_t bd_fsfreeze_count; /* number of freeze > > > requests */ > > > struct mutex bd_fsfreeze_mutex; /* serialize freeze/thaw > > > */ > > > > > > struct partition_meta_info *bd_meta_info; > > > - int bd_writers; > > > > Which tree are you talking about? I don't see 'bd_writers' in both > > linus tree and block-6.7, and for-6.8/block isn't open yet. > > This is introduced from commit dc85fbc92365 ("block: Add config option > to not allow writing to mounted devices") from linux-next by Jan. Patch isn't supposed to be against linux-next, and either you need to base the change against maintainer tree(fs) or block tree when Jan's change lands linus tree. Thanks, Ming
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index d5c5e59ddbd2..f7d40692dd94 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -49,9 +49,10 @@ struct block_device { bool bd_write_holder; bool bd_has_submit_bio; dev_t bd_dev; + struct inode *bd_inode; /* will die */ + atomic_t bd_openers; spinlock_t bd_size_lock; /* for bd_inode->i_size updates */ - struct inode * bd_inode; /* will die */ void * bd_claiming; void * bd_holder; const struct blk_holder_ops *bd_holder_ops;
The .bd_inode field of block_device is used in IO fast path of blkdev_write_iter() and blkdev_llseek(), so it is more efficient to keep it into the 1st cacheline. .bd_openers is only touched in open()/close(), and .bd_size_lock is only for updating bdev capacity, which is in slow path too. So swap .bd_inode layout with .bd_openers & .bd_size_lock to move .bd_inode into the 1st cache line. Cc: Yu Kuai <yukuai3@huawei.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- include/linux/blk_types.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)