* [PATCH V2 0/2] nvme: fix module ref count Oops @ 2020-09-16 3:53 Chaitanya Kulkarni 2020-09-16 3:53 ` [PATCH V2 1/2] nvme-core: fix nvme " Chaitanya Kulkarni 2020-09-16 3:53 ` [PATCH V2 2/2] nvme: decouple nvme_get_ctrl() from file open Chaitanya Kulkarni 0 siblings, 2 replies; 9+ messages in thread From: Chaitanya Kulkarni @ 2020-09-16 3:53 UTC (permalink / raw) To: linux-nvme; +Cc: kbusch, hch, Chaitanya Kulkarni, sagi [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=y, Size: 921 bytes --] Hi, For nvme-ctrl char device we don't currently get the ctrl's module refcount. This leads to the Oops. In this series, we get/put the module refcount in nvme-ctrl char dev open/release, lift the file opening from the host-core to caller in the NVMeOF target passthru and take the ctrl refcount in into the nvmet_passthru_ctrl_enable(). -Chaitanya Changes from V1: - 1. Move last patch to get the module refcount to start of the series. 2. De-couple the module refcount get/put from nvme_dev_open() and nvme_dev_release(). Chaitanya Kulkarni (2): nvme-core: fix nvme module ref count Oops nvme: decouple nvme_get_ctrl() from file open drivers/nvme/host/core.c | 28 +++++++++++++++++----------- drivers/nvme/host/nvme.h | 2 +- drivers/nvme/target/nvmet.h | 1 + drivers/nvme/target/passthru.c | 19 +++++++++++++++---- 4 files changed, 34 insertions(+), 16 deletions(-) -- 2.22.1 [-- Attachment #2: Type: text/plain, Size: 158 bytes --] _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V2 1/2] nvme-core: fix nvme module ref count Oops 2020-09-16 3:53 [PATCH V2 0/2] nvme: fix module ref count Oops Chaitanya Kulkarni @ 2020-09-16 3:53 ` Chaitanya Kulkarni 2020-09-16 6:47 ` Christoph Hellwig 2020-09-16 15:58 ` Logan Gunthorpe 2020-09-16 3:53 ` [PATCH V2 2/2] nvme: decouple nvme_get_ctrl() from file open Chaitanya Kulkarni 1 sibling, 2 replies; 9+ messages in thread From: Chaitanya Kulkarni @ 2020-09-16 3:53 UTC (permalink / raw) To: linux-nvme; +Cc: kbusch, hch, Chaitanya Kulkarni, sagi Introduce car dev relase function, get/put the module refernece which allows us to fix the potential Oops which can be easily reproduced with NVMeOF passthru ctrl :- Entering kdb (current=0xffff8887f8290000, pid 3128) on processor 30 Oops: (null) due to oops @ 0xffffffffa01019ad CPU: 30 PID: 3128 Comm: bash Tainted: G W OE 5.8.0-rc4nvme-5.9+ #35 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.4 RIP: 0010:nvme_free_ctrl+0x234/0x285 [nvme_core] Code: 57 10 a0 e8 73 bf 02 e1 ba 3d 11 00 00 48 c7 c6 98 33 10 a0 48 c7 c7 1d 57 10 a0 e8 5b bf 02 e1 8 RSP: 0018:ffffc90001d63de0 EFLAGS: 00010246 RAX: ffffffffa05c0440 RBX: ffff8888119e45a0 RCX: 0000000000000000 RDX: 0000000000000000 RSI: ffff8888177e9550 RDI: ffff8888119e43b0 RBP: ffff8887d4768000 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: ffffc90001d63c90 R12: ffff8888119e43b0 R13: ffff8888119e5108 R14: dead000000000100 R15: ffff8888119e5108 FS: 00007f1ef27b0740(0000) GS:ffff888817600000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffffffa05c0470 CR3: 00000007f6bee000 CR4: 00000000003406e0 Call Trace: device_release+0x27/0x80 kobject_put+0x98/0x170 nvmet_passthru_ctrl_disable+0x4a/0x70 [nvmet] nvmet_passthru_enable_store+0x4c/0x90 [nvmet] configfs_write_file+0xe6/0x150 vfs_write+0xba/0x1e0 ksys_write+0x5f/0xe0 do_syscall_64+0x52/0xb0 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f1ef1eb2840 Code: Bad RIP value. RSP: 002b:00007fffdbff0eb8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f1ef1eb2840 RDX: 0000000000000002 RSI: 00007f1ef27d2000 RDI: 0000000000000001 RBP: 00007f1ef27d2000 R08: 000000000000000a R09: 00007f1ef27b0740 R10: 0000000000000001 R11: 0000000000000246 R12: 00007f1ef2186400 R13: 0000000000000002 R14: 0000000000000001 R15: 0000000000000000 With this patch fix we take the module ref count in nvme_dev_open() and release that ref count in newly introduced nvme_dev_release(). Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- drivers/nvme/host/core.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 8b75f6ca0b61..c5f9d64b2bec 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -3261,10 +3261,22 @@ static int nvme_dev_open(struct inode *inode, struct file *file) return -EWOULDBLOCK; } + if (!try_module_get(ctrl->ops->module)) + return -EINVAL; + file->private_data = ctrl; return 0; } +static int nvme_dev_release(struct inode *inode, struct file *file) +{ + struct nvme_ctrl *ctrl = + container_of(inode->i_cdev, struct nvme_ctrl, cdev); + + module_put(ctrl->ops->module); + return 0; +} + static int nvme_dev_user_cmd(struct nvme_ctrl *ctrl, void __user *argp) { struct nvme_ns *ns; @@ -3327,6 +3339,7 @@ static long nvme_dev_ioctl(struct file *file, unsigned int cmd, static const struct file_operations nvme_dev_fops = { .owner = THIS_MODULE, .open = nvme_dev_open, + .release = nvme_dev_release, .unlocked_ioctl = nvme_dev_ioctl, .compat_ioctl = compat_ptr_ioctl, }; -- 2.22.1 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/2] nvme-core: fix nvme module ref count Oops 2020-09-16 3:53 ` [PATCH V2 1/2] nvme-core: fix nvme " Chaitanya Kulkarni @ 2020-09-16 6:47 ` Christoph Hellwig 2020-09-16 15:58 ` Logan Gunthorpe 1 sibling, 0 replies; 9+ messages in thread From: Christoph Hellwig @ 2020-09-16 6:47 UTC (permalink / raw) To: Chaitanya Kulkarni; +Cc: kbusch, hch, linux-nvme, sagi Applied to nvme-5.9. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/2] nvme-core: fix nvme module ref count Oops 2020-09-16 3:53 ` [PATCH V2 1/2] nvme-core: fix nvme " Chaitanya Kulkarni 2020-09-16 6:47 ` Christoph Hellwig @ 2020-09-16 15:58 ` Logan Gunthorpe 2020-09-16 16:01 ` Christoph Hellwig 1 sibling, 1 reply; 9+ messages in thread From: Logan Gunthorpe @ 2020-09-16 15:58 UTC (permalink / raw) To: Chaitanya Kulkarni, linux-nvme; +Cc: kbusch, hch, sagi On 2020-09-15 9:53 p.m., Chaitanya Kulkarni wrote: > Introduce car dev relase function, get/put the module refernece which > allows us to fix the potential Oops which can be easily reproduced with > NVMeOF passthru ctrl :- > > Entering kdb (current=0xffff8887f8290000, pid 3128) on processor 30 Oops: (null) > due to oops @ 0xffffffffa01019ad > CPU: 30 PID: 3128 Comm: bash Tainted: G W OE 5.8.0-rc4nvme-5.9+ #35 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.4 > RIP: 0010:nvme_free_ctrl+0x234/0x285 [nvme_core] > Code: 57 10 a0 e8 73 bf 02 e1 ba 3d 11 00 00 48 c7 c6 98 33 10 a0 48 c7 c7 1d 57 10 a0 e8 5b bf 02 e1 8 > RSP: 0018:ffffc90001d63de0 EFLAGS: 00010246 > RAX: ffffffffa05c0440 RBX: ffff8888119e45a0 RCX: 0000000000000000 > RDX: 0000000000000000 RSI: ffff8888177e9550 RDI: ffff8888119e43b0 > RBP: ffff8887d4768000 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: ffffc90001d63c90 R12: ffff8888119e43b0 > R13: ffff8888119e5108 R14: dead000000000100 R15: ffff8888119e5108 > FS: 00007f1ef27b0740(0000) GS:ffff888817600000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ffffffffa05c0470 CR3: 00000007f6bee000 CR4: 00000000003406e0 > Call Trace: > device_release+0x27/0x80 > kobject_put+0x98/0x170 > nvmet_passthru_ctrl_disable+0x4a/0x70 [nvmet] > nvmet_passthru_enable_store+0x4c/0x90 [nvmet] > configfs_write_file+0xe6/0x150 > vfs_write+0xba/0x1e0 > ksys_write+0x5f/0xe0 > do_syscall_64+0x52/0xb0 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > RIP: 0033:0x7f1ef1eb2840 > Code: Bad RIP value. > RSP: 002b:00007fffdbff0eb8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 > RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f1ef1eb2840 > RDX: 0000000000000002 RSI: 00007f1ef27d2000 RDI: 0000000000000001 > RBP: 00007f1ef27d2000 R08: 000000000000000a R09: 00007f1ef27b0740 > R10: 0000000000000001 R11: 0000000000000246 R12: 00007f1ef2186400 > R13: 0000000000000002 R14: 0000000000000001 R15: 0000000000000000 > > With this patch fix we take the module ref count in nvme_dev_open() and > release that ref count in newly introduced nvme_dev_release(). > > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> > --- > drivers/nvme/host/core.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 8b75f6ca0b61..c5f9d64b2bec 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -3261,10 +3261,22 @@ static int nvme_dev_open(struct inode *inode, struct file *file) > return -EWOULDBLOCK; > } > > + if (!try_module_get(ctrl->ops->module)) > + return -EINVAL; Aren't we also still missing the nvme_get_ctrl() here? We have a reference to the controller that's not counted; which was the original bug, and we need a reference to the module to be able to put that reference... Logan _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/2] nvme-core: fix nvme module ref count Oops 2020-09-16 15:58 ` Logan Gunthorpe @ 2020-09-16 16:01 ` Christoph Hellwig 0 siblings, 0 replies; 9+ messages in thread From: Christoph Hellwig @ 2020-09-16 16:01 UTC (permalink / raw) To: Logan Gunthorpe; +Cc: kbusch, sagi, linux-nvme, Chaitanya Kulkarni, hch On Wed, Sep 16, 2020 at 09:58:38AM -0600, Logan Gunthorpe wrote: > > > On 2020-09-15 9:53 p.m., Chaitanya Kulkarni wrote: > > Introduce car dev relase function, get/put the module refernece which > > allows us to fix the potential Oops which can be easily reproduced with > > NVMeOF passthru ctrl :- > > > > Entering kdb (current=0xffff8887f8290000, pid 3128) on processor 30 Oops: (null) > > due to oops @ 0xffffffffa01019ad > > CPU: 30 PID: 3128 Comm: bash Tainted: G W OE 5.8.0-rc4nvme-5.9+ #35 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.4 > > RIP: 0010:nvme_free_ctrl+0x234/0x285 [nvme_core] > > Code: 57 10 a0 e8 73 bf 02 e1 ba 3d 11 00 00 48 c7 c6 98 33 10 a0 48 c7 c7 1d 57 10 a0 e8 5b bf 02 e1 8 > > RSP: 0018:ffffc90001d63de0 EFLAGS: 00010246 > > RAX: ffffffffa05c0440 RBX: ffff8888119e45a0 RCX: 0000000000000000 > > RDX: 0000000000000000 RSI: ffff8888177e9550 RDI: ffff8888119e43b0 > > RBP: ffff8887d4768000 R08: 0000000000000000 R09: 0000000000000000 > > R10: 0000000000000000 R11: ffffc90001d63c90 R12: ffff8888119e43b0 > > R13: ffff8888119e5108 R14: dead000000000100 R15: ffff8888119e5108 > > FS: 00007f1ef27b0740(0000) GS:ffff888817600000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: ffffffffa05c0470 CR3: 00000007f6bee000 CR4: 00000000003406e0 > > Call Trace: > > device_release+0x27/0x80 > > kobject_put+0x98/0x170 > > nvmet_passthru_ctrl_disable+0x4a/0x70 [nvmet] > > nvmet_passthru_enable_store+0x4c/0x90 [nvmet] > > configfs_write_file+0xe6/0x150 > > vfs_write+0xba/0x1e0 > > ksys_write+0x5f/0xe0 > > do_syscall_64+0x52/0xb0 > > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > RIP: 0033:0x7f1ef1eb2840 > > Code: Bad RIP value. > > RSP: 002b:00007fffdbff0eb8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 > > RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f1ef1eb2840 > > RDX: 0000000000000002 RSI: 00007f1ef27d2000 RDI: 0000000000000001 > > RBP: 00007f1ef27d2000 R08: 000000000000000a R09: 00007f1ef27b0740 > > R10: 0000000000000001 R11: 0000000000000246 R12: 00007f1ef2186400 > > R13: 0000000000000002 R14: 0000000000000001 R15: 0000000000000000 > > > > With this patch fix we take the module ref count in nvme_dev_open() and > > release that ref count in newly introduced nvme_dev_release(). > > > > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> > > --- > > drivers/nvme/host/core.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > > index 8b75f6ca0b61..c5f9d64b2bec 100644 > > --- a/drivers/nvme/host/core.c > > +++ b/drivers/nvme/host/core.c > > @@ -3261,10 +3261,22 @@ static int nvme_dev_open(struct inode *inode, struct file *file) > > return -EWOULDBLOCK; > > } > > > > + if (!try_module_get(ctrl->ops->module)) > > + return -EINVAL; > > Aren't we also still missing the nvme_get_ctrl() here? We have a > reference to the controller that's not counted; which was the original > bug, and we need a reference to the module to be able to put that > reference... Yes, indeed. Pulled from nvme-5.9 again.. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V2 2/2] nvme: decouple nvme_get_ctrl() from file open 2020-09-16 3:53 [PATCH V2 0/2] nvme: fix module ref count Oops Chaitanya Kulkarni 2020-09-16 3:53 ` [PATCH V2 1/2] nvme-core: fix nvme " Chaitanya Kulkarni @ 2020-09-16 3:53 ` Chaitanya Kulkarni 2020-09-16 6:52 ` Christoph Hellwig 1 sibling, 1 reply; 9+ messages in thread From: Chaitanya Kulkarni @ 2020-09-16 3:53 UTC (permalink / raw) To: linux-nvme; +Cc: kbusch, hch, Chaitanya Kulkarni, sagi Rename nvme_ctrl_get_by_path() -> nvme_ctrl_get_by_file() and lift the file opening and error handling in the caller so that we can unwind appropriately in the error path (in nvmet_passthru_ctrl_enable()). Now that we decoupled the file open/close from host/core.c move the nvme_get_ctrl() to nvmet_passthru_ctrl_enable(), also close the file before we release the passthru controller's reference. Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- drivers/nvme/host/core.c | 15 ++++----------- drivers/nvme/host/nvme.h | 2 +- drivers/nvme/target/nvmet.h | 1 + drivers/nvme/target/passthru.c | 19 +++++++++++++++---- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index c5f9d64b2bec..c446584d8b12 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4649,28 +4649,21 @@ void nvme_sync_queues(struct nvme_ctrl *ctrl) } EXPORT_SYMBOL_GPL(nvme_sync_queues); -struct nvme_ctrl *nvme_ctrl_get_by_path(const char *path) +struct nvme_ctrl *nvme_ctrl_get_by_file(struct file *f) { struct nvme_ctrl *ctrl; - struct file *f; - - f = filp_open(path, O_RDWR, 0); - if (IS_ERR(f)) - return ERR_CAST(f); if (f->f_op != &nvme_dev_fops) { ctrl = ERR_PTR(-EINVAL); - goto out_close; + goto out; } ctrl = f->private_data; - nvme_get_ctrl(ctrl); -out_close: - filp_close(f, NULL); +out: return ctrl; } -EXPORT_SYMBOL_NS_GPL(nvme_ctrl_get_by_path, NVME_TARGET_PASSTHRU); +EXPORT_SYMBOL_NS_GPL(nvme_ctrl_get_by_file, NVME_TARGET_PASSTHRU); /* * Check we didn't inadvertently grow the command structure sizes: diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 9fd45ff656da..2cb966653a33 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -835,7 +835,7 @@ static inline void nvme_hwmon_init(struct nvme_ctrl *ctrl) { } u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode); void nvme_execute_passthru_rq(struct request *rq); -struct nvme_ctrl *nvme_ctrl_get_by_path(const char *path); +struct nvme_ctrl *nvme_ctrl_get_by_file(struct file *f); struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid); void nvme_put_ns(struct nvme_ns *ns); diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h index 47ee3fb193bd..477439acb8e1 100644 --- a/drivers/nvme/target/nvmet.h +++ b/drivers/nvme/target/nvmet.h @@ -248,6 +248,7 @@ struct nvmet_subsys { #ifdef CONFIG_NVME_TARGET_PASSTHRU struct nvme_ctrl *passthru_ctrl; char *passthru_ctrl_path; + struct file *passthru_ctrl_file; struct config_group passthru_group; #endif /* CONFIG_NVME_TARGET_PASSTHRU */ }; diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c index 8bd7f656e240..84f9daea81db 100644 --- a/drivers/nvme/target/passthru.c +++ b/drivers/nvme/target/passthru.c @@ -473,12 +473,13 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req) int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) { + const char *pt_path = subsys->passthru_ctrl_path; struct nvme_ctrl *ctrl; int ret = -EINVAL; void *old; mutex_lock(&subsys->lock); - if (!subsys->passthru_ctrl_path) + if (!pt_path) goto out_unlock; if (subsys->passthru_ctrl) goto out_unlock; @@ -488,15 +489,22 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) goto out_unlock; } - ctrl = nvme_ctrl_get_by_path(subsys->passthru_ctrl_path); + subsys->passthru_ctrl_file = filp_open(pt_path, O_RDWR, 0); + if (IS_ERR(subsys->passthru_ctrl_file)) { + ret = PTR_ERR(subsys->passthru_ctrl_file); + goto out_unlock; + } + + ctrl = nvme_ctrl_get_by_file(subsys->passthru_ctrl_file); if (IS_ERR(ctrl)) { ret = PTR_ERR(ctrl); pr_err("failed to open nvme controller %s\n", subsys->passthru_ctrl_path); - - goto out_unlock; + goto out_put_file; } + nvme_get_ctrl(ctrl); + old = xa_cmpxchg(&passthru_subsystems, ctrl->cntlid, NULL, subsys, GFP_KERNEL); if (xa_is_err(old)) { @@ -522,6 +530,8 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) out_put_ctrl: nvme_put_ctrl(ctrl); +out_put_file: + filp_close(subsys->passthru_ctrl_file, NULL); out_unlock: mutex_unlock(&subsys->lock); return ret; @@ -531,6 +541,7 @@ static void __nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys) { if (subsys->passthru_ctrl) { xa_erase(&passthru_subsystems, subsys->passthru_ctrl->cntlid); + filp_close(subsys->passthru_ctrl_file, NULL); nvme_put_ctrl(subsys->passthru_ctrl); } subsys->passthru_ctrl = NULL; -- 2.22.1 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V2 2/2] nvme: decouple nvme_get_ctrl() from file open 2020-09-16 3:53 ` [PATCH V2 2/2] nvme: decouple nvme_get_ctrl() from file open Chaitanya Kulkarni @ 2020-09-16 6:52 ` Christoph Hellwig 2020-09-16 6:54 ` Christoph Hellwig 2020-09-16 16:07 ` Logan Gunthorpe 0 siblings, 2 replies; 9+ messages in thread From: Christoph Hellwig @ 2020-09-16 6:52 UTC (permalink / raw) To: Chaitanya Kulkarni; +Cc: kbusch, hch, linux-nvme, sagi [-- Attachment #1: Type: text/plain, Size: 78 bytes --] This needs to be split into a fix and a cleanup. Attached is how I'd do it. [-- Attachment #2: 0001-nvmet-ensure-the-passthrough-controller-has-a-refere.patch --] [-- Type: text/x-patch, Size: 1343 bytes --] From 497f52797e6d068712ecd794bd54d59fd7af3036 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig <hch@lst.de> Date: Wed, 16 Sep 2020 08:47:50 +0200 Subject: nvmet: ensure the passthrough controller has a reference to the transport Grab a reference to the transport driver to ensure it can't be unloaded while a passthrough controller is active. Fixes: c1fef73f793b ("nvmet: add passthru code to process commands") Reported-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/nvme/target/passthru.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c index 8bd7f656e240b2..dacfa7435d0b2f 100644 --- a/drivers/nvme/target/passthru.c +++ b/drivers/nvme/target/passthru.c @@ -517,6 +517,7 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) subsys->ver = NVME_VS(1, 2, 1); } + __module_get(subsys->passthru_ctrl->ops->module); mutex_unlock(&subsys->lock); return 0; @@ -531,6 +532,7 @@ static void __nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys) { if (subsys->passthru_ctrl) { xa_erase(&passthru_subsystems, subsys->passthru_ctrl->cntlid); + module_put(subsys->passthru_ctrl->ops->module); nvme_put_ctrl(subsys->passthru_ctrl); } subsys->passthru_ctrl = NULL; -- 2.28.0 [-- Attachment #3: 0002-nvme-decouple-nvme_get_ctrl-from-file-open.patch --] [-- Type: text/x-patch, Size: 4014 bytes --] From e912746df46bed02b695c9869544a111f0f7487f Mon Sep 17 00:00:00 2001 From: Christoph Hellwig <hch@lst.de> Date: Wed, 16 Sep 2020 08:48:21 +0200 Subject: nvme: decouple nvme_get_ctrl() from file open Lift opening the file from nvme_ctrl_get_by_path into the caller, just keeping a simple nvme_ctrl_from_file helper. Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> [hch: refactored a bit, split the bug fixes into a separate prep patch] Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/nvme/host/core.c | 25 +++++-------------------- drivers/nvme/host/nvme.h | 2 +- drivers/nvme/target/passthru.c | 27 ++++++++++++++++----------- 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index c5f9d64b2bec02..f51ccb43002231 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4649,28 +4649,13 @@ void nvme_sync_queues(struct nvme_ctrl *ctrl) } EXPORT_SYMBOL_GPL(nvme_sync_queues); -struct nvme_ctrl *nvme_ctrl_get_by_path(const char *path) +struct nvme_ctrl *nvme_ctrl_from_file(struct file *file) { - struct nvme_ctrl *ctrl; - struct file *f; - - f = filp_open(path, O_RDWR, 0); - if (IS_ERR(f)) - return ERR_CAST(f); - - if (f->f_op != &nvme_dev_fops) { - ctrl = ERR_PTR(-EINVAL); - goto out_close; - } - - ctrl = f->private_data; - nvme_get_ctrl(ctrl); - -out_close: - filp_close(f, NULL); - return ctrl; + if (file->f_op != &nvme_dev_fops) + return NULL; + return file->private_data; } -EXPORT_SYMBOL_NS_GPL(nvme_ctrl_get_by_path, NVME_TARGET_PASSTHRU); +EXPORT_SYMBOL_NS_GPL(nvme_ctrl_from_file, NVME_TARGET_PASSTHRU); /* * Check we didn't inadvertently grow the command structure sizes: diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 9fd45ff656da81..1e6aaa7102627a 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -835,7 +835,7 @@ static inline void nvme_hwmon_init(struct nvme_ctrl *ctrl) { } u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode); void nvme_execute_passthru_rq(struct request *rq); -struct nvme_ctrl *nvme_ctrl_get_by_path(const char *path); +struct nvme_ctrl *nvme_ctrl_from_file(struct file *file); struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid); void nvme_put_ns(struct nvme_ns *ns); diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c index dacfa7435d0b2f..c0fd20d8097ee8 100644 --- a/drivers/nvme/target/passthru.c +++ b/drivers/nvme/target/passthru.c @@ -474,6 +474,7 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req) int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) { struct nvme_ctrl *ctrl; + struct file *file; int ret = -EINVAL; void *old; @@ -488,24 +489,28 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) goto out_unlock; } - ctrl = nvme_ctrl_get_by_path(subsys->passthru_ctrl_path); - if (IS_ERR(ctrl)) { - ret = PTR_ERR(ctrl); + file = filp_open(subsys->passthru_ctrl_path, O_RDWR, 0); + if (IS_ERR(file)) { + ret = PTR_ERR(file); + goto out_unlock; + } + + ctrl = nvme_ctrl_from_file(file); + if (!ctrl) { pr_err("failed to open nvme controller %s\n", subsys->passthru_ctrl_path); - - goto out_unlock; + goto out_put_file; } old = xa_cmpxchg(&passthru_subsystems, ctrl->cntlid, NULL, subsys, GFP_KERNEL); if (xa_is_err(old)) { ret = xa_err(old); - goto out_put_ctrl; + goto out_put_file; } if (old) - goto out_put_ctrl; + goto out_put_file; subsys->passthru_ctrl = ctrl; subsys->ver = ctrl->vs; @@ -517,12 +522,12 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) subsys->ver = NVME_VS(1, 2, 1); } + nvme_get_ctrl(ctrl); __module_get(subsys->passthru_ctrl->ops->module); - mutex_unlock(&subsys->lock); - return 0; + ret = 0; -out_put_ctrl: - nvme_put_ctrl(ctrl); +out_put_file: + filp_close(file, NULL); out_unlock: mutex_unlock(&subsys->lock); return ret; -- 2.28.0 [-- Attachment #4: Type: text/plain, Size: 158 bytes --] _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V2 2/2] nvme: decouple nvme_get_ctrl() from file open 2020-09-16 6:52 ` Christoph Hellwig @ 2020-09-16 6:54 ` Christoph Hellwig 2020-09-16 16:07 ` Logan Gunthorpe 1 sibling, 0 replies; 9+ messages in thread From: Christoph Hellwig @ 2020-09-16 6:54 UTC (permalink / raw) To: Chaitanya Kulkarni; +Cc: kbusch, hch, linux-nvme, sagi FYI, the second one should be attributed to you - git rebase keeps messing up the author when resolving conflicts.. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 2/2] nvme: decouple nvme_get_ctrl() from file open 2020-09-16 6:52 ` Christoph Hellwig 2020-09-16 6:54 ` Christoph Hellwig @ 2020-09-16 16:07 ` Logan Gunthorpe 1 sibling, 0 replies; 9+ messages in thread From: Logan Gunthorpe @ 2020-09-16 16:07 UTC (permalink / raw) To: Christoph Hellwig, Chaitanya Kulkarni; +Cc: kbusch, sagi, linux-nvme On 2020-09-16 12:52 a.m., Christoph Hellwig wrote: > This needs to be split into a fix and a cleanup. > > Attached is how I'd do it. > Christoph's patches for this look good to me. Whomever runs with them can add: Reviewed-by: Logan Gunthorpe <logang@deltatee.com> _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-09-16 16:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-16 3:53 [PATCH V2 0/2] nvme: fix module ref count Oops Chaitanya Kulkarni 2020-09-16 3:53 ` [PATCH V2 1/2] nvme-core: fix nvme " Chaitanya Kulkarni 2020-09-16 6:47 ` Christoph Hellwig 2020-09-16 15:58 ` Logan Gunthorpe 2020-09-16 16:01 ` Christoph Hellwig 2020-09-16 3:53 ` [PATCH V2 2/2] nvme: decouple nvme_get_ctrl() from file open Chaitanya Kulkarni 2020-09-16 6:52 ` Christoph Hellwig 2020-09-16 6:54 ` Christoph Hellwig 2020-09-16 16:07 ` Logan Gunthorpe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.