From: Matthew Wilcox <willy@infradead.org>
To: Jung-JaeJoon <rgbi3307@gmail.com>
Cc: "Liam R. Howlett" <Liam.Howlett@oracle.com>,
Andrew Morton <akpm@linux-foundation.org>,
Peng Zhang <zhangpeng.00@bytedance.com>,
maple-tree@lists.infradead.org, linux-mm@kvack.org,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Modified XArray entry bit flags as macro constants
Date: Fri, 24 May 2024 04:05:49 +0100 [thread overview]
Message-ID: <ZlAEDbOR6Ch-Y__C@casper.infradead.org> (raw)
In-Reply-To: <20240524024945.9309-1-rgbi3307@naver.com>
On Fri, May 24, 2024 at 11:49:45AM +0900, Jung-JaeJoon wrote:
> From: Jung-JaeJoon <rgbi3307@gmail.com>
>
> It would be better to modify the operation on the last two bits of the entry
> with a macro constant name rather than using a numeric constant.
>
> #define XA_VALUE_ENTRY 1UL
> #define XA_INTERNAL_ENTRY 2UL
> #define XA_POINTER_ENTRY 3UL
>
> In particular, in the xa_to_node() function, it is more consistent and efficient
> to perform a logical AND operation as shown below than a subtraction operation.
>
> - return (struct xa_node *)((unsigned long)entry - 2);
> + return (struct xa_node *)((unsigned long)entry & ~XA_INTERNAL_ENTRY);
>
> Additionally, it is better to modify the if condition below
> in the mas_store_root() function of lib/maple_tree.c to the xa_is_internal() inline function.
>
> - else if (((unsigned long) (entry) & 3) == 2)
> + else if (xa_is_internal(entry))
>
> And there is no reason to declare XA_CHECK_SCHED as an enum data type.
> -enum {
> - XA_CHECK_SCHED = 4096,
> -};
> +#define XA_CHECK_SCHED 4096
Thank you for your patch. I agree with none of this. Rejected.
prev parent reply other threads:[~2024-05-24 3:06 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-24 2:49 [PATCH] Modified XArray entry bit flags as macro constants Jung-JaeJoon
2024-05-24 3:05 ` Matthew Wilcox [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZlAEDbOR6Ch-Y__C@casper.infradead.org \
--to=willy@infradead.org \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=maple-tree@lists.infradead.org \
--cc=rgbi3307@gmail.com \
--cc=zhangpeng.00@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.