Re: Wait for mutex to become unlocked

[Paul E. McKenney]

On Wed, May 04, 2022 at 10:04:44PM -0700, Paul E. McKenney wrote:
> On Thu, May 05, 2022 at 03:14:53AM +0200, Thomas Gleixner wrote:
> > On Thu, May 05 2022 at 01:38, Matthew Wilcox wrote:
> > > On Thu, May 05, 2022 at 02:22:30AM +0200, Thomas Gleixner wrote:
> > >> > That is, rwsem_wait_read() puts the thread on the rwsem's wait queue,
> > >> > and wakes it up without giving it the lock.  Now this thread will never
> > >> > be able to block any thread that tries to acquire mmap_sem for write.
> > >> 
> > >> Never?
> > >> 
> > >>  	if (down_read_trylock(&vma->sem)) {
> > >> 
> > >> ---> preemption by writer
> > >
> > > Ah!  This is a different semaphore.  Yes, it can be preempted while
> > > holding the VMA rwsem and block a thread which is trying to modify the
> > > VMA which will then block all threads from faulting _on that VMA_,
> > > but it won't affect page faults on any other VMA.
> > 
> > Ooops. Missed that detail. Too many semaphores here.
> > 
> > > It's only Better, not Best (the Best approach was proposed on Monday
> > > afternoon, and the other MM developers asked us to only go as far as
> > > Better and see if that was good enough).
> > 
> > :)
> > 
> > >> The information gathered from /proc/pid/smaps is unreliable at the point
> > >> where the lock is dropped already today. So it does not make a
> > >> difference whether the VMAs have a 'read me if you really think it's
> > >> useful' sideband information which gets updated when the VMA changes and
> > >> allows to do:
> > >
> > > Mmm.  I'm not sure that we want to maintain the smaps information on
> > > the off chance that somebody wants to query it.
> > 
> > Fair enough, but then the question is whether it's more reasonable to
> > document that if you want to read that nonsense, then you have to live
> > with the consequences. The problem with many of those interfaces is that
> > they have been added for whatever reasons, became ABI and people are
> > suddenly making performance claims which might not be justified at all.
> > 
> > We really have to make our mind up and make decisions whether we want to
> > solve every "I want a pony" complaint just because.
> > 
> > >> But looking at the stuff which gets recomputed and reevaluated in that
> > >> proc/smaps code this makes a lot of sense, because most if not all of
> > >> this information is already known at the point where the VMA is modified
> > >> while holding mmap_sem for useful reasons, no?
> > >
> > > I suspect the only way to know is to try to implement it, and then
> > > benchmark it.
> > 
> > Sure. There are other ways than having a RCU protected info, e.g. a
> > sequence count which ensures that the to be read information is
> > consistent.
> 
> So the thought is to maintain the /proc/smaps information separately,
> so that it can just be read out, correct?  If so...
> 
> As you say, sequence counts can check consistency, but something else
> is required to protect any dereferences of pointers to data that might
> be freed.  One approach is to place the /proc/smaps information somewhere
> that cannot be freed during /proc/smaps scan.  The place that comes
> immediately to mind is the mm_struct, but I suspect that the /proc/smaps
> information will need to be variable length, especially on 64-bit systems.
> 
> Another approach is to allocate space for the /proc/smaps information
> dynamically, using RCU to protect only reads of only that information.
> But you seem to be thinking of something else.  Or maybe your point is
> that the use of RCU can be restricted to this /proc/smaps information?
> 
> Yet another approach is to use reference counts, but of course the counts
> need to live outside of the structure being protected.  If the summary
> information is not to block expansion of the address space (which is
> the asked-for pony), this gets tricky due to the need to quickly and
> repeatedly enlarge the memory holding the /proc/smaps information.

Ah, maybe you were thinking of protecting the pointer dereference (and
maybe also a reference-count increment) with a lock (perhaps in the
mm_struct?), where the lock's critical section disabled preemption.
The lock would be released before scanning the information, and your
sequence count could then retry inconsistent reads.  (With some limit
beyond which inconsistency is accepted, just as inconsistent /proc/smaps
reads can happen today).

Or again, am I missing a trick here?

							Thanx, Paul