Re: [PATCH] fs/proc/task_mmu: fix stale walk_end after a completed scan

[Chris Down <chris@chrisdown.name>]

Hi Matt,

This patch doesn't apply as sent, because your mail client mangled it. It
wrapped long lines, including the hunk header and the Fixes tag, and took
the leading space off the blank context lines. git am rejects it as
corrupt.

Please send the next version with git send-email or b4 send rather than
through Gmail, or set your client up as
Documentation/process/email-clients.rst describes. It's worth sending it to
yourself first and checking that git am takes it.

Matt Salsamendi writes:
>+	/* A completed walk may follow an earlier full bounce buffer. */
>+	if (!ret || !p.arg.walk_end)
>+		p.arg.walk_end = p.arg.end;

While this fixes the refill case, ret being 0 doesn't always mean the walk
got to the end of the range. pagemap_scan_hugetlb_entry() can't
write-protect only part of a huge page. So with PM_SCAN_WP_MATCHING, when
only part of a written huge page fits in what's left of max_pages, it backs
that page out. It sets walk_end to the start of the page and returns 0 on
purpose.

Before this patch, the caller got walk_end pointing at that page. With it,
walk_end comes back as end, so the caller is told the whole range was
scanned. But the page was neither returned nor write-protected. From a
quick test with a single written 2M hugetlb page, PM_SCAN_WP_MATCHING and
max_pages = 1, the ioctl returns 0 with walk_end at the start of the page
before this patch, and at the end of the range with it.

Maybe it would be simpler to clear walk_end before each walk_page_range()
call, and keep the existing check after the loop as it is? Then that check
only sees what the last walk set. In the same quick test, that fixes the
refill case too, and the hugetlb case behaves as it did before. It would be
good for the selftest Andrew asked for to cover the hugetlb case as well.

Thanks!

Chris