Re: [PATCH 00/12] fscache: Some prep work for fscache rewrite

From: Jeff Layton <jlayton@redhat.com>
To: David Howells <dhowells@redhat.com>, linux-cachefs@redhat.com
Cc: Anna Schumaker <anna.schumaker@netapp.com>,
	Steve French <sfrench@samba.org>,
	Dominique Martinet <asmadeus@codewreck.org>,
	David Wysochanski <dwysocha@redhat.com>,
	linux-afs@lists.infradead.org, linux-nfs@vger.kernel.org,
	linux-cifs@vger.kernel.org, ceph-devel@vger.kernel.org,
	v9fs-developer@lists.sourceforge.net,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 00/12] fscache: Some prep work for fscache rewrite
Date: Tue, 24 Aug 2021 10:25:20 -0400	[thread overview]
Message-ID: <01c442d2b3aff85e0e95bfefb44ac6e77eb3373f.camel@redhat.com> (raw)
In-Reply-To: <162431188431.2908479.14031376932042135080.stgit@warthog.procyon.org.uk>

On Mon, 2021-06-21 at 22:44 +0100, David Howells wrote:
> Here are some patches that perform some preparatory work for the fscache
> rewrite that's being worked on.  These include:
> 
>  (1) Always select netfs stats when enabling fscache stats since they're
>      displayed through the same procfile.
> 
>  (2) Add a cookie debug ID that can be used in tracepoints instead of a
>      pointer and cache it in the netfs_cache_resources struct rather than
>      in the netfs_read_request struct to make it more available.
> 
>  (3) Use file_inode() in cachefiles rather than dereferencing file->f_inode
>      directly.
> 
>  (4) Provide a procfile to display fscache cookies.
> 
>  (5) Remove the fscache and cachefiles histogram procfiles.
> 
>  (6) Remove the fscache object list procfile.
> 
>  (7) Avoid using %p in fscache and cachefiles as the value is hashed and
>      not comparable to the register dump in an oops trace.
> 
>  (8) Fix the cookie hash function to actually achieve useful dispersion.
> 
>  (9) Fix fscache_cookie_put() so that it doesn't dereference the cookie
>      pointer in the tracepoint after the refcount has been decremented
>      (we're only allowed to do that if we decremented it to zero).
> 
> (10) Use refcount_t rather than atomic_t for the fscache_cookie refcount.
> 
> The patches can be found on this branch:
> 
> 	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-next
> 
> David
> ---
> David Howells (12):
>       fscache: Select netfs stats if fscache stats are enabled
>       netfs: Move cookie debug ID to struct netfs_cache_resources
>       cachefiles: Use file_inode() rather than accessing ->f_inode
>       fscache: Add a cookie debug ID and use that in traces
>       fscache: Procfile to display cookies
>       fscache, cachefiles: Remove the histogram stuff
>       fscache: Remove the object list procfile
>       fscache: Change %p in format strings to something else
>       cachefiles: Change %p in format strings to something else
>       fscache: Fix cookie key hashing
>       fscache: Fix fscache_cookie_put() to not deref after dec
>       fscache: Use refcount_t for the cookie refcount instead of atomic_t
> 
> 
>  fs/cachefiles/Kconfig             |  19 --
>  fs/cachefiles/Makefile            |   2 -
>  fs/cachefiles/bind.c              |   2 -
>  fs/cachefiles/interface.c         |   6 +-
>  fs/cachefiles/internal.h          |  25 --
>  fs/cachefiles/io.c                |   6 +-
>  fs/cachefiles/key.c               |   2 +-
>  fs/cachefiles/main.c              |   7 -
>  fs/cachefiles/namei.c             |  61 ++---
>  fs/cachefiles/proc.c              | 114 --------
>  fs/cachefiles/xattr.c             |   4 +-
>  fs/fscache/Kconfig                |  24 --
>  fs/fscache/Makefile               |   2 -
>  fs/fscache/cache.c                |  11 +-
>  fs/fscache/cookie.c               | 201 +++++++++++----
>  fs/fscache/fsdef.c                |   3 +-
>  fs/fscache/histogram.c            |  87 -------
>  fs/fscache/internal.h             |  57 +---
>  fs/fscache/main.c                 |  39 +++
>  fs/fscache/netfs.c                |   2 +-
>  fs/fscache/object-list.c          | 414 ------------------------------
>  fs/fscache/object.c               |   8 -
>  fs/fscache/operation.c            |   3 -
>  fs/fscache/page.c                 |   6 -
>  fs/fscache/proc.c                 |  20 +-
>  include/linux/fscache-cache.h     |   4 -
>  include/linux/fscache.h           |   4 +-
>  include/linux/netfs.h             |   2 +-
>  include/trace/events/cachefiles.h |  68 ++---
>  include/trace/events/fscache.h    | 160 ++++++------
>  include/trace/events/netfs.h      |   2 +-
>  31 files changed, 367 insertions(+), 998 deletions(-)
>  delete mode 100644 fs/cachefiles/proc.c
>  delete mode 100644 fs/fscache/histogram.c
>  delete mode 100644 fs/fscache/object-list.c
> 
> 

This all looks good (modulo a nitpicky changelog comment). You can add:

Reviewed-by: Jeff Layton <jlayton@redhat.com>