Re: [PATCH v2] autofs: add per dentry expire timeout

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 15/8/24 11:10, Ian Kent wrote:
On 14/8/24 22:39, Christian Brauner wrote:
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@xxxxxxxxxx>
---
  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?

Not sure I understand what your saying here.

Having re-read what you said I think I understand what your saying now.


I was mislead by the lower bound wording, the warning talks about a per-dentry timeout setting

that's longer than the timeout set for this indirect autofs mount. TBH I'm not even sure this

warning is worthwhile since most user space builds should just leave in-use mounts mounted at exit

and automount should just re-instate them at startup and continue on as though nothing had happened.

But I thought it worth a warning so that users know the timeouts they are using are longer than the

parent (global for these indirect mounts) timeout.


I can certainly get rid of it if you prefer.


Ian



The (amd) auto-mounted mounts are each meant to be able to expire independently,

according to the timeout set for each of them rather than use the global timeout set

in the autofs (parent) mount.


But your comment is useful because I do use a polling mechanism in user space to check

for expiration and the frequency of checking becomes a problem when I introduce this.

Setting a lower bound may make the frequency to short (for very large directories) and

I think there will be difficulties working out when the frequency needs to be reduced

after changes. But these are problems that I think can be left to user space, not sure

yet. For now it will be adequate to go with the default frequency based on the parent

mount as it is now (ie. a check frequency of one quarter of the expire global timeout).


Ian






[Index of Archives]     [Linux Filesystem Development]     [Linux Ext4]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux