Re: [PATCH v4 1/1] dcache: Translating dentry into pathname withouttaking rename_lock

From: Waiman Long
Date: Mon Sep 09 2013 - 18:57:49 EST


On 09/09/2013 03:28 PM, Al Viro wrote:
On Mon, Sep 09, 2013 at 08:10:29PM +0100, Al Viro wrote:
On Mon, Sep 09, 2013 at 02:46:57PM -0400, Waiman Long wrote:

I am fine with your proposed change as long as it gets the job done.
I suspect that the real problem is the unlock part of read_seqretry_or_unlock();
for d_walk() we want to be able to check if we need retry and continue walking
if we do not. Let's do it that way: I've applied your patch as is, with the
next step being
* split read_seqretry_or_unlock():
need_seqretry() (return (!(seq& 1)&& read_seqretry(lock, seq))
done_seqretry() (if (seq& 1) write_sequnlock(lock, seq)),
your if (read_seqretry_or_unlock(&rename_lock,&seq))
goto restart;
becoming
if (need_seqretry(&rename_lock, seq)) {
seq = 1;
goto restart;
}
done_seqretry(&rename_lock, seq);

Then d_walk() is trivially massaged to use of read_seqbegin_or_lock(),
need_seqretry() and done_seqretry(). Give me a few, I'll post it...
OK, how about this? It splits read_seqretry_or_unlock(), takes
rcu_read_{lock,unlock} in the callers and converts d_walk() to those
primitives. I've pushed that and your commit into vfs.git#experimental
(head at 48f5ec2, should propagate in a few); guys, please give it a look
and comment.

The changes look good to me. I was planning to take rcu_read_lock() out and doing something similar, but your change is good. BTW, I think Linus want to add some comments on why RCU lock is needed without the rename_lock, but I can put that in with a follow-up patch once the current change is merged.

Thank for your help and inspiration on this patch.

-Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/