* [PATCH] vfio/mdev: Move the compat_class initialization to module init
@ 2023-06-26 13:36 Eric Farman
2023-06-26 14:17 ` Jason Gunthorpe
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Eric Farman @ 2023-06-26 13:36 UTC (permalink / raw)
To: Kirti Wankhede, Alex Williamson, Christoph Hellwig
Cc: Matthew Rosato, Kevin Tian, Jason Gunthorpe, Tony Krowiak, kvm,
Eric Farman, Alexander Egorenkov
The pointer to mdev_bus_compat_class is statically defined at the top
of mdev_core, and was originally (commit 7b96953bc640 ("vfio: Mediated
device Core driver") serialized by the parent_list_lock. The blamed
commit removed this mutex, leaving the pointer initialization
unserialized. As a result, the creation of multiple MDEVs in parallel
(such as during boot) can encounter errors during the creation of the
sysfs entries, such as:
[ 8.337509] sysfs: cannot create duplicate filename '/class/mdev_bus'
[ 8.337514] vfio_ccw 0.0.01d8: MDEV: Registered
[ 8.337516] CPU: 13 PID: 946 Comm: driverctl Not tainted 6.4.0-rc7 #20
[ 8.337522] Hardware name: IBM 3906 M05 780 (LPAR)
[ 8.337525] Call Trace:
[ 8.337528] [<0000000162b0145a>] dump_stack_lvl+0x62/0x80
[ 8.337540] [<00000001622aeb30>] sysfs_warn_dup+0x78/0x88
[ 8.337549] [<00000001622aeca6>] sysfs_create_dir_ns+0xe6/0xf8
[ 8.337552] [<0000000162b04504>] kobject_add_internal+0xf4/0x340
[ 8.337557] [<0000000162b04d48>] kobject_add+0x78/0xd0
[ 8.337561] [<0000000162b04e0a>] kobject_create_and_add+0x6a/0xb8
[ 8.337565] [<00000001627a110e>] class_compat_register+0x5e/0x90
[ 8.337572] [<000003ff7fd815da>] mdev_register_parent+0x102/0x130 [mdev]
[ 8.337581] [<000003ff7fdc7f2c>] vfio_ccw_sch_probe+0xe4/0x178 [vfio_ccw]
[ 8.337588] [<0000000162a7833c>] css_probe+0x44/0x80
[ 8.337599] [<000000016279f4da>] really_probe+0xd2/0x460
[ 8.337603] [<000000016279fa08>] driver_probe_device+0x40/0xf0
[ 8.337606] [<000000016279fb78>] __device_attach_driver+0xc0/0x140
[ 8.337610] [<000000016279cbe0>] bus_for_each_drv+0x90/0xd8
[ 8.337618] [<00000001627a00b0>] __device_attach+0x110/0x190
[ 8.337621] [<000000016279c7c8>] bus_rescan_devices_helper+0x60/0xb0
[ 8.337626] [<000000016279cd48>] drivers_probe_store+0x48/0x80
[ 8.337632] [<00000001622ac9b0>] kernfs_fop_write_iter+0x138/0x1f0
[ 8.337635] [<00000001621e5e14>] vfs_write+0x1ac/0x2f8
[ 8.337645] [<00000001621e61d8>] ksys_write+0x70/0x100
[ 8.337650] [<0000000162b2bdc4>] __do_syscall+0x1d4/0x200
[ 8.337656] [<0000000162b3c828>] system_call+0x70/0x98
[ 8.337664] kobject: kobject_add_internal failed for mdev_bus with -EEXIST, don't try to register things with the same name in the same directory.
[ 8.337668] kobject: kobject_create_and_add: kobject_add error: -17
[ 8.337674] vfio_ccw: probe of 0.0.01d9 failed with error -12
[ 8.342941] vfio_ccw_mdev aeb9ca91-10c6-42bc-a168-320023570aea: Adding to iommu group 2
Move the initialization of the mdev_bus_compat_class pointer to the
init path, to match the cleanup in module exit. This way the code
in mdev_register_parent() can simply link the new parent to it,
rather than determining whether initialization is required first.
Fixes: 89345d5177aa ("vfio/mdev: embedd struct mdev_parent in the parent data structure")
Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
drivers/vfio/mdev/mdev_core.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 58f91b3bd670..ed4737de4528 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -72,12 +72,6 @@ int mdev_register_parent(struct mdev_parent *parent, struct device *dev,
parent->nr_types = nr_types;
atomic_set(&parent->available_instances, mdev_driver->max_instances);
- if (!mdev_bus_compat_class) {
- mdev_bus_compat_class = class_compat_register("mdev_bus");
- if (!mdev_bus_compat_class)
- return -ENOMEM;
- }
-
ret = parent_create_sysfs_files(parent);
if (ret)
return ret;
@@ -251,13 +245,24 @@ int mdev_device_remove(struct mdev_device *mdev)
static int __init mdev_init(void)
{
- return bus_register(&mdev_bus_type);
+ int ret;
+
+ ret = bus_register(&mdev_bus_type);
+ if (ret)
+ return ret;
+
+ mdev_bus_compat_class = class_compat_register("mdev_bus");
+ if (!mdev_bus_compat_class) {
+ bus_unregister(&mdev_bus_type);
+ return -ENOMEM;
+ }
+
+ return 0;
}
static void __exit mdev_exit(void)
{
- if (mdev_bus_compat_class)
- class_compat_unregister(mdev_bus_compat_class);
+ class_compat_unregister(mdev_bus_compat_class);
bus_unregister(&mdev_bus_type);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] vfio/mdev: Move the compat_class initialization to module init
2023-06-26 13:36 [PATCH] vfio/mdev: Move the compat_class initialization to module init Eric Farman
@ 2023-06-26 14:17 ` Jason Gunthorpe
2023-06-26 18:47 ` Anthony Krowiak
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Jason Gunthorpe @ 2023-06-26 14:17 UTC (permalink / raw)
To: Eric Farman
Cc: Kirti Wankhede, Alex Williamson, Christoph Hellwig,
Matthew Rosato, Kevin Tian, Tony Krowiak, kvm,
Alexander Egorenkov
On Mon, Jun 26, 2023 at 03:36:42PM +0200, Eric Farman wrote:
> The pointer to mdev_bus_compat_class is statically defined at the top
> of mdev_core, and was originally (commit 7b96953bc640 ("vfio: Mediated
> device Core driver") serialized by the parent_list_lock. The blamed
> commit removed this mutex, leaving the pointer initialization
> unserialized. As a result, the creation of multiple MDEVs in parallel
> (such as during boot) can encounter errors during the creation of the
> sysfs entries, such as:
>
> [ 8.337509] sysfs: cannot create duplicate filename '/class/mdev_bus'
> [ 8.337514] vfio_ccw 0.0.01d8: MDEV: Registered
> [ 8.337516] CPU: 13 PID: 946 Comm: driverctl Not tainted 6.4.0-rc7 #20
> [ 8.337522] Hardware name: IBM 3906 M05 780 (LPAR)
> [ 8.337525] Call Trace:
> [ 8.337528] [<0000000162b0145a>] dump_stack_lvl+0x62/0x80
> [ 8.337540] [<00000001622aeb30>] sysfs_warn_dup+0x78/0x88
> [ 8.337549] [<00000001622aeca6>] sysfs_create_dir_ns+0xe6/0xf8
> [ 8.337552] [<0000000162b04504>] kobject_add_internal+0xf4/0x340
> [ 8.337557] [<0000000162b04d48>] kobject_add+0x78/0xd0
> [ 8.337561] [<0000000162b04e0a>] kobject_create_and_add+0x6a/0xb8
> [ 8.337565] [<00000001627a110e>] class_compat_register+0x5e/0x90
> [ 8.337572] [<000003ff7fd815da>] mdev_register_parent+0x102/0x130 [mdev]
> [ 8.337581] [<000003ff7fdc7f2c>] vfio_ccw_sch_probe+0xe4/0x178 [vfio_ccw]
> [ 8.337588] [<0000000162a7833c>] css_probe+0x44/0x80
> [ 8.337599] [<000000016279f4da>] really_probe+0xd2/0x460
> [ 8.337603] [<000000016279fa08>] driver_probe_device+0x40/0xf0
> [ 8.337606] [<000000016279fb78>] __device_attach_driver+0xc0/0x140
> [ 8.337610] [<000000016279cbe0>] bus_for_each_drv+0x90/0xd8
> [ 8.337618] [<00000001627a00b0>] __device_attach+0x110/0x190
> [ 8.337621] [<000000016279c7c8>] bus_rescan_devices_helper+0x60/0xb0
> [ 8.337626] [<000000016279cd48>] drivers_probe_store+0x48/0x80
> [ 8.337632] [<00000001622ac9b0>] kernfs_fop_write_iter+0x138/0x1f0
> [ 8.337635] [<00000001621e5e14>] vfs_write+0x1ac/0x2f8
> [ 8.337645] [<00000001621e61d8>] ksys_write+0x70/0x100
> [ 8.337650] [<0000000162b2bdc4>] __do_syscall+0x1d4/0x200
> [ 8.337656] [<0000000162b3c828>] system_call+0x70/0x98
> [ 8.337664] kobject: kobject_add_internal failed for mdev_bus with -EEXIST, don't try to register things with the same name in the same directory.
> [ 8.337668] kobject: kobject_create_and_add: kobject_add error: -17
> [ 8.337674] vfio_ccw: probe of 0.0.01d9 failed with error -12
> [ 8.342941] vfio_ccw_mdev aeb9ca91-10c6-42bc-a168-320023570aea: Adding to iommu group 2
>
> Move the initialization of the mdev_bus_compat_class pointer to the
> init path, to match the cleanup in module exit. This way the code
> in mdev_register_parent() can simply link the new parent to it,
> rather than determining whether initialization is required first.
>
> Fixes: 89345d5177aa ("vfio/mdev: embedd struct mdev_parent in the parent data structure")
> Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
> drivers/vfio/mdev/mdev_core.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vfio/mdev: Move the compat_class initialization to module init
2023-06-26 13:36 [PATCH] vfio/mdev: Move the compat_class initialization to module init Eric Farman
2023-06-26 14:17 ` Jason Gunthorpe
@ 2023-06-26 18:47 ` Anthony Krowiak
2023-06-27 4:51 ` Christoph Hellwig
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Anthony Krowiak @ 2023-06-26 18:47 UTC (permalink / raw)
To: Eric Farman, Kirti Wankhede, Alex Williamson, Christoph Hellwig
Cc: Matthew Rosato, Kevin Tian, Jason Gunthorpe, kvm,
Alexander Egorenkov
Reviewed-by: Tony Krowiak <akrowiak@linux.ibm.com>
On 6/26/23 9:36 AM, Eric Farman wrote:
> The pointer to mdev_bus_compat_class is statically defined at the top
> of mdev_core, and was originally (commit 7b96953bc640 ("vfio: Mediated
> device Core driver") serialized by the parent_list_lock. The blamed
> commit removed this mutex, leaving the pointer initialization
> unserialized. As a result, the creation of multiple MDEVs in parallel
> (such as during boot) can encounter errors during the creation of the
> sysfs entries, such as:
>
> [ 8.337509] sysfs: cannot create duplicate filename '/class/mdev_bus'
> [ 8.337514] vfio_ccw 0.0.01d8: MDEV: Registered
> [ 8.337516] CPU: 13 PID: 946 Comm: driverctl Not tainted 6.4.0-rc7 #20
> [ 8.337522] Hardware name: IBM 3906 M05 780 (LPAR)
> [ 8.337525] Call Trace:
> [ 8.337528] [<0000000162b0145a>] dump_stack_lvl+0x62/0x80
> [ 8.337540] [<00000001622aeb30>] sysfs_warn_dup+0x78/0x88
> [ 8.337549] [<00000001622aeca6>] sysfs_create_dir_ns+0xe6/0xf8
> [ 8.337552] [<0000000162b04504>] kobject_add_internal+0xf4/0x340
> [ 8.337557] [<0000000162b04d48>] kobject_add+0x78/0xd0
> [ 8.337561] [<0000000162b04e0a>] kobject_create_and_add+0x6a/0xb8
> [ 8.337565] [<00000001627a110e>] class_compat_register+0x5e/0x90
> [ 8.337572] [<000003ff7fd815da>] mdev_register_parent+0x102/0x130 [mdev]
> [ 8.337581] [<000003ff7fdc7f2c>] vfio_ccw_sch_probe+0xe4/0x178 [vfio_ccw]
> [ 8.337588] [<0000000162a7833c>] css_probe+0x44/0x80
> [ 8.337599] [<000000016279f4da>] really_probe+0xd2/0x460
> [ 8.337603] [<000000016279fa08>] driver_probe_device+0x40/0xf0
> [ 8.337606] [<000000016279fb78>] __device_attach_driver+0xc0/0x140
> [ 8.337610] [<000000016279cbe0>] bus_for_each_drv+0x90/0xd8
> [ 8.337618] [<00000001627a00b0>] __device_attach+0x110/0x190
> [ 8.337621] [<000000016279c7c8>] bus_rescan_devices_helper+0x60/0xb0
> [ 8.337626] [<000000016279cd48>] drivers_probe_store+0x48/0x80
> [ 8.337632] [<00000001622ac9b0>] kernfs_fop_write_iter+0x138/0x1f0
> [ 8.337635] [<00000001621e5e14>] vfs_write+0x1ac/0x2f8
> [ 8.337645] [<00000001621e61d8>] ksys_write+0x70/0x100
> [ 8.337650] [<0000000162b2bdc4>] __do_syscall+0x1d4/0x200
> [ 8.337656] [<0000000162b3c828>] system_call+0x70/0x98
> [ 8.337664] kobject: kobject_add_internal failed for mdev_bus with -EEXIST, don't try to register things with the same name in the same directory.
> [ 8.337668] kobject: kobject_create_and_add: kobject_add error: -17
> [ 8.337674] vfio_ccw: probe of 0.0.01d9 failed with error -12
> [ 8.342941] vfio_ccw_mdev aeb9ca91-10c6-42bc-a168-320023570aea: Adding to iommu group 2
>
> Move the initialization of the mdev_bus_compat_class pointer to the
> init path, to match the cleanup in module exit. This way the code
> in mdev_register_parent() can simply link the new parent to it,
> rather than determining whether initialization is required first.
>
> Fixes: 89345d5177aa ("vfio/mdev: embedd struct mdev_parent in the parent data structure")
> Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
> drivers/vfio/mdev/mdev_core.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 58f91b3bd670..ed4737de4528 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -72,12 +72,6 @@ int mdev_register_parent(struct mdev_parent *parent, struct device *dev,
> parent->nr_types = nr_types;
> atomic_set(&parent->available_instances, mdev_driver->max_instances);
>
> - if (!mdev_bus_compat_class) {
> - mdev_bus_compat_class = class_compat_register("mdev_bus");
> - if (!mdev_bus_compat_class)
> - return -ENOMEM;
> - }
> -
> ret = parent_create_sysfs_files(parent);
> if (ret)
> return ret;
> @@ -251,13 +245,24 @@ int mdev_device_remove(struct mdev_device *mdev)
>
> static int __init mdev_init(void)
> {
> - return bus_register(&mdev_bus_type);
> + int ret;
> +
> + ret = bus_register(&mdev_bus_type);
> + if (ret)
> + return ret;
> +
> + mdev_bus_compat_class = class_compat_register("mdev_bus");
> + if (!mdev_bus_compat_class) {
> + bus_unregister(&mdev_bus_type);
> + return -ENOMEM;
> + }
> +
> + return 0;
> }
>
> static void __exit mdev_exit(void)
> {
> - if (mdev_bus_compat_class)
> - class_compat_unregister(mdev_bus_compat_class);
> + class_compat_unregister(mdev_bus_compat_class);
> bus_unregister(&mdev_bus_type);
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vfio/mdev: Move the compat_class initialization to module init
2023-06-26 13:36 [PATCH] vfio/mdev: Move the compat_class initialization to module init Eric Farman
2023-06-26 14:17 ` Jason Gunthorpe
2023-06-26 18:47 ` Anthony Krowiak
@ 2023-06-27 4:51 ` Christoph Hellwig
2023-06-27 6:05 ` Tian, Kevin
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2023-06-27 4:51 UTC (permalink / raw)
To: Eric Farman
Cc: Kirti Wankhede, Alex Williamson, Christoph Hellwig,
Matthew Rosato, Kevin Tian, Jason Gunthorpe, Tony Krowiak, kvm,
Alexander Egorenkov
Yeah, this is the structure that the code should have had since the
very beginning:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] vfio/mdev: Move the compat_class initialization to module init
2023-06-26 13:36 [PATCH] vfio/mdev: Move the compat_class initialization to module init Eric Farman
` (2 preceding siblings ...)
2023-06-27 4:51 ` Christoph Hellwig
@ 2023-06-27 6:05 ` Tian, Kevin
2023-06-27 21:01 ` Alex Williamson
2023-06-28 7:39 ` Kirti Wankhede
5 siblings, 0 replies; 7+ messages in thread
From: Tian, Kevin @ 2023-06-27 6:05 UTC (permalink / raw)
To: Eric Farman, Kirti Wankhede, Alex Williamson, Christoph Hellwig
Cc: Matthew Rosato, Jason Gunthorpe, Tony Krowiak,
kvm@vger.kernel.org, Alexander Egorenkov
> From: Eric Farman <farman@linux.ibm.com>
> Sent: Monday, June 26, 2023 9:37 PM
>
> The pointer to mdev_bus_compat_class is statically defined at the top
> of mdev_core, and was originally (commit 7b96953bc640 ("vfio: Mediated
> device Core driver") serialized by the parent_list_lock. The blamed
> commit removed this mutex, leaving the pointer initialization
> unserialized. As a result, the creation of multiple MDEVs in parallel
> (such as during boot) can encounter errors during the creation of the
> sysfs entries, such as:
>
> [ 8.337509] sysfs: cannot create duplicate filename '/class/mdev_bus'
> [ 8.337514] vfio_ccw 0.0.01d8: MDEV: Registered
> [ 8.337516] CPU: 13 PID: 946 Comm: driverctl Not tainted 6.4.0-rc7 #20
> [ 8.337522] Hardware name: IBM 3906 M05 780 (LPAR)
> [ 8.337525] Call Trace:
> [ 8.337528] [<0000000162b0145a>] dump_stack_lvl+0x62/0x80
> [ 8.337540] [<00000001622aeb30>] sysfs_warn_dup+0x78/0x88
> [ 8.337549] [<00000001622aeca6>] sysfs_create_dir_ns+0xe6/0xf8
> [ 8.337552] [<0000000162b04504>] kobject_add_internal+0xf4/0x340
> [ 8.337557] [<0000000162b04d48>] kobject_add+0x78/0xd0
> [ 8.337561] [<0000000162b04e0a>] kobject_create_and_add+0x6a/0xb8
> [ 8.337565] [<00000001627a110e>] class_compat_register+0x5e/0x90
> [ 8.337572] [<000003ff7fd815da>] mdev_register_parent+0x102/0x130
> [mdev]
> [ 8.337581] [<000003ff7fdc7f2c>] vfio_ccw_sch_probe+0xe4/0x178
> [vfio_ccw]
> [ 8.337588] [<0000000162a7833c>] css_probe+0x44/0x80
> [ 8.337599] [<000000016279f4da>] really_probe+0xd2/0x460
> [ 8.337603] [<000000016279fa08>] driver_probe_device+0x40/0xf0
> [ 8.337606] [<000000016279fb78>] __device_attach_driver+0xc0/0x140
> [ 8.337610] [<000000016279cbe0>] bus_for_each_drv+0x90/0xd8
> [ 8.337618] [<00000001627a00b0>] __device_attach+0x110/0x190
> [ 8.337621] [<000000016279c7c8>]
> bus_rescan_devices_helper+0x60/0xb0
> [ 8.337626] [<000000016279cd48>] drivers_probe_store+0x48/0x80
> [ 8.337632] [<00000001622ac9b0>] kernfs_fop_write_iter+0x138/0x1f0
> [ 8.337635] [<00000001621e5e14>] vfs_write+0x1ac/0x2f8
> [ 8.337645] [<00000001621e61d8>] ksys_write+0x70/0x100
> [ 8.337650] [<0000000162b2bdc4>] __do_syscall+0x1d4/0x200
> [ 8.337656] [<0000000162b3c828>] system_call+0x70/0x98
> [ 8.337664] kobject: kobject_add_internal failed for mdev_bus with -
> EEXIST, don't try to register things with the same name in the same directory.
> [ 8.337668] kobject: kobject_create_and_add: kobject_add error: -17
> [ 8.337674] vfio_ccw: probe of 0.0.01d9 failed with error -12
> [ 8.342941] vfio_ccw_mdev aeb9ca91-10c6-42bc-a168-320023570aea:
> Adding to iommu group 2
>
> Move the initialization of the mdev_bus_compat_class pointer to the
> init path, to match the cleanup in module exit. This way the code
> in mdev_register_parent() can simply link the new parent to it,
> rather than determining whether initialization is required first.
>
> Fixes: 89345d5177aa ("vfio/mdev: embedd struct mdev_parent in the parent
> data structure")
> Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vfio/mdev: Move the compat_class initialization to module init
2023-06-26 13:36 [PATCH] vfio/mdev: Move the compat_class initialization to module init Eric Farman
` (3 preceding siblings ...)
2023-06-27 6:05 ` Tian, Kevin
@ 2023-06-27 21:01 ` Alex Williamson
2023-06-28 7:39 ` Kirti Wankhede
5 siblings, 0 replies; 7+ messages in thread
From: Alex Williamson @ 2023-06-27 21:01 UTC (permalink / raw)
To: Eric Farman
Cc: Kirti Wankhede, Christoph Hellwig, Matthew Rosato, Kevin Tian,
Jason Gunthorpe, Tony Krowiak, kvm, Alexander Egorenkov
On Mon, 26 Jun 2023 15:36:42 +0200
Eric Farman <farman@linux.ibm.com> wrote:
> The pointer to mdev_bus_compat_class is statically defined at the top
> of mdev_core, and was originally (commit 7b96953bc640 ("vfio: Mediated
> device Core driver") serialized by the parent_list_lock. The blamed
> commit removed this mutex, leaving the pointer initialization
> unserialized. As a result, the creation of multiple MDEVs in parallel
> (such as during boot) can encounter errors during the creation of the
> sysfs entries, such as:
>
> [ 8.337509] sysfs: cannot create duplicate filename '/class/mdev_bus'
> [ 8.337514] vfio_ccw 0.0.01d8: MDEV: Registered
> [ 8.337516] CPU: 13 PID: 946 Comm: driverctl Not tainted 6.4.0-rc7 #20
> [ 8.337522] Hardware name: IBM 3906 M05 780 (LPAR)
> [ 8.337525] Call Trace:
> [ 8.337528] [<0000000162b0145a>] dump_stack_lvl+0x62/0x80
> [ 8.337540] [<00000001622aeb30>] sysfs_warn_dup+0x78/0x88
> [ 8.337549] [<00000001622aeca6>] sysfs_create_dir_ns+0xe6/0xf8
> [ 8.337552] [<0000000162b04504>] kobject_add_internal+0xf4/0x340
> [ 8.337557] [<0000000162b04d48>] kobject_add+0x78/0xd0
> [ 8.337561] [<0000000162b04e0a>] kobject_create_and_add+0x6a/0xb8
> [ 8.337565] [<00000001627a110e>] class_compat_register+0x5e/0x90
> [ 8.337572] [<000003ff7fd815da>] mdev_register_parent+0x102/0x130 [mdev]
> [ 8.337581] [<000003ff7fdc7f2c>] vfio_ccw_sch_probe+0xe4/0x178 [vfio_ccw]
> [ 8.337588] [<0000000162a7833c>] css_probe+0x44/0x80
> [ 8.337599] [<000000016279f4da>] really_probe+0xd2/0x460
> [ 8.337603] [<000000016279fa08>] driver_probe_device+0x40/0xf0
> [ 8.337606] [<000000016279fb78>] __device_attach_driver+0xc0/0x140
> [ 8.337610] [<000000016279cbe0>] bus_for_each_drv+0x90/0xd8
> [ 8.337618] [<00000001627a00b0>] __device_attach+0x110/0x190
> [ 8.337621] [<000000016279c7c8>] bus_rescan_devices_helper+0x60/0xb0
> [ 8.337626] [<000000016279cd48>] drivers_probe_store+0x48/0x80
> [ 8.337632] [<00000001622ac9b0>] kernfs_fop_write_iter+0x138/0x1f0
> [ 8.337635] [<00000001621e5e14>] vfs_write+0x1ac/0x2f8
> [ 8.337645] [<00000001621e61d8>] ksys_write+0x70/0x100
> [ 8.337650] [<0000000162b2bdc4>] __do_syscall+0x1d4/0x200
> [ 8.337656] [<0000000162b3c828>] system_call+0x70/0x98
> [ 8.337664] kobject: kobject_add_internal failed for mdev_bus with -EEXIST, don't try to register things with the same name in the same directory.
> [ 8.337668] kobject: kobject_create_and_add: kobject_add error: -17
> [ 8.337674] vfio_ccw: probe of 0.0.01d9 failed with error -12
> [ 8.342941] vfio_ccw_mdev aeb9ca91-10c6-42bc-a168-320023570aea: Adding to iommu group 2
>
> Move the initialization of the mdev_bus_compat_class pointer to the
> init path, to match the cleanup in module exit. This way the code
> in mdev_register_parent() can simply link the new parent to it,
> rather than determining whether initialization is required first.
>
> Fixes: 89345d5177aa ("vfio/mdev: embedd struct mdev_parent in the parent data structure")
> Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
> drivers/vfio/mdev/mdev_core.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
Applied to vfio next branch for v6.5. Thanks!
Alex
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 58f91b3bd670..ed4737de4528 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -72,12 +72,6 @@ int mdev_register_parent(struct mdev_parent *parent, struct device *dev,
> parent->nr_types = nr_types;
> atomic_set(&parent->available_instances, mdev_driver->max_instances);
>
> - if (!mdev_bus_compat_class) {
> - mdev_bus_compat_class = class_compat_register("mdev_bus");
> - if (!mdev_bus_compat_class)
> - return -ENOMEM;
> - }
> -
> ret = parent_create_sysfs_files(parent);
> if (ret)
> return ret;
> @@ -251,13 +245,24 @@ int mdev_device_remove(struct mdev_device *mdev)
>
> static int __init mdev_init(void)
> {
> - return bus_register(&mdev_bus_type);
> + int ret;
> +
> + ret = bus_register(&mdev_bus_type);
> + if (ret)
> + return ret;
> +
> + mdev_bus_compat_class = class_compat_register("mdev_bus");
> + if (!mdev_bus_compat_class) {
> + bus_unregister(&mdev_bus_type);
> + return -ENOMEM;
> + }
> +
> + return 0;
> }
>
> static void __exit mdev_exit(void)
> {
> - if (mdev_bus_compat_class)
> - class_compat_unregister(mdev_bus_compat_class);
> + class_compat_unregister(mdev_bus_compat_class);
> bus_unregister(&mdev_bus_type);
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] vfio/mdev: Move the compat_class initialization to module init
2023-06-26 13:36 [PATCH] vfio/mdev: Move the compat_class initialization to module init Eric Farman
` (4 preceding siblings ...)
2023-06-27 21:01 ` Alex Williamson
@ 2023-06-28 7:39 ` Kirti Wankhede
5 siblings, 0 replies; 7+ messages in thread
From: Kirti Wankhede @ 2023-06-28 7:39 UTC (permalink / raw)
To: Eric Farman, Alex Williamson, Christoph Hellwig
Cc: Matthew Rosato, Kevin Tian, Jason Gunthorpe, Tony Krowiak,
kvm@vger.kernel.org, Alexander Egorenkov
> -----Original Message-----
> From: Eric Farman <farman@linux.ibm.com>
> Sent: Monday, June 26, 2023 7:07 PM
> To: Kirti Wankhede <kwankhede@nvidia.com>; Alex Williamson
> <alex.williamson@redhat.com>; Christoph Hellwig <hch@lst.de>
> Cc: Matthew Rosato <mjrosato@linux.ibm.com>; Kevin Tian
> <kevin.tian@intel.com>; Jason Gunthorpe <jgg@nvidia.com>; Tony Krowiak
> <akrowiak@linux.ibm.com>; kvm@vger.kernel.org; Eric Farman
> <farman@linux.ibm.com>; Alexander Egorenkov <egorenar@linux.ibm.com>
> Subject: [PATCH] vfio/mdev: Move the compat_class initialization to module init
>
> The pointer to mdev_bus_compat_class is statically defined at the top
> of mdev_core, and was originally (commit 7b96953bc640 ("vfio: Mediated
> device Core driver") serialized by the parent_list_lock. The blamed
> commit removed this mutex, leaving the pointer initialization
> unserialized. As a result, the creation of multiple MDEVs in parallel
> (such as during boot) can encounter errors during the creation of the
> sysfs entries, such as:
>
> [ 8.337509] sysfs: cannot create duplicate filename '/class/mdev_bus'
> [ 8.337514] vfio_ccw 0.0.01d8: MDEV: Registered
> [ 8.337516] CPU: 13 PID: 946 Comm: driverctl Not tainted 6.4.0-rc7 #20
> [ 8.337522] Hardware name: IBM 3906 M05 780 (LPAR)
> [ 8.337525] Call Trace:
> [ 8.337528] [<0000000162b0145a>] dump_stack_lvl+0x62/0x80
> [ 8.337540] [<00000001622aeb30>] sysfs_warn_dup+0x78/0x88
> [ 8.337549] [<00000001622aeca6>] sysfs_create_dir_ns+0xe6/0xf8
> [ 8.337552] [<0000000162b04504>] kobject_add_internal+0xf4/0x340
> [ 8.337557] [<0000000162b04d48>] kobject_add+0x78/0xd0
> [ 8.337561] [<0000000162b04e0a>] kobject_create_and_add+0x6a/0xb8
> [ 8.337565] [<00000001627a110e>] class_compat_register+0x5e/0x90
> [ 8.337572] [<000003ff7fd815da>] mdev_register_parent+0x102/0x130 [mdev]
> [ 8.337581] [<000003ff7fdc7f2c>] vfio_ccw_sch_probe+0xe4/0x178 [vfio_ccw]
> [ 8.337588] [<0000000162a7833c>] css_probe+0x44/0x80
> [ 8.337599] [<000000016279f4da>] really_probe+0xd2/0x460
> [ 8.337603] [<000000016279fa08>] driver_probe_device+0x40/0xf0
> [ 8.337606] [<000000016279fb78>] __device_attach_driver+0xc0/0x140
> [ 8.337610] [<000000016279cbe0>] bus_for_each_drv+0x90/0xd8
> [ 8.337618] [<00000001627a00b0>] __device_attach+0x110/0x190
> [ 8.337621] [<000000016279c7c8>] bus_rescan_devices_helper+0x60/0xb0
> [ 8.337626] [<000000016279cd48>] drivers_probe_store+0x48/0x80
> [ 8.337632] [<00000001622ac9b0>] kernfs_fop_write_iter+0x138/0x1f0
> [ 8.337635] [<00000001621e5e14>] vfs_write+0x1ac/0x2f8
> [ 8.337645] [<00000001621e61d8>] ksys_write+0x70/0x100
> [ 8.337650] [<0000000162b2bdc4>] __do_syscall+0x1d4/0x200
> [ 8.337656] [<0000000162b3c828>] system_call+0x70/0x98
> [ 8.337664] kobject: kobject_add_internal failed for mdev_bus with -EEXIST,
> don't try to register things with the same name in the same directory.
> [ 8.337668] kobject: kobject_create_and_add: kobject_add error: -17
> [ 8.337674] vfio_ccw: probe of 0.0.01d9 failed with error -12
> [ 8.342941] vfio_ccw_mdev aeb9ca91-10c6-42bc-a168-320023570aea: Adding
> to iommu group 2
>
> Move the initialization of the mdev_bus_compat_class pointer to the
> init path, to match the cleanup in module exit. This way the code
> in mdev_register_parent() can simply link the new parent to it,
> rather than determining whether initialization is required first.
>
> Fixes: 89345d5177aa ("vfio/mdev: embedd struct mdev_parent in the parent data
> structure")
> Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
> drivers/vfio/mdev/mdev_core.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 58f91b3bd670..ed4737de4528 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -72,12 +72,6 @@ int mdev_register_parent(struct mdev_parent *parent,
> struct device *dev,
> parent->nr_types = nr_types;
> atomic_set(&parent->available_instances, mdev_driver->max_instances);
>
> - if (!mdev_bus_compat_class) {
> - mdev_bus_compat_class = class_compat_register("mdev_bus");
> - if (!mdev_bus_compat_class)
> - return -ENOMEM;
> - }
> -
> ret = parent_create_sysfs_files(parent);
> if (ret)
> return ret;
> @@ -251,13 +245,24 @@ int mdev_device_remove(struct mdev_device *mdev)
>
> static int __init mdev_init(void)
> {
> - return bus_register(&mdev_bus_type);
> + int ret;
> +
> + ret = bus_register(&mdev_bus_type);
> + if (ret)
> + return ret;
> +
> + mdev_bus_compat_class = class_compat_register("mdev_bus");
> + if (!mdev_bus_compat_class) {
> + bus_unregister(&mdev_bus_type);
> + return -ENOMEM;
> + }
> +
> + return 0;
> }
>
> static void __exit mdev_exit(void)
> {
> - if (mdev_bus_compat_class)
> - class_compat_unregister(mdev_bus_compat_class);
> + class_compat_unregister(mdev_bus_compat_class);
> bus_unregister(&mdev_bus_type);
> }
>
> --
> 2.39.2
Reviewed By: Kirti Wankhede <kwankhede@nvidia.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-28 9:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-26 13:36 [PATCH] vfio/mdev: Move the compat_class initialization to module init Eric Farman
2023-06-26 14:17 ` Jason Gunthorpe
2023-06-26 18:47 ` Anthony Krowiak
2023-06-27 4:51 ` Christoph Hellwig
2023-06-27 6:05 ` Tian, Kevin
2023-06-27 21:01 ` Alex Williamson
2023-06-28 7:39 ` Kirti Wankhede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox