On 18.08.20 05:05, Wei Yang wrote: > On Mon, Aug 17, 2020 at 07:07:04PM +0200, David Hildenbrand wrote: >> On 17.08.20 18:05, Alexander Duyck wrote: >>> >>> >>> On 8/17/2020 2:35 AM, David Hildenbrand wrote: >>>> On 17.08.20 10:48, Wei Yang wrote: >>>>> If "page" is the list head, list_for_each_entry_safe() would stop >>>>> iteration. >>>>> >>>>> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxxxxxxxxxx> >>>>> --- >>>>> mm/page_reporting.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/mm/page_reporting.c b/mm/page_reporting.c >>>>> index 3bbd471cfc81..aaaa3605123d 100644 >>>>> --- a/mm/page_reporting.c >>>>> +++ b/mm/page_reporting.c >>>>> @@ -178,7 +178,7 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone, >>>>> * the new head of the free list before we release the >>>>> * zone lock. >>>>> */ >>>>> - if (&page->lru != list && !list_is_first(&page->lru, list)) >>>>> + if (!list_is_first(&page->lru, list)) >>>>> list_rotate_to_front(&page->lru, list); >>>>> >>>>> /* release lock before waiting on report processing */ >>>>> >>>> >>>> Is this a fix or a cleanup? If it's a fix, can this be reproduced easily >>>> and what ere the effects? >>>> >>> >>> This should be a clean-up. Since the &page->lru != list will always be true. >>> >> >> Makes sense, maybe we can make that a little bit clearer in the patch >> description. >> > > Ok, do you have some suggestion on the description? > > A clean-up for commit xxx? > > I would appreciate your suggestion :-) > I'd go with something like " mm/page_reporting: drop stale list head check in page_reporting_cycle list_for_each_entry_safe() guarantees that we will never stumble over the list head; "&page->lru != list" will always evaluate to true. Let's simplify. " to stress that this is a pure simplifcation. Reviewed-by: David Hildenbrand <david@xxxxxxxxxx> >>> If I recall at some point the that was a check for &next->lru != list >>> but I think I pulled out an additional conditional check somewhere so >>> that we just go through the start of the loop again and iterate over >>> reported pages until we are guaranteed to have a non-reported page to >>> rotate to the top of the list with the general idea being that we wanted >>> the allocator to pull non-reported pages before reported pages. >> >> -- >> Thanks, >> >> David / dhildenb > -- Thanks, David / dhildenb
