From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9267C3DA42 for ; Sat, 13 Jul 2024 12:54:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2EF946B007B; Sat, 13 Jul 2024 08:54:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 29E6F6B0083; Sat, 13 Jul 2024 08:54:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 18D8D6B0085; Sat, 13 Jul 2024 08:54:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id F039A6B007B for ; Sat, 13 Jul 2024 08:54:16 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 98392160EF9 for ; Sat, 13 Jul 2024 12:54:16 +0000 (UTC) X-FDA: 82334722512.13.90BC562 Received: from out30-132.freemail.mail.aliyun.com (out30-132.freemail.mail.aliyun.com [115.124.30.132]) by imf17.hostedemail.com (Postfix) with ESMTP id 04E1540009 for ; Sat, 13 Jul 2024 12:54:12 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=hOcZHyJ8; dmarc=pass (policy=none) header.from=linux.alibaba.com; spf=pass (imf17.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.132 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720875225; a=rsa-sha256; cv=none; b=f1qRPfEXyfXwbzzJbv0/+2FY4mJHL0SLq+Vo1UDjoTUgY6JKodnEPU82gg0P/J8yE3LVC7 Ey/5kSzWXS0a4Utx/ZIoTdtZQrOL3ONqpArr7EUB3a/gFPpIciS8JD6mollah4Tv63G/ae 3JaMs4u2HMtgW8C3DsoD3o0pdPHiUrg= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=hOcZHyJ8; dmarc=pass (policy=none) header.from=linux.alibaba.com; spf=pass (imf17.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.132 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720875225; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=U3/MbRKAUT1nHEYQQc4tHTxgsmxngVtkmZqCwxsFAJo=; b=lgI177vMfSj9ciIcddTMJFzid8USADWwG4durRPUKi920b9yYdbfCB8V24gRo1bULQry0p W4rtQ2fSt2bA/K7Kjr4bdnYB+gpSCWWlQiiEcwT1vJVL3R51WO81u5aWdwWM2TfGJVr5fp QGZGXtGfLIDtjkJdwod2YZqNtS3tRp8= DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1720875249; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=U3/MbRKAUT1nHEYQQc4tHTxgsmxngVtkmZqCwxsFAJo=; b=hOcZHyJ8aEHgWtbfXRHZ78SqPlVmltQnhte4xLe43AvW1IYRpLXEdJg8ywWF6QIDlnbcw2gElyUn4x9heZ9FDTYAMA+rryVCaF4zrfq/ZlyxS9fyKFuAjxEg7dS/7jcGAQlg3+BqzqVW6rPXHg3N48yw+JwvVfaCxLjnkEh2sOw= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R211e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam033037067110;MF=baolin.wang@linux.alibaba.com;NM=1;PH=DS;RN=10;SR=0;TI=SMTPD_---0WAR-9Iw_1720875246; Received: from 30.32.91.177(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0WAR-9Iw_1720875246) by smtp.aliyun-inc.com; Sat, 13 Jul 2024 20:54:08 +0800 Message-ID: Date: Sat, 13 Jul 2024 20:54:06 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 2/2] mm: mTHP stats for pagecache folio allocations To: Ryan Roberts , Andrew Morton , Hugh Dickins , Jonathan Corbet , "Matthew Wilcox (Oracle)" , David Hildenbrand , Barry Song , Lance Yang Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20240711072929.3590000-1-ryan.roberts@arm.com> <20240711072929.3590000-3-ryan.roberts@arm.com> <9e0d84e5-2319-4425-9760-2c6bb23fc390@linux.alibaba.com> <29f0fc5a-c2b7-4925-9bdb-fd2abe5383ae@arm.com> From: Baolin Wang In-Reply-To: <29f0fc5a-c2b7-4925-9bdb-fd2abe5383ae@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 04E1540009 X-Stat-Signature: 8p71u51wj9fe9nahypotntf1oq85hw4e X-Rspam-User: X-HE-Tag: 1720875252-949204 X-HE-Meta: U2FsdGVkX1+0b3j9jYmKPEOeTdpsEXEwwtRWqjrvbdpuCDGT6VtU9DCRrzNROXCUu1JXjVIqiKW2uWwUWtjXWlpX1atEg2uX3EQdUVdpbA1hyhY44+AksLAxU6hP9oD1skCjt7n9vXBrTcV+Wm67hSkt+5HhFToPvEicHgFJaT6/fvclJgXyA9v6ert9sdwvNiPvf31n2moDkAd+wPlUDYfiXw9t4GMs8J8TlDLtl3czla9H6fh89kLu5ym3kVdL1jKAL9A2a9cKuE06h9hthj+liB9/6ZEn9RvJqXwgAOjrGiUc17YeIWLO82usObTgnfpv7P9QCbX3dmJW7MNL0cwqUmar2iBGAvvsuEyY7Pdv7OXvt2Oiz29P1tpMkLEMPFwTnz9hvuyTeMZWt3636yNxoQWKG8cy64ehDDBjeNcbhJFWnmBqtTsm8EGkHLEOfX6e1W85Z+A3IkIFLjwO9m4EbY78AEUJKe4du3FXcBKxsfKdWseASm4SBMzxoEGXcGpBUTNZTJgLLYPXKWGwnc9+nHg8RCifhK6CiZgFij++ZFG6v3En+7zmnckOuas6cxNUjEVuqN8qtMqKlUXr9QVf6gcyR9ba4bF//ECl9JlYqZdEL4tz+ArlpJgns2gYnxnYXsDCon5Q/I4PH6mruqdu0YNH+Ll9mwwXNLo+Y2ymMnnaYwsWuP3AjTRjWQhhP0Ev6Fhx9F5md0lo5iOooGV/VNFEOpbtyiIMGv5Hi+5Sz4iNLdshvYr7T3TGqr0ZTAGiFEeEr/PrYcmQny+Qt9/4sqOq+c/tgk4KiTmzOrb+Ztkwl1PifKlh0VyWJR70DxFDdl4T3We/Xcn9nbj4ZLEO0iyxPDObxlpGqORiYhNakeRXV+RM26EIKBhQIKNfYTQOJ0wYXy8mYBZUFq4TXKlYV0ap/gDtWi5poRtlZFFK9pEdCrYZD/N2TsT/rFHnCJl8vWDoRh3fodZIO/p Vq9CNPf7 cJmwM5jMHq0mE6kFOlSoWJbBKoRuTkDefVBrda43Qy8pbXnHVE/J27acSHuewwje1ZpGJ+qrKYKqsHQdTgybjSt2Jy6oFtSOpqho5o/xSijkZ9UPJXLic+Xm8JXNhtf2KEBNlbNh2GDrFjefx69Y0mYYL09LLAUJ0EVF8bj4HxWz7UvN8+GHO/PGjy9+9HNBK2/pXRCZ1rDllXV/c5frWYGUfzojjMGYfo5auHR0cjsAkbwbg6LuTtsH2KQ8X6eqo3An6 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: 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?