Re: [PATCH 2/2] nfs: fix return err if inode exiting in nfs_instantiate

[Rui Xiang <rui.xiang@huawei.com>]

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 <rui.xiang@huawei.com> 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 <rui.xiang@huawei.com>
>>>> ---
>>>> 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