Re: RFC: MSI/MSI-X implementation
On Fri, Nov 07, 2014 at 04:41:55PM +0900, Kengo NAKAHARA wrote: Could you comment the specification and implementation? The user should not be on the hook to set processor affinity for the interrupts. That is more properly the responsibility of the designer and OS. Dave -- David Young dyo...@pobox.comUrbana, IL(217) 721-9981
Re: RFC: MSI/MSI-X implementation
Hi, (2014/11/13 11:54), David Young wrote: On Fri, Nov 07, 2014 at 04:41:55PM +0900, Kengo NAKAHARA wrote: Could you comment the specification and implementation? The user should not be on the hook to set processor affinity for the interrupts. That is more properly the responsibility of the designer and OS. I wrote unclear explanation..., so please let me redescribe. This MSI/MSI-X API *design* is independent from processor affinity. The device dirvers can use MSI/MSI-X and processor affinity independently of each other. In other words, legacy interrupts and INTx interrupts can use processor affinity still. Furthermore, MSI/MSI-X may or may not use processor affinity. On the other hand, this MSI/MSI-X API *x86 implementation* is dependent to my IRQ affinity code, due to I want to commonalize similar code. # The IRQ affinity code has not merged NetBSD-current yet, so I should # point out it. Thanks, -- // Internet Initiative Japan Inc. Device Engineering Section, Core Product Development Department, Product Division, Technology Unit Kengo NAKAHARA k-nakah...@iij.ad.jp
Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]
Hi, I'm back to the work. Here is a new patch: http://www.netbsd.org/~ozaki-r/psz-ifnet.diff I think the patch reflects rmind's suggestions: - Use pserialize for IFNET_FOREACH - but use a lock for blockable/sleepable critical sections - cpu_intr_p workaround for HW interrupt Any comments? Thanks, ozaki-r On Tue, Aug 26, 2014 at 4:28 AM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Ryota Ozaki ozak...@netbsd.org wrote: I generally agree with Dennis that is not the way we want to take in the long-term. The cost of read-write lock is very high. The plan is to use passive serialisation to protect the interfaces and their addresses. Also, the ultimate goal would also be to use a better data structure (linked lists are not really efficient) and change the way interfaces are referenced i.e. instead of referencing ifnet_t, the network stack should use a unique ID. I have no objection to the direction. My concern is an intermediate solution. Right, but I think we can still avoid read-write lock for the intermediate solution. ID based interface referencing requires a greater revamp. Note that the code paths looking up the interface or its address(es) should not block (if they do, the code can be rearranged). Some codes under sys/compat can be blocked during the iterations, for example linux_getifconf at [1] that may block due to copyout. [1] http://nxr.netbsd.org/xref/src/sys/compat/linux/common/linux_socket.c#1134 Yes, but it is not performance sensitive path. You can acquire the lock used on the pserialize(9) writer side. Basically, we need to optimise the IP input/output paths; the rest can be heavy-locked for now. Also, in the long run, ifnet list should not be accessed from the hard interrupt context -- all users ought to be running in the softintr(9) context. The ifnet list is accessed in m_reclaim that may be called from hardware interrupt context via say MCL_GET. Yes, but my point was that we should eliminate the access of ifnet list, as well as other network stack structures, from the hard interrupt. This means converting the existing code to use softintr(9). In the long term, the drivers should always trigger softintr(9) and L2 never be called from the hard interrupt context. As for m_reclaim(9), we might workaround it with a test on cpu_intr_p() for now. We may need to take an intermediate solution, but I think we can already switch to pserialize(9) + reference counting on ifnet_t for the ip_input/ip_output() paths. I need to resume my work on the routing subsystem patch-up, though. I think we need to get rid of blockable operations mentioned the above. Or just treat them as writers. -- Mindaugas
Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]
Date: Thu, 13 Nov 2014 12:43:26 +0900 From: Ryota Ozaki ozak...@netbsd.org Here is a new patch: http://www.netbsd.org/~ozaki-r/psz-ifnet.diff I think the patch reflects rmind's suggestions: - Use pserialize for IFNET_FOREACH - but use a lock for blockable/sleepable critical sections - cpu_intr_p workaround for HW interrupt Any comments? Hmm...some quick notes from a non-expert in sys/net: - You call malloc(M_WAITOK) while the ifnet lock is held, in if_alloc_sadl_locked, which is not allowed. - You call copyout in a pserialize read section, in ifconf, which is not allowed because copyout may block. - I don't know what cpu_intr_p is working around but it's probably not a good idea! Generally, all that you are allowed to do in a pserialize read section is read a small piece of information, or grab a reference to a data structure which you are then going to use outside the read section. I don't think it's going to be easy to scalably parallelize this code without restructuring it, unless as a stop-gap you use a heaver-weight reader-writer lock like the prwlock at https://www.NetBSD.org/~riastradh/tmp/20140517/rwlock/prwlock.c. (No idea how much overhead this might add.)
Re: kernel constructor
On Wed, Nov 12, 2014 at 2:53 AM, Taylor R Campbell campb...@mumble.net wrote: Date: Tue, 11 Nov 2014 17:42:51 + From: Antti Kantee po...@iki.fi 2: init_main ordering I think that code reading is an absolute requirement there, i.e. we should be able to know offline what will happen at runtime. Maybe that problem is better addressed with an offline preprocessor which figures out the correct order? rcorder(8)...? I'll imlement tsort in config(1), because config(1) knows module dependency. Module objects will be ordered when linking, that is also reflected in the order of constructors. I believe this is good enough for most cases.
Re: kernel constructor
(snip) How about spending your energy to investigate real dependencies, for example: http://nxr.netbsd.org/xref/src/sys/kern/init_main.c#559 Does pax depend on veriexec? Does ipsec depend on pax?
Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]
On Thu, Nov 13, 2014 at 1:26 PM, Taylor R Campbell campbell+netbsd-tech-k...@mumble.net wrote: Date: Thu, 13 Nov 2014 12:43:26 +0900 From: Ryota Ozaki ozak...@netbsd.org Here is a new patch: http://www.netbsd.org/~ozaki-r/psz-ifnet.diff I think the patch reflects rmind's suggestions: - Use pserialize for IFNET_FOREACH - but use a lock for blockable/sleepable critical sections - cpu_intr_p workaround for HW interrupt Any comments? Hmm...some quick notes from a non-expert in sys/net: - You call malloc(M_WAITOK) while the ifnet lock is held, in if_alloc_sadl_locked, which is not allowed. Oh, I didn't know that restriction. LOCKDEBUG didn't correct me... - You call copyout in a pserialize read section, in ifconf, which is not allowed because copyout may block. Which one? I think I've fixed such usages this time. - I don't know what cpu_intr_p is working around but it's probably not a good idea! Yes :-| We have to fix that in the future, but it works as same as it is until we get rid of all KERNE_LOCK. Generally, all that you are allowed to do in a pserialize read section is read a small piece of information, or grab a reference to a data structure which you are then going to use outside the read section. Yes. I'm implementing a facility of the latter for ifunit: http://www.netbsd.org/~ozaki-r/ifget-ifput.diff I don't think it's going to be easy to scalably parallelize this code without restructuring it, unless as a stop-gap you use a heaver-weight reader-writer lock like the prwlock at https://www.NetBSD.org/~riastradh/tmp/20140517/rwlock/prwlock.c. (No idea how much overhead this might add.) Could you elaborate how to use it? Thanks, ozaki-r
