Messages in this thread | | | Subject | Re: [PATCH 01/16] block: add disk/bio-based accounting helpers | From | Konstantin Khlebnikov <> | Date | Mon, 25 May 2020 15:28:07 +0300 |
| |
On 25/05/2020 14.29, Christoph Hellwig wrote: > Add two new helpers to simplify I/O accounting for bio based drivers. > Currently these drivers use the generic_start_io_acct and > generic_end_io_acct helpers which have very cumbersome calling > conventions, don't actually return the time they started accounting, > and try to deal with accounting for partitions, which can't happen > for bio based drivers. The new helpers will be used to subsequently > replace uses of the old helpers. > > The main function is the bio based wrappes in blkdev.h, but for zram > which wants to account rw_page based I/O lower level routines are > provided as well. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/blk-core.c | 34 ++++++++++++++++++++++++++++++++++ > include/linux/blkdev.h | 26 ++++++++++++++++++++++++++ > 2 files changed, 60 insertions(+) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 77e57c2e8d602..8973104f88d90 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -1432,6 +1432,40 @@ void blk_account_io_start(struct request *rq, bool new_io) > part_stat_unlock(); > } > > +unsigned long disk_start_io_acct(struct gendisk *disk, unsigned int sectors, > + unsigned int op) > +{ > + struct hd_struct *part = &disk->part0; > + const int sgrp = op_stat_group(op); > + unsigned long now = READ_ONCE(jiffies); > + > + part_stat_lock(); > + update_io_ticks(part, now, false); > + part_stat_inc(part, ios[sgrp]); > + part_stat_add(part, sectors[sgrp], sectors); > + part_stat_local_inc(part, in_flight[op_is_write(op)]); > + part_stat_unlock(); > + > + return now; > +} > +EXPORT_SYMBOL(disk_start_io_acct); > + > +void disk_end_io_acct(struct gendisk *disk, unsigned int op, > + unsigned long start_time) > +{ > + struct hd_struct *part = &disk->part0; > + const int sgrp = op_stat_group(op); > + unsigned long now = READ_ONCE(jiffies); > + unsigned long duration = now - start_time;
I think it would be better to leave this jiffies legacy nonsense in callers and pass here request duration in nanoseconds.
So rewriting them to nanoseconds later wouldn't touch generic code.
> + > + part_stat_lock(); > + update_io_ticks(part, now, true); > + part_stat_add(part, nsecs[sgrp], jiffies_to_nsecs(duration)); > + part_stat_local_dec(part, in_flight[op_is_write(op)]); > + part_stat_unlock(); > +} > +EXPORT_SYMBOL(disk_end_io_acct); > + > /* > * Steal bios from a request and add them to a bio list. > * The request must not have been partially completed before. > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index 7d10f4e632325..76d01a8a13b80 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -1892,4 +1892,30 @@ static inline void blk_wake_io_task(struct task_struct *waiter) > wake_up_process(waiter); > } > > +unsigned long disk_start_io_acct(struct gendisk *disk, unsigned int sectors, > + unsigned int op); > +void disk_end_io_acct(struct gendisk *disk, unsigned int op, > + unsigned long start_time); > + > +/** > + * bio_start_io_acct - start I/O accounting for bio based drivers > + * @bio: bio to start account for > + * > + * Returns the start time that should be passed back to bio_end_io_acct(). > + */ > +static inline unsigned long bio_start_io_acct(struct bio *bio) > +{ > + return disk_start_io_acct(bio->bi_disk, bio_sectors(bio), bio_op(bio)); > +} > + > +/** > + * bio_end_io_acct - end I/O accounting for bio based drivers > + * @bio: bio to end account for > + * @start: start time returned by bio_start_io_acct() > + */ > +static inline void bio_end_io_acct(struct bio *bio, unsigned long start_time) > +{ > + return disk_end_io_acct(bio->bi_disk, bio_op(bio), start_time); > +} > + > #endif >
| |