[no subject]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Leave the initiaization of @cg to a later phase where @sgx_cg is
guaranteed not being NULL, or initialize @cg to NULL here and update later.

> +	struct misc_cg *next_cg = NULL;
> +
> +	/*
> +	 * Make sure there are some free pages at both cgroup and global levels.
> +	 * In both cases, only make one attempt of reclamation to avoid lengthy
> +	 * block on the caller.
> +	 */
> +	if (sgx_cg && sgx_cgroup_should_reclaim(sgx_cg))
> +		next_cg = sgx_cgroup_reclaim_pages(cg, next_cg, current->mm);

I don't quite follow the logic.

First of all, sgx_cgroup_reclaim_pages() isn't called in a loop, so why
not just do:

	next_cg = sgx_cgroup_reclaim_pages(cg, NULL, current->mm);

And what is the point of set @next_cg here, since ...


> +
> +	if (next_cg != cg)
> +		put_misc_cg(next_cg);
> +
> +	next_cg = NULL;

... here @next_cg is reset to NULL ?

Looks the only reason is you need to do ...

	put_misc_cg(next_cg);

... above?

This piece of code appears repeatedly in this file.  Is there any way we
can get rid of it?

>   	if (sgx_should_reclaim_global(SGX_NR_LOW_PAGES))
> -		sgx_reclaim_pages_global(current->mm);
> +		next_cg = sgx_reclaim_pages_global(next_cg, current->mm);

And this doesn't seems "global reclaim" at all?

Because it essentially equals to:

	next_cg = sgx_reclaim_pages_global(NULL, current->mm);

which always reclaims from the ROOT.

So each call to sgx_reclaim_direct() will always reclaim from the ROOT --
any other LRUs in the hierarchy will unlikely to get any chance to be
reclaimed.

> +
> +	if (next_cg != misc_cg_root())
> +		put_misc_cg(next_cg);
> +
> +	sgx_put_cg(sgx_cg);
>   }
>   
>   static int ksgxd(void *p)
>   {
> +	struct misc_cg *next_cg = NULL;
> +
>   	set_freezable();
>   
>   	/*
> @@ -437,11 +489,15 @@ static int ksgxd(void *p)
>   				     kthread_should_stop() ||
>   				     sgx_should_reclaim_global(SGX_NR_HIGH_PAGES));
>   
> -		if (sgx_should_reclaim_global(SGX_NR_HIGH_PAGES))
> +		while (!kthread_should_stop() && sgx_should_reclaim_global(SGX_NR_HIGH_PAGES)) {
>   			/* Indirect reclaim, no mm to charge, so NULL: */
> -			sgx_reclaim_pages_global(NULL);
> +			next_cg = sgx_reclaim_pages_global(next_cg, NULL);
> +			cond_resched();

Should the 'put_misc_cg(next_cg)' be done within the while() loop but not
below?
> +		}
>   
> -		cond_resched();
> +		if (next_cg != misc_cg_root())
> +			put_misc_cg(next_cg);
> +		next_cg = NULL;

Again, it doesn't seems "global reclaim" here, since you always restart
from the ROOT once the target pages have been reclaimed.

AFAICT it's completely different from the existing global reclaim.

>   	}
>   
>   	return 0;
> @@ -583,6 +639,7 @@ int sgx_unmark_page_reclaimable(struct sgx_epc_page *page)
>    */
>   struct sgx_epc_page *sgx_alloc_epc_page(void *owner, enum sgx_reclaim reclaim)
>   {
> +	struct misc_cg *next_cg = NULL;
>   	struct sgx_cgroup *sgx_cg;
>   	struct sgx_epc_page *page;
>   	int ret;
> @@ -616,10 +673,19 @@ struct sgx_epc_page *sgx_alloc_epc_page(void *owner, enum sgx_reclaim reclaim)
>   			break;
>   		}
>   
> -		sgx_reclaim_pages_global(current->mm);
> +		/*
> +		 * At this point, the usage within this cgroup is under its
> +		 * limit but there is no physical page left for allocation.
> +		 * Perform a global reclaim to get some pages released from any
> +		 * cgroup with reclaimable pages.
> +		 */
> +		next_cg = sgx_reclaim_pages_global(next_cg, current->mm);
>   		cond_resched();
>   	}

Ditto IIUC.







[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux