Message-ID: <20240814-darauf-schund-23ec844f4a09@brauner>
Date: Wed, 14 Aug 2024 16:39:31 +0200
From: Christian Brauner <brauner@...nel.org>
To: Ian Kent <raven@...maw.net>
Cc: Al Viro <viro@...iv.linux.org.uk>, 
	autofs mailing list <autofs@...r.kernel.org>, linux-fsdevel <linux-fsdevel@...r.kernel.org>, 
	Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] autofs: add per dentry expire timeout

On Wed, Aug 14, 2024 at 05:02:31PM GMT, Ian Kent wrote:
> Add ability to set per-dentry mount expire timeout to autofs.
> 
> There are two fairly well known automounter map formats, the autofs
> format and the amd format (more or less System V and Berkley).
> 
> Some time ago Linux autofs added an amd map format parser that
> implemented a fair amount of the amd functionality. This was done
> within the autofs infrastructure and some functionality wasn't
> implemented because it either didn't make sense or required extra
> kernel changes. The idea was to restrict changes to be within the
> existing autofs functionality as much as possible and leave changes
> with a wider scope to be considered later.
> 
> One of these changes is implementing the amd options:
> 1) "unmount", expire this mount according to a timeout (same as the
>    current autofs default).
> 2) "nounmount", don't expire this mount (same as setting the autofs
>    timeout to 0 except only for this specific mount) .
> 3) "utimeout=<seconds>", expire this mount using the specified
>    timeout (again same as setting the autofs timeout but only for
>    this mount).
> 
> To implement these options per-dentry expire timeouts need to be
> implemented for autofs indirect mounts. This is because all map keys
> (mounts) for autofs indirect mounts use an expire timeout stored in
> the autofs mount super block info. structure and all indirect mounts
> use the same expire timeout.
> 
> Now I have a request to add the "nounmount" option so I need to add
> the per-dentry expire handling to the kernel implementation to do this.
> 
> The implementation uses the trailing path component to identify the
> mount (and is also used as the autofs map key) which is passed in the
> autofs_dev_ioctl structure path field. The expire timeout is passed
> in autofs_dev_ioctl timeout field (well, of the timeout union).
> 
> If the passed in timeout is equal to -1 the per-dentry timeout and
> flag are cleared providing for the "unmount" option. If the timeout
> is greater than or equal to 0 the timeout is set to the value and the
> flag is also set. If the dentry timeout is 0 the dentry will not expire
> by timeout which enables the implementation of the "nounmount" option
> for the specific mount. When the dentry timeout is greater than zero it
> allows for the implementation of the "utimeout=<seconds>" option.
> 
> Signed-off-by: Ian Kent <raven@...maw.net>
> ---
>  fs/autofs/autofs_i.h         |  4 ++
>  fs/autofs/dev-ioctl.c        | 97 ++++++++++++++++++++++++++++++++++--
>  fs/autofs/expire.c           |  7 ++-
>  fs/autofs/inode.c            |  2 +
>  include/uapi/linux/auto_fs.h |  2 +-
>  5 files changed, 104 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
> index 8c1d587b3eef..77c7991d89aa 100644
> --- a/fs/autofs/autofs_i.h
> +++ b/fs/autofs/autofs_i.h
> @@ -62,6 +62,7 @@ struct autofs_info {
>  	struct list_head expiring;
>  
>  	struct autofs_sb_info *sbi;
> +	unsigned long exp_timeout;
>  	unsigned long last_used;
>  	int count;
>  
> @@ -81,6 +82,9 @@ struct autofs_info {
>  					*/
>  #define AUTOFS_INF_PENDING	(1<<2) /* dentry pending mount */
>  
> +#define AUTOFS_INF_EXPIRE_SET	(1<<3) /* per-dentry expire timeout set for
> +					  this mount point.
> +					*/
>  struct autofs_wait_queue {
>  	wait_queue_head_t queue;
>  	struct autofs_wait_queue *next;
> diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c
> index 5bf781ea6d67..f011e026358e 100644
> --- a/fs/autofs/dev-ioctl.c
> +++ b/fs/autofs/dev-ioctl.c
> @@ -128,7 +128,13 @@ static int validate_dev_ioctl(int cmd, struct autofs_dev_ioctl *param)
>  			goto out;
>  		}
>  
> +		/* Setting the per-dentry expire timeout requires a trailing
> +		 * path component, ie. no '/', so invert the logic of the
> +		 * check_name() return for AUTOFS_DEV_IOCTL_TIMEOUT_CMD.
> +		 */
>  		err = check_name(param->path);
> +		if (cmd == AUTOFS_DEV_IOCTL_TIMEOUT_CMD)
> +			err = err ? 0 : -EINVAL;
>  		if (err) {
>  			pr_warn("invalid path supplied for cmd(0x%08x)\n",
>  				cmd);
> @@ -396,16 +402,97 @@ static int autofs_dev_ioctl_catatonic(struct file *fp,
>  	return 0;
>  }
>  
> -/* Set the autofs mount timeout */
> +/*
> + * Set the autofs mount expire timeout.
> + *
> + * There are two places an expire timeout can be set, in the autofs
> + * super block info. (this is all that's needed for direct and offset
> + * mounts because there's a distinct mount corresponding to each of
> + * these) and per-dentry within within the dentry info. If a per-dentry
> + * timeout is set it will override the expire timeout set in the parent
> + * autofs super block info.
> + *
> + * If setting the autofs super block expire timeout the autofs_dev_ioctl
> + * size field will be equal to the autofs_dev_ioctl structure size. If
> + * setting the per-dentry expire timeout the mount point name is passed
> + * in the autofs_dev_ioctl path field and the size field updated to
> + * reflect this.
> + *
> + * Setting the autofs mount expire timeout sets the timeout in the super
> + * block info. struct. Setting the per-dentry timeout does a little more.
> + * If the timeout is equal to -1 the per-dentry timeout (and flag) is
> + * cleared which reverts to using the super block timeout, otherwise if
> + * timeout is 0 the timeout is set to this value and the flag is left
> + * set which disables expiration for the mount point, lastly the flag
> + * and the timeout are set enabling the dentry to use this timeout.
> + */
>  static int autofs_dev_ioctl_timeout(struct file *fp,
>  				    struct autofs_sb_info *sbi,
>  				    struct autofs_dev_ioctl *param)
>  {
> -	unsigned long timeout;
> +	unsigned long timeout = param->timeout.timeout;
> +
> +	/* If setting the expire timeout for an individual indirect
> +	 * mount point dentry the mount trailing component path is
> +	 * placed in param->path and param->size adjusted to account
> +	 * for it otherwise param->size it is set to the structure
> +	 * size.
> +	 */
> +	if (param->size == AUTOFS_DEV_IOCTL_SIZE) {
> +		param->timeout.timeout = sbi->exp_timeout / HZ;
> +		sbi->exp_timeout = timeout * HZ;
> +	} else {
> +		struct dentry *base = fp->f_path.dentry;
> +		struct inode *inode = base->d_inode;
> +		int path_len = param->size - AUTOFS_DEV_IOCTL_SIZE - 1;
> +		struct dentry *dentry;
> +		struct autofs_info *ino;
> +
> +		if (!autofs_type_indirect(sbi->type))
> +			return -EINVAL;
> +
> +		/* An expire timeout greater than the superblock timeout
> +		 * could be a problem at shutdown but the super block
> +		 * timeout itself can change so all we can really do is
> +		 * warn the user.
> +		 */
> +		if (timeout >= sbi->exp_timeout)
> +			pr_warn("per-mount expire timeout is greater than "
> +				"the parent autofs mount timeout which could "
> +				"prevent shutdown\n");

Wouldn't it be possible to just record the lowest known per-dentry
timeout in idk sbi->exp_lower_bound and reject sbi->exp_timeout changes
that go below that?

Powered by blists - more mailing lists