On Thu, 2012-06-21 at 15:18 -0700, Jan Sanislo wrote:
> > Can you offer any information about the situation when you get these
> > fails? Such as what type of autofs activity was happening, light/heavy
> > mount activity, mounts being attempted simple indirect or direct mounts,
> > IOW what do the maps look like and which type of entries were being used
> > at the time.
>
> There was no file system activity at all. Basically, a file system
> was automounted by doing an ls and that was all. The failure occurs
> (it appears) when automount tries to do an expire. The automount
> maps look like:
>
> /etc/auto.master:
> /homes/gws /etc/auto.homes.gws -rw,intr,nosuid,nodev,strict
>
> /etc/auto.homes.gws:
> user1 xxx:/y/z/&
> user2 aaa:/b/c/&
> ......
>
> Doing an "ls /homes/gws/user1" will succeed. Several minutes later
> during an expiration attempt the failures occur.
Right, a simple indirect map, there shouldn't be any surprises using
that.
I tested my version of the patch last night and while there is still a
problem I haven't been able to track down it's nothing like what you're
seeing.
Anyway, here is the version I used with 3.5.0-rc3 applied to 3.4.2.
It applied cleanly for me so it should behave.
autofs4 - fix get_next_positive_subdir()
From: Ian Kent <raven@xxxxxxxxxx>
Following a report of a crash during an automount expire I found that
the locking in fs/autofs4/expire.c:get_next_positive_subdir() was wrong.
Not only is the locking wrong but the function is more complex than it
needs to be.
The function is meant to calculate (and dget) the next entry in the list
of directories contained in the root of an autofs mount point (an autofs
indirect mount to be precise). The main problem was that the d_lock of
the owner of the list was not being taken when walking the list, which
lead to list corruption under load. The only other lock that needs to
be taken is against the next dentry candidate so it can be checked for
usability.
Signed-off-by: Ian Kent <raven@xxxxxxxxxx>
---
fs/autofs4/expire.c | 31 +++++++++++++------------------
1 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
index 1feb68e..8c0e56d 100644
--- a/fs/autofs4/expire.c
+++ b/fs/autofs4/expire.c
@@ -94,25 +94,21 @@ static struct dentry *get_next_positive_subdir(struct dentry *prev,
{
struct autofs_sb_info *sbi = autofs4_sbi(root->d_sb);
struct list_head *next;
- struct dentry *p, *q;
+ struct dentry *q;
spin_lock(&sbi->lookup_lock);
+ spin_lock(&root->d_lock);
- if (prev == NULL) {
- spin_lock(&root->d_lock);
+ if (prev)
+ next = prev->d_u.d_child.next;
+ else {
prev = dget_dlock(root);
next = prev->d_subdirs.next;
- p = prev;
- goto start;
}
- p = prev;
- spin_lock(&p->d_lock);
-again:
- next = p->d_u.d_child.next;
-start:
+cont:
if (next == &root->d_subdirs) {
- spin_unlock(&p->d_lock);
+ spin_unlock(&root->d_lock);
spin_unlock(&sbi->lookup_lock);
dput(prev);
return NULL;
@@ -121,16 +117,15 @@ start:
q = list_entry(next, struct dentry, d_u.d_child);
spin_lock_nested(&q->d_lock, DENTRY_D_LOCK_NESTED);
- /* Negative dentry - try next */
- if (!simple_positive(q)) {
- spin_unlock(&p->d_lock);
- lock_set_subclass(&q->d_lock.dep_map, 0, _RET_IP_);
- p = q;
- goto again;
+ /* Already gone or negative dentry (under construction) - try next */
+ if (q->d_count == 0 || !simple_positive(q)) {
+ spin_unlock(&q->d_lock);
+ next = q->d_u.d_child.next;
+ goto cont;
}
dget_dlock(q);
spin_unlock(&q->d_lock);
- spin_unlock(&p->d_lock);
+ spin_unlock(&root->d_lock);
spin_unlock(&sbi->lookup_lock);
dput(prev);
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
