[PATCHv3] nvme: quiet user passthrough command errors

[PATCHv3] nvme: quiet user passthrough command errors

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

The driver is spamming the kernel logs for entirely harmless errors from
user space submitting unsupported commands. Just silence the errors.
The application has direct access to command status, so there's no need
to log these.

And since every passthrough command now uses the quiet flag, move the
setting to the common initializer.

Cc: Alan Adamson <alan.adamson@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
v2->v3:

  Removed the last open-coded RQF_QUIET flag users since the setting is
  redundant with this generic change.

 drivers/nvme/host/core.c | 4 +---
 drivers/nvme/host/pci.c  | 2 --
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 686c55cb5d1a..543782dcfba9 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -657,7 +657,7 @@ static inline void nvme_clear_nvme_request(struct request *req)
 	nvme_req(req)->status = 0;
 	nvme_req(req)->retries = 0;
 	nvme_req(req)->flags = 0;
-	req->rq_flags |= RQF_DONTPREP;
+	req->rq_flags |= RQF_DONTPREP | RQF_QUIET;
 }
 
 /* initialize a passthrough request */
@@ -1037,7 +1037,6 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 			goto out;
 	}
 
-	req->rq_flags |= RQF_QUIET;
 	ret = nvme_execute_rq(req, at_head);
 	if (result && ret >= 0)
 		*result = nvme_req(req)->result;
@@ -1226,7 +1225,6 @@ static void nvme_keep_alive_work(struct work_struct *work)
 	rq->timeout = ctrl->kato * HZ;
 	rq->end_io = nvme_keep_alive_end_io;
 	rq->end_io_data = ctrl;
-	rq->rq_flags |= RQF_QUIET;
 	blk_execute_rq_nowait(rq, false);
 }
 
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 72b5c1addbff..8cd92283cbfc 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1436,7 +1436,6 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req)
 
 	abort_req->end_io = abort_endio;
 	abort_req->end_io_data = NULL;
-	abort_req->rq_flags |= RQF_QUIET;
 	blk_execute_rq_nowait(abort_req, false);
 
 	/*
@@ -2485,7 +2484,6 @@ static int nvme_delete_queue(struct nvme_queue *nvmeq, u8 opcode)
 	req->end_io_data = nvmeq;
 
 	init_completion(&nvmeq->delete_done);
-	req->rq_flags |= RQF_QUIET;
 	blk_execute_rq_nowait(req, false);
 	return 0;
 }
-- 
2.30.2

Re: [PATCHv3] nvme: quiet user passthrough command errors

[Chaitanya Kulkarni]

> 
> On Oct 28, 2022, at 7:44 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 commands. Just silence the errors.
> The application has direct access to command status, so there's no need
> to log these.
> 
> And since every passthrough command now uses the quiet flag, move the
> setting to the common initializer.
> 
> Cc: Alan Adamson <alan.adamson@oracle.com>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> 

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCHv3] nvme: quiet user passthrough command errors

[Jens Axboe]

On 10/28/22 8:44 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 commands. Just silence the errors.
> The application has direct access to command status, so there's no need
> to log these.
> 
> And since every passthrough command now uses the quiet flag, move the
> setting to the common initializer.

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe

Re: [PATCHv3] nvme: quiet user passthrough command errors

[Alan Adamson]

> On Oct 28, 2022, at 7:44 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 commands. Just silence the errors.
> The application has direct access to command status, so there's no need
> to log these.
> 
> And since every passthrough command now uses the quiet flag, move the
> setting to the common initializer.
> 
> Cc: Alan Adamson <alan.adamson@oracle.com>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> v2->v3:
> 
>  Removed the last open-coded RQF_QUIET flag users since the setting is
>  redundant with this generic change.
> 
> drivers/nvme/host/core.c | 4 +---
> drivers/nvme/host/pci.c  | 2 --
> 2 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 686c55cb5d1a..543782dcfba9 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -657,7 +657,7 @@ static inline void nvme_clear_nvme_request(struct request *req)
> 	nvme_req(req)->status = 0;
> 	nvme_req(req)->retries = 0;
> 	nvme_req(req)->flags = 0;
> -	req->rq_flags |= RQF_DONTPREP;
> +	req->rq_flags |= RQF_DONTPREP | RQF_QUIET;
> }
> 
> /* initialize a passthrough request */
> @@ -1037,7 +1037,6 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
> 			goto out;
> 	}
> 
> -	req->rq_flags |= RQF_QUIET;
> 	ret = nvme_execute_rq(req, at_head);
> 	if (result && ret >= 0)
> 		*result = nvme_req(req)->result;
> @@ -1226,7 +1225,6 @@ static void nvme_keep_alive_work(struct work_struct *work)
> 	rq->timeout = ctrl->kato * HZ;
> 	rq->end_io = nvme_keep_alive_end_io;
> 	rq->end_io_data = ctrl;
> -	rq->rq_flags |= RQF_QUIET;
> 	blk_execute_rq_nowait(rq, false);
> }
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 72b5c1addbff..8cd92283cbfc 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1436,7 +1436,6 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req)
> 
> 	abort_req->end_io = abort_endio;
> 	abort_req->end_io_data = NULL;
> -	abort_req->rq_flags |= RQF_QUIET;
> 	blk_execute_rq_nowait(abort_req, false);
> 
> 	/*
> @@ -2485,7 +2484,6 @@ static int nvme_delete_queue(struct nvme_queue *nvmeq, u8 opcode)
> 	req->end_io_data = nvmeq;
> 
> 	init_completion(&nvmeq->delete_done);
> -	req->rq_flags |= RQF_QUIET;
> 	blk_execute_rq_nowait(req, false);
> 	return 0;
> }
> -- 
> 2.30.2
> 

Since all requests (not just user space initiated requests) call nvme_clear_nvme_requests(), all requests will set RQF_QUIET, so no error will get logged.

Alan

Re: [PATCHv3] nvme: quiet user passthrough command errors

[Keith Busch]

On Fri, Oct 28, 2022 at 07:08:51PM +0000, Alan Adamson wrote:
> 
> Since all requests (not just user space initiated requests) call
> nvme_clear_nvme_requests(), all requests will set RQF_QUIET, so no
> error will get logged.

Ugh, yes! I even said this needs to go in the nvme_init_request() and
then proceeded to botch that in the wrong function. Thanks, I'll send
another version.