* [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
* [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 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
* 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).