All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] block: don't embed integrity_kobj into gendisk
@ 2023-03-09 20:23 Thomas Weißschuh
  2023-03-09 21:05 ` Mirsad Goran Todorovac
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Thomas Weißschuh @ 2023-03-09 20:23 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, linux-kernel, Greg Kroah-Hartman, Mirsad Todorovac,
	Thomas Weißschuh

A struct kobject is only supposed to be embedded into objects which
lifetime it will manage.
Objects of type struct gendisk however are refcounted by their part0
block_device.
Therefore the integrity_kobj should not be embedded but split into its
own independently managed object.

This will also provide a proper .release function for the ktype which
avoid warnings like the following:
kobject: 'integrity' (000000005198bea8): does not have a release() function, it is broken and must be fixed.

While modifying blk_integrity_del() also drop the explicit call to
kobject_uevent(KOBJ_REMOVE) as the driver care will do this
automatically.

Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/lkml/60b2b66c-22c9-1d38-ed1c-7b7d95e32720@alu.unizg.hr/
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 block/blk-integrity.c  | 32 ++++++++++++++++++++++++--------
 include/linux/blkdev.h |  2 +-
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 8f01d786f5cb..40adf33f5535 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -218,10 +218,15 @@ struct integrity_sysfs_entry {
 	ssize_t (*store)(struct blk_integrity *, const char *, size_t);
 };
 
+static inline struct gendisk *integrity_kobj_to_disk(struct kobject *kobj)
+{
+	return dev_to_disk(kobj_to_dev(kobj->parent));
+}
+
 static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
 				   char *page)
 {
-	struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+	struct gendisk *disk = integrity_kobj_to_disk(kobj);
 	struct blk_integrity *bi = &disk->queue->integrity;
 	struct integrity_sysfs_entry *entry =
 		container_of(attr, struct integrity_sysfs_entry, attr);
@@ -233,7 +238,7 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
 				    struct attribute *attr, const char *page,
 				    size_t count)
 {
-	struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+	struct gendisk *disk = integrity_kobj_to_disk(kobj);
 	struct blk_integrity *bi = &disk->queue->integrity;
 	struct integrity_sysfs_entry *entry =
 		container_of(attr, struct integrity_sysfs_entry, attr);
@@ -356,9 +361,15 @@ static const struct sysfs_ops integrity_ops = {
 	.store	= &integrity_attr_store,
 };
 
+static void integrity_release(struct kobject *kobj)
+{
+	kfree(kobj);
+}
+
 static const struct kobj_type integrity_ktype = {
 	.default_groups = integrity_groups,
 	.sysfs_ops	= &integrity_ops,
+	.release        = integrity_release,
 };
 
 static blk_status_t blk_integrity_nop_fn(struct blk_integrity_iter *iter)
@@ -442,16 +453,21 @@ int blk_integrity_add(struct gendisk *disk)
 {
 	int ret;
 
-	ret = kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
+	disk->integrity_kobj = kmalloc(sizeof(*disk->integrity_kobj), GFP_KERNEL);
+	if (!disk->integrity_kobj)
+		return -ENOMEM;
+
+	ret = kobject_init_and_add(disk->integrity_kobj, &integrity_ktype,
 				   &disk_to_dev(disk)->kobj, "%s", "integrity");
-	if (!ret)
-		kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
+	if (ret)
+		kobject_put(disk->integrity_kobj);
+	else
+		kobject_uevent(disk->integrity_kobj, KOBJ_ADD);
+
 	return ret;
 }
 
 void blk_integrity_del(struct gendisk *disk)
 {
-	kobject_uevent(&disk->integrity_kobj, KOBJ_REMOVE);
-	kobject_del(&disk->integrity_kobj);
-	kobject_put(&disk->integrity_kobj);
+	kobject_put(disk->integrity_kobj);
 }
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d1aee08f8c18..2fbfb3277a2b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -164,7 +164,7 @@ struct gendisk {
 	atomic_t sync_io;		/* RAID */
 	struct disk_events *ev;
 #ifdef  CONFIG_BLK_DEV_INTEGRITY
-	struct kobject integrity_kobj;
+	struct kobject *integrity_kobj;
 #endif	/* CONFIG_BLK_DEV_INTEGRITY */
 
 #ifdef CONFIG_BLK_DEV_ZONED

---
base-commit: 44889ba56cbb3d51154660ccd15818bc77276696
change-id: 20230309-kobj_release-gendisk_integrity-e26c0bc126aa

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-03-14  9:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-09 20:23 [PATCH] block: don't embed integrity_kobj into gendisk Thomas Weißschuh
2023-03-09 21:05 ` Mirsad Goran Todorovac
2023-03-09 21:23   ` Thomas Weißschuh
2023-03-09 21:46     ` Mirsad Goran Todorovac
2023-03-09 23:26       ` Thomas Weißschuh
2023-03-10  8:52         ` Mirsad Todorovac
2023-03-10 13:42           ` Andy Shevchenko
2023-03-09 21:47     ` Andy Shevchenko
2023-03-09 21:20 ` Martin K. Petersen
2023-03-14  9:15 ` kernel test robot

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.