Re: [PATCH v3] x86/sgx: Fix deadlock and race conditions between fork() and EPC reclaim

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Mon, Apr 06, 2020 at 09:15:57AM -0700, Sean Christopherson wrote:
> >  	encl->ssaframesize = secs->ssa_frame_size;
> > +	encl->mm_list_version = 1;
> 
> This is unnecessary.  A mm_list_version of '0' means the list walk started
> when there were no mm structs associated with the enclave, i.e. skipping
> everything related to walking the list is ok.  It's subtle, and I dislike
> relying on that behavior, but IMO it's preferable to incorrectly implying
> that a list version of '0' is somehow bad.

'0' means whatever code requires to mean. There is no absolute meaning.

> > +	for ( ; ; ) {
> > +		next = encl->mm_list_version;
> >  
> > -		down_read(&encl_mm->mm->mmap_sem);
> > +		if (version == next)
> > +			break;
> 
> Functionally this works, but I personally find it the logic kludgy, and it
> generates worse code.  Not that we're at the point where counting uops is a
> to priority, but I don't think it makes sense to go out of our way to make
> the resulting code worse.

I'll pick v2 then.

/Jarkko