Re: [PATCH v4 1/3] mm, swap: make SWAPFILE_CLUSTER runtime

[IBM]

Barry Song <baohua@xxxxxxxxxx> writes:

> On Fri, Jun 19, 2026 at 12:41 PM Ritesh Harjani (IBM)
> <ritesh.list@xxxxxxxxx> wrote:
>>
>> On PowerPC Book3S64, MMU is selected at runtime, so macros like
>> PMD_SHIFT are effectively runtime variables in the Book3S64 code. THP
>
> Not an expert on Book3S64—could you explain the runtime variables in
> more detail? Does enabling THP_SWAP on PowerPC cause any build issues?
>

yes, build issues. We cannot declare array sizes by using runtime
variables. That's what this patch series fixes.

>> swap code uses these macros to size some of its array data structures
>> based on PMD_ORDER e.g. SWAPFILE_CLUSTER macro is used for this very
>> purpose.
>> Hence this patch makes the users of SWAPFILE_CLUSTER to use this macro value at
>> runtime and also modifies swap_table and swap_memcg_table which were earlier
>> using this macro for defining the number of table entries.
>>
>> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
>> ---
>>  mm/swap_table.h |  6 ++----
>>  mm/swapfile.c   | 17 ++++++++++++-----
>>  2 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/swap_table.h b/mm/swap_table.h
>> index e6613e62f8d0..90e2a7852300 100644
>> --- a/mm/swap_table.h
>> +++ b/mm/swap_table.h
>> @@ -8,16 +8,14 @@
>>
>>  /* A typical flat array in each cluster as swap table */
>>  struct swap_table {
>> -       atomic_long_t entries[SWAPFILE_CLUSTER];
>> +       DECLARE_FLEX_ARRAY(atomic_long_t, entries);
>>  };
>>
>>  /* For storing memcg private id */
>>  struct swap_memcg_table {
>> -       unsigned short id[SWAPFILE_CLUSTER];
>> +       DECLARE_FLEX_ARRAY(unsigned short, id);
>>  };
>>
>> -#define SWP_TABLE_USE_PAGE (sizeof(struct swap_table) == PAGE_SIZE)
>> -
>>  /*
>>   * A swap table entry represents the status of a swap slot on a swap
>>   * (physical or virtual) device. The swap table in each cluster is a
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index 78b49b0658ad..4bf11c5b87eb 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -129,6 +129,8 @@ static DEFINE_PER_CPU(struct percpu_swap_cluster, percpu_swap_cluster) = {
>>         .lock = INIT_LOCAL_LOCK(),
>>  };
>>
>> +static bool swap_table_use_page __ro_after_init;
>
> Does a static key help here?

That IMO won't give much benefit, given the allocation by either kmem or
alloc pages, anyway dominates the cost. Also I believe this is exactly
the usecase where branch predictor helps signficantly and reliably given
the variable is ro_after_init.

>
> Best Regards
> Barry

Thanks Barry for looking into this.

-ritesh