| Message ID | 20210511134857.1581273-3-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup and fixup for huge_memory | expand |
On Tue, May 11, 2021 at 09:48:54PM +0800, Miaohe Lin wrote: > Now that we can represent the location of ->deferred_list instead of > ->mapping + ->index, make use of it to improve readability. > > Reviewed-by: Yang Shi <shy828301@gmail.com> > Reviewed-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/huge_memory.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index 63ed6b25deaa..76ca1eb2a223 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -2868,7 +2868,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink, > spin_lock_irqsave(&ds_queue->split_queue_lock, flags); > /* Take pin on all head pages to avoid freeing them under us */ > list_for_each_safe(pos, next, &ds_queue->split_queue) { > - page = list_entry((void *)pos, struct page, mapping); > + page = list_entry((void *)pos, struct page, deferred_list); > page = compound_head(page); This is an equivalent transformation, but it doesn't really go far enough. I think you want something like this: struct page *page, *next; list_for_each_entry_safe(page, next, &ds_queue->split_queue, deferred_list) { struct page *head = page - 1; ... then use head throughout ... }
On 2021/5/12 7:03, Matthew Wilcox wrote: > On Tue, May 11, 2021 at 09:48:54PM +0800, Miaohe Lin wrote: >> Now that we can represent the location of ->deferred_list instead of >> ->mapping + ->index, make use of it to improve readability. >> >> Reviewed-by: Yang Shi <shy828301@gmail.com> >> Reviewed-by: David Hildenbrand <david@redhat.com> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/huge_memory.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> index 63ed6b25deaa..76ca1eb2a223 100644 >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -2868,7 +2868,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink, >> spin_lock_irqsave(&ds_queue->split_queue_lock, flags); >> /* Take pin on all head pages to avoid freeing them under us */ >> list_for_each_safe(pos, next, &ds_queue->split_queue) { >> - page = list_entry((void *)pos, struct page, mapping); >> + page = list_entry((void *)pos, struct page, deferred_list); >> page = compound_head(page); > > This is an equivalent transformation, but it doesn't really go far > enough. I think you want something like this: > > struct page *page, *next; > > list_for_each_entry_safe(page, next, &ds_queue->split_queue, > deferred_list) { > struct page *head = page - 1; > ... then use head throughout ... > } > Many thanks for your time and reminder. list_for_each_entry_safe is equivalent to list_for_each_safe + list_entry and there is many places using list_for_each_safe + list_entry, so I think it's ok to keep the code as it is. Thanks again. :) > . >
diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 63ed6b25deaa..76ca1eb2a223 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2868,7 +2868,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink, spin_lock_irqsave(&ds_queue->split_queue_lock, flags); /* Take pin on all head pages to avoid freeing them under us */ list_for_each_safe(pos, next, &ds_queue->split_queue) { - page = list_entry((void *)pos, struct page, mapping); + page = list_entry((void *)pos, struct page, deferred_list); page = compound_head(page); if (get_page_unless_zero(page)) { list_move(page_deferred_list(page), &list); @@ -2883,7 +2883,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink, spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags); list_for_each_safe(pos, next, &list) { - page = list_entry((void *)pos, struct page, mapping); + page = list_entry((void *)pos, struct page, deferred_list); if (!trylock_page(page)) goto next; /* split_huge_page() removes page from list on success */