linux-kernel - Re: [PATCH 22/24] HWPOISON: add memory cgroup filter

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 3 Dec 2009 23:03:23 +0800
From:	Wu Fengguang <fengguang.wu@...el.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc:	Andi Kleen <andi@...stfloor.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Daisuke Nishimura <nishimura@....nes.nec.co.jp>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	Li Zefan <lizf@...fujitsu.com>,
	Paul Menage <menage@...gle.com>, Nick Piggin <npiggin@...e.de>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 22/24] HWPOISON: add memory cgroup filter

On Thu, Dec 03, 2009 at 10:58:40AM +0800, KAMEZAWA Hiroyuki wrote:
> On Thu, 3 Dec 2009 10:47:39 +0800
> Wu Fengguang <fengguang.wu@...el.com> wrote:
> 
> > On Thu, Dec 03, 2009 at 10:28:22AM +0800, KAMEZAWA Hiroyuki wrote:
> > Ah please forgive my memcg ignorance..  Then how about bring back the
> > old css_id() based scheme (old patch follows)?
> > 
> maybe enough. but please take care of the fact that css is can be "reused"
> once freed.

OK.

> > > If you have more patches to be usable the function above,
> > > I recommend you to post this with some real-use patches, in step by step.
> > 
> > Do you mean user space test case? Here is a simple one:
> > 
> >         #!/bin/sh
> > 
> >         TEST_PROG=usemem
> >         TEST_PARM="-m 100 -s 100"
> > 
> >         test -d /cgroup/hwpoison && rmdir /cgroup/hwpoison
> >         mkdir /cgroup/hwpoison
> > 
> >         $TEST_PROG $TEST_PARM &
> >         echo `pidof $TEST_PROG` > /cgroup/hwpoison/tasks
> > 
> >         memcg_id=$(</cgroup/hwpoison/memory.id)
> >         echo $memcg_id > /debug/hwpoison/corrupt-filter-memcg
> > 
> >         ./corrupt-all-pfn
> > 
> Ah, this is nice to be put into changelog or some documentation.

Good idea, I'll add it.

> > > patch 19,20 is ok for me.
> > 
> > Thanks,
> > Fengguang
> > ---
> > memcg: show memory.id in cgroupfs
> > 
> > The hwpoison test suite need to selectively inject hwpoison to some
> > targeted task pages, and must not kill important system processes
> > such as init.
> > 
> > The memory cgroup serves this purpose well. We can put the target
> > processes under the control of a memory cgroup, tell the hwpoison
> > injection code the id of that memory cgroup so that it will only
> > poison pages associated with it.
> > 
> > Signed-off-by: Wu Fengguang <fengguang.wu@...el.com>
> 
> No objections from me. please use "id" check. or adds new flag to
> struct mem_cgroup, as you like.

There's a 3rd option: inode number in cgroupfs.

test case:
                memcg_ino=$(ls -id /cgroup/hwpoison | cut -f1 -d' ')
                echo $memcg_ino > /debug/hwpoison/corrupt-filter-memcg

kernel code:
                hwpoison_filter_memcg ==
                memcg->css->cgroup->dentry->d_inode->i_ino

It's pretty long chain, but performance is not a big concern for test
purpose :) As long as the inode number will be accessible and unique
in long term.

This avoids adding extra interfaces to memcg. What do you think?

> The style I prefer is
> ==
>  struct mem_cgroup {
>   ....
>   bool hwpoison_test_enabled;
>  };
> 
> +#ifdef CONFIG_HWPOISON_INJECT /* for now, only user is hwpoison testing */
> +	{
> +		.name = "hwpoison_test_enable",
> +		.read_u64 = ....
> +	},
> +#endif
> 
> and.
> 	mem = try_get_mem_cgroup_from_page(p);
> 	if (mem_cgroup_is_under_poison_test(mem))
> 		ret = true;
> 	mem_cgroup_put(mem);	/* calls css_put() */

It seems mem_cgroup_put() does atomic_dec_and_test(&mem->refcnt).
Is that changed to css_put() recently?

> Maybe not difficult. and this is an usual way. But it's ok if you don't want to
> scannter HWPOISON things to other function's files. This is test operation.
> 
> So, "including real use case and patches" is only my request, for this time.

OK, thanks for the review!

Regards,
Fengguang
--
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/