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 437CDC3DA42 for ; Sat, 13 Jul 2024 11:00:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BBDF36B0085; Sat, 13 Jul 2024 07:00:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B6D306B0088; Sat, 13 Jul 2024 07:00:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A35386B0089; Sat, 13 Jul 2024 07:00:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 8351F6B0085 for ; Sat, 13 Jul 2024 07:00:23 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 2BFBC140E60 for ; Sat, 13 Jul 2024 11:00:23 +0000 (UTC) X-FDA: 82334435526.08.BFF0A0E Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf05.hostedemail.com (Postfix) with ESMTP id 4430A10002F for ; Sat, 13 Jul 2024 11:00:21 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=none; spf=pass (imf05.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720868403; a=rsa-sha256; cv=none; b=DbMfI7MpvAO42PTcZEZUtngj3ygUiIwilVCuf9gon5wdLwEPNKr/5ImhngkSNbzv80E6WQ b79hLTzOIKdVmT/tWTAaIiD/bEFKYTn2m/xwumsoZQipZrz2ozdEyMYQwHxCzIvAZvV5xD YCnmVmIht73BUdvN4UEQIb7yJ2DRgjo= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=none; spf=pass (imf05.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720868403; 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; bh=TT6P5BMOxuyhKXwYTH8VUu+XfORYrqzP01+OeP+43CE=; b=RPnTMTJXe6VTgJ1sYUTKag1hpEyuEARIApX7uxd6vytcM5LnN2m0G7dkqGh8zaCy64dwAI ++gDpvwq7poQfWlzlpHe6teTTa1JDl8ruZjNrM9oVAlqXsqDNY5URfGVuxSHXUYsE9jRel /C16ZbRAYqac1T9BmrARrm8zXBnmhkQ= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BE9461007; Sat, 13 Jul 2024 04:00:45 -0700 (PDT) Received: from [10.57.77.253] (unknown [10.57.77.253]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 129FD3F762; Sat, 13 Jul 2024 04:00:18 -0700 (PDT) Message-ID: <29f0fc5a-c2b7-4925-9bdb-fd2abe5383ae@arm.com> Date: Sat, 13 Jul 2024 12:00:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 2/2] mm: mTHP stats for pagecache folio allocations Content-Language: en-GB To: Baolin Wang , 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> From: Ryan Roberts In-Reply-To: <9e0d84e5-2319-4425-9760-2c6bb23fc390@linux.alibaba.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Stat-Signature: kcnsjoawdxogt69u6n1pjp1oye8mx8md X-Rspamd-Queue-Id: 4430A10002F X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1720868421-456214 X-HE-Meta: U2FsdGVkX19hfwUB99OCWhPGehv3oJnTLtIGGIjNw/ijZCxGH38adL/Rnluqk/i4C4Iv1gRerDJMS2ve49cYz618QlQhMcDLAT+2GLmb5ZCErpbKLD/McFPh0N756LvePeKuMeqvzsafqfLpGWon0Q5WmFx5yTNQVqKmBXOQouV6md31UE2/c5kSR+bKS096dKaMIDaG5Ze0/9j1V2EZd+rINMrQyMkeV3yBDMINb2wK00Gjpk4FFZZtvgGoEuqnpmypbn+auXw0MY0iKvs2BETxe3Q+supWb4bGrH374jNTEgUnh6eU1Xs6rEsbx6uuNgY1vEtjEsc1SsLnzZeLiX8wWgpm1FRQF5feo5BZJWwDiqurwBFZo6cmCtr6XasM5o+Fyp8MaKp21yqC3sXyzwcXqlMbOJL3xMf0JrR7Fkqd2Qz71zjqX3whH/DE66dtgCjkNN4ygMH+aM6/k0VIeTtO9QKWAOzGiE0hgO5uxmGdF0OsuiDShc9JhY0bLhIyz5VwN4fEbDrsnGTPxbu2O1GJjYUyp2dW1UPIDGlAeB10h7joUBjMSNI04Shl2lti5ABdW5Pvr4ihU88kU7Pa05ZYd5G8CW/CSmYWmCzzJ0Mf2tSJ2Iq2Aki9YrS2HsiCuQ+7h+Ql10E0SUHVNaxgzWMwiuzsFXv/NC+xMtjOEySTk/3OrXTAcI6sNtS7JOQwLLqeQxdmlg5nARR7uJoDUwKm+6ZIKwC9H4gepi4csnY3Vb15to609PaSL0J45g6cGZcZuzBjDKdJe0dXuYMbjnrqzu5+hTM1RplUldMMgQKdzVMXpPKPwnTeDjT88NdikBU8B9CYOvUcwyvsQ9NfPKWMQmqi9nk4GOGjv1kFO/FQzG+fY0WSQsZrUhO/emjiPsc44qrZAKaGVTxukTLa7GtPG5UPKFbUv+Z0GENMzkAMpeZwIoMXDvyoPpx7r+/T1Z5HzSig10MDhYsaUd3 keGtA5+9 cWQfFRPz0dYjt0wJ6rnS9Pm14xgbvNdkXEQuMQmzRWINbkO57KzbBHZtqDfQa7BWZCRWXWl+BQEnSD21xOBYp3yEAuE4ujVaQXD8BVHtYYQU3uwfTGVV7B4ntX0vewZa+uHnjZTSHrm+Bf3Uiiz0E7w7s9heZWP2BnyZd7Mcp836Qn2k= 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: [...] >> +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? > >> +    kobject_put(&stats->kobj); >> +err: >> +    return ret; >>   }