On Tue, Jun 30, 2020 at 07:17:25AM +0800, Qu Wenruo wrote: > On 2020/6/30 上午5:30, David Sterba wrote: > > On Sun, Jun 28, 2020 at 01:07:15PM +0800, Qu Wenruo wrote: > >> +QGROUP_ATTR(rfer, reference); > > > > Note that this is 'referenced'. > > > >> +QGROUP_ATTR(excl, exclusive); > >> +QGROUP_ATTR(max_rfer, max_reference); > > > > And here max_referenced. > > > >> +QGROUP_ATTR(max_excl, max_exclusive); > >> +QGROUP_ATTR(lim_flags, limit_flags); > >> +QGROUP_RSV_ATTR(data, BTRFS_QGROUP_RSV_DATA); > >> +QGROUP_RSV_ATTR(meta_pertrans, BTRFS_QGROUP_RSV_META_PERTRANS); > >> +QGROUP_RSV_ATTR(meta_prealloc, BTRFS_QGROUP_RSV_META_PREALLOC); > > > > The two above fixed but otherwise it's good, thanks. > > > > The qgroup membership and relations could be added to the sysfs export > > too, but we're limited by the PAGE_SIZE output buffer so the information > > could be incomplete. > > > Yep, PAGE_SIZE is one limitation. > But we can also go another direction, just using a new dir for related > qgroups, then we can workaround the limitation. > > But personally speaking, the main objective is still the rsv_* members > for debug. > I don't really want to turn the sysfs interface into a way to export all > qgroup info yet. Well, that's a bit of a problem, if you're designing a public interface for your own needs and neglecting that it will be used as a way to read qgroup information. Sooner or later someone will ask for the additional information to be added. If it's just for debugging then it should be under 'debug', but as the bug reporter does not want to apply patches/change config you ask to make it visible unconditionally. I'm ok with that as long as the interface is done right because any mistake will be potentially much harder to fix in the future. We need to at lest have a solid idea how to extend it, not neccessarily to implement it right now. Adding the group membership to qgroup directories under the qgroup directory as symlinks between the kobjects sounds ok-ish to me but it's a new idea and I need to think about it.
