From: Christoph Hellwig <hch@infradead.org>
To: Jeff Layton <jeff.layton@primarydata.com>
Cc: bfields@fieldses.org, trond.myklebust@primarydata.com,
bhalevy@primarydata.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 9/9] NFSd: protect delegation setup with the i_lock
Date: Mon, 2 Jun 2014 11:20:26 -0700 [thread overview]
Message-ID: <20140602182026.GA27238@infradead.org> (raw)
In-Reply-To: <20140602101733.651cd9c8@tlielax.poochiereds.net>
On Mon, Jun 02, 2014 at 10:17:33AM -0400, Jeff Layton wrote:
> Ok, fair enough. A later patch in the series adds a per nfs4_file lock
> (the fi_lock) that we can use for this purpose in lieu of the i_lock.
> Nesting that within the i_lock shouldn't be too painful.
i_lock is part of some very complex locking hierachies in the inode
and dentry caches. Long term I'd prefer to get the file locking code
detangled from that, but let's keep it simple for now and nest
the nfs4_file lock inside for now.
> We could do that within the rpc_call_prepare operation, but the main
> problem there is that the delegation could leak if the rpc task
> allocation fails. Would you be amenable to adding a "cb_prepare"
> operation or something that we could use to run things from the
> workqueue before the actual callback runs?
I guess we'll have to do it then. I'll see if it can be done nicely.
I suspect the root problem of all this is that the file locking code
calls the lock manager ops with i_lock held, which isn't very nice.
next prev parent reply other threads:[~2014-06-02 18:20 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-30 13:09 [PATCH 0/9] nfsd: bugfixes and preliminary patches for client_mutex removal Jeff Layton
2014-05-30 13:09 ` [PATCH 1/9] nfsd: make nfsd4_encode_fattr static Jeff Layton
2014-05-30 13:09 ` [PATCH 2/9] nfsd: fix laundromat next-run-time calculation Jeff Layton
2014-05-30 13:09 ` [PATCH 3/9] nfsd4: use recall_lock for delegation hashing Jeff Layton
2014-05-30 13:09 ` [PATCH 4/9] nfsd: fix setting of NFS4_OO_CONFIRMED in nfsd4_open Jeff Layton
2014-05-30 13:09 ` [PATCH 5/9] nfsd: remove unneeded zeroing of fields in nfsd4_proc_compound Jeff Layton
2014-05-30 13:09 ` [PATCH 6/9] nfsd4: rename recall_lock to state_lock Jeff Layton
2014-05-30 13:09 ` [PATCH 7/9] nfsd4: hash deleg stateid only on successful nfs4_set_delegation Jeff Layton
2014-05-30 13:09 ` [PATCH 8/9] NFSd: Protect addition to the file_hashtbl Jeff Layton
2014-06-05 16:12 ` J. Bruce Fields
2014-06-05 16:18 ` Trond Myklebust
2014-06-05 16:27 ` Jeff Layton
2014-05-30 13:09 ` [PATCH 9/9] NFSd: protect delegation setup with the i_lock Jeff Layton
2014-06-02 8:46 ` Christoph Hellwig
2014-06-02 14:17 ` Jeff Layton
2014-06-02 18:20 ` Christoph Hellwig [this message]
2014-06-02 18:48 ` Jeff Layton
2014-05-30 13:13 ` [PATCH 10/9] nfsd: remove initial assignment of "p" in nfsd4_encode_security_label Jeff Layton
2014-06-04 19:56 ` [PATCH 0/9] nfsd: bugfixes and preliminary patches for client_mutex removal J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140602182026.GA27238@infradead.org \
--to=hch@infradead.org \
--cc=bfields@fieldses.org \
--cc=bhalevy@primarydata.com \
--cc=jeff.layton@primarydata.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@primarydata.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).