Re: [PATCH v3 04/18] mm/huge_memory: split the routine for splitting anon and file folio

[David Hildenbrand (Arm)]

> 
>>> +      * place. Keep the head @folio frozen until the end: sub entries
>>> +      * in swap cache must be updated first, so a concurrent
>>> +      * swap_cache_get_folio() cannot return the head folio for a sub
>>> +      * entry (folio_try_get() will fail on the head @folio until unfreeze).
>>> +      */
>>> +     for (new_folio = folio_next(folio); new_folio != end_folio;
>>> +          new_folio = next) {
>>> +             next = folio_next(new_folio);
>>> +             zone_device_private_split_cb(folio, new_folio);
>>> +             folio_ref_unfreeze(new_folio,
>>> +                                folio_cache_ref_count(new_folio) + 1);
>>> +             if (do_lru)
>>> +                     lru_add_split_folio(folio, new_folio, lruvec, list);
>>> +             if (ci)
>>> +                     __swap_cache_replace_folio(ci, folio, new_folio);
>>> +     }
>>
>> This smells like duplicate code now. That should better be factored out?
>>
> 
> There were some discussion on this in previous series, I tried to
> implement a macro or helper, but it ended up being uglier. Right now
> the duplication is just about 3 lines of for loop due to the split,
> and the extra loop duplication existed before, so I think it's fine at
> this point, maybe a helper later if this grows.

Actually I wanted to drop this comment after digging into the code, but forgot.
So it's fine with me.

-- 
Cheers,

David