* [PATCH 0/2] A few code cleanups @ 2019-11-25 16:06 Edmund Nadolski 2019-11-25 16:06 ` [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns Edmund Nadolski 2019-11-25 16:06 ` [PATCH 2/2] nvme: else following return is not needed Edmund Nadolski 0 siblings, 2 replies; 10+ messages in thread From: Edmund Nadolski @ 2019-11-25 16:06 UTC (permalink / raw) To: edmund.nadolski, linux-nvme, kbusch This patchset adds a few simple cleanups to the nvme code. Edmund Nadolski (2): nvme: remove unused return code from nvme_alloc_ns nvme: else following return is not needed drivers/nvme/host/core.c | 21 ++++++--------------- drivers/nvme/host/pci.c | 2 +- 2 files changed, 7 insertions(+), 16 deletions(-) -- 2.20.1 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns 2019-11-25 16:06 [PATCH 0/2] A few code cleanups Edmund Nadolski @ 2019-11-25 16:06 ` Edmund Nadolski 2019-11-25 16:09 ` Johannes Thumshirn 2019-11-25 16:06 ` [PATCH 2/2] nvme: else following return is not needed Edmund Nadolski 1 sibling, 1 reply; 10+ messages in thread From: Edmund Nadolski @ 2019-11-25 16:06 UTC (permalink / raw) To: edmund.nadolski, linux-nvme, kbusch The return code of nvme_alloc_ns is never used, so change it to void. Signed-off-by: Edmund Nadolski <edmund.nadolski@intel.com> --- drivers/nvme/host/core.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 9696404a6182..c22d986d053a 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -3446,7 +3446,7 @@ static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns) return 0; } -static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) +static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) { struct nvme_ns *ns; struct gendisk *disk; @@ -3456,13 +3456,11 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) ns = kzalloc_node(sizeof(*ns), GFP_KERNEL, node); if (!ns) - return -ENOMEM; + return; ns->queue = blk_mq_init_queue(ctrl->tagset); - if (IS_ERR(ns->queue)) { - ret = PTR_ERR(ns->queue); + if (IS_ERR(ns->queue)) goto out_free_ns; - } if (ctrl->opts && ctrl->opts->data_digest) ns->queue->backing_dev_info->capabilities @@ -3485,10 +3483,8 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) if (ret) goto out_free_queue; - if (id->ncap == 0) { - ret = -EINVAL; + if (id->ncap == 0) goto out_free_id; - } ret = nvme_init_ns_head(ns, nsid, id); if (ret) @@ -3497,10 +3493,8 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) nvme_set_disk_name(disk_name, ns, ctrl, &flags); disk = alloc_disk_node(0, node); - if (!disk) { - ret = -ENOMEM; + if (!disk) goto out_unlink_ns; - } disk->fops = &nvme_fops; disk->private_data = ns; @@ -3531,7 +3525,7 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name); kfree(id); - return 0; + return; out_put_disk: put_disk(ns->disk); out_unlink_ns: @@ -3545,9 +3539,6 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) blk_cleanup_queue(ns->queue); out_free_ns: kfree(ns); - if (ret > 0) - ret = blk_status_to_errno(nvme_error_status(ret)); - return ret; } static void nvme_ns_remove(struct nvme_ns *ns) -- 2.20.1 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns 2019-11-25 16:06 ` [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns Edmund Nadolski @ 2019-11-25 16:09 ` Johannes Thumshirn 2019-11-25 16:46 ` Nadolski, Edmund 0 siblings, 1 reply; 10+ messages in thread From: Johannes Thumshirn @ 2019-11-25 16:09 UTC (permalink / raw) To: Edmund Nadolski, linux-nvme, kbusch On 25/11/2019 17:06, Edmund Nadolski wrote: > The return code of nvme_alloc_ns is never used, so change it > to void. Oh no, please do it the other way round, check the return value of nvme_alloc_ns() and handle error properly. nvme_alloc_ns() is doing memory allocations and these can fail (although less likely because of the GFP_KERNEL allocations). -- Johannes Thumshirn SUSE Labs Filesystems jthumshirn@suse.de +49 911 74053 689 SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns 2019-11-25 16:09 ` Johannes Thumshirn @ 2019-11-25 16:46 ` Nadolski, Edmund 2019-11-25 17:06 ` Keith Busch 0 siblings, 1 reply; 10+ messages in thread From: Nadolski, Edmund @ 2019-11-25 16:46 UTC (permalink / raw) To: Johannes Thumshirn, Edmund Nadolski, linux-nvme, kbusch On 11/25/2019 9:09 AM, Johannes Thumshirn wrote: > On 25/11/2019 17:06, Edmund Nadolski wrote: >> The return code of nvme_alloc_ns is never used, so change it >> to void. > > Oh no, please do it the other way round, check the return value of > nvme_alloc_ns() and handle error properly. > > nvme_alloc_ns() is doing memory allocations and these can fail (although > less likely because of the GFP_KERNEL allocations). I considered that, tho it looked to me like the caller (nvme_validate_ns) and its callers (nvme_scan_ns_list, nvme_scan_ns_sequential) don't really care, and intend simply to continue on regardless of memory alloc failure(s). I can either add the -ENOMEM handling, or clarify the caller's intent w/some comments. Which is the way to go? Thanks, Ed _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns 2019-11-25 16:46 ` Nadolski, Edmund @ 2019-11-25 17:06 ` Keith Busch 2019-11-26 4:01 ` Chaitanya Kulkarni 0 siblings, 1 reply; 10+ messages in thread From: Keith Busch @ 2019-11-25 17:06 UTC (permalink / raw) To: Nadolski, Edmund; +Cc: linux-nvme, Johannes Thumshirn On Mon, Nov 25, 2019 at 09:46:08AM -0700, Nadolski, Edmund wrote: > On 11/25/2019 9:09 AM, Johannes Thumshirn wrote: > > On 25/11/2019 17:06, Edmund Nadolski wrote: > > > The return code of nvme_alloc_ns is never used, so change it > > > to void. > > > > Oh no, please do it the other way round, check the return value of > > nvme_alloc_ns() and handle error properly. > > > > nvme_alloc_ns() is doing memory allocations and these can fail (although > > less likely because of the GFP_KERNEL allocations). > > I considered that, tho it looked to me like the caller (nvme_validate_ns) > and its callers (nvme_scan_ns_list, nvme_scan_ns_sequential) don't really > care, and intend simply to continue on regardless of memory alloc > failure(s). > > I can either add the -ENOMEM handling, or clarify the caller's intent w/some > comments. Which is the way to go? It would be informative to just log the error that occurred so we have something indicating why an expected namespace wasn't created. Otherwise, I can't think of anything else the caller should do in response to an error in this path. _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns 2019-11-25 17:06 ` Keith Busch @ 2019-11-26 4:01 ` Chaitanya Kulkarni 2019-11-26 16:47 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: Chaitanya Kulkarni @ 2019-11-26 4:01 UTC (permalink / raw) To: Keith Busch, Nadolski, Edmund; +Cc: linux-nvme, Johannes Thumshirn On 11/25/2019 09:07 AM, Keith Busch wrote: > It would be informative to just log the error that occurred so we have > something indicating why an expected namespace wasn't created. Otherwise, > I can't think of anything else the caller should do in response to an > error in this path. Please do this, shouldn't be ignoring return value completely at first place. _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns 2019-11-26 4:01 ` Chaitanya Kulkarni @ 2019-11-26 16:47 ` Christoph Hellwig 0 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2019-11-26 16:47 UTC (permalink / raw) To: Chaitanya Kulkarni Cc: Keith Busch, Nadolski, Edmund, linux-nvme, Johannes Thumshirn On Tue, Nov 26, 2019 at 04:01:11AM +0000, Chaitanya Kulkarni wrote: > On 11/25/2019 09:07 AM, Keith Busch wrote: > > It would be informative to just log the error that occurred so we have > > something indicating why an expected namespace wasn't created. Otherwise, > > I can't think of anything else the caller should do in response to an > > error in this path. > > Please do this, shouldn't be ignoring return value completely at first > place. Well, what would you do instead? We call nvme_alloc_ns during the initial probe and every reset / recan. If we fail to allocate a controller for whatever reason we'd much rather still have a working controller with at least the admin queue and maybe other namespaces rather than shutting everything down. So just logging a message inside nvme_alloc_ns as suggested by Keith sounds like a good idea to me. _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] nvme: else following return is not needed 2019-11-25 16:06 [PATCH 0/2] A few code cleanups Edmund Nadolski 2019-11-25 16:06 ` [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns Edmund Nadolski @ 2019-11-25 16:06 ` Edmund Nadolski 2019-11-26 16:48 ` Christoph Hellwig 1 sibling, 1 reply; 10+ messages in thread From: Edmund Nadolski @ 2019-11-25 16:06 UTC (permalink / raw) To: edmund.nadolski, linux-nvme, kbusch Remove unnecessary keyword in nvme_create_queue(). Signed-off-by: Edmund Nadolski <edmund.nadolski@intel.com> --- drivers/nvme/host/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index c40a672e5047..76aa4bef27b8 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -1548,7 +1548,7 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) result = adapter_alloc_sq(dev, qid, nvmeq); if (result < 0) return result; - else if (result) + if (result) goto release_cq; nvmeq->cq_vector = vector; -- 2.20.1 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nvme: else following return is not needed 2019-11-25 16:06 ` [PATCH 2/2] nvme: else following return is not needed Edmund Nadolski @ 2019-11-26 16:48 ` Christoph Hellwig 2019-11-26 17:38 ` Keith Busch 0 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2019-11-26 16:48 UTC (permalink / raw) To: Edmund Nadolski; +Cc: kbusch, linux-nvme On Mon, Nov 25, 2019 at 09:06:12AM -0700, Edmund Nadolski wrote: > Remove unnecessary keyword in nvme_create_queue(). > > Signed-off-by: Edmund Nadolski <edmund.nadolski@intel.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nvme: else following return is not needed 2019-11-26 16:48 ` Christoph Hellwig @ 2019-11-26 17:38 ` Keith Busch 0 siblings, 0 replies; 10+ messages in thread From: Keith Busch @ 2019-11-26 17:38 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Edmund Nadolski, linux-nvme Applied to nvme/for-5.5 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-11-26 17:38 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-25 16:06 [PATCH 0/2] A few code cleanups Edmund Nadolski 2019-11-25 16:06 ` [PATCH 1/2] nvme: remove unused return code from nvme_alloc_ns Edmund Nadolski 2019-11-25 16:09 ` Johannes Thumshirn 2019-11-25 16:46 ` Nadolski, Edmund 2019-11-25 17:06 ` Keith Busch 2019-11-26 4:01 ` Chaitanya Kulkarni 2019-11-26 16:47 ` Christoph Hellwig 2019-11-25 16:06 ` [PATCH 2/2] nvme: else following return is not needed Edmund Nadolski 2019-11-26 16:48 ` Christoph Hellwig 2019-11-26 17:38 ` Keith Busch
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).