Return-Path: linux-nfs-owner@vger.kernel.org Received: from szxga01-in.huawei.com ([119.145.14.64]:45375 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750990Ab3LKGKH (ORCPT ); Wed, 11 Dec 2013 01:10:07 -0500 Message-ID: <52A801A4.30809@huawei.com> Date: Wed, 11 Dec 2013 14:09:40 +0800 From: Rui Xiang MIME-Version: 1.0 To: Trond Myklebust CC: Linux NFS Mailing List Subject: Re: [PATCH 2/2] nfs: fix return err if inode exiting in nfs_instantiate References: <1386322217-27436-1-git-send-email-rui.xiang@huawei.com> <1386322217-27436-2-git-send-email-rui.xiang@huawei.com> <52A59EAB.20200@huawei.com> <1386692771.2879.2.camel@leira.trondhjem.org> In-Reply-To: <1386692771.2879.2.camel@leira.trondhjem.org> Content-Type: text/plain; charset="UTF-8" Sender: linux-nfs-owner@vger.kernel.org List-ID: On 2013/12/11 0:26, Trond Myklebust wrote: > On Mon, 2013-12-09 at 18:42 +0800, Rui Xiang wrote: >> On 2013/12/6 21:46, Trond Myklebust wrote: >>> >>> On Dec 6, 2013, at 4:30, Rui Xiang wrote: >>> >>>> In common function nfs_instantiate to create, mkdir, and mknod, >>>> if dentry->d_inode exits, it should return -EEXIST instead of >>>> -EACCES. >>>> >>>> Signed-off-by: Rui Xiang >>>> --- >>>> fs/nfs/dir.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c >>>> index 2518865..e570b37 100644 >>>> --- a/fs/nfs/dir.c >>>> +++ b/fs/nfs/dir.c >>>> @@ -1547,7 +1547,7 @@ int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fhandle, >>>> struct dentry *parent = dget_parent(dentry); >>>> struct inode *dir = parent->d_inode; >>>> struct inode *inode; >>>> - int error = -EACCES; >>>> + int error = -EEXIST; >>>> >>>> d_drop(dentry); >>>> >>> >>> That looks like it should rather be a WARN_ON(). If the caller has set the dentry's inode before creating the file, then something is really wrong. >> It can return -EEXIST to say that the file already exits, then will exit. But why does it need a WARN_ON(). Please give me some advise. >> > > If we ever hit that condition then it means that the caller is doing > something very wrong. That's what the WARN_ON (or WARN_ON_ONCE) would be > there to check. > OK. In that case, is it necessary to send a new patch with the WARN_ON? Thanks Rui