Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-02 Thread Hugh Dickins
On Thu, 1 Nov 2012, Dave Jones wrote:
> On Thu, Nov 01, 2012 at 04:48:41PM -0700, Hugh Dickins wrote:
>  > 
>  > Fedora turns on CONFIG_DEBUG_VM?
> 
> Yes.
>  
>  > All mm developers should thank you for the wider testing exposure;
>  > but I'm not so sure that Fedora users should thank you for turning
>  > it on - really it's for mm developers to wrap around !assertions or
>  > more expensive checks (e.g. checking calls) in their development.
> 
> The last time I did some benchmarking the impact wasn't as ridiculous
> as say lockdep, or spinlock debug.

I think you're safe to assume that (outside of an individual developer's
private tree) it will never be nearly as heavy as lockdep or debug
pagealloc.  I hadn't thought of spinlock debug as a heavy one, but
yes, I guess it would be heavier than almost all VM_BUG_ON()s.

> Maybe the benchmarks I was using
> weren't pushing the VM very hard, but it seemed to me that the value
> in getting info in potential problems early was higher than a small
> performance increase.

We thank you.  I may have been over-estimating how much we put inside
those VM_BUG_ON()s, sorry.  Just so long as you're aware that there's
a danger that one day we might slip something heavier in there.

Those few explicit #ifdef CONFIG_DEBUG_VMs sometimes found in mm/
are probably the worst: you might want to check on the current crop.

> 
>  > Or did I read a few months ago that some change had been made to
>  > such definitions, and VM_BUG_ON(contents) are evaluated even when
>  > the config option is off?  I do hope I'm mistaken on that.
> 
> Pretty sure that isn't the case. I remember Andrew chastising people
> a few times for putting checks in VM_BUG_ON's that needed to stay around 
> even when the config option was off. Perhaps you were thinking of one
> of those incidents ?

Avoiding side-effects in BUG_ON and VM_BUG_ON.  Yes, that comes up
from time to time, and I'm a believer on that.  I think the discussion
I'm mis/remembering sprung out of one of those: someone was surprised
by the disassembly they found when it was configured off.

The correct answer is to try it for myself and see.  Not today.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-02 Thread Hugh Dickins
On Thu, 1 Nov 2012, Dave Jones wrote:
 On Thu, Nov 01, 2012 at 04:48:41PM -0700, Hugh Dickins wrote:
   
   Fedora turns on CONFIG_DEBUG_VM?
 
 Yes.
  
   All mm developers should thank you for the wider testing exposure;
   but I'm not so sure that Fedora users should thank you for turning
   it on - really it's for mm developers to wrap around !assertions or
   more expensive checks (e.g. checking calls) in their development.
 
 The last time I did some benchmarking the impact wasn't as ridiculous
 as say lockdep, or spinlock debug.

I think you're safe to assume that (outside of an individual developer's
private tree) it will never be nearly as heavy as lockdep or debug
pagealloc.  I hadn't thought of spinlock debug as a heavy one, but
yes, I guess it would be heavier than almost all VM_BUG_ON()s.

 Maybe the benchmarks I was using
 weren't pushing the VM very hard, but it seemed to me that the value
 in getting info in potential problems early was higher than a small
 performance increase.

We thank you.  I may have been over-estimating how much we put inside
those VM_BUG_ON()s, sorry.  Just so long as you're aware that there's
a danger that one day we might slip something heavier in there.

Those few explicit #ifdef CONFIG_DEBUG_VMs sometimes found in mm/
are probably the worst: you might want to check on the current crop.

 
   Or did I read a few months ago that some change had been made to
   such definitions, and VM_BUG_ON(contents) are evaluated even when
   the config option is off?  I do hope I'm mistaken on that.
 
 Pretty sure that isn't the case. I remember Andrew chastising people
 a few times for putting checks in VM_BUG_ON's that needed to stay around 
 even when the config option was off. Perhaps you were thinking of one
 of those incidents ?

Avoiding side-effects in BUG_ON and VM_BUG_ON.  Yes, that comes up
from time to time, and I'm a believer on that.  I think the discussion
I'm mis/remembering sprung out of one of those: someone was surprised
by the disassembly they found when it was configured off.

The correct answer is to try it for myself and see.  Not today.

Hugh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Dave Jones
On Thu, Nov 01, 2012 at 04:48:41PM -0700, Hugh Dickins wrote:
 > On Thu, 1 Nov 2012, Dave Jones wrote:
 > > On Thu, Nov 01, 2012 at 04:03:40PM -0700, Hugh Dickins wrote:
 > >  > 
 > >  > Except... earlier in the thread you explained how you hacked
 > >  > #define VM_BUG_ON(cond) WARN_ON(cond)
 > >  > to get this to come out as a warning instead of a bug,
 > >  > and now it looks as if "a user" has here done the same.
 > >  > 
 > >  > Which is very much a user's right, of course; but does
 > >  > make me wonder whether that user might actually be davej ;)
 > > 
 > > indirectly. I made the same change in the Fedora kernel a while ago
 > > to test a hypothesis that we weren't getting any VM_BUG_ON reports.
 > 
 > Fedora turns on CONFIG_DEBUG_VM?

Yes.
 
 > All mm developers should thank you for the wider testing exposure;
 > but I'm not so sure that Fedora users should thank you for turning
 > it on - really it's for mm developers to wrap around !assertions or
 > more expensive checks (e.g. checking calls) in their development.

The last time I did some benchmarking the impact wasn't as ridiculous
as say lockdep, or spinlock debug. Maybe the benchmarks I was using
weren't pushing the VM very hard, but it seemed to me that the value
in getting info in potential problems early was higher than a small
performance increase.

 > Or did I read a few months ago that some change had been made to
 > such definitions, and VM_BUG_ON(contents) are evaluated even when
 > the config option is off?  I do hope I'm mistaken on that.

Pretty sure that isn't the case. I remember Andrew chastising people
a few times for putting checks in VM_BUG_ON's that needed to stay around 
even when the config option was off. Perhaps you were thinking of one
of those incidents ?

Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Hugh Dickins
On Thu, 1 Nov 2012, Dave Jones wrote:
> On Thu, Nov 01, 2012 at 04:03:40PM -0700, Hugh Dickins wrote:
>  > 
>  > Except... earlier in the thread you explained how you hacked
>  > #define VM_BUG_ON(cond) WARN_ON(cond)
>  > to get this to come out as a warning instead of a bug,
>  > and now it looks as if "a user" has here done the same.
>  > 
>  > Which is very much a user's right, of course; but does
>  > make me wonder whether that user might actually be davej ;)
> 
> indirectly. I made the same change in the Fedora kernel a while ago
> to test a hypothesis that we weren't getting any VM_BUG_ON reports.

Fedora turns on CONFIG_DEBUG_VM?

All mm developers should thank you for the wider testing exposure;
but I'm not so sure that Fedora users should thank you for turning
it on - really it's for mm developers to wrap around !assertions or
more expensive checks (e.g. checking calls) in their development.

Or did I read a few months ago that some change had been made to
such definitions, and VM_BUG_ON(contents) are evaluated even when
the config option is off?  I do hope I'm mistaken on that.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Dave Jones
On Thu, Nov 01, 2012 at 04:03:40PM -0700, Hugh Dickins wrote:
 > > I just noticed we had a user report hitting this same warning, but
 > > with a different trace..
 > > 
 > > : [] warn_slowpath_common+0x7f/0xc0
 > > : [] warn_slowpath_null+0x1a/0x20
 > > : [] shmem_getpage_gfp+0x7f3/0x830
 > > : [] ? vma_adjust+0x3ed/0x620
 > > : [] shmem_file_aio_read+0x1f2/0x380
 > > : [] do_sync_read+0xa7/0xe0
 > > : [] vfs_read+0xa9/0x180
 > > : [] sys_read+0x4a/0x90
 > > : [] system_call_fastpath+0x16/0x1b
 > 
 > Equally explicable by Hannes's hypothesis;
 > but useful supporting evidence, thank you.
 > 
 > Except... earlier in the thread you explained how you hacked
 > #define VM_BUG_ON(cond) WARN_ON(cond)
 > to get this to come out as a warning instead of a bug,
 > and now it looks as if "a user" has here done the same.
 > 
 > Which is very much a user's right, of course; but does
 > make me wonder whether that user might actually be davej ;)

indirectly. I made the same change in the Fedora kernel a while ago
to test a hypothesis that we weren't getting any VM_BUG_ON reports.

Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Hugh Dickins
On Thu, 1 Nov 2012, Dave Jones wrote:
> On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
>  > On Wed, 24 Oct 2012, Dave Jones wrote:
>  > 
>  > > Machine under significant load (4gb memory used, swap usage fluctuating)
>  > > triggered this...
>  > > 
>  > > WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
>  > > 
>  > > 1148 error = shmem_add_to_page_cache(page, 
> mapping, index,
>  > > 1149 gfp, 
> swp_to_radix_entry(swap));
>  > > 1150 /* We already confirmed swap, and make no 
> allocation */
>  > > 1151 VM_BUG_ON(error);
>  > > 1152 }
>  > 
>  > That's very surprising.  Easy enough to handle an error there, but
>  > of course I made it a VM_BUG_ON because it violates my assumptions:
>  > I rather need to understand how this can be, and I've no idea.
> 
> I just noticed we had a user report hitting this same warning, but
> with a different trace..
> 
> : [] warn_slowpath_common+0x7f/0xc0
> : [] warn_slowpath_null+0x1a/0x20
> : [] shmem_getpage_gfp+0x7f3/0x830
> : [] ? vma_adjust+0x3ed/0x620
> : [] shmem_file_aio_read+0x1f2/0x380
> : [] do_sync_read+0xa7/0xe0
> : [] vfs_read+0xa9/0x180
> : [] sys_read+0x4a/0x90
> : [] system_call_fastpath+0x16/0x1b

Equally explicable by Hannes's hypothesis;
but useful supporting evidence, thank you.

Except... earlier in the thread you explained how you hacked
#define VM_BUG_ON(cond) WARN_ON(cond)
to get this to come out as a warning instead of a bug,
and now it looks as if "a user" has here done the same.

Which is very much a user's right, of course; but does
make me wonder whether that user might actually be davej ;)

Never mind, whatever, it's more justification for the fix - which
I've honestly not forgotten, but somehow not got around to sending
(with a couple of others even longer outstanding).  On its way
shortly, for some unpredictable value of shortly.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Dave Jones
On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
 > On Wed, 24 Oct 2012, Dave Jones wrote:
 > 
 > > Machine under significant load (4gb memory used, swap usage fluctuating)
 > > triggered this...
 > > 
 > > WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
 > > Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
 > > Call Trace:
 > >  [] warn_slowpath_common+0x7f/0xc0
 > >  [] warn_slowpath_null+0x1a/0x20
 > >  [] shmem_getpage_gfp+0xa5c/0xa70
 > >  [] ? shmem_getpage_gfp+0x29e/0xa70
 > >  [] shmem_fault+0x4f/0xa0
 > >  [] __do_fault+0x71/0x5c0
 > >  [] ? __lock_acquire+0x306/0x1ba0
 > >  [] ? local_clock+0x89/0xa0
 > >  [] handle_pte_fault+0x97/0xae0
 > >  [] ? sub_preempt_count+0x79/0xd0
 > >  [] ? delay_tsc+0xae/0x120
 > >  [] ? __const_udelay+0x28/0x30
 > >  [] handle_mm_fault+0x289/0x350
 > >  [] __do_page_fault+0x18e/0x530
 > >  [] ? local_clock+0x89/0xa0
 > >  [] ? get_parent_ip+0x11/0x50
 > >  [] ? get_parent_ip+0x11/0x50
 > >  [] ? sub_preempt_count+0x79/0xd0
 > >  [] ? rcu_user_exit+0xc9/0xf0
 > >  [] do_page_fault+0x2b/0x50
 > >  [] page_fault+0x28/0x30
 > >  [] ? copy_user_enhanced_fast_string+0x9/0x20
 > >  [] ? sys_futimesat+0x41/0xe0
 > >  [] ? syscall_trace_enter+0x25/0x2c0
 > >  [] ? tracesys+0x7e/0xe6
 > >  [] tracesys+0xe1/0xe6
 > > 
 > > 
 > > 
 > > 1148 error = shmem_add_to_page_cache(page, 
 > > mapping, index,
 > > 1149 gfp, 
 > > swp_to_radix_entry(swap));
 > > 1150 /* We already confirmed swap, and make no 
 > > allocation */
 > > 1151 VM_BUG_ON(error);
 > > 1152 }
 > 
 > That's very surprising.  Easy enough to handle an error there, but
 > of course I made it a VM_BUG_ON because it violates my assumptions:
 > I rather need to understand how this can be, and I've no idea.

I just noticed we had a user report hitting this same warning, but
with a different trace..

: [] warn_slowpath_common+0x7f/0xc0
: [] warn_slowpath_null+0x1a/0x20
: [] shmem_getpage_gfp+0x7f3/0x830
: [] ? vma_adjust+0x3ed/0x620
: [] shmem_file_aio_read+0x1f2/0x380
: [] do_sync_read+0xa7/0xe0
: [] vfs_read+0xa9/0x180
: [] sys_read+0x4a/0x90
: [] system_call_fastpath+0x16/0x1b

Dave
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Dave Jones
On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
  On Wed, 24 Oct 2012, Dave Jones wrote:
  
   Machine under significant load (4gb memory used, swap usage fluctuating)
   triggered this...
   
   WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
   Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
   Call Trace:
[8107100f] warn_slowpath_common+0x7f/0xc0
[8107106a] warn_slowpath_null+0x1a/0x20
[811903fc] shmem_getpage_gfp+0xa5c/0xa70
[8118fc3e] ? shmem_getpage_gfp+0x29e/0xa70
[81190e4f] shmem_fault+0x4f/0xa0
[8119f391] __do_fault+0x71/0x5c0
[810e1ac6] ? __lock_acquire+0x306/0x1ba0
[810b6ff9] ? local_clock+0x89/0xa0
[811a2767] handle_pte_fault+0x97/0xae0
[816d1069] ? sub_preempt_count+0x79/0xd0
[8136d68e] ? delay_tsc+0xae/0x120
[8136d578] ? __const_udelay+0x28/0x30
[811a4a39] handle_mm_fault+0x289/0x350
[816d091e] __do_page_fault+0x18e/0x530
[810b6ff9] ? local_clock+0x89/0xa0
[810b0e51] ? get_parent_ip+0x11/0x50
[810b0e51] ? get_parent_ip+0x11/0x50
[816d1069] ? sub_preempt_count+0x79/0xd0
[8112d389] ? rcu_user_exit+0xc9/0xf0
[816d0ceb] do_page_fault+0x2b/0x50
[816cd3b8] page_fault+0x28/0x30
[8136d259] ? copy_user_enhanced_fast_string+0x9/0x20
[8121c181] ? sys_futimesat+0x41/0xe0
[8102bf35] ? syscall_trace_enter+0x25/0x2c0
[816d5625] ? tracesys+0x7e/0xe6
[816d5688] tracesys+0xe1/0xe6
   
   
   
   1148 error = shmem_add_to_page_cache(page, 
   mapping, index,
   1149 gfp, 
   swp_to_radix_entry(swap));
   1150 /* We already confirmed swap, and make no 
   allocation */
   1151 VM_BUG_ON(error);
   1152 }
  
  That's very surprising.  Easy enough to handle an error there, but
  of course I made it a VM_BUG_ON because it violates my assumptions:
  I rather need to understand how this can be, and I've no idea.

I just noticed we had a user report hitting this same warning, but
with a different trace..

: [8105b84f] warn_slowpath_common+0x7f/0xc0
: [8105b8aa] warn_slowpath_null+0x1a/0x20
: [81143c73] shmem_getpage_gfp+0x7f3/0x830
: [81158c9d] ? vma_adjust+0x3ed/0x620
: [81143f02] shmem_file_aio_read+0x1f2/0x380
: [8118e487] do_sync_read+0xa7/0xe0
: [8118eda9] vfs_read+0xa9/0x180
: [8118eeca] sys_read+0x4a/0x90
: [816226e9] system_call_fastpath+0x16/0x1b

Dave
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Hugh Dickins
On Thu, 1 Nov 2012, Dave Jones wrote:
 On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
   On Wed, 24 Oct 2012, Dave Jones wrote:
   
Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()

1148 error = shmem_add_to_page_cache(page, 
 mapping, index,
1149 gfp, 
 swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no 
 allocation */
1151 VM_BUG_ON(error);
1152 }
   
   That's very surprising.  Easy enough to handle an error there, but
   of course I made it a VM_BUG_ON because it violates my assumptions:
   I rather need to understand how this can be, and I've no idea.
 
 I just noticed we had a user report hitting this same warning, but
 with a different trace..
 
 : [8105b84f] warn_slowpath_common+0x7f/0xc0
 : [8105b8aa] warn_slowpath_null+0x1a/0x20
 : [81143c73] shmem_getpage_gfp+0x7f3/0x830
 : [81158c9d] ? vma_adjust+0x3ed/0x620
 : [81143f02] shmem_file_aio_read+0x1f2/0x380
 : [8118e487] do_sync_read+0xa7/0xe0
 : [8118eda9] vfs_read+0xa9/0x180
 : [8118eeca] sys_read+0x4a/0x90
 : [816226e9] system_call_fastpath+0x16/0x1b

Equally explicable by Hannes's hypothesis;
but useful supporting evidence, thank you.

Except... earlier in the thread you explained how you hacked
#define VM_BUG_ON(cond) WARN_ON(cond)
to get this to come out as a warning instead of a bug,
and now it looks as if a user has here done the same.

Which is very much a user's right, of course; but does
make me wonder whether that user might actually be davej ;)

Never mind, whatever, it's more justification for the fix - which
I've honestly not forgotten, but somehow not got around to sending
(with a couple of others even longer outstanding).  On its way
shortly, for some unpredictable value of shortly.

Hugh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Dave Jones
On Thu, Nov 01, 2012 at 04:03:40PM -0700, Hugh Dickins wrote:
   I just noticed we had a user report hitting this same warning, but
   with a different trace..
   
   : [8105b84f] warn_slowpath_common+0x7f/0xc0
   : [8105b8aa] warn_slowpath_null+0x1a/0x20
   : [81143c73] shmem_getpage_gfp+0x7f3/0x830
   : [81158c9d] ? vma_adjust+0x3ed/0x620
   : [81143f02] shmem_file_aio_read+0x1f2/0x380
   : [8118e487] do_sync_read+0xa7/0xe0
   : [8118eda9] vfs_read+0xa9/0x180
   : [8118eeca] sys_read+0x4a/0x90
   : [816226e9] system_call_fastpath+0x16/0x1b
  
  Equally explicable by Hannes's hypothesis;
  but useful supporting evidence, thank you.
  
  Except... earlier in the thread you explained how you hacked
  #define VM_BUG_ON(cond) WARN_ON(cond)
  to get this to come out as a warning instead of a bug,
  and now it looks as if a user has here done the same.
  
  Which is very much a user's right, of course; but does
  make me wonder whether that user might actually be davej ;)

indirectly. I made the same change in the Fedora kernel a while ago
to test a hypothesis that we weren't getting any VM_BUG_ON reports.

Dave

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Hugh Dickins
On Thu, 1 Nov 2012, Dave Jones wrote:
 On Thu, Nov 01, 2012 at 04:03:40PM -0700, Hugh Dickins wrote:
   
   Except... earlier in the thread you explained how you hacked
   #define VM_BUG_ON(cond) WARN_ON(cond)
   to get this to come out as a warning instead of a bug,
   and now it looks as if a user has here done the same.
   
   Which is very much a user's right, of course; but does
   make me wonder whether that user might actually be davej ;)
 
 indirectly. I made the same change in the Fedora kernel a while ago
 to test a hypothesis that we weren't getting any VM_BUG_ON reports.

Fedora turns on CONFIG_DEBUG_VM?

All mm developers should thank you for the wider testing exposure;
but I'm not so sure that Fedora users should thank you for turning
it on - really it's for mm developers to wrap around !assertions or
more expensive checks (e.g. checking calls) in their development.

Or did I read a few months ago that some change had been made to
such definitions, and VM_BUG_ON(contents) are evaluated even when
the config option is off?  I do hope I'm mistaken on that.

Hugh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-11-01 Thread Dave Jones
On Thu, Nov 01, 2012 at 04:48:41PM -0700, Hugh Dickins wrote:
  On Thu, 1 Nov 2012, Dave Jones wrote:
   On Thu, Nov 01, 2012 at 04:03:40PM -0700, Hugh Dickins wrote:
 
 Except... earlier in the thread you explained how you hacked
 #define VM_BUG_ON(cond) WARN_ON(cond)
 to get this to come out as a warning instead of a bug,
 and now it looks as if a user has here done the same.
 
 Which is very much a user's right, of course; but does
 make me wonder whether that user might actually be davej ;)
   
   indirectly. I made the same change in the Fedora kernel a while ago
   to test a hypothesis that we weren't getting any VM_BUG_ON reports.
  
  Fedora turns on CONFIG_DEBUG_VM?

Yes.
 
  All mm developers should thank you for the wider testing exposure;
  but I'm not so sure that Fedora users should thank you for turning
  it on - really it's for mm developers to wrap around !assertions or
  more expensive checks (e.g. checking calls) in their development.

The last time I did some benchmarking the impact wasn't as ridiculous
as say lockdep, or spinlock debug. Maybe the benchmarks I was using
weren't pushing the VM very hard, but it seemed to me that the value
in getting info in potential problems early was higher than a small
performance increase.

  Or did I read a few months ago that some change had been made to
  such definitions, and VM_BUG_ON(contents) are evaluated even when
  the config option is off?  I do hope I'm mistaken on that.

Pretty sure that isn't the case. I remember Andrew chastising people
a few times for putting checks in VM_BUG_ON's that needed to stay around 
even when the config option was off. Perhaps you were thinking of one
of those incidents ?

Dave
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Ni zhan Chen

On 10/26/2012 05:48 AM, Hugh Dickins wrote:

On Thu, 25 Oct 2012, Johannes Weiner wrote:

On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:

On Wed, 24 Oct 2012, Dave Jones wrote:


Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49

1148 error = shmem_add_to_page_cache(page, mapping, 
index,
1149 gfp, 
swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no 
allocation */
1151 VM_BUG_ON(error);
1152 }

That's very surprising.  Easy enough to handle an error there, but
of course I made it a VM_BUG_ON because it violates my assumptions:
I rather need to understand how this can be, and I've no idea.

Could it be concurrent truncation clearing out the entry between
shmem_confirm_swap() and shmem_add_to_page_cache()?  I don't see
anything preventing that.

The empty slot would not match the expected swap entry this call
passes in and the returned error would be -ENOENT.

Excellent notion, many thanks Hannes, I believe you've got it.

I've hit that truncation problem in swapoff (and commented on it
in shmem_unuse_inode), but never hit it or considered it here.
I think of the page lock as holding it stable, but truncation's
free_swap_and_cache only does a trylock on the swapcache page,
so we're not secured against that possibility.


Hi Hugh,

Even though free_swap_and_cache only does a trylock on the swapcache 
page, but it doens't call delete_from_swap_cache and the associated 
entry should still be there, I am interested in what you have already 
introduce to protect it?




So I'd like to change it to VM_BUG_ON(error && error != -ENOENT),
but there's a little tidying up to do in the -ENOENT case, which


Do you mean radix_tree_insert will return -ENOENT if the associated 
entry is not present? Why I can't find this return value in the function 
radix_tree_insert?



needs more thought.  A delete_from_swap_cache(page) - though we
can be lazy and leave that to reclaim for such a rare occurrence -
and probably a mem_cgroup uncharge; but the memcg hooks are always
the hardest to get right, I'll have think about that one carefully.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: mailto:"d...@kvack.org;> em...@kvack.org 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Ni zhan Chen

On 10/26/2012 05:27 AM, Hugh Dickins wrote:

On Thu, 25 Oct 2012, Ni zhan Chen wrote:

On 10/25/2012 02:59 PM, Hugh Dickins wrote:

On Thu, 25 Oct 2012, Ni zhan Chen wrote:

I think it maybe caused by your commit [d189922862e03ce: shmem: fix
negative
rss in memcg memory.stat], one question:

Well, yes, I added the VM_BUG_ON in that commit.


if function shmem_confirm_swap confirm the entry has already brought back
from swap by a racing thread,

The reverse: true confirms that the swap entry has not been brought back
from swap by a racing thread; false indicates that there has been a race.


then why call shmem_add_to_page_cache to add
page from swapcache to pagecache again?

Adding it to pagecache again, after such a race, would set error to
-EEXIST (originating from radix_tree_insert); but we don't do that,
we add it to pagecache when it has not already been added.

Or that's the intention: but Dave seems to have found an unexpected
exception, despite us holding the page lock across all this.

(But if it weren't for the memcg and replace_page issues, I'd much
prefer to let shmem_add_to_page_cache discover the race as before.)

Hugh

Hi Hugh

Thanks for your response. You mean the -EEXIST originating from
radix_tree_insert, in radix_tree_insert:
if (slot != NULL)
 return -EEXIST;
But why slot should be NULL? if no race, the pagecache related radix tree
entry should be RADIX_TREE_EXCEPTIONAL_ENTRY+swap_entry_t.val, where I miss?

I was describing what would happen in a case that should not exist,
that you had thought the common case.  In actuality, the entry should
not be NULL, it should be as you say there.


Thanks for your patience. So in the common case, the entry should be the 
value I mentioned, then why has this check?

if (slot != NULL)
return -EEXIST;

the common case will return -EEXIST.



Hugh



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Hugh Dickins
On Thu, 25 Oct 2012, Johannes Weiner wrote:
> On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
> > On Wed, 24 Oct 2012, Dave Jones wrote:
> > 
> > > Machine under significant load (4gb memory used, swap usage fluctuating)
> > > triggered this...
> > > 
> > > WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
> > > Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
> > > 
> > > 1148 error = shmem_add_to_page_cache(page, 
> > > mapping, index,
> > > 1149 gfp, 
> > > swp_to_radix_entry(swap));
> > > 1150 /* We already confirmed swap, and make no 
> > > allocation */
> > > 1151 VM_BUG_ON(error);
> > > 1152 }
> > 
> > That's very surprising.  Easy enough to handle an error there, but
> > of course I made it a VM_BUG_ON because it violates my assumptions:
> > I rather need to understand how this can be, and I've no idea.
> 
> Could it be concurrent truncation clearing out the entry between
> shmem_confirm_swap() and shmem_add_to_page_cache()?  I don't see
> anything preventing that.
> 
> The empty slot would not match the expected swap entry this call
> passes in and the returned error would be -ENOENT.

Excellent notion, many thanks Hannes, I believe you've got it.

I've hit that truncation problem in swapoff (and commented on it
in shmem_unuse_inode), but never hit it or considered it here.
I think of the page lock as holding it stable, but truncation's
free_swap_and_cache only does a trylock on the swapcache page,
so we're not secured against that possibility.

So I'd like to change it to VM_BUG_ON(error && error != -ENOENT),
but there's a little tidying up to do in the -ENOENT case, which
needs more thought.  A delete_from_swap_cache(page) - though we
can be lazy and leave that to reclaim for such a rare occurrence -
and probably a mem_cgroup uncharge; but the memcg hooks are always
the hardest to get right, I'll have think about that one carefully.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Hugh Dickins
On Thu, 25 Oct 2012, Dave Jones wrote:
> On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
> 
>  > Clutching at straws, I expect this is entirely irrelevant, but:
>  > there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
>  > in current linux.git; rather, there's a VM_BUG_ON on line 1149.
>  > 
>  > So you've inserted a couple of lines for some reason (more useful
>  > trinity behaviour, perhaps)? 
> 
> detritus from the recent mpol_to_str bug that I was chasing.
> Shouldn't be relevant...
> 
>  > And have some config option I'm
>  > unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?
> 
> Yes, I do have this..
> 
> -#define VM_BUG_ON(cond) BUG_ON(cond)
> +#define VM_BUG_ON(cond) WARN_ON(cond)
> 
> because I got tired of things not going over my usb serial port when I hit 
> them
> a while ago. BUG_ON is pretty unfriendly to bug finding.

Makes sense, thanks for the reassurance.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Hugh Dickins
On Thu, 25 Oct 2012, Ni zhan Chen wrote:
> On 10/25/2012 02:59 PM, Hugh Dickins wrote:
> > On Thu, 25 Oct 2012, Ni zhan Chen wrote:
> > > 
> > > I think it maybe caused by your commit [d189922862e03ce: shmem: fix
> > > negative
> > > rss in memcg memory.stat], one question:
> > Well, yes, I added the VM_BUG_ON in that commit.
> > 
> > > if function shmem_confirm_swap confirm the entry has already brought back
> > > from swap by a racing thread,
> > The reverse: true confirms that the swap entry has not been brought back
> > from swap by a racing thread; false indicates that there has been a race.
> > 
> > > then why call shmem_add_to_page_cache to add
> > > page from swapcache to pagecache again?
> > Adding it to pagecache again, after such a race, would set error to
> > -EEXIST (originating from radix_tree_insert); but we don't do that,
> > we add it to pagecache when it has not already been added.
> > 
> > Or that's the intention: but Dave seems to have found an unexpected
> > exception, despite us holding the page lock across all this.
> > 
> > (But if it weren't for the memcg and replace_page issues, I'd much
> > prefer to let shmem_add_to_page_cache discover the race as before.)
> > 
> > Hugh
> 
> Hi Hugh
> 
> Thanks for your response. You mean the -EEXIST originating from
> radix_tree_insert, in radix_tree_insert:
> if (slot != NULL)
> return -EEXIST;
> But why slot should be NULL? if no race, the pagecache related radix tree
> entry should be RADIX_TREE_EXCEPTIONAL_ENTRY+swap_entry_t.val, where I miss?

I was describing what would happen in a case that should not exist,
that you had thought the common case.  In actuality, the entry should
not be NULL, it should be as you say there.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Johannes Weiner
On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
> On Wed, 24 Oct 2012, Dave Jones wrote:
> 
> > Machine under significant load (4gb memory used, swap usage fluctuating)
> > triggered this...
> > 
> > WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
> > Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
> > Call Trace:
> >  [] warn_slowpath_common+0x7f/0xc0
> >  [] warn_slowpath_null+0x1a/0x20
> >  [] shmem_getpage_gfp+0xa5c/0xa70
> >  [] ? shmem_getpage_gfp+0x29e/0xa70
> >  [] shmem_fault+0x4f/0xa0
> >  [] __do_fault+0x71/0x5c0
> >  [] ? __lock_acquire+0x306/0x1ba0
> >  [] ? local_clock+0x89/0xa0
> >  [] handle_pte_fault+0x97/0xae0
> >  [] ? sub_preempt_count+0x79/0xd0
> >  [] ? delay_tsc+0xae/0x120
> >  [] ? __const_udelay+0x28/0x30
> >  [] handle_mm_fault+0x289/0x350
> >  [] __do_page_fault+0x18e/0x530
> >  [] ? local_clock+0x89/0xa0
> >  [] ? get_parent_ip+0x11/0x50
> >  [] ? get_parent_ip+0x11/0x50
> >  [] ? sub_preempt_count+0x79/0xd0
> >  [] ? rcu_user_exit+0xc9/0xf0
> >  [] do_page_fault+0x2b/0x50
> >  [] page_fault+0x28/0x30
> >  [] ? copy_user_enhanced_fast_string+0x9/0x20
> >  [] ? sys_futimesat+0x41/0xe0
> >  [] ? syscall_trace_enter+0x25/0x2c0
> >  [] ? tracesys+0x7e/0xe6
> >  [] tracesys+0xe1/0xe6
> > 
> > 
> > 
> > 1148 error = shmem_add_to_page_cache(page, mapping, 
> > index,
> > 1149 gfp, 
> > swp_to_radix_entry(swap));
> > 1150 /* We already confirmed swap, and make no 
> > allocation */
> > 1151 VM_BUG_ON(error);
> > 1152 }
> 
> That's very surprising.  Easy enough to handle an error there, but
> of course I made it a VM_BUG_ON because it violates my assumptions:
> I rather need to understand how this can be, and I've no idea.

Could it be concurrent truncation clearing out the entry between
shmem_confirm_swap() and shmem_add_to_page_cache()?  I don't see
anything preventing that.

The empty slot would not match the expected swap entry this call
passes in and the returned error would be -ENOENT.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Dave Jones
On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:

 > > 1148 error = shmem_add_to_page_cache(page, 
 > > mapping, index,
 > > 1149 gfp, 
 > > swp_to_radix_entry(swap));
 > > 1150 /* We already confirmed swap, and make no 
 > > allocation */
 > > 1151 VM_BUG_ON(error);
 > > 1152 }
 > 
 > That's very surprising.  Easy enough to handle an error there, but
 > of course I made it a VM_BUG_ON because it violates my assumptions:
 > I rather need to understand how this can be, and I've no idea.
 > 
 > Clutching at straws, I expect this is entirely irrelevant, but:
 > there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
 > in current linux.git; rather, there's a VM_BUG_ON on line 1149.
 > 
 > So you've inserted a couple of lines for some reason (more useful
 > trinity behaviour, perhaps)? 

detritus from the recent mpol_to_str bug that I was chasing.
Shouldn't be relevant...

diff -durpN '--exclude-from=/home/davej/.exclude' src/git-trees/kernel/linux/mm>
--- src/git-trees/kernel/linux/mm/shmem.c   2012-10-12 10:01:46.613408580 ->
+++ linux-dj/mm/shmem.c 2012-10-15 12:31:32.979653309 -0400
@@ -885,13 +885,15 @@ redirty:
 static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
 {
char buffer[64];
+   int ret;
 
if (!mpol || mpol->mode == MPOL_DEFAULT)
return; /* show nothing */
 
-   mpol_to_str(buffer, sizeof(buffer), mpol, 1);
-
-   seq_printf(seq, ",mpol=%s", buffer);
+   memset(buffer, 0, sizeof(buffer));
+   ret = mpol_to_str(buffer, sizeof(buffer), mpol, 1);
+   if (ret > 0)
+   seq_printf(seq, ",mpol=%s", buffer);
 }


 > And have some config option I'm
 > unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?

Yes, I do have this..

-#define VM_BUG_ON(cond) BUG_ON(cond)
+#define VM_BUG_ON(cond) WARN_ON(cond)

because I got tired of things not going over my usb serial port when I hit them
a while ago. BUG_ON is pretty unfriendly to bug finding.

Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Ni zhan Chen

On 10/25/2012 02:59 PM, Hugh Dickins wrote:

On Thu, 25 Oct 2012, Ni zhan Chen wrote:

On 10/25/2012 12:36 PM, Hugh Dickins wrote:

On Wed, 24 Oct 2012, Dave Jones wrote:


Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49

1148 error = shmem_add_to_page_cache(page,
mapping, index,
1149 gfp,
swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no
allocation */
1151 VM_BUG_ON(error);
1152 }

That's very surprising.  Easy enough to handle an error there, but
of course I made it a VM_BUG_ON because it violates my assumptions:
I rather need to understand how this can be, and I've no idea.

Clutching at straws, I expect this is entirely irrelevant, but:
there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
in current linux.git; rather, there's a VM_BUG_ON on line 1149.

So you've inserted a couple of lines for some reason (more useful
trinity behaviour, perhaps)?  And have some config option I'm
unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?

Hi Hugh,

I think it maybe caused by your commit [d189922862e03ce: shmem: fix negative
rss in memcg memory.stat], one question:

Well, yes, I added the VM_BUG_ON in that commit.


if function shmem_confirm_swap confirm the entry has already brought back
from swap by a racing thread,

The reverse: true confirms that the swap entry has not been brought back
from swap by a racing thread; false indicates that there has been a race.


then why call shmem_add_to_page_cache to add
page from swapcache to pagecache again?

Adding it to pagecache again, after such a race, would set error to
-EEXIST (originating from radix_tree_insert); but we don't do that,
we add it to pagecache when it has not already been added.

Or that's the intention: but Dave seems to have found an unexpected
exception, despite us holding the page lock across all this.

(But if it weren't for the memcg and replace_page issues, I'd much
prefer to let shmem_add_to_page_cache discover the race as before.)

Hugh


Hi Hugh

Thanks for your response. You mean the -EEXIST originating from 
radix_tree_insert, in radix_tree_insert:

if (slot != NULL)
return -EEXIST;
But why slot should be NULL? if no race, the pagecache related radix 
tree entry should be RADIX_TREE_EXCEPTIONAL_ENTRY+swap_entry_t.val, 
where I miss?


Regards,
Chen




otherwise, will goto unlock and then go to repeat? where I miss?

Regards,
Chen


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Hugh Dickins
On Thu, 25 Oct 2012, Ni zhan Chen wrote:
> On 10/25/2012 12:36 PM, Hugh Dickins wrote:
> > On Wed, 24 Oct 2012, Dave Jones wrote:
> > 
> > > Machine under significant load (4gb memory used, swap usage fluctuating)
> > > triggered this...
> > > 
> > > WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
> > > Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
> > > 
> > > 1148 error = shmem_add_to_page_cache(page,
> > > mapping, index,
> > > 1149 gfp,
> > > swp_to_radix_entry(swap));
> > > 1150 /* We already confirmed swap, and make no
> > > allocation */
> > > 1151 VM_BUG_ON(error);
> > > 1152 }
> > That's very surprising.  Easy enough to handle an error there, but
> > of course I made it a VM_BUG_ON because it violates my assumptions:
> > I rather need to understand how this can be, and I've no idea.
> > 
> > Clutching at straws, I expect this is entirely irrelevant, but:
> > there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
> > in current linux.git; rather, there's a VM_BUG_ON on line 1149.
> > 
> > So you've inserted a couple of lines for some reason (more useful
> > trinity behaviour, perhaps)?  And have some config option I'm
> > unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?
> 
> Hi Hugh,
> 
> I think it maybe caused by your commit [d189922862e03ce: shmem: fix negative
> rss in memcg memory.stat], one question:

Well, yes, I added the VM_BUG_ON in that commit.

> 
> if function shmem_confirm_swap confirm the entry has already brought back
> from swap by a racing thread,

The reverse: true confirms that the swap entry has not been brought back
from swap by a racing thread; false indicates that there has been a race.

> then why call shmem_add_to_page_cache to add
> page from swapcache to pagecache again?

Adding it to pagecache again, after such a race, would set error to
-EEXIST (originating from radix_tree_insert); but we don't do that,
we add it to pagecache when it has not already been added.

Or that's the intention: but Dave seems to have found an unexpected
exception, despite us holding the page lock across all this.

(But if it weren't for the memcg and replace_page issues, I'd much
prefer to let shmem_add_to_page_cache discover the race as before.)

Hugh

> otherwise, will goto unlock and then go to repeat? where I miss?
> 
> Regards,
> Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Hugh Dickins
On Thu, 25 Oct 2012, Ni zhan Chen wrote:
 On 10/25/2012 12:36 PM, Hugh Dickins wrote:
  On Wed, 24 Oct 2012, Dave Jones wrote:
  
   Machine under significant load (4gb memory used, swap usage fluctuating)
   triggered this...
   
   WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
   Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
   
   1148 error = shmem_add_to_page_cache(page,
   mapping, index,
   1149 gfp,
   swp_to_radix_entry(swap));
   1150 /* We already confirmed swap, and make no
   allocation */
   1151 VM_BUG_ON(error);
   1152 }
  That's very surprising.  Easy enough to handle an error there, but
  of course I made it a VM_BUG_ON because it violates my assumptions:
  I rather need to understand how this can be, and I've no idea.
  
  Clutching at straws, I expect this is entirely irrelevant, but:
  there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
  in current linux.git; rather, there's a VM_BUG_ON on line 1149.
  
  So you've inserted a couple of lines for some reason (more useful
  trinity behaviour, perhaps)?  And have some config option I'm
  unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?
 
 Hi Hugh,
 
 I think it maybe caused by your commit [d189922862e03ce: shmem: fix negative
 rss in memcg memory.stat], one question:

Well, yes, I added the VM_BUG_ON in that commit.

 
 if function shmem_confirm_swap confirm the entry has already brought back
 from swap by a racing thread,

The reverse: true confirms that the swap entry has not been brought back
from swap by a racing thread; false indicates that there has been a race.

 then why call shmem_add_to_page_cache to add
 page from swapcache to pagecache again?

Adding it to pagecache again, after such a race, would set error to
-EEXIST (originating from radix_tree_insert); but we don't do that,
we add it to pagecache when it has not already been added.

Or that's the intention: but Dave seems to have found an unexpected
exception, despite us holding the page lock across all this.

(But if it weren't for the memcg and replace_page issues, I'd much
prefer to let shmem_add_to_page_cache discover the race as before.)

Hugh

 otherwise, will goto unlock and then go to repeat? where I miss?
 
 Regards,
 Chen
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Ni zhan Chen

On 10/25/2012 02:59 PM, Hugh Dickins wrote:

On Thu, 25 Oct 2012, Ni zhan Chen wrote:

On 10/25/2012 12:36 PM, Hugh Dickins wrote:

On Wed, 24 Oct 2012, Dave Jones wrote:


Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49

1148 error = shmem_add_to_page_cache(page,
mapping, index,
1149 gfp,
swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no
allocation */
1151 VM_BUG_ON(error);
1152 }

That's very surprising.  Easy enough to handle an error there, but
of course I made it a VM_BUG_ON because it violates my assumptions:
I rather need to understand how this can be, and I've no idea.

Clutching at straws, I expect this is entirely irrelevant, but:
there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
in current linux.git; rather, there's a VM_BUG_ON on line 1149.

So you've inserted a couple of lines for some reason (more useful
trinity behaviour, perhaps)?  And have some config option I'm
unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?

Hi Hugh,

I think it maybe caused by your commit [d189922862e03ce: shmem: fix negative
rss in memcg memory.stat], one question:

Well, yes, I added the VM_BUG_ON in that commit.


if function shmem_confirm_swap confirm the entry has already brought back
from swap by a racing thread,

The reverse: true confirms that the swap entry has not been brought back
from swap by a racing thread; false indicates that there has been a race.


then why call shmem_add_to_page_cache to add
page from swapcache to pagecache again?

Adding it to pagecache again, after such a race, would set error to
-EEXIST (originating from radix_tree_insert); but we don't do that,
we add it to pagecache when it has not already been added.

Or that's the intention: but Dave seems to have found an unexpected
exception, despite us holding the page lock across all this.

(But if it weren't for the memcg and replace_page issues, I'd much
prefer to let shmem_add_to_page_cache discover the race as before.)

Hugh


Hi Hugh

Thanks for your response. You mean the -EEXIST originating from 
radix_tree_insert, in radix_tree_insert:

if (slot != NULL)
return -EEXIST;
But why slot should be NULL? if no race, the pagecache related radix 
tree entry should be RADIX_TREE_EXCEPTIONAL_ENTRY+swap_entry_t.val, 
where I miss?


Regards,
Chen




otherwise, will goto unlock and then go to repeat? where I miss?

Regards,
Chen


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Dave Jones
On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:

   1148 error = shmem_add_to_page_cache(page, 
   mapping, index,
   1149 gfp, 
   swp_to_radix_entry(swap));
   1150 /* We already confirmed swap, and make no 
   allocation */
   1151 VM_BUG_ON(error);
   1152 }
  
  That's very surprising.  Easy enough to handle an error there, but
  of course I made it a VM_BUG_ON because it violates my assumptions:
  I rather need to understand how this can be, and I've no idea.
  
  Clutching at straws, I expect this is entirely irrelevant, but:
  there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
  in current linux.git; rather, there's a VM_BUG_ON on line 1149.
  
  So you've inserted a couple of lines for some reason (more useful
  trinity behaviour, perhaps)? 

detritus from the recent mpol_to_str bug that I was chasing.
Shouldn't be relevant...

diff -durpN '--exclude-from=/home/davej/.exclude' src/git-trees/kernel/linux/mm
--- src/git-trees/kernel/linux/mm/shmem.c   2012-10-12 10:01:46.613408580 -
+++ linux-dj/mm/shmem.c 2012-10-15 12:31:32.979653309 -0400
@@ -885,13 +885,15 @@ redirty:
 static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
 {
char buffer[64];
+   int ret;
 
if (!mpol || mpol-mode == MPOL_DEFAULT)
return; /* show nothing */
 
-   mpol_to_str(buffer, sizeof(buffer), mpol, 1);
-
-   seq_printf(seq, ,mpol=%s, buffer);
+   memset(buffer, 0, sizeof(buffer));
+   ret = mpol_to_str(buffer, sizeof(buffer), mpol, 1);
+   if (ret  0)
+   seq_printf(seq, ,mpol=%s, buffer);
 }


  And have some config option I'm
  unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?

Yes, I do have this..

-#define VM_BUG_ON(cond) BUG_ON(cond)
+#define VM_BUG_ON(cond) WARN_ON(cond)

because I got tired of things not going over my usb serial port when I hit them
a while ago. BUG_ON is pretty unfriendly to bug finding.

Dave

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Johannes Weiner
On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
 On Wed, 24 Oct 2012, Dave Jones wrote:
 
  Machine under significant load (4gb memory used, swap usage fluctuating)
  triggered this...
  
  WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
  Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
  Call Trace:
   [8107100f] warn_slowpath_common+0x7f/0xc0
   [8107106a] warn_slowpath_null+0x1a/0x20
   [811903fc] shmem_getpage_gfp+0xa5c/0xa70
   [8118fc3e] ? shmem_getpage_gfp+0x29e/0xa70
   [81190e4f] shmem_fault+0x4f/0xa0
   [8119f391] __do_fault+0x71/0x5c0
   [810e1ac6] ? __lock_acquire+0x306/0x1ba0
   [810b6ff9] ? local_clock+0x89/0xa0
   [811a2767] handle_pte_fault+0x97/0xae0
   [816d1069] ? sub_preempt_count+0x79/0xd0
   [8136d68e] ? delay_tsc+0xae/0x120
   [8136d578] ? __const_udelay+0x28/0x30
   [811a4a39] handle_mm_fault+0x289/0x350
   [816d091e] __do_page_fault+0x18e/0x530
   [810b6ff9] ? local_clock+0x89/0xa0
   [810b0e51] ? get_parent_ip+0x11/0x50
   [810b0e51] ? get_parent_ip+0x11/0x50
   [816d1069] ? sub_preempt_count+0x79/0xd0
   [8112d389] ? rcu_user_exit+0xc9/0xf0
   [816d0ceb] do_page_fault+0x2b/0x50
   [816cd3b8] page_fault+0x28/0x30
   [8136d259] ? copy_user_enhanced_fast_string+0x9/0x20
   [8121c181] ? sys_futimesat+0x41/0xe0
   [8102bf35] ? syscall_trace_enter+0x25/0x2c0
   [816d5625] ? tracesys+0x7e/0xe6
   [816d5688] tracesys+0xe1/0xe6
  
  
  
  1148 error = shmem_add_to_page_cache(page, mapping, 
  index,
  1149 gfp, 
  swp_to_radix_entry(swap));
  1150 /* We already confirmed swap, and make no 
  allocation */
  1151 VM_BUG_ON(error);
  1152 }
 
 That's very surprising.  Easy enough to handle an error there, but
 of course I made it a VM_BUG_ON because it violates my assumptions:
 I rather need to understand how this can be, and I've no idea.

Could it be concurrent truncation clearing out the entry between
shmem_confirm_swap() and shmem_add_to_page_cache()?  I don't see
anything preventing that.

The empty slot would not match the expected swap entry this call
passes in and the returned error would be -ENOENT.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Hugh Dickins
On Thu, 25 Oct 2012, Ni zhan Chen wrote:
 On 10/25/2012 02:59 PM, Hugh Dickins wrote:
  On Thu, 25 Oct 2012, Ni zhan Chen wrote:
   
   I think it maybe caused by your commit [d189922862e03ce: shmem: fix
   negative
   rss in memcg memory.stat], one question:
  Well, yes, I added the VM_BUG_ON in that commit.
  
   if function shmem_confirm_swap confirm the entry has already brought back
   from swap by a racing thread,
  The reverse: true confirms that the swap entry has not been brought back
  from swap by a racing thread; false indicates that there has been a race.
  
   then why call shmem_add_to_page_cache to add
   page from swapcache to pagecache again?
  Adding it to pagecache again, after such a race, would set error to
  -EEXIST (originating from radix_tree_insert); but we don't do that,
  we add it to pagecache when it has not already been added.
  
  Or that's the intention: but Dave seems to have found an unexpected
  exception, despite us holding the page lock across all this.
  
  (But if it weren't for the memcg and replace_page issues, I'd much
  prefer to let shmem_add_to_page_cache discover the race as before.)
  
  Hugh
 
 Hi Hugh
 
 Thanks for your response. You mean the -EEXIST originating from
 radix_tree_insert, in radix_tree_insert:
 if (slot != NULL)
 return -EEXIST;
 But why slot should be NULL? if no race, the pagecache related radix tree
 entry should be RADIX_TREE_EXCEPTIONAL_ENTRY+swap_entry_t.val, where I miss?

I was describing what would happen in a case that should not exist,
that you had thought the common case.  In actuality, the entry should
not be NULL, it should be as you say there.

Hugh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Hugh Dickins
On Thu, 25 Oct 2012, Dave Jones wrote:
 On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
 
   Clutching at straws, I expect this is entirely irrelevant, but:
   there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
   in current linux.git; rather, there's a VM_BUG_ON on line 1149.
   
   So you've inserted a couple of lines for some reason (more useful
   trinity behaviour, perhaps)? 
 
 detritus from the recent mpol_to_str bug that I was chasing.
 Shouldn't be relevant...
 
   And have some config option I'm
   unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?
 
 Yes, I do have this..
 
 -#define VM_BUG_ON(cond) BUG_ON(cond)
 +#define VM_BUG_ON(cond) WARN_ON(cond)
 
 because I got tired of things not going over my usb serial port when I hit 
 them
 a while ago. BUG_ON is pretty unfriendly to bug finding.

Makes sense, thanks for the reassurance.

Hugh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Hugh Dickins
On Thu, 25 Oct 2012, Johannes Weiner wrote:
 On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:
  On Wed, 24 Oct 2012, Dave Jones wrote:
  
   Machine under significant load (4gb memory used, swap usage fluctuating)
   triggered this...
   
   WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
   Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
   
   1148 error = shmem_add_to_page_cache(page, 
   mapping, index,
   1149 gfp, 
   swp_to_radix_entry(swap));
   1150 /* We already confirmed swap, and make no 
   allocation */
   1151 VM_BUG_ON(error);
   1152 }
  
  That's very surprising.  Easy enough to handle an error there, but
  of course I made it a VM_BUG_ON because it violates my assumptions:
  I rather need to understand how this can be, and I've no idea.
 
 Could it be concurrent truncation clearing out the entry between
 shmem_confirm_swap() and shmem_add_to_page_cache()?  I don't see
 anything preventing that.
 
 The empty slot would not match the expected swap entry this call
 passes in and the returned error would be -ENOENT.

Excellent notion, many thanks Hannes, I believe you've got it.

I've hit that truncation problem in swapoff (and commented on it
in shmem_unuse_inode), but never hit it or considered it here.
I think of the page lock as holding it stable, but truncation's
free_swap_and_cache only does a trylock on the swapcache page,
so we're not secured against that possibility.

So I'd like to change it to VM_BUG_ON(error  error != -ENOENT),
but there's a little tidying up to do in the -ENOENT case, which
needs more thought.  A delete_from_swap_cache(page) - though we
can be lazy and leave that to reclaim for such a rare occurrence -
and probably a mem_cgroup uncharge; but the memcg hooks are always
the hardest to get right, I'll have think about that one carefully.

Hugh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Ni zhan Chen

On 10/26/2012 05:27 AM, Hugh Dickins wrote:

On Thu, 25 Oct 2012, Ni zhan Chen wrote:

On 10/25/2012 02:59 PM, Hugh Dickins wrote:

On Thu, 25 Oct 2012, Ni zhan Chen wrote:

I think it maybe caused by your commit [d189922862e03ce: shmem: fix
negative
rss in memcg memory.stat], one question:

Well, yes, I added the VM_BUG_ON in that commit.


if function shmem_confirm_swap confirm the entry has already brought back
from swap by a racing thread,

The reverse: true confirms that the swap entry has not been brought back
from swap by a racing thread; false indicates that there has been a race.


then why call shmem_add_to_page_cache to add
page from swapcache to pagecache again?

Adding it to pagecache again, after such a race, would set error to
-EEXIST (originating from radix_tree_insert); but we don't do that,
we add it to pagecache when it has not already been added.

Or that's the intention: but Dave seems to have found an unexpected
exception, despite us holding the page lock across all this.

(But if it weren't for the memcg and replace_page issues, I'd much
prefer to let shmem_add_to_page_cache discover the race as before.)

Hugh

Hi Hugh

Thanks for your response. You mean the -EEXIST originating from
radix_tree_insert, in radix_tree_insert:
if (slot != NULL)
 return -EEXIST;
But why slot should be NULL? if no race, the pagecache related radix tree
entry should be RADIX_TREE_EXCEPTIONAL_ENTRY+swap_entry_t.val, where I miss?

I was describing what would happen in a case that should not exist,
that you had thought the common case.  In actuality, the entry should
not be NULL, it should be as you say there.


Thanks for your patience. So in the common case, the entry should be the 
value I mentioned, then why has this check?

if (slot != NULL)
return -EEXIST;

the common case will return -EEXIST.



Hugh



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-25 Thread Ni zhan Chen

On 10/26/2012 05:48 AM, Hugh Dickins wrote:

On Thu, 25 Oct 2012, Johannes Weiner wrote:

On Wed, Oct 24, 2012 at 09:36:27PM -0700, Hugh Dickins wrote:

On Wed, 24 Oct 2012, Dave Jones wrote:


Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49

1148 error = shmem_add_to_page_cache(page, mapping, 
index,
1149 gfp, 
swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no 
allocation */
1151 VM_BUG_ON(error);
1152 }

That's very surprising.  Easy enough to handle an error there, but
of course I made it a VM_BUG_ON because it violates my assumptions:
I rather need to understand how this can be, and I've no idea.

Could it be concurrent truncation clearing out the entry between
shmem_confirm_swap() and shmem_add_to_page_cache()?  I don't see
anything preventing that.

The empty slot would not match the expected swap entry this call
passes in and the returned error would be -ENOENT.

Excellent notion, many thanks Hannes, I believe you've got it.

I've hit that truncation problem in swapoff (and commented on it
in shmem_unuse_inode), but never hit it or considered it here.
I think of the page lock as holding it stable, but truncation's
free_swap_and_cache only does a trylock on the swapcache page,
so we're not secured against that possibility.


Hi Hugh,

Even though free_swap_and_cache only does a trylock on the swapcache 
page, but it doens't call delete_from_swap_cache and the associated 
entry should still be there, I am interested in what you have already 
introduce to protect it?




So I'd like to change it to VM_BUG_ON(error  error != -ENOENT),
but there's a little tidying up to do in the -ENOENT case, which


Do you mean radix_tree_insert will return -ENOENT if the associated 
entry is not present? Why I can't find this return value in the function 
radix_tree_insert?



needs more thought.  A delete_from_swap_cache(page) - though we
can be lazy and leave that to reclaim for such a rare occurrence -
and probably a mem_cgroup uncharge; but the memcg hooks are always
the hardest to get right, I'll have think about that one carefully.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-24 Thread Ni zhan Chen

On 10/25/2012 12:36 PM, Hugh Dickins wrote:

On Wed, 24 Oct 2012, Dave Jones wrote:


Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
Call Trace:
  [] warn_slowpath_common+0x7f/0xc0
  [] warn_slowpath_null+0x1a/0x20
  [] shmem_getpage_gfp+0xa5c/0xa70
  [] ? shmem_getpage_gfp+0x29e/0xa70
  [] shmem_fault+0x4f/0xa0
  [] __do_fault+0x71/0x5c0
  [] ? __lock_acquire+0x306/0x1ba0
  [] ? local_clock+0x89/0xa0
  [] handle_pte_fault+0x97/0xae0
  [] ? sub_preempt_count+0x79/0xd0
  [] ? delay_tsc+0xae/0x120
  [] ? __const_udelay+0x28/0x30
  [] handle_mm_fault+0x289/0x350
  [] __do_page_fault+0x18e/0x530
  [] ? local_clock+0x89/0xa0
  [] ? get_parent_ip+0x11/0x50
  [] ? get_parent_ip+0x11/0x50
  [] ? sub_preempt_count+0x79/0xd0
  [] ? rcu_user_exit+0xc9/0xf0
  [] do_page_fault+0x2b/0x50
  [] page_fault+0x28/0x30
  [] ? copy_user_enhanced_fast_string+0x9/0x20
  [] ? sys_futimesat+0x41/0xe0
  [] ? syscall_trace_enter+0x25/0x2c0
  [] ? tracesys+0x7e/0xe6
  [] tracesys+0xe1/0xe6



1148 error = shmem_add_to_page_cache(page, mapping, 
index,
1149 gfp, 
swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no 
allocation */
1151 VM_BUG_ON(error);
1152 }

That's very surprising.  Easy enough to handle an error there, but
of course I made it a VM_BUG_ON because it violates my assumptions:
I rather need to understand how this can be, and I've no idea.

Clutching at straws, I expect this is entirely irrelevant, but:
there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
in current linux.git; rather, there's a VM_BUG_ON on line 1149.

So you've inserted a couple of lines for some reason (more useful
trinity behaviour, perhaps)?  And have some config option I'm
unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?


Hi Hugh,

I think it maybe caused by your commit [d189922862e03ce: shmem: fix 
negative rss in memcg memory.stat], one question:


if function shmem_confirm_swap confirm the entry has already brought 
back from swap by a racing thread, then why call shmem_add_to_page_cache 
to add page from swapcache to pagecache again? otherwise, will goto 
unlock and then go to repeat? where I miss?


Regards,
Chen



Hugh



  total   used   free sharedbuffers cached
Mem:   388552828540641031464  0   9624  19208
-/+ buffers/cache:28252321060296
Swap:  6029308  306565998652

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: mailto:"d...@kvack.org;> em...@kvack.org 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-24 Thread Hugh Dickins
On Wed, 24 Oct 2012, Dave Jones wrote:

> Machine under significant load (4gb memory used, swap usage fluctuating)
> triggered this...
> 
> WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
> Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
> Call Trace:
>  [] warn_slowpath_common+0x7f/0xc0
>  [] warn_slowpath_null+0x1a/0x20
>  [] shmem_getpage_gfp+0xa5c/0xa70
>  [] ? shmem_getpage_gfp+0x29e/0xa70
>  [] shmem_fault+0x4f/0xa0
>  [] __do_fault+0x71/0x5c0
>  [] ? __lock_acquire+0x306/0x1ba0
>  [] ? local_clock+0x89/0xa0
>  [] handle_pte_fault+0x97/0xae0
>  [] ? sub_preempt_count+0x79/0xd0
>  [] ? delay_tsc+0xae/0x120
>  [] ? __const_udelay+0x28/0x30
>  [] handle_mm_fault+0x289/0x350
>  [] __do_page_fault+0x18e/0x530
>  [] ? local_clock+0x89/0xa0
>  [] ? get_parent_ip+0x11/0x50
>  [] ? get_parent_ip+0x11/0x50
>  [] ? sub_preempt_count+0x79/0xd0
>  [] ? rcu_user_exit+0xc9/0xf0
>  [] do_page_fault+0x2b/0x50
>  [] page_fault+0x28/0x30
>  [] ? copy_user_enhanced_fast_string+0x9/0x20
>  [] ? sys_futimesat+0x41/0xe0
>  [] ? syscall_trace_enter+0x25/0x2c0
>  [] ? tracesys+0x7e/0xe6
>  [] tracesys+0xe1/0xe6
> 
> 
> 
> 1148 error = shmem_add_to_page_cache(page, mapping, 
> index,
> 1149 gfp, 
> swp_to_radix_entry(swap));
> 1150 /* We already confirmed swap, and make no 
> allocation */
> 1151 VM_BUG_ON(error);
> 1152 }

That's very surprising.  Easy enough to handle an error there, but
of course I made it a VM_BUG_ON because it violates my assumptions:
I rather need to understand how this can be, and I've no idea.

Clutching at straws, I expect this is entirely irrelevant, but:
there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
in current linux.git; rather, there's a VM_BUG_ON on line 1149.

So you've inserted a couple of lines for some reason (more useful
trinity behaviour, perhaps)?  And have some config option I'm
unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?

Hugh

> 
> 
>  total   used   free sharedbuffers cached
> Mem:   388552828540641031464  0   9624  19208
> -/+ buffers/cache:28252321060296
> Swap:  6029308  306565998652
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-24 Thread Dave Jones
Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
Call Trace:
 [] warn_slowpath_common+0x7f/0xc0
 [] warn_slowpath_null+0x1a/0x20
 [] shmem_getpage_gfp+0xa5c/0xa70
 [] ? shmem_getpage_gfp+0x29e/0xa70
 [] shmem_fault+0x4f/0xa0
 [] __do_fault+0x71/0x5c0
 [] ? __lock_acquire+0x306/0x1ba0
 [] ? local_clock+0x89/0xa0
 [] handle_pte_fault+0x97/0xae0
 [] ? sub_preempt_count+0x79/0xd0
 [] ? delay_tsc+0xae/0x120
 [] ? __const_udelay+0x28/0x30
 [] handle_mm_fault+0x289/0x350
 [] __do_page_fault+0x18e/0x530
 [] ? local_clock+0x89/0xa0
 [] ? get_parent_ip+0x11/0x50
 [] ? get_parent_ip+0x11/0x50
 [] ? sub_preempt_count+0x79/0xd0
 [] ? rcu_user_exit+0xc9/0xf0
 [] do_page_fault+0x2b/0x50
 [] page_fault+0x28/0x30
 [] ? copy_user_enhanced_fast_string+0x9/0x20
 [] ? sys_futimesat+0x41/0xe0
 [] ? syscall_trace_enter+0x25/0x2c0
 [] ? tracesys+0x7e/0xe6
 [] tracesys+0xe1/0xe6



1148 error = shmem_add_to_page_cache(page, mapping, 
index,
1149 gfp, 
swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no 
allocation */
1151 VM_BUG_ON(error);
1152 }


 total   used   free sharedbuffers cached
Mem:   388552828540641031464  0   9624  19208
-/+ buffers/cache:28252321060296
Swap:  6029308  306565998652


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-24 Thread Dave Jones
Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
Call Trace:
 [8107100f] warn_slowpath_common+0x7f/0xc0
 [8107106a] warn_slowpath_null+0x1a/0x20
 [811903fc] shmem_getpage_gfp+0xa5c/0xa70
 [8118fc3e] ? shmem_getpage_gfp+0x29e/0xa70
 [81190e4f] shmem_fault+0x4f/0xa0
 [8119f391] __do_fault+0x71/0x5c0
 [810e1ac6] ? __lock_acquire+0x306/0x1ba0
 [810b6ff9] ? local_clock+0x89/0xa0
 [811a2767] handle_pte_fault+0x97/0xae0
 [816d1069] ? sub_preempt_count+0x79/0xd0
 [8136d68e] ? delay_tsc+0xae/0x120
 [8136d578] ? __const_udelay+0x28/0x30
 [811a4a39] handle_mm_fault+0x289/0x350
 [816d091e] __do_page_fault+0x18e/0x530
 [810b6ff9] ? local_clock+0x89/0xa0
 [810b0e51] ? get_parent_ip+0x11/0x50
 [810b0e51] ? get_parent_ip+0x11/0x50
 [816d1069] ? sub_preempt_count+0x79/0xd0
 [8112d389] ? rcu_user_exit+0xc9/0xf0
 [816d0ceb] do_page_fault+0x2b/0x50
 [816cd3b8] page_fault+0x28/0x30
 [8136d259] ? copy_user_enhanced_fast_string+0x9/0x20
 [8121c181] ? sys_futimesat+0x41/0xe0
 [8102bf35] ? syscall_trace_enter+0x25/0x2c0
 [816d5625] ? tracesys+0x7e/0xe6
 [816d5688] tracesys+0xe1/0xe6



1148 error = shmem_add_to_page_cache(page, mapping, 
index,
1149 gfp, 
swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no 
allocation */
1151 VM_BUG_ON(error);
1152 }


 total   used   free sharedbuffers cached
Mem:   388552828540641031464  0   9624  19208
-/+ buffers/cache:28252321060296
Swap:  6029308  306565998652


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-24 Thread Hugh Dickins
On Wed, 24 Oct 2012, Dave Jones wrote:

 Machine under significant load (4gb memory used, swap usage fluctuating)
 triggered this...
 
 WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
 Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
 Call Trace:
  [8107100f] warn_slowpath_common+0x7f/0xc0
  [8107106a] warn_slowpath_null+0x1a/0x20
  [811903fc] shmem_getpage_gfp+0xa5c/0xa70
  [8118fc3e] ? shmem_getpage_gfp+0x29e/0xa70
  [81190e4f] shmem_fault+0x4f/0xa0
  [8119f391] __do_fault+0x71/0x5c0
  [810e1ac6] ? __lock_acquire+0x306/0x1ba0
  [810b6ff9] ? local_clock+0x89/0xa0
  [811a2767] handle_pte_fault+0x97/0xae0
  [816d1069] ? sub_preempt_count+0x79/0xd0
  [8136d68e] ? delay_tsc+0xae/0x120
  [8136d578] ? __const_udelay+0x28/0x30
  [811a4a39] handle_mm_fault+0x289/0x350
  [816d091e] __do_page_fault+0x18e/0x530
  [810b6ff9] ? local_clock+0x89/0xa0
  [810b0e51] ? get_parent_ip+0x11/0x50
  [810b0e51] ? get_parent_ip+0x11/0x50
  [816d1069] ? sub_preempt_count+0x79/0xd0
  [8112d389] ? rcu_user_exit+0xc9/0xf0
  [816d0ceb] do_page_fault+0x2b/0x50
  [816cd3b8] page_fault+0x28/0x30
  [8136d259] ? copy_user_enhanced_fast_string+0x9/0x20
  [8121c181] ? sys_futimesat+0x41/0xe0
  [8102bf35] ? syscall_trace_enter+0x25/0x2c0
  [816d5625] ? tracesys+0x7e/0xe6
  [816d5688] tracesys+0xe1/0xe6
 
 
 
 1148 error = shmem_add_to_page_cache(page, mapping, 
 index,
 1149 gfp, 
 swp_to_radix_entry(swap));
 1150 /* We already confirmed swap, and make no 
 allocation */
 1151 VM_BUG_ON(error);
 1152 }

That's very surprising.  Easy enough to handle an error there, but
of course I made it a VM_BUG_ON because it violates my assumptions:
I rather need to understand how this can be, and I've no idea.

Clutching at straws, I expect this is entirely irrelevant, but:
there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
in current linux.git; rather, there's a VM_BUG_ON on line 1149.

So you've inserted a couple of lines for some reason (more useful
trinity behaviour, perhaps)?  And have some config option I'm
unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?

Hugh

 
 
  total   used   free sharedbuffers cached
 Mem:   388552828540641031464  0   9624  19208
 -/+ buffers/cache:28252321060296
 Swap:  6029308  306565998652
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: shmem_getpage_gfp VM_BUG_ON triggered. [3.7rc2]

2012-10-24 Thread Ni zhan Chen

On 10/25/2012 12:36 PM, Hugh Dickins wrote:

On Wed, 24 Oct 2012, Dave Jones wrote:


Machine under significant load (4gb memory used, swap usage fluctuating)
triggered this...

WARNING: at mm/shmem.c:1151 shmem_getpage_gfp+0xa5c/0xa70()
Pid: 29795, comm: trinity-child4 Not tainted 3.7.0-rc2+ #49
Call Trace:
  [8107100f] warn_slowpath_common+0x7f/0xc0
  [8107106a] warn_slowpath_null+0x1a/0x20
  [811903fc] shmem_getpage_gfp+0xa5c/0xa70
  [8118fc3e] ? shmem_getpage_gfp+0x29e/0xa70
  [81190e4f] shmem_fault+0x4f/0xa0
  [8119f391] __do_fault+0x71/0x5c0
  [810e1ac6] ? __lock_acquire+0x306/0x1ba0
  [810b6ff9] ? local_clock+0x89/0xa0
  [811a2767] handle_pte_fault+0x97/0xae0
  [816d1069] ? sub_preempt_count+0x79/0xd0
  [8136d68e] ? delay_tsc+0xae/0x120
  [8136d578] ? __const_udelay+0x28/0x30
  [811a4a39] handle_mm_fault+0x289/0x350
  [816d091e] __do_page_fault+0x18e/0x530
  [810b6ff9] ? local_clock+0x89/0xa0
  [810b0e51] ? get_parent_ip+0x11/0x50
  [810b0e51] ? get_parent_ip+0x11/0x50
  [816d1069] ? sub_preempt_count+0x79/0xd0
  [8112d389] ? rcu_user_exit+0xc9/0xf0
  [816d0ceb] do_page_fault+0x2b/0x50
  [816cd3b8] page_fault+0x28/0x30
  [8136d259] ? copy_user_enhanced_fast_string+0x9/0x20
  [8121c181] ? sys_futimesat+0x41/0xe0
  [8102bf35] ? syscall_trace_enter+0x25/0x2c0
  [816d5625] ? tracesys+0x7e/0xe6
  [816d5688] tracesys+0xe1/0xe6



1148 error = shmem_add_to_page_cache(page, mapping, 
index,
1149 gfp, 
swp_to_radix_entry(swap));
1150 /* We already confirmed swap, and make no 
allocation */
1151 VM_BUG_ON(error);
1152 }

That's very surprising.  Easy enough to handle an error there, but
of course I made it a VM_BUG_ON because it violates my assumptions:
I rather need to understand how this can be, and I've no idea.

Clutching at straws, I expect this is entirely irrelevant, but:
there isn't a warning on line 1151 of mm/shmem.c in 3.7.0-rc2 nor
in current linux.git; rather, there's a VM_BUG_ON on line 1149.

So you've inserted a couple of lines for some reason (more useful
trinity behaviour, perhaps)?  And have some config option I'm
unfamiliar with, that mutates a BUG_ON or VM_BUG_ON into a warning?


Hi Hugh,

I think it maybe caused by your commit [d189922862e03ce: shmem: fix 
negative rss in memcg memory.stat], one question:


if function shmem_confirm_swap confirm the entry has already brought 
back from swap by a racing thread, then why call shmem_add_to_page_cache 
to add page from swapcache to pagecache again? otherwise, will goto 
unlock and then go to repeat? where I miss?


Regards,
Chen



Hugh



  total   used   free sharedbuffers cached
Mem:   388552828540641031464  0   9624  19208
-/+ buffers/cache:28252321060296
Swap:  6029308  306565998652

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/