Messages in this thread | | | Subject | Re: [f2fs-dev] [PATCH v4 2/2] f2fs: introduce periodic iostat io latency traces | From | Chao Yu <> | Date | Sat, 21 Aug 2021 05:41:38 +0800 |
| |
On 2021/8/20 23:23, Daeho Jeong wrote: > On Fri, Aug 20, 2021 at 3:50 AM Chao Yu <chao@kernel.org> wrote: >> >> On 2021/8/20 11:52, Daeho Jeong wrote: >>> +void iostat_update_and_unbind_ctx(struct bio *bio, int rw) >>> +{ >>> + struct bio_iostat_ctx *iostat_ctx = bio->bi_private; >>> + int sync_type = bio->bi_opf & REQ_SYNC ? 0 : 1; >> >> int sync_type = bio->bi_opf & REQ_SYNC ? 1 : 0; >> >> Right? > > This means just type, not boolean number. So, I set type 0 is sync and > type 1 is async.
How about changing this to is_sync or similar name of bool type variable?
> >> >>> int f2fs_init_iostat(struct f2fs_sb_info *sbi) >>> { >>> /* init iostat info */ >>> spin_lock_init(&sbi->iostat_lock); >>> + spin_lock_init(&sbi->iostat_lat_lock); >>> sbi->iostat_enable = false; >>> sbi->iostat_period_ms = DEFAULT_IOSTAT_PERIOD_MS; >>> + sbi->iostat_io_lat = f2fs_kzalloc(sbi, sizeof(struct iostat_lat_info), >>> + GFP_KERNEL); >>> + if (!sbi->iostat_io_lat) >>> + return -ENOMEM; >> >> What do you think of just embedding iostat_io_lat structure into f2fs_sb_info >> structure? it's minor thing though. >> > > I also wanted to do that, but if we embed this type, we need to define > that structure in f2fs.h file. > Is it okay with you?
Oh, correct, it will be more appropriate to define the structure in iostat.h.
Is it fine to just use memset(io_lat, 0, sizeof(struct iostat_lat_info)) to reset all fields in f2fs_reset_iostat()?
Thanks,
> >> Thanks,
| |