[PATCH] fs/namei: waiting for mutex during name lookups is "killable"

[PATCH] fs/namei: waiting for mutex during name lookups is "killable"

[Max Kellermann]

Use mutex_lock_killable() instead of mutex_lock() during name lookups,
to allow killing the process while it's waiting.

Signed-off-by: Max Kellermann <mk@cm4all.com>
---
 fs/namei.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 3d15072..08a3163 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1169,7 +1169,10 @@ retry:
 		struct inode *dir = parent->d_inode;
 		BUG_ON(nd->inode != dir);
 
-		mutex_lock(&dir->i_mutex);
+		err = mutex_lock_killable(&dir->i_mutex);
+		if (unlikely(err))
+			return err;
+
 		dentry = d_lookup(parent, name);
 		if (likely(!dentry)) {
 			dentry = d_alloc_and_lookup(parent, name, nd);
@@ -2166,7 +2169,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (nd->last.name[nd->last.len])
 		goto exit;
 
-	mutex_lock(&dir->d_inode->i_mutex);
+	error = mutex_lock_killable(&dir->d_inode->i_mutex);
+	if (unlikely(error))
+		goto exit;
 
 	dentry = lookup_hash(nd);
 	error = PTR_ERR(dentry);

Re: [PATCH] fs/namei: waiting for mutex during name lookups is "killable"

[Matthew Wilcox]

On Mon, Nov 28, 2011 at 12:39:18PM +0100, Max Kellermann wrote:
> Use mutex_lock_killable() instead of mutex_lock() during name lookups,
> to allow killing the process while it's waiting.

This is cool.  Could you describe what situations this mutex gets held
for a user-noticable length of time, and how you tested this patch?

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] fs/namei: waiting for mutex during name lookups is "killable"

[Max Kellermann]

On 2011/11/28 13:19, Matthew Wilcox <matthew@wil.cx> wrote:
> On Mon, Nov 28, 2011 at 12:39:18PM +0100, Max Kellermann wrote:
> > Use mutex_lock_killable() instead of mutex_lock() during name lookups,
> > to allow killing the process while it's waiting.
> 
> This is cool.  Could you describe what situations this mutex gets held
> for a user-noticable length of time, and how you tested this patch?

I experienced lots of blocked unkillable processes in an environment
that uses NFS mounts extensively.

There seems to be a lot wrong with Linux's NFS client; it waits for
RPC responses synchronously while the VFS layer holds mutexes, a
behaviour that should be avoided.  My patch does not try to address
this core issue.  Instead, it attempts to mitigate a nasty side
effect.

Most blocked processes were hanging inside stat(), in the code
locations that my patch changes.  After this patch, I could verify
that the affected processes could be killed, instead of hanging in the
task list until the one process holding the mutex would give up the
RPC request.

The patch has been in production on a few heavily loaded servers for
nearly a week, and I did not observe any negative side effects.

Max