On Fri 23-08-24 14:47:35, Christian Brauner wrote:
> The i_state member is an unsigned long so that it can be used with the
> wait bit infrastructure which expects unsigned long. This wastes 4 bytes
> which we're unlikely to ever use. Switch to using the var event wait
> mechanism using the address of the bit. Thanks to Linus for the address
> idea.
>
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
Looks good. Interesting trick with the wakeup address. I'm not sure we
100% need it since e.g. __I_NEW waiters are unlikely to combine with
__I_SYNC waiters so sharing a waitqueue should not cause big issues but I
guess we'll deal with that if we run out of waiting bits with the current
scheme. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/inode.c | 11 +++++++++++
> include/linux/fs.h | 15 +++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 154f8689457f..877c64a1bf63 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -472,6 +472,17 @@ static void __inode_add_lru(struct inode *inode, bool rotate)
> inode->i_state |= I_REFERENCED;
> }
>
> +struct wait_queue_head *inode_bit_waitqueue(struct wait_bit_queue_entry *wqe,
> + struct inode *inode, u32 bit)
> +{
> + void *bit_address;
> +
> + bit_address = inode_state_wait_address(inode, bit);
> + init_wait_var_entry(wqe, bit_address, 0);
> + return __var_waitqueue(bit_address);
> +}
> +EXPORT_SYMBOL(inode_bit_waitqueue);
> +
> /*
> * Add inode to LRU if needed (inode is unused and clean).
> *
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 23e7d46b818a..1d895b8cb801 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -744,6 +744,21 @@ struct inode {
> void *i_private; /* fs or device private pointer */
> } __randomize_layout;
>
> +/*
> + * Get bit address from inode->i_state to use with wait_var_event()
> + * infrastructre.
> + */
> +#define inode_state_wait_address(inode, bit) ((char *)&(inode)->i_state + (bit))
> +
> +struct wait_queue_head *inode_bit_waitqueue(struct wait_bit_queue_entry *wqe,
> + struct inode *inode, u32 bit);
> +
> +static inline void inode_wake_up_bit(struct inode *inode, u32 bit)
> +{
> + /* Caller is responsible for correct memory barriers. */
> + wake_up_var(inode_state_wait_address(inode, bit));
> +}
> +
> struct timespec64 timestamp_truncate(struct timespec64 t, struct inode *inode);
>
> static inline unsigned int i_blocksize(const struct inode *node)
>
> --
> 2.43.0
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
