>  struct sgx_epc_page *sgx_alloc_epc_page(void *owner, bool reclaim)  {
> +     struct sgx_epc_cgroup *epc_cg;
>       struct sgx_epc_page *page;
> +     int ret;
> +
> +     epc_cg = sgx_get_current_epc_cg();
> +     ret = sgx_epc_cgroup_try_charge(epc_cg);
> +     if (ret) {
> +             sgx_put_epc_cg(epc_cg);
> +             return ERR_PTR(ret);
> +     }
> 
>       for ( ; ; ) {
>               page = __sgx_alloc_epc_page();
> @@ -567,8 +578,10 @@ struct sgx_epc_page *sgx_alloc_epc_page(void
> *owner, bool reclaim)
>                       break;
>               }
> 
> -             if (list_empty(&sgx_active_page_list))
> -                     return ERR_PTR(-ENOMEM);
> +             if (list_empty(&sgx_active_page_list)) {
> +                     page = ERR_PTR(-ENOMEM);
> +                     break;
> +             }

(Sorry for replying from Outlook because I am in travel for Chinese New Year.)

Perhaps I am missing something but I don't understand this change.

An empty sgx_active_page_list means you cannot reclaim any page from it, so why 
need to break?

> 
>               if (!reclaim) {
>                       page = ERR_PTR(-EBUSY);
> @@ -580,10 +593,25 @@ struct sgx_epc_page *sgx_alloc_epc_page(void
> *owner, bool reclaim)
>                       break;
>               }
> 
> +             /*
> +              * Need to do a global reclamation if cgroup was not full but
> free
> +              * physical pages run out, causing __sgx_alloc_epc_page() to
> fail.
> +              */
>               sgx_reclaim_pages();
>               cond_resched();
>       }

And why adding this comment, especially in this patch?

I don't see it brings additional clarity because there's only global reclaim 
now, no matter whether cgroup is enabled or not. 

Reply via email to