Re: [PATCHv8 1/6] block, fs: restore kiocb based write hint processing

[Christoph Hellwig <hch@lst.de>]

On Thu, Oct 17, 2024 at 09:09:32AM -0700, Keith Busch wrote:
> From: Kanchan Joshi <joshi.k@samsung.com>
> 
> struct kiocb has a 2 bytes hole that developed post commit 41d36a9f3e53
> ("fs: remove kiocb.ki_hint"). But write hint returned with commit
> 449813515d3e ("block, fs: Restore the per-bio/request data lifetime
> fields").
> 
> This patch uses the leftover space in kiocb to carve 2 byte field
> ki_write_hint. Restore the code that operates on kiocb to use
> ki_write_hint instead of inode hint value.
> 
> This does not change any behavior, but needed to enable per-io hints.

In this version it doesn't really restore anything, but adds a new
write hinting capability.   Similarly to the bio patch we'll probably
need to make clear what is in there instead of having it completely
untyped (the exact same appraoch as for the bio should work).

> index bbd05f1a21453..73629e26becbe 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -409,7 +409,7 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio,
>  		bio->bi_end_io = dio_bio_end_io;
>  	if (dio->is_pinned)
>  		bio_set_flag(bio, BIO_PAGE_PINNED);
> -	bio->bi_write_hint = file_inode(dio->iocb->ki_filp)->i_write_hint;
> +	bio->bi_write_hint = dio->iocb->ki_write_hint;
>  
>  	sdio->bio = bio;
>  	sdio->logical_offset_in_bio = sdio->cur_page_fs_offset;
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index f637aa0706a31..fff43f121ee65 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -397,7 +397,7 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
>  		fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
>  					  GFP_KERNEL);
>  		bio->bi_iter.bi_sector = iomap_sector(iomap, pos);
> -		bio->bi_write_hint = inode->i_write_hint;
> +		bio->bi_write_hint = dio->iocb->ki_write_hint;

File system (helper) code should not directly apply this limit,
but the file system needs to set it.

> +static inline enum rw_hint file_write_hint(struct file *filp)
> +{
> +	return file_inode(filp)->i_write_hint;
> +}
> +
>  static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
>  {
>  	*kiocb = (struct kiocb) {
>  		.ki_filp = filp,
>  		.ki_flags = filp->f_iocb_flags,
>  		.ki_ioprio = get_current_ioprio(),
> +		.ki_write_hint = file_write_hint(filp),

And we'll need to distinguish between the per-inode and per file
hint.  I.e. don't blindly initialize ki_write_hint to the per-inode
one here, but make that conditional in the file operation.