[PATCH] vfs: dcache: cond_resched in shrink_dentry_list

* [PATCH] vfs: dcache: cond_resched in shrink_dentry_list
@ 2013-03-25 17:22 Greg Thelen
  2013-03-25 23:56 ` Dave Chinner
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Thelen @ 2013-03-25 17:22 UTC (permalink / raw)
  To: Alexander Viro; +Cc: Dave Chinner, linux-fsdevel, linux-kernel, Greg Thelen

Call cond_resched() from shrink_dentry_list() to preserve
shrink_dcache_parent() interactivity.

void shrink_dcache_parent(struct dentry * parent)
{
	while ((found = select_parent(parent, &dispose)) != 0)
		shrink_dentry_list(&dispose);
}

select_parent() populates the dispose list with dentries which
shrink_dentry_list() then deletes.  select_parent() carefully uses
need_resched() to avoid doing too much work at once.  But neither
shrink_dcache_parent() nor its called functions call cond_resched().
So once need_resched() is set select_parent() will return single
dentry dispose list which is then deleted by shrink_dentry_list().
This is inefficient when there are a lot of dentry to process.  This
can cause softlockup and hurts interactivity on non preemptable
kernels.

This change adds a call to cond_resched() in shrink_dentry_list().
The primary benefit of this is that need_resched() is quickly cleared
so that future calls to select_parent() are able to efficiently return
a big batch of dentry.  A theoretically secondary benefit of this
change is that shrink_dentry_list() is willing to give up the
processor when working on a huge number of dentry.

These additional cond_resched() do not seem to impact performance, at
least for the workload below.

Here is a program which can cause soft lockup on a if other system
activity sets need_resched().

	int main()
	{
	        struct rlimit rlim;
	        int i;
	        int f[100000];
	        char buf[20];
	        struct timeval t1, t2;
	        double diff;

	        /* cleanup past run */
	        system("rm -rf x");

	        /* boost nfile rlimit */
	        rlim.rlim_cur = 200000;
	        rlim.rlim_max = 200000;
	        if (setrlimit(RLIMIT_NOFILE, &rlim))
	                err(1, "setrlimit");

	        /* make directory for files */
	        if (mkdir("x", 0700))
	                err(1, "mkdir");

	        if (gettimeofday(&t1, NULL))
	                err(1, "gettimeofday");

	        /* populate directory with open files */
	        for (i = 0; i < 100000; i++) {
	                snprintf(buf, sizeof(buf), "x/%d", i);
	                f[i] = open(buf, O_CREAT);
	                if (f[i] == -1)
	                        err(1, "open");
	        }

	        /* close some of the files */
	        for (i = 0; i < 85000; i++)
	                close(f[i]);

	        /* unlink all files, even open ones */
	        system("rm -rf x");

	        if (gettimeofday(&t2, NULL))
	                err(1, "gettimeofday");

	        diff = (((double)t2.tv_sec * 1000000 + t2.tv_usec) -
	                ((double)t1.tv_sec * 1000000 + t1.tv_usec));

	        printf("done: %g elapsed\n", diff/1e6);
	        return 0;
	}

Signed-off-by: Greg Thelen <gthelen@google.com>
---
 fs/dcache.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/dcache.c b/fs/dcache.c
index fbfae008..105e973 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -818,6 +818,8 @@ static void shrink_dentry_list(struct list_head *list)
 
 		try_prune_one_dentry(dentry);
 
+		cond_resched();
+
 		rcu_read_lock();
 	}
 	rcu_read_unlock();
-- 
1.8.1.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread