* [PATCH] vfs: fix shrink_dcache_parent() livelock
@ 2012-01-10 11:47 Miklos Szeredi
2012-01-10 17:22 ` [PATCH v2] " Miklos Szeredi
0 siblings, 1 reply; 4+ messages in thread
From: Miklos Szeredi @ 2012-01-10 11:47 UTC (permalink / raw)
To: Al Viro
Cc: Linus Torvalds, Dave Chinner, Christoph Hellwig, linux-fsdevel,
linux-kernel, mgorman, gregkh, akpm
Here's my fix updated and tested on top of latest git.
Thanks,
Miklos
----
From: Miklos Szeredi <mszeredi@suse.cz>
Two (or more) concurrent calls of shrink_dcache_parent() on the same
dentry may cause shrink_dcache_parent() to loop forever.
Here's what appears to happen:
1 - CPU0: select_parent(P) finds C and puts it on dispose list, returns 1
2 - CPU1: select_parent(P) locks P->d_lock
3 - CPU0: shrink_dentry_list() locks C->d_lock
dentry_kill(C) tries to lock P->d_lock but fails, unlocks C->d_lock
4 - CPU1: select_parent(P) locks C->d_lock,
moves C from dispose list being processed on CPU0 to the new
dispose list, returns 1
5 - CPU0: shrink_dentry_list() finds dispose list empty, returns
6 - Goto 2 with CPU0 and CPU1 switched
Basically select_parent() steals the dentry from shrink_dentry_list()
and thinks it found a new one, causing shrink_dentry_list() to think
it's making progress and loop over and over.
One way to trigger this is to make udev calls stat() on the sysfs file
while it is going away.
Having a file in /lib/udev/rules.d/ with only this one rule seems to the
trick:
ATTR{vendor}=="0x8086", ATTR{device}=="0x10ca", ENV{PCI_SLOT_NAME}="%k", ENV{MATCHADDR}="$attr{address}", RUN+="/bin/true"
Then execute the following loop:
while true; do
echo -bond0 > /sys/class/net/bonding_masters
echo +bond0 > /sys/class/net/bonding_masters
echo -bond1 > /sys/class/net/bonding_masters
echo +bond1 > /sys/class/net/bonding_masters
done
This should shortly trigger the soft lockup detector. Note that
DEBUG_SPINLOCK seems to change the timing enough to prevent the hang
from happening.
One fix would be to check all callers and prevent concurrent calls to
shrink_dcache_parent(). But I think a better solution is to stop the
stealing behavior.
This patch adds a new dentry flag that is set when the dentry is added
to the dispose list. The flag is cleared in dentry_lru_del() in case
the dentry gets a new reference just before being pruned.
If the dentry has this flag, select_parent() will skip it and let
shrink_dentry_list() retry pruning it. With select_parent() skipping
those dentries there will not be the appearance of progress (new
dentries found) hence shrink_dcache_parent() will not loop forever.
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/dcache.c | 14 ++++++++++----
include/linux/dcache.h | 1 +
2 files changed, 11 insertions(+), 4 deletions(-)
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2012-01-10 11:36:46.000000000 +0100
+++ linux-2.6/fs/dcache.c 2012-01-10 11:46:41.000000000 +0100
@@ -243,6 +243,7 @@ static void dentry_lru_add(struct dentry
static void __dentry_lru_del(struct dentry *dentry)
{
list_del_init(&dentry->d_lru);
+ dentry->d_flags &= ~DCACHE_SHRINK_LIST;
dentry->d_sb->s_nr_dentry_unused--;
dentry_stat.nr_unused--;
}
@@ -1097,14 +1098,19 @@ static int select_parent(struct dentry *
/*
* move only zero ref count dentries to the dispose list.
+ *
+ * Those which are presently on the shrink list, being processed
+ * by shrink_dentry_list(), shouldn't be moved. Otherwise the
+ * loop in shrink_dcache_parent() might not make any progress
+ * and loop forever.
*/
- if (!dentry->d_count) {
+ if (dentry->d_count) {
+ dentry_lru_del(dentry);
+ } else if (!(dentry->d_flags & DCACHE_SHRINK_LIST)) {
dentry_lru_move_list(dentry, dispose);
+ dentry->d_flags |= DCACHE_SHRINK_LIST;
found++;
- } else {
- dentry_lru_del(dentry);
}
-
/*
* We can return to the caller if we have found some (this
* ensures forward progress). We'll be coming back to find
Index: linux-2.6/include/linux/dcache.h
===================================================================
--- linux-2.6.orig/include/linux/dcache.h 2012-01-10 11:36:46.000000000 +0100
+++ linux-2.6/include/linux/dcache.h 2012-01-10 11:39:24.000000000 +0100
@@ -203,6 +203,7 @@ struct dentry_operations {
#define DCACHE_CANT_MOUNT 0x0100
#define DCACHE_GENOCIDE 0x0200
+#define DCACHE_SHRINK_LIST 0x0400
#define DCACHE_NFSFS_RENAMED 0x1000
/* this dentry has been "silly renamed" and has to be deleted on the last
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] vfs: fix shrink_dcache_parent() livelock
2012-01-10 11:47 [PATCH] vfs: fix shrink_dcache_parent() livelock Miklos Szeredi
@ 2012-01-10 17:22 ` Miklos Szeredi
2012-01-10 17:30 ` Linus Torvalds
2012-01-10 18:05 ` Al Viro
0 siblings, 2 replies; 4+ messages in thread
From: Miklos Szeredi @ 2012-01-10 17:22 UTC (permalink / raw)
To: Al Viro
Cc: Linus Torvalds, Dave Chinner, Christoph Hellwig, linux-fsdevel,
linux-kernel, mgorman, gregkh, akpm
Set flag in prune_dcache_sb() for consistency. Add stable@ to cc.
Please apply.
Thanks,
Miklos
----
Subject: vfs: fix shrink_dcache_parent() livelock
From: Miklos Szeredi <mszeredi@suse.cz>
Two (or more) concurrent calls of shrink_dcache_parent() on the same dentry may
cause shrink_dcache_parent() to loop forever.
Here's what appears to happen:
1 - CPU0: select_parent(P) finds C and puts it on dispose list, returns 1
2 - CPU1: select_parent(P) locks P->d_lock
3 - CPU0: shrink_dentry_list() locks C->d_lock
dentry_kill(C) tries to lock P->d_lock but fails, unlocks C->d_lock
4 - CPU1: select_parent(P) locks C->d_lock,
moves C from dispose list being processed on CPU0 to the new
dispose list, returns 1
5 - CPU0: shrink_dentry_list() finds dispose list empty, returns
6 - Goto 2 with CPU0 and CPU1 switched
Basically select_parent() steals the dentry from shrink_dentry_list() and thinks
it found a new one, causing shrink_dentry_list() to think it's making progress
and loop over and over.
One way to trigger this is to make udev calls stat() on the sysfs file while it
is going away.
Having a file in /lib/udev/rules.d/ with only this one rule seems to the trick:
ATTR{vendor}=="0x8086", ATTR{device}=="0x10ca", ENV{PCI_SLOT_NAME}="%k", ENV{MATCHADDR}="$attr{address}", RUN+="/bin/true"
Then execute the following loop:
while true; do
echo -bond0 > /sys/class/net/bonding_masters
echo +bond0 > /sys/class/net/bonding_masters
echo -bond1 > /sys/class/net/bonding_masters
echo +bond1 > /sys/class/net/bonding_masters
done
One fix would be to check all callers and prevent concurrent calls to
shrink_dcache_parent(). But I think a better solution is to stop the
stealing behavior.
This patch adds a new dentry flag that is set when the dentry is added to the
dispose list. The flag is cleared in dentry_lru_del() in case the dentry gets a
new reference just before being pruned.
If the dentry has this flag, select_parent() will skip it and let
shrink_dentry_list() retry pruning it. With select_parent() skipping those
dentries there will not be the appearance of progress (new dentries found) when
there is none, hence shrink_dcache_parent() will not loop forever.
Set the flag is also set in prune_dcache_sb() for consistency as suggested by
Linus.
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: stable@vger.kernel.org
---
fs/dcache.c | 15 +++++++++++----
include/linux/dcache.h | 1 +
2 files changed, 12 insertions(+), 4 deletions(-)
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2012-01-10 16:18:30.000000000 +0100
+++ linux-2.6/fs/dcache.c 2012-01-10 17:39:46.000000000 +0100
@@ -243,6 +243,7 @@ static void dentry_lru_add(struct dentry
static void __dentry_lru_del(struct dentry *dentry)
{
list_del_init(&dentry->d_lru);
+ dentry->d_flags &= ~DCACHE_SHRINK_LIST;
dentry->d_sb->s_nr_dentry_unused--;
dentry_stat.nr_unused--;
}
@@ -806,6 +807,7 @@ void prune_dcache_sb(struct super_block
spin_unlock(&dentry->d_lock);
} else {
list_move_tail(&dentry->d_lru, &tmp);
+ dentry->d_flags |= DCACHE_SHRINK_LIST;
spin_unlock(&dentry->d_lock);
if (!--count)
break;
@@ -1097,14 +1099,19 @@ static int select_parent(struct dentry *
/*
* move only zero ref count dentries to the dispose list.
+ *
+ * Those which are presently on the shrink list, being processed
+ * by shrink_dentry_list(), shouldn't be moved. Otherwise the
+ * loop in shrink_dcache_parent() might not make any progress
+ * and loop forever.
*/
- if (!dentry->d_count) {
+ if (dentry->d_count) {
+ dentry_lru_del(dentry);
+ } else if (!(dentry->d_flags & DCACHE_SHRINK_LIST)) {
dentry_lru_move_list(dentry, dispose);
+ dentry->d_flags |= DCACHE_SHRINK_LIST;
found++;
- } else {
- dentry_lru_del(dentry);
}
-
/*
* We can return to the caller if we have found some (this
* ensures forward progress). We'll be coming back to find
Index: linux-2.6/include/linux/dcache.h
===================================================================
--- linux-2.6.orig/include/linux/dcache.h 2012-01-10 16:18:30.000000000 +0100
+++ linux-2.6/include/linux/dcache.h 2012-01-10 17:36:29.000000000 +0100
@@ -203,6 +203,7 @@ struct dentry_operations {
#define DCACHE_CANT_MOUNT 0x0100
#define DCACHE_GENOCIDE 0x0200
+#define DCACHE_SHRINK_LIST 0x0400
#define DCACHE_NFSFS_RENAMED 0x1000
/* this dentry has been "silly renamed" and has to be deleted on the last
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] vfs: fix shrink_dcache_parent() livelock
2012-01-10 17:22 ` [PATCH v2] " Miklos Szeredi
@ 2012-01-10 17:30 ` Linus Torvalds
2012-01-10 18:05 ` Al Viro
1 sibling, 0 replies; 4+ messages in thread
From: Linus Torvalds @ 2012-01-10 17:30 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Al Viro, Dave Chinner, Christoph Hellwig, linux-fsdevel,
linux-kernel, mgorman, gregkh, akpm
On Tue, Jan 10, 2012 at 9:22 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> Set flag in prune_dcache_sb() for consistency. Add stable@ to cc.
Looks good to me.
Al, I'm leaving it to you and forgetting about this issue for now,
Linus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] vfs: fix shrink_dcache_parent() livelock
2012-01-10 17:22 ` [PATCH v2] " Miklos Szeredi
2012-01-10 17:30 ` Linus Torvalds
@ 2012-01-10 18:05 ` Al Viro
1 sibling, 0 replies; 4+ messages in thread
From: Al Viro @ 2012-01-10 18:05 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Linus Torvalds, Dave Chinner, Christoph Hellwig, linux-fsdevel,
linux-kernel, mgorman, gregkh, akpm
On Tue, Jan 10, 2012 at 06:22:25PM +0100, Miklos Szeredi wrote:
> Set flag in prune_dcache_sb() for consistency. Add stable@ to cc.
>
> Please apply.
Done.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-01-10 18:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-10 11:47 [PATCH] vfs: fix shrink_dcache_parent() livelock Miklos Szeredi
2012-01-10 17:22 ` [PATCH v2] " Miklos Szeredi
2012-01-10 17:30 ` Linus Torvalds
2012-01-10 18:05 ` Al Viro
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).