Commit eaf5f907 authored Jan 10, 2012 by Miklos Szeredi Committed by Al Viro Jan 10, 2012

fix shrink_dcache_parent() livelock



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
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

parent f4947fbc

fs/dcache.c

+11 −4

Original line number	Diff line number	Diff line
		@@ -243,6 +243,7 @@ static void dentry_lru_add(struct dentry *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 @@ relock:
		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 @@ resume:

		/*
		* 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

include/linux/dcache.h

+1 −0

Original line number	Diff line number	Diff line
		@@ -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