Re: [PATCH v1 2/2] mm: mTHP stats for pagecache folio allocations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2024/7/13 19:00, Ryan Roberts wrote:
[...]

+static int thpsize_create(int order, struct kobject *parent)
   {
       unsigned long size = (PAGE_SIZE << order) / SZ_1K;
+    struct thpsize_child *stats;
       struct thpsize *thpsize;
       int ret;
   +    /*
+     * Each child object (currently only "stats" directory) holds a
+     * reference to the top-level thpsize object, so we can drop our ref to
+     * the top-level once stats is setup. Then we just need to drop a
+     * reference on any children to clean everything up. We can't just use
+     * the attr group name for the stats subdirectory because there may be
+     * multiple attribute groups to populate inside stats and overlaying
+     * using the name property isn't supported in that way; each attr group
+     * name, if provided, must be unique in the parent directory.
+     */
+
       thpsize = kzalloc(sizeof(*thpsize), GFP_KERNEL);
-    if (!thpsize)
-        return ERR_PTR(-ENOMEM);
+    if (!thpsize) {
+        ret = -ENOMEM;
+        goto err;
+    }
+    thpsize->order = order;
         ret = kobject_init_and_add(&thpsize->kobj, &thpsize_ktype, parent,
                      "hugepages-%lukB", size);
       if (ret) {
           kfree(thpsize);
-        return ERR_PTR(ret);
+        goto err;
       }
   -    ret = sysfs_create_group(&thpsize->kobj, &thpsize_attr_group);
-    if (ret) {
+    stats = kzalloc(sizeof(*stats), GFP_KERNEL);
+    if (!stats) {
           kobject_put(&thpsize->kobj);
-        return ERR_PTR(ret);
+        ret = -ENOMEM;
+        goto err;
       }
   -    ret = sysfs_create_group(&thpsize->kobj, &stats_attr_group);
+    ret = kobject_init_and_add(&stats->kobj, &thpsize_child_ktype,
+                   &thpsize->kobj, "stats");
+    kobject_put(&thpsize->kobj);
       if (ret) {
-        kobject_put(&thpsize->kobj);
-        return ERR_PTR(ret);
+        kfree(stats);
+        goto err;
       }
   -    thpsize->order = order;
-    return thpsize;
+    if (BIT(order) & THP_ORDERS_ALL_ANON) {
+        ret = sysfs_create_group(&thpsize->kobj, &thpsize_attr_group);
+        if (ret)
+            goto err_put;
+
+        ret = sysfs_create_group(&stats->kobj, &stats_attr_group);
+        if (ret)
+            goto err_put;
+    }
+
+    if (BIT(order) & PAGECACHE_LARGE_ORDERS) {
+        ret = sysfs_create_group(&stats->kobj, &file_stats_attr_group);
+        if (ret)
+            goto err_put;
+    }
+
+    list_add(&stats->node, &thpsize_child_list);
+    return 0;
+err_put:

IIUC, I think you should call 'sysfs_remove_group' to remove the group before
putting the kobject.

Are you sure about that? As I understood it, sysfs_create_group() was
conceptually modifying the state of the kobj, so when the kobj gets destroyed,
all its state is tidied up. __kobject_del() (called on the last kobject_put())
calls sysfs_remove_groups() and tidies up the sysfs state as far as I can see?

IIUC, __kobject_del() only removes the ktype defaut groups by 'sysfs_remove_groups(kobj, ktype->default_groups)', but your created groups are not added into the ktype->default_groups. That means you should mannuly remove them, or am I miss something?




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux