Re: [bug report] mm: avoid leaving partial pfn mappings around in error case

[Linus Torvalds <torvalds@linux-foundation.org>]

On Sun, 15 Sept 2024 at 12:08, Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> The lru_add_drain() function at the start of zap_page_range_single() takes a
> mutex.

Yes, that shouldn't be problematic. But:

> It's the preempt_disable() in gru_fault() which is the issue.  The call tree
> is:
>
> gru_fault() <- disables preempt
> -> remap_pfn_range()
>    -> remap_pfn_range_notrack()

That code is very odd. It was invalid to call remap_pfn_range() with
preemption disabled even before, because it will allocate the page
tables that it fills in.

But presumably *that* never happened in practice, and so nobody
noticed how broken that code was before.

Now smatch seems to see a new problem, but I *think* it's because
smatch didn't notice the sleeping by p4d_alloc() / pud_alloc() /
pmd_alloc() because those allocations are all conditional (so smatch
doesn't see them as static violations).

Put another way: I do not believe this is a new issue, but perhaps a
"new to smatch" issue?

I do get the feeling that even despite the pmd_alloc() having that
conditional, smatch should have seen how it goes to __pmd_alloc() and
then to pmd_alloc_one() and pagetable_alloc_noprof() ->
alloc_pages_noprof().

Have you maybe disabled those warnings from before? Or is there some
other reason why smatch just considers that to be ok with preemption
disabled?

              Linus