On 2024/11/14 10:10, Qiang Liu wrote:
From: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
On 2024/11/12 17:47, baolin.wang@xxxxxxxxxxxxxxxxx wrote:
On 2024/11/12 10:16, liuq131@xxxxxxxxxxxxxxx wrote:
"We assume that the block we are currently processing is distributed
as follows:
0 1 2 511
--------------------------------------------------
| |
| |
---------------------------------------------------
Index 0 and 1 are both pages with an order of 0.
Index 2 has a bogus order (let's assume the order is 9).
When the for loop reaches index 2, it will enter the following code:
/*
* For compound pages such as THP and hugetlbfs, we can save
* potentially a lot of iterations if we skip them at once.
* The check is racy, but we can consider only valid values
* and the only danger is skipping too much.
*/
if (PageCompound(page)) {
const unsigned int order = compound_order(page);
if (blockpfn + (1UL << order) <= end_pfn) {
blockpfn += (1UL << order) - 1;
page += (1UL << order) - 1;
nr_scanned += (1UL << order) - 1;
}
goto isolate_fail;
}
After exiting the for loop:
blockpfn =basepfn+ 2+2^9 = basepfn+514
endpfn = basepfn +512
total_isolated = 2
nr_scanned = 514
In your case, the 'blockpfn' will not be updated to 'basepfn+514',
because 'blockpfn + (1UL << order) > end_pfn', right? And remember the
'end_pfn' is the end of the pageblock.
So I'm still confused about your case. Is this from code inspection?
You're right, the situation where blockpfn > end_pfn would not actually
occur here.
I encountered this issue in the 4.19 kernel, which did not have this check.
I didn't carefully examine this scenario later. Sorry about that.
Never mind:)
However, when blockpfn == end_pfn, I believe the patch is still applicable,
but the git log needs to be updated. Is there still an opportunity to
submit
a revised version of the patch?
Of course yes, and please describe your issue clearly in the next
verion. However, IIUC when blockpfn == end_pfn in your case, the
'total_isolated' is still 0.
/*
* Be careful to not go outside of the pageblock.
*/
if (unlikely(blockpfn > end_pfn))
blockpfn = end_pfn;
So this can happen
/*
* If strict isolation is requested by CMA then check that all the
* pages requested were isolated. If there were any failures, 0 is
* returned and CMA will fail.
*/
if (strict && blockpfn < end_pfn)
total_isolated = 0;
If processed according to the old code, it will not enter the if
statement to reset total_isolated, but the correct handling is to
reset total_isolated to 0.
Please do not top-posting:
"
- Use interleaved ("inline") replies, which makes your response easier
to read. (i.e. avoid top-posting -- the practice of putting your
answer above the quoted text you are responding to.) For more details,
see
:ref:`Documentation/process/submitting-patches.rst
<interleaved_replies>`.
"
