* "mm: fix lazyfree BUG_ON check in try_to_unmap_one()" build error
@ 2017-03-09 4:29 Sergey Senozhatsky
2017-03-09 6:02 ` Minchan Kim
0 siblings, 1 reply; 6+ messages in thread
From: Sergey Senozhatsky @ 2017-03-09 4:29 UTC (permalink / raw)
To: Minchan Kim
Cc: Johannes Weiner, Michal Hocko, Andrew Morton, linux-kernel,
linux-mm
Hello Minchan,
/* I can't https://marc.info/?l=linux-kernel&m=148886631303107 thread
in my mail box for some reason so the Reply-To message-id may be wrong. */
commit "mm: fix lazyfree BUG_ON check in try_to_unmap_one()"
(mmotm fd07630cbf59bead90046dd3e5cfd891e58e6987)
if (VM_WARN_ON_ONCE(PageSwapBacked(page) !=
PageSwapCache(page))) {
...
}
does not compile on !CONFIG_DEBUG_VM configs, because VM_WARN_ONCE() is
#define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
In file included from ./include/linux/mmdebug.h:4:0,
from ./include/linux/mm.h:8,
from mm/rmap.c:48:
mm/rmap.c: In function ‘try_to_unmap_one’:
./include/linux/bug.h:45:33: error: void value not ignored as it ought to be
#define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
^
./include/linux/mmdebug.h:49:31: note: in expansion of macro ‘BUILD_BUG_ON_INVALID’
#define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond)
^~~~~~~~~~~~~~~~~~~~
mm/rmap.c:1416:8: note: in expansion of macro ‘VM_WARN_ON_ONCE’
if (VM_WARN_ON_ONCE(PageSwapBacked(page) !=
^~~~~~~~~~~~~~~
-ss
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: "mm: fix lazyfree BUG_ON check in try_to_unmap_one()" build error
2017-03-09 4:29 "mm: fix lazyfree BUG_ON check in try_to_unmap_one()" build error Sergey Senozhatsky
@ 2017-03-09 6:02 ` Minchan Kim
2017-03-09 8:50 ` Michal Hocko
2017-03-09 21:27 ` Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Minchan Kim @ 2017-03-09 6:02 UTC (permalink / raw)
To: Sergey Senozhatsky, Andrew Morton
Cc: Johannes Weiner, Michal Hocko, Andrew Morton, linux-kernel,
linux-mm
Hi Sergey,
On Thu, Mar 09, 2017 at 01:29:08PM +0900, Sergey Senozhatsky wrote:
> Hello Minchan,
>
> /* I can't https://marc.info/?l=linux-kernel&m=148886631303107 thread
> in my mail box for some reason so the Reply-To message-id may be wrong. */
>
>
>
> commit "mm: fix lazyfree BUG_ON check in try_to_unmap_one()"
> (mmotm fd07630cbf59bead90046dd3e5cfd891e58e6987)
>
>
> if (VM_WARN_ON_ONCE(PageSwapBacked(page) !=
> PageSwapCache(page))) {
> ...
> }
>
>
> does not compile on !CONFIG_DEBUG_VM configs, because VM_WARN_ONCE() is
>
> #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
>
>
>
> In file included from ./include/linux/mmdebug.h:4:0,
> from ./include/linux/mm.h:8,
> from mm/rmap.c:48:
> mm/rmap.c: In function ‘try_to_unmap_one’:
> ./include/linux/bug.h:45:33: error: void value not ignored as it ought to be
> #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
> ^
> ./include/linux/mmdebug.h:49:31: note: in expansion of macro ‘BUILD_BUG_ON_INVALID’
> #define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond)
> ^~~~~~~~~~~~~~~~~~~~
> mm/rmap.c:1416:8: note: in expansion of macro ‘VM_WARN_ON_ONCE’
> if (VM_WARN_ON_ONCE(PageSwapBacked(page) !=
> ^~~~~~~~~~~~~~~
>
> -ss
>
Thanks for the report, Sergey!
If others are not against, I want to go this.
>From 38b10e560d066c2cef8f9d028e14008cefdaa3e0 Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan@kernel.org>
Date: Thu, 9 Mar 2017 14:58:23 +0900
Subject: [PATCH] mm: do not use VM_WARN_ON_ONCE as if condition
Sergey reported VM_WARN_ON_ONCE returns void with !CONFIG_DEBUG_VM
so we cannot use it as if's condition unlike WARN_ON.
This patch fixes it.
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
mm/rmap.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mm/rmap.c b/mm/rmap.c
index 1d82057144ba..7d24bb93445b 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1413,12 +1413,11 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
* Store the swap location in the pte.
* See handle_pte_fault() ...
*/
- if (VM_WARN_ON_ONCE(PageSwapBacked(page) !=
- PageSwapCache(page))) {
+ if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
+ WARN_ON_ONCE(1);
ret = SWAP_FAIL;
page_vma_mapped_walk_done(&pvmw);
break;
-
}
/* MADV_FREE page check */
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: "mm: fix lazyfree BUG_ON check in try_to_unmap_one()" build error
2017-03-09 6:02 ` Minchan Kim
@ 2017-03-09 8:50 ` Michal Hocko
2017-03-09 21:27 ` Andrew Morton
1 sibling, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2017-03-09 8:50 UTC (permalink / raw)
To: Minchan Kim
Cc: Sergey Senozhatsky, Andrew Morton, Johannes Weiner, linux-kernel,
linux-mm
On Thu 09-03-17 15:02:26, Minchan Kim wrote:
[...]
> >From 38b10e560d066c2cef8f9d028e14008cefdaa3e0 Mon Sep 17 00:00:00 2001
> From: Minchan Kim <minchan@kernel.org>
> Date: Thu, 9 Mar 2017 14:58:23 +0900
> Subject: [PATCH] mm: do not use VM_WARN_ON_ONCE as if condition
>
> Sergey reported VM_WARN_ON_ONCE returns void with !CONFIG_DEBUG_VM
> so we cannot use it as if's condition unlike WARN_ON.
I would swear I've seen WARN_ON_ONCE there when looking at the previous
patch! Btw. could have simply s@VM_@@
> This patch fixes it.
>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> mm/rmap.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 1d82057144ba..7d24bb93445b 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1413,12 +1413,11 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
> * Store the swap location in the pte.
> * See handle_pte_fault() ...
> */
> - if (VM_WARN_ON_ONCE(PageSwapBacked(page) !=
> - PageSwapCache(page))) {
> + if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
> + WARN_ON_ONCE(1);
> ret = SWAP_FAIL;
> page_vma_mapped_walk_done(&pvmw);
> break;
> -
> }
>
> /* MADV_FREE page check */
> --
> 2.7.4
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: "mm: fix lazyfree BUG_ON check in try_to_unmap_one()" build error
2017-03-09 6:02 ` Minchan Kim
2017-03-09 8:50 ` Michal Hocko
@ 2017-03-09 21:27 ` Andrew Morton
2017-03-10 0:45 ` Minchan Kim
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2017-03-09 21:27 UTC (permalink / raw)
To: Minchan Kim
Cc: Sergey Senozhatsky, Johannes Weiner, Michal Hocko, linux-kernel,
linux-mm
On Thu, 9 Mar 2017 15:02:26 +0900 Minchan Kim <minchan@kernel.org> wrote:
> Sergey reported VM_WARN_ON_ONCE returns void with !CONFIG_DEBUG_VM
> so we cannot use it as if's condition unlike WARN_ON.
Can we instead fix VM_WARN_ON_ONCE()?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: "mm: fix lazyfree BUG_ON check in try_to_unmap_one()" build error
2017-03-09 21:27 ` Andrew Morton
@ 2017-03-10 0:45 ` Minchan Kim
2017-03-10 9:31 ` Vlastimil Babka
0 siblings, 1 reply; 6+ messages in thread
From: Minchan Kim @ 2017-03-10 0:45 UTC (permalink / raw)
To: Andrew Morton
Cc: Sergey Senozhatsky, Johannes Weiner, Michal Hocko, linux-kernel,
linux-mm
Hi Andrew,
On Thu, Mar 09, 2017 at 01:27:06PM -0800, Andrew Morton wrote:
> On Thu, 9 Mar 2017 15:02:26 +0900 Minchan Kim <minchan@kernel.org> wrote:
>
> > Sergey reported VM_WARN_ON_ONCE returns void with !CONFIG_DEBUG_VM
> > so we cannot use it as if's condition unlike WARN_ON.
>
> Can we instead fix VM_WARN_ON_ONCE()?
I thought the direction but the reason to decide WARN_ON_ONCE in this case
is losing of benefit with using CONFIG_DEBU_VM if we go that way.
I think the benefit with VM_WARN_ON friends is that it should be completely
out from the binary in !CONFIG_DEBUG_VM. However, if we fix VM_WARN_ON
like WARN_ON to !!condition, at least, compiler should generate condition
check and return so it's not what CONFIG_DEBUG_VM want, IMHO.
However, if guys believe it's okay to add some instructions to debug VM
although we disable CONFIG_DEBUG_VM, we can go that way.
It's a just policy matter. ;-)
Anyway, Even though we fix VM_WARN_ON_ONCE, in my case, WARN_ON_ONCE is
better because we should do !!condition regardless of CONFIG_DEBUG_VM
and if so, WARN_ON is more wide coverage than VM_WARN_ON which only works
with CONFIG_DEBUG_VM.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: "mm: fix lazyfree BUG_ON check in try_to_unmap_one()" build error
2017-03-10 0:45 ` Minchan Kim
@ 2017-03-10 9:31 ` Vlastimil Babka
0 siblings, 0 replies; 6+ messages in thread
From: Vlastimil Babka @ 2017-03-10 9:31 UTC (permalink / raw)
To: Minchan Kim, Andrew Morton
Cc: Sergey Senozhatsky, Johannes Weiner, Michal Hocko, linux-kernel,
linux-mm
On 03/10/2017 01:45 AM, Minchan Kim wrote:
> Hi Andrew,
>
> On Thu, Mar 09, 2017 at 01:27:06PM -0800, Andrew Morton wrote:
>> On Thu, 9 Mar 2017 15:02:26 +0900 Minchan Kim <minchan@kernel.org> wrote:
>>
>>> Sergey reported VM_WARN_ON_ONCE returns void with !CONFIG_DEBUG_VM
>>> so we cannot use it as if's condition unlike WARN_ON.
>>
>> Can we instead fix VM_WARN_ON_ONCE()?
>
> I thought the direction but the reason to decide WARN_ON_ONCE in this case
> is losing of benefit with using CONFIG_DEBU_VM if we go that way.
>
> I think the benefit with VM_WARN_ON friends is that it should be completely
> out from the binary in !CONFIG_DEBUG_VM. However, if we fix VM_WARN_ON
> like WARN_ON to !!condition, at least, compiler should generate condition
> check and return so it's not what CONFIG_DEBUG_VM want, IMHO.
> However, if guys believe it's okay to add some instructions to debug VM
> although we disable CONFIG_DEBUG_VM, we can go that way.
> It's a just policy matter. ;-)
>
> Anyway, Even though we fix VM_WARN_ON_ONCE, in my case, WARN_ON_ONCE is
> better because we should do !!condition regardless of CONFIG_DEBUG_VM
> and if so, WARN_ON is more wide coverage than VM_WARN_ON which only works
> with CONFIG_DEBUG_VM.
Agreed. WARN_ON...() can work that way as one can't disable them
(AFAIK), but VM_* variants are optional for overhead reasons.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-10 9:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-09 4:29 "mm: fix lazyfree BUG_ON check in try_to_unmap_one()" build error Sergey Senozhatsky
2017-03-09 6:02 ` Minchan Kim
2017-03-09 8:50 ` Michal Hocko
2017-03-09 21:27 ` Andrew Morton
2017-03-10 0:45 ` Minchan Kim
2017-03-10 9:31 ` Vlastimil Babka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox