| Message ID | 20220412085616.1409626-4-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | dm: io accounting & polling improvement | expand |
On Tue, Apr 12 2022 at 4:56P -0400, Ming Lei <ming.lei@redhat.com> wrote: > All the other 4 parameters are retrieved from the 'dm_io' instance, so > not necessary to pass all four to dm_io_acct(). > > Signed-off-by: Ming Lei <ming.lei@redhat.com> Yeah, commit 0ab30b4079e103 ("dm: eliminate copying of dm_io fields in dm_io_dec_pending") could've gone further to do what you've done here in this patch. But it stopped short because of the additional "games" associated with the late assignment of io->orig_bio that is in the dm-5.19 branch. Mike > --- > drivers/md/dm.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/md/dm.c b/drivers/md/dm.c > index 62f7af815ef8..ed85cd1165a4 100644 > --- a/drivers/md/dm.c > +++ b/drivers/md/dm.c > @@ -498,9 +498,12 @@ static bool bio_is_flush_with_data(struct bio *bio) > return ((bio->bi_opf & REQ_PREFLUSH) && bio->bi_iter.bi_size); > } > > -static void dm_io_acct(bool end, struct mapped_device *md, struct bio *bio, > - unsigned long start_time, struct dm_stats_aux *stats_aux) > +static void dm_io_acct(struct dm_io *io, bool end) > { > + struct dm_stats_aux *stats_aux = &io->stats_aux; > + unsigned long start_time = io->start_time; > + struct mapped_device *md = io->md; > + struct bio *bio = io->orig_bio; > bool is_flush_with_data; > unsigned int bi_size; > > @@ -528,7 +531,7 @@ static void dm_io_acct(bool end, struct mapped_device *md, struct bio *bio, > > static void __dm_start_io_acct(struct dm_io *io) > { > - dm_io_acct(false, io->md, io->orig_bio, io->start_time, &io->stats_aux); > + dm_io_acct(io, false); > } > > static void dm_start_io_acct(struct dm_io *io, struct bio *clone) > @@ -557,7 +560,7 @@ static void dm_start_io_acct(struct dm_io *io, struct bio *clone) > > static void dm_end_io_acct(struct dm_io *io) > { > - dm_io_acct(true, io->md, io->orig_bio, io->start_time, &io->stats_aux); > + dm_io_acct(io, true); > } > > static struct dm_io *alloc_io(struct mapped_device *md, struct bio *bio) > -- > 2.31.1 > > -- > dm-devel mailing list > dm-devel@redhat.com > https://listman.redhat.com/mailman/listinfo/dm-devel > -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
On Tue, Apr 12, 2022 at 04:28:59PM -0400, Mike Snitzer wrote: > On Tue, Apr 12 2022 at 4:56P -0400, > Ming Lei <ming.lei@redhat.com> wrote: > > > All the other 4 parameters are retrieved from the 'dm_io' instance, so > > not necessary to pass all four to dm_io_acct(). > > > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > Yeah, commit 0ab30b4079e103 ("dm: eliminate copying of dm_io fields in > dm_io_dec_pending") could've gone further to do what you've done here > in this patch. > > But it stopped short because of the additional "games" associated with > the late assignment of io->orig_bio that is in the dm-5.19 branch. OK, I will rebase on dm-5.19, but IMO the idea of late assignment of io->orig_bio isn't good, same with splitting one bio just for accounting, things shouldn't be so tricky. Thanks, Ming -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 62f7af815ef8..ed85cd1165a4 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -498,9 +498,12 @@ static bool bio_is_flush_with_data(struct bio *bio) return ((bio->bi_opf & REQ_PREFLUSH) && bio->bi_iter.bi_size); } -static void dm_io_acct(bool end, struct mapped_device *md, struct bio *bio, - unsigned long start_time, struct dm_stats_aux *stats_aux) +static void dm_io_acct(struct dm_io *io, bool end) { + struct dm_stats_aux *stats_aux = &io->stats_aux; + unsigned long start_time = io->start_time; + struct mapped_device *md = io->md; + struct bio *bio = io->orig_bio; bool is_flush_with_data; unsigned int bi_size; @@ -528,7 +531,7 @@ static void dm_io_acct(bool end, struct mapped_device *md, struct bio *bio, static void __dm_start_io_acct(struct dm_io *io) { - dm_io_acct(false, io->md, io->orig_bio, io->start_time, &io->stats_aux); + dm_io_acct(io, false); } static void dm_start_io_acct(struct dm_io *io, struct bio *clone) @@ -557,7 +560,7 @@ static void dm_start_io_acct(struct dm_io *io, struct bio *clone) static void dm_end_io_acct(struct dm_io *io) { - dm_io_acct(true, io->md, io->orig_bio, io->start_time, &io->stats_aux); + dm_io_acct(io, true); } static struct dm_io *alloc_io(struct mapped_device *md, struct bio *bio)
All the other 4 parameters are retrieved from the 'dm_io' instance, so not necessary to pass all four to dm_io_acct(). Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/md/dm.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)