From: Brendan Jackman <jackmanb@google.com>
To: Vlastimil Babka <vbabka@suse.cz>,
Andrew Morton <akpm@linux-foundation.org>
Cc: Suren Baghdasaryan <surenb@google.com>,
Michal Hocko <mhocko@suse.com>,
Johannes Weiner <hannes@cmpxchg.org>, Zi Yan <ziy@nvidia.com>,
<linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm/page_alloc: Harmonize should_compact_retry() type
Date: Fri, 29 Aug 2025 11:20:36 +0000 [thread overview]
Message-ID: <DCEVDXL7WZYH.320PPWEYJI0NN@google.com> (raw)
In-Reply-To: <a6571b18-a63a-4e9f-b911-7cc10c72a4c1@suse.cz>
On Thu Aug 28, 2025 at 6:31 PM UTC, Vlastimil Babka wrote:
> On 8/27/25 17:30, Brendan Jackman wrote:
>> On Wed Aug 27, 2025 at 2:13 AM UTC, Andrew Morton wrote:
>>> On Tue, 26 Aug 2025 14:06:54 +0000 Brendan Jackman <jackmanb@google.com> wrote:
>>>
>>>> Currently order is signed in one version of the function and unsigned in
>>>> the other. Tidy that up.
>>>>
>>>> In page_alloc.c, order is unsigned in the vast majority of cases. But,
>>>> there is a cluster of exceptions in compaction-related code (probably
>>>> stemming from the fact that compact_control.order is signed). So, prefer
>>>> local consistency and make this one signed too.
>>>>
>>>
>>> grumble, pet peeve. Negative orders make no sense. Can we make
>>> cc->order unsigned in order (heh) to make everything nice?
>>
>> I think we can't "just" do that:
>
> That part should be easy to convert to a compact_control flag.
> Zi's point about going negative seems like more prone to overlook some case.
> But worth trying and the cleanups I'd say.
OK, I can take a look next week. From a quick glance it does seem to be
worth having a sniff, there could be bugs in there where code is
expecting non-negative and doing stuff like using it as a shift index,
in cases where it can actually be negative.
prev parent reply other threads:[~2025-08-29 11:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-26 14:06 [PATCH] mm/page_alloc: Harmonize should_compact_retry() type Brendan Jackman
2025-08-26 21:15 ` Zi Yan
2025-08-27 2:13 ` Andrew Morton
2025-08-27 2:29 ` Zi Yan
2025-08-27 15:30 ` Brendan Jackman
2025-08-28 18:31 ` Vlastimil Babka
2025-08-29 11:20 ` Brendan Jackman [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=DCEVDXL7WZYH.320PPWEYJI0NN@google.com \
--to=jackmanb@google.com \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=surenb@google.com \
--cc=vbabka@suse.cz \
--cc=ziy@nvidia.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.