[v2,04/15] fs: Restore write hint support

This patch reverts a small subset of commit c75e707fe1aa ("block: remove
the per-bio/request write hint"). The following functionality has been
restored:
- In F2FS, store data lifetime information in struct bio.
- In fs/iomap and fs/mpage.c, restore the code that sets the data
  lifetime.

A new header file is introduced for the new bio_[sg]et_data_lifetime()
functions because there is no other header file yet that includes both
<linux/fs.h> and <linux/ioprio.h>.

The value WRITE_LIFE_NONE is mapped onto the data lifetime 0. This is
consistent with NVMe TPAR4093a. From that TPAR: "A value of 1h specifies
the shortest Data Lifetime. A value of 3Fh specifies the longest Data
Lifetime." This is also consistent with the SCSI specifications. From
T10 document 23-024r3: "0h: no relative lifetime is applicable; 1h:
shortest relative lifetime; ...; 3fh: longest relative lifetime".

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 fs/f2fs/data.c              |  3 +++
 fs/iomap/buffered-io.c      |  3 +++
 fs/mpage.c                  |  2 ++
 include/linux/fs-lifetime.h | 20 ++++++++++++++++++++
 4 files changed, 28 insertions(+)
 create mode 100644 include/linux/fs-lifetime.h

On 10/6/2023 1:10 AM, Bart Van Assche wrote:
> This patch reverts a small subset of commit c75e707fe1aa ("block: remove
> the per-bio/request write hint"). The following functionality has been
> restored:
> - In F2FS, store data lifetime information in struct bio.
> - In fs/iomap and fs/mpage.c, restore the code that sets the data
>    lifetime.
> 
> A new header file is introduced for the new bio_[sg]et_data_lifetime()
> functions because there is no other header file yet that includes both
> <linux/fs.h> and <linux/ioprio.h>.
> 
> The value WRITE_LIFE_NONE is mapped onto the data lifetime 0. This is
> consistent with NVMe TPAR4093a. From that TPAR: "A value of 1h specifies
> the shortest Data Lifetime. A value of 3Fh specifies the longest Data
> Lifetime." This is also consistent with the SCSI specifications. From
> T10 document 23-024r3: "0h: no relative lifetime is applicable; 1h:
> shortest relative lifetime; ...; 3fh: longest relative lifetime".
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   fs/f2fs/data.c              |  3 +++
>   fs/iomap/buffered-io.c      |  3 +++
>   fs/mpage.c                  |  2 ++
>   include/linux/fs-lifetime.h | 20 ++++++++++++++++++++
>   4 files changed, 28 insertions(+)
>   create mode 100644 include/linux/fs-lifetime.h
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 916e317ac925..2962cb335897 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -6,6 +6,7 @@
>    *             http://www.samsung.com/
>    */
>   #include <linux/fs.h>
> +#include <linux/fs-lifetime.h>
>   #include <linux/f2fs_fs.h>
>   #include <linux/buffer_head.h>
>   #include <linux/sched/mm.h>
> @@ -478,6 +479,8 @@ static struct bio *__bio_alloc(struct f2fs_io_info *fio, int npages)
>   	} else {
>   		bio->bi_end_io = f2fs_write_end_io;
>   		bio->bi_private = sbi;
> +		bio_set_data_lifetime(bio,
> +			f2fs_io_type_to_rw_hint(sbi, fio->type, fio->temp));
>   	}
>   	iostat_alloc_and_bind_ctx(sbi, bio, NULL);
>   
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 644479ccefbd..9bf05342ca65 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -6,6 +6,7 @@
>   #include <linux/module.h>
>   #include <linux/compiler.h>
>   #include <linux/fs.h>
> +#include <linux/fs-lifetime.h>
>   #include <linux/iomap.h>
>   #include <linux/pagemap.h>
>   #include <linux/uio.h>
> @@ -1660,6 +1661,7 @@ iomap_alloc_ioend(struct inode *inode, struct iomap_writepage_ctx *wpc,
>   			       REQ_OP_WRITE | wbc_to_write_flags(wbc),
>   			       GFP_NOFS, &iomap_ioend_bioset);
>   	bio->bi_iter.bi_sector = sector;
> +	bio_set_data_lifetime(bio, inode->i_write_hint);
>   	wbc_init_bio(wbc, bio);
>   
>   	ioend = container_of(bio, struct iomap_ioend, io_inline_bio);
> @@ -1690,6 +1692,7 @@ iomap_chain_bio(struct bio *prev)
>   	new = bio_alloc(prev->bi_bdev, BIO_MAX_VECS, prev->bi_opf, GFP_NOFS);
>   	bio_clone_blkg_association(new, prev);
>   	new->bi_iter.bi_sector = bio_end_sector(prev);
> +	bio_set_data_lifetime(new, bio_get_data_lifetime(prev));
>   
>   	bio_chain(prev, new);
>   	bio_get(prev);		/* for iomap_finish_ioend */
> diff --git a/fs/mpage.c b/fs/mpage.c
> index 242e213ee064..888ca71c9ea7 100644
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -20,6 +20,7 @@
>   #include <linux/gfp.h>
>   #include <linux/bio.h>
>   #include <linux/fs.h>
> +#include <linux/fs-lifetime.h>
>   #include <linux/buffer_head.h>
>   #include <linux/blkdev.h>
>   #include <linux/highmem.h>
> @@ -612,6 +613,7 @@ static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc,
>   				GFP_NOFS);
>   		bio->bi_iter.bi_sector = blocks[0] << (blkbits - 9);
>   		wbc_init_bio(wbc, bio);
> +		bio_set_data_lifetime(bio, inode->i_write_hint);
>   	}
>   
>   	/*
> diff --git a/include/linux/fs-lifetime.h b/include/linux/fs-lifetime.h
> new file mode 100644
> index 000000000000..0e652e00cfab
> --- /dev/null
> +++ b/include/linux/fs-lifetime.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <linux/bio.h>
> +#include <linux/fs.h>
> +#include <linux/ioprio.h>
> +
> +static inline enum rw_hint bio_get_data_lifetime(struct bio *bio)
> +{
> +	/* +1 to map 0 onto WRITE_LIFE_NONE. */
> +	return IOPRIO_PRIO_LIFETIME(bio->bi_ioprio) + 1;
> +}
> +
> +static inline void bio_set_data_lifetime(struct bio *bio, enum rw_hint lifetime)
> +{
> +	/* -1 to map WRITE_LIFE_NONE onto 0. */
> +	if (lifetime != 0)
> +		lifetime--;

How the driver can figure when lifetime is not set, and when it is set 
to WRITE_LIFE_NONE? If it uses IOPRIO_PRIO_LIFETIME (as patch 8 does), 
it will see 0 in both cases.
F2FS fs-based whint_mode seems to expect distinct streams for 
WRITE_LIFE_NOT_SET and WRITE_LIFE_NONE.

On Thu, Oct 05, 2023 at 12:40:50PM -0700, Bart Van Assche wrote:
> This patch reverts a small subset of commit c75e707fe1aa ("block: remove
> the per-bio/request write hint"). The following functionality has been
> restored:

Please explain this in terms of what you add.  The fact that it restores
something isn't more than a little footnote added at the end.

> --- /dev/null
> +++ b/include/linux/fs-lifetime.h

The name seems a bit odd for something that primarily deals with bios.
bio-lifetime.h would seem like a better fit.

> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <linux/bio.h>
> +#include <linux/fs.h>
> +#include <linux/ioprio.h>
> +
> +static inline enum rw_hint bio_get_data_lifetime(struct bio *bio)
> +{
> +	/* +1 to map 0 onto WRITE_LIFE_NONE. */
> +	return IOPRIO_PRIO_LIFETIME(bio->bi_ioprio) + 1;

This seems a little to magic.  Why not a lookup table?

> +}
> +
> +static inline void bio_set_data_lifetime(struct bio *bio, enum rw_hint lifetime)

Please avoid the overly long line.

> +	/* -1 to map WRITE_LIFE_NONE onto 0. */
> +	if (lifetime != 0)
> +		lifetime--;
> +	WARN_ON_ONCE(lifetime & ~IOPRIO_LIFETIME_MASK);

I'd return here instead of propagating the bogus value.