On 2/6/2021 3:58 AM, David Rientjes wrote: >> In the code, when COMPACT_SKIPPED is being returned, the page will >> always be NULL. So, I'm not sure how much useful it is for the page == >> NULL check here. Or I failed to understand your point here? >> > Your code is short-circuiting the rest of __alloc_pages_direct_compact() > where the return value is dictated by whether page is NULL or non-NULL. > We can't leak a captured page if we are testing for it being NULL or > non-NULL, which is what the rest of __alloc_pages_direct_compact() does > *before* your change. So the idea was to add a check the page is actually > NULL here since you are now relying on the return value of > compact_zone_order() to be COMPACT_SKIPPED to infer page == NULL. > > I agree that's currently true in the code, I was trying to catch any > errors where current->capture_control.page was non-NULL but > try_to_compact_pages() returns COMPACT_SKIPPED. There's some complexity > here. Thanks for the detailed explanation. This looks fine to me. I will send V2 with this information in the commit log. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project