Re: [PATCH for_v21 1/2] x86/sgx: Use SRCU to protect mm_list during reclaim

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Fri, Jul 12, 2019 at 12:13:07AM +0300, Jarkko Sakkinen wrote:
> On Thu, Jul 11, 2019 at 09:16:24AM -0700, Sean Christopherson wrote:
> > Not taking a reference to encl_mm is not just an optimization, it's also
> > functionally necessary and a major motivation to moving to SRCu. Putting
> > the reference can invoke sgx_encl_mm_release(), which calls
> > synchronize_srcu() and will deadlock if done while holding the SRCU read
> > lock.  Not taking a reference paves the way for additional refcounting
> > improvements that would be extremely difficult to implement when using
> > the custom walker due to cyclical dependencies on the refcount.
> 
> I'm not sure I get this. The existing code does not have a call to
> synchronize_srcu().

Does this read any better?

  Not taking a reference to encl_mm is not just an optimization, it's also
  functionally necessary and a major motivation to moving to SRCU.  From a
  functional perspective, putting the encl_mm reference can invoke
  sgx_encl_mm_release(), which now calls synchronize_srcu() and thus will
  deadlock if triggered while holding the SRCU read lock.  In terms of
  motivation, not taking a reference paves the way for additional refcounting
  improvements that would be extremely difficult to implement when using
  the custom walker due to cyclical dependencies on the encl_mm's refcount.
 
> > -	if (!encl)
> > -		return;
> > +	lockdep_assert_held_exclusive(&mm->mmap_sem);
> 
> Just a question: what does it check (12:10AM too tired to check,
> apologies)?

Asserts that the caller has done down_write(&mmap_sem), i.e. guarantees
that sgx_encl_mm_add() cannot be called in parallel on the same mm_struct.