| Message ID | 20231211090000.9578-6-hongyu.jin.cn@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | Fix I/O priority lost in device-mapper | expand |
On Mon, 11 Dec 2023, Hongyu Jin wrote: > From: Hongyu Jin <hongyu.jin@unisoc.com> > > The original submitting bio->bi_ioprio setting can be retained by > struct dm_crypt_io::base_bio, we set the original bio's ioprio to > the cloned bio for write. > > Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com> Thanks, Reviewed-by: Eric Wheeler <dm-crypt@linux.ewheeler.net> > --- > drivers/md/dm-crypt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c > index 6de107aff331..b67fec865f00 100644 > --- a/drivers/md/dm-crypt.c > +++ b/drivers/md/dm-crypt.c > @@ -1683,6 +1683,7 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned int size) > GFP_NOIO, &cc->bs); > clone->bi_private = io; > clone->bi_end_io = crypt_endio; > + clone->bi_ioprio = bio_prio(io->base_bio); > > remaining_size = size; > > -- > 2.34.1 > > >
On Mon, Dec 11 2023 at 4:00P -0500, Hongyu Jin <hongyu.jin.cn@gmail.com> wrote: > From: Hongyu Jin <hongyu.jin@unisoc.com> > > The original submitting bio->bi_ioprio setting can be retained by > struct dm_crypt_io::base_bio, we set the original bio's ioprio to > the cloned bio for write. > > Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com> > --- > drivers/md/dm-crypt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c > index 6de107aff331..b67fec865f00 100644 > --- a/drivers/md/dm-crypt.c > +++ b/drivers/md/dm-crypt.c > @@ -1683,6 +1683,7 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned int size) > GFP_NOIO, &cc->bs); > clone->bi_private = io; > clone->bi_end_io = crypt_endio; > + clone->bi_ioprio = bio_prio(io->base_bio); Weird use of bio_prio() wrapper given the assignment to clone->bi_ioprio. I'd prefer: clone->bi_ioprio = io->base_bio->bi_ioprio; Some additional info to be mindful of: This encryption bio has always been unique (ever since dm-crypt stopped using the block layer's methods for cloning with 2007's commit 2f9941b6c55d7). Prior to commit 2f9941b6c55d7, dm-crypt used to call __bio_clone() to make sure not to miss cloning other capabilities -- and __bio_clone() does exist again as of commit a0e8de798dd67 but it is private to bio.c (in service to bio_alloc_clone, etc). My point: because we aren't using traditional bio cloning (due to not wanting to share the bio_vec) we also aren't transferring over the cgroup (via bio_clone_blkg_association), etc. That can be a secondary concern that you don't need to worry about (but it is something Mikulas and I need to look at closer). Mike
From: Hongyu Jin <hongyu.jin@unisoc.com>
A high-priority task obtains data from the dm-verity device using the
RT IO priority, during the verification, the IO reading FEC and hash
by kworker loses the RT priority and is blocked by the low-priority IO.
dm-crypt has the same problem in the process of writing data.
This is because io_context and blkcg are missing.
Move bio_set_ioprio() into submit_bio():
1. Only call bio_set_ioprio() once to set the priority of original bio,
the bio that cloned and splited from original bio will auto inherit
the priority of original bio in clone process.
2. Make the IO priority of the original bio to be passed to dm,
and the dm target inherits the IO priority as needed.
Changes in v4:
- Modify commit message by Suggestion
- Modify patch for dm-crypt
Changes in v3:
- Split patch for device-mapper
- Add patch to fix dm-crypy I/O priority question
- Add block patch to review together
- Fix some error in v2 patch
Changes in v2:
- Add ioprio field in struct dm_io_region
- Initial struct dm_io_region::ioprio to IOPRIO_DEFAULT
- Add two interface
**
Hongyu Jin (5):
block: Fix bio IO priority setting
dm: Support I/O priority for dm_io()
dm-bufio: Support I/O priority
dm verity: Fix I/O priority lost when read FEC and hash
dm-crypt: Fix lost ioprio when queuing write bios
block/blk-core.c | 10 ++++++
block/blk-mq.c | 11 ------
drivers/md/dm-bufio.c | 36 ++++++++++---------
drivers/md/dm-crypt.c | 1 +
drivers/md/dm-ebs-target.c | 8 ++---
drivers/md/dm-integrity.c | 7 +++-
drivers/md/dm-io.c | 1 +
drivers/md/dm-log.c | 1 +
drivers/md/dm-raid1.c | 2 ++
drivers/md/dm-snap-persistent.c | 5 +--
drivers/md/dm-verity-fec.c | 5 +--
drivers/md/dm-verity-target.c | 8 +++--
drivers/md/dm-writecache.c | 4 +++
drivers/md/persistent-data/dm-block-manager.c | 6 ++--
include/linux/dm-bufio.h | 6 ++--
include/linux/dm-io.h | 2 ++
16 files changed, 69 insertions(+), 44 deletions(-)
On Tue, Dec 12, 2023 at 07:11:45PM +0800, Hongyu Jin wrote: > From: Hongyu Jin <hongyu.jin@unisoc.com> > > A high-priority task obtains data from the dm-verity device using the > RT IO priority, during the verification, the IO reading FEC and hash > by kworker loses the RT priority and is blocked by the low-priority IO. > dm-crypt has the same problem in the process of writing data. > > This is because io_context and blkcg are missing. > > Move bio_set_ioprio() into submit_bio(): > 1. Only call bio_set_ioprio() once to set the priority of original bio, > the bio that cloned and splited from original bio will auto inherit > the priority of original bio in clone process. > > 2. Make the IO priority of the original bio to be passed to dm, > and the dm target inherits the IO priority as needed. > What commit does this patch series apply to? - Eric
Eric Biggers <ebiggers@kernel.org> 于2023年12月13日周三 12:45写道： > > On Tue, Dec 12, 2023 at 07:11:45PM +0800, Hongyu Jin wrote: > > From: Hongyu Jin <hongyu.jin@unisoc.com> > > > > A high-priority task obtains data from the dm-verity device using the > > RT IO priority, during the verification, the IO reading FEC and hash > > by kworker loses the RT priority and is blocked by the low-priority IO. > > dm-crypt has the same problem in the process of writing data. > > > > This is because io_context and blkcg are missing. > > > > Move bio_set_ioprio() into submit_bio(): > > 1. Only call bio_set_ioprio() once to set the priority of original bio, > > the bio that cloned and splited from original bio will auto inherit > > the priority of original bio in clone process. > > > > 2. Make the IO priority of the original bio to be passed to dm, > > and the dm target inherits the IO priority as needed. > > > > What commit does this patch series apply to? > > - Eric Changes are based on the master branch commit 9bacdd8996c7 (origin/master, origin/HEAD) Merge tag 'for-6.7-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
From: Hongyu Jin <hongyu.jin@unisoc.com>
A high-priority task obtains data from the dm-verity device using the
RT IO priority, during the verification, the IO reading FEC and hash
by kworker loses the RT priority and is blocked by the low-priority IO.
dm-crypt has the same problem in the process of writing data.
This is because io_context and blkcg are missing.
Move bio_set_ioprio() into submit_bio():
1. Only call bio_set_ioprio() once to set the priority of original bio,
the bio that cloned and splited from original bio will auto inherit
the priority of original bio in clone process.
2. Make the IO priority of the original bio to be passed to dm,
and the dm target inherits the IO priority as needed.
All changes are based on commit 9bacdd8996c7 ("Merge tag 'for-6.7-rc1-tag'
of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux")
on the master branch.
Changes in v5:
- Rewrite patch 2, add ioprio parameter in dm_io();
- Modify dm_io() in patch 3
Changes in v4:
- Modify commit message by Suggestion
- Modify patch for dm-crypt
Changes in v3:
- Split patch for device-mapper
- Add patch to fix dm-crypy I/O priority question
- Add block patch to review together
- Fix some error in v2 patch
Changes in v2:
- Add ioprio field in struct dm_io_region
- Initial struct dm_io_region::ioprio to IOPRIO_DEFAULT
- Add two interface
Hongyu Jin (5):
block: Fix bio IO priority setting
dm: Support I/O priority for dm_io()
dm-bufio: Support I/O priority
dm verity: Fix I/O priority lost when read FEC and hash
dm-crypt: Fix lost ioprio when queuing write bios
block/blk-core.c | 10 +++++
block/blk-mq.c | 11 ------
drivers/md/dm-bufio.c | 39 ++++++++++---------
drivers/md/dm-crypt.c | 1 +
drivers/md/dm-ebs-target.c | 8 ++--
drivers/md/dm-integrity.c | 12 +++---
drivers/md/dm-io.c | 23 ++++++-----
drivers/md/dm-kcopyd.c | 4 +-
drivers/md/dm-log.c | 4 +-
drivers/md/dm-raid1.c | 6 +--
drivers/md/dm-snap-persistent.c | 8 ++--
drivers/md/dm-verity-fec.c | 5 ++-
drivers/md/dm-verity-target.c | 8 +++-
drivers/md/dm-writecache.c | 8 ++--
drivers/md/persistent-data/dm-block-manager.c | 6 +--
include/linux/dm-bufio.h | 5 +--
include/linux/dm-io.h | 3 +-
17 files changed, 85 insertions(+), 76 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 6de107aff331..b67fec865f00 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1683,6 +1683,7 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned int size) GFP_NOIO, &cc->bs); clone->bi_private = io; clone->bi_end_io = crypt_endio; + clone->bi_ioprio = bio_prio(io->base_bio); remaining_size = size;