Re: [PATCH 0/9] Scalability requirements for sysv ipc - v3

Previous thread: [PATCH] IB/ehca: Protect QP against destroying until all async events for it are handled. by Stefan Roscher on Wednesday, May 7, 2008 - 4:19 am. (5 messages)

Next thread: [PATCH 1/9] Change the idr structure by Nadia.Derbey on Wednesday, May 7, 2008 - 4:35 am. (3 messages)
From: Nadia.Derbey
Date: Wednesday, May 7, 2008 - 4:35 am

After scalability problems have been detected when using the sysV ipcs, I
have proposed to use an RCU based implementation of the IDR api instead (see
threads http://lkml.org/lkml/2008/4/11/212 and
http://lkml.org/lkml/2008/4/29/295).

This resulted in many people asking to convert the idr API and make it
rcu safe (because most of the code was duplicated and thus unmaintanable
and unreviewable).

So here is a first attempt.

The important change wrt to the idr API itself is during idr removes:
idr layers are freed after a grace period, instead of being moved to the
free list.

The important change wrt to ipcs, is that idr_find() can now be called
locklessly inside a rcu read critical section.

Here are the results I've got for the pmsg test sent by Manfred: 

   2.6.25-rc3-mm1   2.6.25-rc3-mm1+   2.6.25-mm1   Patched 2.6.25-mm1
1         1168441           1064021       876000               947488
2         1094264            921059      1549592              1730685
3         2082520           1738165      1694370              2324880
4         2079929           1695521       404553              2400408
5         2898758            406566       391283              3246580
6         2921417            261275       263249              3752148
7         3308761            126056       191742              4243142
8         3329456            100129       141722              4275780

1st column: stock 2.6.25-rc3-mm1
2nd column: 2.6.25-rc3-mm1 + ipc patches (store ipcs into idrs)
3nd column: stock 2.6.25-mm1
4th column: 2.6.25-mm1 + this pacth series.

I'll send a chart as an answer to this mail: don't know how to do that
with quilt :-(


Reviewers are more than ever welcome!

Patches should be applied on linux-2.6.25-mm1, in the following order:

[ PATCH 01/09 ] : idr_add_rcu_head.patch
[ PATCH 02/09 ] : idr_rename_routines.patch
[ PATCH 03/09 ] : idr_fix_printk.patch
[ PATCH 04/09 ] : idr_rc_to_errno.patch
[ PATCH 05/09 ] : idr_get_new_rcu_safe.patch
[ PATCH 06/09 ...
From: Nadia Derbey
Date: Wednesday, May 7, 2008 - 4:41 am

You'll find in attachment the corresponding chart, and also the pmsg 
code (originally sent by Manfred).

Regards,
Nadia

From: KOSAKI Motohiro
Date: Wednesday, May 7, 2008 - 6:19 am

Hi

nice improvement.

this result is slightly odd.

similar to 2.6.25-rc3-mm1 and patched-2.6.25-mm1
similar to 2.6.25-mm1 and 2.6.25-rc3-mm1

but

not similar to 2.6.25-rc3-mm1 and patched-2.6.25-rc3-mm1
not similar to 2.6.25-mm1 and patched-2.6.25-mm1

Is patched-2.6.25-rc3-mm1 and patched-2.6.25-mm1 applied the same patch?
or I misunderstand how to see your chart?
--

From: Nadia Derbey
Date: Tuesday, May 13, 2008 - 7:10 am

Well, looks like the description was not clear, sorry for that!
1st column: 2.6.25-rc3-mm1 with original ipc implementation
                            (i.e. ipcs stored in an array)
2nd column: 2.6.25-rc3-mm1, with ipcs stored in an idr tree
             actually, that's when the performance regression
             had been introduced.
3rd column: 2.6.25-mm1, still with the same implementation
             i.e. still with degraded performances.
4th column: 2.6.25-mm1 + rcu-based implementation of the idr,
             which is the current patch series.

Hope this makes things more clear.

Regards,
Nadia
--

From: KOSAKI Motohiro
Date: Tuesday, May 13, 2008 - 9:22 pm

Oh, I see that your patchset has very nice performance result :)
Thanks!


--

From: Paul E. McKenney
Date: Friday, May 30, 2008 - 1:22 am

I guess in my case, next Tuesday was not an issue.  :-/

Anyway, the idr.c changes look good to me.  Not sure why you are using
INIT_RCU_HEAD() given that call_rcu() completely initializes the fields.
Using INIT_RCU_HEAD() doesn't cause any problems, but does add needless
code.

Commentary below, looks good from an RCU viewpoint.





OK, we aren't dereferencing.  Besides, we should hold the update-side



















--

From: Nadia Derbey
Date: Sunday, June 1, 2008 - 10:53 pm

Well the INIT_RCU_HEAD calls are here because I first thought of moving 
the freed idr layers to the free list instead of directly removing them.
So the INIT_RCU_HEAD was a way for me to have a "blank" structure when 
getting a layer from the free list, i.e. reusing an old layer.
But I finally gave up the idea... but left the INI_RCU_HEAD() calls.
Will fix this.
But I have a "process" question: should I resend the complete patch 
series or would a patch above the already submitted ones be enough?

Regards,


--

Previous thread: [PATCH] IB/ehca: Protect QP against destroying until all async events for it are handled. by Stefan Roscher on Wednesday, May 7, 2008 - 4:19 am. (5 messages)

Next thread: [PATCH 1/9] Change the idr structure by Nadia.Derbey on Wednesday, May 7, 2008 - 4:35 am. (3 messages)