[v2] nvme: Print capabilities changes just once
Commit Message
This current dev_info() could be very verbose and being printed very
frequently depending on some userspace application sending some specific
commands.
Just print this message once and skip it until the controller resets.
Use a controller flag (NVME_CTRL_DIRTY_CAPABILITY) to track if the
capability needs a reset.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
drivers/nvme/host/core.c | 6 +++++-
drivers/nvme/host/nvme.h | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
Comments
On Thu, Jun 15, 2023 at 02:49:03AM -0700, Breno Leitao wrote:
> This current dev_info() could be very verbose and being printed very
> frequently depending on some userspace application sending some specific
> commands.
>
> Just print this message once and skip it until the controller resets.
> Use a controller flag (NVME_CTRL_DIRTY_CAPABILITY) to track if the
> capability needs a reset.
Looks good.
Reviewed-by: Keith Busch <kbusch@kernel.org>
On 6/15/2023 2:49 AM, Breno Leitao wrote:
> This current dev_info() could be very verbose and being printed very
> frequently depending on some userspace application sending some specific
> commands.
>
> Just print this message once and skip it until the controller resets.
> Use a controller flag (NVME_CTRL_DIRTY_CAPABILITY) to track if the
> capability needs a reset.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
On Thu, Jun 15, 2023 at 02:49:03AM -0700, Breno Leitao wrote:
> if (effects & NVME_CMD_EFFECTS_CCC) {
> - dev_info(ctrl->device,
> + if (!test_and_set_bit(NVME_CTRL_DIRTY_CAPABILITY,
> + &ctrl->flags)) {
> + dev_info(ctrl->device,
> "controller capabilities changed, reset may be required to take effect.\n");
> + }
> }
Thanks, patch applied to nvme-6.5.
A question for Christoph and future consideration: I *think* the
complications that had the driver stop refreshing the controller
attributes are largely mitigated by the fact we now handle effects
after the request is freed. Perhaps I'm missing something. Is there
a risk in just bringing that feature back into the driver?
On Wed, Jun 21, 2023 at 10:46:17AM -0600, Keith Busch wrote:
> On Thu, Jun 15, 2023 at 02:49:03AM -0700, Breno Leitao wrote:
> > if (effects & NVME_CMD_EFFECTS_CCC) {
> > - dev_info(ctrl->device,
> > + if (!test_and_set_bit(NVME_CTRL_DIRTY_CAPABILITY,
> > + &ctrl->flags)) {
> > + dev_info(ctrl->device,
> > "controller capabilities changed, reset may be required to take effect.\n");
> > + }
> > }
>
> Thanks, patch applied to nvme-6.5.
>
> A question for Christoph and future consideration: I *think* the
> complications that had the driver stop refreshing the controller
> attributes are largely mitigated by the fact we now handle effects
> after the request is freed. Perhaps I'm missing something. Is there
> a risk in just bringing that feature back into the driver?
No, the main issue was and is that we can't just call
nvme_init_ctrl_finish on a live controller.
@@ -1125,8 +1125,11 @@ void nvme_passthru_end(struct nvme_ctrl *ctrl, u32 effects,
mutex_unlock(&ctrl->scan_lock);
}
if (effects & NVME_CMD_EFFECTS_CCC) {
- dev_info(ctrl->device,
+ if (!test_and_set_bit(NVME_CTRL_DIRTY_CAPABILITY,
+ &ctrl->flags)) {
+ dev_info(ctrl->device,
"controller capabilities changed, reset may be required to take effect.\n");
+ }
}
if (effects & (NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC)) {
nvme_queue_scan(ctrl);
@@ -3280,6 +3283,7 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl, bool was_suspended)
return ret;
}
+ clear_bit(NVME_CTRL_DIRTY_CAPABILITY, &ctrl->flags);
ctrl->identified = true;
return 0;
@@ -243,6 +243,7 @@ enum nvme_ctrl_flags {
NVME_CTRL_STARTED_ONCE = 2,
NVME_CTRL_STOPPED = 3,
NVME_CTRL_SKIP_ID_CNS_CS = 4,
+ NVME_CTRL_DIRTY_CAPABILITY = 5,
};
struct nvme_ctrl {