public inbox for v9fs@lists.linux.dev
 help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: Eric Van Hensbergen <ericvh@kernel.org>,
	Dan Carpenter <dan.carpenter@linaro.org>,
	Latchesar Ionkov <lucho@ionkov.net>,
	v9fs@lists.linux.dev, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] fs/9p: delete unnnecessary condition
Date: Fri, 24 Oct 2025 22:34:21 +0900	[thread overview]
Message-ID: <aPuAXaHNWMhZOeuv@codewreck.org> (raw)
In-Reply-To: <13655400.bYd4YqkHfl@silver>

Christian Schoenebeck wrote on Fri, Oct 24, 2025 at 01:59:46PM +0200:
> On Friday, October 24, 2025 1:26:00 PM CEST Dan Carpenter wrote:
> > We already know that "retval" is negative, so there is no need to check
> > again.  Also the statement is not indented far enough.  Delete it.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> 
> Fixes: 43c36a5
> Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> 
> Apparently a manual revert copy paste error. The rest of the revert commit
> LGTM.

Ah, I left that part of the reverted hunk while keeping the HEAD part:
```
<<<<<<< HEAD
                }
                if (v9inode->cache_validity & V9FS_INO_INVALID_ATTR) {
                        p9_debug(P9_DEBUG_VFS, "dentry: %pd (%p) invalidated due to type change\n",
                                 dentry, dentry);
                        return 0;
                }
                if (retval < 0) {
                        p9_debug(P9_DEBUG_VFS,
                                "refresh inode: dentry = %pd (%p), got error %pe\n",
                                dentry, dentry, ERR_PTR(retval));
||||||| 290434474c33 (fs/9p: Refresh metadata in d_revalidate for uncached mode too)
                if (!cached && v9inode->cache_validity & V9FS_INO_INVALID_ATTR)
                        return 0;
                if (retval < 0)
=======
                if (retval < 0)
>>>>>>> parent of 290434474c33 (fs/9p: Refresh metadata in d_revalidate for uncached mode too)
                        return retval;
```

For a proper revert I should have removed the first `if
(v9inode->cache_validity & V9FS_INO_INVALID_ATTR)` too :/

OTOH it still makes sense even without the rest of the patch (the only
reason V9FS_INO_INVALID_ATTR would still be set is on type change, and
in that case we do want to return 0 even on refresh inode error)


Anyway, thanksfully this particular double retval < 0 check is harmless
-- I'll pick this up for -next for now but hopefully we'll be able to
"revert the revert" and fix the other problems Tingmao pointed out by
the time we reach the 6.19 merge window...

Thank you both!
-- 
Dominique Martinet | Asmadeus

  reply	other threads:[~2025-10-24 13:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-24 11:26 [PATCH] fs/9p: delete unnnecessary condition Dan Carpenter
2025-10-24 11:59 ` Christian Schoenebeck
2025-10-24 13:34   ` Dominique Martinet [this message]
2025-10-25 15:39     ` Tingmao Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPuAXaHNWMhZOeuv@codewreck.org \
    --to=asmadeus@codewreck.org \
    --cc=dan.carpenter@linaro.org \
    --cc=ericvh@kernel.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=v9fs@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox