Re: [PATCH] mm/shmem: remove redundant entry_order variable in shmem_split_large_entry()

["Jackie Liu" <liu.yun@linux.dev>]

Hi Baolin.


2025年9月8日 14:05, "Baolin Wang" <baolin.wang@linux.alibaba.com mailto:baolin.wang@linux.alibaba.com?to=%22Baolin%20Wang%22%20%3Cbaolin.wang%40linux.alibaba.com%3E > 写到:


> 
> On 2025/9/8 10:39, Jackie Liu wrote:
> 
> > 
> > From: Jackie Liu <liuyun01@kylinos.cn>
> >  Commit d53c78fffe7a ("mm/shmem: use xas_try_split() in shmem_split_large_entry()")
> >  reworked shmem_split_large_entry() to use xas_try_split() and simplified
> >  the splitting logic. After that change, the local variable `entry_order`
> >  became redundant since its value was always immediately copied to `cur_order`.
> >  Remove `entry_order` and use `cur_order` directly to simplify the code.
> > 
> The commit message is a bit misleading. It does not introduce a redundant variable by commit d53c78fffe7a. Instead, it's because after commit 93c0476e7057 ("mm/shmem, swap: rework swap entry and index calculation for large swapin"), the 'entry_order' will never be returned.
> 

Aha, Thanks, you’re right. This is indeed the correct commit. When I applied this series of patches locally,
I accidentally used an earlier prerequisite patch. I made a mistake when copying it. 
Thanks again—I’ll update the comment and resend version 2. 

-- 
Jackie Liu

> > 
> > No functional change intended.
> >  Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >  ---
> > 
> With the commit message updated:
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> 
> > 
> > mm/shmem.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >  diff --git a/mm/shmem.c b/mm/shmem.c
> >  index e2c76a30802b..53d28ea6e52b 100644
> >  --- a/mm/shmem.c
> >  +++ b/mm/shmem.c
> >  @@ -2235,7 +2235,7 @@ static int shmem_split_large_entry(struct inode *inode, pgoff_t index,
> >  {
> >  struct address_space *mapping = inode->i_mapping;
> >  XA_STATE_ORDER(xas, &mapping->i_pages, index, 0);
> >  - int split_order = 0, entry_order;
> >  + int split_order = 0;
> >  int i;
> >  > /* Convert user data gfp flags to xarray node gfp flags */
> >  @@ -2253,15 +2253,12 @@ static int shmem_split_large_entry(struct inode *inode, pgoff_t index,
> >  goto unlock;
> >  }
> >  > - entry_order = xas_get_order(&xas);
> >  -
> >  - if (!entry_order)
> >  + cur_order = xas_get_order(&xas);
> >  + if (!cur_order)
> >  goto unlock;
> >  > /* Try to split large swap entry in pagecache */
> >  - cur_order = entry_order;
> >  - swap_index = round_down(index, 1 << entry_order);
> >  -
> >  + swap_index = round_down(index, 1 << cur_order);
> >  split_order = xas_try_split_min_order(cur_order);
> >  > while (cur_order > 0) {
> >
>