Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <1564080768.11067.22.camel@lca.pw>
Subject: Re: [PATCH 00/10] make "order" unsigned int
From:   Qian Cai <cai@lca.pw>
To:     Pengfei Li <lpf.vector@gmail.com>, akpm@linux-foundation.org
Cc:     mgorman@techsingularity.net, mhocko@suse.com, vbabka@suse.cz,
        aryabinin@virtuozzo.com, osalvador@suse.de, rostedt@goodmis.org,
        mingo@redhat.com, pavel.tatashin@microsoft.com, rppt@linux.ibm.com,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org
Date:   Thu, 25 Jul 2019 14:52:48 -0400
In-Reply-To: <20190725184253.21160-1-lpf.vector@gmail.com>
References: <20190725184253.21160-1-lpf.vector@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 2019-07-26 at 02:42 +0800, Pengfei Li wrote:
> Objective
> ----
> The motivation for this series of patches is use unsigned int for
> "order" in compaction.c, just like in other memory subsystems.

I suppose you will need more justification for this change. Right now, I don't
see much real benefit apart from possibly introducing more regressions in those
tricky areas of the code. Also, your testing seems quite lightweight.

> 
> In addition, did some cleanup about "order" in page_alloc
> and vmscan.
> 
> 
> Description
> ----
> Directly modifying the type of "order" to unsigned int is ok in most
> places, because "order" is always non-negative.
> 
> But there are two places that are special, one is next_search_order()
> and the other is compact_node().
> 
> For next_search_order(), order may be negative. It can be avoided by
> some modifications.
> 
> For compact_node(), order = -1 means performing manual compaction.
> It can be avoided by specifying order = MAX_ORDER.
> 
> Key changes in [PATCH 05/10] mm/compaction: make "order" and
> "search_order" unsigned.
> 
> More information can be obtained from commit messages.
> 
> 
> Test
> ----
> I have done some stress testing locally and have not found any problems.
> 
> In addition, local tests indicate no performance impact.
> 
> 
> Pengfei Li (10):
>   mm/page_alloc: use unsigned int for "order" in should_compact_retry()
>   mm/page_alloc: use unsigned int for "order" in __rmqueue_fallback()
>   mm/page_alloc: use unsigned int for "order" in should_compact_retry()
>   mm/page_alloc: remove never used "order" in alloc_contig_range()
>   mm/compaction: make "order" and "search_order" unsigned int in struct
>     compact_control
>   mm/compaction: make "order" unsigned int in compaction.c
>   trace/events/compaction: make "order" unsigned int
>   mm/compaction: use unsigned int for "compact_order_failed" in struct
>     zone
>   mm/compaction: use unsigned int for "kcompactd_max_order" in struct
>     pglist_data
>   mm/vmscan: use unsigned int for "kswapd_order" in struct pglist_data
> 
>  include/linux/compaction.h        |  30 +++----
>  include/linux/mmzone.h            |   8 +-
>  include/trace/events/compaction.h |  40 +++++-----
>  include/trace/events/kmem.h       |   6 +-
>  include/trace/events/oom.h        |   6 +-
>  include/trace/events/vmscan.h     |   4 +-
>  mm/compaction.c                   | 127 +++++++++++++++---------------
>  mm/internal.h                     |   6 +-
>  mm/page_alloc.c                   |  16 ++--
>  mm/vmscan.c                       |   6 +-
>  10 files changed, 126 insertions(+), 123 deletions(-)
>