All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Mel Gorman <mgorman@suse.de>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Rik van Riel <riel@redhat.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Minchan Kim <minchan@kernel.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH 6/6] mm: use atomic bit operations in set_pageblock_flags_group()
Date: Mon, 3 Mar 2014 17:28:46 +0900	[thread overview]
Message-ID: <20140303082846.GB28899@lge.com> (raw)
In-Reply-To: <1393596904-16537-7-git-send-email-vbabka@suse.cz>

On Fri, Feb 28, 2014 at 03:15:04PM +0100, Vlastimil Babka wrote:
> set_pageblock_flags_group() is used to set either migratetype or skip bit of a
> pageblock. Setting migratetype is done under zone->lock (except from __init
> code), however changing the skip bits is not protected and the pageblock flags
> bitmap packs migratetype and skip bits together and uses non-atomic bit ops.
> Therefore, races between setting migratetype and skip bit are possible and the
> non-atomic read-modify-update of the skip bit may cause lost updates to
> migratetype bits, resulting in invalid migratetype values, which are in turn
> used to e.g. index free_list array.
> 
> The race has been observed to happen and cause panics, albeit during
> development of series that increases frequency of migratetype changes through
> {start,undo}_isolate_page_range() calls.
> 
> Two possible solutions were investigated: 1) using zone->lock for changing
> pageblock_skip bit and 2) changing the bitmap operations to be atomic. The
> problem of 1) is that zone->lock is already contended and almost never held in
> the compaction code that updates pageblock_skip bits. Solution 2) should scale
> better, but adds atomic operations also to migratype changes which are already
> protected by zone->lock.

How about 3) introduce new bitmap for pageblock_skip?
I guess that migratetype bitmap is read-intensive and set/clear pageblock_skip
could make performance degradation.

> 
> Using mmtests' stress-highalloc benchmark, little difference was found between
> the two solutions. The base is 3.13 with recent compaction series by myself and
> Joonsoo Kim applied.
> 
>                 3.13        3.13        3.13
>                 base     2)atomic     1)lock
> User         6103.92     6072.09     6178.79
> System       1039.68     1033.96     1042.92
> Elapsed      2114.27     2090.20     2110.23
> 

I really wonder how 2) is better than base although there is a little difference.
Is it the avg result of 10 runs? Do you have any idea what happens?

Thanks.

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

WARNING: multiple messages have this Message-ID (diff)
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Mel Gorman <mgorman@suse.de>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Rik van Riel <riel@redhat.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Minchan Kim <minchan@kernel.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH 6/6] mm: use atomic bit operations in set_pageblock_flags_group()
Date: Mon, 3 Mar 2014 17:28:46 +0900	[thread overview]
Message-ID: <20140303082846.GB28899@lge.com> (raw)
In-Reply-To: <1393596904-16537-7-git-send-email-vbabka@suse.cz>

On Fri, Feb 28, 2014 at 03:15:04PM +0100, Vlastimil Babka wrote:
> set_pageblock_flags_group() is used to set either migratetype or skip bit of a
> pageblock. Setting migratetype is done under zone->lock (except from __init
> code), however changing the skip bits is not protected and the pageblock flags
> bitmap packs migratetype and skip bits together and uses non-atomic bit ops.
> Therefore, races between setting migratetype and skip bit are possible and the
> non-atomic read-modify-update of the skip bit may cause lost updates to
> migratetype bits, resulting in invalid migratetype values, which are in turn
> used to e.g. index free_list array.
> 
> The race has been observed to happen and cause panics, albeit during
> development of series that increases frequency of migratetype changes through
> {start,undo}_isolate_page_range() calls.
> 
> Two possible solutions were investigated: 1) using zone->lock for changing
> pageblock_skip bit and 2) changing the bitmap operations to be atomic. The
> problem of 1) is that zone->lock is already contended and almost never held in
> the compaction code that updates pageblock_skip bits. Solution 2) should scale
> better, but adds atomic operations also to migratype changes which are already
> protected by zone->lock.

How about 3) introduce new bitmap for pageblock_skip?
I guess that migratetype bitmap is read-intensive and set/clear pageblock_skip
could make performance degradation.

> 
> Using mmtests' stress-highalloc benchmark, little difference was found between
> the two solutions. The base is 3.13 with recent compaction series by myself and
> Joonsoo Kim applied.
> 
>                 3.13        3.13        3.13
>                 base     2)atomic     1)lock
> User         6103.92     6072.09     6178.79
> System       1039.68     1033.96     1042.92
> Elapsed      2114.27     2090.20     2110.23
> 

I really wonder how 2) is better than base although there is a little difference.
Is it the avg result of 10 runs? Do you have any idea what happens?

Thanks.


  reply	other threads:[~2014-03-03  8:28 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-28 14:14 [PATCH 0/6] close pageblock_migratetype and pageblock_skip races Vlastimil Babka
2014-02-28 14:14 ` Vlastimil Babka
2014-02-28 14:14 ` [PATCH 1/6] mm: call get_pageblock_migratetype() under zone->lock where possible Vlastimil Babka
2014-02-28 14:14   ` Vlastimil Babka
2014-02-28 14:15 ` [PATCH 2/6] mm: add get_pageblock_migratetype_nolock() for cases where locking is undesirable Vlastimil Babka
2014-02-28 14:15   ` Vlastimil Babka
2014-03-03  8:22   ` Joonsoo Kim
2014-03-03  8:22     ` Joonsoo Kim
2014-03-03 13:54     ` Vlastimil Babka
2014-03-03 13:54       ` Vlastimil Babka
2014-03-04  0:55       ` Joonsoo Kim
2014-03-04  0:55         ` Joonsoo Kim
2014-03-04 12:16         ` Vlastimil Babka
2014-03-04 12:16           ` Vlastimil Babka
2014-03-05  0:29           ` Joonsoo Kim
2014-03-05  0:29             ` Joonsoo Kim
2014-03-05  0:37   ` Joonsoo Kim
2014-03-05  0:37     ` Joonsoo Kim
2014-02-28 14:15 ` [PATCH 3/6] mm: add is_migrate_isolate_page_nolock() " Vlastimil Babka
2014-02-28 14:15   ` Vlastimil Babka
2014-03-05  0:39   ` Joonsoo Kim
2014-03-05  0:39     ` Joonsoo Kim
2014-02-28 14:15 ` [PATCH 4/6] mm: add set_pageblock_migratetype_nolock() for calls outside zone->lock Vlastimil Babka
2014-02-28 14:15   ` Vlastimil Babka
2014-02-28 14:15 ` [PATCH 5/6] mm: compaction: do not set pageblock skip bit when already set Vlastimil Babka
2014-02-28 14:15   ` Vlastimil Babka
2014-02-28 14:15 ` [PATCH 6/6] mm: use atomic bit operations in set_pageblock_flags_group() Vlastimil Babka
2014-02-28 14:15   ` Vlastimil Babka
2014-03-03  8:28   ` Joonsoo Kim [this message]
2014-03-03  8:28     ` Joonsoo Kim
2014-03-03 12:46     ` Vlastimil Babka
2014-03-03 12:46       ` Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140303082846.GB28899@lge.com \
    --to=iamjoonsoo.kim@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan@kernel.org \
    --cc=riel@redhat.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.