is synchronize_net in inet_register_protosw necessary?

is synchronize_net in inet_register_protosw necessary?

[junjie cai]

hi all.
i am a newbie to linux kernel.
in a arm926 board i found that it took about 30ms to finish
the (net/ipv4/af_inet.c:898) inet_register_protosw
because of the synchronize_net call during profiling.
synchronize_net finally calls synchronize_rcu, so i think
this is to make the change visiable after a list_add_rcu.
but according to the Document/listRCU.txt it seems that
a list insertation does not necessarily do call_rcu etc.
may i have any mistakes, please kindly tell me.

Re: is synchronize_net in inet_register_protosw necessary?

[Paul E. McKenney]

On Tue, Jun 14, 2005 at 11:18:08PM +0900, junjie cai wrote:
> hi all.
> i am a newbie to linux kernel.
> in a arm926 board i found that it took about 30ms to finish
> the (net/ipv4/af_inet.c:898) inet_register_protosw
> because of the synchronize_net call during profiling.
> synchronize_net finally calls synchronize_rcu, so i think
> this is to make the change visiable after a list_add_rcu.
> but according to the Document/listRCU.txt it seems that
> a list insertation does not necessarily do call_rcu etc.
> may i have any mistakes, please kindly tell me.

>From a strict RCU viewpoint, you are quite correct.  But sometimes
the overall locking protocol (which almost always includes other things
besides just RCU) places additional constraints on the code.  My guess is
that the networking folks needed to ensure that the new protocol is seen
by all packets that are received after inet_register_protosw() returns.

But I need to defer to networking guys on this one.

							Thanx, Paul

Re: is synchronize_net in inet_register_protosw necessary?

[Paul E. McKenney]

On Thu, Jun 16, 2005 at 10:15:02AM -0700, Paul E. McKenney wrote:
> On Tue, Jun 14, 2005 at 11:18:08PM +0900, junjie cai wrote:
> > hi all.
> > i am a newbie to linux kernel.
> > in a arm926 board i found that it took about 30ms to finish
> > the (net/ipv4/af_inet.c:898) inet_register_protosw
> > because of the synchronize_net call during profiling.
> > synchronize_net finally calls synchronize_rcu, so i think
> > this is to make the change visiable after a list_add_rcu.
> > but according to the Document/listRCU.txt it seems that
> > a list insertation does not necessarily do call_rcu etc.
> > may i have any mistakes, please kindly tell me.
> 
> From a strict RCU viewpoint, you are quite correct.  But sometimes
> the overall locking protocol (which almost always includes other things
> besides just RCU) places additional constraints on the code.  My guess is
> that the networking folks needed to ensure that the new protocol is seen
> by all packets that are received after inet_register_protosw() returns.
> 
> But I need to defer to networking guys on this one.

Hello, Junjie,

Ran into one of the networking guys off-list.  Apparently, the
synchronize_net() is there out of paranoia.  It might be necessary,
but he could not think of a reason for its being there.  If you want
to shave 30ms off of your boot time by removing it, here is his
suggested test procedure:

o	Write a small dummy protocol as a module.

o	On an SMP machine, have one process repeated modprobe/rmmod
	while another process repeatedly does socket() calls for
	the dummy protocol.

							Thanx, Paul

Re: is synchronize_net in inet_register_protosw necessary?

[junjie cai]

Hello,Paul,

Thank you very much for your reply.
So may I think that it would be ok to remove the synchronize_net 
from  inet_register_protosw on a UP platform?
I am doing embeded development so I don't have SMP to do the test.
I think it is ok for me to do just local modify to fit our needs.
Thanks again.
                                 junjie

On 6/17/05, Paul E. McKenney <paulmck@us.ibm.com> wrote:
> On Thu, Jun 16, 2005 at 10:15:02AM -0700, Paul E. McKenney wrote:
> > On Tue, Jun 14, 2005 at 11:18:08PM +0900, junjie cai wrote:
> > > hi all.
> > > i am a newbie to linux kernel.
> > > in a arm926 board i found that it took about 30ms to finish
> > > the (net/ipv4/af_inet.c:898) inet_register_protosw
> > > because of the synchronize_net call during profiling.
> > > synchronize_net finally calls synchronize_rcu, so i think
> > > this is to make the change visiable after a list_add_rcu.
> > > but according to the Document/listRCU.txt it seems that
> > > a list insertation does not necessarily do call_rcu etc.
> > > may i have any mistakes, please kindly tell me.
> >
> > From a strict RCU viewpoint, you are quite correct.  But sometimes
> > the overall locking protocol (which almost always includes other things
> > besides just RCU) places additional constraints on the code.  My guess is
> > that the networking folks needed to ensure that the new protocol is seen
> > by all packets that are received after inet_register_protosw() returns.
> >
> > But I need to defer to networking guys on this one.
> 
> Hello, Junjie,
> 
> Ran into one of the networking guys off-list.  Apparently, the
> synchronize_net() is there out of paranoia.  It might be necessary,
> but he could not think of a reason for its being there.  If you want
> to shave 30ms off of your boot time by removing it, here is his
> suggested test procedure:
> 
> o       Write a small dummy protocol as a module.
> 
> o       On an SMP machine, have one process repeated modprobe/rmmod
>         while another process repeatedly does socket() calls for
>         the dummy protocol.
> 
>                                                         Thanx, Paul
>

Re: is synchronize_net in inet_register_protosw necessary?

[Paul E. McKenney]

On Fri, Jun 17, 2005 at 10:37:37AM +0900, junjie cai wrote:
> Hello,Paul,
> 
> Thank you very much for your reply.
> So may I think that it would be ok to remove the synchronize_net 
> from  inet_register_protosw on a UP platform?
> I am doing embeded development so I don't have SMP to do the test.
> I think it is ok for me to do just local modify to fit our needs.

Hello, Junjie,

If I was in your position, I would want to do the testing.  If there
is a problem removing it (which might show up in UP operation, given
unfortunate sequences of packet-reception interrupts at boot time),
your users might not be so happy to encounter a boot-time failure every
so often, right?

If you do the testing, and are willing to hand out your dummy protocol
module, you might find that someone is willing to run the test on an
SMP system.  If it works out, then the change might go into mainline
Linux, so that you don't have to repeat the removal on your next
project.

Your choice, of course!

						Thanx, Paul

> Thanks again.
>                                  junjie
> 
> On 6/17/05, Paul E. McKenney <paulmck@us.ibm.com> wrote:
> > On Thu, Jun 16, 2005 at 10:15:02AM -0700, Paul E. McKenney wrote:
> > > On Tue, Jun 14, 2005 at 11:18:08PM +0900, junjie cai wrote:
> > > > hi all.
> > > > i am a newbie to linux kernel.
> > > > in a arm926 board i found that it took about 30ms to finish
> > > > the (net/ipv4/af_inet.c:898) inet_register_protosw
> > > > because of the synchronize_net call during profiling.
> > > > synchronize_net finally calls synchronize_rcu, so i think
> > > > this is to make the change visiable after a list_add_rcu.
> > > > but according to the Document/listRCU.txt it seems that
> > > > a list insertation does not necessarily do call_rcu etc.
> > > > may i have any mistakes, please kindly tell me.
> > >
> > > From a strict RCU viewpoint, you are quite correct.  But sometimes
> > > the overall locking protocol (which almost always includes other things
> > > besides just RCU) places additional constraints on the code.  My guess is
> > > that the networking folks needed to ensure that the new protocol is seen
> > > by all packets that are received after inet_register_protosw() returns.
> > >
> > > But I need to defer to networking guys on this one.
> > 
> > Hello, Junjie,
> > 
> > Ran into one of the networking guys off-list.  Apparently, the
> > synchronize_net() is there out of paranoia.  It might be necessary,
> > but he could not think of a reason for its being there.  If you want
> > to shave 30ms off of your boot time by removing it, here is his
> > suggested test procedure:
> > 
> > o       Write a small dummy protocol as a module.
> > 
> > o       On an SMP machine, have one process repeated modprobe/rmmod
> >         while another process repeatedly does socket() calls for
> >         the dummy protocol.
> > 
> >                                                         Thanx, Paul
> >
>