Re: [PATCH 6/6] autofs - add strictexpire mount option
Hi Ian,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc2]
[cannot apply to next-20181115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Ian-Kent/autofs-improve-ioctl-sbi-checks/20181114-021150
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
>> fs/autofs/init.c:19:25: warning: symbol 'autofs_fs_type' was not declared.
>> Should it be static?
--
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
>> fs/autofs/inode.c:372:35: error: cannot dereference this type
fs/autofs/inode.c:373:35: error: cannot dereference this type
>> fs/autofs/inode.c:372:35: warning: unknown expression (8 46)
fs/autofs/inode.c:373:35: warning: unknown expression (8 46)
fs/autofs/inode.c: In function 'autofs_fill_super':
fs/autofs/inode.c:372:28: error: invalid type argument of '->' (have 'struct
autofs_fs_params')
root_inode->i_uid = params->uid;
^~
fs/autofs/inode.c:373:28: error: invalid type argument of '->' (have 'struct
autofs_fs_params')
root_inode->i_gid = params->gid;
^~
vim +372 fs/autofs/inode.c
e35160168 Ian Kent 2018-11-13 324
e35160168 Ian Kent 2018-11-13 325 int autofs_fill_super(struct super_block
*s, void *data, int silent)
e35160168 Ian Kent 2018-11-13 326 {
e35160168 Ian Kent 2018-11-13 327 struct inode *root_inode;
e35160168 Ian Kent 2018-11-13 328 struct dentry *root;
f4507adde Ian Kent 2018-11-13 329 struct autofs_fs_params params;
e35160168 Ian Kent 2018-11-13 330 struct autofs_sb_info *sbi;
e35160168 Ian Kent 2018-11-13 331 struct autofs_info *ino;
e35160168 Ian Kent 2018-11-13 332 int ret = -EINVAL;
e35160168 Ian Kent 2018-11-13 333
e35160168 Ian Kent 2018-11-13 334 sbi = autofs_alloc_sbi(s);
e35160168 Ian Kent 2018-11-13 335 if (!sbi)
e35160168 Ian Kent 2018-11-13 336 return -ENOMEM;
e35160168 Ian Kent 2018-11-13 337
e35160168 Ian Kent 2018-11-13 338 pr_debug("starting up, sbi = %p\n",
sbi);
e35160168 Ian Kent 2018-11-13 339
e35160168 Ian Kent 2018-11-13 340 s->s_fs_info = sbi;
ebc921ca9 Ian Kent 2018-06-07 341 s->s_blocksize = 1024;
ebc921ca9 Ian Kent 2018-06-07 342 s->s_blocksize_bits = 10;
ebc921ca9 Ian Kent 2018-06-07 343 s->s_magic = AUTOFS_SUPER_MAGIC;
ebc921ca9 Ian Kent 2018-06-07 344 s->s_op = _sops;
ebc921ca9 Ian Kent 2018-06-07 345 s->s_d_op = _dentry_operations;
ebc921ca9 Ian Kent 2018-06-07 346 s->s_time_gran = 1;
ebc921ca9 Ian Kent 2018-06-07 347
ebc921ca9 Ian Kent 2018-06-07 348 /*
ebc921ca9 Ian Kent 2018-06-07 349 * Get the root inode and dentry, but
defer checking for errors.
ebc921ca9 Ian Kent 2018-06-07 350 */
ebc921ca9 Ian Kent 2018-06-07 351 ino = autofs_new_ino(sbi);
ebc921ca9 Ian Kent 2018-06-07 352 if (!ino) {
ebc921ca9 Ian Kent 2018-06-07 353 ret = -ENOMEM;
ebc921ca9 Ian Kent 2018-06-07 354 goto fail_free;
ebc921ca9 Ian Kent 2018-06-07 355 }
ebc921ca9 Ian Kent 2018-06-07 356 root_inode = autofs_get_inode(s,
S_IFDIR | 0755);
eef302f31 Ian Kent 2018-11-13 357 if (!root_inode) {
eef302f31 Ian Kent 2018-11-13 358 ret = -ENOMEM;
eef302f31 Ian Kent 2018-11-13 359 goto fail_ino;
eef302f31 Ian Kent 2018-11-13 360 }
ebc921ca9 Ian Kent 2018-06-07 361 root = d_make_root(root_inode);
ebc921ca9 Ian Kent 2018-06-07 362 if (!root)
eef302f31 Ian Kent 2018-11-13 363 goto fail_iput;
ebc921ca9 Ian Kent 2018-06-07 364
ebc921ca9 Ian Kent 2018-06-07 365 root->d_fsdata = ino;
ebc921ca9 Ian Kent 2018-06-07 366
f4507adde Ian Kent 2018-11-13 367 memset(, 0, sizeof(struct
autofs_fs_params));
f4507adde Ian Kent 2018-11-13 368 if (autofs_parse_options(data,
)) {
ebc921ca9 Ian Kent 2018-06-07 369 pr_err("called with bogus
options\n");
ebc921ca9 Ian Kent 2018-06-07 370 goto fail_dput;
ebc921ca9 Ian Kent 2018-06-07 371 }
f4507adde Ian Kent 2018-11-13 @372 root_inode->i_uid = params->uid;
f4507adde Ian Kent 2018-11-13 373 root_inode->i_gid = params->gid;
ebc921ca9 Ian Kent 2018-06-07 374
f4507adde Ian Kent 2018-11-13 375 ret = autofs_apply_sbi_options(sbi,
);
f4507adde Ian Kent 2018-11-13 376 if (ret)
ebc921ca9 Ian Kent 2018-06-07 377 goto fail_dput;
ebc921ca9 Ian Kent 2018-06-07 378
ebc921ca9 Ian Kent 2018-06-07 379 if (autofs_type_trigger(sbi->type))
ebc921ca9 Ian Kent 2018-06-07 380
__managed_dentry_set_managed(root);
Re: [PATCH 6/6] autofs - add strictexpire mount option
Hi Ian,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc2]
[cannot apply to next-20181115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Ian-Kent/autofs-improve-ioctl-sbi-checks/20181114-021150
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
>> fs/autofs/init.c:19:25: warning: symbol 'autofs_fs_type' was not declared.
>> Should it be static?
--
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
>> fs/autofs/inode.c:372:35: error: cannot dereference this type
fs/autofs/inode.c:373:35: error: cannot dereference this type
>> fs/autofs/inode.c:372:35: warning: unknown expression (8 46)
fs/autofs/inode.c:373:35: warning: unknown expression (8 46)
fs/autofs/inode.c: In function 'autofs_fill_super':
fs/autofs/inode.c:372:28: error: invalid type argument of '->' (have 'struct
autofs_fs_params')
root_inode->i_uid = params->uid;
^~
fs/autofs/inode.c:373:28: error: invalid type argument of '->' (have 'struct
autofs_fs_params')
root_inode->i_gid = params->gid;
^~
vim +372 fs/autofs/inode.c
e35160168 Ian Kent 2018-11-13 324
e35160168 Ian Kent 2018-11-13 325 int autofs_fill_super(struct super_block
*s, void *data, int silent)
e35160168 Ian Kent 2018-11-13 326 {
e35160168 Ian Kent 2018-11-13 327 struct inode *root_inode;
e35160168 Ian Kent 2018-11-13 328 struct dentry *root;
f4507adde Ian Kent 2018-11-13 329 struct autofs_fs_params params;
e35160168 Ian Kent 2018-11-13 330 struct autofs_sb_info *sbi;
e35160168 Ian Kent 2018-11-13 331 struct autofs_info *ino;
e35160168 Ian Kent 2018-11-13 332 int ret = -EINVAL;
e35160168 Ian Kent 2018-11-13 333
e35160168 Ian Kent 2018-11-13 334 sbi = autofs_alloc_sbi(s);
e35160168 Ian Kent 2018-11-13 335 if (!sbi)
e35160168 Ian Kent 2018-11-13 336 return -ENOMEM;
e35160168 Ian Kent 2018-11-13 337
e35160168 Ian Kent 2018-11-13 338 pr_debug("starting up, sbi = %p\n",
sbi);
e35160168 Ian Kent 2018-11-13 339
e35160168 Ian Kent 2018-11-13 340 s->s_fs_info = sbi;
ebc921ca9 Ian Kent 2018-06-07 341 s->s_blocksize = 1024;
ebc921ca9 Ian Kent 2018-06-07 342 s->s_blocksize_bits = 10;
ebc921ca9 Ian Kent 2018-06-07 343 s->s_magic = AUTOFS_SUPER_MAGIC;
ebc921ca9 Ian Kent 2018-06-07 344 s->s_op = _sops;
ebc921ca9 Ian Kent 2018-06-07 345 s->s_d_op = _dentry_operations;
ebc921ca9 Ian Kent 2018-06-07 346 s->s_time_gran = 1;
ebc921ca9 Ian Kent 2018-06-07 347
ebc921ca9 Ian Kent 2018-06-07 348 /*
ebc921ca9 Ian Kent 2018-06-07 349 * Get the root inode and dentry, but
defer checking for errors.
ebc921ca9 Ian Kent 2018-06-07 350 */
ebc921ca9 Ian Kent 2018-06-07 351 ino = autofs_new_ino(sbi);
ebc921ca9 Ian Kent 2018-06-07 352 if (!ino) {
ebc921ca9 Ian Kent 2018-06-07 353 ret = -ENOMEM;
ebc921ca9 Ian Kent 2018-06-07 354 goto fail_free;
ebc921ca9 Ian Kent 2018-06-07 355 }
ebc921ca9 Ian Kent 2018-06-07 356 root_inode = autofs_get_inode(s,
S_IFDIR | 0755);
eef302f31 Ian Kent 2018-11-13 357 if (!root_inode) {
eef302f31 Ian Kent 2018-11-13 358 ret = -ENOMEM;
eef302f31 Ian Kent 2018-11-13 359 goto fail_ino;
eef302f31 Ian Kent 2018-11-13 360 }
ebc921ca9 Ian Kent 2018-06-07 361 root = d_make_root(root_inode);
ebc921ca9 Ian Kent 2018-06-07 362 if (!root)
eef302f31 Ian Kent 2018-11-13 363 goto fail_iput;
ebc921ca9 Ian Kent 2018-06-07 364
ebc921ca9 Ian Kent 2018-06-07 365 root->d_fsdata = ino;
ebc921ca9 Ian Kent 2018-06-07 366
f4507adde Ian Kent 2018-11-13 367 memset(, 0, sizeof(struct
autofs_fs_params));
f4507adde Ian Kent 2018-11-13 368 if (autofs_parse_options(data,
)) {
ebc921ca9 Ian Kent 2018-06-07 369 pr_err("called with bogus
options\n");
ebc921ca9 Ian Kent 2018-06-07 370 goto fail_dput;
ebc921ca9 Ian Kent 2018-06-07 371 }
f4507adde Ian Kent 2018-11-13 @372 root_inode->i_uid = params->uid;
f4507adde Ian Kent 2018-11-13 373 root_inode->i_gid = params->gid;
ebc921ca9 Ian Kent 2018-06-07 374
f4507adde Ian Kent 2018-11-13 375 ret = autofs_apply_sbi_options(sbi,
);
f4507adde Ian Kent 2018-11-13 376 if (ret)
ebc921ca9 Ian Kent 2018-06-07 377 goto fail_dput;
ebc921ca9 Ian Kent 2018-06-07 378
ebc921ca9 Ian Kent 2018-06-07 379 if (autofs_type_trigger(sbi->type))
ebc921ca9 Ian Kent 2018-06-07 380
__managed_dentry_set_managed(root);
[PATCH 6/6] autofs - add strictexpire mount option
Commit 092a53452b (("autofs: take more care to not update last_used
on path walk") helped to (partially) resolve a problem where automounts
were not expiring due to aggressive accesses from user space.
This patch was later reverted because, for very large environments,
it meant more mount requests from clients and when there are a lot
of clients this caused a fairly significant increase in server load.
But there is a need for both types of expire check, depending on use
case, so add a mount option to allow for strict update of last use
of autofs dentrys (which just means not updating the last use on path
walk accesses).
Signed-off-by: Ian Kent
---
fs/autofs/autofs_i.h |1 +
fs/autofs/inode.c| 13 -
fs/autofs/root.c |5 -
include/uapi/linux/auto_fs.h |2 +-
4 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index 032cbb12531a..fa3733beb52e 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -102,6 +102,7 @@ struct autofs_wait_queue {
#define AUTOFS_SBI_MAGIC 0x6d4a556d
#define AUTOFS_SBI_CATATONIC 0x0001
+#define AUTOFS_SBI_STRICTEXPIRE 0x0002
struct autofs_sb_info {
u32 magic;
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 5e774852ae84..75d61f5a3069 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -87,6 +87,8 @@ static int autofs_show_options(struct seq_file *m, struct
dentry *root)
seq_printf(m, ",direct");
else
seq_printf(m, ",indirect");
+ if (sbi->flags & AUTOFS_SBI_STRICTEXPIRE)
+ seq_printf(m, ",strictexpire");
#ifdef CONFIG_CHECKPOINT_RESTORE
if (sbi->pipe)
seq_printf(m, ",pipe_ino=%ld", file_inode(sbi->pipe)->i_ino);
@@ -116,11 +118,12 @@ struct autofs_fs_params {
bool pgrp_set;
int min_proto;
int max_proto;
+ bool strictexpire;
unsigned int type;
};
enum {Opt_err, Opt_fd, Opt_uid, Opt_gid, Opt_pgrp, Opt_minproto, Opt_maxproto,
- Opt_indirect, Opt_direct, Opt_offset};
+ Opt_indirect, Opt_direct, Opt_offset, Opt_strictexpire};
static const match_table_t tokens = {
{Opt_fd, "fd=%u"},
@@ -132,6 +135,7 @@ static const match_table_t tokens = {
{Opt_indirect, "indirect"},
{Opt_direct, "direct"},
{Opt_offset, "offset"},
+ {Opt_strictexpire, "strictexpire"},
{Opt_err, NULL}
};
@@ -155,6 +159,7 @@ static int autofs_parse_options(char *options, struct
autofs_fs_params *params)
params->max_proto = AUTOFS_MAX_PROTO_VERSION;
params->pgrp_set = false;
+ params->strictexpire = false;
while ((p = strsep(, ",")) != NULL) {
int token;
@@ -210,6 +215,9 @@ static int autofs_parse_options(char *options, struct
autofs_fs_params *params)
case Opt_offset:
set_autofs_type_offset(>type);
break;
+ case Opt_strictexpire:
+ params->strictexpire = true;
+ break;
default:
return 1;
}
@@ -275,6 +283,9 @@ static int autofs_apply_sbi_options(struct autofs_sb_info
*sbi,
if (err < 0)
goto out_fput;
+ if (params->strictexpire)
+ sbi->flags |= AUTOFS_SBI_STRICTEXPIRE;
+
sbi->flags &= ~AUTOFS_SBI_CATATONIC;
return 0;
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 164ccd3402cf..1246f396bf0e 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -275,8 +275,11 @@ static int autofs_mount_wait(const struct path *path, bool
rcu_walk)
pr_debug("waiting for mount name=%pd\n", path->dentry);
status = autofs_wait(sbi, path, NFY_MOUNT);
pr_debug("mount wait done status=%d\n", status);
+ ino->last_used = jiffies;
+ return status;
}
- ino->last_used = jiffies;
+ if (!(sbi->flags & AUTOFS_SBI_STRICTEXPIRE))
+ ino->last_used = jiffies;
return status;
}
diff --git a/include/uapi/linux/auto_fs.h b/include/uapi/linux/auto_fs.h
index df31aa9c9a8c..082119630b49 100644
--- a/include/uapi/linux/auto_fs.h
+++ b/include/uapi/linux/auto_fs.h
@@ -23,7 +23,7 @@
#define AUTOFS_MIN_PROTO_VERSION 3
#define AUTOFS_MAX_PROTO_VERSION 5
-#define AUTOFS_PROTO_SUBVERSION3
+#define AUTOFS_PROTO_SUBVERSION4
/*
* The wait_queue_token (autofs_wqt_t) is part of a structure which is passed
[PATCH 6/6] autofs - add strictexpire mount option
Commit 092a53452b (("autofs: take more care to not update last_used
on path walk") helped to (partially) resolve a problem where automounts
were not expiring due to aggressive accesses from user space.
This patch was later reverted because, for very large environments,
it meant more mount requests from clients and when there are a lot
of clients this caused a fairly significant increase in server load.
But there is a need for both types of expire check, depending on use
case, so add a mount option to allow for strict update of last use
of autofs dentrys (which just means not updating the last use on path
walk accesses).
Signed-off-by: Ian Kent
---
fs/autofs/autofs_i.h |1 +
fs/autofs/inode.c| 13 -
fs/autofs/root.c |5 -
include/uapi/linux/auto_fs.h |2 +-
4 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index 032cbb12531a..fa3733beb52e 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -102,6 +102,7 @@ struct autofs_wait_queue {
#define AUTOFS_SBI_MAGIC 0x6d4a556d
#define AUTOFS_SBI_CATATONIC 0x0001
+#define AUTOFS_SBI_STRICTEXPIRE 0x0002
struct autofs_sb_info {
u32 magic;
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 5e774852ae84..75d61f5a3069 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -87,6 +87,8 @@ static int autofs_show_options(struct seq_file *m, struct
dentry *root)
seq_printf(m, ",direct");
else
seq_printf(m, ",indirect");
+ if (sbi->flags & AUTOFS_SBI_STRICTEXPIRE)
+ seq_printf(m, ",strictexpire");
#ifdef CONFIG_CHECKPOINT_RESTORE
if (sbi->pipe)
seq_printf(m, ",pipe_ino=%ld", file_inode(sbi->pipe)->i_ino);
@@ -116,11 +118,12 @@ struct autofs_fs_params {
bool pgrp_set;
int min_proto;
int max_proto;
+ bool strictexpire;
unsigned int type;
};
enum {Opt_err, Opt_fd, Opt_uid, Opt_gid, Opt_pgrp, Opt_minproto, Opt_maxproto,
- Opt_indirect, Opt_direct, Opt_offset};
+ Opt_indirect, Opt_direct, Opt_offset, Opt_strictexpire};
static const match_table_t tokens = {
{Opt_fd, "fd=%u"},
@@ -132,6 +135,7 @@ static const match_table_t tokens = {
{Opt_indirect, "indirect"},
{Opt_direct, "direct"},
{Opt_offset, "offset"},
+ {Opt_strictexpire, "strictexpire"},
{Opt_err, NULL}
};
@@ -155,6 +159,7 @@ static int autofs_parse_options(char *options, struct
autofs_fs_params *params)
params->max_proto = AUTOFS_MAX_PROTO_VERSION;
params->pgrp_set = false;
+ params->strictexpire = false;
while ((p = strsep(, ",")) != NULL) {
int token;
@@ -210,6 +215,9 @@ static int autofs_parse_options(char *options, struct
autofs_fs_params *params)
case Opt_offset:
set_autofs_type_offset(>type);
break;
+ case Opt_strictexpire:
+ params->strictexpire = true;
+ break;
default:
return 1;
}
@@ -275,6 +283,9 @@ static int autofs_apply_sbi_options(struct autofs_sb_info
*sbi,
if (err < 0)
goto out_fput;
+ if (params->strictexpire)
+ sbi->flags |= AUTOFS_SBI_STRICTEXPIRE;
+
sbi->flags &= ~AUTOFS_SBI_CATATONIC;
return 0;
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 164ccd3402cf..1246f396bf0e 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -275,8 +275,11 @@ static int autofs_mount_wait(const struct path *path, bool
rcu_walk)
pr_debug("waiting for mount name=%pd\n", path->dentry);
status = autofs_wait(sbi, path, NFY_MOUNT);
pr_debug("mount wait done status=%d\n", status);
+ ino->last_used = jiffies;
+ return status;
}
- ino->last_used = jiffies;
+ if (!(sbi->flags & AUTOFS_SBI_STRICTEXPIRE))
+ ino->last_used = jiffies;
return status;
}
diff --git a/include/uapi/linux/auto_fs.h b/include/uapi/linux/auto_fs.h
index df31aa9c9a8c..082119630b49 100644
--- a/include/uapi/linux/auto_fs.h
+++ b/include/uapi/linux/auto_fs.h
@@ -23,7 +23,7 @@
#define AUTOFS_MIN_PROTO_VERSION 3
#define AUTOFS_MAX_PROTO_VERSION 5
-#define AUTOFS_PROTO_SUBVERSION3
+#define AUTOFS_PROTO_SUBVERSION4
/*
* The wait_queue_token (autofs_wqt_t) is part of a structure which is passed
