* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-26 17:07 ` [PATCH] nvme: quiet user passthrough command errors Keith Busch
@ 2022-10-26 18:04 ` Chaitanya Kulkarni
2022-10-26 18:08 ` Alan Adamson
2022-10-26 18:19 ` Keith Busch
2022-10-26 21:27 ` Alan Adamson
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-26 18:04 UTC (permalink / raw)
To: Keith Busch, linux-nvme@lists.infradead.org
Cc: hch@lst.de, sagi@grimberg.me, Keith Busch, Daniel Wagner,
Alan Adamson
On 10/26/2022 10:07 AM, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> The driver is spamming the kernel logs for entirely harmless errors from
> user space submitting unsupported commmands. Just silence the errors.
> The application has direct access to command status, so there's no need
> to log these.
>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Alan Adamson <alan.adamson@oracle.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
This should do the trick, however it will be great if reporter
can provide tested-by.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-26 18:04 ` Chaitanya Kulkarni
@ 2022-10-26 18:08 ` Alan Adamson
2022-10-26 18:19 ` Keith Busch
1 sibling, 0 replies; 10+ messages in thread
From: Alan Adamson @ 2022-10-26 18:08 UTC (permalink / raw)
To: Chaitanya Kulkarni
Cc: Keith Busch, linux-nvme@lists.infradead.org, hch@lst.de,
sagi@grimberg.me, Keith Busch, Daniel Wagner
> On Oct 26, 2022, at 11:04 AM, Chaitanya Kulkarni <chaitanyak@nvidia.com> wrote:
>
> On 10/26/2022 10:07 AM, Keith Busch wrote:
>> From: Keith Busch <kbusch@kernel.org>
>>
>> The driver is spamming the kernel logs for entirely harmless errors from
>> user space submitting unsupported commmands. Just silence the errors.
>> The application has direct access to command status, so there's no need
>> to log these.
>>
>> Cc: Daniel Wagner <dwagner@suse.de>
>> Cc: Alan Adamson <alan.adamson@oracle.com>
>> Signed-off-by: Keith Busch <kbusch@kernel.org>
>> ---
>
>
> This should do the trick, however it will be great if reporter
> can provide tested-by.
>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
>
> -ck
>
>
I’ll be testing it today.
Alan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-26 18:04 ` Chaitanya Kulkarni
2022-10-26 18:08 ` Alan Adamson
@ 2022-10-26 18:19 ` Keith Busch
1 sibling, 0 replies; 10+ messages in thread
From: Keith Busch @ 2022-10-26 18:19 UTC (permalink / raw)
To: Chaitanya Kulkarni
Cc: Keith Busch, linux-nvme@lists.infradead.org, hch@lst.de,
sagi@grimberg.me, Daniel Wagner, Alan Adamson
On Wed, Oct 26, 2022 at 06:04:32PM +0000, Chaitanya Kulkarni wrote:
> On 10/26/2022 10:07 AM, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> >
> > The driver is spamming the kernel logs for entirely harmless errors from
> > user space submitting unsupported commmands. Just silence the errors.
> > The application has direct access to command status, so there's no need
> > to log these.
> >
> > Cc: Daniel Wagner <dwagner@suse.de>
> > Cc: Alan Adamson <alan.adamson@oracle.com>
> > Signed-off-by: Keith Busch <kbusch@kernel.org>
> > ---
>
>
> This should do the trick, however it will be great if reporter
> can provide tested-by.
Pretty easy to test. Just send an invalid command from user space:
# nvme admin-passthru /dev/nvme0 --opcode=0xff
nvme0: Admin Cmd(0xff), I/O Error (sct 0x0 / sc 0x1) DNR
With the patch applied, the kernel message is suppressed.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-26 17:07 ` [PATCH] nvme: quiet user passthrough command errors Keith Busch
2022-10-26 18:04 ` Chaitanya Kulkarni
@ 2022-10-26 21:27 ` Alan Adamson
2022-10-27 5:41 ` Kanchan Joshi
2022-10-27 13:14 ` Pankaj Raghav
3 siblings, 0 replies; 10+ messages in thread
From: Alan Adamson @ 2022-10-26 21:27 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme@lists.infradead.org, hch@lst.de, sagi@grimberg.me,
Keith Busch, Daniel Wagner
> On Oct 26, 2022, at 10:07 AM, Keith Busch <kbusch@meta.com> wrote:
>
> From: Keith Busch <kbusch@kernel.org>
>
> The driver is spamming the kernel logs for entirely harmless errors from
> user space submitting unsupported commmands. Just silence the errors.
> The application has direct access to command status, so there's no need
> to log these.
>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Alan Adamson <alan.adamson@oracle.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> drivers/nvme/host/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 686c55cb5d1a..da874172a31d 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1156,6 +1156,7 @@ int nvme_execute_passthru_rq(struct request *rq, u32 *effects)
> struct nvme_ns *ns = rq->q->queuedata;
>
> *effects = nvme_passthru_start(ctrl, ns, cmd->common.opcode);
> + rq->rq_flags |= RQF_QUIET;
> return nvme_execute_rq(rq, false);
> }
> EXPORT_SYMBOL_NS_GPL(nvme_execute_passthru_rq, NVME_TARGET_PASSTHRU);
> --
> 2.30.2
>
Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
I tested the change with blktests which uses user passthrough:
Before patch:
[ 69.041515] nvme0n1: Read(0x2) @ LBA 0, 1 blocks, Unrecovered Read Error (sct 0x2 / sc 0x81) DNR
[ 69.041557] critical medium error, dev nvme0n1, sector 0 op 0x0:(READ) flags 0x800 phys_seg 1 prio class 2
[ 69.055721] nvme0n1: Read(0x2) @ LBA 0, 1 blocks, Unknown (sct 0x3 / sc 0x75) DNR
[ 69.055754] I/O error, dev nvme0n1, sector 0 op 0x0:(READ) flags 0x800 phys_seg 1 prio class 2
[ 69.067335] nvme0n1: Write(0x1) @ LBA 0, 1 blocks, Write Fault (sct 0x2 / sc 0x80) DNR
[ 69.067385] critical medium error, dev nvme0n1, sector 0 op 0x1:(WRITE) flags 0x8800 phys_seg 1 prio class 2
[ 69.085449] nvme0: Identify(0x6), Access Denied (sct 0x2 / sc 0x86) DNR
[ 69.103265] nvme0: Unknown(0x96), Invalid Command Opcode (sct 0x0 / sc 0x1) DNR
After patch:
[ 167.375311] nvme0n1: Read(0x2) @ LBA 0, 1 blocks, Unrecovered Read Error (sct 0x2 / sc 0x81) DNR
[ 167.375362] critical medium error, dev nvme0n1, sector 0 op 0x0:(READ) flags 0x800 phys_seg 1 prio class 2
[ 167.388812] nvme0n1: Read(0x2) @ LBA 0, 1 blocks, Unknown (sct 0x3 / sc 0x75) DNR
[ 167.388839] I/O error, dev nvme0n1, sector 0 op 0x0:(READ) flags 0x800 phys_seg 1 prio class 2
[ 167.399907] nvme0n1: Write(0x1) @ LBA 0, 1 blocks, Write Fault (sct 0x2 / sc 0x80) DNR
[ 167.399928] critical medium error, dev nvme0n1, sector 0 op 0x1:(WRITE) flags 0x8800 phys_seg 1 prio class 2
The messages that are due to passthrough are no longer present as expcted.
Tested-by: Alan Adamson <alan.adamson@oracle.com>
blktests will need to updated, specifically blktests/tests/nvme/039.out.
Alan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-26 17:07 ` [PATCH] nvme: quiet user passthrough command errors Keith Busch
2022-10-26 18:04 ` Chaitanya Kulkarni
2022-10-26 21:27 ` Alan Adamson
@ 2022-10-27 5:41 ` Kanchan Joshi
2022-10-27 13:17 ` Jens Axboe
2022-10-27 13:14 ` Pankaj Raghav
3 siblings, 1 reply; 10+ messages in thread
From: Kanchan Joshi @ 2022-10-27 5:41 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, hch, sagi, Keith Busch, Daniel Wagner, Alan Adamson
[-- Attachment #1: Type: text/plain, Size: 1572 bytes --]
On Wed, Oct 26, 2022 at 10:07:15AM -0700, Keith Busch wrote:
>From: Keith Busch <kbusch@kernel.org>
>
>The driver is spamming the kernel logs for entirely harmless errors from
>user space submitting unsupported commmands. Just silence the errors.
>The application has direct access to command status, so there's no need
>to log these.
>
>Cc: Daniel Wagner <dwagner@suse.de>
>Cc: Alan Adamson <alan.adamson@oracle.com>
>Signed-off-by: Keith Busch <kbusch@kernel.org>
>---
> drivers/nvme/host/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>index 686c55cb5d1a..da874172a31d 100644
>--- a/drivers/nvme/host/core.c
>+++ b/drivers/nvme/host/core.c
>@@ -1156,6 +1156,7 @@ int nvme_execute_passthru_rq(struct request *rq, u32 *effects)
> struct nvme_ns *ns = rq->q->queuedata;
>
> *effects = nvme_passthru_start(ctrl, ns, cmd->common.opcode);
>+ rq->rq_flags |= RQF_QUIET;
> return nvme_execute_rq(rq, false);
> }
Can we do this for uring-passthrough path too? Like below patch -
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 81f5550b670d..a91cefc38506 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -556,6 +556,7 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
} else {
req->end_io = nvme_uring_cmd_end_io;
}
+ req->rq_flags |= RQF_QUIET;
blk_execute_rq_nowait(req, false);
return -EIOCBQUEUED;
}
Looks good otherwise.
Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-27 5:41 ` Kanchan Joshi
@ 2022-10-27 13:17 ` Jens Axboe
2022-10-27 15:00 ` Keith Busch
0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2022-10-27 13:17 UTC (permalink / raw)
To: Kanchan Joshi, Keith Busch
Cc: linux-nvme, hch, sagi, Keith Busch, Daniel Wagner, Alan Adamson
On 10/26/22 11:41 PM, Kanchan Joshi wrote:
> On Wed, Oct 26, 2022 at 10:07:15AM -0700, Keith Busch wrote:
>> From: Keith Busch <kbusch@kernel.org>
>>
>> The driver is spamming the kernel logs for entirely harmless errors from
>> user space submitting unsupported commmands. Just silence the errors.
>> The application has direct access to command status, so there's no need
>> to log these.
>>
>> Cc: Daniel Wagner <dwagner@suse.de>
>> Cc: Alan Adamson <alan.adamson@oracle.com>
>> Signed-off-by: Keith Busch <kbusch@kernel.org>
>> ---
>> drivers/nvme/host/core.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 686c55cb5d1a..da874172a31d 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -1156,6 +1156,7 @@ int nvme_execute_passthru_rq(struct request *rq, u32 *effects)
>> struct nvme_ns *ns = rq->q->queuedata;
>>
>> *effects = nvme_passthru_start(ctrl, ns, cmd->common.opcode);
>> + rq->rq_flags |= RQF_QUIET;
>> return nvme_execute_rq(rq, false);
>> }
>
> Can we do this for uring-passthrough path too? Like below patch -
>
> diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
> index 81f5550b670d..a91cefc38506 100644
> --- a/drivers/nvme/host/ioctl.c
> +++ b/drivers/nvme/host/ioctl.c
> @@ -556,6 +556,7 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> } else {
> req->end_io = nvme_uring_cmd_end_io;
> }
> + req->rq_flags |= RQF_QUIET;
> blk_execute_rq_nowait(req, false);
> return -EIOCBQUEUED;
> }
>
> Looks good otherwise.
> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
Agree, we should do it for this path too.
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-27 13:17 ` Jens Axboe
@ 2022-10-27 15:00 ` Keith Busch
2022-10-27 16:40 ` Chaitanya Kulkarni
0 siblings, 1 reply; 10+ messages in thread
From: Keith Busch @ 2022-10-27 15:00 UTC (permalink / raw)
To: Jens Axboe
Cc: Kanchan Joshi, Keith Busch, linux-nvme, hch, sagi, Daniel Wagner,
Alan Adamson
On Thu, Oct 27, 2022 at 07:17:48AM -0600, Jens Axboe wrote:
> On 10/26/22 11:41 PM, Kanchan Joshi wrote:
> > On Wed, Oct 26, 2022 at 10:07:15AM -0700, Keith Busch wrote:
> >> From: Keith Busch <kbusch@kernel.org>
> >>
> >> The driver is spamming the kernel logs for entirely harmless errors from
> >> user space submitting unsupported commmands. Just silence the errors.
> >> The application has direct access to command status, so there's no need
> >> to log these.
> >>
> >> Cc: Daniel Wagner <dwagner@suse.de>
> >> Cc: Alan Adamson <alan.adamson@oracle.com>
> >> Signed-off-by: Keith Busch <kbusch@kernel.org>
> >> ---
> >> drivers/nvme/host/core.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> >> index 686c55cb5d1a..da874172a31d 100644
> >> --- a/drivers/nvme/host/core.c
> >> +++ b/drivers/nvme/host/core.c
> >> @@ -1156,6 +1156,7 @@ int nvme_execute_passthru_rq(struct request *rq, u32 *effects)
> >> struct nvme_ns *ns = rq->q->queuedata;
> >>
> >> *effects = nvme_passthru_start(ctrl, ns, cmd->common.opcode);
> >> + rq->rq_flags |= RQF_QUIET;
> >> return nvme_execute_rq(rq, false);
> >> }
> >
> > Can we do this for uring-passthrough path too? Like below patch -
> >
> > diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
> > index 81f5550b670d..a91cefc38506 100644
> > --- a/drivers/nvme/host/ioctl.c
> > +++ b/drivers/nvme/host/ioctl.c
> > @@ -556,6 +556,7 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> > } else {
> > req->end_io = nvme_uring_cmd_end_io;
> > }
> > + req->rq_flags |= RQF_QUIET;
> > blk_execute_rq_nowait(req, false);
> > return -EIOCBQUEUED;
> > }
> >
> > Looks good otherwise.
> > Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
>
> Agree, we should do it for this path too.
Okay, that would make 4 places the driver is setting the QUIET flag,
which is all of the passthrough requests. Instead of doing them
individually, I'll just move the setting to their common helper:
nvme_init_command().
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-27 15:00 ` Keith Busch
@ 2022-10-27 16:40 ` Chaitanya Kulkarni
0 siblings, 0 replies; 10+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-27 16:40 UTC (permalink / raw)
To: Keith Busch
Cc: Jens Axboe, Kanchan Joshi, Keith Busch,
linux-nvme@lists.infradead.org, hch@lst.de, sagi@grimberg.me,
Daniel Wagner, Alan Adamson
>
> On Oct 27, 2022, at 8:02 AM, Keith Busch <kbusch@kernel.org> wrote:
>
> On Thu, Oct 27, 2022 at 07:17:48AM -0600, Jens Axboe wrote:
>>> On 10/26/22 11:41 PM, Kanchan Joshi wrote:
>>> On Wed, Oct 26, 2022 at 10:07:15AM -0700, Keith Busch wrote:
>>>> From: Keith Busch <kbusch@kernel.org>
>>>>
>>>> The driver is spamming the kernel logs for entirely harmless errors from
>>>> user space submitting unsupported commmands. Just silence the errors.
>>>> The application has direct access to command status, so there's no need
>>>> to log these.
>>>>
>>>> Cc: Daniel Wagner <dwagner@suse.de>
>>>> Cc: Alan Adamson <alan.adamson@oracle.com>
>>>> Signed-off-by: Keith Busch <kbusch@kernel.org>
>>>> ---
>>>> drivers/nvme/host/core.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>>> index 686c55cb5d1a..da874172a31d 100644
>>>> --- a/drivers/nvme/host/core.c
>>>> +++ b/drivers/nvme/host/core.c
>>>> @@ -1156,6 +1156,7 @@ int nvme_execute_passthru_rq(struct request *rq, u32 *effects)
>>>> struct nvme_ns *ns = rq->q->queuedata;
>>>>
>>>> *effects = nvme_passthru_start(ctrl, ns, cmd->common.opcode);
>>>> + rq->rq_flags |= RQF_QUIET;
>>>> return nvme_execute_rq(rq, false);
>>>> }
>>>
>>> Can we do this for uring-passthrough path too? Like below patch -
>>>
>>> diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
>>> index 81f5550b670d..a91cefc38506 100644
>>> --- a/drivers/nvme/host/ioctl.c
>>> +++ b/drivers/nvme/host/ioctl.c
>>> @@ -556,6 +556,7 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
>>> } else {
>>> req->end_io = nvme_uring_cmd_end_io;
>>> }
>>> + req->rq_flags |= RQF_QUIET;
>>> blk_execute_rq_nowait(req, false);
>>> return -EIOCBQUEUED;
>>> }
>>>
>>> Looks good otherwise.
>>> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
>>
>> Agree, we should do it for this path too.
>
> Okay, that would make 4 places the driver is setting the QUIET flag,
> which is all of the passthrough requests. Instead of doing them
> individually, I'll just move the setting to their common helper:
> nvme_init_command().
>
Yes please.
-ck
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nvme: quiet user passthrough command errors
2022-10-26 17:07 ` [PATCH] nvme: quiet user passthrough command errors Keith Busch
` (2 preceding siblings ...)
2022-10-27 5:41 ` Kanchan Joshi
@ 2022-10-27 13:14 ` Pankaj Raghav
3 siblings, 0 replies; 10+ messages in thread
From: Pankaj Raghav @ 2022-10-27 13:14 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, hch, sagi, Keith Busch, Daniel Wagner, Alan Adamson,
Pankaj Raghav
On Wed, Oct 26, 2022 at 10:07:15AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> The driver is spamming the kernel logs for entirely harmless errors from
> user space submitting unsupported commmands. Just silence the errors.
> The application has direct access to command status, so there's no need
> to log these.
>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Alan Adamson <alan.adamson@oracle.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
>
Shouldn't we also add a Fixes tag here?
^ permalink raw reply [flat|nested] 10+ messages in thread