ceph-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/2] ceph: add debugfs entries signifying new mount syntax support
@ 2021-08-18  6:01 Venky Shankar
  2021-08-18  6:01 ` [RFC 1/2] ceph: add helpers to create/cleanup debugfs sub-directories under "ceph" directory Venky Shankar
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Venky Shankar @ 2021-08-18  6:01 UTC (permalink / raw)
  To: jlayton, pdonnell; +Cc: ceph-devel, Venky Shankar

[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/dev_support
    /sys/kernel/debug/ceph/dev_support/v2
    ....
    ....

Note that there is no entry signifying v1 mount syntax. That's because
the kernel still supports mounting with old syntax and older kernels do
not have debug entries for the same.

Venky Shankar (2):
  ceph: add helpers to create/cleanup debugfs sub-directories under
    "ceph" directory
  ceph: add debugfs entries for v2 (new) mount syntax support

 fs/ceph/debugfs.c            | 28 ++++++++++++++++++++++++++++
 fs/ceph/super.c              |  3 +++
 fs/ceph/super.h              |  2 ++
 include/linux/ceph/debugfs.h |  3 +++
 net/ceph/debugfs.c           | 26 ++++++++++++++++++++++++--
 5 files changed, 60 insertions(+), 2 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC 1/2] ceph: add helpers to create/cleanup debugfs sub-directories under "ceph" directory
  2021-08-18  6:01 [RFC 0/2] ceph: add debugfs entries signifying new mount syntax support Venky Shankar
@ 2021-08-18  6:01 ` Venky Shankar
  2021-08-18 11:34   ` Venky Shankar
  2021-08-18  6:01 ` [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support Venky Shankar
  2021-08-18 13:09 ` [RFC 0/2] ceph: add debugfs entries signifying new " Jeff Layton
  2 siblings, 1 reply; 11+ messages in thread
From: Venky Shankar @ 2021-08-18  6:01 UTC (permalink / raw)
  To: jlayton, pdonnell; +Cc: ceph-devel, Venky Shankar

Callers can use this helper to create a subdirectory under
"ceph" directory in debugfs to place custom files for exporting
information to userspace.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
---
 include/linux/ceph/debugfs.h |  3 +++
 net/ceph/debugfs.c           | 26 ++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/linux/ceph/debugfs.h b/include/linux/ceph/debugfs.h
index 8b3a1a7a953a..c372e6cb8aae 100644
--- a/include/linux/ceph/debugfs.h
+++ b/include/linux/ceph/debugfs.h
@@ -10,5 +10,8 @@ extern void ceph_debugfs_cleanup(void);
 extern void ceph_debugfs_client_init(struct ceph_client *client);
 extern void ceph_debugfs_client_cleanup(struct ceph_client *client);
 
+extern struct dentry *ceph_debugfs_create_subdir(const char *subdir);
+extern void ceph_debugfs_cleanup_subdir(struct dentry *subdir_dentry);
+
 #endif
 
diff --git a/net/ceph/debugfs.c b/net/ceph/debugfs.c
index 2110439f8a24..eabac20b3ff4 100644
--- a/net/ceph/debugfs.c
+++ b/net/ceph/debugfs.c
@@ -404,6 +404,18 @@ void ceph_debugfs_cleanup(void)
 	debugfs_remove(ceph_debugfs_dir);
 }
 
+struct dentry *ceph_debugfs_create_subdir(const char *subdir)
+{
+	return debugfs_create_dir(subdir, ceph_debugfs_dir);
+}
+EXPORT_SYMBOL(ceph_debugfs_create_subdir);
+
+void ceph_debugfs_cleanup_subdir(struct dentry *subdir_dentry)
+{
+	debugfs_remove(subdir_dentry);
+}
+EXPORT_SYMBOL(ceph_debugfs_cleanup_subdir);
+
 void ceph_debugfs_client_init(struct ceph_client *client)
 {
 	char name[80];
@@ -413,7 +425,7 @@ void ceph_debugfs_client_init(struct ceph_client *client)
 
 	dout("ceph_debugfs_client_init %p %s\n", client, name);
 
-	client->debugfs_dir = debugfs_create_dir(name, ceph_debugfs_dir);
+	client->debugfs_dir = ceph_debugfs_create_subdir(name);
 
 	client->monc.debugfs_file = debugfs_create_file("monc",
 						      0400,
@@ -454,7 +466,7 @@ void ceph_debugfs_client_cleanup(struct ceph_client *client)
 	debugfs_remove(client->debugfs_monmap);
 	debugfs_remove(client->osdc.debugfs_file);
 	debugfs_remove(client->monc.debugfs_file);
-	debugfs_remove(client->debugfs_dir);
+	ceph_debugfs_cleanup_subdir(client->debugfs_dir);
 }
 
 #else  /* CONFIG_DEBUG_FS */
@@ -475,4 +487,14 @@ void ceph_debugfs_client_cleanup(struct ceph_client *client)
 {
 }
 
+struct dentry *ceph_debugfs_create_subdir(const char *subdir)
+{
+}
+EXPORT_SYMBOL(ceph_debugfs_create_subdir);
+
+void ceph_debugfs_cleanup_subdir(struct dentry *subdir_dentry)
+{
+}
+EXPORT_SYMBOL(ceph_debugfs_cleanup_subdir);
+
 #endif  /* CONFIG_DEBUG_FS */
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support
  2021-08-18  6:01 [RFC 0/2] ceph: add debugfs entries signifying new mount syntax support Venky Shankar
  2021-08-18  6:01 ` [RFC 1/2] ceph: add helpers to create/cleanup debugfs sub-directories under "ceph" directory Venky Shankar
@ 2021-08-18  6:01 ` Venky Shankar
  2021-08-21  1:52   ` Patrick Donnelly
  2021-08-18 13:09 ` [RFC 0/2] ceph: add debugfs entries signifying new " Jeff Layton
  2 siblings, 1 reply; 11+ messages in thread
From: Venky Shankar @ 2021-08-18  6:01 UTC (permalink / raw)
  To: jlayton, pdonnell; +Cc: ceph-devel, Venky Shankar

Signed-off-by: Venky Shankar <vshankar@redhat.com>
---
 fs/ceph/debugfs.c | 28 ++++++++++++++++++++++++++++
 fs/ceph/super.c   |  3 +++
 fs/ceph/super.h   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 66989c880adb..d19f15ace781 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -22,6 +22,12 @@
 #include "mds_client.h"
 #include "metric.h"
 
+#define MNT_DEV_SUPPORT_DIR   "dev_support"
+#define MNT_DEV_V2_FILE  "v2"
+
+static struct dentry *ceph_mnt_dev_support_dir;
+static struct dentry *ceph_mnt_dev_v2_file;
+
 static int mdsmap_show(struct seq_file *s, void *p)
 {
 	int i;
@@ -416,6 +422,20 @@ void ceph_fs_debugfs_init(struct ceph_fs_client *fsc)
 						  &status_fops);
 }
 
+void ceph_fs_debugfs_mnt_dev_init(void)
+{
+	ceph_mnt_dev_support_dir = ceph_debugfs_create_subdir(MNT_DEV_SUPPORT_DIR);
+	ceph_mnt_dev_v2_file = debugfs_create_file(MNT_DEV_V2_FILE,
+						   0400,
+						   ceph_mnt_dev_support_dir,
+						   NULL, NULL);
+}
+
+void ceph_fs_debugfs_mnt_dev_cleanup(void)
+{
+	debugfs_remove(ceph_mnt_dev_v2_file);
+	ceph_debugfs_cleanup_subdir(ceph_mnt_dev_support_dir);
+}
 
 #else  /* CONFIG_DEBUG_FS */
 
@@ -427,4 +447,12 @@ void ceph_fs_debugfs_cleanup(struct ceph_fs_client *fsc)
 {
 }
 
+void ceph_fs_debugfs_mnt_dev_init(void)
+{
+}
+
+void ceph_fs_debugfs_mnt_dev_cleanup(void)
+{
+}
+
 #endif  /* CONFIG_DEBUG_FS */
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 609ffc8c2d78..21e4a8249afd 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1404,6 +1404,8 @@ static int __init init_ceph(void)
 	if (ret)
 		goto out_caches;
 
+	ceph_fs_debugfs_mnt_dev_init();
+
 	pr_info("loaded (mds proto %d)\n", CEPH_MDSC_PROTOCOL);
 
 	return 0;
@@ -1417,6 +1419,7 @@ static int __init init_ceph(void)
 static void __exit exit_ceph(void)
 {
 	dout("exit_ceph\n");
+	ceph_fs_debugfs_mnt_dev_cleanup();
 	unregister_filesystem(&ceph_fs_type);
 	destroy_caches();
 }
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 8f71184b7c85..3c63c1adcfaa 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -1231,6 +1231,8 @@ extern int ceph_locks_to_pagelist(struct ceph_filelock *flocks,
 /* debugfs.c */
 extern void ceph_fs_debugfs_init(struct ceph_fs_client *client);
 extern void ceph_fs_debugfs_cleanup(struct ceph_fs_client *client);
+extern void ceph_fs_debugfs_mnt_dev_init(void);
+extern void ceph_fs_debugfs_mnt_dev_cleanup(void);
 
 /* quota.c */
 static inline bool __ceph_has_any_quota(struct ceph_inode_info *ci)
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC 1/2] ceph: add helpers to create/cleanup debugfs sub-directories under "ceph" directory
  2021-08-18  6:01 ` [RFC 1/2] ceph: add helpers to create/cleanup debugfs sub-directories under "ceph" directory Venky Shankar
@ 2021-08-18 11:34   ` Venky Shankar
  0 siblings, 0 replies; 11+ messages in thread
From: Venky Shankar @ 2021-08-18 11:34 UTC (permalink / raw)
  To: Jeff Layton, Patrick Donnelly; +Cc: ceph-devel

There's a build issue with non-debugfs builds -- I'll fix it up and resend.

On Wed, Aug 18, 2021 at 11:31 AM Venky Shankar <vshankar@redhat.com> wrote:
>
> Callers can use this helper to create a subdirectory under
> "ceph" directory in debugfs to place custom files for exporting
> information to userspace.
>
> Signed-off-by: Venky Shankar <vshankar@redhat.com>
> ---
>  include/linux/ceph/debugfs.h |  3 +++
>  net/ceph/debugfs.c           | 26 ++++++++++++++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/ceph/debugfs.h b/include/linux/ceph/debugfs.h
> index 8b3a1a7a953a..c372e6cb8aae 100644
> --- a/include/linux/ceph/debugfs.h
> +++ b/include/linux/ceph/debugfs.h
> @@ -10,5 +10,8 @@ extern void ceph_debugfs_cleanup(void);
>  extern void ceph_debugfs_client_init(struct ceph_client *client);
>  extern void ceph_debugfs_client_cleanup(struct ceph_client *client);
>
> +extern struct dentry *ceph_debugfs_create_subdir(const char *subdir);
> +extern void ceph_debugfs_cleanup_subdir(struct dentry *subdir_dentry);
> +
>  #endif
>
> diff --git a/net/ceph/debugfs.c b/net/ceph/debugfs.c
> index 2110439f8a24..eabac20b3ff4 100644
> --- a/net/ceph/debugfs.c
> +++ b/net/ceph/debugfs.c
> @@ -404,6 +404,18 @@ void ceph_debugfs_cleanup(void)
>         debugfs_remove(ceph_debugfs_dir);
>  }
>
> +struct dentry *ceph_debugfs_create_subdir(const char *subdir)
> +{
> +       return debugfs_create_dir(subdir, ceph_debugfs_dir);
> +}
> +EXPORT_SYMBOL(ceph_debugfs_create_subdir);
> +
> +void ceph_debugfs_cleanup_subdir(struct dentry *subdir_dentry)
> +{
> +       debugfs_remove(subdir_dentry);
> +}
> +EXPORT_SYMBOL(ceph_debugfs_cleanup_subdir);
> +
>  void ceph_debugfs_client_init(struct ceph_client *client)
>  {
>         char name[80];
> @@ -413,7 +425,7 @@ void ceph_debugfs_client_init(struct ceph_client *client)
>
>         dout("ceph_debugfs_client_init %p %s\n", client, name);
>
> -       client->debugfs_dir = debugfs_create_dir(name, ceph_debugfs_dir);
> +       client->debugfs_dir = ceph_debugfs_create_subdir(name);
>
>         client->monc.debugfs_file = debugfs_create_file("monc",
>                                                       0400,
> @@ -454,7 +466,7 @@ void ceph_debugfs_client_cleanup(struct ceph_client *client)
>         debugfs_remove(client->debugfs_monmap);
>         debugfs_remove(client->osdc.debugfs_file);
>         debugfs_remove(client->monc.debugfs_file);
> -       debugfs_remove(client->debugfs_dir);
> +       ceph_debugfs_cleanup_subdir(client->debugfs_dir);
>  }
>
>  #else  /* CONFIG_DEBUG_FS */
> @@ -475,4 +487,14 @@ void ceph_debugfs_client_cleanup(struct ceph_client *client)
>  {
>  }
>
> +struct dentry *ceph_debugfs_create_subdir(const char *subdir)
> +{
> +}
> +EXPORT_SYMBOL(ceph_debugfs_create_subdir);
> +
> +void ceph_debugfs_cleanup_subdir(struct dentry *subdir_dentry)
> +{
> +}
> +EXPORT_SYMBOL(ceph_debugfs_cleanup_subdir);
> +
>  #endif  /* CONFIG_DEBUG_FS */
> --
> 2.27.0
>


-- 
Cheers,
Venky


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC 0/2] ceph: add debugfs entries signifying new mount syntax support
  2021-08-18  6:01 [RFC 0/2] ceph: add debugfs entries signifying new mount syntax support Venky Shankar
  2021-08-18  6:01 ` [RFC 1/2] ceph: add helpers to create/cleanup debugfs sub-directories under "ceph" directory Venky Shankar
  2021-08-18  6:01 ` [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support Venky Shankar
@ 2021-08-18 13:09 ` Jeff Layton
  2021-08-18 13:17   ` Venky Shankar
  2 siblings, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2021-08-18 13:09 UTC (permalink / raw)
  To: Venky Shankar, pdonnell; +Cc: ceph-devel

On Wed, 2021-08-18 at 11:31 +0530, Venky Shankar wrote:
> [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.
> 

Is this a known bug you're talking about or are you just speculating
about the potential for bugs there?

> 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/dev_support
>     /sys/kernel/debug/ceph/dev_support/v2
>     ....
>     ....
> 
> Note that there is no entry signifying v1 mount syntax. That's because
> the kernel still supports mounting with old syntax and older kernels do
> not have debug entries for the same.
> 
> Venky Shankar (2):
>   ceph: add helpers to create/cleanup debugfs sub-directories under
>     "ceph" directory
>   ceph: add debugfs entries for v2 (new) mount syntax support
> 
>  fs/ceph/debugfs.c            | 28 ++++++++++++++++++++++++++++
>  fs/ceph/super.c              |  3 +++
>  fs/ceph/super.h              |  2 ++
>  include/linux/ceph/debugfs.h |  3 +++
>  net/ceph/debugfs.c           | 26 ++++++++++++++++++++++++--
>  5 files changed, 60 insertions(+), 2 deletions(-)
> 

I'm not a huge fan of this approach overall as it requires that you have
access to debugfs, and that's not guaranteed to be available everywhere.
If you want to add this for debugging purposes, that's fine, but I don't
think you want the mount helper to rely on this infrastructure.

-- 
Jeff Layton <jlayton@redhat.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC 0/2] ceph: add debugfs entries signifying new mount syntax support
  2021-08-18 13:09 ` [RFC 0/2] ceph: add debugfs entries signifying new " Jeff Layton
@ 2021-08-18 13:17   ` Venky Shankar
  2021-08-18 13:23     ` Jeff Layton
  0 siblings, 1 reply; 11+ messages in thread
From: Venky Shankar @ 2021-08-18 13:17 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Patrick Donnelly, ceph-devel

On Wed, Aug 18, 2021 at 6:39 PM Jeff Layton <jlayton@redhat.com> wrote:
>
> On Wed, 2021-08-18 at 11:31 +0530, Venky Shankar wrote:
> > [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.
> >
>
> Is this a known bug you're talking about or are you just speculating
> about the potential for bugs there?

Potential bugs.

>
> > 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/dev_support
> >     /sys/kernel/debug/ceph/dev_support/v2
> >     ....
> >     ....
> >
> > Note that there is no entry signifying v1 mount syntax. That's because
> > the kernel still supports mounting with old syntax and older kernels do
> > not have debug entries for the same.
> >
> > Venky Shankar (2):
> >   ceph: add helpers to create/cleanup debugfs sub-directories under
> >     "ceph" directory
> >   ceph: add debugfs entries for v2 (new) mount syntax support
> >
> >  fs/ceph/debugfs.c            | 28 ++++++++++++++++++++++++++++
> >  fs/ceph/super.c              |  3 +++
> >  fs/ceph/super.h              |  2 ++
> >  include/linux/ceph/debugfs.h |  3 +++
> >  net/ceph/debugfs.c           | 26 ++++++++++++++++++++++++--
> >  5 files changed, 60 insertions(+), 2 deletions(-)
> >
>
> I'm not a huge fan of this approach overall as it requires that you have
> access to debugfs, and that's not guaranteed to be available everywhere.
> If you want to add this for debugging purposes, that's fine, but I don't
> think you want the mount helper to rely on this infrastructure.

Right. The use-case here is probably to rely on it during teuthology
tests where the mount fails (and the tests) when using the new syntax
but the kernel has v2 syntax support.

I recall the discussion on having some sort of `--no-fallback` option
to not fall-back to old syntax, but since we have the debugfs entries,
we may as well rely on those (at least for testing).

>
> --
> Jeff Layton <jlayton@redhat.com>
>


-- 
Cheers,
Venky


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC 0/2] ceph: add debugfs entries signifying new mount syntax support
  2021-08-18 13:17   ` Venky Shankar
@ 2021-08-18 13:23     ` Jeff Layton
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Layton @ 2021-08-18 13:23 UTC (permalink / raw)
  To: Venky Shankar; +Cc: Patrick Donnelly, ceph-devel

On Wed, 2021-08-18 at 18:47 +0530, Venky Shankar wrote:
> On Wed, Aug 18, 2021 at 6:39 PM Jeff Layton <jlayton@redhat.com> wrote:
> > 
> > On Wed, 2021-08-18 at 11:31 +0530, Venky Shankar wrote:
> > > [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.
> > > 
> > 
> > Is this a known bug you're talking about or are you just speculating
> > about the potential for bugs there?
> 
> Potential bugs.
> 
> > 
> > > 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/dev_support
> > >     /sys/kernel/debug/ceph/dev_support/v2
> > >     ....
> > >     ....
> > > 
> > > Note that there is no entry signifying v1 mount syntax. That's because
> > > the kernel still supports mounting with old syntax and older kernels do
> > > not have debug entries for the same.
> > > 
> > > Venky Shankar (2):
> > >   ceph: add helpers to create/cleanup debugfs sub-directories under
> > >     "ceph" directory
> > >   ceph: add debugfs entries for v2 (new) mount syntax support
> > > 
> > >  fs/ceph/debugfs.c            | 28 ++++++++++++++++++++++++++++
> > >  fs/ceph/super.c              |  3 +++
> > >  fs/ceph/super.h              |  2 ++
> > >  include/linux/ceph/debugfs.h |  3 +++
> > >  net/ceph/debugfs.c           | 26 ++++++++++++++++++++++++--
> > >  5 files changed, 60 insertions(+), 2 deletions(-)
> > > 
> > 
> > I'm not a huge fan of this approach overall as it requires that you have
> > access to debugfs, and that's not guaranteed to be available everywhere.
> > If you want to add this for debugging purposes, that's fine, but I don't
> > think you want the mount helper to rely on this infrastructure.
> 
> Right. The use-case here is probably to rely on it during teuthology
> tests where the mount fails (and the tests) when using the new syntax
> but the kernel has v2 syntax support.
> 
> I recall the discussion on having some sort of `--no-fallback` option
> to not fall-back to old syntax, but since we have the debugfs entries,
> we may as well rely on those (at least for testing).
> 

Ok, I think that sounds reasonable.

-- 
Jeff Layton <jlayton@redhat.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support
  2021-08-18  6:01 ` [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support Venky Shankar
@ 2021-08-21  1:52   ` Patrick Donnelly
  2021-08-23  4:45     ` Venky Shankar
  0 siblings, 1 reply; 11+ messages in thread
From: Patrick Donnelly @ 2021-08-21  1:52 UTC (permalink / raw)
  To: Venky Shankar; +Cc: Jeff Layton, Ceph Development

On Wed, Aug 18, 2021 at 2:01 AM Venky Shankar <vshankar@redhat.com> wrote:
>
> [...]

Is "debugfs" the right place for this? I do wonder if that can be
dropped/disabled via some obscure kernel config?

Also "debugX" doesn't sound like the proper place for a feature flag
of the kernel. I just did a quick check on my system and I do see:

$ ls /sys/fs/ext4/features
batched_discard  casefold  encryption  fast_commit  lazy_itable_init
meta_bg_resize  metadata_csum_seed  test_dummy_encryption_v2  verity

Perhaps we need something similar for fs/ceph?

-- 
Patrick Donnelly, Ph.D.
He / Him / His
Principal Software Engineer
Red Hat Sunnyvale, CA
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support
  2021-08-21  1:52   ` Patrick Donnelly
@ 2021-08-23  4:45     ` Venky Shankar
  2021-08-23  5:31       ` Venky Shankar
  0 siblings, 1 reply; 11+ messages in thread
From: Venky Shankar @ 2021-08-23  4:45 UTC (permalink / raw)
  To: Patrick Donnelly; +Cc: Jeff Layton, Ceph Development

On Sat, Aug 21, 2021 at 7:23 AM Patrick Donnelly <pdonnell@redhat.com> wrote:
>
> On Wed, Aug 18, 2021 at 2:01 AM Venky Shankar <vshankar@redhat.com> wrote:
> >
> > [...]
>
> Is "debugfs" the right place for this? I do wonder if that can be
> dropped/disabled via some obscure kernel config?

The primary use for this (v2 syntax entry) is for catching bugs in v2
mount syntax implementation which sounds more like a form of
"debugging". Sysfs represents the whole device model as seen from the
kernel.

And, sysfs is optional too (CONFIG_SYSFS).

>
> Also "debugX" doesn't sound like the proper place for a feature flag
> of the kernel. I just did a quick check on my system and I do see:
>
> $ ls /sys/fs/ext4/features
> batched_discard  casefold  encryption  fast_commit  lazy_itable_init
> meta_bg_resize  metadata_csum_seed  test_dummy_encryption_v2  verity
>
> Perhaps we need something similar for fs/ceph?
>
> --
> Patrick Donnelly, Ph.D.
> He / Him / His
> Principal Software Engineer
> Red Hat Sunnyvale, CA
> GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
>


-- 
Cheers,
Venky


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support
  2021-08-23  4:45     ` Venky Shankar
@ 2021-08-23  5:31       ` Venky Shankar
  2021-08-23 10:32         ` Jeff Layton
  0 siblings, 1 reply; 11+ messages in thread
From: Venky Shankar @ 2021-08-23  5:31 UTC (permalink / raw)
  To: Patrick Donnelly; +Cc: Jeff Layton, Ceph Development

On Mon, Aug 23, 2021 at 10:15 AM Venky Shankar <vshankar@redhat.com> wrote:
>
> On Sat, Aug 21, 2021 at 7:23 AM Patrick Donnelly <pdonnell@redhat.com> wrote:
> >
> > On Wed, Aug 18, 2021 at 2:01 AM Venky Shankar <vshankar@redhat.com> wrote:
> > >
> > > [...]
> >
> > Is "debugfs" the right place for this? I do wonder if that can be
> > dropped/disabled via some obscure kernel config?
>
> The primary use for this (v2 syntax entry) is for catching bugs in v2
> mount syntax implementation which sounds more like a form of
> "debugging". Sysfs represents the whole device model as seen from the
> kernel.

So, Jeff in another thread suggested that we make these debugfs
entries generic (client_features). In that case, I'm ok with having
these in sysfs.

>
> And, sysfs is optional too (CONFIG_SYSFS).
>
> >
> > Also "debugX" doesn't sound like the proper place for a feature flag
> > of the kernel. I just did a quick check on my system and I do see:
> >
> > $ ls /sys/fs/ext4/features
> > batched_discard  casefold  encryption  fast_commit  lazy_itable_init
> > meta_bg_resize  metadata_csum_seed  test_dummy_encryption_v2  verity
> >
> > Perhaps we need something similar for fs/ceph?
> >
> > --
> > Patrick Donnelly, Ph.D.
> > He / Him / His
> > Principal Software Engineer
> > Red Hat Sunnyvale, CA
> > GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
> >
>
>
> --
> Cheers,
> Venky



-- 
Cheers,
Venky


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support
  2021-08-23  5:31       ` Venky Shankar
@ 2021-08-23 10:32         ` Jeff Layton
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Layton @ 2021-08-23 10:32 UTC (permalink / raw)
  To: Venky Shankar, Patrick Donnelly; +Cc: Ceph Development

On Mon, 2021-08-23 at 11:01 +0530, Venky Shankar wrote:
> On Mon, Aug 23, 2021 at 10:15 AM Venky Shankar <vshankar@redhat.com> wrote:
> > 
> > On Sat, Aug 21, 2021 at 7:23 AM Patrick Donnelly <pdonnell@redhat.com> wrote:
> > > 
> > > On Wed, Aug 18, 2021 at 2:01 AM Venky Shankar <vshankar@redhat.com> wrote:
> > > > 
> > > > [...]
> > > 
> > > Is "debugfs" the right place for this? I do wonder if that can be
> > > dropped/disabled via some obscure kernel config?
> > 
> > The primary use for this (v2 syntax entry) is for catching bugs in v2
> > mount syntax implementation which sounds more like a form of
> > "debugging". Sysfs represents the whole device model as seen from the
> > kernel.
> 
> So, Jeff in another thread suggested that we make these debugfs
> entries generic (client_features). In that case, I'm ok with having
> these in sysfs.
> 

I think debugfs is fine for this. This is mainly for teuthology, and in
that case we'll have debugfs available.

One of the reasons to use debugfs here is that stuff in there is
specifically _not_ considered part of the kernel's ABI, so we can more
easily make changes to it in the future w/o worrying as much about
backward compatibility.

> > 
> > And, sysfs is optional too (CONFIG_SYSFS).
> > 
> > > 
> > > Also "debugX" doesn't sound like the proper place for a feature flag
> > > of the kernel. I just did a quick check on my system and I do see:
> > > 
> > > $ ls /sys/fs/ext4/features
> > > batched_discard  casefold  encryption  fast_commit  lazy_itable_init
> > > meta_bg_resize  metadata_csum_seed  test_dummy_encryption_v2  verity
> > > 
> > > Perhaps we need something similar for fs/ceph?
> > > 
> > > --
> > > Patrick Donnelly, Ph.D.
> > > He / Him / His
> > > Principal Software Engineer
> > > Red Hat Sunnyvale, CA
> > > GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
> > > 
> > 
> > 
> > --
> > Cheers,
> > Venky
> 
> 
> 

-- 
Jeff Layton <jlayton@redhat.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-23 10:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18  6:01 [RFC 0/2] ceph: add debugfs entries signifying new mount syntax support Venky Shankar
2021-08-18  6:01 ` [RFC 1/2] ceph: add helpers to create/cleanup debugfs sub-directories under "ceph" directory Venky Shankar
2021-08-18 11:34   ` Venky Shankar
2021-08-18  6:01 ` [RFC 2/2] ceph: add debugfs entries for v2 (new) mount syntax support Venky Shankar
2021-08-21  1:52   ` Patrick Donnelly
2021-08-23  4:45     ` Venky Shankar
2021-08-23  5:31       ` Venky Shankar
2021-08-23 10:32         ` Jeff Layton
2021-08-18 13:09 ` [RFC 0/2] ceph: add debugfs entries signifying new " Jeff Layton
2021-08-18 13:17   ` Venky Shankar
2021-08-18 13:23     ` Jeff Layton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).