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

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Thu, 3 Dec 2009 10:47:39 +0800
Wu Fengguang <fengguang.wu@intel.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.


> > 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.


> > 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@intel.com>

No objections from me. please use "id" check. or adds new flag to
struct mem_cgroup, as you like.

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() */

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.

Thanks,
-Kame


> ---
>  mm/memcontrol.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> --- linux-mm.orig/mm/memcontrol.c	2009-09-07 16:01:02.000000000 +0800
> +++ linux-mm/mm/memcontrol.c	2009-09-11 18:20:55.000000000 +0800
> @@ -2510,6 +2510,13 @@ mem_cgroup_get_recursive_idx_stat(struct
>  	*val = d.val;
>  }
>  
> +#ifdef CONFIG_HWPOISON_INJECT
> +static u64 mem_cgroup_id_read(struct cgroup *cont, struct cftype *cft)
> +{
> +	return css_id(cgroup_subsys_state(cont, mem_cgroup_subsys_id));
> +}
> +#endif
> +
>  static u64 mem_cgroup_read(struct cgroup *cont, struct cftype *cft)
>  {
>  	struct mem_cgroup *mem = mem_cgroup_from_cont(cont);
> @@ -2841,6 +2848,12 @@ static int mem_cgroup_swappiness_write(s
>  
>  
>  static struct cftype mem_cgroup_files[] = {
> +#ifdef CONFIG_HWPOISON_INJECT /* for now, only user is hwpoison testing */
> +	{
> +		.name = "id",
> +		.read_u64 = mem_cgroup_id_read,
> +	},
> +#endif
>  	{
>  		.name = "usage_in_bytes",
>  		.private = MEMFILE_PRIVATE(_MEM, RES_USAGE),
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>