On Wed, Oct 20, 2021 at 6:13 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Wed, 2021-10-20 at 18:04 +0530, Venky Shankar wrote: > > On Tue, Oct 19, 2021 at 2:02 PM Ilya Dryomov <idryomov@xxxxxxxxx> wrote: > > > > > > On Fri, Oct 1, 2021 at 7:05 AM Venky Shankar <vshankar@xxxxxxxxxx> wrote: > > > > > > > > v4: > > > > - use mount_syntax_v1,.. as file names > > > > > > > > [This is based on top of new mount syntax series] > > > > > > > > Patrick proposed the idea of having debugfs entries to signify if > > > > kernel supports the new (v2) mount syntax. The primary use of this > > > > information is to catch any bugs in the new syntax implementation. > > > > > > > > This would be done as follows:: > > > > > > > > The userspace mount helper tries to mount using the new mount syntax > > > > and fallsback to using old syntax if the mount using new syntax fails. > > > > However, a bug in the new mount syntax implementation can silently > > > > result in the mount helper switching to old syntax. > > > > > > > > So, the debugfs entries can be relied upon by the mount helper to > > > > check if the kernel supports the new mount syntax. Cases when the > > > > mount using the new syntax fails, but the kernel does support the > > > > new mount syntax, the mount helper could probably log before switching > > > > to the old syntax (or fail the mount altogether when run in test mode). > > > > > > > > Debugfs entries are as follows:: > > > > > > > > /sys/kernel/debug/ceph/ > > > > .... > > > > .... > > > > /sys/kernel/debug/ceph/meta > > > > /sys/kernel/debug/ceph/meta/client_features > > > > /sys/kernel/debug/ceph/meta/client_features/mount_syntax_v2 > > > > /sys/kernel/debug/ceph/meta/client_features/mount_syntax_v1 > > > > .... > > > > .... > > > > > > Hi Venky, Jeff, > > > > > > If this is supposed to be used in the wild and not just in teuthology, > > > I would be wary of going with debugfs. debugfs isn't always available > > > (it is actually compiled out in some configurations, it may or may not > > > be mounted, etc). With the new mount syntax feature it is not a big > > > deal because the mount helper should do just fine without it but with > > > other features we may find ourselves in a situation where the mount > > > helper (or something else) just *has* to know whether the feature is > > > supported or not and falling back to "no" if debugfs is not available > > > is undesirable or too much work. > > > > > > I don't have a great suggestion though. When I needed to do this in > > > the past for RADOS feature bits, I went with a read-only kernel module > > > parameter [1]. They are exported via sysfs which is guaranteed to be > > > available. Perhaps we should do the same for mount_syntax -- have it > > > be either 1 or 2, allowing it to be revved in the future? > > > > I'm ok with exporting via sysfs (since it's guaranteed). My only ask > > here would be to have the mount support information present itself as > > files rather than file contents to avoid writing parsing stuff in > > userspace, which is ok, however, relying on stat() is nicer. > > > > You should be able to do that by just making a read-only parameter > called "mount_syntax_v2", and then you can test for it by doing > something like: > > # stat /sys/module/ceph/parameters/mount_syntax_v2 > > The contents of the file can be blank (or just return Y or something). Yep. That's fine. So, we no longer have these entries in subdirectories (meta/client_features/...) and those end up under parameters? I do see subdirectories under other modules, so it's probably doable with sysfs. > > > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d6a3408a77807037872892c2a2034180fcc08d12 > > > > > > Thanks, > > > > > > Ilya > > > > > > > > > -- > Jeff Layton <jlayton@xxxxxxxxxx> > -- Cheers, Venky