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
