2021-10-04 17:54:19

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH v1 7/7] NFS: Remove remaining usages of NFSDBG_FSCACHE

On Sun, 2021-10-03 at 15:22 -0400, Dave Wysochanski wrote:
> The NFS fscache interface has removed all dfprintks so remove the
> NFSDBG_FSCACHE defines.
>
> Signed-off-by: Dave Wysochanski <[email protected]>
> ---
>  fs/nfs/fscache-index.c      | 2 --
>  fs/nfs/fscache.c            | 2 --
>  include/uapi/linux/nfs_fs.h | 2 +-
>  3 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/fs/nfs/fscache-index.c b/fs/nfs/fscache-index.c
> index 4bd5ce736193..71bb4270641f 100644
> --- a/fs/nfs/fscache-index.c
> +++ b/fs/nfs/fscache-index.c
> @@ -17,8 +17,6 @@
>  #include "internal.h"
>  #include "fscache.h"
>  
> -#define NFSDBG_FACILITY                NFSDBG_FSCACHE
> -
>  /*
>   * Define the NFS filesystem for FS-Cache.  Upon registration FS-
> Cache sticks
>   * the cookie for the top-level index object for NFS into here.  The
> top-level
> diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
> index d199ee103dc6..016e6cb13d28 100644
> --- a/fs/nfs/fscache.c
> +++ b/fs/nfs/fscache.c
> @@ -21,8 +21,6 @@
>  #include "fscache.h"
>  #include "nfstrace.h"
>  
> -#define NFSDBG_FACILITY                NFSDBG_FSCACHE
> -
>  static struct rb_root nfs_fscache_keys = RB_ROOT;
>  static DEFINE_SPINLOCK(nfs_fscache_keys_lock);
>  
> diff --git a/include/uapi/linux/nfs_fs.h
> b/include/uapi/linux/nfs_fs.h
> index 3afe3767c55d..caa8d2234958 100644
> --- a/include/uapi/linux/nfs_fs.h
> +++ b/include/uapi/linux/nfs_fs.h
> @@ -52,7 +52,7 @@
>  #define NFSDBG_CALLBACK                0x0100
>  #define NFSDBG_CLIENT          0x0200
>  #define NFSDBG_MOUNT           0x0400
> -#define NFSDBG_FSCACHE         0x0800
> +#define NFSDBG_UNUSED          0x0800 /* unused; was FSCACHE */

Please leave the name and value unchanged. I'm fine with adding the
comment telling people not to bother using it, but this is supposed to
be part of a user API so it can't be modified unless we're absolutely
certain it isn't being used by anyone.

The other changes are fine.

>  #define NFSDBG_PNFS            0x1000
>  #define NFSDBG_PNFS_LD         0x2000
>  #define NFSDBG_STATE           0x4000

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]



2021-10-05 13:53:52

by David Wysochanski

[permalink] [raw]
Subject: Re: [PATCH v1 7/7] NFS: Remove remaining usages of NFSDBG_FSCACHE

On Mon, Oct 4, 2021 at 11:57 AM Trond Myklebust <[email protected]> wrote:
>
> On Sun, 2021-10-03 at 15:22 -0400, Dave Wysochanski wrote:
> > The NFS fscache interface has removed all dfprintks so remove the
> > NFSDBG_FSCACHE defines.
> >
> > Signed-off-by: Dave Wysochanski <[email protected]>
> > ---
> > fs/nfs/fscache-index.c | 2 --
> > fs/nfs/fscache.c | 2 --
> > include/uapi/linux/nfs_fs.h | 2 +-
> > 3 files changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/fs/nfs/fscache-index.c b/fs/nfs/fscache-index.c
> > index 4bd5ce736193..71bb4270641f 100644
> > --- a/fs/nfs/fscache-index.c
> > +++ b/fs/nfs/fscache-index.c
> > @@ -17,8 +17,6 @@
> > #include "internal.h"
> > #include "fscache.h"
> >
> > -#define NFSDBG_FACILITY NFSDBG_FSCACHE
> > -
> > /*
> > * Define the NFS filesystem for FS-Cache. Upon registration FS-
> > Cache sticks
> > * the cookie for the top-level index object for NFS into here. The
> > top-level
> > diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
> > index d199ee103dc6..016e6cb13d28 100644
> > --- a/fs/nfs/fscache.c
> > +++ b/fs/nfs/fscache.c
> > @@ -21,8 +21,6 @@
> > #include "fscache.h"
> > #include "nfstrace.h"
> >
> > -#define NFSDBG_FACILITY NFSDBG_FSCACHE
> > -
> > static struct rb_root nfs_fscache_keys = RB_ROOT;
> > static DEFINE_SPINLOCK(nfs_fscache_keys_lock);
> >
> > diff --git a/include/uapi/linux/nfs_fs.h
> > b/include/uapi/linux/nfs_fs.h
> > index 3afe3767c55d..caa8d2234958 100644
> > --- a/include/uapi/linux/nfs_fs.h
> > +++ b/include/uapi/linux/nfs_fs.h
> > @@ -52,7 +52,7 @@
> > #define NFSDBG_CALLBACK 0x0100
> > #define NFSDBG_CLIENT 0x0200
> > #define NFSDBG_MOUNT 0x0400
> > -#define NFSDBG_FSCACHE 0x0800
> > +#define NFSDBG_UNUSED 0x0800 /* unused; was FSCACHE */
>
> Please leave the name and value unchanged. I'm fine with adding the
> comment telling people not to bother using it, but this is supposed to
> be part of a user API so it can't be modified unless we're absolutely
> certain it isn't being used by anyone.
>
Ok I will post a v2 and leave NFSDBG_FSCACHE defined for now but add
the comment. But once there's no more usages in the kernel, I'm not sure
what the proper way to deprecate and remove it would be though.

I can post a nfs-utils patch to deprecate (or remove) the usage of fscache
in rpcdebug too. What's the proper way to deprecate and remove rpcdebug
flags, or is there some reason we don't ever want to do it?


> The other changes are fine.
>
Thanks for the review.