Messages in this thread | | | Date | Mon, 29 Sep 2008 23:38:36 -0400 | From | Mathieu Desnoyers <> | Subject | Re: [PATCH] markers: fix unregister bug and reenter bug |
| |
* Lai Jiangshan (laijs@cn.fujitsu.com) wrote: > Mathieu Desnoyers wrote: > > Hi Lai, > > > > I'll have to nack this fix : > > > > One fix I already posted makes sure every marker unregister callers call > > synchronize_sched() _at some point_ before module unload. It thus makes > > sure we can do batch unregistration without doing multiple > > synchronize_sched calls. > > 1) the new API marker_synchronize_unregister() is ugly, it separate one thing > into two steps. > user have to remember to call marker_synchronize_unregister() and have > to know what he can do and what he can't do before > marker_synchronize_unregister(). >
Hum, yes it does separate unregistration and synchronization in two steps for a very precise purpose : I don't want unregistration of 100 markers to take ~30 seconds on a heavily loaded machine.
> 2) IMO, synchronous code is better than asynchronous in non-critical-path. > we need synchronize_sched() for free(old). >
free(old) is only done in call_rcu. the rcu callback is forced by a rcu_barrier() if two consecutive operations are done on the same marker.
> you fixes haven't fix the reenter bug. >
I don't see any reentrancy bug here. Have you actually experienced such an issue ? Can you give me an example of a bogus execution trace (step-by-step operations) ?
Thanks,
Mathieu
> I recommend my fix. > > > > > Also, there is no need to do the synchronize_sched with the marker mutex > > held. call_rcu_sched takes care of making sure the previous quiescent > > state is over before calling kfree. This means that when we return from > > the register/unregister functions, there may still be markers "in > > flight" using the old markers. Again, why would it be a problem ? > > > > Thanks, > > > > Mathieu > > > > P.S. : I'll send along the patches I am referring to. Ingo, those should > > probably be merged if they are not in -tip already. > > > >
-- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |