On Thu, Oct 08, 2015 at 06:41:40PM +0300, Kirill Tkhai wrote:
...
> >> @@ -326,6 +326,10 @@ struct mem_cgroup {
> >> /* For oom notifier event fd */
> >> struct list_head oom_notify;
> >>
> >> +#ifdef CONFIG_CLEANCACHE
> >> + bool cleancache_disabled;
> >> +#endif
> >
>
> This is not a good place to put 1-byte variable. Above and below are
> unsigned long variables, so this lose 7 byte just by design.
>
> I advise you to put it together with any another bool variable.
np
>
> >> +
> >> /*
> >> * Should we move charges of a task when a task is moved into this
> >> * mem_cgroup ? And what type of charges should we move ?
> >> @@ -1577,6 +1581,37 @@ bool mem_cgroup_low(struct mem_cgroup *root, struct
> >> mem_cgroup *memcg)
> >> return true;
> >> }
> >>
> >> +#ifdef CONFIG_CLEANCACHE
> >> +bool mem_cgroup_cleancache_disabled(struct page *page)
> >> +{
> >> + struct mem_cgroup *memcg = NULL;
> >> + struct page_cgroup *pc;
> >> + bool ret = false;
> >> +
> >> + if (mem_cgroup_disabled())
> >> + goto out;
> >> +
> >> + pc = lookup_page_cgroup(page);
> >> + if (!PageCgroupUsed(pc))
> >> + goto out;
> >> +
> >> + lock_page_cgroup(pc);
> >> + if (!PageCgroupUsed(pc))
> >> + goto out_unlock;
> >> +
> >> + for (memcg = pc->mem_cgroup; memcg; memcg = parent_mem_cgroup(memcg)) {
> >> + if (memcg->cleancache_disabled) {
> >> + ret = true;
> >> + break;
> >> + }
> >> + }
> >> +out_unlock:
> >> + unlock_page_cgroup(pc);
> >> +out:
> >> + return ret;
> >> +}
> >
> I really dislike the design. This code is likely case and it should be as
> lightweight
> as possible. But the patch complicates it instead of making that on unlikely
> path.
>
> Can't we in mem_cgroup_disable_cleancache_write() propagate
> mem_cgroup::cleancache value
> to all of our children? Yeah, we will have to have a possibility to differ
> manually and
> propagated values, but an implementation of a couple of flags instead of bool
> value should
> not be difficult here.
Makes sense, will rework this part.
Thanks,
Vladimir
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
