| Message ID | 20220305160424.1040102-7-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Generic per-sb io stats | expand |
On Sat, 5 Mar 2022 at 17:04, Amir Goldstein <amir73il@gmail.com> wrote: > > Show optional collected per-sb io stats in /proc/<pid>/mountstats > for filesystems that do not implement their own show_stats() method > and have generic per-sb stats enabled. I still think it's wrong to extend the /proc/*/mount* family of interfaces. The big issue is that the kernel has to generate this info for *all* mounts, even though the user may be just looking for information on a specific mount. Your workaround is to enable the info generation for only a subset of fs types, but this doesn't solve the fundamental issue. So let's please implement a per-mount interface. Yes, it's a much bigger project, but one which needs to be done at one point, and which would be generally useful. There was tons of discussion around this when dhowells tried to create one, and there was a suggestion by Linus which I think all parties found acceptable: - return basic info in a binary format (similar to e.g. statx) - after the fix binary structure allow misc info to be added using an ascii format And since generating the info may be expensive in some cases, the interface would need to allow selective queries (which statx does for binary fields, but for ascii ones this is a new challenge). I think allowing the user to query the list of supported fields should also be possible (again, statx supports this). So there are a number of requirements for this interface, and I'm not quite sure what the best solution is. I can try to put something together if there are no objections... Thanks, Miklos
On Thu, Mar 10, 2022 at 11:54 AM Miklos Szeredi <miklos@szeredi.hu> wrote: > > On Sat, 5 Mar 2022 at 17:04, Amir Goldstein <amir73il@gmail.com> wrote: > > > > Show optional collected per-sb io stats in /proc/<pid>/mountstats > > for filesystems that do not implement their own show_stats() method > > and have generic per-sb stats enabled. > > I still think it's wrong to extend the /proc/*/mount* family of > interfaces. The big issue is that the kernel has to generate this > info for *all* mounts, even though the user may be just looking for > information on a specific mount. Your workaround is to enable the > info generation for only a subset of fs types, but this doesn't solve > the fundamental issue. > > So let's please implement a per-mount interface. Yes, it's a much > bigger project, but one which needs to be done at one point, and which > would be generally useful. There was tons of discussion around this > when dhowells tried to create one, and there was a suggestion by Linus > which I think all parties found acceptable: > > - return basic info in a binary format (similar to e.g. statx) > - after the fix binary structure allow misc info to be added using an > ascii format > > And since generating the info may be expensive in some cases, the > interface would need to allow selective queries (which statx does for > binary fields, but for ascii ones this is a new challenge). > > I think allowing the user to query the list of supported fields should > also be possible (again, statx supports this). > > So there are a number of requirements for this interface, and I'm not > quite sure what the best solution is. I can try to put something > together if there are no objections... No objections on my part. Feel free to use the iostat info as a case study for how common vfs info could be exported using this interface. Thanks, Amir.
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c index 49650e54d2f8..9054a909e031 100644 --- a/fs/proc_namespace.c +++ b/fs/proc_namespace.c @@ -11,6 +11,7 @@ #include <linux/nsproxy.h> #include <linux/security.h> #include <linux/fs_struct.h> +#include <linux/fs_iostats.h> #include <linux/sched/task.h> #include "proc/internal.h" /* only for get_proc_task() in ->open() */ @@ -232,6 +233,21 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt) if (sb->s_op->show_stats) { seq_putc(m, ' '); err = sb->s_op->show_stats(m, mnt_path.dentry); + } else if (sb_has_iostats(sb)) { + struct sb_iostats *iostats = sb_iostats(sb); + + /* Similar to /proc/<pid>/io */ + seq_printf(m, "\n" + "\ttimes: %lld %lld\n" + "\trchar: %lld\n" + "\twchar: %lld\n" + "\tsyscr: %lld\n" + "\tsyscw: %lld\n", + iostats->start_time, ktime_get_seconds(), + sb_iostats_counter_read(sb, SB_IOSTATS_CHARS_RD), + sb_iostats_counter_read(sb, SB_IOSTATS_CHARS_WR), + sb_iostats_counter_read(sb, SB_IOSTATS_SYSCALLS_RD), + sb_iostats_counter_read(sb, SB_IOSTATS_SYSCALLS_WR)); } seq_putc(m, '\n');
Show optional collected per-sb io stats in /proc/<pid>/mountstats for filesystems that do not implement their own show_stats() method and have generic per-sb stats enabled. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/proc_namespace.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)