From: KAMEZAWA Hiroyuki on
On Wed, 9 Dec 2009 12:47:27 -0800
Paul Menage <menage(a)google.com> wrote:

> On Tue, Dec 8, 2009 at 1:16 PM, Andi Kleen <andi(a)firstfloor.org> wrote:
> >
> > The hwpoison test suite need to inject hwpoison to a collection of
> > selected task pages, and must not touch pages not owned by them and
> > thus kill important system processes such as init. (But it's OK to
> > mis-hwpoison free/unowned pages as well as shared clean pages.
> > Mis-hwpoison of shared dirty pages will kill all tasks, so the test
> > suite will target all or non of such tasks in the first place.)
>
> While the functionality sounds useful, the interface (passing an inode
> number) feels a bit ugly to me. Also, if that group is deleted and a
> new cgroup created, you could end up reusing the inode number.
>
I agree.

> How about an approach where you write either the cgroup path (relative
> to the memcg mount) or an fd open on the desired cgroup? Then you
> could store a (counted) css reference rather than an inode number,
> which would make the filter function cleaner too, since it would just
> need to compare css objects.
>
Sounds reasonable.

Thanks,
-Kame


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andi Kleen on
> While the functionality sounds useful, the interface (passing an inode
> number) feels a bit ugly to me. Also, if that group is deleted and a
> new cgroup created, you could end up reusing the inode number.

Please note this is just a testing interface, doesn't need to be
100% fool-proof. It'll never be used in production.

>
> How about an approach where you write either the cgroup path (relative
> to the memcg mount) or an fd open on the desired cgroup? Then you
> could store a (counted) css reference rather than an inode number,
> which would make the filter function cleaner too, since it would just
> need to compare css objects.

Sounds complicated, I assume it would be much more code?
I would prefer to keep the testing interfaces as simple as possible.

-Andi
--
ak(a)linux.intel.com -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Balbir Singh on
* Andi Kleen <andi(a)firstfloor.org> [2009-12-10 02:42:12]:

> > While the functionality sounds useful, the interface (passing an inode
> > number) feels a bit ugly to me. Also, if that group is deleted and a
> > new cgroup created, you could end up reusing the inode number.
>
> Please note this is just a testing interface, doesn't need to be
> 100% fool-proof. It'll never be used in production.
>
> >
> > How about an approach where you write either the cgroup path (relative
> > to the memcg mount) or an fd open on the desired cgroup? Then you
> > could store a (counted) css reference rather than an inode number,
> > which would make the filter function cleaner too, since it would just
> > need to compare css objects.
>
> Sounds complicated, I assume it would be much more code?
> I would prefer to keep the testing interfaces as simple as possible.
>

We do this for cgroupstats and the code is not very complicated. In
case you want to look, the user space code is at
Documentation/accounting/getdelays.c and the kernel code is in
kernel/taskstats.c

--
Balbir
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Wu Fengguang on
On Thu, Dec 10, 2009 at 10:21:13AM +0800, Balbir Singh wrote:
> * Andi Kleen <andi(a)firstfloor.org> [2009-12-10 02:42:12]:
>
> > > While the functionality sounds useful, the interface (passing an inode
> > > number) feels a bit ugly to me. Also, if that group is deleted and a
> > > new cgroup created, you could end up reusing the inode number.
> >
> > Please note this is just a testing interface, doesn't need to be
> > 100% fool-proof. It'll never be used in production.
> >
> > >
> > > How about an approach where you write either the cgroup path (relative
> > > to the memcg mount) or an fd open on the desired cgroup? Then you
> > > could store a (counted) css reference rather than an inode number,
> > > which would make the filter function cleaner too, since it would just
> > > need to compare css objects.
> >
> > Sounds complicated, I assume it would be much more code?
> > I would prefer to keep the testing interfaces as simple as possible.
> >
>
> We do this for cgroupstats and the code is not very complicated. In
> case you want to look, the user space code is at
> Documentation/accounting/getdelays.c and the kernel code is in
> kernel/taskstats.c

Balbir, thanks for the tip.

We could keep an fd open on the desired cgroup, in user space:

#!/bin/bash

mkdir /cgroup/hwpoison && \
exec 9<>/cgroup/hwpoison/tasks || exit 1

A bit simpler than an in-kernel fget_light() or CSS refcount :)

Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andi Kleen on
Wu Fengguang <fengguang.wu(a)intel.com> writes:
>
> We could keep an fd open on the desired cgroup, in user space:
>
> #!/bin/bash
>
> mkdir /cgroup/hwpoison && \
> exec 9<>/cgroup/hwpoison/tasks || exit 1
>
> A bit simpler than an in-kernel fget_light() or CSS refcount :)

FYI, I decided to not do any of this in .33, but just keep the
ugly-but-working inode hack. We can look at fixing that for .34.
These interfaces are debugfs, so can be changed.

-Andi

--
ak(a)linux.intel.com -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/