lkml.org 
[lkml]   [2009]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] memcg: coalescing uncharge at unmap and truncation (fixed coimpile bug)
On Fri, 2 Oct 2009 16:02:13 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

>
> ...
>
> In massive parallel enviroment, res_counter can be a performance bottleneck.
> One strong techinque to reduce lock contention is reducing calls by
> coalescing some amount of calls into one.
>
> Considering charge/uncharge chatacteristic,
> - charge is done one by one via demand-paging.
> - uncharge is done by
> - in chunk at munmap, truncate, exit, execve...
> - one by one via vmscan/paging.
>
> It seems we have a chance in uncharge at unmap/truncation.
>
> This patch is a for coalescing uncharge. For avoiding scattering memcg's
> structure to functions under /mm, this patch adds memcg batch uncharge
> information to the task.
>
> The degree of coalescing depends on callers
> - at invalidate/trucate... pagevec size
> - at unmap ....ZAP_BLOCK_SIZE
> (memory itself will be freed in this degree.)
> Then, we'll not coalescing too much.
>
>
> ...
>
> +static void
> +__do_uncharge(struct mem_cgroup *mem, const enum charge_type ctype)
> +{
> + struct memcg_batch_info *batch = NULL;
> + bool uncharge_memsw = true;
> + /* If swapout, usage of swap doesn't decrease */
> + if (!do_swap_account || ctype == MEM_CGROUP_CHARGE_TYPE_SWAPOUT)
> + uncharge_memsw = false;
> + /*
> + * do_batch > 0 when unmapping pages or inode invalidate/truncate.
> + * In those cases, all pages freed continously can be expected to be in
> + * the same cgroup and we have chance to coalesce uncharges.
> + * And, we do uncharge one by one if this is killed by OOM.
> + */
> + if (!current->memcg_batch.do_batch || test_thread_flag(TIF_MEMDIE))
> + goto direct_uncharge;
> +
> + batch = &current->memcg_batch;
> + /*
> + * In usual, we do css_get() when we remember memcg pointer.
> + * But in this case, we keep res->usage until end of a series of
> + * uncharges. Then, it's ok to ignore memcg's refcnt.
> + */
> + if (!batch->memcg)
> + batch->memcg = mem;
> + /*
> + * In typical case, batch->memcg == mem. This means we can
> + * merge a series of uncharges to an uncharge of res_counter.
> + * If not, we uncharge res_counter ony by one.
> + */
> + if (batch->memcg != mem)
> + goto direct_uncharge;
> + /* remember freed charge and uncharge it later */
> + batch->pages += PAGE_SIZE;

->pages is really confusingly named. It doesn't count pages, it counts
bytes!

We could call it `bytes', but perhaps charge_bytes would be more
communicative?

> +/*
> + * batch_start/batch_end is called in unmap_page_range/invlidate/trucate.
> + * In that cases, pages are freed continuously and we can expect pages
> + * are in the same memcg. All these calls itself limits the number of
> + * pages freed at once, then uncharge_start/end() is called properly.
> + */
> +
> +void mem_cgroup_uncharge_start(void)
> +{
> + if (!current->memcg_batch.do_batch) {
> + current->memcg_batch.memcg = NULL;
> + current->memcg_batch.pages = 0;
> + current->memcg_batch.memsw = 0;

what's memsw?

> + }
> + current->memcg_batch.do_batch++;
> +}
> +
>
> ...
>
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR /* memcg uses this to do batch job */
> + struct memcg_batch_info {
> + int do_batch;
> + struct mem_cgroup *memcg;
> + long pages, memsw;
> + } memcg_batch;
> +#endif

I find the most valuable documetnation is that which is devoted to the
data structures. This one didn't get any :(

Negative values of `pages' and `memsw' are meaningless, so it would be
better to give them an unsigned type. That matches the
res_counter_charge() expectations also.




\
 
 \ /
  Last update: 2009-10-09 00:27    [W:0.068 / U:4.064 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site