[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <18ae4b82-b18a-4ed8-8b72-4a9697a4dc8f@suse.cz>
Date: Fri, 21 Nov 2025 10:08:19 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
"Paul E. McKenney" <paulmck@...nel.org>
Cc: Matthew Wilcox <willy@...radead.org>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Suren Baghdasaryan <surenb@...gle.com>,
Shakeel Butt <shakeel.butt@...ux.dev>, Jann Horn <jannh@...gle.com>,
stable@...r.kernel.org, syzbot+131f9eb2b5807573275c@...kaller.appspotmail.com
Subject: Re: [PATCH] mm/mmap_lock: Reset maple state on lock_vma_under_rcu()
retry
(sorry for the late reply)
On 11/13/25 12:05, Lorenzo Stoakes wrote:
>> > > I think one source of confusion for me with maple tree operations is - what
>> > > to do if we are in a position where some kind of reset is needed?
>> > >
>> > > So even if I'd realised 'aha we need to reset this' it wouldn't be obvious
>> > > to me that we ought to set to the address.
>> >
>> > I think that's a separate problem.
>> >
>> > > > +++ b/mm/mmap_lock.c
>> > > > @@ -257,6 +257,7 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
>> > > > if (PTR_ERR(vma) == -EAGAIN) {
>> > > > count_vm_vma_lock_event(VMA_LOCK_MISS);
>> > > > /* The area was replaced with another one */
>> > > > + mas_set(&mas, address);
>> > >
>> > > I wonder if we could detect that the RCU lock was released (+ reacquired) in
>> > > mas_walk() in a debug mode, like CONFIG_VM_DEBUG_MAPLE_TREE?
>> >
>> > Dropping and reacquiring the RCU read lock should have been a big red
>> > flag. I didn't have time to review the patches, but if I had, I would
>> > have suggested passing the mas down to the routine that drops the rcu
>> > read lock so it can be invalidated before dropping the readlock.
>>
>> There has been some academic efforts to check for RCU-protected pointers
>> leaking from one RCU read-side critical section to another, but nothing
>> useful has come from this. :-/
>
> Ugh a pity. I was hoping we could do (in debug mode only obv) something
> absolutely roughly like:
>
> On init:
>
> mas->rcu_critical_section = rcu_get_critical_section_blah();
AFAICT, get_state_synchronize_rcu()?
> ...
>
> On walk:
>
> VM_WARN_ON(rcu_critical_section_blah() != mas->rcu_critical_section);
And here, poll_state_synchronize_rcu()?
It wouldn't detect directly that we dropped and reacquired the rcu read
lock, but with enough testing, that would at some point translate to a new
grace period between the first and second read lock, and we'd catch it then?
> But sounds like that isn't feasible.
I don't think what Paul says means your suggestion is not feasible. I think
he says there are no known ways to do this checking automagicallt. But your
suggestion is doing it manually for a specific case. I guess it depends on
how many maple tree functions we'd have to change and how ugly it would be.
> I always like the idea of us having debug stuff that helps highlight dumb
> mistakes very quickly, no matter how silly they might be :)
>
>>
>> But rcu_pointer_handoff() and unrcu_pointer() are intended not only for
>> documentation, but also to suppress the inevitable false positives should
>> anyone figure out how to detect leaking of RCU-protected pointers.
>>
>> Thanx, Paul
>
> Cheers, Lorenzo
Powered by blists - more mailing lists