From patchwork Sun Jul 3 06:29:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 9211191 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 64E3C60571 for ; Sun, 3 Jul 2016 06:31:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 56E91285C1 for ; Sun, 3 Jul 2016 06:31:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4BA2F285E2; Sun, 3 Jul 2016 06:31:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3B5E5285C1 for ; Sun, 3 Jul 2016 06:31:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750935AbcGCGbL (ORCPT ); Sun, 3 Jul 2016 02:31:11 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:59840 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750798AbcGCGbJ (ORCPT ); Sun, 3 Jul 2016 02:31:09 -0400 Received: from viro by ZenIV.linux.org.uk with local (Exim 4.86_2 #1 (Red Hat Linux)) id 1bJauY-00059E-67; Sun, 03 Jul 2016 06:30:06 +0000 Date: Sun, 3 Jul 2016 07:29:46 +0100 From: Al Viro To: Oleg Drokin Cc: Mailing List , "" Subject: Re: More parallel atomic_open/d_splice_alias fun with NFS and possibly more FSes. Message-ID: <20160703062917.GG14480@ZenIV.linux.org.uk> References: <20160617042914.GD14480@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Jun 25, 2016 at 12:38:40PM -0400, Oleg Drokin wrote: > Sorry to nag you about this, but did any of those pan out? > > d_alloc_parallel() sounds like a bit too heavy there, esp. considering we came in with > a dentry already (though a potentially shared one, I understand). > Would not it be better to try and establish some dentry locking rule for calling into > d_splice_alias() instead? At least then the callers can make sure the dentry does > not change under them? > Though I guess if there's dentry locking like that, we might as well do all the > checking in d_splice_alias(), but that means the unhashed dentries would no > longer be disallowed which is a change of semantic from now.-- FWIW, the only interesting case here is this: * no O_CREAT in flags (otherwise the parent is held exclusive). * dentry is found in hash * dentry is negative * dentry has passed ->d_revalidate() (i.e. in case of NFS it had nfs_neg_need_reval() return false). Only two instances are non-trivial in that respect - NFS and Lustre. Everything else will simply fail open() with ENOENT in that case. And at least for NFS we could bloody well do d_drop + d_alloc_parallel + finish_no_open and bugger off in case it's not in_lookup, otherwise do pretty much what we do in case we'd got in_lookup from the very beginning. Some adjustments are needed for that case (basically, we need to make sure we hit d_lookup_done() matching that d_alloc_parallel() and deal with refcounting correctly). Tentative NFS patch follows; I don't understand Lustre well enough, but it looks like a plausible strategy there as well. --- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index d8015a03..5474e39 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1485,11 +1485,13 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, struct file *file, unsigned open_flags, umode_t mode, int *opened) { + DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq); struct nfs_open_context *ctx; struct dentry *res; struct iattr attr = { .ia_valid = ATTR_OPEN }; struct inode *inode; unsigned int lookup_flags = 0; + bool switched = false; int err; /* Expect a negative dentry */ @@ -1528,6 +1530,17 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, attr.ia_size = 0; } + if (!(open_flags & O_CREAT) && !d_unhashed(dentry)) { + d_drop(dentry); + switched = true; + dentry = d_alloc_parallel(dentry->d_parent, + &dentry->d_name, &wq); + if (IS_ERR(dentry)) + return PTR_ERR(dentry); + if (unlikely(!d_in_lookup(dentry))) + return finish_no_open(file, dentry); + } + ctx = create_nfs_open_context(dentry, open_flags); err = PTR_ERR(ctx); if (IS_ERR(ctx)) @@ -1563,14 +1576,23 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry, trace_nfs_atomic_open_exit(dir, ctx, open_flags, err); put_nfs_open_context(ctx); out: + if (unlikely(switched)) { + d_lookup_done(dentry); + dput(dentry); + } return err; no_open: res = nfs_lookup(dir, dentry, lookup_flags); - err = PTR_ERR(res); + if (switched) { + d_lookup_done(dentry); + if (!res) + res = dentry; + else + dput(dentry); + } if (IS_ERR(res)) - goto out; - + return PTR_ERR(res); return finish_no_open(file, res); } EXPORT_SYMBOL_GPL(nfs_atomic_open);