* Liam R. Howlett [230704 14:47]: > * Linus Torvalds [230704 14:36]: > > On Tue, 4 Jul 2023 at 11:25, Liam R. Howlett wrote: > > > > > > validate_mm() calls are too spread out and duplicated in numerous > > > locations. Also, now that the stack write is done under the write lock, > > > it is not necessary to validate the mm prior to write operations. > > > > So while I applied the fixes directly since I was doing all the > > write-locking stuff (and asked for the locking cleanup), I'm hoping > > these kinds of cleanups will now go back to normal and go through > > Andrew. > > > > I do have a question related to the write locking: now that we should > > always hold the mmap lock for writing when doing any modifications, > > can the "lock_is_held()" assertions be tightened? > > > > Right now it's "any locking", but for actual modification it should > > probably be using > > > > lockdep_is_held_type(mt->ma_external_lock, 1) For completeness of the email tread; it turns out we want 0 as the last parameter. (include/linux/lockdep.h) /* * Acquire a lock. * * Values for "read": * * 0: exclusive (write) acquire * 1: read-acquire (no recursion allowed) * 2: read-acquire with same-instance recursion allowed * * Values for check: * * 0: simple checks (freeing, held-at-exit-time, etc.) * 1: full validation */ ... /* * Same "read" as for lock_acquire(), except -1 means any. */ extern int lock_is_held_type(const struct lockdep_map *lock, int read); > > > > but there's just one 'mt_lock_is_held()' function (presumably because > > the internal lock is always just a spinlock that doesn't have the > > reader/writer distinction). > > Ah, yes. I was trying to do just that, but ran into an issue and backed > out of fully fixing this portion up until later. > Here are two patches to increase the strictness of the maple tree locking. I've boot tested them on x86_64 with the bots config and ensured the lockdep problem was resolved. The first introduces the new mt_write_locked() function, which ensures the lock type is for writing. The second updates the munmap path to avoid triggering the warnings associated with dropping the mmap_lock prior to freeing the VMAs. Thanks, Liam