Re: boot warnings due to swap: make each swap partition have one address_space

2013-02-03 Thread Hugh Dickins
On Wed, 30 Jan 2013, Shaohua Li wrote:
> On Sun, Jan 27, 2013 at 01:40:40PM -0800, Hugh Dickins wrote:
> > 
> > I'm glad Minchan has now pointed you to Rik's posting of two years ago:
> > I think there are more important changes to be made in that direction.
> 
> Not sure how others use multiple swaps, but current lock contention forces us
> to use multiple swaps. I haven't carefully think about Rik's posting, but 
> looks
> it doesn't solve the lock contention problem.

Nobody had reported any swap lock contention problem before your patch,
so no, Rik's posting wasn't directed at that.  I always thought swap
writing patterns a much bigger problem.

But if lock contention there is, then I think it can be implemented
with reducing that in mind.  There are two levels of allocation: one
to allocate the tokens which we will insert in page tables, and one
to allocate the final diskspace to which those tokens will point.

(I may be using totally different language from Rik,
it's the principles that I have in mind, not his actual posting.)

Allocating the tokens can very well be done with per-cpu batches,
perhaps of SWAP_CLUSTER_MAX 32 to match vmscan.c's batching: there
is no significance to their ordering.  And allocating the diskspace
would want to be done in batches, to maximize contiguous writing.

That may not solve all the swap_info_get() contention which you saw,
but should help some.

I'm thinking that we go with your per-swapper-space locking for now;
but I wouldn't mind taking it out again later, if we arrive at a
better solution which benefits even those with a single swap area.

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: boot warnings due to swap: make each swap partition have one address_space

2013-02-03 Thread Hugh Dickins
On Wed, 30 Jan 2013, Shaohua Li wrote:
 On Sun, Jan 27, 2013 at 01:40:40PM -0800, Hugh Dickins wrote:
  
  I'm glad Minchan has now pointed you to Rik's posting of two years ago:
  I think there are more important changes to be made in that direction.
 
 Not sure how others use multiple swaps, but current lock contention forces us
 to use multiple swaps. I haven't carefully think about Rik's posting, but 
 looks
 it doesn't solve the lock contention problem.

Nobody had reported any swap lock contention problem before your patch,
so no, Rik's posting wasn't directed at that.  I always thought swap
writing patterns a much bigger problem.

But if lock contention there is, then I think it can be implemented
with reducing that in mind.  There are two levels of allocation: one
to allocate the tokens which we will insert in page tables, and one
to allocate the final diskspace to which those tokens will point.

(I may be using totally different language from Rik,
it's the principles that I have in mind, not his actual posting.)

Allocating the tokens can very well be done with per-cpu batches,
perhaps of SWAP_CLUSTER_MAX 32 to match vmscan.c's batching: there
is no significance to their ordering.  And allocating the diskspace
would want to be done in batches, to maximize contiguous writing.

That may not solve all the swap_info_get() contention which you saw,
but should help some.

I'm thinking that we go with your per-swapper-space locking for now;
but I wouldn't mind taking it out again later, if we arrive at a
better solution which benefits even those with a single swap area.

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: boot warnings due to swap: make each swap partition have one address_space

2013-01-30 Thread Shaohua Li
On Sun, Jan 27, 2013 at 01:40:40PM -0800, Hugh Dickins wrote:
> On Sun, 27 Jan 2013, Shaohua Li wrote:
> > On Sat, Jan 26, 2013 at 06:16:05PM -0800, Hugh Dickins wrote:
> > > On Fri, 25 Jan 2013, Shaohua Li wrote:
> > > > On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
> > > > 
> > > > Subject: give-each-swapper-space-separate-backing_dev_info
> > > > 
> > > > The backing_dev_info can't be shared by all swapper address space.
> > > 
> > > Whyever not?  It's perfectly normal for different inodes/address_spaces
> > > to share a single backing_dev!  Sasha's trace says that it's wrong to
> > > initialize it MAX_SWAPFILES times: fair enough.  But why should I now
> > > want to spend 32kB (not even counting their __percpu counters) on all
> > > these pseudo-backing_devs?
> > 
> > That's correct, silly me. Updated it.
> 
> Looks much more to my taste, thank you!
> 
> > > 
> > > p.s. a grand little change would be to move page_cluster and swap_setup()
> > > from mm/swap.c to mm/swap_state.c: they have nothing to do with the other
> > > contents of swap.c, and everything to do with the contents of 
> > > swap_state.c.
> > > Why swap.c is called swap.c is rather a mystery.
> > 
> > Tried, but looks page_cluster is used in sysctl, moving to swap_state.c will
> > make it optional. don't want to add another #ifdef, so give up.
> 
> Good point, thanks for trying, maybe I'll attack it next time it
> irritates me.
> 
> I don't yet know whether I approve of your changes or not, but running
> with them to see (and I'll send another bugfix separately in a moment).
> 
> I was the one who removed the swap_device_lock() which 2.4 used,
> because it almost always ended up having to take both swap_list_lock()
> and swap_device_lock(si).  You seem to have done a much better job of
> separating them usefully, but I need to convince myself that it does
> end up safely.
> 
> My reservations so far would be: how many installations actually have
> more than one swap area, so is it a good tradeoff to add more overhead
> to help those at the (slight) expense of everyone else?  The increasingly
> ugly page_mapping() worries me, and the static array of swapper_spaces
> annoys me a little.
> 
> I'm glad Minchan has now pointed you to Rik's posting of two years ago:
> I think there are more important changes to be made in that direction.

Not sure how others use multiple swaps, but current lock contention forces us
to use multiple swaps. I haven't carefully think about Rik's posting, but looks
it doesn't solve the lock contention problem.

Thanks,
Shaohua
--
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: boot warnings due to swap: make each swap partition have one address_space

2013-01-30 Thread Shaohua Li
On Sun, Jan 27, 2013 at 01:40:40PM -0800, Hugh Dickins wrote:
 On Sun, 27 Jan 2013, Shaohua Li wrote:
  On Sat, Jan 26, 2013 at 06:16:05PM -0800, Hugh Dickins wrote:
   On Fri, 25 Jan 2013, Shaohua Li wrote:
On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:

Subject: give-each-swapper-space-separate-backing_dev_info

The backing_dev_info can't be shared by all swapper address space.
   
   Whyever not?  It's perfectly normal for different inodes/address_spaces
   to share a single backing_dev!  Sasha's trace says that it's wrong to
   initialize it MAX_SWAPFILES times: fair enough.  But why should I now
   want to spend 32kB (not even counting their __percpu counters) on all
   these pseudo-backing_devs?
  
  That's correct, silly me. Updated it.
 
 Looks much more to my taste, thank you!
 
   
   p.s. a grand little change would be to move page_cluster and swap_setup()
   from mm/swap.c to mm/swap_state.c: they have nothing to do with the other
   contents of swap.c, and everything to do with the contents of 
   swap_state.c.
   Why swap.c is called swap.c is rather a mystery.
  
  Tried, but looks page_cluster is used in sysctl, moving to swap_state.c will
  make it optional. don't want to add another #ifdef, so give up.
 
 Good point, thanks for trying, maybe I'll attack it next time it
 irritates me.
 
 I don't yet know whether I approve of your changes or not, but running
 with them to see (and I'll send another bugfix separately in a moment).
 
 I was the one who removed the swap_device_lock() which 2.4 used,
 because it almost always ended up having to take both swap_list_lock()
 and swap_device_lock(si).  You seem to have done a much better job of
 separating them usefully, but I need to convince myself that it does
 end up safely.
 
 My reservations so far would be: how many installations actually have
 more than one swap area, so is it a good tradeoff to add more overhead
 to help those at the (slight) expense of everyone else?  The increasingly
 ugly page_mapping() worries me, and the static array of swapper_spaces
 annoys me a little.
 
 I'm glad Minchan has now pointed you to Rik's posting of two years ago:
 I think there are more important changes to be made in that direction.

Not sure how others use multiple swaps, but current lock contention forces us
to use multiple swaps. I haven't carefully think about Rik's posting, but looks
it doesn't solve the lock contention problem.

Thanks,
Shaohua
--
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: boot warnings due to swap: make each swap partition have one address_space

2013-01-29 Thread Valdis . Kletnieks
On Sun, 27 Jan 2013 13:40:40 -0800, Hugh Dickins said:

> My reservations so far would be: how many installations actually have
> more than one swap area, so is it a good tradeoff to add more overhead
> to help those at the (slight) expense of everyone else?  The increasingly
> ugly page_mapping() worries me, and the static array of swapper_spaces
> annoys me a little.

Right now, probably few.  But the number may go up a lot if the whole
'zram-for-swapspace' thing catches on and/or ships in a distro...


pgpqA5wagdB_3.pgp
Description: PGP signature


Re: boot warnings due to swap: make each swap partition have one address_space

2013-01-29 Thread Valdis . Kletnieks
On Sun, 27 Jan 2013 13:40:40 -0800, Hugh Dickins said:

 My reservations so far would be: how many installations actually have
 more than one swap area, so is it a good tradeoff to add more overhead
 to help those at the (slight) expense of everyone else?  The increasingly
 ugly page_mapping() worries me, and the static array of swapper_spaces
 annoys me a little.

Right now, probably few.  But the number may go up a lot if the whole
'zram-for-swapspace' thing catches on and/or ships in a distro...


pgpqA5wagdB_3.pgp
Description: PGP signature


Re: boot warnings due to swap: make each swap partition have one address_space

2013-01-27 Thread Hugh Dickins
On Sun, 27 Jan 2013, Shaohua Li wrote:
> On Sat, Jan 26, 2013 at 06:16:05PM -0800, Hugh Dickins wrote:
> > On Fri, 25 Jan 2013, Shaohua Li wrote:
> > > On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
> > > 
> > > Subject: give-each-swapper-space-separate-backing_dev_info
> > > 
> > > The backing_dev_info can't be shared by all swapper address space.
> > 
> > Whyever not?  It's perfectly normal for different inodes/address_spaces
> > to share a single backing_dev!  Sasha's trace says that it's wrong to
> > initialize it MAX_SWAPFILES times: fair enough.  But why should I now
> > want to spend 32kB (not even counting their __percpu counters) on all
> > these pseudo-backing_devs?
> 
> That's correct, silly me. Updated it.

Looks much more to my taste, thank you!

> > 
> > p.s. a grand little change would be to move page_cluster and swap_setup()
> > from mm/swap.c to mm/swap_state.c: they have nothing to do with the other
> > contents of swap.c, and everything to do with the contents of swap_state.c.
> > Why swap.c is called swap.c is rather a mystery.
> 
> Tried, but looks page_cluster is used in sysctl, moving to swap_state.c will
> make it optional. don't want to add another #ifdef, so give up.

Good point, thanks for trying, maybe I'll attack it next time it
irritates me.

I don't yet know whether I approve of your changes or not, but running
with them to see (and I'll send another bugfix separately in a moment).

I was the one who removed the swap_device_lock() which 2.4 used,
because it almost always ended up having to take both swap_list_lock()
and swap_device_lock(si).  You seem to have done a much better job of
separating them usefully, but I need to convince myself that it does
end up safely.

My reservations so far would be: how many installations actually have
more than one swap area, so is it a good tradeoff to add more overhead
to help those at the (slight) expense of everyone else?  The increasingly
ugly page_mapping() worries me, and the static array of swapper_spaces
annoys me a little.

I'm glad Minchan has now pointed you to Rik's posting of two years ago:
I think there are more important changes to be made in that direction.

Hugh

> 
> 
> Subject: init-swap-space-backing-dev-info-once
> 
> 
> Sasha reported:
> Commit "swap: make each swap partition have one address_space" is triggering
> a series of warnings on boot:
> 
> [3.446071] [ cut here ]
> [3.446664] WARNING: at lib/debugobjects.c:261 
> debug_print_object+0x8e/0xb0()
> [3.447715] ODEBUG: init active (active state 0) object type: 
> percpu_counter hint:   (null)
> [3.450360] Modules linked in:
> [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
> 3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
> [3.454508] Call Trace:
> [3.455248]  [] warn_slowpath_common+0x8c/0xc0
> [3.455248]  [] warn_slowpath_fmt+0x41/0x50
> [3.455248]  [] debug_print_object+0x8e/0xb0
> [3.455248]  [] __debug_object_init+0x20b/0x290
> [3.455248]  [] debug_object_init+0x15/0x20
> [3.455248]  [] __percpu_counter_init+0x6d/0xe0
> [3.455248]  [] bdi_init+0x1ac/0x270
> [3.455248]  [] swap_setup+0x3b/0x87
> [3.455248]  [] ? swap_setup+0x87/0x87
> [3.455248]  [] kswapd_init+0x11/0x7c
> [3.455248]  [] do_one_initcall+0x8a/0x180
> [3.455248]  [] do_basic_setup+0x96/0xb4
> [3.455248]  [] ? loglevel+0x31/0x31
> [3.455248]  [] ? sched_init_smp+0x150/0x157
> [3.455248]  [] kernel_init_freeable+0xd2/0x14c
> [3.455248]  [] ? rest_init+0x140/0x140
> [3.455248]  [] kernel_init+0x9/0xf0
> [3.455248]  [] ret_from_fork+0x7c/0xb0
> [3.455248]  [] ? rest_init+0x140/0x140
> [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
> 
> Initialize swap space backing_dev_info once to avoid the warning.
> 
> Reported-by: Sasha Levin 
> Signed-off-by: Shaohua Li 
> ---
>  mm/swap.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/mm/swap.c
> ===
> --- linux.orig/mm/swap.c  2013-01-27 21:26:21.942696713 +0800
> +++ linux/mm/swap.c   2013-01-27 21:27:29.233865394 +0800
> @@ -858,8 +858,8 @@ void __init swap_setup(void)
>  #ifdef CONFIG_SWAP
>   int i;
>  
> + bdi_init(swapper_spaces[0].backing_dev_info);
>   for (i = 0; i < MAX_SWAPFILES; i++) {
> - bdi_init(swapper_spaces[i].backing_dev_info);
>   spin_lock_init(_spaces[i].tree_lock);
>   INIT_LIST_HEAD(_spaces[i].i_mmap_nonlinear);
>   }
> 
--
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: boot warnings due to swap: make each swap partition have one address_space

2013-01-27 Thread Shaohua Li
On Sat, Jan 26, 2013 at 06:16:05PM -0800, Hugh Dickins wrote:
> On Fri, 25 Jan 2013, Shaohua Li wrote:
> > On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
> > > Hi folks,
> > > 
> > > Commit "swap: make each swap partition have one address_space" is 
> > > triggering
> > > a series of warnings on boot:
> > > 
> > > [3.446071] [ cut here ]
> > > [3.446664] WARNING: at lib/debugobjects.c:261 
> > > debug_print_object+0x8e/0xb0()
> > > [3.447715] ODEBUG: init active (active state 0) object type: 
> > > percpu_counter hint:   (null)
> > > [3.450360] Modules linked in:
> > > [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
> > > 3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
> > > [3.454508] Call Trace:
> > > [3.455248]  [] warn_slowpath_common+0x8c/0xc0
> > > [3.455248]  [] warn_slowpath_fmt+0x41/0x50
> > > [3.455248]  [] debug_print_object+0x8e/0xb0
> > > [3.455248]  [] __debug_object_init+0x20b/0x290
> > > [3.455248]  [] debug_object_init+0x15/0x20
> > > [3.455248]  [] __percpu_counter_init+0x6d/0xe0
> > > [3.455248]  [] bdi_init+0x1ac/0x270
> > > [3.455248]  [] swap_setup+0x3b/0x87
> > > [3.455248]  [] ? swap_setup+0x87/0x87
> > > [3.455248]  [] kswapd_init+0x11/0x7c
> > > [3.455248]  [] do_one_initcall+0x8a/0x180
> > > [3.455248]  [] do_basic_setup+0x96/0xb4
> > > [3.455248]  [] ? loglevel+0x31/0x31
> > > [3.455248]  [] ? sched_init_smp+0x150/0x157
> > > [3.455248]  [] kernel_init_freeable+0xd2/0x14c
> > > [3.455248]  [] ? rest_init+0x140/0x140
> > > [3.455248]  [] kernel_init+0x9/0xf0
> > > [3.455248]  [] ret_from_fork+0x7c/0xb0
> > > [3.455248]  [] ? rest_init+0x140/0x140
> > > [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
> > > 
> > > I haven't looked deeper into it yet, and will do so tomorrow, unless this
> > > spew is obvious to anyone.
> > 
> > Does this one help?
> > 
> > Subject: give-each-swapper-space-separate-backing_dev_info
> > 
> > The backing_dev_info can't be shared by all swapper address space.
> 
> Whyever not?  It's perfectly normal for different inodes/address_spaces
> to share a single backing_dev!  Sasha's trace says that it's wrong to
> initialize it MAX_SWAPFILES times: fair enough.  But why should I now
> want to spend 32kB (not even counting their __percpu counters) on all
> these pseudo-backing_devs?

That's correct, silly me. Updated it.
> 
> p.s. a grand little change would be to move page_cluster and swap_setup()
> from mm/swap.c to mm/swap_state.c: they have nothing to do with the other
> contents of swap.c, and everything to do with the contents of swap_state.c.
> Why swap.c is called swap.c is rather a mystery.

Tried, but looks page_cluster is used in sysctl, moving to swap_state.c will
make it optional. don't want to add another #ifdef, so give up.


Subject: init-swap-space-backing-dev-info-once


Sasha reported:
Commit "swap: make each swap partition have one address_space" is triggering
a series of warnings on boot:

[3.446071] [ cut here ]
[3.446664] WARNING: at lib/debugobjects.c:261 debug_print_object+0x8e/0xb0()
[3.447715] ODEBUG: init active (active state 0) object type: percpu_counter 
hint:   (null)
[3.450360] Modules linked in:
[3.451593] Pid: 1, comm: swapper/0 Tainted: GW
3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
[3.454508] Call Trace:
[3.455248]  [] warn_slowpath_common+0x8c/0xc0
[3.455248]  [] warn_slowpath_fmt+0x41/0x50
[3.455248]  [] debug_print_object+0x8e/0xb0
[3.455248]  [] __debug_object_init+0x20b/0x290
[3.455248]  [] debug_object_init+0x15/0x20
[3.455248]  [] __percpu_counter_init+0x6d/0xe0
[3.455248]  [] bdi_init+0x1ac/0x270
[3.455248]  [] swap_setup+0x3b/0x87
[3.455248]  [] ? swap_setup+0x87/0x87
[3.455248]  [] kswapd_init+0x11/0x7c
[3.455248]  [] do_one_initcall+0x8a/0x180
[3.455248]  [] do_basic_setup+0x96/0xb4
[3.455248]  [] ? loglevel+0x31/0x31
[3.455248]  [] ? sched_init_smp+0x150/0x157
[3.455248]  [] kernel_init_freeable+0xd2/0x14c
[3.455248]  [] ? rest_init+0x140/0x140
[3.455248]  [] kernel_init+0x9/0xf0
[3.455248]  [] ret_from_fork+0x7c/0xb0
[3.455248]  [] ? rest_init+0x140/0x140
[3.455248] ---[ end trace 0b176d5c0f21bffb ]---

Initialize swap space backing_dev_info once to avoid the warning.

Reported-by: Sasha Levin 
Signed-off-by: Shaohua Li 
---
 mm/swap.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/mm/swap.c
===
--- linux.orig/mm/swap.c2013-01-27 21:26:21.942696713 +0800
+++ linux/mm/swap.c 2013-01-27 21:27:29.233865394 +0800
@@ -858,8 +858,8 @@ void __init swap_setup(void)
 #ifdef CONFIG_SWAP
int i;
 
+   bdi_init(swapper_spaces[0].backing_dev_info);
for (i = 0; i < MAX_SWAPFILES; i++) {
- 

Re: boot warnings due to swap: make each swap partition have one address_space

2013-01-27 Thread Shaohua Li
On Sat, Jan 26, 2013 at 06:16:05PM -0800, Hugh Dickins wrote:
 On Fri, 25 Jan 2013, Shaohua Li wrote:
  On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
   Hi folks,
   
   Commit swap: make each swap partition have one address_space is 
   triggering
   a series of warnings on boot:
   
   [3.446071] [ cut here ]
   [3.446664] WARNING: at lib/debugobjects.c:261 
   debug_print_object+0x8e/0xb0()
   [3.447715] ODEBUG: init active (active state 0) object type: 
   percpu_counter hint:   (null)
   [3.450360] Modules linked in:
   [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
   3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
   [3.454508] Call Trace:
   [3.455248]  [8110d1bc] warn_slowpath_common+0x8c/0xc0
   [3.455248]  [8110d291] warn_slowpath_fmt+0x41/0x50
   [3.455248]  [81a2bb5e] debug_print_object+0x8e/0xb0
   [3.455248]  [81a2c26b] __debug_object_init+0x20b/0x290
   [3.455248]  [81a2c305] debug_object_init+0x15/0x20
   [3.455248]  [81a3fbed] __percpu_counter_init+0x6d/0xe0
   [3.455248]  [81231bdc] bdi_init+0x1ac/0x270
   [3.455248]  [8618f20b] swap_setup+0x3b/0x87
   [3.455248]  [8618f257] ? swap_setup+0x87/0x87
   [3.455248]  [8618f268] kswapd_init+0x11/0x7c
   [3.455248]  [810020ca] do_one_initcall+0x8a/0x180
   [3.455248]  [86168cfd] do_basic_setup+0x96/0xb4
   [3.455248]  [861685ae] ? loglevel+0x31/0x31
   [3.455248]  [861885cd] ? sched_init_smp+0x150/0x157
   [3.455248]  [86168ded] kernel_init_freeable+0xd2/0x14c
   [3.455248]  [83cade10] ? rest_init+0x140/0x140
   [3.455248]  [83cade19] kernel_init+0x9/0xf0
   [3.455248]  [83d5727c] ret_from_fork+0x7c/0xb0
   [3.455248]  [83cade10] ? rest_init+0x140/0x140
   [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
   
   I haven't looked deeper into it yet, and will do so tomorrow, unless this
   spew is obvious to anyone.
  
  Does this one help?
  
  Subject: give-each-swapper-space-separate-backing_dev_info
  
  The backing_dev_info can't be shared by all swapper address space.
 
 Whyever not?  It's perfectly normal for different inodes/address_spaces
 to share a single backing_dev!  Sasha's trace says that it's wrong to
 initialize it MAX_SWAPFILES times: fair enough.  But why should I now
 want to spend 32kB (not even counting their __percpu counters) on all
 these pseudo-backing_devs?

That's correct, silly me. Updated it.
 
 p.s. a grand little change would be to move page_cluster and swap_setup()
 from mm/swap.c to mm/swap_state.c: they have nothing to do with the other
 contents of swap.c, and everything to do with the contents of swap_state.c.
 Why swap.c is called swap.c is rather a mystery.

Tried, but looks page_cluster is used in sysctl, moving to swap_state.c will
make it optional. don't want to add another #ifdef, so give up.


Subject: init-swap-space-backing-dev-info-once


Sasha reported:
Commit swap: make each swap partition have one address_space is triggering
a series of warnings on boot:

[3.446071] [ cut here ]
[3.446664] WARNING: at lib/debugobjects.c:261 debug_print_object+0x8e/0xb0()
[3.447715] ODEBUG: init active (active state 0) object type: percpu_counter 
hint:   (null)
[3.450360] Modules linked in:
[3.451593] Pid: 1, comm: swapper/0 Tainted: GW
3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
[3.454508] Call Trace:
[3.455248]  [8110d1bc] warn_slowpath_common+0x8c/0xc0
[3.455248]  [8110d291] warn_slowpath_fmt+0x41/0x50
[3.455248]  [81a2bb5e] debug_print_object+0x8e/0xb0
[3.455248]  [81a2c26b] __debug_object_init+0x20b/0x290
[3.455248]  [81a2c305] debug_object_init+0x15/0x20
[3.455248]  [81a3fbed] __percpu_counter_init+0x6d/0xe0
[3.455248]  [81231bdc] bdi_init+0x1ac/0x270
[3.455248]  [8618f20b] swap_setup+0x3b/0x87
[3.455248]  [8618f257] ? swap_setup+0x87/0x87
[3.455248]  [8618f268] kswapd_init+0x11/0x7c
[3.455248]  [810020ca] do_one_initcall+0x8a/0x180
[3.455248]  [86168cfd] do_basic_setup+0x96/0xb4
[3.455248]  [861685ae] ? loglevel+0x31/0x31
[3.455248]  [861885cd] ? sched_init_smp+0x150/0x157
[3.455248]  [86168ded] kernel_init_freeable+0xd2/0x14c
[3.455248]  [83cade10] ? rest_init+0x140/0x140
[3.455248]  [83cade19] kernel_init+0x9/0xf0
[3.455248]  [83d5727c] ret_from_fork+0x7c/0xb0
[3.455248]  [83cade10] ? rest_init+0x140/0x140
[3.455248] ---[ end trace 0b176d5c0f21bffb ]---

Initialize swap space backing_dev_info once to avoid the warning.

Reported-by: Sasha Levin sasha.le...@oracle.com
Signed-off-by: Shaohua Li 

Re: boot warnings due to swap: make each swap partition have one address_space

2013-01-27 Thread Hugh Dickins
On Sun, 27 Jan 2013, Shaohua Li wrote:
 On Sat, Jan 26, 2013 at 06:16:05PM -0800, Hugh Dickins wrote:
  On Fri, 25 Jan 2013, Shaohua Li wrote:
   On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
   
   Subject: give-each-swapper-space-separate-backing_dev_info
   
   The backing_dev_info can't be shared by all swapper address space.
  
  Whyever not?  It's perfectly normal for different inodes/address_spaces
  to share a single backing_dev!  Sasha's trace says that it's wrong to
  initialize it MAX_SWAPFILES times: fair enough.  But why should I now
  want to spend 32kB (not even counting their __percpu counters) on all
  these pseudo-backing_devs?
 
 That's correct, silly me. Updated it.

Looks much more to my taste, thank you!

  
  p.s. a grand little change would be to move page_cluster and swap_setup()
  from mm/swap.c to mm/swap_state.c: they have nothing to do with the other
  contents of swap.c, and everything to do with the contents of swap_state.c.
  Why swap.c is called swap.c is rather a mystery.
 
 Tried, but looks page_cluster is used in sysctl, moving to swap_state.c will
 make it optional. don't want to add another #ifdef, so give up.

Good point, thanks for trying, maybe I'll attack it next time it
irritates me.

I don't yet know whether I approve of your changes or not, but running
with them to see (and I'll send another bugfix separately in a moment).

I was the one who removed the swap_device_lock() which 2.4 used,
because it almost always ended up having to take both swap_list_lock()
and swap_device_lock(si).  You seem to have done a much better job of
separating them usefully, but I need to convince myself that it does
end up safely.

My reservations so far would be: how many installations actually have
more than one swap area, so is it a good tradeoff to add more overhead
to help those at the (slight) expense of everyone else?  The increasingly
ugly page_mapping() worries me, and the static array of swapper_spaces
annoys me a little.

I'm glad Minchan has now pointed you to Rik's posting of two years ago:
I think there are more important changes to be made in that direction.

Hugh

 
 
 Subject: init-swap-space-backing-dev-info-once
 
 
 Sasha reported:
 Commit swap: make each swap partition have one address_space is triggering
 a series of warnings on boot:
 
 [3.446071] [ cut here ]
 [3.446664] WARNING: at lib/debugobjects.c:261 
 debug_print_object+0x8e/0xb0()
 [3.447715] ODEBUG: init active (active state 0) object type: 
 percpu_counter hint:   (null)
 [3.450360] Modules linked in:
 [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
 3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
 [3.454508] Call Trace:
 [3.455248]  [8110d1bc] warn_slowpath_common+0x8c/0xc0
 [3.455248]  [8110d291] warn_slowpath_fmt+0x41/0x50
 [3.455248]  [81a2bb5e] debug_print_object+0x8e/0xb0
 [3.455248]  [81a2c26b] __debug_object_init+0x20b/0x290
 [3.455248]  [81a2c305] debug_object_init+0x15/0x20
 [3.455248]  [81a3fbed] __percpu_counter_init+0x6d/0xe0
 [3.455248]  [81231bdc] bdi_init+0x1ac/0x270
 [3.455248]  [8618f20b] swap_setup+0x3b/0x87
 [3.455248]  [8618f257] ? swap_setup+0x87/0x87
 [3.455248]  [8618f268] kswapd_init+0x11/0x7c
 [3.455248]  [810020ca] do_one_initcall+0x8a/0x180
 [3.455248]  [86168cfd] do_basic_setup+0x96/0xb4
 [3.455248]  [861685ae] ? loglevel+0x31/0x31
 [3.455248]  [861885cd] ? sched_init_smp+0x150/0x157
 [3.455248]  [86168ded] kernel_init_freeable+0xd2/0x14c
 [3.455248]  [83cade10] ? rest_init+0x140/0x140
 [3.455248]  [83cade19] kernel_init+0x9/0xf0
 [3.455248]  [83d5727c] ret_from_fork+0x7c/0xb0
 [3.455248]  [83cade10] ? rest_init+0x140/0x140
 [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
 
 Initialize swap space backing_dev_info once to avoid the warning.
 
 Reported-by: Sasha Levin sasha.le...@oracle.com
 Signed-off-by: Shaohua Li s...@fusionio.com
 ---
  mm/swap.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 Index: linux/mm/swap.c
 ===
 --- linux.orig/mm/swap.c  2013-01-27 21:26:21.942696713 +0800
 +++ linux/mm/swap.c   2013-01-27 21:27:29.233865394 +0800
 @@ -858,8 +858,8 @@ void __init swap_setup(void)
  #ifdef CONFIG_SWAP
   int i;
  
 + bdi_init(swapper_spaces[0].backing_dev_info);
   for (i = 0; i  MAX_SWAPFILES; i++) {
 - bdi_init(swapper_spaces[i].backing_dev_info);
   spin_lock_init(swapper_spaces[i].tree_lock);
   INIT_LIST_HEAD(swapper_spaces[i].i_mmap_nonlinear);
   }
 
--
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  

Re: boot warnings due to swap: make each swap partition have one address_space

2013-01-26 Thread Hugh Dickins
On Fri, 25 Jan 2013, Shaohua Li wrote:
> On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
> > Hi folks,
> > 
> > Commit "swap: make each swap partition have one address_space" is triggering
> > a series of warnings on boot:
> > 
> > [3.446071] [ cut here ]
> > [3.446664] WARNING: at lib/debugobjects.c:261 
> > debug_print_object+0x8e/0xb0()
> > [3.447715] ODEBUG: init active (active state 0) object type: 
> > percpu_counter hint:   (null)
> > [3.450360] Modules linked in:
> > [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
> > 3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
> > [3.454508] Call Trace:
> > [3.455248]  [] warn_slowpath_common+0x8c/0xc0
> > [3.455248]  [] warn_slowpath_fmt+0x41/0x50
> > [3.455248]  [] debug_print_object+0x8e/0xb0
> > [3.455248]  [] __debug_object_init+0x20b/0x290
> > [3.455248]  [] debug_object_init+0x15/0x20
> > [3.455248]  [] __percpu_counter_init+0x6d/0xe0
> > [3.455248]  [] bdi_init+0x1ac/0x270
> > [3.455248]  [] swap_setup+0x3b/0x87
> > [3.455248]  [] ? swap_setup+0x87/0x87
> > [3.455248]  [] kswapd_init+0x11/0x7c
> > [3.455248]  [] do_one_initcall+0x8a/0x180
> > [3.455248]  [] do_basic_setup+0x96/0xb4
> > [3.455248]  [] ? loglevel+0x31/0x31
> > [3.455248]  [] ? sched_init_smp+0x150/0x157
> > [3.455248]  [] kernel_init_freeable+0xd2/0x14c
> > [3.455248]  [] ? rest_init+0x140/0x140
> > [3.455248]  [] kernel_init+0x9/0xf0
> > [3.455248]  [] ret_from_fork+0x7c/0xb0
> > [3.455248]  [] ? rest_init+0x140/0x140
> > [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
> > 
> > I haven't looked deeper into it yet, and will do so tomorrow, unless this
> > spew is obvious to anyone.
> 
> Does this one help?
> 
> Subject: give-each-swapper-space-separate-backing_dev_info
> 
> The backing_dev_info can't be shared by all swapper address space.

Whyever not?  It's perfectly normal for different inodes/address_spaces
to share a single backing_dev!  Sasha's trace says that it's wrong to
initialize it MAX_SWAPFILES times: fair enough.  But why should I now
want to spend 32kB (not even counting their __percpu counters) on all
these pseudo-backing_devs?

Hugh

p.s. a grand little change would be to move page_cluster and swap_setup()
from mm/swap.c to mm/swap_state.c: they have nothing to do with the other
contents of swap.c, and everything to do with the contents of swap_state.c.
Why swap.c is called swap.c is rather a mystery.

> 
> Reported-by: Sasha Levin 
> Signed-off-by: Shaohua Li 
> ---
>  mm/swap.c   |1 +
>  mm/swap_state.c |   11 +++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> Index: linux/mm/swap.c
> ===
> --- linux.orig/mm/swap.c  2013-01-22 10:11:58.310933234 +0800
> +++ linux/mm/swap.c   2013-01-25 12:14:49.524863610 +0800
> @@ -859,6 +859,7 @@ void __init swap_setup(void)
>   int i;
>  
>   for (i = 0; i < MAX_SWAPFILES; i++) {
> + swapper_spaces[i].backing_dev_info += i;
>   bdi_init(swapper_spaces[i].backing_dev_info);
>   spin_lock_init(_spaces[i].tree_lock);
>   INIT_LIST_HEAD(_spaces[i].i_mmap_nonlinear);
> Index: linux/mm/swap_state.c
> ===
> --- linux.orig/mm/swap_state.c2013-01-24 18:08:05.149390977 +0800
> +++ linux/mm/swap_state.c 2013-01-25 12:14:12.849323671 +0800
> @@ -31,16 +31,19 @@ static const struct address_space_operat
>   .migratepage= migrate_page,
>  };
>  
> -static struct backing_dev_info swap_backing_dev_info = {
> - .name   = "swap",
> - .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK | BDI_CAP_SWAP_BACKED,
> +static struct backing_dev_info swap_backing_dev_info[MAX_SWAPFILES] = {
> + [0 ... MAX_SWAPFILES - 1] = {
> + .name   = "swap",
> + .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK |
> + BDI_CAP_SWAP_BACKED,
> + }
>  };
>  
>  struct address_space swapper_spaces[MAX_SWAPFILES] = {
>   [0 ... MAX_SWAPFILES - 1] = {
>   .page_tree  = RADIX_TREE_INIT(GFP_ATOMIC|__GFP_NOWARN),
>   .a_ops  = _aops,
> - .backing_dev_info = _backing_dev_info,
> + .backing_dev_info = _backing_dev_info[0],
>   }
>  };
>  
> 
--
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: boot warnings due to swap: make each swap partition have one address_space

2013-01-26 Thread Hugh Dickins
On Fri, 25 Jan 2013, Shaohua Li wrote:
 On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
  Hi folks,
  
  Commit swap: make each swap partition have one address_space is triggering
  a series of warnings on boot:
  
  [3.446071] [ cut here ]
  [3.446664] WARNING: at lib/debugobjects.c:261 
  debug_print_object+0x8e/0xb0()
  [3.447715] ODEBUG: init active (active state 0) object type: 
  percpu_counter hint:   (null)
  [3.450360] Modules linked in:
  [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
  3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
  [3.454508] Call Trace:
  [3.455248]  [8110d1bc] warn_slowpath_common+0x8c/0xc0
  [3.455248]  [8110d291] warn_slowpath_fmt+0x41/0x50
  [3.455248]  [81a2bb5e] debug_print_object+0x8e/0xb0
  [3.455248]  [81a2c26b] __debug_object_init+0x20b/0x290
  [3.455248]  [81a2c305] debug_object_init+0x15/0x20
  [3.455248]  [81a3fbed] __percpu_counter_init+0x6d/0xe0
  [3.455248]  [81231bdc] bdi_init+0x1ac/0x270
  [3.455248]  [8618f20b] swap_setup+0x3b/0x87
  [3.455248]  [8618f257] ? swap_setup+0x87/0x87
  [3.455248]  [8618f268] kswapd_init+0x11/0x7c
  [3.455248]  [810020ca] do_one_initcall+0x8a/0x180
  [3.455248]  [86168cfd] do_basic_setup+0x96/0xb4
  [3.455248]  [861685ae] ? loglevel+0x31/0x31
  [3.455248]  [861885cd] ? sched_init_smp+0x150/0x157
  [3.455248]  [86168ded] kernel_init_freeable+0xd2/0x14c
  [3.455248]  [83cade10] ? rest_init+0x140/0x140
  [3.455248]  [83cade19] kernel_init+0x9/0xf0
  [3.455248]  [83d5727c] ret_from_fork+0x7c/0xb0
  [3.455248]  [83cade10] ? rest_init+0x140/0x140
  [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
  
  I haven't looked deeper into it yet, and will do so tomorrow, unless this
  spew is obvious to anyone.
 
 Does this one help?
 
 Subject: give-each-swapper-space-separate-backing_dev_info
 
 The backing_dev_info can't be shared by all swapper address space.

Whyever not?  It's perfectly normal for different inodes/address_spaces
to share a single backing_dev!  Sasha's trace says that it's wrong to
initialize it MAX_SWAPFILES times: fair enough.  But why should I now
want to spend 32kB (not even counting their __percpu counters) on all
these pseudo-backing_devs?

Hugh

p.s. a grand little change would be to move page_cluster and swap_setup()
from mm/swap.c to mm/swap_state.c: they have nothing to do with the other
contents of swap.c, and everything to do with the contents of swap_state.c.
Why swap.c is called swap.c is rather a mystery.

 
 Reported-by: Sasha Levin sasha.le...@oracle.com
 Signed-off-by: Shaohua Li s...@fusionio.com
 ---
  mm/swap.c   |1 +
  mm/swap_state.c |   11 +++
  2 files changed, 8 insertions(+), 4 deletions(-)
 
 Index: linux/mm/swap.c
 ===
 --- linux.orig/mm/swap.c  2013-01-22 10:11:58.310933234 +0800
 +++ linux/mm/swap.c   2013-01-25 12:14:49.524863610 +0800
 @@ -859,6 +859,7 @@ void __init swap_setup(void)
   int i;
  
   for (i = 0; i  MAX_SWAPFILES; i++) {
 + swapper_spaces[i].backing_dev_info += i;
   bdi_init(swapper_spaces[i].backing_dev_info);
   spin_lock_init(swapper_spaces[i].tree_lock);
   INIT_LIST_HEAD(swapper_spaces[i].i_mmap_nonlinear);
 Index: linux/mm/swap_state.c
 ===
 --- linux.orig/mm/swap_state.c2013-01-24 18:08:05.149390977 +0800
 +++ linux/mm/swap_state.c 2013-01-25 12:14:12.849323671 +0800
 @@ -31,16 +31,19 @@ static const struct address_space_operat
   .migratepage= migrate_page,
  };
  
 -static struct backing_dev_info swap_backing_dev_info = {
 - .name   = swap,
 - .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK | BDI_CAP_SWAP_BACKED,
 +static struct backing_dev_info swap_backing_dev_info[MAX_SWAPFILES] = {
 + [0 ... MAX_SWAPFILES - 1] = {
 + .name   = swap,
 + .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK |
 + BDI_CAP_SWAP_BACKED,
 + }
  };
  
  struct address_space swapper_spaces[MAX_SWAPFILES] = {
   [0 ... MAX_SWAPFILES - 1] = {
   .page_tree  = RADIX_TREE_INIT(GFP_ATOMIC|__GFP_NOWARN),
   .a_ops  = swap_aops,
 - .backing_dev_info = swap_backing_dev_info,
 + .backing_dev_info = swap_backing_dev_info[0],
   }
  };
  
 
--
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: boot warnings due to swap: make each swap partition have one address_space

2013-01-25 Thread Sasha Levin
On 01/24/2013 11:25 PM, Shaohua Li wrote:
> On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
>> Hi folks,
>>
>> Commit "swap: make each swap partition have one address_space" is triggering
>> a series of warnings on boot:
>>
>> [3.446071] [ cut here ]
>> [3.446664] WARNING: at lib/debugobjects.c:261 
>> debug_print_object+0x8e/0xb0()
>> [3.447715] ODEBUG: init active (active state 0) object type: 
>> percpu_counter hint:   (null)
>> [3.450360] Modules linked in:
>> [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
>> 3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
>> [3.454508] Call Trace:
>> [3.455248]  [] warn_slowpath_common+0x8c/0xc0
>> [3.455248]  [] warn_slowpath_fmt+0x41/0x50
>> [3.455248]  [] debug_print_object+0x8e/0xb0
>> [3.455248]  [] __debug_object_init+0x20b/0x290
>> [3.455248]  [] debug_object_init+0x15/0x20
>> [3.455248]  [] __percpu_counter_init+0x6d/0xe0
>> [3.455248]  [] bdi_init+0x1ac/0x270
>> [3.455248]  [] swap_setup+0x3b/0x87
>> [3.455248]  [] ? swap_setup+0x87/0x87
>> [3.455248]  [] kswapd_init+0x11/0x7c
>> [3.455248]  [] do_one_initcall+0x8a/0x180
>> [3.455248]  [] do_basic_setup+0x96/0xb4
>> [3.455248]  [] ? loglevel+0x31/0x31
>> [3.455248]  [] ? sched_init_smp+0x150/0x157
>> [3.455248]  [] kernel_init_freeable+0xd2/0x14c
>> [3.455248]  [] ? rest_init+0x140/0x140
>> [3.455248]  [] kernel_init+0x9/0xf0
>> [3.455248]  [] ret_from_fork+0x7c/0xb0
>> [3.455248]  [] ? rest_init+0x140/0x140
>> [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
>>
>> I haven't looked deeper into it yet, and will do so tomorrow, unless this
>> spew is obvious to anyone.
> 
> Does this one help?

[snip]

Yup, it did. Thanks!


Thanks,
Sasha

--
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: boot warnings due to swap: make each swap partition have one address_space

2013-01-25 Thread Sasha Levin
On 01/24/2013 11:25 PM, Shaohua Li wrote:
 On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
 Hi folks,

 Commit swap: make each swap partition have one address_space is triggering
 a series of warnings on boot:

 [3.446071] [ cut here ]
 [3.446664] WARNING: at lib/debugobjects.c:261 
 debug_print_object+0x8e/0xb0()
 [3.447715] ODEBUG: init active (active state 0) object type: 
 percpu_counter hint:   (null)
 [3.450360] Modules linked in:
 [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
 3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
 [3.454508] Call Trace:
 [3.455248]  [8110d1bc] warn_slowpath_common+0x8c/0xc0
 [3.455248]  [8110d291] warn_slowpath_fmt+0x41/0x50
 [3.455248]  [81a2bb5e] debug_print_object+0x8e/0xb0
 [3.455248]  [81a2c26b] __debug_object_init+0x20b/0x290
 [3.455248]  [81a2c305] debug_object_init+0x15/0x20
 [3.455248]  [81a3fbed] __percpu_counter_init+0x6d/0xe0
 [3.455248]  [81231bdc] bdi_init+0x1ac/0x270
 [3.455248]  [8618f20b] swap_setup+0x3b/0x87
 [3.455248]  [8618f257] ? swap_setup+0x87/0x87
 [3.455248]  [8618f268] kswapd_init+0x11/0x7c
 [3.455248]  [810020ca] do_one_initcall+0x8a/0x180
 [3.455248]  [86168cfd] do_basic_setup+0x96/0xb4
 [3.455248]  [861685ae] ? loglevel+0x31/0x31
 [3.455248]  [861885cd] ? sched_init_smp+0x150/0x157
 [3.455248]  [86168ded] kernel_init_freeable+0xd2/0x14c
 [3.455248]  [83cade10] ? rest_init+0x140/0x140
 [3.455248]  [83cade19] kernel_init+0x9/0xf0
 [3.455248]  [83d5727c] ret_from_fork+0x7c/0xb0
 [3.455248]  [83cade10] ? rest_init+0x140/0x140
 [3.455248] ---[ end trace 0b176d5c0f21bffb ]---

 I haven't looked deeper into it yet, and will do so tomorrow, unless this
 spew is obvious to anyone.
 
 Does this one help?

[snip]

Yup, it did. Thanks!


Thanks,
Sasha

--
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: boot warnings due to swap: make each swap partition have one address_space

2013-01-24 Thread Shaohua Li
On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
> Hi folks,
> 
> Commit "swap: make each swap partition have one address_space" is triggering
> a series of warnings on boot:
> 
> [3.446071] [ cut here ]
> [3.446664] WARNING: at lib/debugobjects.c:261 
> debug_print_object+0x8e/0xb0()
> [3.447715] ODEBUG: init active (active state 0) object type: 
> percpu_counter hint:   (null)
> [3.450360] Modules linked in:
> [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
> 3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
> [3.454508] Call Trace:
> [3.455248]  [] warn_slowpath_common+0x8c/0xc0
> [3.455248]  [] warn_slowpath_fmt+0x41/0x50
> [3.455248]  [] debug_print_object+0x8e/0xb0
> [3.455248]  [] __debug_object_init+0x20b/0x290
> [3.455248]  [] debug_object_init+0x15/0x20
> [3.455248]  [] __percpu_counter_init+0x6d/0xe0
> [3.455248]  [] bdi_init+0x1ac/0x270
> [3.455248]  [] swap_setup+0x3b/0x87
> [3.455248]  [] ? swap_setup+0x87/0x87
> [3.455248]  [] kswapd_init+0x11/0x7c
> [3.455248]  [] do_one_initcall+0x8a/0x180
> [3.455248]  [] do_basic_setup+0x96/0xb4
> [3.455248]  [] ? loglevel+0x31/0x31
> [3.455248]  [] ? sched_init_smp+0x150/0x157
> [3.455248]  [] kernel_init_freeable+0xd2/0x14c
> [3.455248]  [] ? rest_init+0x140/0x140
> [3.455248]  [] kernel_init+0x9/0xf0
> [3.455248]  [] ret_from_fork+0x7c/0xb0
> [3.455248]  [] ? rest_init+0x140/0x140
> [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
> 
> I haven't looked deeper into it yet, and will do so tomorrow, unless this
> spew is obvious to anyone.

Does this one help?

Subject: give-each-swapper-space-separate-backing_dev_info

The backing_dev_info can't be shared by all swapper address space.

Reported-by: Sasha Levin 
Signed-off-by: Shaohua Li 
---
 mm/swap.c   |1 +
 mm/swap_state.c |   11 +++
 2 files changed, 8 insertions(+), 4 deletions(-)

Index: linux/mm/swap.c
===
--- linux.orig/mm/swap.c2013-01-22 10:11:58.310933234 +0800
+++ linux/mm/swap.c 2013-01-25 12:14:49.524863610 +0800
@@ -859,6 +859,7 @@ void __init swap_setup(void)
int i;
 
for (i = 0; i < MAX_SWAPFILES; i++) {
+   swapper_spaces[i].backing_dev_info += i;
bdi_init(swapper_spaces[i].backing_dev_info);
spin_lock_init(_spaces[i].tree_lock);
INIT_LIST_HEAD(_spaces[i].i_mmap_nonlinear);
Index: linux/mm/swap_state.c
===
--- linux.orig/mm/swap_state.c  2013-01-24 18:08:05.149390977 +0800
+++ linux/mm/swap_state.c   2013-01-25 12:14:12.849323671 +0800
@@ -31,16 +31,19 @@ static const struct address_space_operat
.migratepage= migrate_page,
 };
 
-static struct backing_dev_info swap_backing_dev_info = {
-   .name   = "swap",
-   .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK | BDI_CAP_SWAP_BACKED,
+static struct backing_dev_info swap_backing_dev_info[MAX_SWAPFILES] = {
+   [0 ... MAX_SWAPFILES - 1] = {
+   .name   = "swap",
+   .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK |
+   BDI_CAP_SWAP_BACKED,
+   }
 };
 
 struct address_space swapper_spaces[MAX_SWAPFILES] = {
[0 ... MAX_SWAPFILES - 1] = {
.page_tree  = RADIX_TREE_INIT(GFP_ATOMIC|__GFP_NOWARN),
.a_ops  = _aops,
-   .backing_dev_info = _backing_dev_info,
+   .backing_dev_info = _backing_dev_info[0],
}
 };
 
--
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/


boot warnings due to swap: make each swap partition have one address_space

2013-01-24 Thread Sasha Levin
Hi folks,

Commit "swap: make each swap partition have one address_space" is triggering
a series of warnings on boot:

[3.446071] [ cut here ]
[3.446664] WARNING: at lib/debugobjects.c:261 debug_print_object+0x8e/0xb0()
[3.447715] ODEBUG: init active (active state 0) object type: percpu_counter 
hint:   (null)
[3.450360] Modules linked in:
[3.451593] Pid: 1, comm: swapper/0 Tainted: GW
3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
[3.454508] Call Trace:
[3.455248]  [] warn_slowpath_common+0x8c/0xc0
[3.455248]  [] warn_slowpath_fmt+0x41/0x50
[3.455248]  [] debug_print_object+0x8e/0xb0
[3.455248]  [] __debug_object_init+0x20b/0x290
[3.455248]  [] debug_object_init+0x15/0x20
[3.455248]  [] __percpu_counter_init+0x6d/0xe0
[3.455248]  [] bdi_init+0x1ac/0x270
[3.455248]  [] swap_setup+0x3b/0x87
[3.455248]  [] ? swap_setup+0x87/0x87
[3.455248]  [] kswapd_init+0x11/0x7c
[3.455248]  [] do_one_initcall+0x8a/0x180
[3.455248]  [] do_basic_setup+0x96/0xb4
[3.455248]  [] ? loglevel+0x31/0x31
[3.455248]  [] ? sched_init_smp+0x150/0x157
[3.455248]  [] kernel_init_freeable+0xd2/0x14c
[3.455248]  [] ? rest_init+0x140/0x140
[3.455248]  [] kernel_init+0x9/0xf0
[3.455248]  [] ret_from_fork+0x7c/0xb0
[3.455248]  [] ? rest_init+0x140/0x140
[3.455248] ---[ end trace 0b176d5c0f21bffb ]---

I haven't looked deeper into it yet, and will do so tomorrow, unless this
spew is obvious to anyone.


Thanks,
Sasha
--
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/


boot warnings due to swap: make each swap partition have one address_space

2013-01-24 Thread Sasha Levin
Hi folks,

Commit swap: make each swap partition have one address_space is triggering
a series of warnings on boot:

[3.446071] [ cut here ]
[3.446664] WARNING: at lib/debugobjects.c:261 debug_print_object+0x8e/0xb0()
[3.447715] ODEBUG: init active (active state 0) object type: percpu_counter 
hint:   (null)
[3.450360] Modules linked in:
[3.451593] Pid: 1, comm: swapper/0 Tainted: GW
3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
[3.454508] Call Trace:
[3.455248]  [8110d1bc] warn_slowpath_common+0x8c/0xc0
[3.455248]  [8110d291] warn_slowpath_fmt+0x41/0x50
[3.455248]  [81a2bb5e] debug_print_object+0x8e/0xb0
[3.455248]  [81a2c26b] __debug_object_init+0x20b/0x290
[3.455248]  [81a2c305] debug_object_init+0x15/0x20
[3.455248]  [81a3fbed] __percpu_counter_init+0x6d/0xe0
[3.455248]  [81231bdc] bdi_init+0x1ac/0x270
[3.455248]  [8618f20b] swap_setup+0x3b/0x87
[3.455248]  [8618f257] ? swap_setup+0x87/0x87
[3.455248]  [8618f268] kswapd_init+0x11/0x7c
[3.455248]  [810020ca] do_one_initcall+0x8a/0x180
[3.455248]  [86168cfd] do_basic_setup+0x96/0xb4
[3.455248]  [861685ae] ? loglevel+0x31/0x31
[3.455248]  [861885cd] ? sched_init_smp+0x150/0x157
[3.455248]  [86168ded] kernel_init_freeable+0xd2/0x14c
[3.455248]  [83cade10] ? rest_init+0x140/0x140
[3.455248]  [83cade19] kernel_init+0x9/0xf0
[3.455248]  [83d5727c] ret_from_fork+0x7c/0xb0
[3.455248]  [83cade10] ? rest_init+0x140/0x140
[3.455248] ---[ end trace 0b176d5c0f21bffb ]---

I haven't looked deeper into it yet, and will do so tomorrow, unless this
spew is obvious to anyone.


Thanks,
Sasha
--
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: boot warnings due to swap: make each swap partition have one address_space

2013-01-24 Thread Shaohua Li
On Thu, Jan 24, 2013 at 10:45:57PM -0500, Sasha Levin wrote:
 Hi folks,
 
 Commit swap: make each swap partition have one address_space is triggering
 a series of warnings on boot:
 
 [3.446071] [ cut here ]
 [3.446664] WARNING: at lib/debugobjects.c:261 
 debug_print_object+0x8e/0xb0()
 [3.447715] ODEBUG: init active (active state 0) object type: 
 percpu_counter hint:   (null)
 [3.450360] Modules linked in:
 [3.451593] Pid: 1, comm: swapper/0 Tainted: GW
 3.8.0-rc4-next-20130124-sasha-4-g838a1b4 #266
 [3.454508] Call Trace:
 [3.455248]  [8110d1bc] warn_slowpath_common+0x8c/0xc0
 [3.455248]  [8110d291] warn_slowpath_fmt+0x41/0x50
 [3.455248]  [81a2bb5e] debug_print_object+0x8e/0xb0
 [3.455248]  [81a2c26b] __debug_object_init+0x20b/0x290
 [3.455248]  [81a2c305] debug_object_init+0x15/0x20
 [3.455248]  [81a3fbed] __percpu_counter_init+0x6d/0xe0
 [3.455248]  [81231bdc] bdi_init+0x1ac/0x270
 [3.455248]  [8618f20b] swap_setup+0x3b/0x87
 [3.455248]  [8618f257] ? swap_setup+0x87/0x87
 [3.455248]  [8618f268] kswapd_init+0x11/0x7c
 [3.455248]  [810020ca] do_one_initcall+0x8a/0x180
 [3.455248]  [86168cfd] do_basic_setup+0x96/0xb4
 [3.455248]  [861685ae] ? loglevel+0x31/0x31
 [3.455248]  [861885cd] ? sched_init_smp+0x150/0x157
 [3.455248]  [86168ded] kernel_init_freeable+0xd2/0x14c
 [3.455248]  [83cade10] ? rest_init+0x140/0x140
 [3.455248]  [83cade19] kernel_init+0x9/0xf0
 [3.455248]  [83d5727c] ret_from_fork+0x7c/0xb0
 [3.455248]  [83cade10] ? rest_init+0x140/0x140
 [3.455248] ---[ end trace 0b176d5c0f21bffb ]---
 
 I haven't looked deeper into it yet, and will do so tomorrow, unless this
 spew is obvious to anyone.

Does this one help?

Subject: give-each-swapper-space-separate-backing_dev_info

The backing_dev_info can't be shared by all swapper address space.

Reported-by: Sasha Levin sasha.le...@oracle.com
Signed-off-by: Shaohua Li s...@fusionio.com
---
 mm/swap.c   |1 +
 mm/swap_state.c |   11 +++
 2 files changed, 8 insertions(+), 4 deletions(-)

Index: linux/mm/swap.c
===
--- linux.orig/mm/swap.c2013-01-22 10:11:58.310933234 +0800
+++ linux/mm/swap.c 2013-01-25 12:14:49.524863610 +0800
@@ -859,6 +859,7 @@ void __init swap_setup(void)
int i;
 
for (i = 0; i  MAX_SWAPFILES; i++) {
+   swapper_spaces[i].backing_dev_info += i;
bdi_init(swapper_spaces[i].backing_dev_info);
spin_lock_init(swapper_spaces[i].tree_lock);
INIT_LIST_HEAD(swapper_spaces[i].i_mmap_nonlinear);
Index: linux/mm/swap_state.c
===
--- linux.orig/mm/swap_state.c  2013-01-24 18:08:05.149390977 +0800
+++ linux/mm/swap_state.c   2013-01-25 12:14:12.849323671 +0800
@@ -31,16 +31,19 @@ static const struct address_space_operat
.migratepage= migrate_page,
 };
 
-static struct backing_dev_info swap_backing_dev_info = {
-   .name   = swap,
-   .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK | BDI_CAP_SWAP_BACKED,
+static struct backing_dev_info swap_backing_dev_info[MAX_SWAPFILES] = {
+   [0 ... MAX_SWAPFILES - 1] = {
+   .name   = swap,
+   .capabilities   = BDI_CAP_NO_ACCT_AND_WRITEBACK |
+   BDI_CAP_SWAP_BACKED,
+   }
 };
 
 struct address_space swapper_spaces[MAX_SWAPFILES] = {
[0 ... MAX_SWAPFILES - 1] = {
.page_tree  = RADIX_TREE_INIT(GFP_ATOMIC|__GFP_NOWARN),
.a_ops  = swap_aops,
-   .backing_dev_info = swap_backing_dev_info,
+   .backing_dev_info = swap_backing_dev_info[0],
}
 };
 
--
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/