LinuxLists.cc - mmotm 2010-01-06-14-34 uploaded

[permalink] [raw]

Subject: [PATCH -mmotm] memcg: implement memory thresholds document fixes

Each memcg-implement-memory-thresholds.patch and
memcg-add-interface-to-move-charge-at-task-migration.patch try to add a new
section to Documentation/cgroup/memory.txt, so the document has been a bit
mangled when these patches are merged at the same time.

This patch fixes it.

Signed-off-by: Daisuke Nishimura <[email protected]>
Cc: Kirill A. Shutemov <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Cc: Balbir Singh <[email protected]>
---
This patch can be applied after memcg-implement-memory-thresholds-checkpatch-fixes.patch.

Documentation/cgroups/memory.txt | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index 226a955..aad7d05 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -458,7 +458,15 @@ Note: Those pages and swaps must be charged to the old cgroup.
Note: More type of pages(e.g. file cache, shmem,) will be supported by other
bits in future.

-8.3 Memory thresholds
+8.3 TODO
+
+- Add support for other types of pages(e.g. file cache, shmem, etc.).
+- Implement madvise(2) to let users decide the vma to be moved or not to be
+ moved.
+- All of moving charge operations are done under cgroup_mutex. It's not good
+ behavior to hold the mutex too long, so we may need some trick.
+
+9. Memory thresholds

Memory controler implements memory thresholds using cgroups notification
API (see cgroups.txt). It allows to register multiple memory and memsw
@@ -475,16 +483,7 @@ threshold in any direction.

It's applicable for root and non-root cgroup.

-
-8.4 TODO
-
-- Add support for other types of pages(e.g. file cache, shmem, etc.).
-- Implement madvise(2) to let users decide the vma to be moved or not to be
- moved.
-- All of moving charge operations are done under cgroup_mutex. It's not good
- behavior to hold the mutex too long, so we may need some trick.
-
-9. TODO
+10. TODO

1. Add support for accounting huge pages (as a separate controller)
2. Make per-cgroup scanner reclaim not-shared pages first
--
1.5.6.1

2010-01-07 01:09:52

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (kernel/cgroup)

On Wed, 06 Jan 2010 14:34:36 -0800 [email protected] wrote:

> The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
>
> http://userweb.kernel.org/~akpm/mmotm/
>
> and will soon be available at
>
> git://zen-kernel.org/kernel/mmotm.git
>
> It contains the following patches against 2.6.33-rc3:

kernel/cgroup.c: In function 'cgroup_write_event_control':
kernel/cgroup.c:2949: error: implicit declaration of function 'eventfd_fget'
kernel/cgroup.c:2949: warning: assignment makes pointer from integer without a cast
kernel/cgroup.c:2955: error: implicit declaration of function 'eventfd_ctx_fileget'
kernel/cgroup.c:2955: warning: assignment makes pointer from integer without a cast
make[2]: *** [kernel/cgroup.o] Error 1

config attached.

---
~Randy

Attachments:

config-r5933 (50.63 kB)

2010-01-07 01:11:47

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (mm/memcontrol)

On Wed, 06 Jan 2010 14:34:36 -0800 [email protected] wrote:

> The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
>
> http://userweb.kernel.org/~akpm/mmotm/
>
> and will soon be available at
>
> git://zen-kernel.org/kernel/mmotm.git
>
> It contains the following patches against 2.6.33-rc3:

mm/memcontrol.c: In function 'is_target_pte_for_mc':
mm/memcontrol.c:3985: error: implicit declaration of function 'mem_cgroup_count_swap_user'
mm/memcontrol.c: In function 'mem_cgroup_move_charge_pte_range':
mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'

config attached.

---
~Randy

Attachments:

config-r5922 (57.83 kB)

2010-01-07 01:21:23

[permalink] [raw]

Subject: Re: [PATCH -mmotm] memcg: implement memory thresholds document fixes

On Thu, 7 Jan 2010 09:57:14 +0900
Daisuke Nishimura <[email protected]> wrote:

> Each memcg-implement-memory-thresholds.patch and
> memcg-add-interface-to-move-charge-at-task-migration.patch try to add a new
> section to Documentation/cgroup/memory.txt, so the document has been a bit
> mangled when these patches are merged at the same time.
>
> This patch fixes it.
>

Acked-by: KAMEZAWA Hiroyuki <[email protected]>

BTW, I'll prepare total update for memcg (especially around percpu counter).
Do you have something may conflict in plan ?

Thanks,
-Kame

> Signed-off-by: Daisuke Nishimura <[email protected]>
> Cc: Kirill A. Shutemov <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Balbir Singh <[email protected]>
> ---
> This patch can be applied after memcg-implement-memory-thresholds-checkpatch-fixes.patch.
>
> Documentation/cgroups/memory.txt | 21 ++++++++++-----------
> 1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> index 226a955..aad7d05 100644
> --- a/Documentation/cgroups/memory.txt
> +++ b/Documentation/cgroups/memory.txt
> @@ -458,7 +458,15 @@ Note: Those pages and swaps must be charged to the old cgroup.
> Note: More type of pages(e.g. file cache, shmem,) will be supported by other
> bits in future.
>
> -8.3 Memory thresholds
> +8.3 TODO
> +
> +- Add support for other types of pages(e.g. file cache, shmem, etc.).
> +- Implement madvise(2) to let users decide the vma to be moved or not to be
> + moved.
> +- All of moving charge operations are done under cgroup_mutex. It's not good
> + behavior to hold the mutex too long, so we may need some trick.
> +
> +9. Memory thresholds
>
> Memory controler implements memory thresholds using cgroups notification
> API (see cgroups.txt). It allows to register multiple memory and memsw
> @@ -475,16 +483,7 @@ threshold in any direction.
>
> It's applicable for root and non-root cgroup.
>
> -
> -8.4 TODO
> -
> -- Add support for other types of pages(e.g. file cache, shmem, etc.).
> -- Implement madvise(2) to let users decide the vma to be moved or not to be
> - moved.
> -- All of moving charge operations are done under cgroup_mutex. It's not good
> - behavior to hold the mutex too long, so we may need some trick.
> -
> -9. TODO
> +10. TODO
>
> 1. Add support for accounting huge pages (as a separate controller)
> 2. Make per-cgroup scanner reclaim not-shared pages first
> --
> 1.5.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2010-01-07 01:40:37

[permalink] [raw]

Subject: Re: [PATCH -mmotm] memcg: implement memory thresholds document fixes

On Thu, 7 Jan 2010 10:18:05 +0900, KAMEZAWA Hiroyuki <[email protected]> wrote:
> On Thu, 7 Jan 2010 09:57:14 +0900
> Daisuke Nishimura <[email protected]> wrote:
>
> > Each memcg-implement-memory-thresholds.patch and
> > memcg-add-interface-to-move-charge-at-task-migration.patch try to add a new
> > section to Documentation/cgroup/memory.txt, so the document has been a bit
> > mangled when these patches are merged at the same time.
> >
> > This patch fixes it.
> >
>
> Acked-by: KAMEZAWA Hiroyuki <[email protected]>
>
> BTW, I'll prepare total update for memcg (especially around percpu counter).
> Do you have something may conflict in plan ?
>
Not for now.
Of course, I have a plan that I will add a file/shmem page support to move-charge feature,
but I think we should stabilize and clean up current memcg in mmotm first.
(My biggest concern now is the influence of transparent hugepage to vm and memcg.)

Thanks,
Daisuke Nishimura.

2010-01-07 01:43:45

[permalink] [raw]

Subject: Re: [PATCH -mmotm] memcg: implement memory thresholds document fixes

On Thu, 7 Jan 2010 10:34:17 +0900
Daisuke Nishimura <[email protected]> wrote:

> On Thu, 7 Jan 2010 10:18:05 +0900, KAMEZAWA Hiroyuki <[email protected]> wrote:
> > On Thu, 7 Jan 2010 09:57:14 +0900
> > Daisuke Nishimura <[email protected]> wrote:
> >
> > > Each memcg-implement-memory-thresholds.patch and
> > > memcg-add-interface-to-move-charge-at-task-migration.patch try to add a new
> > > section to Documentation/cgroup/memory.txt, so the document has been a bit
> > > mangled when these patches are merged at the same time.
> > >
> > > This patch fixes it.
> > >
> >
> > Acked-by: KAMEZAWA Hiroyuki <[email protected]>
> >
> > BTW, I'll prepare total update for memcg (especially around percpu counter).
> > Do you have something may conflict in plan ?
> >
> Not for now.

Okay, thank you. I'll prepare some.

> Of course, I have a plan that I will add a file/shmem page support to move-charge feature,
> but I think we should stabilize and clean up current memcg in mmotm first.
agreed.

> (My biggest concern now is the influence of transparent hugepage to vm and memcg.)
>
Yes, it's interesting challenge but has concerns for us ;)

Thanks,
-Kame

2010-01-07 02:18:56

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (mm/memcontrol)

Thank you for your report.

On Wed, 6 Jan 2010 17:10:58 -0800, Randy Dunlap <[email protected]> wrote:
> On Wed, 06 Jan 2010 14:34:36 -0800 [email protected] wrote:
>
> > The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
> >
> > http://userweb.kernel.org/~akpm/mmotm/
> >
> > and will soon be available at
> >
> > git://zen-kernel.org/kernel/mmotm.git
> >
> > It contains the following patches against 2.6.33-rc3:
>
>
> mm/memcontrol.c: In function 'is_target_pte_for_mc':
> mm/memcontrol.c:3985: error: implicit declaration of function 'mem_cgroup_count_swap_user'
> mm/memcontrol.c: In function 'mem_cgroup_move_charge_pte_range':
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
>
>
> config attached.
>
I'm sorry I missed the !CONFIG_SWAP or !CONFIG_CGROUP_MEM_RES_CTLR_SWAP case.

I'll prepare fixes.

Thanks,
Daisuke Nishimura.

2010-01-07 02:25:32

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (mm/memcontrol)

On Thu, 7 Jan 2010 11:13:19 +0900
Daisuke Nishimura <[email protected]> wrote:

> Thank you for your report.

> > config attached.
> >
> I'm sorry I missed the !CONFIG_SWAP or !CONFIG_CGROUP_MEM_RES_CTLR_SWAP case.
>
> I'll prepare fixes.
>
Nishimura-san, could you double check this ?

Andrew, this is a fix onto Nishimura-san's memcg move account patch series.
Maybe this -> patches/memcg-move-charges-of-anonymous-swap.patch

Thanks,
-Kame
==

Build fix to following build error when CONFIG_CGROUP_MEM_RES_CTLR_SWAP is off.

mm/memcontrol.c: In function 'is_target_pte_for_mc':
mm/memcontrol.c:3985: error: implicit declaration of function 'mem_cgroup_count_swap_user'
mm/memcontrol.c: In function 'mem_cgroup_move_charge_pte_range':
mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'

CC: Daisuke Nishimura <[email protected]>
Reported-by: Randy Dunlap <[email protected]>
Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
---
mm/memcontrol.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Index: ref-mmotm/mm/memcontrol.c
===================================================================
--- ref-mmotm.orig/mm/memcontrol.c
+++ ref-mmotm/mm/memcontrol.c
@@ -2369,7 +2369,7 @@ static int mem_cgroup_move_swap_account(
}
#else
static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
- struct mem_cgroup *from, struct mem_cgroup *to)
+ struct mem_cgroup *from, struct mem_cgroup *to, bool need_fixup)
{
return -EINVAL;
}
@@ -3976,7 +3976,7 @@ static int is_target_pte_for_mc(struct v

if (!pte_present(ptent)) {
/* TODO: handle swap of shmes/tmpfs */
- if (pte_none(ptent) || pte_file(ptent))
+ if (pte_none(ptent) || pte_file(ptent) || !do_swap_account)
return 0;
else if (is_swap_pte(ptent)) {
ent = pte_to_swp_entry(ptent);

2010-01-07 02:35:17

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (kernel/cgroup)

On Wed, 6 Jan 2010 17:08:49 -0800
Randy Dunlap <[email protected]> wrote:

> On Wed, 06 Jan 2010 14:34:36 -0800 [email protected] wrote:
>
> > The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
> >
> > http://userweb.kernel.org/~akpm/mmotm/
> >
> > and will soon be available at
> >
> > git://zen-kernel.org/kernel/mmotm.git
> >
> > It contains the following patches against 2.6.33-rc3:
>
>
> kernel/cgroup.c: In function 'cgroup_write_event_control':
> kernel/cgroup.c:2949: error: implicit declaration of function 'eventfd_fget'
> kernel/cgroup.c:2949: warning: assignment makes pointer from integer without a cast
> kernel/cgroup.c:2955: error: implicit declaration of function 'eventfd_ctx_fileget'
> kernel/cgroup.c:2955: warning: assignment makes pointer from integer without a cast
> make[2]: *** [kernel/cgroup.o] Error 1
>

Hmm, how about this rather than adding #ifdefs..
Paul, Li, how do you think ?
==
Cgroup's eventdf feature has depenedecy to EVENTFD

This is a patch onto
cgroup-implement-eventfd-based-generic-api-for-notifications.patch

CC: Li Zefan <[email protected]>
CC: Kirill A. Shutemov <[email protected]>
CC: Paul Menage <[email protected]>
Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>

---
init/Kconfig | 1 +
1 file changed, 1 insertion(+)

Index: ref-mmotm/init/Kconfig
===================================================================
--- ref-mmotm.orig/init/Kconfig
+++ ref-mmotm/init/Kconfig
@@ -509,6 +509,7 @@ endchoice

menuconfig CGROUPS
boolean "Control Group support"
+ depends on EVENTFD
help
This option adds support for grouping sets of processes together, for
use with process control subsystems such as Cpusets, CFS, memory

2010-01-07 02:41:19

by Li Zefan

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (kernel/cgroup)

KAMEZAWA Hiroyuki wrote:
> On Wed, 6 Jan 2010 17:08:49 -0800
> Randy Dunlap <[email protected]> wrote:
>
>> On Wed, 06 Jan 2010 14:34:36 -0800 [email protected] wrote:
>>
>>> The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
>>>
>>> http://userweb.kernel.org/~akpm/mmotm/
>>>
>>> and will soon be available at
>>>
>>> git://zen-kernel.org/kernel/mmotm.git
>>>
>>> It contains the following patches against 2.6.33-rc3:
>>
>> kernel/cgroup.c: In function 'cgroup_write_event_control':
>> kernel/cgroup.c:2949: error: implicit declaration of function 'eventfd_fget'
>> kernel/cgroup.c:2949: warning: assignment makes pointer from integer without a cast
>> kernel/cgroup.c:2955: error: implicit declaration of function 'eventfd_ctx_fileget'
>> kernel/cgroup.c:2955: warning: assignment makes pointer from integer without a cast
>> make[2]: *** [kernel/cgroup.o] Error 1
>>
>
> Hmm, how about this rather than adding #ifdefs..
> Paul, Li, how do you think ?

I think make CONFIG_CGROUPS select EVENTFD would be better.

> ==
> Cgroup's eventdf feature has depenedecy to EVENTFD
>
> This is a patch onto
> cgroup-implement-eventfd-based-generic-api-for-notifications.patch
>
> CC: Li Zefan <[email protected]>
> CC: Kirill A. Shutemov <[email protected]>
> CC: Paul Menage <[email protected]>
> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
>
> ---
> init/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: ref-mmotm/init/Kconfig
> ===================================================================
> --- ref-mmotm.orig/init/Kconfig
> +++ ref-mmotm/init/Kconfig
> @@ -509,6 +509,7 @@ endchoice
>
> menuconfig CGROUPS
> boolean "Control Group support"
> + depends on EVENTFD
> help
> This option adds support for grouping sets of processes together, for
> use with process control subsystems such as Cpusets, CFS, memory
>
>
>

2010-01-07 02:50:44

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (kernel/cgroup)

On Thu, 07 Jan 2010 10:41:15 +0800
Li Zefan <[email protected]> wrote:

> KAMEZAWA Hiroyuki wrote:
> > On Wed, 6 Jan 2010 17:08:49 -0800
> > Randy Dunlap <[email protected]> wrote:
> >
> >> On Wed, 06 Jan 2010 14:34:36 -0800 [email protected] wrote:
> >>
> >>> The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
> >>>
> >>> http://userweb.kernel.org/~akpm/mmotm/
> >>>
> >>> and will soon be available at
> >>>
> >>> git://zen-kernel.org/kernel/mmotm.git
> >>>
> >>> It contains the following patches against 2.6.33-rc3:
> >>
> >> kernel/cgroup.c: In function 'cgroup_write_event_control':
> >> kernel/cgroup.c:2949: error: implicit declaration of function 'eventfd_fget'
> >> kernel/cgroup.c:2949: warning: assignment makes pointer from integer without a cast
> >> kernel/cgroup.c:2955: error: implicit declaration of function 'eventfd_ctx_fileget'
> >> kernel/cgroup.c:2955: warning: assignment makes pointer from integer without a cast
> >> make[2]: *** [kernel/cgroup.o] Error 1
> >>
> >
> > Hmm, how about this rather than adding #ifdefs..
> > Paul, Li, how do you think ?
>
> I think make CONFIG_CGROUPS select EVENTFD would be better.
>
Hm, as far as I know, SELECT is not recommended. But yes, it seems side-effect
of select is small here. But I avoid it as much as possible, usually.

Moreover, in this case,
- EVENTFD is a feature which is enabled by default.
- cgroup is "Say N if unsure" config.

In this case, "depends on" is good.

Thanks,
-Kame

2010-01-07 03:03:56

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (mm/memcontrol)

Thank you for your fix.

On Thu, 7 Jan 2010 11:21:50 +0900, KAMEZAWA Hiroyuki <[email protected]> wrote:
> On Thu, 7 Jan 2010 11:13:19 +0900
> Daisuke Nishimura <[email protected]> wrote:
>
> > Thank you for your report.
>
> > > config attached.
> > >
> > I'm sorry I missed the !CONFIG_SWAP or !CONFIG_CGROUP_MEM_RES_CTLR_SWAP case.
> >
> > I'll prepare fixes.
> >
> Nishimura-san, could you double check this ?
>
It seems that this cannot fix the !CONFIG_SWAP case in my environment.

> Andrew, this is a fix onto Nishimura-san's memcg move account patch series.
> Maybe this -> patches/memcg-move-charges-of-anonymous-swap.patch
>
I think both memcg-move-charges-of-anonymous-swap.patch and
memcg-improve-performance-in-moving-swap-charge.patch need to be fixed.

> mm/memcontrol.c: In function 'is_target_pte_for_mc':
> mm/memcontrol.c:3985: error: implicit declaration of function 'mem_cgroup_count_swap_user'
This derives from a bug of memcg-move-charges-of-anonymous-swap.patch,

and

> mm/memcontrol.c: In function 'mem_cgroup_move_charge_pte_range':
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
this derives from that of memcg-improve-performance-in-moving-swap-charge.patch.

I'm now testing my patch in some configs, and will post later.

Thanks,
Daisuke Nishimura.

> Thanks,
> -Kame
> ==
>
> Build fix to following build error when CONFIG_CGROUP_MEM_RES_CTLR_SWAP is off.
>
> mm/memcontrol.c: In function 'is_target_pte_for_mc':
> mm/memcontrol.c:3985: error: implicit declaration of function 'mem_cgroup_count_swap_user'
> mm/memcontrol.c: In function 'mem_cgroup_move_charge_pte_range':
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
>
> CC: Daisuke Nishimura <[email protected]>
> Reported-by: Randy Dunlap <[email protected]>
> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
> ---
> mm/memcontrol.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: ref-mmotm/mm/memcontrol.c
> ===================================================================
> --- ref-mmotm.orig/mm/memcontrol.c
> +++ ref-mmotm/mm/memcontrol.c
> @@ -2369,7 +2369,7 @@ static int mem_cgroup_move_swap_account(
> }
> #else
> static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
> - struct mem_cgroup *from, struct mem_cgroup *to)
> + struct mem_cgroup *from, struct mem_cgroup *to, bool need_fixup)
> {
> return -EINVAL;
> }
> @@ -3976,7 +3976,7 @@ static int is_target_pte_for_mc(struct v
>
> if (!pte_present(ptent)) {
> /* TODO: handle swap of shmes/tmpfs */
> - if (pte_none(ptent) || pte_file(ptent))
> + if (pte_none(ptent) || pte_file(ptent) || !do_swap_account)
> return 0;
> else if (is_swap_pte(ptent)) {
> ent = pte_to_swp_entry(ptent);
>

2010-01-07 03:05:05

by Li Zefan

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (kernel/cgroup)

>>>>> It contains the following patches against 2.6.33-rc3:
>>>> kernel/cgroup.c: In function 'cgroup_write_event_control':
>>>> kernel/cgroup.c:2949: error: implicit declaration of function 'eventfd_fget'
>>>> kernel/cgroup.c:2949: warning: assignment makes pointer from integer without a cast
>>>> kernel/cgroup.c:2955: error: implicit declaration of function 'eventfd_ctx_fileget'
>>>> kernel/cgroup.c:2955: warning: assignment makes pointer from integer without a cast
>>>> make[2]: *** [kernel/cgroup.o] Error 1
>>>>
>>> Hmm, how about this rather than adding #ifdefs..
>>> Paul, Li, how do you think ?
>> I think make CONFIG_CGROUPS select EVENTFD would be better.
>>
> Hm, as far as I know, SELECT is not recommended. But yes, it seems side-effect
> of select is small here. But I avoid it as much as possible, usually.
>
> Moreover, in this case,
> - EVENTFD is a feature which is enabled by default.
> - cgroup is "Say N if unsure" config.
>
> In this case, "depends on" is good.
>

I thought we use "depends on" if it's obvious by the config name
that a config depends on another config, and use "select" otherwise.

Referring to Documentation/Kbuild/kconfig-language.txt:

select should be used with care.
...
In general use select only for non-visible symbols
(no prompts anywhere) and for symbols with no dependencies.

It does suggest "depends on" is better. :)

2010-01-07 03:05:48

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (mm/memcontrol)

On Thu, 7 Jan 2010 11:59:01 +0900
Daisuke Nishimura <[email protected]> wrote:

> Thank you for your fix.
>
> On Thu, 7 Jan 2010 11:21:50 +0900, KAMEZAWA Hiroyuki <[email protected]> wrote:
> > On Thu, 7 Jan 2010 11:13:19 +0900
> > Daisuke Nishimura <[email protected]> wrote:
> >
> > > Thank you for your report.
> >
> > > > config attached.
> > > >
> > > I'm sorry I missed the !CONFIG_SWAP or !CONFIG_CGROUP_MEM_RES_CTLR_SWAP case.
> > >
> > > I'll prepare fixes.
> > >
> > Nishimura-san, could you double check this ?
> >
> It seems that this cannot fix the !CONFIG_SWAP case in my environment.
>
> > Andrew, this is a fix onto Nishimura-san's memcg move account patch series.
> > Maybe this -> patches/memcg-move-charges-of-anonymous-swap.patch
> >
> I think both memcg-move-charges-of-anonymous-swap.patch and
> memcg-improve-performance-in-moving-swap-charge.patch need to be fixed.
>
> > mm/memcontrol.c: In function 'is_target_pte_for_mc':
> > mm/memcontrol.c:3985: error: implicit declaration of function 'mem_cgroup_count_swap_user'
> This derives from a bug of memcg-move-charges-of-anonymous-swap.patch,
>
> and
>
> > mm/memcontrol.c: In function 'mem_cgroup_move_charge_pte_range':
> > mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> > mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> > mm/memcontrol.c:4220: error: too many arguments to function 'mem_cgroup_move_swap_account'
> this derives from that of memcg-improve-performance-in-moving-swap-charge.patch.
>
>
> I'm now testing my patch in some configs, and will post later.
>
Okay, plz.

Thanks,
-Kame

2010-01-07 03:48:30

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded

On Wed, 06 Jan 2010 14:34:36 PST, [email protected] said:
> The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
>
> http://userweb.kernel.org/~akpm/mmotm/

Building with CONFIG_SQUASHFS=m, CONFIG_SQUASHFS_LZMA=y ,
and CONFIG_DECOMPRESS_LZMA=n fails:

Kernel: arch/x86/boot/bzImage is ready (#1)
Building modules, stage 2.
MODPOST 219 modules
ERROR: "unlzma" [fs/squashfs/squashfs.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

Looks like a missing select/depends for DECOMPRESS_LZMA. Somebody else can
decide which it should be...

Attachments:

(No filename) (227.00 B)

2010-01-07 04:02:14

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded

On Wed, 06 Jan 2010 22:48:16 EST, [email protected] said:

> Building with CONFIG_SQUASHFS=m, CONFIG_SQUASHFS_LZMA=y ,
> and CONFIG_DECOMPRESS_LZMA=n fails:

> Looks like a missing select/depends for DECOMPRESS_LZMA. Somebody else can
> decide which it should be...

Digging further:

x Symbol: DECOMPRESS_LZMA [=n] x
x Selected by: RD_LZMA [=n] && BLK_DEV_INITRD [=y] || SQUASHFS_LZMA [=y x

How the heck did this happen? Looks like a SELECT *is* there but it's
not firing??!?

Attachments:

(No filename) (227.00 B)

2010-01-07 04:09:41

[permalink] [raw]

Subject: [PATCH -mmotm] build fix for memcg-move-charges-of-anonymous-swap.patch

build fix in !CONFIG_SWAP case.

CC mm/memcontrol.o
mm/memcontrol.c: In function 'is_target_pte_for_mc':
mm/memcontrol.c:3648: error: implicit declaration of function 'mem_cgroup_count_swap_user'
make[1]: *** [mm/memcontrol.o] Error 1
make: *** [mm] Error 2

Reported-by: Randy Dunlap <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Signed-off-by: Daisuke Nishimura <[email protected]>
---
This can be applied after memcg-move-charges-of-anonymous-swap.patch.

include/linux/swap.h | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index d9b06f7..2e1d5c9 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -287,6 +287,10 @@ extern int shmem_unuse(swp_entry_t entry, struct page *page);

extern void swap_unplug_io_fn(struct backing_dev_info *, struct page *);

+#ifdef CONFIG_CGROUP_MEM_RES_CTLR
+extern int mem_cgroup_count_swap_user(swp_entry_t ent, struct page **pagep);
+#endif
+
#ifdef CONFIG_SWAP
/* linux/mm/page_io.c */
extern int swap_readpage(struct page *);
@@ -356,7 +360,6 @@ static inline void disable_swap_token(void)
#ifdef CONFIG_CGROUP_MEM_RES_CTLR
extern void
mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout);
-extern int mem_cgroup_count_swap_user(swp_entry_t ent, struct page **pagep);
#else
static inline void
mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout)
--
1.5.6.1

2010-01-07 04:10:36

[permalink] [raw]

Subject: [PATCH -mmotm] build fix for memcg-improve-performance-in-moving-swap-charge.patch

build fix in !CONFIG_CGROUP_MEM_RES_CTLR_SWAP case.

CC mm/memcontrol.o
mm/memcontrol.c: In function 'mem_cgroup_move_charge_pte_range':
mm/memcontrol.c:3899: error: too many arguments to function 'mem_cgroup_move_swap_account'
mm/memcontrol.c:3899: error: too many arguments to function 'mem_cgroup_move_swap_account'
mm/memcontrol.c:3899: error: too many arguments to function 'mem_cgroup_move_swap_account'
make[1]: *** [mm/memcontrol.o] Error 1
make: *** [mm] Error 2

Reported-by: Randy Dunlap <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Signed-off-by: Daisuke Nishimura <[email protected]>
---
This can be applied after memcg-improve-performance-in-moving-swap-charge.patch.

mm/memcontrol.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5360d48..65df8d2 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2326,7 +2326,7 @@ static int mem_cgroup_move_swap_account(swp_entry_t entry,
}
#else
static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
- struct mem_cgroup *from, struct mem_cgroup *to)
+ struct mem_cgroup *from, struct mem_cgroup *to, bool need_fixup)
{
return -EINVAL;
}
--
1.5.6.1

2010-01-07 04:33:50

[permalink] [raw]

Subject: Re: [PATCH -mmotm] build fix for memcg-move-charges-of-anonymous-swap.patch

On Thu, 7 Jan 2010 13:06:09 +0900
Daisuke Nishimura <[email protected]> wrote:

> build fix in !CONFIG_SWAP case.
>
> CC mm/memcontrol.o
> mm/memcontrol.c: In function 'is_target_pte_for_mc':
> mm/memcontrol.c:3648: error: implicit declaration of function 'mem_cgroup_count_swap_user'
> make[1]: *** [mm/memcontrol.o] Error 1
> make: *** [mm] Error 2
>
> Reported-by: Randy Dunlap <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Signed-off-by: Daisuke Nishimura <[email protected]>

Hmm, this doesn't seem include fix for CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n
==
static int is_target_pte_for_mc(struct vm_area_struct *vma,
unsigned long addr, pte_t ptent, union mc_target *target)
{
....
else if (is_swap_pte(ptent)) {
ent = pte_to_swp_entry(ptent);
if (!move_anon || non_swap_entry(ent))
return 0;
usage_count = mem_cgroup_count_swap_user(ent, &page);
}
==
At least, !do_swap_account check is necessary, I think.
I'm sorry if I miss something...

-Kame

> ---
> This can be applied after memcg-move-charges-of-anonymous-swap.patch.
>
> include/linux/swap.h | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index d9b06f7..2e1d5c9 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -287,6 +287,10 @@ extern int shmem_unuse(swp_entry_t entry, struct page *page);
>
> extern void swap_unplug_io_fn(struct backing_dev_info *, struct page *);
>
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR
> +extern int mem_cgroup_count_swap_user(swp_entry_t ent, struct page **pagep);
> +#endif
> +
> #ifdef CONFIG_SWAP
> /* linux/mm/page_io.c */
> extern int swap_readpage(struct page *);
> @@ -356,7 +360,6 @@ static inline void disable_swap_token(void)
> #ifdef CONFIG_CGROUP_MEM_RES_CTLR
> extern void
> mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout);
> -extern int mem_cgroup_count_swap_user(swp_entry_t ent, struct page **pagep);
> #else
> static inline void
> mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout)
> --
> 1.5.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2010-01-07 04:34:26

[permalink] [raw]

Subject: Re: [PATCH -mmotm] build fix for memcg-improve-performance-in-moving-swap-charge.patch

On Thu, 7 Jan 2010 13:06:31 +0900
Daisuke Nishimura <[email protected]> wrote:

> build fix in !CONFIG_CGROUP_MEM_RES_CTLR_SWAP case.
>
> CC mm/memcontrol.o
> mm/memcontrol.c: In function 'mem_cgroup_move_charge_pte_range':
> mm/memcontrol.c:3899: error: too many arguments to function 'mem_cgroup_move_swap_account'
> mm/memcontrol.c:3899: error: too many arguments to function 'mem_cgroup_move_swap_account'
> mm/memcontrol.c:3899: error: too many arguments to function 'mem_cgroup_move_swap_account'
> make[1]: *** [mm/memcontrol.o] Error 1
> make: *** [mm] Error 2
>
> Reported-by: Randy Dunlap <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Signed-off-by: Daisuke Nishimura <[email protected]>
Acked-by: KAMEZAWA Hiroyuki <[email protected]>

2010-01-07 05:11:17

[permalink] [raw]

Subject: Re: [PATCH -mmotm] build fix for memcg-move-charges-of-anonymous-swap.patch

On Thu, 7 Jan 2010 13:30:26 +0900
KAMEZAWA Hiroyuki <[email protected]> wrote:

> On Thu, 7 Jan 2010 13:06:09 +0900
> Daisuke Nishimura <[email protected]> wrote:
>
> > build fix in !CONFIG_SWAP case.
> >
> > CC mm/memcontrol.o
> > mm/memcontrol.c: In function 'is_target_pte_for_mc':
> > mm/memcontrol.c:3648: error: implicit declaration of function 'mem_cgroup_count_swap_user'
> > make[1]: *** [mm/memcontrol.o] Error 1
> > make: *** [mm] Error 2
> >
> > Reported-by: Randy Dunlap <[email protected]>
> > Cc: KAMEZAWA Hiroyuki <[email protected]>
> > Signed-off-by: Daisuke Nishimura <[email protected]>
>
> Hmm, this doesn't seem include fix for CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n
> ==
> static int is_target_pte_for_mc(struct vm_area_struct *vma,
> unsigned long addr, pte_t ptent, union mc_target *target)
> {
> ....
> else if (is_swap_pte(ptent)) {
> ent = pte_to_swp_entry(ptent);
> if (!move_anon || non_swap_entry(ent))
> return 0;
> usage_count = mem_cgroup_count_swap_user(ent, &page);
> }
> ==
> At least, !do_swap_account check is necessary, I think.
> I'm sorry if I miss something...
>

Get follwoing after this patch with !CONFIG_SWAP case.
==
mm/built-in.o: In function `is_target_pte_for_mc':
/home/kamezawa/Kernel/ref-mmotm/mm/memcontrol.c:3985: undefined reference to `mem_cgroup_count_swap_user'

I think !do_swap_count check in is_target_pte_for_mc() should be added.

Thanks,
-Kame

2010-01-07 05:33:33

[permalink] [raw]

Subject: Re: [PATCH -mmotm] build fix for memcg-move-charges-of-anonymous-swap.patch

On Thu, 7 Jan 2010 13:30:26 +0900, KAMEZAWA Hiroyuki <[email protected]> wrote:
> On Thu, 7 Jan 2010 13:06:09 +0900
> Daisuke Nishimura <[email protected]> wrote:
>
> > build fix in !CONFIG_SWAP case.
> >
> > CC mm/memcontrol.o
> > mm/memcontrol.c: In function 'is_target_pte_for_mc':
> > mm/memcontrol.c:3648: error: implicit declaration of function 'mem_cgroup_count_swap_user'
> > make[1]: *** [mm/memcontrol.o] Error 1
> > make: *** [mm] Error 2
> >
> > Reported-by: Randy Dunlap <[email protected]>
> > Cc: KAMEZAWA Hiroyuki <[email protected]>
> > Signed-off-by: Daisuke Nishimura <[email protected]>
>
> Hmm, this doesn't seem include fix for CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n
> ==
> static int is_target_pte_for_mc(struct vm_area_struct *vma,
> unsigned long addr, pte_t ptent, union mc_target *target)
> {
> ....
> else if (is_swap_pte(ptent)) {
> ent = pte_to_swp_entry(ptent);
> if (!move_anon || non_swap_entry(ent))
> return 0;
> usage_count = mem_cgroup_count_swap_user(ent, &page);
> }
> ==
> At least, !do_swap_account check is necessary, I think.
> I'm sorry if I miss something...
>
mem_cgroup_count_swap_user() is defined in CONFIG_CGROUP_MEM_RES_CTLR case,
so the build error has nothing to do with CONFIG_CGROUP_MEM_RES_CTLR_SWAP(i.e. do_swap_account).
And I think adding !do_swap_account would ignore unmaped-but-not-uncharged-yet
swapcache in CONFIG_SWAP && !CONFIG_CGROUP_MEM_RES_CTLR_SWAP case.
(it would not be a big problem though).

Anyway, I'm sorry that the first patch was wrong...
This is the correct one.

===
From: Daisuke Nishimura <[email protected]>

build fix in !CONFIG_SWAP case.

CC mm/memcontrol.o
mm/memcontrol.c: In function 'is_target_pte_for_mc':
mm/memcontrol.c:3648: error: implicit declaration of function 'mem_cgroup_count_swap_user'
make[1]: *** [mm/memcontrol.o] Error 1
make: *** [mm] Error 2

Reported-by: Randy Dunlap <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Signed-off-by: Daisuke Nishimura <[email protected]>
---
This can be applied after memcg-move-charges-of-anonymous-swap.patch.

include/linux/swap.h | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index d9b06f7..c2a4295 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -488,6 +488,14 @@ mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent)
{
}

+#ifdef CONFIG_CGROUP_MEM_RES_CTLR
+static inline int
+mem_cgroup_count_swap_user(swp_entry_t ent, struct page **pagep)
+{
+ return 0;
+}
+#endif
+
#endif /* CONFIG_SWAP */
#endif /* __KERNEL__*/
#endif /* _LINUX_SWAP_H */
--
1.5.6.1

2010-01-07 05:55:46

[permalink] [raw]

Subject: Re: [PATCH -mmotm] build fix for memcg-move-charges-of-anonymous-swap.patch

On Thu, 7 Jan 2010 14:14:01 +0900
Daisuke Nishimura <[email protected]> wrote:

> Anyway, I'm sorry that the first patch was wrong...
> This is the correct one.
>
> ===
> From: Daisuke Nishimura <[email protected]>
>
> build fix in !CONFIG_SWAP case.
>
> CC mm/memcontrol.o
> mm/memcontrol.c: In function 'is_target_pte_for_mc':
> mm/memcontrol.c:3648: error: implicit declaration of function 'mem_cgroup_count_swap_user'
> make[1]: *** [mm/memcontrol.o] Error 1
> make: *** [mm] Error 2
>
> Reported-by: Randy Dunlap <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Signed-off-by: Daisuke Nishimura <[email protected]>

Thank you.

Acked-by: KAMEZAWA Hiroyuki <[email protected]>

BTW, maybe it's time to
- remove EXPERIMENTAL from CONFIG_CGROUP_MEM_RES_CTRL_SWAP
and more,
- remove CONFIG_CGROUP_MEM_RES_CTRL_SWAP
(to reduce complicated #ifdefs and replace them with CONFIG_SWAP.)

It's very stable as far as I test.

Thanks,
-Kame

2010-01-07 06:29:17

by Kirill A. Shutemov

[permalink] [raw]

Subject: Re: [PATCH -mmotm] memcg: implement memory thresholds document fixes

On Thu, Jan 7, 2010 at 2:57 AM, Daisuke Nishimura
<[email protected]> wrote:
> Each memcg-implement-memory-thresholds.patch and
> memcg-add-interface-to-move-charge-at-task-migration.patch try to add a new
> section to Documentation/cgroup/memory.txt, so the document has been a bit
> mangled when these patches are merged at the same time.
>
> This patch fixes it.

Acked-by: Kirill A. Shutemov <[email protected]>

2010-01-07 08:53:05

by Balbir Singh

[permalink] [raw]

Subject: Re: [PATCH -mmotm] memcg: implement memory thresholds document fixes

* KAMEZAWA Hiroyuki <[email protected]> [2010-01-07 10:18:05]:

> On Thu, 7 Jan 2010 09:57:14 +0900
> Daisuke Nishimura <[email protected]> wrote:
>
> > Each memcg-implement-memory-thresholds.patch and
> > memcg-add-interface-to-move-charge-at-task-migration.patch try to add a new
> > section to Documentation/cgroup/memory.txt, so the document has been a bit
> > mangled when these patches are merged at the same time.
> >
> > This patch fixes it.
> >
>
> Acked-by: KAMEZAWA Hiroyuki <[email protected]>
>
> BTW, I'll prepare total update for memcg (especially around percpu counter).
> Do you have something may conflict in plan ?

Kame, could you clarify percpu counter? Is this on for resource
counter scalability patches I had?

--
Balbir

2010-01-07 08:54:27

by Balbir Singh

[permalink] [raw]

Subject: Re: [PATCH -mmotm] memcg: implement memory thresholds document fixes

* KAMEZAWA Hiroyuki <[email protected]> [2010-01-07 10:40:18]:

> > (My biggest concern now is the influence of transparent hugepage to vm and memcg.)
> >
> Yes, it's interesting challenge but has concerns for us ;)
>

Yes, I agree. I spoke to Andrea and he said that transparent pages
should not affect us much directly. He also added support for memcg,
but this is definitely something we should watch for.

--
Balbir

2010-01-07 09:07:19

[permalink] [raw]

Subject: Re: [PATCH -mmotm] memcg: implement memory thresholds document fixes

On Thu, 7 Jan 2010 14:22:56 +0530
Balbir Singh <[email protected]> wrote:

> * KAMEZAWA Hiroyuki <[email protected]> [2010-01-07 10:18:05]:
>
> > On Thu, 7 Jan 2010 09:57:14 +0900
> > Daisuke Nishimura <[email protected]> wrote:
> >
> > > Each memcg-implement-memory-thresholds.patch and
> > > memcg-add-interface-to-move-charge-at-task-migration.patch try to add a new
> > > section to Documentation/cgroup/memory.txt, so the document has been a bit
> > > mangled when these patches are merged at the same time.
> > >
> > > This patch fixes it.
> > >
> >
> > Acked-by: KAMEZAWA Hiroyuki <[email protected]>
> >
> > BTW, I'll prepare total update for memcg (especially around percpu counter).
> > Do you have something may conflict in plan ?
>
> Kame, could you clarify percpu counter? Is this on for resource
> counter scalability patches I had?
>
No.

Now. memcg's percpu counter uses following code.
==
static inline void
__mem_cgroup_stat_set_safe(struct mem_cgroup_stat_cpu *stat,
enum mem_cgroup_stat_index idx, s64 val)
{
stat->count[idx] = val;
}
static int mem_cgroup_size(void)
{
int cpustat_size = nr_cpu_ids * sizeof(struct mem_cgroup_stat_cpu);
return sizeof(struct mem_cgroup) + cpustat_size;
}

static struct mem_cgroup *mem_cgroup_alloc(void)
{
struct mem_cgroup *mem;
int size = mem_cgroup_size();

if (size < PAGE_SIZE)
mem = kmalloc(size, GFP_KERNEL);
else
mem = vmalloc(size);

==

But this is not NUMA-aware and slow. i.e. BAD.

Now, we have good codes for percpu_alloc(). we should use it.

like this => http://patchwork.kernel.org/patch/58662/

Things will be simplified.
I need to rewrite all to catch up recent changes _AND_ we have to
detect why 2 seconds of overhead is added by threshold patches.
And hopefuly, reduce it. I think softlimit/threshold event counter
can be rewritten in unified clean way.

Thanks,
-Kame

2010-01-07 11:23:21

[permalink] [raw]

Subject: How should we handle CONFIG_CGROUP_MEM_RES_CTRL_SWAP (Re: [PATCH -mmotm] build fix for memcg-move-charges-of-anonymous-swap.patch)

(Changed the subject and Cc list)

On Thu, 7 Jan 2010 14:52:23 +0900
KAMEZAWA Hiroyuki <[email protected]> wrote:

> BTW, maybe it's time to
> - remove EXPERIMENTAL from CONFIG_CGROUP_MEM_RES_CTRL_SWAP
> and more,
> - remove CONFIG_CGROUP_MEM_RES_CTRL_SWAP
> (to reduce complicated #ifdefs and replace them with CONFIG_SWAP.)
>
> It's very stable as far as I test.
>
I agree on both.

Balbir-san, What do you think ?

2010-01-07 12:38:49

by Balbir Singh

[permalink] [raw]

Subject: Re: How should we handle CONFIG_CGROUP_MEM_RES_CTRL_SWAP (Re: [PATCH -mmotm] build fix for memcg-move-charges-of-anonymous-swap.patch)

* Daisuke Nishimura <[email protected]> [2010-01-07 20:23:35]:

> (Changed the subject and Cc list)
>
> On Thu, 7 Jan 2010 14:52:23 +0900
> KAMEZAWA Hiroyuki <[email protected]> wrote:
>
> > BTW, maybe it's time to
> > - remove EXPERIMENTAL from CONFIG_CGROUP_MEM_RES_CTRL_SWAP
> > and more,
> > - remove CONFIG_CGROUP_MEM_RES_CTRL_SWAP
> > (to reduce complicated #ifdefs and replace them with CONFIG_SWAP.)
> >
> > It's very stable as far as I test.
> >
> I agree on both.
>
> Balbir-san, What do you think ?
>

I agree, the experimental marking can go and CONFIG_SWAP can replace
the current config option.

--
Balbir

2010-01-07 16:05:07

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (squashfs+lib/decomp)

On Wed, 06 Jan 2010 23:01:47 -0500 [email protected] wrote:

> On Wed, 06 Jan 2010 22:48:16 EST, [email protected] said:
>
> > Building with CONFIG_SQUASHFS=m, CONFIG_SQUASHFS_LZMA=y ,
> > and CONFIG_DECOMPRESS_LZMA=n fails:
>
> > Looks like a missing select/depends for DECOMPRESS_LZMA. Somebody else can
> > decide which it should be...
>
> Digging further:
>
> x Symbol: DECOMPRESS_LZMA [=n] x
> x Selected by: RD_LZMA [=n] && BLK_DEV_INITRD [=y] || SQUASHFS_LZMA [=y x
>
> How the heck did this happen? Looks like a SELECT *is* there but it's
> not firing??!?

I saw this build error in linux-next and sent a patch for it -- it's below.
However, it doesn't appear to be exactly the same config as yours.

---
From: Randy Dunlap <[email protected]>

When CONFIG_SQUASHFS=m and CONFIG_DECOMPRESS_LZMA=m, decompress_lzma
is built but then discarded from the library because no built-in code
uses it, so change it from a lib- to an obj- to force it to be kept
in the library.

ERROR: "unlzma" [fs/squashfs/squashfs.ko] undefined!

Signed-off-by: Randy Dunlap <[email protected]>
Cc: Phillip Lougher <[email protected]>
Cc: Michal Marek <[email protected]>
---
lib/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-next-20100101.orig/lib/Makefile
+++ linux-next-20100101/lib/Makefile
@@ -69,7 +69,7 @@ obj-$(CONFIG_LZO_DECOMPRESS) += lzo/

lib-$(CONFIG_DECOMPRESS_GZIP) += decompress_inflate.o
lib-$(CONFIG_DECOMPRESS_BZIP2) += decompress_bunzip2.o
-lib-$(CONFIG_DECOMPRESS_LZMA) += decompress_unlzma.o
+obj-$(CONFIG_DECOMPRESS_LZMA) += decompress_unlzma.o

obj-$(CONFIG_TEXTSEARCH) += textsearch.o
obj-$(CONFIG_TEXTSEARCH_KMP) += ts_kmp.o

2010-01-07 16:49:53

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (kernel/cgroup)

On Thu, 7 Jan 2010 11:31:54 +0900 KAMEZAWA Hiroyuki wrote:

> On Wed, 6 Jan 2010 17:08:49 -0800
> Randy Dunlap <[email protected]> wrote:
>
> > On Wed, 06 Jan 2010 14:34:36 -0800 [email protected] wrote:
> >
> > > The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
> > >
> > > http://userweb.kernel.org/~akpm/mmotm/
> > >
> > > and will soon be available at
> > >
> > > git://zen-kernel.org/kernel/mmotm.git
> > >
> > > It contains the following patches against 2.6.33-rc3:
> >
> >
> > kernel/cgroup.c: In function 'cgroup_write_event_control':
> > kernel/cgroup.c:2949: error: implicit declaration of function 'eventfd_fget'
> > kernel/cgroup.c:2949: warning: assignment makes pointer from integer without a cast
> > kernel/cgroup.c:2955: error: implicit declaration of function 'eventfd_ctx_fileget'
> > kernel/cgroup.c:2955: warning: assignment makes pointer from integer without a cast
> > make[2]: *** [kernel/cgroup.o] Error 1
> >
>
> Hmm, how about this rather than adding #ifdefs..
> Paul, Li, how do you think ?
> ==
> Cgroup's eventdf feature has depenedecy to EVENTFD
>
> This is a patch onto
> cgroup-implement-eventfd-based-generic-api-for-notifications.patch
>
> CC: Li Zefan <[email protected]>
> CC: Kirill A. Shutemov <[email protected]>
> CC: Paul Menage <[email protected]>
> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>

Acked-by: Randy Dunlap <[email protected]>

Thanks.

> ---
> init/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: ref-mmotm/init/Kconfig
> ===================================================================
> --- ref-mmotm.orig/init/Kconfig
> +++ ref-mmotm/init/Kconfig
> @@ -509,6 +509,7 @@ endchoice
>
> menuconfig CGROUPS
> boolean "Control Group support"
> + depends on EVENTFD
> help
> This option adds support for grouping sets of processes together, for
> use with process control subsystems such as Cpusets, CFS, memory
>

---
~Randy

2010-01-07 17:14:14

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded

2010-01-07 17:29:44

by Kirill A. Shutemov

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (kernel/cgroup)

On Thu, Jan 7, 2010 at 4:31 AM, KAMEZAWA Hiroyuki
<[email protected]> wrote:
> On Wed, 6 Jan 2010 17:08:49 -0800
> Randy Dunlap <[email protected]> wrote:
>
>> On Wed, 06 Jan 2010 14:34:36 -0800 [email protected] wrote:
>>
>> > The mm-of-the-moment snapshot 2010-01-06-14-34 has been uploaded to
>> >
>> > http://userweb.kernel.org/~akpm/mmotm/
>> >
>> > and will soon be available at
>> >
>> > git://zen-kernel.org/kernel/mmotm.git
>> >
>> > It contains the following patches against 2.6.33-rc3:
>>
>>
>> kernel/cgroup.c: In function 'cgroup_write_event_control':
>> kernel/cgroup.c:2949: error: implicit declaration of function 'eventfd_fget'
>> kernel/cgroup.c:2949: warning: assignment makes pointer from integer without a cast
>> kernel/cgroup.c:2955: error: implicit declaration of function 'eventfd_ctx_fileget'
>> kernel/cgroup.c:2955: warning: assignment makes pointer from integer without a cast
>> make[2]: *** [kernel/cgroup.o] Error 1
>>
>
> Hmm, how about this rather than adding #ifdefs..
> Paul, Li, how do you think ?
> ==
> Cgroup's eventdf feature has depenedecy to EVENTFD
>
> This is a patch onto
> cgroup-implement-eventfd-based-generic-api-for-notifications.patch
>
> CC: Li Zefan <[email protected]>
> CC: Kirill A. Shutemov <[email protected]>
> CC: Paul Menage <[email protected]>
> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
>
> ---
> init/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: ref-mmotm/init/Kconfig
> ===================================================================
> --- ref-mmotm.orig/init/Kconfig
> +++ ref-mmotm/init/Kconfig
> @@ -509,6 +509,7 @@ endchoice
>
> menuconfig CGROUPS
> boolean "Control Group support"
> + depends on EVENTFD
> help
> This option adds support for grouping sets of processes together, for
> use with process control subsystems such as Cpusets, CFS, memory
>
>

Acked-by: Kirill A. Shutemov <[email protected]>

2010-01-08 06:55:45

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (squashfs+lib/decomp)

On Thu, 07 Jan 2010 08:04:02 PST, Randy Dunlap said:
> On Wed, 06 Jan 2010 23:01:47 -0500 [email protected] wrote:
>
> > On Wed, 06 Jan 2010 22:48:16 EST, [email protected] said:
> >
> > > Building with CONFIG_SQUASHFS=m, CONFIG_SQUASHFS_LZMA=y ,
> > > and CONFIG_DECOMPRESS_LZMA=n fails:
> >
> > > Looks like a missing select/depends for DECOMPRESS_LZMA. Somebody else can
> > > decide which it should be...
> >
> > Digging further:
> >
> > x Symbol: DECOMPRESS_LZMA [=n] x
> > x Selected by: RD_LZMA [=n] && BLK_DEV_INITRD [=y] || SQUASHFS_LZMA [=y x
> >
> > How the heck did this happen? Looks like a SELECT *is* there but it's
> > not firing??!?
>
> I saw this build error in linux-next and sent a patch for it -- it's below.
> However, it doesn't appear to be exactly the same config as yours.

Yeah, that patch is how you ended up cc'ed - it was already in the linux-next
patch in Andrew's -mmotm tarball. It looked like you addressed one corner case
of the problem, and I tripped over another. But now I'm trying to get my
brain wrapped around how I was able to get SQUASHFS_LZMA=y, and it had a
select for DECOMPRESS_LZMA, but that *still* was 'n' anyhow? How did we end
up with both && and || in that 'Selected by:' line? Is that saying that
RD_LZMA needs to be on too?

Adding some plausible-sounding kbuild-related entries from MAINTAINERS to
the cc: list, maybe they can explain it...

Attachments:

(No filename) (227.00 B)

2010-01-08 16:38:52

[permalink] [raw]

Subject: Re: mmotm 2010-01-06-14-34 uploaded (squashfs+lib/decomp)

On Fri, 08 Jan 2010 01:54:54 -0500 [email protected] wrote:

> On Thu, 07 Jan 2010 08:04:02 PST, Randy Dunlap said:
> > On Wed, 06 Jan 2010 23:01:47 -0500 [email protected] wrote:
> >
> > > On Wed, 06 Jan 2010 22:48:16 EST, [email protected] said:
> > >
> > > > Building with CONFIG_SQUASHFS=m, CONFIG_SQUASHFS_LZMA=y ,
> > > > and CONFIG_DECOMPRESS_LZMA=n fails:
> > >
> > > > Looks like a missing select/depends for DECOMPRESS_LZMA. Somebody else can
> > > > decide which it should be...
> > >
> > > Digging further:
> > >
> > > x Symbol: DECOMPRESS_LZMA [=n] x
> > > x Selected by: RD_LZMA [=n] && BLK_DEV_INITRD [=y] || SQUASHFS_LZMA [=y x
> > >
> > > How the heck did this happen? Looks like a SELECT *is* there but it's
> > > not firing??!?
> >
> > I saw this build error in linux-next and sent a patch for it -- it's below.
> > However, it doesn't appear to be exactly the same config as yours.
>
> Yeah, that patch is how you ended up cc'ed - it was already in the linux-next
> patch in Andrew's -mmotm tarball. It looked like you addressed one corner case
> of the problem, and I tripped over another. But now I'm trying to get my
> brain wrapped around how I was able to get SQUASHFS_LZMA=y, and it had a
> select for DECOMPRESS_LZMA, but that *still* was 'n' anyhow? How did we end
> up with both && and || in that 'Selected by:' line? Is that saying that
> RD_LZMA needs to be on too?

The menuconfig display is truncated (use left/right arrow keys to pan it).
The xconfig display shows this:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
DECOMPRESS_LZMA

type: tristate
reverse dep: RD_LZMA && BLK_DEV_INITRD || SQUASHFS_LZMA && MISC_FILESYSTEMS && SQUASHFS

defined at lib/Kconfig:117

There is no help available for this kernel option.
Symbol: DECOMPRESS_LZMA [=n]
Selected by: RD_LZMA [=n] && BLK_DEV_INITRD [=n] || SQUASHFS_LZMA [=n] && MISC_FILESYSTEMS [=n] && SQUASHFS [=n]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In the "reverse dep:" line, && has higher precedence that || does, so it's:

reverse dep: (RD_LZMA && BLK_DEV_INITRD) || (SQUASHFS_LZMA && MISC_FILESYSTEMS && SQUASHFS)

So RD_LZMA and BLK_DEV_INITRD don't matter.

> Adding some plausible-sounding kbuild-related entries from MAINTAINERS to
> the cc: list, maybe they can explain it...

---
~Randy

2010-01-08 22:22:14