From: vbabka@suse.cz (Vlastimil Babka)
To: linux-arm-kernel@lists.infradead.org
Subject: Suspicious error for CMA stress test
Date: Sat, 19 Mar 2016 23:11:24 +0100 [thread overview]
Message-ID: <56EDCE8C.8030607@suse.cz> (raw)
In-Reply-To: <56ECFEAC.3010606@huawei.com>
On 03/19/2016 08:24 AM, Hanjun Guo wrote:
> On 2016/3/18 22:10, Vlastimil Babka wrote:
>>>>
>>>> Oh, ok, will try to send proper patch, once I figure out what to write in
>>>> the changelog :)
>>> Thanks in advance!
>>>
>> OK, here it is. Hanjun can you please retest this, as I'm not sure if you had
>
> I tested this new patch with stress for more than one hour, and it works!
That's good news, thanks!
> Since Lucas has comments on it, I'm willing to test further versions if needed.
>
> One minor comments below,
>
>> the same code due to the followup one-liner patches in the thread. Lucas, see if
>> it helps with your issue as well. Laura and Joonsoo, please also test and review
>> and check changelog if my perception of the problem is accurate :)
>>
>> Thanks
>>
> [...]
>> + if (max_order < MAX_ORDER) {
>> + /* If we are here, it means order is >= pageblock_order.
>> + * We want to prevent merge between freepages on isolate
>> + * pageblock and normal pageblock. Without this, pageblock
>> + * isolation could cause incorrect freepage or CMA accounting.
>> + *
>> + * We don't want to hit this code for the more frequent
>> + * low-order merging.
>> + */
>> + if (unlikely(has_isolate_pageblock(zone))) {
>
> In the first version of your patch, it's
>
> + if (IS_ENABLED(CONFIG_CMA) &&
> + unlikely(has_isolate_pageblock(zone))) {
>
> Why remove the IS_ENABLED(CONFIG_CMA) in the new version?
Previously I thought the problem was CMA-specific, but after more detailed look
I think it's not, as start_isolate_page_range() releases zone lock between
pageblocks, so unexpected merging due to races can happen also between isolated
and non-isolated non-CMA pageblocks. This function is called from memory hotplug
code, and recently also alloc_contig_range() itself is outside CONFIG_CMA for
allocating gigantic hugepages. Joonsoo's original commit 3c60509 was also not
restricted to CMA, and same with his patch earlier in this thread.
Hmm I guess another alternate solution would indeed be to modify
start_isolate_page_range() and undo_isolate_page_range() to hold zone->lock
across MAX_ORDER blocks (not whole requested range, as that could lead to
hardlockups). But that still wouldn't help Lucas, IUUC.
> Thanks
> Hanjun
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Vlastimil Babka <vbabka@suse.cz>
To: Hanjun Guo <guohanjun@huawei.com>, Joonsoo Kim <js1304@gmail.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
"Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>,
Laura Abbott <labbott@redhat.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Sasha Levin <sasha.levin@oracle.com>,
Laura Abbott <lauraa@codeaurora.org>,
qiuxishi <qiuxishi@huawei.com>,
Catalin Marinas <Catalin.Marinas@arm.com>,
Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de>,
dingtinahong <dingtianhong@huawei.com>,
chenjie6@huawei.com, "linux-mm@kvack.org" <linux-mm@kvack.org>,
Lucas Stach <l.stach@pengutronix.de>
Subject: Re: Suspicious error for CMA stress test
Date: Sat, 19 Mar 2016 23:11:24 +0100 [thread overview]
Message-ID: <56EDCE8C.8030607@suse.cz> (raw)
In-Reply-To: <56ECFEAC.3010606@huawei.com>
On 03/19/2016 08:24 AM, Hanjun Guo wrote:
> On 2016/3/18 22:10, Vlastimil Babka wrote:
>>>>
>>>> Oh, ok, will try to send proper patch, once I figure out what to write in
>>>> the changelog :)
>>> Thanks in advance!
>>>
>> OK, here it is. Hanjun can you please retest this, as I'm not sure if you had
>
> I tested this new patch with stress for more than one hour, and it works!
That's good news, thanks!
> Since Lucas has comments on it, I'm willing to test further versions if needed.
>
> One minor comments below,
>
>> the same code due to the followup one-liner patches in the thread. Lucas, see if
>> it helps with your issue as well. Laura and Joonsoo, please also test and review
>> and check changelog if my perception of the problem is accurate :)
>>
>> Thanks
>>
> [...]
>> + if (max_order < MAX_ORDER) {
>> + /* If we are here, it means order is >= pageblock_order.
>> + * We want to prevent merge between freepages on isolate
>> + * pageblock and normal pageblock. Without this, pageblock
>> + * isolation could cause incorrect freepage or CMA accounting.
>> + *
>> + * We don't want to hit this code for the more frequent
>> + * low-order merging.
>> + */
>> + if (unlikely(has_isolate_pageblock(zone))) {
>
> In the first version of your patch, it's
>
> + if (IS_ENABLED(CONFIG_CMA) &&
> + unlikely(has_isolate_pageblock(zone))) {
>
> Why remove the IS_ENABLED(CONFIG_CMA) in the new version?
Previously I thought the problem was CMA-specific, but after more detailed look
I think it's not, as start_isolate_page_range() releases zone lock between
pageblocks, so unexpected merging due to races can happen also between isolated
and non-isolated non-CMA pageblocks. This function is called from memory hotplug
code, and recently also alloc_contig_range() itself is outside CONFIG_CMA for
allocating gigantic hugepages. Joonsoo's original commit 3c60509 was also not
restricted to CMA, and same with his patch earlier in this thread.
Hmm I guess another alternate solution would indeed be to modify
start_isolate_page_range() and undo_isolate_page_range() to hold zone->lock
across MAX_ORDER blocks (not whole requested range, as that could lead to
hardlockups). But that still wouldn't help Lucas, IUUC.
> Thanks
> Hanjun
>
>
--
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: Vlastimil Babka <vbabka@suse.cz>
To: Hanjun Guo <guohanjun@huawei.com>, Joonsoo Kim <js1304@gmail.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
"Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>,
Laura Abbott <labbott@redhat.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Sasha Levin <sasha.levin@oracle.com>,
Laura Abbott <lauraa@codeaurora.org>,
qiuxishi <qiuxishi@huawei.com>,
Catalin Marinas <Catalin.Marinas@arm.com>,
Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de>,
dingtinahong <dingtianhong@huawei.com>,
chenjie6@huawei.com, "linux-mm@kvack.org" <linux-mm@kvack.org>,
Lucas Stach <l.stach@pengutronix.de>
Subject: Re: Suspicious error for CMA stress test
Date: Sat, 19 Mar 2016 23:11:24 +0100 [thread overview]
Message-ID: <56EDCE8C.8030607@suse.cz> (raw)
In-Reply-To: <56ECFEAC.3010606@huawei.com>
On 03/19/2016 08:24 AM, Hanjun Guo wrote:
> On 2016/3/18 22:10, Vlastimil Babka wrote:
>>>>
>>>> Oh, ok, will try to send proper patch, once I figure out what to write in
>>>> the changelog :)
>>> Thanks in advance!
>>>
>> OK, here it is. Hanjun can you please retest this, as I'm not sure if you had
>
> I tested this new patch with stress for more than one hour, and it works!
That's good news, thanks!
> Since Lucas has comments on it, I'm willing to test further versions if needed.
>
> One minor comments below,
>
>> the same code due to the followup one-liner patches in the thread. Lucas, see if
>> it helps with your issue as well. Laura and Joonsoo, please also test and review
>> and check changelog if my perception of the problem is accurate :)
>>
>> Thanks
>>
> [...]
>> + if (max_order < MAX_ORDER) {
>> + /* If we are here, it means order is >= pageblock_order.
>> + * We want to prevent merge between freepages on isolate
>> + * pageblock and normal pageblock. Without this, pageblock
>> + * isolation could cause incorrect freepage or CMA accounting.
>> + *
>> + * We don't want to hit this code for the more frequent
>> + * low-order merging.
>> + */
>> + if (unlikely(has_isolate_pageblock(zone))) {
>
> In the first version of your patch, it's
>
> + if (IS_ENABLED(CONFIG_CMA) &&
> + unlikely(has_isolate_pageblock(zone))) {
>
> Why remove the IS_ENABLED(CONFIG_CMA) in the new version?
Previously I thought the problem was CMA-specific, but after more detailed look
I think it's not, as start_isolate_page_range() releases zone lock between
pageblocks, so unexpected merging due to races can happen also between isolated
and non-isolated non-CMA pageblocks. This function is called from memory hotplug
code, and recently also alloc_contig_range() itself is outside CONFIG_CMA for
allocating gigantic hugepages. Joonsoo's original commit 3c60509 was also not
restricted to CMA, and same with his patch earlier in this thread.
Hmm I guess another alternate solution would indeed be to modify
start_isolate_page_range() and undo_isolate_page_range() to hold zone->lock
across MAX_ORDER blocks (not whole requested range, as that could lead to
hardlockups). But that still wouldn't help Lucas, IUUC.
> Thanks
> Hanjun
>
>
next prev parent reply other threads:[~2016-03-19 22:11 UTC|newest]
Thread overview: 176+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-02 13:52 Suspicious error for CMA stress test Hanjun Guo
2016-03-02 13:52 ` Hanjun Guo
2016-03-03 1:25 ` Laura Abbott
2016-03-03 1:25 ` Laura Abbott
2016-03-03 1:25 ` Laura Abbott
2016-03-03 6:07 ` Hanjun Guo
2016-03-03 6:07 ` Hanjun Guo
2016-03-03 6:07 ` Hanjun Guo
2016-03-03 7:42 ` Joonsoo Kim
2016-03-03 7:42 ` Joonsoo Kim
2016-03-03 7:42 ` Joonsoo Kim
2016-03-03 7:58 ` Hanjun Guo
2016-03-03 7:58 ` Hanjun Guo
2016-03-03 7:58 ` Hanjun Guo
2016-03-03 12:49 ` Hanjun Guo
2016-03-03 12:49 ` Hanjun Guo
2016-03-03 12:49 ` Hanjun Guo
2016-03-03 18:52 ` Laura Abbott
2016-03-03 18:52 ` Laura Abbott
2016-03-03 18:52 ` Laura Abbott
2016-03-04 2:09 ` Joonsoo Kim
2016-03-04 2:09 ` Joonsoo Kim
2016-03-04 2:09 ` Joonsoo Kim
2016-03-04 6:09 ` Hanjun Guo
2016-03-04 6:09 ` Hanjun Guo
2016-03-04 6:09 ` Hanjun Guo
2016-03-04 2:02 ` Joonsoo Kim
2016-03-04 2:02 ` Joonsoo Kim
2016-03-04 2:02 ` Joonsoo Kim
2016-03-04 4:32 ` Joonsoo Kim
2016-03-04 4:32 ` Joonsoo Kim
2016-03-04 4:32 ` Joonsoo Kim
2016-03-04 6:05 ` Hanjun Guo
2016-03-04 6:05 ` Hanjun Guo
2016-03-04 6:05 ` Hanjun Guo
2016-03-04 6:38 ` Joonsoo Kim
2016-03-04 6:38 ` Joonsoo Kim
2016-03-04 6:38 ` Joonsoo Kim
2016-03-04 7:35 ` Hanjun Guo
2016-03-04 7:35 ` Hanjun Guo
2016-03-04 7:35 ` Hanjun Guo
2016-03-07 4:34 ` Joonsoo Kim
2016-03-07 4:34 ` Joonsoo Kim
2016-03-07 4:34 ` Joonsoo Kim
2016-03-07 8:16 ` Leizhen (ThunderTown)
2016-03-07 8:16 ` Leizhen (ThunderTown)
2016-03-07 8:16 ` Leizhen (ThunderTown)
2016-03-07 18:42 ` Laura Abbott
2016-03-07 18:42 ` Laura Abbott
2016-03-07 18:42 ` Laura Abbott
2016-03-08 1:54 ` Leizhen (ThunderTown)
2016-03-08 1:54 ` Leizhen (ThunderTown)
2016-03-08 1:54 ` Leizhen (ThunderTown)
2016-03-09 1:23 ` Leizhen (ThunderTown)
2016-03-09 1:23 ` Leizhen (ThunderTown)
2016-03-09 1:23 ` Leizhen (ThunderTown)
2016-03-11 15:00 ` Joonsoo Kim
2016-03-11 15:00 ` Joonsoo Kim
2016-03-11 15:00 ` Joonsoo Kim
2016-03-11 17:07 ` Vlastimil Babka
2016-03-11 17:07 ` Vlastimil Babka
2016-03-11 17:07 ` Vlastimil Babka
2016-03-14 6:49 ` Joonsoo Kim
2016-03-14 6:49 ` Joonsoo Kim
2016-03-14 6:49 ` Joonsoo Kim
2016-03-14 7:06 ` Vlastimil Babka
2016-03-14 7:06 ` Vlastimil Babka
2016-03-14 7:06 ` Vlastimil Babka
2016-03-14 7:18 ` Joonsoo Kim
2016-03-14 7:18 ` Joonsoo Kim
2016-03-14 7:18 ` Joonsoo Kim
2016-03-14 12:30 ` Vlastimil Babka
2016-03-14 12:30 ` Vlastimil Babka
2016-03-14 12:30 ` Vlastimil Babka
2016-03-14 14:10 ` Joonsoo Kim
2016-03-14 14:10 ` Joonsoo Kim
2016-03-14 14:10 ` Joonsoo Kim
2016-03-16 12:03 ` Vlastimil Babka
2016-03-16 12:03 ` Vlastimil Babka
2016-03-16 12:03 ` Vlastimil Babka
2016-03-16 9:44 ` Hanjun Guo
2016-03-16 9:44 ` Hanjun Guo
2016-03-16 9:44 ` Hanjun Guo
2016-03-17 6:54 ` Joonsoo Kim
2016-03-17 6:54 ` Joonsoo Kim
2016-03-17 6:54 ` Joonsoo Kim
2016-03-17 9:24 ` Hanjun Guo
2016-03-17 9:24 ` Hanjun Guo
2016-03-17 9:24 ` Hanjun Guo
2016-03-17 15:31 ` Joonsoo Kim
2016-03-17 15:31 ` Joonsoo Kim
2016-03-17 15:31 ` Joonsoo Kim
2016-03-18 2:03 ` Hanjun Guo
2016-03-18 2:03 ` Hanjun Guo
2016-03-18 2:03 ` Hanjun Guo
2016-03-17 15:43 ` Vlastimil Babka
2016-03-17 15:43 ` Vlastimil Babka
2016-03-17 15:43 ` Vlastimil Babka
2016-03-17 15:52 ` Joonsoo Kim
2016-03-17 15:52 ` Joonsoo Kim
2016-03-17 15:52 ` Joonsoo Kim
2016-03-18 13:32 ` Lucas Stach
2016-03-18 13:32 ` Lucas Stach
2016-03-18 13:32 ` Lucas Stach
2016-03-21 4:42 ` Joonsoo Kim
2016-03-21 4:42 ` Joonsoo Kim
2016-03-21 4:42 ` Joonsoo Kim
2016-03-22 14:56 ` Lucas Stach
2016-03-22 14:56 ` Lucas Stach
2016-03-22 14:56 ` Lucas Stach
2016-03-23 4:42 ` Joonsoo Kim
2016-03-23 4:42 ` Joonsoo Kim
2016-03-23 4:42 ` Joonsoo Kim
2016-03-18 14:10 ` Vlastimil Babka
2016-03-18 14:10 ` Vlastimil Babka
2016-03-18 14:10 ` Vlastimil Babka
2016-03-18 14:42 ` Lucas Stach
2016-03-18 14:42 ` Lucas Stach
2016-03-18 14:42 ` Lucas Stach
2016-03-18 20:58 ` Vlastimil Babka
2016-03-18 20:58 ` Vlastimil Babka
2016-03-18 20:58 ` Vlastimil Babka
2016-03-22 14:47 ` Lucas Stach
2016-03-22 14:47 ` Lucas Stach
2016-03-22 14:47 ` Lucas Stach
2016-03-19 7:24 ` Hanjun Guo
2016-03-19 7:24 ` Hanjun Guo
2016-03-19 7:24 ` Hanjun Guo
2016-03-19 22:11 ` Vlastimil Babka [this message]
2016-03-19 22:11 ` Vlastimil Babka
2016-03-19 22:11 ` Vlastimil Babka
2016-03-23 4:44 ` Joonsoo Kim
2016-03-23 4:44 ` Joonsoo Kim
2016-03-23 4:44 ` Joonsoo Kim
2016-03-23 8:26 ` Vlastimil Babka
2016-03-23 8:26 ` Vlastimil Babka
2016-03-23 8:26 ` Vlastimil Babka
2016-03-23 8:32 ` Joonsoo Kim
2016-03-23 8:32 ` Joonsoo Kim
2016-03-23 8:32 ` Joonsoo Kim
2016-03-18 12:29 ` Vlastimil Babka
2016-03-18 12:29 ` Vlastimil Babka
2016-03-18 12:29 ` Vlastimil Babka
2016-03-08 4:03 ` Hanjun Guo
2016-03-08 4:03 ` Hanjun Guo
2016-03-08 4:03 ` Hanjun Guo
2016-03-07 12:59 ` Vlastimil Babka
2016-03-07 12:59 ` Vlastimil Babka
2016-03-07 12:59 ` Vlastimil Babka
2016-03-08 7:48 ` Joonsoo Kim
2016-03-08 7:48 ` Joonsoo Kim
2016-03-08 7:48 ` Joonsoo Kim
2016-03-08 10:45 ` Xishi Qiu
2016-03-08 10:45 ` Xishi Qiu
2016-03-08 10:45 ` Xishi Qiu
2016-03-08 15:36 ` Joonsoo Kim
2016-03-08 15:36 ` Joonsoo Kim
2016-03-08 15:36 ` Joonsoo Kim
2016-03-09 2:18 ` Xishi Qiu
2016-03-09 2:18 ` Xishi Qiu
2016-03-09 2:18 ` Xishi Qiu
2016-03-04 5:33 ` Hanjun Guo
2016-03-04 5:33 ` Hanjun Guo
2016-03-04 5:33 ` Hanjun Guo
2016-03-08 1:42 ` Xishi Qiu
2016-03-08 1:42 ` Xishi Qiu
2016-03-08 1:42 ` Xishi Qiu
2016-03-08 8:09 ` Joonsoo Kim
2016-03-08 8:09 ` Joonsoo Kim
2016-03-08 8:09 ` Joonsoo Kim
2016-03-04 6:59 ` Hanjun Guo
2016-03-04 6:59 ` Hanjun Guo
2016-03-04 6:59 ` Hanjun Guo
2016-03-07 4:40 ` Joonsoo Kim
2016-03-07 4:40 ` Joonsoo Kim
2016-03-07 4:40 ` Joonsoo Kim
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=56EDCE8C.8030607@suse.cz \
--to=vbabka@suse.cz \
--cc=linux-arm-kernel@lists.infradead.org \
/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.