* Re: [PATCH v9 1/9] nvme: fail cancelled commands with NVME_SC_HOST_PATH_ERROR [not found] ` <20190829220645.5480-2-sagi@grimberg.me> @ 2019-08-30 18:39 ` Chaitanya Kulkarni 0 siblings, 0 replies; 8+ messages in thread From: Chaitanya Kulkarni @ 2019-08-30 18:39 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme Cc: Keith Busch, James Smart, Hannes Reinecke, Christoph Hellwig Looks good. Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> On 08/29/2019 03:07 PM, Sagi Grimberg wrote: > NVME_SC_ABORT_REQ means that the request was aborted due to > an abort command received. In our case, this is a transport > cancellation, so host pathing error is much more appropriate. > > Also, convert NVME_SC_HOST_PATH_ERROR to BLK_STS_TRANSPORT for > such that callers can understand that the status is a transport > related error. This will be used by the ns scanning code to > understand if it got an error from the controller or that the > controller happens to be unreachable by the transport. > > Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com> > Reviewed-by: Hannes Reinecke <hare@suse.com> > Reviewed-by: James Smart <james.smart@broadcom.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > drivers/nvme/host/core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 0eb5c1bb2f48..11ef174e8399 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -226,6 +226,8 @@ static blk_status_t nvme_error_status(struct request *req) > return BLK_STS_PROTECTION; > case NVME_SC_RESERVATION_CONFLICT: > return BLK_STS_NEXUS; > + case NVME_SC_HOST_PATH_ERROR: > + return BLK_STS_TRANSPORT; > default: > return BLK_STS_IOERR; > } > @@ -294,7 +296,7 @@ bool nvme_cancel_request(struct request *req, void *data, bool reserved) > if (blk_mq_request_completed(req)) > return true; > > - nvme_req(req)->status = NVME_SC_ABORT_REQ; > + nvme_req(req)->status = NVME_SC_HOST_PATH_ERROR; > blk_mq_complete_request(req); > return true; > } > _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20190829220645.5480-3-sagi@grimberg.me>]
* Re: [PATCH v9 2/9] nvme: make nvme_identify_ns propagate errors back [not found] ` <20190829220645.5480-3-sagi@grimberg.me> @ 2019-08-30 18:41 ` Chaitanya Kulkarni 0 siblings, 0 replies; 8+ messages in thread From: Chaitanya Kulkarni @ 2019-08-30 18:41 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme Cc: Keith Busch, James Smart, Hannes Reinecke, Christoph Hellwig Looks good. Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> On 08/29/2019 03:07 PM, Sagi Grimberg wrote: > right now callers of nvme_identify_ns only know that it failed, > but don't know why. Make nvme_identify_ns propagate the error back. > Because nvme_submit_sync_cmd may return a positive status code, we > make nvme_identify_ns receive the id by reference and return that > status up the call chain. > > Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com> > Reviewed-by: Hannes Reinecke <hare@suse.com> > Reviewed-by: James Smart <james.smart@broadcom.com> > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > drivers/nvme/host/core.c | 30 +++++++++++++----------------- > 1 file changed, 13 insertions(+), 17 deletions(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20190829220645.5480-4-sagi@grimberg.me>]
* Re: [PATCH v9 3/9] nvme: make nvme_report_ns_ids propagate error back [not found] ` <20190829220645.5480-4-sagi@grimberg.me> @ 2019-08-30 18:45 ` Chaitanya Kulkarni 0 siblings, 0 replies; 8+ messages in thread From: Chaitanya Kulkarni @ 2019-08-30 18:45 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme Cc: Keith Busch, James Smart, Hannes Reinecke, Christoph Hellwig Starting commit log with "And" seems little bit odd. Otherwise, looks good. Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> On 08/29/2019 03:07 PM, Sagi Grimberg wrote: > And make the callers check the return status and propagate > back accordingly. Also print the return status. > > Reviewed-by: Hannes Reinecke<hare@suse.com> > Reviewed-by: James Smart<james.smart@broadcom.com> > Reviewed-by: Christoph Hellwig<hch@lst.de> > Signed-off-by: Sagi Grimberg<sagi@grimberg.me> > --- > drivers/nvme/host/core.c | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20190829220645.5480-7-sagi@grimberg.me>]
* Re: [PATCH v9 6/9] block_dev: split disk size check out of revalidate_disk [not found] ` <20190829220645.5480-7-sagi@grimberg.me> @ 2019-08-30 18:55 ` Chaitanya Kulkarni 2019-09-09 6:59 ` Hannes Reinecke 1 sibling, 0 replies; 8+ messages in thread From: Chaitanya Kulkarni @ 2019-08-30 18:55 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme Cc: Keith Busch, James Smart, Hannes Reinecke, Christoph Hellwig On 08/29/2019 03:07 PM, Sagi Grimberg wrote: > Allow callers to call separately the ->revalidate_disk() > callout and the disk size change check separately. This > will help the caller to take action based on the return > status of ->revalidate_disk() and run the disk size check > afterwards. Commit message looks odd as compare to other commits, please have a look at following message which uses limit of 72 car per line :- Allow callers to call separately the ->revalidate_disk() callout and the disk size change check separately. This will help the caller to take action based on the return status of ->revalidate_disk() and run the disk size check afterwards. > > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > fs/block_dev.c | 29 +++++++++++++++++------------ > include/linux/fs.h | 1 + > 2 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/fs/block_dev.c b/fs/block_dev.c > index a6f7c892cb4a..4939a093e2f8 100644 > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -1482,6 +1482,21 @@ void check_disk_size_change(struct gendisk *disk, struct block_device *bdev, > } > } > > +void check_disk_size(struct gendisk *disk, bool verbose) > +{ > + struct block_device *bdev = bdget_disk(disk, 0); > + > + if (!bdev) > + return; > + > + mutex_lock(&bdev->bd_mutex); > + check_disk_size_change(disk, bdev, verbose); > + bdev->bd_invalidated = 0; > + mutex_unlock(&bdev->bd_mutex); > + bdput(bdev); > +} > +EXPORT_SYMBOL_GPL(check_disk_size); > + > /** > * revalidate_disk - wrapper for lower-level driver's revalidate_disk call-back > * @disk: struct gendisk to be revalidated > @@ -1501,18 +1516,8 @@ int revalidate_disk(struct gendisk *disk) > * Hidden disks don't have associated bdev so there's no point in > * revalidating it. > */ > - if (!(disk->flags & GENHD_FL_HIDDEN)) { > - struct block_device *bdev = bdget_disk(disk, 0); > - > - if (!bdev) > - return ret; > - > - mutex_lock(&bdev->bd_mutex); > - check_disk_size_change(disk, bdev, ret == 0); > - bdev->bd_invalidated = 0; > - mutex_unlock(&bdev->bd_mutex); > - bdput(bdev); > - } > + if (!(disk->flags & GENHD_FL_HIDDEN)) > + check_disk_size(disk, ret == 0); > return ret; > } > EXPORT_SYMBOL(revalidate_disk); > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 997a530ff4e9..6649ad157317 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2679,6 +2679,7 @@ extern bool is_bad_inode(struct inode *); > extern void check_disk_size_change(struct gendisk *disk, > struct block_device *bdev, bool verbose); > extern int revalidate_disk(struct gendisk *); > +extern void check_disk_size(struct gendisk *disk, bool verbose); > extern int check_disk_change(struct block_device *); > extern int __invalidate_device(struct block_device *, bool); > extern int invalidate_partition(struct gendisk *, int); > _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v9 6/9] block_dev: split disk size check out of revalidate_disk [not found] ` <20190829220645.5480-7-sagi@grimberg.me> 2019-08-30 18:55 ` [PATCH v9 6/9] block_dev: split disk size check out of revalidate_disk Chaitanya Kulkarni @ 2019-09-09 6:59 ` Hannes Reinecke 1 sibling, 0 replies; 8+ messages in thread From: Hannes Reinecke @ 2019-09-09 6:59 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme; +Cc: Keith Busch, Christoph Hellwig, James Smart On 8/30/19 12:06 AM, Sagi Grimberg wrote: > Allow callers to call separately the ->revalidate_disk() > callout and the disk size change check separately. This > will help the caller to take action based on the return > status of ->revalidate_disk() and run the disk size check > afterwards. > > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > fs/block_dev.c | 29 +++++++++++++++++------------ > include/linux/fs.h | 1 + > 2 files changed, 18 insertions(+), 12 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.com> Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 247165 (AG München), GF: Felix Imendörffer _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20190829220645.5480-8-sagi@grimberg.me>]
* Re: [PATCH v9 7/9] nvme: pass status to nvme_error_status [not found] ` <20190829220645.5480-8-sagi@grimberg.me> @ 2019-08-30 19:02 ` Chaitanya Kulkarni 2019-08-30 19:08 ` Sagi Grimberg 2019-09-09 6:59 ` Hannes Reinecke 1 sibling, 1 reply; 8+ messages in thread From: Chaitanya Kulkarni @ 2019-08-30 19:02 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme Cc: Keith Busch, James Smart, Hannes Reinecke, Christoph Hellwig Do you find any issue with passing request structure ? or it is more of a normal cleanup ? On 08/29/2019 03:08 PM, Sagi Grimberg wrote: > No need for the full blown request structure. > > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > drivers/nvme/host/core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 0916c0e8d572..8f0881dec907 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -197,9 +197,9 @@ static inline bool nvme_ns_has_pi(struct nvme_ns *ns) > return ns->pi_type && ns->ms == sizeof(struct t10_pi_tuple); > } > > -static blk_status_t nvme_error_status(struct request *req) > +static blk_status_t nvme_error_status(u16 status) > { > - switch (nvme_req(req)->status & 0x7ff) { > + switch (status & 0x7ff) { > case NVME_SC_SUCCESS: > return BLK_STS_OK; > case NVME_SC_CAP_EXCEEDED: > @@ -262,7 +262,7 @@ static void nvme_retry_req(struct request *req) > > void nvme_complete_rq(struct request *req) > { > - blk_status_t status = nvme_error_status(req); > + blk_status_t status = nvme_error_status(nvme_req(req)->status); > > trace_nvme_complete_rq(req); > > _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v9 7/9] nvme: pass status to nvme_error_status 2019-08-30 19:02 ` [PATCH v9 7/9] nvme: pass status to nvme_error_status Chaitanya Kulkarni @ 2019-08-30 19:08 ` Sagi Grimberg 0 siblings, 0 replies; 8+ messages in thread From: Sagi Grimberg @ 2019-08-30 19:08 UTC (permalink / raw) To: Chaitanya Kulkarni, linux-nvme Cc: Keith Busch, James Smart, Hannes Reinecke, Christoph Hellwig > Do you find any issue with passing request structure ? or it is > more of a normal cleanup ? Its used later on... _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v9 7/9] nvme: pass status to nvme_error_status [not found] ` <20190829220645.5480-8-sagi@grimberg.me> 2019-08-30 19:02 ` [PATCH v9 7/9] nvme: pass status to nvme_error_status Chaitanya Kulkarni @ 2019-09-09 6:59 ` Hannes Reinecke 1 sibling, 0 replies; 8+ messages in thread From: Hannes Reinecke @ 2019-09-09 6:59 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme; +Cc: Keith Busch, Christoph Hellwig, James Smart On 8/30/19 12:06 AM, Sagi Grimberg wrote: > No need for the full blown request structure. > > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > drivers/nvme/host/core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 0916c0e8d572..8f0881dec907 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -197,9 +197,9 @@ static inline bool nvme_ns_has_pi(struct nvme_ns *ns) > return ns->pi_type && ns->ms == sizeof(struct t10_pi_tuple); > } > > -static blk_status_t nvme_error_status(struct request *req) > +static blk_status_t nvme_error_status(u16 status) > { > - switch (nvme_req(req)->status & 0x7ff) { > + switch (status & 0x7ff) { > case NVME_SC_SUCCESS: > return BLK_STS_OK; > case NVME_SC_CAP_EXCEEDED: > @@ -262,7 +262,7 @@ static void nvme_retry_req(struct request *req) > > void nvme_complete_rq(struct request *req) > { > - blk_status_t status = nvme_error_status(req); > + blk_status_t status = nvme_error_status(nvme_req(req)->status); > > trace_nvme_complete_rq(req); > > Reviewed-by: Hannes Reinecke <hare@suse.com> Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 247165 (AG München), GF: Felix Imendörffer _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-09-09 7:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20190829220645.5480-1-sagi@grimberg.me> [not found] ` <20190829220645.5480-2-sagi@grimberg.me> 2019-08-30 18:39 ` [PATCH v9 1/9] nvme: fail cancelled commands with NVME_SC_HOST_PATH_ERROR Chaitanya Kulkarni [not found] ` <20190829220645.5480-3-sagi@grimberg.me> 2019-08-30 18:41 ` [PATCH v9 2/9] nvme: make nvme_identify_ns propagate errors back Chaitanya Kulkarni [not found] ` <20190829220645.5480-4-sagi@grimberg.me> 2019-08-30 18:45 ` [PATCH v9 3/9] nvme: make nvme_report_ns_ids propagate error back Chaitanya Kulkarni [not found] ` <20190829220645.5480-7-sagi@grimberg.me> 2019-08-30 18:55 ` [PATCH v9 6/9] block_dev: split disk size check out of revalidate_disk Chaitanya Kulkarni 2019-09-09 6:59 ` Hannes Reinecke [not found] ` <20190829220645.5480-8-sagi@grimberg.me> 2019-08-30 19:02 ` [PATCH v9 7/9] nvme: pass status to nvme_error_status Chaitanya Kulkarni 2019-08-30 19:08 ` Sagi Grimberg 2019-09-09 6:59 ` Hannes Reinecke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).