linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* "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 a??try_to_unmap_onea??:
./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 a??BUILD_BUG_ON_INVALIDa??
 #define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond)
                               ^~~~~~~~~~~~~~~~~~~~
mm/rmap.c:1416:8: note: in expansion of macro a??VM_WARN_ON_ONCEa??
    if (VM_WARN_ON_ONCE(PageSwapBacked(page) !=
        ^~~~~~~~~~~~~~~

	-ss

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

^ 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, 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 a??try_to_unmap_onea??:
> ./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 a??BUILD_BUG_ON_INVALIDa??
>  #define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond)
>                                ^~~~~~~~~~~~~~~~~~~~
> mm/rmap.c:1416:8: note: in expansion of macro a??VM_WARN_ON_ONCEa??
>     if (VM_WARN_ON_ONCE(PageSwapBacked(page) !=
>         ^~~~~~~~~~~~~~~
> 
> 	-ss
> 

Thanks for the report, Sergey!
If others are not against, I want to go this.

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

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

^ 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()?

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

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

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

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

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

^ 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;
as well as URLs for NNTP newsgroup(s).