Re: rpc.idmapd runs out of file descriptors

[Salvatore Bonaccorso <carnil@debian.org>]

Hi all,

On Wed, Jun 05, 2024 at 05:19:27PM +0200, Sergio Gelato wrote:
> Observed on Debian 12 (nfs-utils 2.6.2):
> 
> May 28 09:40:25 HOSTNAME rpc.idmapd[3602614]: dirscancb: scandir(/run/rpc_pipefs/nfs): Too many open files
> [repeated multiple times]
> 
> Investigation with lsof on one of the affected systems shows that file desciptors are not being closed:
> 
> [...]
> rpc.idmap 675 root  126r      DIR               0,40        0      10813 /run/rpc_pipefs/nfs/clnt11e6 (deleted)
> rpc.idmap 675 root  127u     FIFO               0,40      0t0      10817 /run/rpc_pipefs/nfs/clnt11e6/idmap (deleted)
> rpc.idmap 675 root  128r      DIR               0,40        0      10834 /run/rpc_pipefs/nfs/clnt11ef (deleted)
> rpc.idmap 675 root  129u     FIFO               0,40      0t0      10838 /run/rpc_pipefs/nfs/clnt11ef/idmap (deleted)
> rpc.idmap 675 root  130r      DIR               0,40        0      10855 /run/rpc_pipefs/nfs/clnt11f8 (deleted)
> rpc.idmap 675 root  131u     FIFO               0,40      0t0      10859 /run/rpc_pipefs/nfs/clnt11f8/idmap (deleted)
> 
> Raising the verbosity level to 3 results in no "Stale client:" lines.
> strace shows no close() calls other than for the /run/rpc_pipefs/nfs directory.
> 
> I believe this is because in dirscancb() the loop is exited prematurely
> the first time nfsopen() returns -1, preventing later entries in the queue
> from being reaped. I've tested the patch below, which seems indeed to cure
> the problem. The bug appears to be still unfixed in the current master branch.

> From: Sergio Gelato <Sergio.Gelato@astro.su.se>
> Date: Tue, 4 Jun 2024 16:02:59 +0200
> Subject: rpc.idmapd: nfsopen() failures should not be fatal
> 
> dirscancb() loops over all clnt* subdirectories of /run/rpc_pipefs/nfs/.
> Some of these directories contain /idmap files, others don't. nfsopen()
> returns -1 for the latter; we then want to skip the directory, not abort
> the entire scan.
> ---
>  utils/idmapd/idmapd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index e79c124..f3c540d 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -556,7 +556,7 @@ dirscancb(int fd, short UNUSED(which), void *data)
>  			if (nfsopen(ic) == -1) {
>  				close(ic->ic_dirfd);
>  				free(ic);
> -				goto out;
> +				continue;
>  			}
>  
>  			if (verbose > 2)

Did this felt trough the cracks? Does the patch from Sergio looks good
to you?

Regards,
Salvatore