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 02DAEC02198 for ; Wed, 12 Feb 2025 09:06:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 674CC28000E; Wed, 12 Feb 2025 04:06:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 62571280009; Wed, 12 Feb 2025 04:06:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5144C28000E; Wed, 12 Feb 2025 04:06:25 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 34CC9280009 for ; Wed, 12 Feb 2025 04:06:25 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 48180C155A for ; Wed, 12 Feb 2025 09:06:24 +0000 (UTC) X-FDA: 83110711488.19.42396CA Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf26.hostedemail.com (Postfix) with ESMTP id 73003140006 for ; Wed, 12 Feb 2025 09:06:20 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=none; spf=pass (imf26.hostedemail.com: domain of dev.jain@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=dev.jain@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=1739351182; 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=CXzLQF6jFzMQOjFFkKy/j9mlEx7hzq+FJaN5LBvh0oU=; b=U5c72dPkPzMc/elzDxX97A+c5I5X+olwdZwhq/fYrl6InbjGxRtpPFUE1dRQHmQGkPnwDj R7NZQTxXigKgQLjppdahs1YrTUgMEB7VEEz13xu18om/N5emtqPVhwauzSifRwBwWpvDdI iKyQDQ6o+SAuDrp5OuEchs7HIhApjP0= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=none; spf=pass (imf26.hostedemail.com: domain of dev.jain@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=dev.jain@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1739351182; a=rsa-sha256; cv=none; b=EJOBZtbDealrJY6H1x+hIuc9lJ2iUnslxaX1A8pFXmxIKzK8djF6cr+0B/G89dtg2H3sC9 taRcSWPKhr6+BhsVy5D20tSmHCIpgZplUQOC0ErySqGGU6IjsPI2FnLT4PpbvZg4RZwDAN YSWR5dI+v+RUgfygQ1nBLldRyJNUvdY= 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 8F1EE13D5; Wed, 12 Feb 2025 01:06:40 -0800 (PST) Received: from [10.162.43.26] (K4MQJ0H1H2.blr.arm.com [10.162.43.26]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 548DF3F58B; Wed, 12 Feb 2025 01:06:17 -0800 (PST) Message-ID: <27db405d-666a-4064-b13c-9f1c81b8512a@arm.com> Date: Wed, 12 Feb 2025 14:36:13 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/2] mm/mm.h: Write folio->_flags_1 & 0xff as a macro definition To: liuye , brauner@kernel.org, dhowells@redhat.com, akpm@linux-foundation.org Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20250212025843.80283-1-liuye@kylinos.cn> <20250212025843.80283-3-liuye@kylinos.cn> <1739340112672653.3.seg@mailgw.kylinos.cn> <52fcd6b2-bbe1-4de7-85d1-1e5968f87e0d@kylinos.cn> Content-Language: en-US From: Dev Jain In-Reply-To: <52fcd6b2-bbe1-4de7-85d1-1e5968f87e0d@kylinos.cn> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 73003140006 X-Stat-Signature: fqrwfno5pmmu1xzcpmrbuxjkb3uwjz4r X-HE-Tag: 1739351180-887945 X-HE-Meta: U2FsdGVkX1/b6QQmPBzedr0v8XdxOohOeqm6NIDD1TZQPLoQuPqMXgNuPl7JUzVIawbk1IthoGfLjjoi56uROh+YU1WuEs0lb9KH8c0eCfAXtgxJ26Mw4LjS/Le7r6v7UPvc93ojCN+HlZ/Mp7+K8zljJ6xljFkgDafoXw5Nucvp1dCsHWSro6VVoU+qN7lxDqocoON5j+7CqDUQ1zFXbHZkYSQt25VL6KRWu49O0KVEfzXSuGx4JSqUKIz2xeCxSyES7/W2jCHtx+MntbY1ECrq40T4WIKbC3hsHSTVS+VAF8OabNLsyJnw62+mBTvtPlgKh0BG965rl0+HMBil8ej/QnGykgjobBb80W+LVtkVzzflPGlaW7YtokrrB/Jv/lpfiJrH3yWkVtyGhgqBdrPh6UqMMGQrVLg84ug2ernMQ+RMC2YEAc6Twavip/QQEg0dFAjItA3tDSx/eJlDYomOaN5YCSXPCd2SQ4w/099rjyv3k0ZrFzRSukZOF8WLDg51A2+V0vk0hZi05OQaKnr7S/ovhiLitv3EScNhI+ZbiSP/bu9cgwkZoieaWg2ldsnjiGAKHO6Cam27PjFgOGjjBnhdfCNyIaJDdZUMjJqP+tbgTaU4Kz80ot1OPHUju+zpKH/v6h7hNh2CABN4eJnUneyOmvcEgBlRgZDYox23KzXOZ9tFIYd51j47JGHQUyjQaYhZ49d24aWXEYweSO4H1gswK7WLr/tu2eLaAuU9qTyhSE/i7234w4fRZIGvzxbA7AL6+YJkByG/W9K1KZvKG1jF4+tZeNWFMaouTuhD+VytqDq8E0YbAOegx3S6dOAZUEH+whSiv1c7XiQJnUjCjCf/VL2EV2WH3mV9TuHHKfndMtVonMZ+//SFJwhvH6iFGDgbBlXYg4+Lc5XVROFDS/jqyQy1w2/AHo0AfSIX2u+0NT2wbtlDRlfh1UnY4CQyj/5KLgO3VpTGK07 dOWzyIct orpbbT0jZ4UI3WyRnv0WZEmEIpmFJ7itbflvw5AP4flprWB0i3DJo9yUwx86IuKNJBhjkcqmXaCSeUlNkasD31qnugB8eUllT+fV67kEa/KTmEPRIWnEzpt/L4U4Hcr6VggqzC9k1JrMnFwfclYsWcFONYcVXA+WrWnawRwGW4gDC1vkpqD4pM+mv6+oU9uSPvVM6CtS/4reDTK0JGG4KcF71Vg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000237, 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 12/02/25 12:37 pm, liuye wrote: > > > 在 2025/2/12 13:12, Dev Jain 写道: >> >> >> On 12/02/25 8:28 am, Liu Ye wrote: >>> There are multiple locations in mm.h where (folio->_flags_1 & 0xff) is >>> used. Write it as a macro definition to improve the readability and >>> maintainability of the code. >>> >>> Signed-off-by: Liu Ye >>> --- >>>   include/linux/mm.h | 10 ++++++---- >>>   1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/include/linux/mm.h b/include/linux/mm.h >>> index 7b1068ddcbb7..750e75f45557 100644 >>> --- a/include/linux/mm.h >>> +++ b/include/linux/mm.h >>> @@ -1098,6 +1098,8 @@ int vma_is_stack_for_current(struct vm_area_struct *vma); >>>   struct mmu_gather; >>>   struct inode; >>>   +#define FOLIO_ORDER(folio) ((folio)->_flags_1 & 0xff) >>> + >>>   /* >>>    * compound_order() can be called without holding a reference, which means >>>    * that niceties like page_folio() don't work.  These callers should be >>> @@ -1111,7 +1113,7 @@ static inline unsigned int compound_order(struct page *page) >>>         if (!test_bit(PG_head, &folio->flags)) >>>           return 0; >>> -    return folio->_flags_1 & 0xff; >>> +    return FOLIO_ORDER(folio); >>>   } >>>     /** >>> @@ -1127,7 +1129,7 @@ static inline unsigned int folio_order(const struct folio *folio) >>>   { >>>       if (!folio_test_large(folio)) >>>           return 0; >>> -    return folio->_flags_1 & 0xff; >>> +    return FOLIO_ORDER(folio); >>>   } >>>     #include >>> @@ -2061,7 +2063,7 @@ static inline long folio_nr_pages(const struct folio *folio) >>>   #ifdef CONFIG_64BIT >>>       return folio->_folio_nr_pages; >>>   #else >>> -    return 1L << (folio->_flags_1 & 0xff); >>> +    return 1L << FOLIO_ORDER(folio); >>>   #endif >>>   } >>>   @@ -2086,7 +2088,7 @@ static inline unsigned long compound_nr(struct page *page) >>>   #ifdef CONFIG_64BIT >>>       return folio->_folio_nr_pages; >>>   #else >>> -    return 1L << (folio->_flags_1 & 0xff); >>> +    return 1L << FOLIO_ORDER(folio); >>>   #endif >>>   } >>> >> >> Personally I do not think this is improving readability. You are introducing one more macro for people to decipher instead of directly seeing folio->_flags_1 & 0xff. This is similar to whether to write >> if (x) => do_stuff(), or if (x != 0) => do_stuff(). The former is more "readable" by convention but the latter makes it easier and obvious to understand. >> > Or simply for maintenance purposes, if the meaning of a bit changes, only the macro definition needs to be modified. Well, then let us wait for that time to come :) Personally I am not a fan of over-abstracting, especially when it is just a single line; one benefit I have seen of writing the way it is written right now, is that I actually get reminded where the folio order is actually stored. I have no objection on getting this patch applied, if someone else thinks this is a fruitful abstraction. In any case, you do need to come up with a better name than FOLIO_ORDER, as it is confusing. > > Thanks， > Liu Ye > > >