| Message ID | 1578289125-2491-1-git-send-email-xuyang2018.jy@cn.fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tst_dev_bytes_written: parsing /proc/diskstats instead of sys file | expand |
On Mon, 6 Jan 2020 at 11:09, Yang Xu <xuyang2018.jy@cn.fujitsu.com> wrote: > > Now, tst_dev_bytes_written api can get the whole disk stat correctly(such as /dev/sda), > but can not get partition stat correctly(such as /dev/sda5). > fail as below: > export LTP_DEV=/dev/sda5 > tst_device.c:388: CONF: Test device stat file: /sys/block/sda5/stat not found > > To fix this, use /proc/diskstats to parse instead of /sys/block/<devices>/stat. > Data format as same as diskstats_show function in kernel genhd.c[1]. > > Also, add hint when loop driver doesn't support blk-mq[2]. So that, user can export > LTP_DEV to test. > > [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/genhd.c > [2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b5dd2f60 > > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> > --- > lib/tst_device.c | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > Overall patch looks good to me but with minor comments below. > diff --git a/lib/tst_device.c b/lib/tst_device.c > index 10f71901d..58877c810 100644 > --- a/lib/tst_device.c > +++ b/lib/tst_device.c > @@ -375,24 +375,22 @@ int tst_umount(const char *path) > > unsigned long tst_dev_bytes_written(const char *dev) > { > - struct stat st; > - unsigned long dev_sec_write = 0, dev_bytes_written, io_ticks = 0; > - char dev_stat_path[1024]; > - > - snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat", > - strrchr(dev, '/') + 1); > + unsigned long dev_sec_write = 0, dev_bytes_written; > + unsigned int io_ticks = 0; > + char fmt[1024]; > > - if (stat(dev_stat_path, &st) != 0) > - tst_brkm(TCONF, NULL, "Test device stat file: %s not found", > - dev_stat_path); > + sprintf(fmt, "%%*4d %%*7d %s %%*lu %%*lu %%*lu %%*u %%*lu %%*lu %%lu %%*u %%*u %%u", > + strrchr(dev, '/') + 1); > > - SAFE_FILE_SCANF(NULL, dev_stat_path, > - "%*s %*s %*s %*s %*s %*s %lu %*s %*s %lu", > - &dev_sec_write, &io_ticks); > + FILE_LINES_SCANF(NULL, "/proc/diskstats", fmt, &dev_sec_write, &io_ticks); Shouldn't we use SAFE_FILE_LINES_SCANF() here? > > + /* If we create block device by attaching a free loop device, loop driver > + * needs to support blk-mq(commit b5dd2f6047c), so that kernel can get I/O > + * statistics about loop device. > + */ > if (!io_ticks) > - tst_brkm(TCONF, NULL, "Test device stat file: %s broken", > - dev_stat_path); > + tst_brkm(TCONF, NULL, "Test device %s io_ticks is always 0, " > + "exporting LTP_DEV to test", dev); s/exporting/export/ -Sumit > > dev_bytes_written = (dev_sec_write - prev_dev_sec_write) * 512; > > -- > 2.18.0 > > > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
on 2020/01/06 15:57, Sumit Garg wrote: > On Mon, 6 Jan 2020 at 11:09, Yang Xu <xuyang2018.jy@cn.fujitsu.com> wrote: >> >> Now, tst_dev_bytes_written api can get the whole disk stat correctly(such as /dev/sda), >> but can not get partition stat correctly(such as /dev/sda5). >> fail as below: >> export LTP_DEV=/dev/sda5 >> tst_device.c:388: CONF: Test device stat file: /sys/block/sda5/stat not found >> >> To fix this, use /proc/diskstats to parse instead of /sys/block/<devices>/stat. >> Data format as same as diskstats_show function in kernel genhd.c[1]. >> >> Also, add hint when loop driver doesn't support blk-mq[2]. So that, user can export >> LTP_DEV to test. >> >> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/genhd.c >> [2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b5dd2f60 >> >> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> >> --- >> lib/tst_device.c | 26 ++++++++++++-------------- >> 1 file changed, 12 insertions(+), 14 deletions(-) >> > > Overall patch looks good to me but with minor comments below. Thanks for your review. > >> diff --git a/lib/tst_device.c b/lib/tst_device.c >> index 10f71901d..58877c810 100644 >> --- a/lib/tst_device.c >> +++ b/lib/tst_device.c >> @@ -375,24 +375,22 @@ int tst_umount(const char *path) >> >> unsigned long tst_dev_bytes_written(const char *dev) >> { >> - struct stat st; >> - unsigned long dev_sec_write = 0, dev_bytes_written, io_ticks = 0; >> - char dev_stat_path[1024]; >> - >> - snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat", >> - strrchr(dev, '/') + 1); >> + unsigned long dev_sec_write = 0, dev_bytes_written; >> + unsigned int io_ticks = 0; >> + char fmt[1024]; >> >> - if (stat(dev_stat_path, &st) != 0) >> - tst_brkm(TCONF, NULL, "Test device stat file: %s not found", >> - dev_stat_path); >> + sprintf(fmt, "%%*4d %%*7d %s %%*lu %%*lu %%*lu %%*u %%*lu %%*lu %%lu %%*u %%*u %%u", >> + strrchr(dev, '/') + 1); >> >> - SAFE_FILE_SCANF(NULL, dev_stat_path, >> - "%*s %*s %*s %*s %*s %*s %lu %*s %*s %lu", >> - &dev_sec_write, &io_ticks); >> + FILE_LINES_SCANF(NULL, "/proc/diskstats", fmt, &dev_sec_write, &io_ticks); > > Shouldn't we use SAFE_FILE_LINES_SCANF() here? Yes. I will use SAFE_FILE_LINES_SCANF() in v2. > >> >> + /* If we create block device by attaching a free loop device, loop driver >> + * needs to support blk-mq(commit b5dd2f6047c), so that kernel can get I/O >> + * statistics about loop device. >> + */ >> if (!io_ticks) >> - tst_brkm(TCONF, NULL, "Test device stat file: %s broken", >> - dev_stat_path); >> + tst_brkm(TCONF, NULL, "Test device %s io_ticks is always 0, " >> + "exporting LTP_DEV to test", dev); > > s/exporting/export/ ... > > -Sumit > >> >> dev_bytes_written = (dev_sec_write - prev_dev_sec_write) * 512; >> >> -- >> 2.18.0 >> >> >> >> >> -- >> Mailing list info: https://lists.linux.it/listinfo/ltp > >
diff --git a/lib/tst_device.c b/lib/tst_device.c index 10f71901d..58877c810 100644 --- a/lib/tst_device.c +++ b/lib/tst_device.c @@ -375,24 +375,22 @@ int tst_umount(const char *path) unsigned long tst_dev_bytes_written(const char *dev) { - struct stat st; - unsigned long dev_sec_write = 0, dev_bytes_written, io_ticks = 0; - char dev_stat_path[1024]; - - snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat", - strrchr(dev, '/') + 1); + unsigned long dev_sec_write = 0, dev_bytes_written; + unsigned int io_ticks = 0; + char fmt[1024]; - if (stat(dev_stat_path, &st) != 0) - tst_brkm(TCONF, NULL, "Test device stat file: %s not found", - dev_stat_path); + sprintf(fmt, "%%*4d %%*7d %s %%*lu %%*lu %%*lu %%*u %%*lu %%*lu %%lu %%*u %%*u %%u", + strrchr(dev, '/') + 1); - SAFE_FILE_SCANF(NULL, dev_stat_path, - "%*s %*s %*s %*s %*s %*s %lu %*s %*s %lu", - &dev_sec_write, &io_ticks); + FILE_LINES_SCANF(NULL, "/proc/diskstats", fmt, &dev_sec_write, &io_ticks); + /* If we create block device by attaching a free loop device, loop driver + * needs to support blk-mq(commit b5dd2f6047c), so that kernel can get I/O + * statistics about loop device. + */ if (!io_ticks) - tst_brkm(TCONF, NULL, "Test device stat file: %s broken", - dev_stat_path); + tst_brkm(TCONF, NULL, "Test device %s io_ticks is always 0, " + "exporting LTP_DEV to test", dev); dev_bytes_written = (dev_sec_write - prev_dev_sec_write) * 512;
Now, tst_dev_bytes_written api can get the whole disk stat correctly(such as /dev/sda), but can not get partition stat correctly(such as /dev/sda5). fail as below: export LTP_DEV=/dev/sda5 tst_device.c:388: CONF: Test device stat file: /sys/block/sda5/stat not found To fix this, use /proc/diskstats to parse instead of /sys/block/<devices>/stat. Data format as same as diskstats_show function in kernel genhd.c[1]. Also, add hint when loop driver doesn't support blk-mq[2]. So that, user can export LTP_DEV to test. [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/genhd.c [2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b5dd2f60 Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> --- lib/tst_device.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)