mbox series

[0/4] mm: migrate: more folio conversion

Message ID 20230802095346.87449-1-wangkefeng.wang@huawei.com
Headers
Series mm: migrate: more folio conversion |

Message

Kefeng Wang Aug. 2, 2023, 9:53 a.m. UTC 
  This patch series converts several functions to use folio in migrate.c.

Kefeng Wang (4):
  mm: migrate: use a folio in add_page_for_migration()
  mm: migrate: convert numamigrate_isolate_page() to
    numamigrate_isolate_folio()
  mm: migrate: make migrate_misplaced_page() to take a folio
  mm: migrate: use __folio_test_movable()

 mm/migrate.c | 101 +++++++++++++++++++++++++--------------------------
 1 file changed, 50 insertions(+), 51 deletions(-)

Comments

David Hildenbrand Aug. 2, 2023, 11:47 a.m. UTC | #1 
On 02.08.23 11:53, Kefeng Wang wrote:
> This patch series converts several functions to use folio in migrate.c.

Please clearly spell out in the patch descriptions when you are changing 
mapcount logic and which effects that might have (both, positive and 
negative) for PTE-mapped THP.
Kefeng Wang Aug. 2, 2023, 12:38 p.m. UTC | #2 
On 2023/8/2 19:47, David Hildenbrand wrote:
> On 02.08.23 11:53, Kefeng Wang wrote:
>> This patch series converts several functions to use folio in migrate.c.
> 
> Please clearly spell out in the patch descriptions when you are changing 
> mapcount logic and which effects that might have (both, positive and 
> negative) for PTE-mapped THP.

Oh, I see your comments on another mail[1], the folio_estimated_sharers()
is not good to evaluate the sharing by multiple processes, will read more
history of the mail's discussion, maybe just ignore the first three patches.

Thank.


[1] [PATCH 0/2] don't use mapcount() to check large folio sharing
>
David Hildenbrand Aug. 3, 2023, 9:34 a.m. UTC | #3 
On 02.08.23 14:38, Kefeng Wang wrote:
> 
> 
> On 2023/8/2 19:47, David Hildenbrand wrote:
>> On 02.08.23 11:53, Kefeng Wang wrote:
>>> This patch series converts several functions to use folio in migrate.c.
>>
>> Please clearly spell out in the patch descriptions when you are changing
>> mapcount logic and which effects that might have (both, positive and
>> negative) for PTE-mapped THP.
> 
> Oh, I see your comments on another mail[1], the folio_estimated_sharers()
> is not good to evaluate the sharing by multiple processes, will read more
> history of the mail's discussion, maybe just ignore the first three patches.

It might be good enough for some cases right now. My point is that we 
better just clearly spell out the possible effects of such a change.

(so far you didn't even mention them, and that's sub-optimal when buried 
in a "folio" conversion that better shouldn't change the functionality 
without spelling it out)