| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 11 Feb 2013 14:17:06 +0400
From: Glauber Costa <glommer@...allels.com>
To: Anton Vorontsov <anton.vorontsov@...aro.org>
CC: <cgroups@...r.kernel.org>, Tejun Heo <tj@...nel.org>,
David Rientjes <rientjes@...gle.com>,
Pekka Enberg <penberg@...nel.org>,
Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...e.cz>,
"Kirill A. Shutemov" <kirill@...temov.name>,
Kamezawa Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
Luiz Capitulino <lcapitulino@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Greg Thelen <gthelen@...gle.com>,
Leonid Moiseichuk <leonid.moiseichuk@...ia.com>,
KOSAKI Motohiro <kosaki.motohiro@...il.com>,
Minchan Kim <minchan@...nel.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
John Stultz <john.stultz@...aro.org>, <linux-mm@...ck.org>,
<linux-kernel@...r.kernel.org>, <linaro-kernel@...ts.linaro.org>,
<patches@...aro.org>, <kernel-team@...roid.com>
Subject: Re: [PATCH] memcg: Add memory.pressure_level events
Hi Anton,
> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> new file mode 100644
> index 0000000..7922503
> +struct vmpressure_event {
> + struct eventfd_ctx *efd;
> + enum vmpressure_levels level;
> + struct list_head node;
> +};
> +
> +static bool vmpressure_event(struct vmpressure *vmpr,
> + unsigned long s, unsigned long r)
> +{
> + struct vmpressure_event *ev;
> + int level = vmpressure_calc_level(vmpressure_win, s, r);
> + bool signalled = 0;
> +
> + mutex_lock(&vmpr->events_lock);
> +
> + list_for_each_entry(ev, &vmpr->events, node) {
> + if (level >= ev->level) {
> + eventfd_signal(ev->efd, 1);
> + signalled++;
> + }
> + }
> +
> + mutex_unlock(&vmpr->events_lock);
> +
> + return signalled;
> +}
> +
> +static struct vmpressure *vmpressure_parent(struct vmpressure *vmpr)
> +{
> + struct cgroup *cg = vmpr_to_css(vmpr)->cgroup->parent;
> +
> + if (!cg)
> + return NULL;
> + return cg_to_vmpr(cg);
> +}
Unfortunately, "parent" in memcg have different meanings for information
propagation purposes depending on the value of the flag "use_hierarchy".
That is set for deprecation, but still...
I suggest you use the helper mem_cgroup_parent, that will already give
you the right parent (either immediate parent or root) with all that
taken into account.
> +
> +static int vmpressure_register_level(struct cgroup *cg, struct cftype *cft,
> + struct eventfd_ctx *eventfd,
> + const char *args)
> +{
> + struct vmpressure *vmpr = cg_to_vmpr(cg);
> + struct vmpressure_event *ev;
> + int lvl;
> +
> + for (lvl = 0; lvl < VMPRESSURE_NUM_LEVELS; lvl++) {
> + if (!strcmp(vmpressure_str_levels[lvl], args))
> + break;
> + }
> +
> + if (lvl >= VMPRESSURE_NUM_LEVELS)
> + return -EINVAL;
> +
> + ev = kzalloc(sizeof(*ev), GFP_KERNEL);
> + if (!ev)
> + return -ENOMEM;
> +
> + ev->efd = eventfd;
> + ev->level = lvl;
> +
> + mutex_lock(&vmpr->events_lock);
> + list_add(&ev->node, &vmpr->events);
> + mutex_unlock(&vmpr->events_lock);
> +
> + return 0;
> +}
> +
> +static void vmpressure_unregister_level(struct cgroup *cg, struct cftype *cft,
> + struct eventfd_ctx *eventfd)
> +{
> + struct vmpressure *vmpr = cg_to_vmpr(cg);
> + struct vmpressure_event *ev;
> +
> + mutex_lock(&vmpr->events_lock);
> + list_for_each_entry(ev, &vmpr->events, node) {
> + if (ev->efd != eventfd)
> + continue;
> + list_del(&ev->node);
> + kfree(ev);
> + break;
> + }
> + mutex_unlock(&vmpr->events_lock);
> +}
> +
> +static struct cftype vmpressure_cgroup_files[] = {
> + {
> + .name = "pressure_level",
> + .read = vmpressure_read_level,
> + .register_event = vmpressure_register_level,
> + .unregister_event = vmpressure_unregister_level,
> + },
> + {},
> +};
> +
> +
> +void __init enable_pressure_cgroup(void)
> +{
> + WARN_ON(cgroup_add_cftypes(&mem_cgroup_subsys,
> + vmpressure_cgroup_files));
> +}
There is no functionality discovery going on here, and this is
conditional on nothing. Isn't it better then to just add the register +
read functions to memcontrol.c and add the files in the memcontrol cftype ?
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 88c5fed..34f09b9 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1982,6 +1982,10 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> }
> memcg = mem_cgroup_iter(root, memcg, &reclaim);
> } while (memcg);
> +
> + vmpressure(sc->gfp_mask, sc->target_mem_cgroup,
> + sc->nr_scanned - nr_scanned, nr_reclaimed);
> +
> } while (should_continue_reclaim(zone, sc->nr_reclaimed - nr_reclaimed,
> sc->nr_scanned - nr_scanned, sc));
> }
> @@ -2167,6 +2171,8 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> count_vm_event(ALLOCSTALL);
>
> do {
> + vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> + sc->priority);
> sc->nr_scanned = 0;
> aborted_reclaim = shrink_zones(zonelist, sc);
>
vmscan part seems okay to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists