On Wed, Dec 06, 2023 at 07:39:35PM +0800, Hongyu Jin wrote:
> From: Hongyu Jin <hongyu.jin@xxxxxxxxxx>
>
> when read FEC and hash from disk, I/O priority are inconsistent
> with data block and blocked by other I/O with low I/O priority.
>
> Add dm_bufio_prefetch_by_ioprio() and dm_bufio_read_by_ioprio(),
> can pecific I/O priority for some I/O.
> Make I/O for FEC and hash has same I/O priority with data I/O.
>
> Co-developed-by: Yibin Ding <yibin.ding@xxxxxxxxxx>
> Signed-off-by: Yibin Ding <yibin.ding@xxxxxxxxxx>
> Signed-off-by: Hongyu Jin <hongyu.jin@xxxxxxxxxx>
>
> ---
> Changes in v2:
> - Add ioprio field in struct dm_io_region
> - Initial struct dm_io_region::ioprio to IOPRIO_DEFAULT
> - Add two interface
> ---
> drivers/md/dm-bufio.c | 50 ++++++++++++++++++++++-----------
> drivers/md/dm-integrity.c | 5 ++++
> drivers/md/dm-io.c | 1 +
> drivers/md/dm-log.c | 1 +
> drivers/md/dm-raid1.c | 2 ++
> drivers/md/dm-snap-persistent.c | 2 ++
> drivers/md/dm-verity-fec.c | 3 +-
> drivers/md/dm-verity-target.c | 10 +++++--
> drivers/md/dm-writecache.c | 4 +++
> include/linux/dm-bufio.h | 6 ++++
> include/linux/dm-io.h | 2 ++
> 11 files changed, 66 insertions(+), 20 deletions(-)
Changing so many things in one patch should be avoided if possible. Is there a
way to split this patch up? Maybe first add ioprio support to dm-io, then add
ioprio support to dm-bufio, then make dm-verity set the correct ioprio?
> void *dm_bufio_read(struct dm_bufio_client *c, sector_t block,
> struct dm_buffer **bp)
> +{
> + return dm_bufio_read_by_ioprio(c, block, bp, IOPRIO_DEFAULT);
> +}
> +EXPORT_SYMBOL_GPL(dm_bufio_read);
> +
> +void *dm_bufio_read_by_ioprio(struct dm_bufio_client *c, sector_t block,
> + struct dm_buffer **bp, unsigned short ioprio)
> {
> if (WARN_ON_ONCE(dm_bufio_in_request()))
> return ERR_PTR(-EINVAL);
>
> - return new_read(c, block, NF_READ, bp);
> + return new_read(c, block, NF_READ, bp, ioprio);
> }
> -EXPORT_SYMBOL_GPL(dm_bufio_read);
> +EXPORT_SYMBOL_GPL(dm_bufio_read_by_ioprio);
>
> void *dm_bufio_new(struct dm_bufio_client *c, sector_t block,
> struct dm_buffer **bp)
> @@ -1909,12 +1918,19 @@ void *dm_bufio_new(struct dm_bufio_client *c, sector_t block,
> if (WARN_ON_ONCE(dm_bufio_in_request()))
> return ERR_PTR(-EINVAL);
>
> - return new_read(c, block, NF_FRESH, bp);
> + return new_read(c, block, NF_FRESH, bp, IOPRIO_DEFAULT);
> }
> EXPORT_SYMBOL_GPL(dm_bufio_new);
>
> void dm_bufio_prefetch(struct dm_bufio_client *c,
> sector_t block, unsigned int n_blocks)
> +{
> + return dm_bufio_prefetch_by_ioprio(c, block, n_blocks, IOPRIO_DEFAULT);
> +}
> +EXPORT_SYMBOL_GPL(dm_bufio_prefetch);
> +
> +void dm_bufio_prefetch_by_ioprio(struct dm_bufio_client *c,
> + sector_t block, unsigned int n_blocks, unsigned short ioprio)
I think it would be cleaner to just add the ioprio parameter to dm_bufio_read()
and dm_bufio_prefetch(), instead of adding new functions.
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 26adcfea0302..5945ac1dfdff 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -51,6 +51,7 @@ static DEFINE_STATIC_KEY_FALSE(use_tasklet_enabled);
> struct dm_verity_prefetch_work {
> struct work_struct work;
> struct dm_verity *v;
> + struct dm_verity_io *io;
> sector_t block;
> unsigned int n_blocks;
> };
Isn't it possible for 'io' to complete and be freed while the prefetch work is
still running?
- Eric
