On Sun, Nov 08, 2020 at 11:56:30AM +0800, Hillf Danton wrote:
> On Wed,  4 Nov 2020 16:54:27 Jarkko Sakkinen wrote:
> [...]
> > +/**
> > + * sgx_alloc_epc_page() - Allocate an EPC page
> > + * @owner: the owner of the EPC page
> > + * @reclaim:       reclaim pages if necessary
> > + *
> > + * Iterate through EPC sections and borrow a free EPC page to the caller. 
> > When a
> > + * page is no longer needed it must be released with sgx_free_epc_page(). 
> > If
> > + * @reclaim is set to true, directly reclaim pages when we are out of 
> > pages. No
> > + * mm's can be locked when @reclaim is set to true.
> > + *
> > + * Finally, wake up ksgxswapd when the number of pages goes below the 
> > watermark
> > + * before returning back to the caller.
> > + *
> > + * Return:
> > + *   an EPC page,
> > + *   -errno on error
> > + */
> > +struct sgx_epc_page *sgx_alloc_epc_page(void *owner, bool reclaim)
> > +{
> > +   struct sgx_epc_page *entry;
> 
> Nit: s/entry/epc_page/
> > +
> > +   for ( ; ; ) {
> > +           entry = __sgx_alloc_epc_page();
> > +           if (!IS_ERR(entry)) {
> > +                   entry->owner = owner;
> > +                   break;
> > +           }
> > +
> > +           if (list_empty(&sgx_active_page_list))
> > +                   return ERR_PTR(-ENOMEM);
> > +
> > +           if (!reclaim) {
> > +                   entry = ERR_PTR(-EBUSY);
> > +                   break;
> > +           }
> > +
> > +           if (signal_pending(current)) {
> > +                   entry = ERR_PTR(-ERESTARTSYS);
> > +                   break;
> > +           }
> > +
> > +           sgx_reclaim_pages();
> i
> This is the direct reclaim mode with ksgxswapd that works in
> the background ignored in the entire for loop. But we can go
> with it in parallel, see below, if it tries as hard as it can
> to maitain the watermark in which allocators may have no
> interest.

I think this policy should be left at is and once the code in mainline
further refined. Consider it as a baseline/initial version for
reclaiming code.

> > +           schedule();
> 
> To cut allocator's latency use cond_resched();

Thanks, I'll change this.

> > +   }
> > +
> > +   if (sgx_should_reclaim(SGX_NR_LOW_PAGES))
> > +           wake_up(&ksgxswapd_waitq);
> 
> Nit: s/ksgxswapd/sgxd/ as it seems to have nothing to do with swap,
> given sgx itself is clear and good enough.

Yeah, it also handling kexec() situation, i.e. has multitude of
functions.

> > +
> > +   return entry;
> > +}
> 
> struct sgx_epc_page *sgx_alloc_epc_page(void *owner, bool reclaim)
> {
>       struct sgx_epc_page *epc_page;
> 
>       for (;;) {
>               epc_page = __sgx_alloc_epc_page();
> 
>               if (!IS_ERR(epc_page)) {
>                       epc_page->owner = owner;
>                       return epc_page;
>               }
> 
>               if (signal_pending(current))
>                       return ERR_PTR(-ERESTARTSYS);
> 
>               if (list_empty(&sgx_active_page_list) || !reclaim)
>                       return ERR_PTR(-ENOMEM);
> 
>               wake_up(&ksgxswapd_waitq);
>               cond_resched();
>       }
>       return ERR_PTR(-ENOMEM);
> }

/Jarkko

Reply via email to