On Fri, Aug 25 2017, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
>
> Pull the checks for delegated_inode into break_deleg_wait() to simplify
> the callers a little.
>
> No change in behavior.
>
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> ---
> fs/namei.c | 26 ++++++++++----------------
> fs/open.c | 16 ++++++----------
> fs/utimes.c | 8 +++-----
> include/linux/fs.h | 12 +++++++-----
> 4 files changed, 26 insertions(+), 36 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index ddb6a7c2b3d4..5a93be7b2c9c 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4048,11 +4048,9 @@ static long do_unlinkat(int dfd, const char __user *pathname)
> if (inode)
> iput(inode); /* truncate the inode here */
> inode = NULL;
> - if (delegated_inode) {
> - error = break_deleg_wait(&delegated_inode);
> - if (!error)
> - goto retry_deleg;
> - }
> + error = break_deleg_wait(&delegated_inode, error);
> + if (error == DELEG_RETRY)
> + goto retry_deleg;
<mode=bikeshed>
I don't like the "DELEG_RETRY". You are comparing it against an
'error', but it doesn't start with '-E', so I get confused (happens
often).
If this read:
if (error > 0)
goto retry_deleg;
it would be must more obvious to me what was happening. Clearly the
return value isn't an error, and it isn't "success" either. This is a
pattern I've seen elsewhere.
Alternately you could use "-EAGAIN", but I suspect there is a risk of
unwanted side-effects if you re-use and existing code.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
