linux-kernel - Re: [PATCH v3 1/3] block: move .bd_inode into 1st cacheline of block

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d899b7bc-5942-3359-b37f-4ffd15981abb@huaweicloud.com>
Date:   Wed, 22 Nov 2023 19:17:31 +0800
From:   Yu Kuai <yukuai1@...weicloud.com>
To:     Yu Kuai <yukuai1@...weicloud.com>, ming.lei@...hat.com,
        axboe@...nel.dk
Cc:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        yi.zhang@...wei.com, yangerkun@...wei.com,
        "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH v3 1/3] block: move .bd_inode into 1st cacheline of
 block_device

Hi,

在 2023/11/22 18:31, Yu Kuai 写道:
> From: Ming Lei <ming.lei@...hat.com>
> 
> 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@...wei.com>
> Signed-off-by: Ming Lei <ming.lei@...hat.com>
> Signed-off-by: Yu Kuai <yukuai3@...wei.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 */

Now that we're here, and bdev->bd_inode is always point to the
field inode of struct bdev_inode, which is next to the field bdev,
and the comment "will die" have been exist for a long time.

Maybe I can try to replace all the reference of bdev->bd_inode with
a helper, and then remove this field, then it'll be acceptable to add
a new field "unsigned long bd_flags".

Thanks,
Kuai


> +
>   	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;
>