All of lore.kernel.org
 help / color / mirror / Atom feed
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.

      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.