From: guzebing <guzebing1612@gmail.com>
To: Keith Busch <kbusch@kernel.org>
Cc: axboe@kernel.dk, hch@lst.de, sagi@grimberg.me,
linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
Guzebing <guzebing@bytedance.com>
Subject: Re: [PATCH v4] nvme: reduce firmware activation poll interval
Date: Wed, 12 Aug 2026 11:23:47 +0800 [thread overview]
Message-ID: <52fad18e-9938-4cba-8145-aa88acb7e4b1@gmail.com> (raw)
In-Reply-To: <antR2u1eeBDCr0ux@kbusch-mbp>
On 8/12/26 12:46 AM, Keith Busch wrote:
> On Tue, Jul 28, 2026 at 08:17:35PM +0800, guzebing wrote:
>> +static int nvme_wait_ready(struct nvme_ctrl *ctrl, u32 mask, u32 val,
>> + u32 timeout, const char *op)
>> +{
>> + unsigned long timeout_jiffies = jiffies + timeout * HZ;
>> + u32 csts;
>> + int ret;
>> +
>> + ret = nvme_wait_csts(ctrl, mask, val, timeout_jiffies, false, &csts);
>> + if (ret != -ETIMEDOUT)
>> + return ret;
>> +
>> + dev_err(ctrl->device,
>> + "Device not ready; aborting %s, CSTS=0x%x\n", op, csts);
>> + return -ENODEV;
>
> Actually, I'm to back this out for a moment to think about this. I like
> that you've changed the error to ETIMEDOUT for that condition, but I'm
> not sure we need to convert it to ENODEV. None of the callers seem to
> care about the specific error, and the user visible side effect of the
> condition is more informative than ENODEV.
>
> So if we can get rid of that legacy behavior, then this simplifies even
> more. Thoughts?
When restructuring the polling loop, I tried to preserve the existing
behavior and return values as much as possible. Converting -ETIMEDOUT
to -ENODEV was part of that effort.
However, I agree that -ETIMEDOUT more accurately describes the actual
failure. I checked the callers and confirmed that they do not
distinguish between -ENODEV and -ETIMEDOUT, so propagating -ETIMEDOUT
directly makes sense to me.
Please feel free to fold this change in, or let me know if you would
prefer a v5.
Thanks,
Guzebing
prev parent reply other threads:[~2026-08-12 3:24 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-28 12:17 [PATCH v4] nvme: reduce firmware activation poll interval guzebing
2026-08-11 16:08 ` Keith Busch
2026-08-11 16:46 ` Keith Busch
2026-08-12 3:23 ` guzebing [this message]
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=52fad18e-9938-4cba-8145-aa88acb7e4b1@gmail.com \
--to=guzebing1612@gmail.com \
--cc=axboe@kernel.dk \
--cc=guzebing@bytedance.com \
--cc=hch@lst.de \
--cc=kbusch@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=sagi@grimberg.me \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.