[GIT PULL] afs, rxrpc: Clean up refcounting on afs_cell and afs_server records

[GIT PULL] afs, rxrpc: Clean up refcounting on afs_cell and afs_server records

[David Howells]

Hi Christian,

Could you pull this into the VFS tree onto a stable branch?  The patches
were previously posted here as part of a longer series:

  https://lore.kernel.org/r/20250224234154.2014840-1-dhowells@redhat.com/

The first five patches are fixes that have gone through the net tree to
Linus and hence this patchset is based on the latest merge by Linus and
I've dropped those from my branch.

This fixes an occasional hang that's only really encountered when rmmod'ing
the kafs module, one of the reasons why I'm proposing it for the next merge
window rather than immediate upstreaming.  The changes include:

 (1) Remove the "-o autocell" mount option.  This is obsolete with the
     dynamic root and removing it makes the next patch slightly easier.

 (2) Change how the dynamic root mount is constructed.  Currently, the root
     directory is (de)populated when it is (un)mounted if there are cells
     already configured and, further, pairs of automount points have to be
     created/removed each time a cell is added/deleted.

     This is changed so that readdir on the root dir lists all the known
     cell automount pairs plus the @cell symlinks and the inodes and
     dentries are constructed by lookup on demand.  This simplifies the
     cell management code.

 (3) A few improvements to the afs_volume tracepoint.

 (4) A few improvements to the afs_server tracepoint.

 (5) Pass trace info into the afs_lookup_cell() function to allow the trace
     log to indicate the purpose of the lookup.

 (6) Remove the 'net' parameter from afs_unuse_cell() as it's superfluous.

 (7) In rxrpc, allow a kernel app (such as kafs) to store a word of
     information on rxrpc_peer records.

 (8) Use the information stored on the rxrpc_peer record to point to the
     afs_server record.  This allows the server address lookup to be done
     away with.

 (9) Simplify the afs_server ref/activity accounting to make each one
     self-contained and not garbage collected from the cell management work
     item.

(10) Simplify the afs_cell ref/activity accounting to make each one of
     these also self-contained and not driven by a central management work
     item.

     The current code was intended to make it such that a single timer for
     the namespace and one work item per cell could do all the work
     required to maintain these records.  This, however, made for some
     sequencing problems when cleaning up these records.  Further, the
     attempt to pass refs along with timers and work items made getting it
     right rather tricky when the timer or work item already had a ref
     attached and now a ref had to be got rid of.

Thanks,
David
---
The following changes since commit 1e15510b71c99c6e49134d756df91069f7d18141:

  Merge tag 'net-6.14-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2025-02-27 09:32:42 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/afs-next-20250228

for you to fetch changes up to 96269ca2db2dffe40253877f3f615e5ce6d3cbcd:

  afs: Simplify cell record handling (2025-02-28 08:41:25 +0000)

----------------------------------------------------------------
afs: Fix ref leak in rmmod

----------------------------------------------------------------
David Howells (10):
      afs: Remove the "autocell" mount option
      afs: Change dynroot to create contents on demand
      afs: Improve afs_volume tracing to display a debug ID
      afs: Improve server refcount/active count tracing
      afs: Make afs_lookup_cell() take a trace note
      afs: Drop the net parameter from afs_unuse_cell()
      rxrpc: Allow the app to store private data on peer structs
      afs: Use the per-peer app data provided by rxrpc
      afs: Fix afs_server ref accounting
      afs: Simplify cell record handling

 fs/afs/addr_list.c         |  50 ++++
 fs/afs/cell.c              | 436 ++++++++++++++------------------
 fs/afs/cmservice.c         |  82 +------
 fs/afs/dir.c               |   5 +-
 fs/afs/dynroot.c           | 484 +++++++++++++++---------------------
 fs/afs/fs_probe.c          |  32 ++-
 fs/afs/fsclient.c          |   4 +-
 fs/afs/internal.h          |  98 ++++----
 fs/afs/main.c              |  16 +-
 fs/afs/mntpt.c             |   5 +-
 fs/afs/proc.c              |  15 +-
 fs/afs/rxrpc.c             |   8 +-
 fs/afs/server.c            | 601 +++++++++++++++++++--------------------------
 fs/afs/server_list.c       |   6 +-
 fs/afs/super.c             |  25 +-
 fs/afs/vl_alias.c          |   7 +-
 fs/afs/vl_rotate.c         |   2 +-
 fs/afs/volume.c            |  15 +-
 include/net/af_rxrpc.h     |   2 +
 include/trace/events/afs.h |  83 ++++---
 net/rxrpc/ar-internal.h    |   1 +
 net/rxrpc/peer_object.c    |  30 ++-
 22 files changed, 903 insertions(+), 1104 deletions(-)

Re: [GIT PULL] afs, rxrpc: Clean up refcounting on afs_cell and afs_server records

[Christian Brauner]

On Fri, 28 Feb 2025 08:58:39 +0000, David Howells wrote:
> Could you pull this into the VFS tree onto a stable branch?  The patches
> were previously posted here as part of a longer series:
> 
>   https://lore.kernel.org/r/20250224234154.2014840-1-dhowells@redhat.com/
> 
> The first five patches are fixes that have gone through the net tree to
> Linus and hence this patchset is based on the latest merge by Linus and
> I've dropped those from my branch.
> 
> [...]

Pulled into the vfs-6.15.shared.afs branch of the vfs/vfs.git tree.
Patches in the vfs-6.15.shared.afs branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series or pull request allowing us to
drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.15.shared.afs

https://git.kernel.org/vfs/vfs/c/d0710fad7ac3

Re: [GIT PULL] afs, rxrpc: Clean up refcounting on afs_cell and afs_server records

[Jakub Kicinski]

On Fri, 28 Feb 2025 08:58:39 +0000 David Howells wrote:
> Could you pull this into the VFS tree onto a stable branch?  The patches
> were previously posted here as part of a longer series:
> 
>   https://lore.kernel.org/r/20250224234154.2014840-1-dhowells@redhat.com/
> 
> The first five patches are fixes that have gone through the net tree to
> Linus and hence this patchset is based on the latest merge by Linus and
> I've dropped those from my branch.

FWIW:

fs/afs/cell.c:203:5-22: WARNING: Unsigned expression compared with zero: cell -> dynroot_ino < 0
-- 
pw-bot: nap

Re: [GIT PULL] afs, rxrpc: Clean up refcounting on afs_cell and afs_server records

[David Howells]

Jakub Kicinski <kuba@kernel.org> wrote:

> fs/afs/cell.c:203:5-22: WARNING: Unsigned expression compared with zero: cell -> dynroot_ino < 0

Yeah, thanks - error handling bug.  I can retag it and ask Christian to pull
it again.  (unless he'd rather stack a fix patch)

David

Re: [GIT PULL] afs, rxrpc: Clean up refcounting on afs_cell and afs_server records

[David Howells]

Jakub Kicinski <kuba@kernel.org> wrote:

> fs/afs/cell.c:203:5-22: WARNING: Unsigned expression compared with zero: cell -> dynroot_ino < 0

I'll make this change:

--- a/fs/afs/cell.c
+++ b/fs/afs/cell.c
@@ -200,7 +200,7 @@ static struct afs_cell *afs_alloc_cell(struct afs_net *net,
        atomic_inc(&net->cells_outstanding);
        cell->dynroot_ino = idr_alloc_cyclic(&net->cells_dyn_ino, cell,
                                             2, INT_MAX / 2, GFP_KERNEL);
-       if (cell->dynroot_ino < 0)
+       if ((int)cell->dynroot_ino < 0)
                goto error;
        cell->debug_id = atomic_inc_return(&cell_debug_id);
 

to patch 2 ("afs: Change dynroot to create contents on demand").

I'm not sure why gcc didn't warn about this - I'm sure it used to.

David

Re: [GIT PULL] afs, rxrpc: Clean up refcounting on afs_cell and afs_server records

[David Howells]

David Howells <dhowells@redhat.com> wrote:

>         cell->dynroot_ino = idr_alloc_cyclic(&net->cells_dyn_ino, cell,
>                                              2, INT_MAX / 2, GFP_KERNEL);
> -       if (cell->dynroot_ino < 0)
> +       if ((int)cell->dynroot_ino < 0)
>                 goto error;

That's not right.  I need to copy the error into 'ret' before jumping to
error.  Probably better to do:

	ret = idr_alloc_cyclic(&net->cells_dyn_ino, cell,
                               2, INT_MAX / 2, GFP_KERNEL);
	if (ret < 0)
		goto error;
	cell->dynroot_ino = ret;

David