* [PATCH v3 24/25] sunrpc: Change how dentry's d_lock field is accessed
@ 2013-07-03 20:25 Waiman Long
2013-07-04 4:20 ` Al Viro
0 siblings, 1 reply; 3+ messages in thread
From: Waiman Long @ 2013-07-03 20:25 UTC (permalink / raw)
To: Alexander Viro, Jeff Layton, Miklos Szeredi, Ingo Molnar,
Thomas Gleixner
Cc: Trond Myklebust, J. Bruce Fields, David S. Miller, linux-nfs,
netdev, Waiman Long, linux-fsdevel, linux-kernel, Peter Zijlstra,
Steven Rostedt, Linus Torvalds, Benjamin Herrenschmidt,
Andi Kleen, Chandramouleeswaran, Aswin, Norton, Scott J
Because of the changes made in dcache.h header file, files that
use the d_lock field of the dentry structure need to be changed
accordingly. All the d_lock's spin_lock() and spin_unlock() calls
are replaced by the corresponding d_lock() and d_unlock() calls.
There is no change in logic and everything should just work.
Signed-off-by: Waiman Long <Waiman.Long@hp.com>
---
net/sunrpc/rpc_pipe.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index e7ce4b3..58bc128 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -427,14 +427,14 @@ rpc_info_open(struct inode *inode, struct file *file)
if (!ret) {
struct seq_file *m = file->private_data;
- spin_lock(&file->f_path.dentry->d_lock);
+ d_lock(file->f_path.dentry);
if (!d_unhashed(file->f_path.dentry))
clnt = RPC_I(inode)->private;
if (clnt != NULL && atomic_inc_not_zero(&clnt->cl_count)) {
- spin_unlock(&file->f_path.dentry->d_lock);
+ d_unlock(file->f_path.dentry);
m->private = clnt;
} else {
- spin_unlock(&file->f_path.dentry->d_lock);
+ d_unlock(file->f_path.dentry);
single_release(inode, file);
ret = -EINVAL;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3 24/25] sunrpc: Change how dentry's d_lock field is accessed
2013-07-03 20:25 [PATCH v3 24/25] sunrpc: Change how dentry's d_lock field is accessed Waiman Long
@ 2013-07-04 4:20 ` Al Viro
2013-07-08 16:53 ` Myklebust, Trond
0 siblings, 1 reply; 3+ messages in thread
From: Al Viro @ 2013-07-04 4:20 UTC (permalink / raw)
To: Waiman Long
Cc: Jeff Layton, Miklos Szeredi, Ingo Molnar, Thomas Gleixner,
Trond Myklebust, J. Bruce Fields, David S. Miller, linux-nfs,
netdev, linux-fsdevel, linux-kernel, Peter Zijlstra,
Steven Rostedt, Linus Torvalds, Benjamin Herrenschmidt,
Andi Kleen, Chandramouleeswaran, Aswin, Norton, Scott J
On Wed, Jul 03, 2013 at 04:25:32PM -0400, Waiman Long wrote:
> There is no change in logic and everything should just work.
> - spin_lock(&file->f_path.dentry->d_lock);
> + d_lock(file->f_path.dentry);
> if (!d_unhashed(file->f_path.dentry))
> clnt = RPC_I(inode)->private;
> if (clnt != NULL && atomic_inc_not_zero(&clnt->cl_count)) {
> - spin_unlock(&file->f_path.dentry->d_lock);
> + d_unlock(file->f_path.dentry);
Could somebody explain WTF is being protected here? It's not ->private -
that gets set (and, more importantly, cleared) without ->d_lock in sight.
Trond, that seems to be your code from about three years ago (introduced
in "SUNRPC: Fix a race in rpc_info_open"). What's going on there?
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3 24/25] sunrpc: Change how dentry's d_lock field is accessed
2013-07-04 4:20 ` Al Viro
@ 2013-07-08 16:53 ` Myklebust, Trond
0 siblings, 0 replies; 3+ messages in thread
From: Myklebust, Trond @ 2013-07-08 16:53 UTC (permalink / raw)
To: Al Viro
Cc: Waiman Long, Jeff Layton, Miklos Szeredi, Ingo Molnar,
Thomas Gleixner, J. Bruce Fields, David S. Miller,
linux-nfs@vger.kernel.org, netdev@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
Peter Zijlstra, Steven Rostedt, Linus Torvalds,
Benjamin Herrenschmidt, Andi Kleen, Chandramouleeswaran, Aswin,
Norton, Scott J
On Thu, 2013-07-04 at 05:20 +0100, Al Viro wrote:
> On Wed, Jul 03, 2013 at 04:25:32PM -0400, Waiman Long wrote:
> > There is no change in logic and everything should just work.
>
> > - spin_lock(&file->f_path.dentry->d_lock);
> > + d_lock(file->f_path.dentry);
> > if (!d_unhashed(file->f_path.dentry))
> > clnt = RPC_I(inode)->private;
> > if (clnt != NULL && atomic_inc_not_zero(&clnt->cl_count)) {
> > - spin_unlock(&file->f_path.dentry->d_lock);
> > + d_unlock(file->f_path.dentry);
>
> Could somebody explain WTF is being protected here? It's not ->private -
> that gets set (and, more importantly, cleared) without ->d_lock in sight.
> Trond, that seems to be your code from about three years ago (introduced
> in "SUNRPC: Fix a race in rpc_info_open"). What's going on there?
AFAICR we're using the fact that the dentry will remain hashed until
we're in the process of freeing up the rpc_client. By testing that the
dentry is hashed under the dentry->d_lock, we are assured that the
non-NULL 'clnt' is still pointing to a valid rpc_client, and that it is
safe to access clnt->cl_count.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@netapp.com
www.netapp.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-07-08 16:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-03 20:25 [PATCH v3 24/25] sunrpc: Change how dentry's d_lock field is accessed Waiman Long
2013-07-04 4:20 ` Al Viro
2013-07-08 16:53 ` Myklebust, Trond
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).