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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4BA47C433EF for ; Mon, 21 Feb 2022 02:53:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344099AbiBUCxt (ORCPT ); Sun, 20 Feb 2022 21:53:49 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:57274 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S245743AbiBUCxo (ORCPT ); Sun, 20 Feb 2022 21:53:44 -0500 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 43B4531DD3 for ; Sun, 20 Feb 2022 18:53:21 -0800 (PST) Received: from canpemm500002.china.huawei.com (unknown [172.30.72.56]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4K26K53H4fzdZLM; Mon, 21 Feb 2022 10:52:09 +0800 (CST) Received: from [10.174.177.76] (10.174.177.76) by canpemm500002.china.huawei.com (7.192.104.244) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.21; Mon, 21 Feb 2022 10:53:18 +0800 Subject: Re: [PATCH 6/9] mm/z3fold: move decrement of pool->pages_nr into __release_z3fold_page() To: David Laight CC: "vitaly.wool@konsulko.com" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "akpm@linux-foundation.org" References: <20220219092533.12596-1-linmiaohe@huawei.com> <20220219092533.12596-7-linmiaohe@huawei.com> From: Miaohe Lin Message-ID: Date: Mon, 21 Feb 2022 10:53:18 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.177.76] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To canpemm500002.china.huawei.com (7.192.104.244) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2022/2/20 0:33, David Laight wrote: > From: Miaohe Lin >> Sent: 19 February 2022 09:26 >> >> The z3fold will always do atomic64_dec(&pool->pages_nr) when the >> __release_z3fold_page() is called. Thus we can move decrement of >> pool->pages_nr into __release_z3fold_page() to simplify the code. >> Also we can reduce the size of z3fold.o ~1k. >> Without this patch: >> text data bss dec hex filename >> 15444 1376 8 16828 41bc mm/z3fold.o >> With this patch: >> text data bss dec hex filename >> 15044 1248 8 16300 3fac mm/z3fold.o > > I can't see anything obvious in this patch that would reduce the size much. > OTOH there are some large functions that are pointlessly marked 'inline'. > Maybe the compiler made a better choice? I think so too. > Although it isn't al all obvious why the 'data' size changes. I checked the header of z3fold.o. The size of .data is unchanged while align is changed from 00003818 to 00003688. Maybe this is the reason .data size changes. Section Headers: [Nr] Name Type Address Offset Size EntSize Flags Link Info Align with this patch: [ 3] .data PROGBITS 0000000000000000 00003688 00000000000000c0 0000000000000000 WA 0 0 8 without this patch: [ 3] .data PROGBITS 0000000000000000 00003818 00000000000000c0 0000000000000000 WA 0 0 8 > >> Signed-off-by: Miaohe Lin >> --- >> mm/z3fold.c | 41 ++++++++++++----------------------------- >> 1 file changed, 12 insertions(+), 29 deletions(-) >> >> diff --git a/mm/z3fold.c b/mm/z3fold.c >> index adc0b3fa4906..18a697f6fe32 100644 >> --- a/mm/z3fold.c >> +++ b/mm/z3fold.c >> @@ -520,6 +520,8 @@ static void __release_z3fold_page(struct z3fold_header *zhdr, bool locked) >> list_add(&zhdr->buddy, &pool->stale); >> queue_work(pool->release_wq, &pool->work); >> spin_unlock(&pool->stale_lock); >> + >> + atomic64_dec(&pool->pages_nr); > > Looks like you can move the decrement inside the lock. > If you can do the same for the increment you can avoid the > expensive locked bus cycle. > atomic64_inc(&pool->pages_nr); is only done when init a new or reused z3fold_page. There is no lock around. If we hold pool->lock there, this potential gain might be nullified. Or am I miss something ? Many thanks for your review and reply. > David > >> } >> >> static void release_z3fold_page(struct kref *ref) >> @@ -737,13 +739,9 @@ static struct z3fold_header *compact_single_buddy(struct z3fold_header *zhdr) >> return new_zhdr; >> >> out_fail: >> - if (new_zhdr) { >> - if (kref_put(&new_zhdr->refcount, release_z3fold_page_locked)) >> - atomic64_dec(&pool->pages_nr); >> - else { >> - add_to_unbuddied(pool, new_zhdr); >> - z3fold_page_unlock(new_zhdr); >> - } >> + if (new_zhdr && !kref_put(&new_zhdr->refcount, release_z3fold_page_locked)) { >> + add_to_unbuddied(pool, new_zhdr); >> + z3fold_page_unlock(new_zhdr); >> } >> return NULL; >> >> @@ -816,10 +814,8 @@ static void do_compact_page(struct z3fold_header *zhdr, bool locked) >> list_del_init(&zhdr->buddy); >> spin_unlock(&pool->lock); >> >> - if (kref_put(&zhdr->refcount, release_z3fold_page_locked)) { >> - atomic64_dec(&pool->pages_nr); >> + if (kref_put(&zhdr->refcount, release_z3fold_page_locked)) >> return; >> - } >> >> if (test_bit(PAGE_STALE, &page->private) || >> test_and_set_bit(PAGE_CLAIMED, &page->private)) { >> @@ -829,9 +825,7 @@ static void do_compact_page(struct z3fold_header *zhdr, bool locked) >> >> if (!zhdr->foreign_handles && buddy_single(zhdr) && >> zhdr->mapped_count == 0 && compact_single_buddy(zhdr)) { >> - if (kref_put(&zhdr->refcount, release_z3fold_page_locked)) >> - atomic64_dec(&pool->pages_nr); >> - else { >> + if (!kref_put(&zhdr->refcount, release_z3fold_page_locked)) { >> clear_bit(PAGE_CLAIMED, &page->private); >> z3fold_page_unlock(zhdr); >> } >> @@ -1089,10 +1083,8 @@ static int z3fold_alloc(struct z3fold_pool *pool, size_t size, gfp_t gfp, >> if (zhdr) { >> bud = get_free_buddy(zhdr, chunks); >> if (bud == HEADLESS) { >> - if (kref_put(&zhdr->refcount, >> + if (!kref_put(&zhdr->refcount, >> release_z3fold_page_locked)) >> - atomic64_dec(&pool->pages_nr); >> - else >> z3fold_page_unlock(zhdr); >> pr_err("No free chunks in unbuddied\n"); >> WARN_ON(1); >> @@ -1239,10 +1231,8 @@ static void z3fold_free(struct z3fold_pool *pool, unsigned long handle) >> >> if (!page_claimed) >> free_handle(handle, zhdr); >> - if (kref_put(&zhdr->refcount, release_z3fold_page_locked_list)) { >> - atomic64_dec(&pool->pages_nr); >> + if (kref_put(&zhdr->refcount, release_z3fold_page_locked_list)) >> return; >> - } >> if (page_claimed) { >> /* the page has not been claimed by us */ >> put_z3fold_header(zhdr); >> @@ -1353,9 +1343,7 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries) >> break; >> } >> if (!z3fold_page_trylock(zhdr)) { >> - if (kref_put(&zhdr->refcount, >> - release_z3fold_page)) >> - atomic64_dec(&pool->pages_nr); >> + kref_put(&zhdr->refcount, release_z3fold_page); >> zhdr = NULL; >> continue; /* can't evict at this point */ >> } >> @@ -1366,10 +1354,8 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries) >> */ >> if (zhdr->foreign_handles || >> test_and_set_bit(PAGE_CLAIMED, &page->private)) { >> - if (kref_put(&zhdr->refcount, >> + if (!kref_put(&zhdr->refcount, >> release_z3fold_page_locked)) >> - atomic64_dec(&pool->pages_nr); >> - else >> z3fold_page_unlock(zhdr); >> zhdr = NULL; >> continue; /* can't evict such page */ >> @@ -1447,7 +1433,6 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries) >> if (kref_put(&zhdr->refcount, >> release_z3fold_page_locked)) { >> kmem_cache_free(pool->c_handle, slots); >> - atomic64_dec(&pool->pages_nr); >> return 0; >> } >> /* >> @@ -1669,10 +1654,8 @@ static void z3fold_page_putback(struct page *page) >> if (!list_empty(&zhdr->buddy)) >> list_del_init(&zhdr->buddy); >> INIT_LIST_HEAD(&page->lru); >> - if (kref_put(&zhdr->refcount, release_z3fold_page_locked)) { >> - atomic64_dec(&pool->pages_nr); >> + if (kref_put(&zhdr->refcount, release_z3fold_page_locked)) >> return; >> - } >> spin_lock(&pool->lock); >> list_add(&page->lru, &pool->lru); >> spin_unlock(&pool->lock); >> -- >> 2.23.0 > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) > > . >