On Wed, Mar 2, 2022 at 10:27 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > On Wed, Mar 02, 2022 at 09:43:50AM +0200, Amir Goldstein wrote: > > On Wed, Mar 2, 2022 at 8:59 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > > > > > On Tue, Mar 01, 2022 at 08:42:15PM +0200, Amir Goldstein wrote: > > > > Miklos, > > > > > > > > Following your feedback on v2 [1], I moved the iostats to per-sb. > > > > > > > > Thanks, > > > > Amir. > > > > > > > > [1] https://lore.kernel.org/linux-unionfs/20220228113910.1727819-1-amir73il@xxxxxxxxx/ > > > > > > > > Changes since v2: > > > > - Change from per-mount to per-sb io stats (szeredi) > > > > - Avoid percpu loop when reading mountstats (dchinner) > > > > > > > > Changes since v1: > > > > - Opt-in for per-mount io stats for overlayfs and fuse > > > > > > Why make it optional only for specific filesystem types? Shouldn't > > > every superblock capture these stats and export them in exactly the > > > same place? > > > > > > > I am not sure what you are asking. > > > > Any filesystem can opt-in to get those generic io stats. > > Yes, but why even make it opt-in? Why not just set these up > unconditionally in alloc_super() for all filesystems? Either this is > useful information for userspace montioring and diagnostics, or it's > not useful at all. If it is useful, then all superblocks should > export this stuff rather than just some special subset of > filesystems where individual maintainers have noticed it and thought > "that might be useful". > > Just enable it for every superblock.... > > > This is exactly the same as any filesystem can already opt-in for > > fs specific io stats using the s_op->show_stats() vfs op. > > > > All I did was to provide a generic implementation. > > The generic io stats are collected and displayed for all filesystems the > > same way. > > > > I only included patches for overlayfs and fuse to opt-in for generic io stats, > > because I think those stats should be reported unconditionally (to > > mount options) > > for fuse/overlayfs and I hope that Miklos agrees with me. > > Yup, and I'm asking you why it should be optional - no filesystem > ever sees this information - it's totally generic VFS level code > except for the structure allocation. What's the point of *not* > enabling it for every superblock unconditionally? > > > If there is wide consensus that all filesystems should have those stats > > unconditionally (to mount options), then I can post another patch to make > > the behavior not opt-in, but I have a feeling that this discussion > > That's exactly what I want you to do. We're already having this > discussion, so let's get it over and done with right now. > > > How would you prefer the io stats behavior for xfs (or any fs) to be? > > Unconditional to mount options? > > Opt-in with mount option? (suggest name please) > > Opt-in/out with mount options and default with Kconfig/sysfs tunable? > > Anything else? > > Unconditional, for all filesystems, so they all display the same > stats in exactly same place without any filesystem having to > implement a single line of code anywhere. A single kconfig option > like you already hav is just fine to turn it off for those that > don't want to use it. > Very well then. I'll post this version with your Suggested-by ;-) Thanks, Amir.
