Hugh Dickins | 1 May 04:35 2011
Picon

[PATCH] mmotm: fix hang at startup

Yesterday's mmotm hangs at startup, and with lockdep it reports:
BUG: spinlock recursion on CPU#1, blkid/284 - with bdi_lock_two()
called from bdev_inode_switch_bdi() in the backtrace.  It appears
that this function is sometimes called with new the same as old.

Signed-off-by: Hugh Dickins <hughd <at> google.com>
---
Fix to
writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock.patch

 fs/block_dev.c |    2 ++
 1 file changed, 2 insertions(+)

--- 2.6.39-rc5-mm1/fs/block_dev.c	2011-04-29 18:20:09.183314733 -0700
+++ linux/fs/block_dev.c	2011-04-30 17:55:45.718785263 -0700
 <at>  <at>  -57,6 +57,8  <at>  <at>  static void bdev_inode_switch_bdi(struct
 {
 	struct backing_dev_info *old = inode->i_data.backing_dev_info;

+	if (dst == old)
+		return;
 	bdi_lock_two(&old->wb, &dst->wb);
 	spin_lock(&inode->i_lock);
 	inode->i_data.backing_dev_info = dst;
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Wu Fengguang | 1 May 05:25 2011
Picon

Re: [PATCH] mmotm: fix hang at startup

On Sun, May 01, 2011 at 10:35:38AM +0800, Hugh Dickins wrote:
> Yesterday's mmotm hangs at startup, and with lockdep it reports:
> BUG: spinlock recursion on CPU#1, blkid/284 - with bdi_lock_two()
> called from bdev_inode_switch_bdi() in the backtrace.  It appears
> that this function is sometimes called with new the same as old.
>
> Signed-off-by: Hugh Dickins <hughd <at> google.com>

Thanks!

Reviewed-by: Wu Fengguang <fengguang.wu <at> intel.com>

> Fix to
> writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock.patch
> 
>  fs/block_dev.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> --- 2.6.39-rc5-mm1/fs/block_dev.c	2011-04-29 18:20:09.183314733 -0700
> +++ linux/fs/block_dev.c	2011-04-30 17:55:45.718785263 -0700
>  <at>  <at>  -57,6 +57,8  <at>  <at>  static void bdev_inode_switch_bdi(struct
>  {
>  	struct backing_dev_info *old = inode->i_data.backing_dev_info;
>  
> +	if (dst == old)
> +		return;

nitpick: it could help to add a comment

        /* avoid spinlock recursion */
(Continue reading)

KOSAKI Motohiro | 1 May 08:06 2011

Re: [PATCH 1/7] memcg: add high/low watermark to res_counter

> On Mon 25-04-11 18:28:49, KAMEZAWA Hiroyuki wrote:
> > There are two watermarks added per-memcg including "high_wmark" and "low_wmark".
> > The per-memcg kswapd is invoked when the memcg's memory usage(usage_in_bytes)
> > is higher than the low_wmark. Then the kswapd thread starts to reclaim pages
> > until the usage is lower than the high_wmark.
> 
> I have mentioned this during Ying's patchsets already, but do we really
> want to have this confusing naming? High and low watermarks have
> opposite semantic for zones.

Can you please clarify this? I feel it is not opposite semantics.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo <at> kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont <at> kvack.org"> email <at> kvack.org </a>

KOSAKI Motohiro | 1 May 09:27 2011

Re: [RFC 2/8] compaction: make isolate_lru_page with filter aware

> > With the suggested flags argument from 1/8, this would look like:
> >
> >        flags = ISOLATE_BOTH;
> >        if (!cc->sync)
> >                flags |= ISOLATE_CLEAN;
> >
> > ?
> 
> Yes. I will change it.
> 
> >
> > Anyway, nice change indeed!
> 
> Thanks!

Yeah. That's very nice.
	Reviewed-by: KOSAKI Motohiro <kosaki.motohiro <at> jp.fujitsu.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo <at> kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont <at> kvack.org"> email <at> kvack.org </a>

KOSAKI Motohiro | 1 May 09:47 2011

Re: mmotm 2011-04-29-16-25 uploaded

> On Fri, 29 Apr 2011 16:26:16 -0700 akpm <at> linux-foundation.org wrote:
> 
> > The mm-of-the-moment snapshot 2011-04-29-16-25 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.39-rc5:
> 
> 
> mm-per-node-vmstat-show-proper-vmstats.patch
> 
> when CONFIG_PROC_FS is not enabled:
> 
> drivers/built-in.o: In function `node_read_vmstat':
> node.c:(.text+0x1e995): undefined reference to `vmstat_text'
> 
> from drivers/base/node.c

Thank you for finding that!

From 63ad7c06f082f8423c033b9f54070e14d561db7e Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro <at> jp.fujitsu.com>
Date: Sun, 1 May 2011 16:00:09 +0900
Subject: [PATCH] vmstat: fix build error when SYSFS=y and PROC_FS=n

Randy Dunlap pointed out node.c makes build error when
(Continue reading)

KOSAKI Motohiro | 1 May 15:13 2011

Re: [RFC 4/8] Make clear description of putback_lru_page

> On Thu, Apr 28, 2011 at 08:20:32AM +0900, Minchan Kim wrote:
> > On Wed, Apr 27, 2011 at 5:11 PM, KAMEZAWA Hiroyuki
> > <kamezawa.hiroyu <at> jp.fujitsu.com> wrote:
> > > On Wed, 27 Apr 2011 01:25:21 +0900
> > > Minchan Kim <minchan.kim <at> gmail.com> wrote:
> > >
> > >> Commonly, putback_lru_page is used with isolated_lru_page.
> > >> The isolated_lru_page picks the page in middle of LRU and
> > >> putback_lru_page insert the lru in head of LRU.
> > >> It means it could make LRU churning so we have to be very careful.
> > >> Let's clear description of putback_lru_page.
> > >>
> > >> Cc: KOSAKI Motohiro <kosaki.motohiro <at> jp.fujitsu.com>
> > >> Cc: Mel Gorman <mgorman <at> suse.de>
> > >> Cc: Rik van Riel <riel <at> redhat.com>
> > >> Cc: Andrea Arcangeli <aarcange <at> redhat.com>
> > >> Signed-off-by: Minchan Kim <minchan.kim <at> gmail.com>
> > >
> > > seems good...
> > > Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu <at> jp.fujitsu.com>
> > >
> > > But is there consensus which side of LRU is tail? head?
> > 
> > I don't know. I used to think it's head.
> > If other guys raise a concern as well, let's talk about it. :)
> > Thanks
> 
> I suppose we add new pages to the head of the LRU and reclaim old
> pages from the tail.
> 
(Continue reading)

KOSAKI Motohiro | 1 May 15:19 2011

Re: [RFC 5/8] compaction: remove active list counting

> On Thu, Apr 28, 2011 at 7:50 PM, Mel Gorman <mgorman <at> suse.de> wrote:
> > On Wed, Apr 27, 2011 at 01:25:22AM +0900, Minchan Kim wrote:
> >> acct_isolated of compaction uses page_lru_base_type which returns only
> >> base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE.
> >> So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.
> >>
> >> Cc: KOSAKI Motohiro <kosaki.motohiro <at> jp.fujitsu.com>
> >> Cc: Mel Gorman <mgorman <at> suse.de>
> >> Cc: Rik van Riel <riel <at> redhat.com>
> >> Cc: Andrea Arcangeli <aarcange <at> redhat.com>
> >> Signed-off-by: Minchan Kim <minchan.kim <at> gmail.com>
> >
> > hmm, isolate_migratepages() is doing a linear scan of PFNs and is
> > calling __isolate_lru_page(..ISOLATE_BOTH..). Using page_lru_base_type
> > happens to work because we're only interested in the number of isolated
> > pages and your patch still covers that. Using page_lru might be more
> > accurate in terms of accountancy but does not seem necessary.
> 
> True.
> 
> >
> > Adding a comment explaining why we account for it as inactive and why
> > that's ok would be nice although I admit this is something I should have
> > done when acct_isolated() was introduced.
> 
> When Kame pointed out comment, I wanted to avoid unnecessary comment
> so decided changing it with page_lru although it adds overhead a
> little bit. But Hannes, you and maybe Kame don't want it. I don't mind
> adding comment.
> Okay. fix it in next version.
(Continue reading)

KOSAKI Motohiro | 1 May 15:47 2011

Re: [RFC 6/8] In order putback lru core

> > +/* This structure is used for keeping LRU ordering of isolated page */
> > +struct pages_lru {
> > +        struct page *page;      /* isolated page */
> > +        struct page *prev_page; /* previous page of isolate page as LRU order */
> > +        struct page *next_page; /* next page of isolate page as LRU order */
> > +        struct list_head lru;
> > +};
> >  /*
> 
> So this thing has to be allocated from somewhere. We can't put it
> on the stack as we're already in danger there so we must be using
> kmalloc. In the reclaim paths, this should be avoided obviously.
> For compaction, we might hurt the compaction success rates if pages
> are pinned with control structures. It's something to be wary of.
> 
> At LSF/MM, I stated a preference for swapping the source and
> destination pages in the LRU. This unfortunately means that the LRU
> now contains a page in the process of being migrated to and the backout
> paths for migration failure become a lot more complex. Reclaim should
> be ok as it'll should fail to lock the page and recycle it in the list.
> This avoids allocations but I freely admit that I'm not in the position
> to implement such a thing right now :(

I like swaping to fake page. one way pointer might become dangerous. vmscan can
detect fake page and ignore it.

ie, 
is_fake_page(page)
{
	if (is_stack_addr((void*)page))
(Continue reading)

Minchan Kim | 1 May 17:03 2011
Picon

[PATCH 0/2] Fix an enhance deactive_page

A few days ago, Ying reported a problem.
http://marc.info/?l=linux-mm&m=130403310601663&w=2
After I and Ying dive in problem, We found deactive_page
has a problem. Apparently, It's a BUG so 
[1/2] is fix of the problem and [2/2] is enhancement for 
helping CPU.

Minchan Kim (2):
  [1/2] Check PageUnevictable in lru_deactivate_fn
  [2/2] Filter unevictable page out in deactivate_page

 mm/swap.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo <at> kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont <at> kvack.org"> email <at> kvack.org </a>

Minchan Kim | 1 May 17:03 2011
Picon

[PATCH 1/2] Check PageUnevictable in lru_deactivate_fn

The lru_deactivate_fn should not move page which in on unevictable lru
into inactive list. Otherwise, we can meet BUG when we use isolate_lru_pages
as __isolate_lru_page could return -EINVAL.
It's really BUG and let's fix it.

Reported-by: Ying Han <yinghan <at> google.com>
Tested-by: Ying Han <yinghan <at> google.com>
Signed-off-by: Minchan Kim <minchan.kim <at> gmail.com>
---
 mm/swap.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/mm/swap.c b/mm/swap.c
index a83ec5a..2e9656d 100644
--- a/mm/swap.c
+++ b/mm/swap.c
 <at>  <at>  -429,6 +429,9  <at>  <at>  static void lru_deactivate_fn(struct page *page, void *arg)
 	if (!PageLRU(page))
 		return;

+	if (PageUnevictable(page))
+		return;
+
 	/* Some processes are using the page */
 	if (page_mapped(page))
 		return;
--

-- 
1.7.1

--
(Continue reading)


Gmane