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 D86C0C4345F for ; Wed, 17 Apr 2024 13:23:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 56D2B6B0098; Wed, 17 Apr 2024 09:23:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 51D916B009A; Wed, 17 Apr 2024 09:23:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E49A6B009B; Wed, 17 Apr 2024 09:23:14 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 269DF6B0098 for ; Wed, 17 Apr 2024 09:23:14 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id D3A83C07BA for ; Wed, 17 Apr 2024 13:23:13 +0000 (UTC) X-FDA: 82019089866.08.9D0EA9C Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by imf10.hostedemail.com (Postfix) with ESMTP id 2AF58C000E for ; Wed, 17 Apr 2024 13:23:10 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf10.hostedemail.com: domain of linyunsheng@huawei.com designates 45.249.212.189 as permitted sender) smtp.mailfrom=linyunsheng@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713360191; 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=8ikg6Jmmp9+zRv7NEnyFzsbwMCgTj+/1ukwf8rAstpI=; b=imU+YvSvEdyLkV0TYPDbmVKhg8VPfDFXKP+D6bHzyog4c0UWHzPYvIjY9LP979CGgq0Qvj b2qyNf47lx0fLhQAtJiKMC++kweT5lIG7n9xbZcNhJPiv3DSWmeKS12jTiUYNQpIgPuOCm 3lPKwIbjuRP0xfM5vjfl4U5sQYykvpI= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf10.hostedemail.com: domain of linyunsheng@huawei.com designates 45.249.212.189 as permitted sender) smtp.mailfrom=linyunsheng@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713360191; a=rsa-sha256; cv=none; b=dUFZh6klBNTIM9K1adKclNM0Ar7mrdyxSRjACrdpuaVb+yVql4aOD/WqcJlL1mkxd3pnpC EVL7Rj0GiRTEtAaCXa6M3WAwP8a0INVcpJr7btcaDYRx5f8fDs4W9tCD/lclrEBxWtRKLt JKVKN3G4eHH8hrvWI9Z5FLKnsffo0KU= Received: from mail.maildlp.com (unknown [172.19.163.252]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4VKM3b4PB2zNrxM; Wed, 17 Apr 2024 21:20:43 +0800 (CST) Received: from dggpemm500005.china.huawei.com (unknown [7.185.36.74]) by mail.maildlp.com (Postfix) with ESMTPS id 794CF18007F; Wed, 17 Apr 2024 21:23:07 +0800 (CST) Received: from [10.69.30.204] (10.69.30.204) by dggpemm500005.china.huawei.com (7.185.36.74) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Wed, 17 Apr 2024 21:23:07 +0800 Subject: Re: [PATCH net-next v2 10/15] mm: page_frag: reuse existing bit field of 'va' for pagecnt_bias To: Alexander H Duyck , , , CC: , , Andrew Morton , References: <20240415131941.51153-1-linyunsheng@huawei.com> <20240415131941.51153-11-linyunsheng@huawei.com> <68d1c7d3dfcd780fa3bed0bb71e41d7fb0a8c15d.camel@gmail.com> From: Yunsheng Lin Message-ID: <292c98e5-e58e-3474-f214-44fe6a83e6c5@huawei.com> Date: Wed, 17 Apr 2024 21:23:06 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <68d1c7d3dfcd780fa3bed0bb71e41d7fb0a8c15d.camel@gmail.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.69.30.204] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggpemm500005.china.huawei.com (7.185.36.74) X-Rspam-User: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 2AF58C000E X-Stat-Signature: c6fmbxi7ewbsdmsyx65b14r3ymtbs64c X-HE-Tag: 1713360190-559342 X-HE-Meta: U2FsdGVkX1+EmqPhdWgOzwsjIS3GxYSyhoODbuweOpPV8a4LSUDpLJuBqeADcz6fSDKoKLNvKVriTR1uc7iAO+duITwuCEJCi4F6Tec8FYZt+jMnyKWlUwsZ7SQh7/mDlUvZd4QeK976FHWrJT3VnhSZiB/9kk55VSKO9KjXqqoHZFqWaViID5a2pyJ4o7QS74MQTTcYbScvIjgx+RYuZRuJhcoFmjbDQVS25e225NLrNEvoD69Rq0B+14gXIvDnURelsM8z5V8fpQPIew3abky7KrLjXC5W+dx0uZCmj660f5KW7dkOxW2AvqS4gcZdTk4WQlhrL8ho3QCPKC1XF1xEEjhes2wmVh99yCuuVlNwSQjWoWyt47gf0ILgwMRkl0CzuL2iGVPor+QIatCHn/pdPNnQJg665fAeImnKic9wHrIRV90ASbc1z8SVxq4aatjWrV7CVXYnZQIGQsXFJHNNMN+Dm9zqbSdw/a8Xp8xV18S3WkpP0n9UBj94fPXo6qDesjgm8uka8YZu/U6fjwK+q4qUYNTKjLibfDtjR45uW4lROTM7Udc4tmvo5tTpJRcsS0LJ8CRjuiTLvICA3PC668VSe2ubmaycQuEF72qsWkILSS/8iUsT01wrBtRwTEa5XnPaYaX4+Kvcm8hFKqAOBCAw5ePHPZ3C7+WvZzQ7CYCSxMpMg/ceOdIo1FXLiyRZDizDTnn1e9/6uqrph0HiApDEMaJ6MsJu6c8Wk70/tpr2y6gisMbwRnybQxmvyqLOzD4AlwPLBZGp1v0BH/hkYdCRI5EhrUS8ZppC7hkPLMknmE9LsyES+6TRcFPVDTGHsrkhFNYLnAMo7rWfWqXIXWFa/mOI1VSSgP00xfwf3K86dqHyJeNlLau/AjDdcxFXgsVJRpbgF2iYIHvDVbusQn4ndzmO7Ca/0cXtaq1RH5su9Ip1+TjWOS+CxDH2QVMbf31oJp0/c62q5yQ eEOuBGWp cR/nxh+xLIw6Q02vzpAmZjf4NQSe1IzmrH8+cdCpEYSSTIl+xG3jmOnyNP8w+5FYnUvhfp9AkhdUWBESuWk5/WTtj2euUGagf50qKIx8MfsN2hjMjc3YDPLtW0HiZiUfoP2dOCYj7C/AjAWS3brBRhmO/8qu8aWUbtRaFOOYhj4AQ1icbOZoRiFp+EwJle11Yk+GCBHY9fJvPZOzKU4ei+2BvHw9uIS0qd+WtfIYdmnpmw3PNXrvQ0Fc9M3TGVSIjyI8BIiQ3c66Duujz7KLkw+zyAA== 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/4/17 0:33, Alexander H Duyck wrote: > On Mon, 2024-04-15 at 21:19 +0800, Yunsheng Lin wrote: >> As alignment of 'va' is always aligned with the order of the >> page allocated, we can reuse the LSB bits for the pagecount >> bias, and remove the orginal space needed by 'pagecnt_bias'. >> Also limit the 'fragsz' to be at least the size of >> 'usigned int' to match the limited pagecnt_bias. >> >> Signed-off-by: Yunsheng Lin > > What is the point of this? You are trading off space for size on a data > structure that is only something like 24B in size and only allocated a > few times. As we are going to replace page_frag with page_frag_cache in patch 13, it is not going to only be allocated a few times as mentioned. > >> --- >> include/linux/page_frag_cache.h | 20 +++++++---- >> mm/page_frag_cache.c | 63 +++++++++++++++++++-------------- >> 2 files changed, 50 insertions(+), 33 deletions(-) >> >> diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h >> index 40a7d6da9ef0..a97a1ac017d6 100644 >> --- a/include/linux/page_frag_cache.h >> +++ b/include/linux/page_frag_cache.h >> @@ -9,7 +9,18 @@ >> #define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE) >> >> struct page_frag_cache { >> - void *va; >> + union { >> + void *va; >> + /* we maintain a pagecount bias, so that we dont dirty cache >> + * line containing page->_refcount every time we allocate a >> + * fragment. As 'va' is always aligned with the order of the >> + * page allocated, we can reuse the LSB bits for the pagecount >> + * bias, and its bit width happens to be indicated by the >> + * 'size_mask' below. >> + */ >> + unsigned long pagecnt_bias; >> + >> + }; > > Both va and pagecnt_bias are frequently accessed items. If pagecnt_bias > somehow ends up exceeding the alignment of the page we run the risk of > corrupting data or creating an page fault. > > In my opinion this is not worth the risk especially since with the > previous change your new change results in 0 size savings on 64b > systems as the structure will be aligned to the size of the pointer. But aren't we going to avoid a register usage and loading if reusing the lower bits of 'va' for the 64b systems? And added benefit is the memory saving for 32b systems as mentioned in previous patch. > >> #if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) >> __u16 offset; >> __u16 size_mask:15; >> @@ -18,10 +29,6 @@ struct page_frag_cache { >> __u32 offset:31; >> __u32 pfmemalloc:1; >> #endif >> - /* we maintain a pagecount bias, so that we dont dirty cache line >> - * containing page->_refcount every time we allocate a fragment. >> - */ >> - unsigned int pagecnt_bias; >> }; >> >> static inline void page_frag_cache_init(struct page_frag_cache *nc) >> @@ -56,7 +63,8 @@ static inline void *page_frag_alloc_va_align(struct page_frag_cache *nc, >> gfp_t gfp_mask, >> unsigned int align) >> { >> - WARN_ON_ONCE(!is_power_of_2(align) || align >= PAGE_SIZE); >> + WARN_ON_ONCE(!is_power_of_2(align) || align >= PAGE_SIZE || >> + fragsz < sizeof(unsigned int)); > > What is the reason for this change? Seems like it is to account for an > issue somewhere. If the fragsz is one, we might not have enough pagecnt_bias for it, as we are using the lower bits of 'va' now. > >> >> return __page_frag_alloc_va_align(nc, fragsz, gfp_mask, align);