Hi Jens,
A couple of fixes for -rc3.
Patch 1: Fixes the device id detection for qemu.
Patch 2: Fix for conditional compilation of lightnvm with nvme by Keith.
Patch 3-5: Fixes for memory leaks and boundary check by Tao and Sudip.
Please pick up. Thanks.
Keith Busch (1):
lightnvm: Simplify config when disabled
Matias Bjørling (1):
lightnvm: change vendor and dev id for qemu
Sudip Mukherjee (1):
lightnvm: fix ioctl memory leaks
Wenwei Tao (2):
lightnvm: free memory when gennvm register fails
lightnvm: do device max sectors boundary check first
drivers/lightnvm/core.c | 19 +++++++++++++------
drivers/lightnvm/gennvm.c | 15 ++++++++++-----
drivers/nvme/host/Makefile | 3 ++-
drivers/nvme/host/lightnvm.c | 15 +--------------
drivers/nvme/host/nvme.h | 14 ++++++++++++++
5 files changed, 40 insertions(+), 26 deletions(-)
--
2.1.4
The QEMU NVMe simulator uses the intel vendor, qemu device id, and the
first vendor specific byte to identify a lightnvm compatible nvme
instance.
Instead of using the Intel NVMe QEMU instance vendor and device id,
let's use a preallocated from CNEX Labs instead. This lets us uniquely
identify a QEMU lightnvm device without breaking other vendor specific
work in the qemu device driver.
Reported-by: Christoph Hellwig <[email protected]>
Signed-off-by: Matias Bjørling <[email protected]>
---
drivers/nvme/host/lightnvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 9202d1a..0789265 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -577,7 +577,7 @@ int nvme_nvm_ns_supported(struct nvme_ns *ns, struct nvme_id_ns *id)
struct pci_dev *pdev = to_pci_dev(dev->dev);
/* QEMU NVMe simulator - PCI ID + Vendor specific bit */
- if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x5845 &&
+ if (pdev->vendor == 0x1d1d && pdev->device == 0x1f1f &&
id->vs[0] == 0x1)
return 1;
--
2.1.4
From: Keith Busch <[email protected]>
We shouldn't compile an object file to get empty implementations; conforms
to linux coding style on conditional compilation.
Reported-by: Christoph Hellwig <[email protected]>
Signed-off-by: Keith Busch <[email protected]>
Signed-off-by: Matias Bjørling <[email protected]>
---
drivers/nvme/host/Makefile | 3 ++-
drivers/nvme/host/lightnvm.c | 13 -------------
drivers/nvme/host/nvme.h | 14 ++++++++++++++
3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/nvme/host/Makefile b/drivers/nvme/host/Makefile
index 219dc206..a5fe239 100644
--- a/drivers/nvme/host/Makefile
+++ b/drivers/nvme/host/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_BLK_DEV_NVME) += nvme.o
-nvme-y += pci.o scsi.o lightnvm.o
+lightnvm-$(CONFIG_NVM) := lightnvm.o
+nvme-y += pci.o scsi.o $(lightnvm-y)
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 0789265..7fd20e5 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -22,8 +22,6 @@
#include "nvme.h"
-#ifdef CONFIG_NVM
-
#include <linux/nvme.h>
#include <linux/bitops.h>
#include <linux/lightnvm.h>
@@ -588,14 +586,3 @@ int nvme_nvm_ns_supported(struct nvme_ns *ns, struct nvme_id_ns *id)
return 0;
}
-#else
-int nvme_nvm_register(struct request_queue *q, char *disk_name)
-{
- return 0;
-}
-void nvme_nvm_unregister(struct request_queue *q, char *disk_name) {};
-int nvme_nvm_ns_supported(struct nvme_ns *ns, struct nvme_id_ns *id)
-{
- return 0;
-}
-#endif /* CONFIG_NVM */
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index fdb4e5b..044253d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -136,8 +136,22 @@ int nvme_sg_io(struct nvme_ns *ns, struct sg_io_hdr __user *u_hdr);
int nvme_sg_io32(struct nvme_ns *ns, unsigned long arg);
int nvme_sg_get_version_num(int __user *ip);
+#ifdef CONFIG_NVM
int nvme_nvm_ns_supported(struct nvme_ns *ns, struct nvme_id_ns *id);
int nvme_nvm_register(struct request_queue *q, char *disk_name);
void nvme_nvm_unregister(struct request_queue *q, char *disk_name);
+#else
+static inline int nvme_nvm_register(struct request_queue *q, char *disk_name)
+{
+ return 0;
+}
+
+static inline void nvme_nvm_unregister(struct request_queue *q, char *disk_name) {};
+
+static inline int nvme_nvm_ns_supported(struct nvme_ns *ns, struct nvme_id_ns *id)
+{
+ return 0;
+}
+#endif /* CONFIG_NVM */
#endif /* _NVME_H */
--
2.1.4
From: Wenwei Tao <[email protected]>
free allocated nvm block and gennvm lun structures when
gennvm register fails, otherwise it will cause memory leak.
Signed-off-by: Matias Bjørling <[email protected]>
---
drivers/lightnvm/gennvm.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/lightnvm/gennvm.c b/drivers/lightnvm/gennvm.c
index e20e74e..3969a98 100644
--- a/drivers/lightnvm/gennvm.c
+++ b/drivers/lightnvm/gennvm.c
@@ -207,6 +207,14 @@ static int gennvm_blocks_init(struct nvm_dev *dev, struct gen_nvm *gn)
return 0;
}
+static void gennvm_free(struct nvm_dev *dev)
+{
+ gennvm_blocks_free(dev);
+ gennvm_luns_free(dev);
+ kfree(dev->mp);
+ dev->mp = NULL;
+}
+
static int gennvm_register(struct nvm_dev *dev)
{
struct gen_nvm *gn;
@@ -234,16 +242,13 @@ static int gennvm_register(struct nvm_dev *dev)
return 1;
err:
- kfree(gn);
+ gennvm_free(dev);
return ret;
}
static void gennvm_unregister(struct nvm_dev *dev)
{
- gennvm_blocks_free(dev);
- gennvm_luns_free(dev);
- kfree(dev->mp);
- dev->mp = NULL;
+ gennvm_free(dev);
}
static struct nvm_block *gennvm_get_blk(struct nvm_dev *dev,
--
2.1.4
From: Sudip Mukherjee <[email protected]>
If copy_to_user() fails we returned error but we missed releasing
devices.
Signed-off-by: Sudip Mukherjee <[email protected]>
Signed-off-by: Matias Bjørling <[email protected]>
---
drivers/lightnvm/core.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 5178645..ea50fa5 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -680,8 +680,10 @@ static long nvm_ioctl_info(struct file *file, void __user *arg)
info->tgtsize = tgt_iter;
up_write(&nvm_lock);
- if (copy_to_user(arg, info, sizeof(struct nvm_ioctl_info)))
+ if (copy_to_user(arg, info, sizeof(struct nvm_ioctl_info))) {
+ kfree(info);
return -EFAULT;
+ }
kfree(info);
return 0;
@@ -724,8 +726,11 @@ static long nvm_ioctl_get_devices(struct file *file, void __user *arg)
devices->nr_devices = i;
- if (copy_to_user(arg, devices, sizeof(struct nvm_ioctl_get_devices)))
+ if (copy_to_user(arg, devices,
+ sizeof(struct nvm_ioctl_get_devices))) {
+ kfree(devices);
return -EFAULT;
+ }
kfree(devices);
return 0;
--
2.1.4
From: Wenwei Tao <[email protected]>
do device max_phys_sect boundary check first, otherwise
we will allocate dma_pools for devices whose max sectors
are beyond lightnvm support and register them.
Signed-off-by: Wenwei Tao <[email protected]>
Signed-off-by: Matias Bjørling <[email protected]>
---
drivers/lightnvm/core.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index ea50fa5..ea6dba5 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -308,6 +308,12 @@ int nvm_register(struct request_queue *q, char *disk_name,
if (ret)
goto err_init;
+ if (dev->ops->max_phys_sect > 256) {
+ pr_info("nvm: max sectors supported is 256.\n");
+ ret = -EINVAL;
+ goto err_init;
+ }
+
if (dev->ops->max_phys_sect > 1) {
dev->ppalist_pool = dev->ops->create_dma_pool(dev->q,
"ppalist");
@@ -316,10 +322,6 @@ int nvm_register(struct request_queue *q, char *disk_name,
ret = -ENOMEM;
goto err_init;
}
- } else if (dev->ops->max_phys_sect > 256) {
- pr_info("nvm: max sectors supported is 256.\n");
- ret = -EINVAL;
- goto err_init;
}
down_write(&nvm_lock);
--
2.1.4
> -----Original Message-----
> From: [email protected] [mailto:linux-kernel-
> [email protected]] On Behalf Of Matias Bjørling
> Sent: Tuesday, November 24, 2015 7:27 AM
...
> Instead of using the Intel NVMe QEMU instance vendor and device id,
> let's use a preallocated from CNEX Labs instead. This lets us
...
> /* QEMU NVMe simulator - PCI ID + Vendor specific bit */
> - if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device ==
> 0x5845 &&
> + if (pdev->vendor == 0x1d1d && pdev->device == 0x1f1f &&
> id->vs[0] == 0x1)
Could this patch add PCI_VENDOR_ID_CNEX to the appropriate .h file
and use that instead?
---
Robert Elliott, HPE Persistent Memory
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 11/24/2015 04:52 PM, Elliott, Robert (Persistent Memory) wrote:
>
>
>> -----Original Message-----
>> From: [email protected] [mailto:linux-kernel-
>> [email protected]] On Behalf Of Matias Bjørling
>> Sent: Tuesday, November 24, 2015 7:27 AM
> ...
>> Instead of using the Intel NVMe QEMU instance vendor and device id,
>> let's use a preallocated from CNEX Labs instead. This lets us
> ...
>> /* QEMU NVMe simulator - PCI ID + Vendor specific bit */
>> - if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device ==
>> 0x5845 &&
>> + if (pdev->vendor == 0x1d1d && pdev->device == 0x1f1f &&
>> id->vs[0] == 0x1)
>
> Could this patch add PCI_VENDOR_ID_CNEX to the appropriate .h file
> and use that instead?
We could. But it would only be for this single use-case? Might be a
little overkill to put in pci_ids.h. Opt for lightnvm.h? or somewhere else?
Thanks
On 11/24/2015 11:24 AM, Matias Bjørling wrote:
> On 11/24/2015 04:52 PM, Elliott, Robert (Persistent Memory) wrote:
>>
>>
>>> -----Original Message-----
>>> From: [email protected] [mailto:linux-kernel-
>>> [email protected]] On Behalf Of Matias Bjørling
>>> Sent: Tuesday, November 24, 2015 7:27 AM
>> ...
>>> Instead of using the Intel NVMe QEMU instance vendor and device id,
>>> let's use a preallocated from CNEX Labs instead. This lets us
>> ...
>>> /* QEMU NVMe simulator - PCI ID + Vendor specific bit */
>>> - if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device ==
>>> 0x5845 &&
>>> + if (pdev->vendor == 0x1d1d && pdev->device == 0x1f1f &&
>>> id->vs[0] == 0x1)
>>
>> Could this patch add PCI_VENDOR_ID_CNEX to the appropriate .h file
>> and use that instead?
>
> We could. But it would only be for this single use-case? Might be a
> little overkill to put in pci_ids.h. Opt for lightnvm.h? or somewhere else?
Or just add a comment, this:
if (pdev->vendor == 0x1d1d && pdev->device == 0x1f1f
means nothing to anyone.
--
Jens Axboe