Re: Fw: [PATCH] sunrpc: use better NUMA affinities

Re: Fw: [PATCH] sunrpc: use better NUMA affinities

[Greg Banks]

On 29/07/11 15:32, NeilBrown wrote:
>
> Hi Greg,
>   I saw this patch float past and thought of you... You may not be interested
>   any more, and it may be a perfectly good patch that does not need any
>   comment, but I thought I would let you know anyway.

Thanks Neil.

I've trimmed the cc list to limit the number of copies Trond and Bruce get:)

> From: Eric Dumazet<eric.dumazet@gmail.com>
> To: Trond Myklebust<Trond.Myklebust@netapp.com>
> Cc: "J. Bruce Fields"<bfields@fieldses.org>, Neil Brown<neilb@suse.de>,
> David Miller<davem@davemloft.net>, linux-nfs@vger.kernel.org, netdev
> <netdev@vger.kernel.org>, linux-kernel<linux-kernel@vger.kernel.org>
> Subject: [PATCH] sunrpc: use better NUMA affinities
>
>
> Use NUMA aware allocations to reduce latencies and increase throughput.


Briefly looking at the patch, it doesn't seem wrong but I'm surprised 
it's (still) necessary.

Some years ago at SGI we encountered that same problem; we solved it by 
delaying all the allocation of data structures associated with a thread 
so that they were performed in the thread itself, after the thread had 
been limited to run on a certain set of CPUs.  Thus the thread's normal 
allocation behaviour resulted in all of it's allocations being from 
node-local pages.  It was a pretty ugly patch, but it worked and made a 
huge difference to NFS throughput on large NUMA boxes.

Later Jeff Layton converted the sunrpc svc startup code to use kthreads 
and at the time I read his patches, pointed out this problem, and posted 
my patch for comparison

http://linux-nfs.org/pipermail/nfsv4/2008-May/008760.html

I seem to remember coming to the conclusion that Jeff eventually 
addressed this problem...am I misremembering or did something regress?

-- 
Greg.

Re: Fw: [PATCH] sunrpc: use better NUMA affinities

[Eric Dumazet]

Le vendredi 29 juillet 2011 à 16:05 +1000, Greg Banks a écrit :
> On 29/07/11 15:32, NeilBrown wrote:
> >
> > Hi Greg,
> >   I saw this patch float past and thought of you... You may not be interested
> >   any more, and it may be a perfectly good patch that does not need any
> >   comment, but I thought I would let you know anyway.
> 
> Thanks Neil.
> 
> I've trimmed the cc list to limit the number of copies Trond and Bruce get:)
> 
> > From: Eric Dumazet<eric.dumazet@gmail.com>
> > To: Trond Myklebust<Trond.Myklebust@netapp.com>
> > Cc: "J. Bruce Fields"<bfields@fieldses.org>, Neil Brown<neilb@suse.de>,
> > David Miller<davem@davemloft.net>, linux-nfs@vger.kernel.org, netdev
> > <netdev@vger.kernel.org>, linux-kernel<linux-kernel@vger.kernel.org>
> > Subject: [PATCH] sunrpc: use better NUMA affinities
> >
> >
> > Use NUMA aware allocations to reduce latencies and increase throughput.
> 
> 
> Briefly looking at the patch, it doesn't seem wrong but I'm surprised 
> it's (still) necessary.
> 
> Some years ago at SGI we encountered that same problem; we solved it by 
> delaying all the allocation of data structures associated with a thread 
> so that they were performed in the thread itself, after the thread had 
> been limited to run on a certain set of CPUs.  Thus the thread's normal 
> allocation behaviour resulted in all of it's allocations being from 
> node-local pages.  It was a pretty ugly patch, but it worked and made a 
> huge difference to NFS throughput on large NUMA boxes.
> 
> Later Jeff Layton converted the sunrpc svc startup code to use kthreads 
> and at the time I read his patches, pointed out this problem, and posted 
> my patch for comparison
> 
> http://linux-nfs.org/pipermail/nfsv4/2008-May/008760.html
> 
> I seem to remember coming to the conclusion that Jeff eventually 
> addressed this problem...am I misremembering or did something regress?
> 

Currently, all nfsd kthreads use memory for their kernel stack and
various initial data from a _single_ node, even if you use
sunrpc.pool_mode=pernode  (or percpu)

With my patch, we make sure each thread gets its stack from its local
node.

Check commit 94dcf29a11b3d20a (kthread: use kthread_create_on_node()) to
see how this strategy already was adopted for ksoftirqd, kworker,
migration, and pktgend kthreads.

I only have small machines here (two nodes), so I cannot post
significative bench results, but it seems quite obvious to expect a good
increase.

Re: Fw: [PATCH] sunrpc: use better NUMA affinities

[Greg Banks]

On 29/07/11 16:30, Eric Dumazet wrote:
> Le vendredi 29 juillet 2011 à 16:05 +1000, Greg Banks a écrit :
>> On 29/07/11 15:32, NeilBrown wrote:
>>
>> I seem to remember coming to the conclusion that Jeff eventually
>> addressed this problem...am I misremembering or did something regress?
>>
> Currently, all nfsd kthreads use memory for their kernel stack and
> various initial data from a _single_ node, even if you use
> sunrpc.pool_mode=pernode  (or percpu)

That's just plain broken and I'm very pleased to see you fix it.

I was just surprised that it was still broken and wondering how that 
happened.  Looking at ToT I see that because I dropped the ball in 2008, 
Jeff's patches didn't address the problem.  In ToT 
svc_pool_map_set_cpumask() is called *after* kthread_create() and 
applies to the child thread, *after* it's stack has been allocated on 
the wrong node.  In the working SGI code, svc_pool_map_set_cpumask() is 
called by the parent node on itself *before* calling kernel_thread() or 
doing any of the data structure allocations, thus ensuring that 
everything gets allocated using the default memory allocation policy, 
which on SGI NFS servers was globally tuned to be "node-local".

> With my patch, we make sure each thread gets its stack from its local
> node.
>
> Check commit 94dcf29a11b3d20a (kthread: use kthread_create_on_node()) to
> see how this strategy already was adopted for ksoftirqd, kworker,
> migration, and pktgend kthreads.

Ah, I see.  It's unfortunate that the kthread_create() API ends up being 
passed a CPU number but that's only used to format the name and not for 
sensible things :(

-- 
Greg.

Re: Fw: [PATCH] sunrpc: use better NUMA affinities

[Christoph Hellwig]

On Fri, Jul 29, 2011 at 04:53:21PM +1000, Greg Banks wrote:
> >Check commit 94dcf29a11b3d20a (kthread: use kthread_create_on_node()) to
> >see how this strategy already was adopted for ksoftirqd, kworker,
> >migration, and pktgend kthreads.
> 
> Ah, I see.  It's unfortunate that the kthread_create() API ends up
> being passed a CPU number but that's only used to format the name
> and not for sensible things :(

kthread_create doesn't have a cpu argument - it has a printf-like format
string.

Re: [PATCH] sunrpc: use better NUMA affinities

[Greg Banks]

Sent from my iPhone

On 29/07/2011, at 20:36, Christoph Hellwig <hch@infradead.org> wrote:

> On Fri, Jul 29, 2011 at 04:53:21PM +1000, Greg Banks wrote:
>>> Check commit 94dcf29a11b3d20a (kthread: use kthread_create_on_node 
>>> ()) to
>>> see how this strategy already was adopted for ksoftirqd, kworker,
>>> migration, and pktgend kthreads.
>>
>> Ah, I see.  It's unfortunate that the kthread_create() API ends up
>> being passed a CPU number but that's only used to format the name
>> and not for sensible things :(
>
> kthread_create doesn't have a cpu argument - it has a printf-like  
> format
> string.
>

Sure, and a whole lot of the callsites are ("..._%d", cpu), hence the  
unfortune :(

Greg.

Re: [PATCH] sunrpc: use better NUMA affinities

[Eric Dumazet]

Le vendredi 29 juillet 2011 à 21:58 +1000, Greg Banks a écrit :

> 
> Sure, and a whole lot of the callsites are ("..._%d", cpu), hence the  
> unfortune :(

BTW, we could name nfsd threads differently :

Currently, they all are named : "nfsd"

If SVC_POOL_PERCPU is selected, we could name them : 
nfsd_c0 -> nfsd_cN

If SVC_POOL_PERNODE is selected, we could name them :
nfsd_n0  -> nfsd_nN

That would help to check with "ps aux" which cpu/nodes are under stress.

Re: [PATCH] sunrpc: use better NUMA affinities

[Greg Banks]

Sent from my iPhone

On 29/07/2011, at 22:11, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le vendredi 29 juillet 2011 =C3=A0 21:58 +1000, Greg Banks a =C3=A9cr=
it :
>
>>
>> Sure, and a whole lot of the callsites are ("..._%d", cpu), hence th=
e
>> unfortune :(
>
> BTW, we could name nfsd threads differently :
>
> Currently, they all are named : "nfsd"
>
> If SVC_POOL_PERCPU is selected, we could name them :
> nfsd_c0 -> nfsd_cN
>
> If SVC_POOL_PERNODE is selected, we could name them :
> nfsd_n0  -> nfsd_nN
>
> That would help to check with "ps aux" which cpu/nodes are under =20
> stress.
>
>

I like it!

>

Greg.

Re: [PATCH] sunrpc: use better NUMA affinities

[J. Bruce Fields]

On Fri, Jul 29, 2011 at 11:30:05PM +1000, Greg Banks wrote:
>=20
>=20
> Sent from my iPhone
>=20
> On 29/07/2011, at 22:11, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>=20
> > Le vendredi 29 juillet 2011 =C3=A0 21:58 +1000, Greg Banks a =C3=A9=
crit :
> >
> >>
> >> Sure, and a whole lot of the callsites are ("..._%d", cpu), hence =
the
> >> unfortune :(
> >
> > BTW, we could name nfsd threads differently :
> >
> > Currently, they all are named : "nfsd"
> >
> > If SVC_POOL_PERCPU is selected, we could name them :
> > nfsd_c0 -> nfsd_cN
> >
> > If SVC_POOL_PERNODE is selected, we could name them :
> > nfsd_n0  -> nfsd_nN
> >
> > That would help to check with "ps aux" which cpu/nodes are under =20
> > stress.
> >
> >
>=20
> I like it!

Yup, patch welcomed.--b.

Re: [PATCH] sunrpc: use better NUMA affinities

[J. Bruce Fields]

On Fri, Jul 29, 2011 at 12:48:36PM -0400, bfields wrote:
> On Fri, Jul 29, 2011 at 11:30:05PM +1000, Greg Banks wrote:
> > 
> > 
> > Sent from my iPhone
> > 
> > On 29/07/2011, at 22:11, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > 
> > > Le vendredi 29 juillet 2011 à 21:58 +1000, Greg Banks a écrit :
> > >
> > >>
> > >> Sure, and a whole lot of the callsites are ("..._%d", cpu), hence the
> > >> unfortune :(
> > >
> > > BTW, we could name nfsd threads differently :
> > >
> > > Currently, they all are named : "nfsd"
> > >
> > > If SVC_POOL_PERCPU is selected, we could name them :
> > > nfsd_c0 -> nfsd_cN
> > >
> > > If SVC_POOL_PERNODE is selected, we could name them :
> > > nfsd_n0  -> nfsd_nN
> > >
> > > That would help to check with "ps aux" which cpu/nodes are under  
> > > stress.
> > >
> > >
> > 
> > I like it!
> 
> Yup, patch welcomed.--b.

(Annoying fact: some initscripts stop nfsd using a rough equivalent of
"killall nfsd".  So the name of the threads is arguably ABI.  I think
those initscripts are nuts and deserve what they get, but that may be
because I'm forgetting the reason they do that.)

--b.

Re: [PATCH] sunrpc: use better NUMA affinities

[Eric Dumazet]

Le vendredi 29 juillet 2011 à 12:53 -0400, J. Bruce Fields a écrit :

> (Annoying fact: some initscripts stop nfsd using a rough equivalent of
> "killall nfsd".  So the name of the threads is arguably ABI.  I think
> those initscripts are nuts and deserve what they get, but that may be
> because I'm forgetting the reason they do that.)
> 

Yes, it seems we cant change then...

I tried on a RHEL4 distro and "service nfs stop" could not stop my nfsd
threads if I changed their names as described.

Next, I am going to try nfsd/...  

By the way, any idea why after "service nfs stop" I cannot unload sunrpc
module ?

# lsmod
Module                  Size  Used by
nfsd                  285597  3 
lockd                  76103  1 nfsd
auth_rpcgss            45002  1 nfsd
sunrpc                231915  6 nfsd,lockd,auth_rpcgss
exportfs                3813  1 nfsd

Re: [PATCH] sunrpc: use better NUMA affinities

[Greg Banks]

Sent from my iPhone

On 30/07/2011, at 2:53, "J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Fri, Jul 29, 2011 at 12:48:36PM -0400, bfields wrote:
>> On Fri, Jul 29, 2011 at 11:30:05PM +1000, Greg Banks wrote:
>>>
>>>
>>> Sent from my iPhone
>>>
>>> On 29/07/2011, at 22:11, Eric Dumazet <eric.dumazet@gmail.com>  
>>> wrote:
>>>
>>>> Le vendredi 29 juillet 2011 à 21:58 +1000, Greg Banks a écrit :
>>>>
>>>>>
>>>>> Sure, and a whole lot of the callsites are ("..._%d", cpu),  
>>>>> hence the
>>>>> unfortune :(
>>>>
>>>> BTW, we could name nfsd threads differently :
>>>>
>>>> Currently, they all are named : "nfsd"
>>>>
>>>> If SVC_POOL_PERCPU is selected, we could name them :
>>>> nfsd_c0 -> nfsd_cN
>>>>
>>>> If SVC_POOL_PERNODE is selected, we could name them :
>>>> nfsd_n0  -> nfsd_nN
>>>>
>>>> That would help to check with "ps aux" which cpu/nodes are under
>>>> stress.
>>>>
>>>>
>>>
>>> I like it!
>>
>> Yup, patch welcomed.--b.
>
> (Annoying fact: some initscripts stop nfsd using a rough equivalent of
> "killall nfsd".  So the name of the threads is arguably ABI.  I think
> those initscripts are nuts and deserve what they get, but that may be
> because I'm forgetting the reason they do that.)
>

We had this discussion in May-June 2008; it's because the nfsds were  
once  many many years ago userspace threads.

The "killall nfsd" semantics in those scripts are awful and lead to  
problems shutting down when there are lots of threads. It would  
probably be an improvement to provide a better shutdown mechanism and  
force distros to use it.

Or, you could preserve the effective semantics by having a single  
"nfsd" thread whose purpose is to notice that it's being signalled and  
perform a clean shutdown (perhaps blocking the thread doing the kill()  
call until the shutdown has completed).

Greg.

Re: [PATCH] sunrpc: use better NUMA affinities

[NeilBrown]

On Sat, 30 Jul 2011 06:34:44 +1000 Greg Banks <gnb@fastmail.fm> wrote:

> 
> 
> Sent from my iPhone
> 
> On 30/07/2011, at 2:53, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Fri, Jul 29, 2011 at 12:48:36PM -0400, bfields wrote:
> >> On Fri, Jul 29, 2011 at 11:30:05PM +1000, Greg Banks wrote:
> >>>
> >>>
> >>> Sent from my iPhone
> >>>
> >>> On 29/07/2011, at 22:11, Eric Dumazet <eric.dumazet@gmail.com>  
> >>> wrote:
> >>>
> >>>> Le vendredi 29 juillet 2011 à 21:58 +1000, Greg Banks a écrit :
> >>>>
> >>>>>
> >>>>> Sure, and a whole lot of the callsites are ("..._%d", cpu),  
> >>>>> hence the
> >>>>> unfortune :(
> >>>>
> >>>> BTW, we could name nfsd threads differently :
> >>>>
> >>>> Currently, they all are named : "nfsd"
> >>>>
> >>>> If SVC_POOL_PERCPU is selected, we could name them :
> >>>> nfsd_c0 -> nfsd_cN
> >>>>
> >>>> If SVC_POOL_PERNODE is selected, we could name them :
> >>>> nfsd_n0  -> nfsd_nN
> >>>>
> >>>> That would help to check with "ps aux" which cpu/nodes are under
> >>>> stress.
> >>>>
> >>>>
> >>>
> >>> I like it!
> >>
> >> Yup, patch welcomed.--b.
> >
> > (Annoying fact: some initscripts stop nfsd using a rough equivalent of
> > "killall nfsd".  So the name of the threads is arguably ABI.  I think
> > those initscripts are nuts and deserve what they get, but that may be
> > because I'm forgetting the reason they do that.)
> >
> 
> We had this discussion in May-June 2008; it's because the nfsds were  
> once  many many years ago userspace threads.

Even when they became kernel threads, 'kill' was the only way to kill them -
at first.

> 
> The "killall nfsd" semantics in those scripts are awful and lead to  
> problems shutting down when there are lots of threads. It would  
> probably be an improvement to provide a better shutdown mechanism and  
> force distros to use it.

rpc.nfsd 0

will stop all nfsd threads.  Follow with

  exportfs -f

and you should be done.  I'm not 100% sure about the nfsv4 thread though -
would need to check.

And yes - I would love it if distros could standardise on start/stop scripts
so upstreams could maintain them sensibly.  This is my personal number 1
reason for liking systemd - it pushes for this standardisation.

> 
> Or, you could preserve the effective semantics by having a single  
> "nfsd" thread whose purpose is to notice that it's being signalled and  
> perform a clean shutdown (perhaps blocking the thread doing the kill()  
> call until the shutdown has completed).

That's kinda neat.  It would be an ugly wart to have to keep around, but
sometimes that the price we pay for "no regressions".

NeilBrown

Re: [PATCH] sunrpc: use better NUMA affinities

[J. Bruce Fields]

On Sat, Jul 30, 2011 at 09:30:25AM +1000, NeilBrown wrote:
> On Sat, 30 Jul 2011 06:34:44 +1000 Greg Banks <gnb@fastmail.fm> wrote=
:
>=20
> >=20
> >=20
> > Sent from my iPhone
> >=20
> > On 30/07/2011, at 2:53, "J. Bruce Fields" <bfields@fieldses.org> wr=
ote:
> >=20
> > > On Fri, Jul 29, 2011 at 12:48:36PM -0400, bfields wrote:
> > >> On Fri, Jul 29, 2011 at 11:30:05PM +1000, Greg Banks wrote:
> > >>>
> > >>>
> > >>> Sent from my iPhone
> > >>>
> > >>> On 29/07/2011, at 22:11, Eric Dumazet <eric.dumazet@gmail.com> =
=20
> > >>> wrote:
> > >>>
> > >>>> Le vendredi 29 juillet 2011 =C3=A0 21:58 +1000, Greg Banks a =C3=
=A9crit :
> > >>>>
> > >>>>>
> > >>>>> Sure, and a whole lot of the callsites are ("..._%d", cpu), =20
> > >>>>> hence the
> > >>>>> unfortune :(
> > >>>>
> > >>>> BTW, we could name nfsd threads differently :
> > >>>>
> > >>>> Currently, they all are named : "nfsd"
> > >>>>
> > >>>> If SVC_POOL_PERCPU is selected, we could name them :
> > >>>> nfsd_c0 -> nfsd_cN
> > >>>>
> > >>>> If SVC_POOL_PERNODE is selected, we could name them :
> > >>>> nfsd_n0  -> nfsd_nN
> > >>>>
> > >>>> That would help to check with "ps aux" which cpu/nodes are und=
er
> > >>>> stress.
> > >>>>
> > >>>>
> > >>>
> > >>> I like it!
> > >>
> > >> Yup, patch welcomed.--b.
> > >
> > > (Annoying fact: some initscripts stop nfsd using a rough equivale=
nt of
> > > "killall nfsd".  So the name of the threads is arguably ABI.  I t=
hink
> > > those initscripts are nuts and deserve what they get, but that ma=
y be
> > > because I'm forgetting the reason they do that.)
> > >
> >=20
> > We had this discussion in May-June 2008; it's because the nfsds wer=
e =20
> > once  many many years ago userspace threads.
>=20
> Even when they became kernel threads, 'kill' was the only way to kill=
 them -
> at first.
>=20
> >=20
> > The "killall nfsd" semantics in those scripts are awful and lead to=
 =20
> > problems shutting down when there are lots of threads. It would =20
> > probably be an improvement to provide a better shutdown mechanism a=
nd =20
> > force distros to use it.
>=20
> rpc.nfsd 0
>=20
> will stop all nfsd threads.  Follow with
>=20
>   exportfs -f
>=20
> and you should be done.  I'm not 100% sure about the nfsv4 thread tho=
ugh -
> would need to check.

Should be fine.

--b.

Re: [PATCH] sunrpc: use better NUMA affinities

[Eric Dumazet]

Le vendredi 29 juillet 2011 =C3=A0 19:48 -0400, J. Bruce Fields a =C3=A9=
crit :
> On Sat, Jul 30, 2011 at 09:30:25AM +1000, NeilBrown wrote:
> >=20
> > rpc.nfsd 0
> >=20
> > will stop all nfsd threads.  Follow with
> >=20
> >   exportfs -f
> >=20
> > and you should be done.  I'm not 100% sure about the nfsv4 thread t=
hough -
> > would need to check.
>=20
> Should be fine.
>=20

What are the needed steps to be able to unload nfsd module ?

So far, I am unable to perform this without using -f option.

Re: [PATCH] sunrpc: use better NUMA affinities

[NeilBrown]

On Sat, 30 Jul 2011 06:08:34 +0200 Eric Dumazet <eric.dumazet@gmail.com>
wrote:

> Le vendredi 29 juillet 2011 à 19:48 -0400, J. Bruce Fields a écrit :
> > On Sat, Jul 30, 2011 at 09:30:25AM +1000, NeilBrown wrote:
> > > 
> > > rpc.nfsd 0
> > > 
> > > will stop all nfsd threads.  Follow with
> > > 
> > >   exportfs -f
> > > 
> > > and you should be done.  I'm not 100% sure about the nfsv4 thread though -
> > > would need to check.
> > 
> > Should be fine.
> > 
> 
> What are the needed steps to be able to unload nfsd module ?
> 
> So far, I am unable to perform this without using -f option.
> 
> 
> 
> 

Kill all nfsd threads.
   This can be done with 'kill' or 'rpc.nfsd 0' or
     echo 0 > /proc/fs/nfsd/threads

kill mountd
   killall rpc.mountd

kill idmapd

unexport everything.
   This can be done with 'exportfs -f' and checked by
    grep . /proc/net/rpc/*/content
   this should only contain comments

unmount the 'nfsd' filesystem
   umount /proc/fs/nfsd

rmmod nfsd


(it look me ages to remember idmapd - but 
   lsof /proc/net/rpc/*/*
 gave it away)

NeilBrown

Re: [PATCH] sunrpc: use better NUMA affinities

[Eric Dumazet]

Le samedi 30 juillet 2011 =C3=A0 16:06 +1000, NeilBrown a =C3=A9crit :
> umount /proc/fs/nfsd
>=20
>=20

Thans  a lot, this was the thing I missed !

Re: [PATCH] sunrpc: use better NUMA affinities

[Eric Dumazet]

Le samedi 30 juillet 2011 à 08:23 +0200, Eric Dumazet a écrit :
> Le samedi 30 juillet 2011 à 16:06 +1000, NeilBrown a écrit :
> > umount /proc/fs/nfsd
> > 
> > 
> 
> Thans  a lot, this was the thing I missed !
> 
> 

Hmm, after this, I left things as the were (nfsd module loaded, but a 0
use count )

I got a BUG while updatedb was running,
(I tried this on a debian machine/kernel)

Same bug on :

cat /proc/self/mounts 

I'll take a look at this issue later, after my vacations, unless someone
wants to take a look before me ;)

[1020029.301174] BUG: unable to handle kernel paging request at ffffffffa03da0d0
[1020029.301318] IP: [<ffffffff811105bd>] show_type+0x17/0x5a
[1020029.301436] PGD 1605067 PUD 1609063 PMD 127353067 PTE 0
[1020029.301574] Oops: 0000 [#1] SMP 
[1020029.301637] last sysfs file: /sys/module/nfsd/initstate
[1020029.301722] CPU 6 
[1020029.301765] Modules linked in: nfsd fuse btrfs zlib_deflate crc32c libcrc32c ufs qnx4 hfsplus hfs minix ntfs vfat msdos fat jfs xfs reiserfs ext4 jbd2 crc16 ext2 dm_mod nls_utf8 isofs lockd fscache auth_rpcgss nfs_acl sunrpc loop i5000_edac radeon edac_core ttm drm_kms_helper snd_pcm drm snd_timer snd i2c_algo_bit soundcore i2c_core snd_page_alloc i5k_amb power_supply ipmi_si evdev psmouse rng_core ipmi_msghandler hpilo hpwdt shpchp processor button pcspkr serio_raw pci_hotplug container ext3 jbd mbcache hpsa usbhid hid uhci_hcd cciss scsi_mod ehci_hcd usbcore bnx2 thermal thermal_sys [last unloaded: nfsd]
[1020029.303561] 
[1020029.303588] Pid: 23460, comm: updatedb.mlocat Tainted: G  R          2.6.39-2-amd64 #1 HP ProLiant BL460c G1
[1020029.303751] RIP: 0010:[<ffffffff811105bd>]  [<ffffffff811105bd>] show_type+0x17/0x5a
[1020029.303866] RSP: 0018:ffff880011da7e48  EFLAGS: 00010296
[1020029.303953] RAX: ffffffffa03da0d0 RBX: ffff8801288a4a80 RCX: 0000000000007774
[1020029.304061] RDX: ffffffff814c9935 RSI: ffff880129ea3800 RDI: ffff8801288a4a80
[1020029.304169] RBP: ffff880129ea3800 R08: ffff880011da7e68 R09: ffffffffffffffff
[1020029.304263] R10: ffff880011d883b2 R11: 0000000000000007 R12: ffff88012794a2c0
[1020029.304263] R13: ffff8801288a4a80 R14: ffff88012794a338 R15: 0000000000000000
[1020029.304263] FS:  00007f9cf0dec700(0000) GS:ffff88012fd80000(0000) knlGS:0000000000000000
[1020029.304263] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[1020029.304263] CR2: ffffffffa03da0d0 CR3: 0000000058f90000 CR4: 00000000000006e0
[1020029.304263] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[1020029.304263] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[1020029.304263] Process updatedb.mlocat (pid: 23460, threadinfo ffff880011da6000, task ffff88002741a440)
[1020029.304263] Stack:
[1020029.304263]  ffff8801288a4a80 ffff8801288a4a80 ffff88012794a338 ffffffff81110abf
[1020029.304263]  ffff88012794a2c0 ffff88010d992440 0000000000000400 00007f9cf0df3000
[1020029.304263]  ffff880127960cc0 ffff8801288a4a80 ffff88012794a338 ffffffff8111427a
[1020029.304263] Call Trace:
[1020029.304263]  [<ffffffff81110abf>] ? show_vfsmnt+0x9b/0x134
[1020029.304263]  [<ffffffff8111427a>] ? seq_read+0x276/0x350
[1020029.304263]  [<ffffffff810fc207>] ? vfs_read+0xa1/0xfb
[1020029.304263]  [<ffffffff810d8052>] ? sys_mmap_pgoff+0x127/0x15a
[1020029.304263]  [<ffffffff810fc317>] ? sys_read+0x45/0x6e
[1020029.304263]  [<ffffffff81338e52>] ? system_call_fastpath+0x16/0x1b
[1020029.304263] Code: 01 00 65 48 03 14 25 20 dc 00 00 66 ff 02 c3 f0 ff 47 38 c3 55 48 89 f5 48 c7 c2 35 99 4c 81 53 48 89 fb 48 83 ec 08 48 8b 46 28 
[1020029.304263]  8b 30 e8 c7 36 00 00 48 8b 85 b8 02 00 00 48 85 c0 74 2b 80 
[1020029.304263] RIP  [<ffffffff811105bd>] show_type+0x17/0x5a
[1020029.304263]  RSP <ffff880011da7e48>
[1020029.304263] CR2: ffffffffa03da0d0
[1020029.311417] ---[ end trace ebf19aff1e083659 ]---

Re: [PATCH] sunrpc: use better NUMA affinities

[J. Bruce Fields]

On Sun, Jul 31, 2011 at 08:58:54AM +0200, Eric Dumazet wrote:
> Le samedi 30 juillet 2011 à 08:23 +0200, Eric Dumazet a écrit :
> > Le samedi 30 juillet 2011 à 16:06 +1000, NeilBrown a écrit :
> > > umount /proc/fs/nfsd
> > > 
> > > 
> > 
> > Thans  a lot, this was the thing I missed !
> > 
> > 
> 
> Hmm, after this, I left things as the were (nfsd module loaded, but a 0
> use count )
> 
> I got a BUG while updatedb was running,
> (I tried this on a debian machine/kernel)
> 
> Same bug on :
> 
> cat /proc/self/mounts 

Any luck figuring this out?

I tried reproducing, by stopping everything to get nfsd refcount to
zero:

	# lsmod | grep '^nfsd'
	nfsd                  300025  0 

then running 'cat /proc/self/mounts'.

But I don't see any crash.

This is only 3.1.0-rc1 plus some nfsd patches.

--b.

> 
> I'll take a look at this issue later, after my vacations, unless someone
> wants to take a look before me ;)
> 
> [1020029.301174] BUG: unable to handle kernel paging request at ffffffffa03da0d0
> [1020029.301318] IP: [<ffffffff811105bd>] show_type+0x17/0x5a
> [1020029.301436] PGD 1605067 PUD 1609063 PMD 127353067 PTE 0
> [1020029.301574] Oops: 0000 [#1] SMP 
> [1020029.301637] last sysfs file: /sys/module/nfsd/initstate
> [1020029.301722] CPU 6 
> [1020029.301765] Modules linked in: nfsd fuse btrfs zlib_deflate crc32c libcrc32c ufs qnx4 hfsplus hfs minix ntfs vfat msdos fat jfs xfs reiserfs ext4 jbd2 crc16 ext2 dm_mod nls_utf8 isofs lockd fscache auth_rpcgss nfs_acl sunrpc loop i5000_edac radeon edac_core ttm drm_kms_helper snd_pcm drm snd_timer snd i2c_algo_bit soundcore i2c_core snd_page_alloc i5k_amb power_supply ipmi_si evdev psmouse rng_core ipmi_msghandler hpilo hpwdt shpchp processor button pcspkr serio_raw pci_hotplug container ext3 jbd mbcache hpsa usbhid hid uhci_hcd cciss scsi_mod ehci_hcd usbcore bnx2 thermal thermal_sys [last unloaded: nfsd]
> [1020029.303561] 
> [1020029.303588] Pid: 23460, comm: updatedb.mlocat Tainted: G  R          2.6.39-2-amd64 #1 HP ProLiant BL460c G1
> [1020029.303751] RIP: 0010:[<ffffffff811105bd>]  [<ffffffff811105bd>] show_type+0x17/0x5a
> [1020029.303866] RSP: 0018:ffff880011da7e48  EFLAGS: 00010296
> [1020029.303953] RAX: ffffffffa03da0d0 RBX: ffff8801288a4a80 RCX: 0000000000007774
> [1020029.304061] RDX: ffffffff814c9935 RSI: ffff880129ea3800 RDI: ffff8801288a4a80
> [1020029.304169] RBP: ffff880129ea3800 R08: ffff880011da7e68 R09: ffffffffffffffff
> [1020029.304263] R10: ffff880011d883b2 R11: 0000000000000007 R12: ffff88012794a2c0
> [1020029.304263] R13: ffff8801288a4a80 R14: ffff88012794a338 R15: 0000000000000000
> [1020029.304263] FS:  00007f9cf0dec700(0000) GS:ffff88012fd80000(0000) knlGS:0000000000000000
> [1020029.304263] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [1020029.304263] CR2: ffffffffa03da0d0 CR3: 0000000058f90000 CR4: 00000000000006e0
> [1020029.304263] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [1020029.304263] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [1020029.304263] Process updatedb.mlocat (pid: 23460, threadinfo ffff880011da6000, task ffff88002741a440)
> [1020029.304263] Stack:
> [1020029.304263]  ffff8801288a4a80 ffff8801288a4a80 ffff88012794a338 ffffffff81110abf
> [1020029.304263]  ffff88012794a2c0 ffff88010d992440 0000000000000400 00007f9cf0df3000
> [1020029.304263]  ffff880127960cc0 ffff8801288a4a80 ffff88012794a338 ffffffff8111427a
> [1020029.304263] Call Trace:
> [1020029.304263]  [<ffffffff81110abf>] ? show_vfsmnt+0x9b/0x134
> [1020029.304263]  [<ffffffff8111427a>] ? seq_read+0x276/0x350
> [1020029.304263]  [<ffffffff810fc207>] ? vfs_read+0xa1/0xfb
> [1020029.304263]  [<ffffffff810d8052>] ? sys_mmap_pgoff+0x127/0x15a
> [1020029.304263]  [<ffffffff810fc317>] ? sys_read+0x45/0x6e
> [1020029.304263]  [<ffffffff81338e52>] ? system_call_fastpath+0x16/0x1b
> [1020029.304263] Code: 01 00 65 48 03 14 25 20 dc 00 00 66 ff 02 c3 f0 ff 47 38 c3 55 48 89 f5 48 c7 c2 35 99 4c 81 53 48 89 fb 48 83 ec 08 48 8b 46 28 
> [1020029.304263]  8b 30 e8 c7 36 00 00 48 8b 85 b8 02 00 00 48 85 c0 74 2b 80 
> [1020029.304263] RIP  [<ffffffff811105bd>] show_type+0x17/0x5a
> [1020029.304263]  RSP <ffff880011da7e48>
> [1020029.304263] CR2: ffffffffa03da0d0
> [1020029.311417] ---[ end trace ebf19aff1e083659 ]---
> 
> 

Re: [PATCH] sunrpc: use better NUMA affinities

[Eric Dumazet]

Le vendredi 26 août 2011 à 20:02 -0400, J. Bruce Fields a écrit :
> On Sun, Jul 31, 2011 at 08:58:54AM +0200, Eric Dumazet wrote:
> > Le samedi 30 juillet 2011 à 08:23 +0200, Eric Dumazet a écrit :
> > > Le samedi 30 juillet 2011 à 16:06 +1000, NeilBrown a écrit :
> > > > umount /proc/fs/nfsd
> > > > 
> > > > 
> > > 
> > > Thans  a lot, this was the thing I missed !
> > > 
> > > 
> > 
> > Hmm, after this, I left things as the were (nfsd module loaded, but a 0
> > use count )
> > 
> > I got a BUG while updatedb was running,
> > (I tried this on a debian machine/kernel)
> > 
> > Same bug on :
> > 
> > cat /proc/self/mounts 
> 
> Any luck figuring this out?
> 
> I tried reproducing, by stopping everything to get nfsd refcount to
> zero:
> 
> 	# lsmod | grep '^nfsd'
> 	nfsd                  300025  0 
> 
> then running 'cat /proc/self/mounts'.
> 
> But I don't see any crash.
> 
> This is only 3.1.0-rc1 plus some nfsd patches.
> 
> --b.
> 
> > 
> > I'll take a look at this issue later, after my vacations, unless someone
> > wants to take a look before me ;)

Unfortunately, latest linux tree doesnt even boot on my machine, so I
cannot reproduce the problem right now.

Last message displayed being "Switching to clocksource tsc"

Previous kernels were continuing with :

[    4.448417] 00:09: ttyS1 at I/O 0x2f8 (irq = 3) is a 16550A
[    4.597727] Real Time Clock Driver v1.12b

Might be a problem with latest tty changes 

Re: [PATCH] sunrpc: use better NUMA affinities

[J. Bruce Fields]

On Sat, Jul 30, 2011 at 09:30:25AM +1000, NeilBrown wrote:
> On Sat, 30 Jul 2011 06:34:44 +1000 Greg Banks <gnb@fastmail.fm> wrote:
> > The "killall nfsd" semantics in those scripts are awful and lead to  
> > problems shutting down when there are lots of threads. It would  
> > probably be an improvement to provide a better shutdown mechanism and  
> > force distros to use it.
> 
> rpc.nfsd 0
> 
> will stop all nfsd threads.  Follow with
> 
>   exportfs -f
> 
> and you should be done.  I'm not 100% sure about the nfsv4 thread though -
> would need to check.
> 
> And yes - I would love it if distros could standardise on start/stop scripts
> so upstreams could maintain them sensibly.  This is my personal number 1
> reason for liking systemd - it pushes for this standardisation.

I can't remember if this is getting fixed in Fedora--probably not.  We
should fix it.

I'd also be fine with logging a deprecation message when someone signals
an nfsd thread.

> > Or, you could preserve the effective semantics by having a single  
> > "nfsd" thread whose purpose is to notice that it's being signalled and  
> > perform a clean shutdown (perhaps blocking the thread doing the kill()  
> > call until the shutdown has completed).
> 
> That's kinda neat.  It would be an ugly wart to have to keep around, but
> sometimes that the price we pay for "no regressions".

Yep; patch welcomed.

--b.

Re: Fw: [PATCH] sunrpc: use better NUMA affinities

[J. Bruce Fields]

On Fri, Jul 29, 2011 at 04:53:21PM +1000, Greg Banks wrote:
> On 29/07/11 16:30, Eric Dumazet wrote:
> >Le vendredi 29 juillet 2011 à 16:05 +1000, Greg Banks a écrit :
> >>On 29/07/11 15:32, NeilBrown wrote:
> >>
> >>I seem to remember coming to the conclusion that Jeff eventually
> >>addressed this problem...am I misremembering or did something regress?
> >>
> >Currently, all nfsd kthreads use memory for their kernel stack and
> >various initial data from a _single_ node, even if you use
> >sunrpc.pool_mode=pernode  (or percpu)
> 
> That's just plain broken and I'm very pleased to see you fix it.

Should I take that as a "Reviewed-by"?

> I was just surprised that it was still broken and wondering how that
> happened.  Looking at ToT I see that because I dropped the ball in
> 2008, Jeff's patches didn't address the problem.  In ToT
> svc_pool_map_set_cpumask() is called *after* kthread_create() and
> applies to the child thread, *after* it's stack has been allocated
> on the wrong node.  In the working SGI code,
> svc_pool_map_set_cpumask() is called by the parent node on itself
> *before* calling kernel_thread() or doing any of the data structure
> allocations, thus ensuring that everything gets allocated using the
> default memory allocation policy, which on SGI NFS servers was
> globally tuned to be "node-local".

OK, so would it be enough to just move the svc_pool_map_set_cpumask()
back a few lines, or do we want Eric's approach, in order to have
something that will work better with other memory allocation policies?

--b.

> 
> >With my patch, we make sure each thread gets its stack from its local
> >node.
> >
> >Check commit 94dcf29a11b3d20a (kthread: use kthread_create_on_node()) to
> >see how this strategy already was adopted for ksoftirqd, kworker,
> >migration, and pktgend kthreads.
> 
> Ah, I see.  It's unfortunate that the kthread_create() API ends up
> being passed a CPU number but that's only used to format the name
> and not for sensible things :(
> 
> -- 
> Greg.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sunrpc: use better NUMA affinities

[Greg Banks]

Sent from my iPhone

On 30/07/2011, at 2:45, "J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Fri, Jul 29, 2011 at 04:53:21PM +1000, Greg Banks wrote:
>> On 29/07/11 16:30, Eric Dumazet wrote:
>>> Le vendredi 29 juillet 2011 à 16:05 +1000, Greg Banks a écrit :
>>>> On 29/07/11 15:32, NeilBrown wrote:
>>>>
>>>> I seem to remember coming to the conclusion that Jeff eventually
>>>> addressed this problem...am I misremembering or did something  
>>>> regress?
>>>>
>>> Currently, all nfsd kthreads use memory for their kernel stack and
>>> various initial data from a _single_ node, even if you use
>>> sunrpc.pool_mode=pernode  (or percpu)
>>
>> That's just plain broken and I'm very pleased to see you fix it.
>
> Should I take that as a "Reviewed-by"?

If you like.

>
>> [...] In ToT
>> svc_pool_map_set_cpumask() is called *after* kthread_create() and
>> applies to the child thread, *after* it's stack has been allocated
>> on the wrong node.  In the working SGI code,
>> svc_pool_map_set_cpumask() is called by the parent node on itself
>> *before* calling kernel_thread() or doing any of the data structure
>> allocations, thus ensuring that everything gets allocated using the
>> default memory allocation policy, which on SGI NFS servers was
>> globally tuned to be "node-local".
>
> OK, so would it be enough to just move the svc_pool_map_set_cpumask()
> back a few lines, or do we want Eric's approach, in order to have
> something that will work better with other memory allocation policies?
>

Relying on global policy was easy but not a great idea, I prefer  
Eric's approach of doing it explicitly.

Greg.
>>

Re: Fw: [PATCH] sunrpc: use better NUMA affinities

[J. Bruce Fields]

On Fri, Jul 29, 2011 at 04:05:40PM +1000, Greg Banks wrote:
> On 29/07/11 15:32, NeilBrown wrote:
> >
> >Hi Greg,
> >  I saw this patch float past and thought of you... You may not be interested
> >  any more, and it may be a perfectly good patch that does not need any
> >  comment, but I thought I would let you know anyway.
> 
> Thanks Neil.
> 
> I've trimmed the cc list to limit the number of copies Trond and Bruce get:)

I appreciate the thought, but in future please don't trim cc's.

--b.