Hi, On 2016/01/06 3:40, Taylor R Campbell wrote: > Date: Tue, 5 Jan 2016 17:30:35 +0900 > From: Kengo NAKAHARA <[email protected]> > > I use rw_init() for gif_softc_list_lock instead of rw_obj_alloc(). > However, I still use rw_obj_alloc() for struct gif_softc.gif_lock. > If I use rw_init() for struct gif_softc.gif_lock, it would be the > following code > [...] > krwlock_t gif_lock __aligned(COHERENCY_UNIT); /* > lock for softc */ > int dummy __aligned(COHERENCY_UNIT); /* padding for cache > line */ > [...] > I feel it seems worse... > > There's no need for this inside a struct. The point of > __cacheline_aligned in a global object is to prevent it from sharing a > cache line with unrelated objects, which would cause needless > contention. But inside a struct, the cache line is pretty much > guaranteed to be filled with related objects.
Thank you for your point out. I use rw_init() for gif_lock without __cacheline_aligned. > The only time you need to worry about it in a struct is when there are > independent parts for which you want independent contention, e.g. in > struct pcq (kern/subr_pcq.c). I have a question for confirmation. If the struct has two locks for different purposes, is the COHERENCY_UNIT padding required between the locks isn't it? E.g. in my old patch(gif-mp-ify.patch), is the COHERENCY_UNIT padding is required between gif_lock and struct si_sync which has other lock(si_lock)? # BTW, struct si_sync is removed by if_gif.h:r1.21 > I agree pserialize(9) gives it better MP-scalable than rwlock. However, > I think it may be a premature optimization. The reason I think so is > the following lockstat result of my kernel (which is enable both > NET_MPSAFE and LOCKDEBUG). > > Fair enough -- I just wanted to make sure there was some thought put > into the intended access patterns so that we will be working toward an > MP-scalable, not just an MP-safe, network stack. Exactly. I'm grateful for your point out which reader locks still have high contention. I would like to use pserialize(9) for gif(4) while making other network components such as TX not only MP-safe but also MP-scalable. > There are a couple other things I'm concerned about in the patch -- I > notice you're doing softint_establish with a lock held. Maybe that's > OK, but it makes me a little nervous. I don't have time right now but > I would like to take a closer look in the next few days at some of the > changes like that that your patch makes. It works for my environment for now..., meanwhile I see your point. Thank you for your help. I'm looking forward to your review. Thanks, -- ////////////////////////////////////////////////////////////////////// Internet Initiative Japan Inc. Device Engineering Section, Core Product Development Department, Product Division, Technology Unit Kengo NAKAHARA <[email protected]>
