Matthew Wilcox wrote:
> On Fri, Jan 18, 2008 at 10:36:01AM -0500, Peter Staubach wrote:
>
>> @@ -1025,12 +1027,27 @@ static int fastcall link_path_walk(const
>> mntget(save.mnt);
>>
>> result = __link_path_walk(name, nd);
>> - if (result == -ESTALE) {
>> + while (result == -ESTALE) {
>> + /*
>> + * If no progress was made looking up the pathname,
>> + * then stop and return ENOENT instead of ESTALE.
>> + */
>> + if (nd->dentry == save.dentry) {
>> + result = -ENOENT;
>> + break;
>> + }
>> *nd = save;
>> dget(nd->dentry);
>> mntget(nd->mnt);
>> nd->flags |= LOOKUP_REVAL;
>> result = __link_path_walk(name, nd);
>> + /*
>> + * If no progress was made this time, then return
>> + * ENOENT instead of ESTALE because no recovery
>> + * is possible to recover the stale file handle.
>> + */
>> + if (result == -ESTALE && nd->dentry == save.dentry)
>> + result = -ENOENT;
>> }
>>
>> dput(save.dentry);
>>
>
> Why do you need both of these tests? The first one should be enough,
> surely?
>
>
Yes, good point.
>> @@ -1268,8 +1285,8 @@ int path_lookup_open(int dfd, const char
>> * @create_mode: create intent flags
>> */
>> static int path_lookup_create(int dfd, const char *name,
>> - unsigned int lookup_flags, struct nameidata *nd,
>> - int open_flags, int create_mode)
>> + unsigned int lookup_flags, struct nameidata *nd,
>> + int open_flags, int create_mode)
>>
>
> Gratuitous reformatting?
>
>
Elimination of an overly long line?
>> @@ -1712,7 +1729,10 @@ int open_namei(int dfd, const char *path
>> int acc_mode, error;
>> struct path path;
>> struct dentry *dir;
>> - int count = 0;
>> + int count;
>> +
>> +top:
>> + count = 0;
>>
>> acc_mode = ACC_MODE(flag);
>>
>> @@ -1739,7 +1759,8 @@ int open_namei(int dfd, const char *path
>> /*
>> * Create - we need to know the parent.
>> */
>> - error = path_lookup_create(dfd,pathname,LOOKUP_PARENT,nd,flag,mode);
>> + error = path_lookup_create(dfd, pathname, LOOKUP_PARENT, nd,
>> + flag, mode);
>> if (error)
>> return error;
>>
>> @@ -1812,10 +1833,17 @@ ok:
>> return 0;
>>
>> exit_dput:
>> + if (error == -ESTALE)
>> + d_drop(path.dentry);
>> dput_path(&path, nd);
>> exit:
>> if (!IS_ERR(nd->intent.open.file))
>> release_open_intent(nd);
>> + if (error == -ESTALE) {
>> + d_drop(nd->dentry);
>> + path_release(nd);
>> + goto top;
>> + }
>>
>
> I wonder if a tail-call might not work better here.
"Tail-call"?
Thanx...
ps
On Fri, Jan 18, 2008 at 11:45:52AM -0500, Peter Staubach wrote:
> Matthew Wilcox wrote:
>> On Fri, Jan 18, 2008 at 10:36:01AM -0500, Peter Staubach wrote:
>>> static int path_lookup_create(int dfd, const char *name,
>>> - unsigned int lookup_flags, struct nameidata *nd,
>>> - int open_flags, int create_mode)
>>> + unsigned int lookup_flags, struct nameidata *nd,
>>> + int open_flags, int create_mode)
>>>
>>
>> Gratuitous reformatting?
>>
>>
>
> Elimination of an overly long line?
I usually try to gather any coding style, comment grammar, etc., fixes
into a single patch or two at the beginning of a series. That keeps the
substantive patches (the hardest to understand) shorter.
--b.
>
>>> @@ -1712,7 +1729,10 @@ int open_namei(int dfd, const char *path
>>> int acc_mode, error;
>>> struct path path;
>>> struct dentry *dir;
>>> - int count = 0;
>>> + int count;
>>> +
>>> +top:
>>> + count = 0;
>>> acc_mode = ACC_MODE(flag);
>>> @@ -1739,7 +1759,8 @@ int open_namei(int dfd, const char *path
>>> /*
>>> * Create - we need to know the parent.
>>> */
>>> - error = path_lookup_create(dfd,pathname,LOOKUP_PARENT,nd,flag,mode);
>>> + error = path_lookup_create(dfd, pathname, LOOKUP_PARENT, nd,
>>> + flag, mode);
>>> if (error)
>>> return error;
>>> @@ -1812,10 +1833,17 @@ ok:
>>> return 0;
>>> exit_dput:
>>> + if (error == -ESTALE)
>>> + d_drop(path.dentry);
>>> dput_path(&path, nd);
>>> exit:
>>> if (!IS_ERR(nd->intent.open.file))
>>> release_open_intent(nd);
>>> + if (error == -ESTALE) {
>>> + d_drop(nd->dentry);
>>> + path_release(nd);
>>> + goto top;
>>> + }
>>>
>>
>> I wonder if a tail-call might not work better here.
>
> "Tail-call"?
>
> Thanx...
>
> ps
> -
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
J. Bruce Fields wrote:
> On Fri, Jan 18, 2008 at 11:45:52AM -0500, Peter Staubach wrote:
>
>> Matthew Wilcox wrote:
>>
>>> On Fri, Jan 18, 2008 at 10:36:01AM -0500, Peter Staubach wrote:
>>>
>>>> static int path_lookup_create(int dfd, const char *name,
>>>> - unsigned int lookup_flags, struct nameidata *nd,
>>>> - int open_flags, int create_mode)
>>>> + unsigned int lookup_flags, struct nameidata *nd,
>>>> + int open_flags, int create_mode)
>>>>
>>>>
>>> Gratuitous reformatting?
>>>
>>>
>>>
>> Elimination of an overly long line?
>>
>
> I usually try to gather any coding style, comment grammar, etc., fixes
> into a single patch or two at the beginning of a series. That keeps the
> substantive patches (the hardest to understand) shorter.
>
>
That's probably great advice. I can easily enough undo the change
since it does not affect the functionality of the patch. It was
made while I was doing the analysis for the patch and to make the
style better match the style used in other surrounding routines.
Thanx...
ps
> --b.
>
>
>>>> @@ -1712,7 +1729,10 @@ int open_namei(int dfd, const char *path
>>>> int acc_mode, error;
>>>> struct path path;
>>>> struct dentry *dir;
>>>> - int count = 0;
>>>> + int count;
>>>> +
>>>> +top:
>>>> + count = 0;
>>>> acc_mode = ACC_MODE(flag);
>>>> @@ -1739,7 +1759,8 @@ int open_namei(int dfd, const char *path
>>>> /*
>>>> * Create - we need to know the parent.
>>>> */
>>>> - error = path_lookup_create(dfd,pathname,LOOKUP_PARENT,nd,flag,mode);
>>>> + error = path_lookup_create(dfd, pathname, LOOKUP_PARENT, nd,
>>>> + flag, mode);
>>>> if (error)
>>>> return error;
>>>> @@ -1812,10 +1833,17 @@ ok:
>>>> return 0;
>>>> exit_dput:
>>>> + if (error == -ESTALE)
>>>> + d_drop(path.dentry);
>>>> dput_path(&path, nd);
>>>> exit:
>>>> if (!IS_ERR(nd->intent.open.file))
>>>> release_open_intent(nd);
>>>> + if (error == -ESTALE) {
>>>> + d_drop(nd->dentry);
>>>> + path_release(nd);
>>>> + goto top;
>>>> + }
>>>>
>>>>
>>> I wonder if a tail-call might not work better here.
>>>
>> "Tail-call"?
>>
>> Thanx...
>>
>> ps
>> -
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>