Ryota Ozaki wrote:
> Hi,
>
> This is another work toward MPSAFE networking.
> sys/net/if.c contains several global variables
> (e.g., ifnet_list) that should be protected
> from parallel accesses somehow once we get rid
> of the big kernel lock.
>
> Currently there is a mutex for serializing
> a
On Mon, Aug 4, 2014 at 8:21 AM, Mindaugas Rasiukevicius
wrote:
> Ryota Ozaki wrote:
>> Hi,
>>
>> This is another work toward MPSAFE networking.
>> sys/net/if.c contains several global variables
>> (e.g., ifnet_list) that should be protected
>> from parallel accesses somehow once we get rid
>> of
Ryota Ozaki 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
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?
Th
Date: Thu, 13 Nov 2014 12:43:26 +0900
From: Ryota Ozaki
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_in
On Thu, Nov 13, 2014 at 1:26 PM, Taylor R Campbell
wrote:
>Date: Thu, 13 Nov 2014 12:43:26 +0900
>From: Ryota Ozaki
>
>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
>
Date: Thu, 13 Nov 2014 15:37:29 +0900
From: Ryota Ozaki
On Thu, Nov 13, 2014 at 1:26 PM, Taylor R Campbell
wrote:
> - 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 did
On Thu, Nov 13, 2014 at 04:26:52AM +, Taylor R Campbell wrote:
>Date: Thu, 13 Nov 2014 12:43:26 +0900
>From: Ryota Ozaki
>
>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_FO
On Fri, Nov 14, 2014 at 2:01 AM, Taylor R Campbell
wrote:
>Date: Thu, 13 Nov 2014 15:37:29 +0900
>From: Ryota Ozaki
>
>On Thu, Nov 13, 2014 at 1:26 PM, Taylor R Campbell
> wrote:
>> - You call malloc(M_WAITOK) while the ifnet lock is held, in
>> if_alloc_sadl_locked, whi
Date: Fri, 14 Nov 2014 13:59:10 +0900
From: Ryota Ozaki
Related to mutex I have two questions:
- If we really want to alloc an object with holding a mutex,
what should we do? use pool(9)? or we should avoid such a situation?
Preallocate before you acquire the lock. See the kmem
On Fri, Nov 14, 2014 at 4:42 PM, Taylor R Campbell
wrote:
>Date: Fri, 14 Nov 2014 13:59:10 +0900
>From: Ryota Ozaki
>
>Related to mutex I have two questions:
>- If we really want to alloc an object with holding a mutex,
> what should we do? use pool(9)? or we should avoid suc
>> Otherwise (and even so), someone is likely to remove the kernel lock
>> without understanding the constraints and wind up with an interrupt
>> handler on cpu0 munging the ifnet list at the same time as a thread on
>> cpu1.
>>
>>Anyway we should get rid of calling m_reclaim in hardware interr
On Fri, Nov 14, 2014 at 5:28 PM, Ryota Ozaki wrote:
(snip)
>> I see. Ugh. OK, at the very least, you should write a big scary
>> comment among the locking rules for the ifnet list that explains these
>> constraints and lays out the roadmap for obviating them.
>
> Sorry for that. I'll describe ab
On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki wrote:
> - http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
There are many copyout()'s while IFNET_LOCK() is taken.
I'd agree with dyoung@, I think restructure should be done first...
On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki wrote:
> - http://www.netbsd.org/~ozaki-r/ifget-ifput.diff
Is it possible to have another "dying" flag in struct refcount, so
that ifput() can atomically set it without taking a lock, then later
call a g/c call?
On Fri, Nov 14, 2014 at 6:31 AM, David Young wrote:
> On Thu, Nov 13, 2014 at 04:26:52AM +, Taylor R Campbell wrote:
>>Date: Thu, 13 Nov 2014 12:43:26 +0900
>>From: Ryota Ozaki
>>
>>Here is a new patch: http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
>>
>>I think the patch refle
On Mon, Nov 17, 2014 at 11:39 PM, Masao Uebayashi wrote:
> On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki wrote:
>> - http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
>
> There are many copyout()'s while IFNET_LOCK() is taken.
I'm trying to get rid of them.
>
> I'd agree with dyoung@, I think restru
On Tue, Nov 18, 2014 at 11:24 AM, Ryota Ozaki wrote:
> First I'm not against restructuring, though I hoped minimum
> restructuring on non-performance-sensitive paths.
My understanding is that, non-performance-sensitive paths (e.g.
ioctl()'s) also touches performance-sensitive data structures (e.g
On Tue, Nov 18, 2014 at 12:36 PM, Masao Uebayashi wrote:
> On Tue, Nov 18, 2014 at 11:24 AM, Ryota Ozaki wrote:
>> First I'm not against restructuring, though I hoped minimum
>> restructuring on non-performance-sensitive paths.
>
> My understanding is that, non-performance-sensitive paths (e.g.
>
On Tue, Nov 18, 2014 at 12:07 AM, Masao Uebayashi wrote:
> On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki wrote:
>> - http://www.netbsd.org/~ozaki-r/ifget-ifput.diff
>
> Is it possible to have another "dying" flag in struct refcount, so
> that ifput() can atomically set it without taking a lock, th
On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki wrote:
> - http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
IFNET_RENTER();
IFNET_FOREACH() {
if->if_watchdog();
}
IFNET_REXIT();
This is not safe; if_watchdog() may resend packets, too slow operation
to do in critical section.
I think you need
Date: Tue, 18 Nov 2014 13:31:49 +0900
From: Ryota Ozaki
I have to say sorry for my bad writing; I misled you about
"free an ifnet object" due to my forgetfulness about the fact that
an ifnet object is normally embedded into a softc of each driver
(via ethercom) in NetBSD. (I.e.,
On Wed, Nov 19, 2014 at 12:18 AM, Masao Uebayashi wrote:
> On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki wrote:
>> - http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
>
> IFNET_RENTER();
> IFNET_FOREACH() {
> if->if_watchdog();
> }
> IFNET_REXIT();
>
> This is not safe; if_watchdog() may resen
On Wed, Nov 19, 2014 at 3:11 AM, Taylor R Campbell
wrote:
>Date: Tue, 18 Nov 2014 13:31:49 +0900
>From: Ryota Ozaki
>
>I have to say sorry for my bad writing; I misled you about
>"free an ifnet object" due to my forgetfulness about the fact that
>an ifnet object is normally em
On Wed, Nov 19, 2014 at 11:24 AM, Ryota Ozaki wrote:
> Weird :-/
I don't think so. For fast paths to access data really fast, slow
paths take a little way around (pre-allocation, etc). This is a fair
trade-off. If you can achieve such a goal (e.g. lockless access of
list) without restructuring
On Wed, Nov 19, 2014 at 2:38 PM, Masao Uebayashi wrote:
> On Wed, Nov 19, 2014 at 11:24 AM, Ryota Ozaki wrote:
>> Weird :-/
>
> I don't think so. For fast paths to access data really fast, slow
> paths take a little way around (pre-allocation, etc). This is a fair
> trade-off. If you can achie
On Thu, Nov 20, 2014 at 11:38 AM, Ryota Ozaki wrote:
> Actually FreeBSD seems to have a callout for each interface. Even more
> they killed the common if_watchdog facility and each driver has its own.
What happens if many (e.g. 1,000) virtual ifs are present?
> Anyway here is a patch:
> http://w
On Thu, Nov 20, 2014 at 11:51 AM, Masao Uebayashi wrote:
> On Thu, Nov 20, 2014 at 11:38 AM, Ryota Ozaki wrote:
>> Actually FreeBSD seems to have a callout for each interface. Even more
>> they killed the common if_watchdog facility and each driver has its own.
>
> What happens if many (e.g. 1,00
Looks good to me.
But you should test before commit. :)
On Thu, Nov 20, 2014 at 1:35 PM, Masao Uebayashi wrote:
> Looks good to me.
Thanks!
>
> But you should test before commit. :)
Of course :) ...actually I needed to tweak if.h
to make build.sh release work (because of missing
callout_t).
So yet another question; an easy way to fix the
problem is
Date: Wed, 19 Nov 2014 12:51:06 +0900
From: Ryota Ozaki
On Wed, Nov 19, 2014 at 3:11 AM, Taylor R Campbell
wrote:
> do {
> refcnt = m->m_refcnt;
> if (refcnt == 1) {
> mutex_enter(&m->m_lock);
>
On Thu, Nov 20, 2014 at 6:16 PM, Ryota Ozaki wrote:
> BTW, moving #include out of #ifdef _KERNEL
> doesn't work because struct callout of callout.h conflicts
> with that of external/bsd/am-utils. (Fixing this conflict
> is an option though...)
>
> Thanks,
> ozaki-r
>
> diff --git a/sys/net/if.h
http://marc.info/?t=14167055271&r=1&w=2
Following the ideas raised in that thread:
- Allocate callout_t dynamically. struct ifnet only has a pointer to struct
callout, which will not be read by netstat(1) anyway.
- Prefer the name "slowtimo" to "watchdog", because those callbacks do a
l
Date: Mon, 24 Nov 2014 11:23:28 +0900
From: Masao Uebayashi
http://marc.info/?t=14167055271&r=1&w=2
Following the ideas raised in that thread:
- Allocate callout_t dynamically. struct ifnet only has a pointer to struct
callout, which will not be read by netstat(1) anywa
(I'm trying, but I can't follow up all mails soon, because I need more
than x2 energy & time to write English than you.)
On Mon, Nov 24, 2014 at 12:12 PM, Taylor R Campbell
wrote:
>Date: Mon, 24 Nov 2014 11:23:28 +0900
>From: Masao Uebayashi
>
>http://marc.info/?t=14167055271&r=1
On Mon, Nov 24, 2014 at 3:26 PM, Masao Uebayashi wrote:
> netstat(1) needs to know struct type information (by building if.c) to
s/ (by building if.c) / (by including if.h) /
could you also fix netstat, etc., to use sysctl to obtain this
info, while you're at it, so we can provide a stable ABI for this
such that changing internel kernel information doesn't matter.
.mrg.
Date: Mon, 24 Nov 2014 15:26:21 +0900
From: Masao Uebayashi
(I'm trying, but I can't follow up all mails soon, because I need more
than x2 energy & time to write English than you.)
I understand! I will keep this one brief.
On Mon, Nov 24, 2014 at 12:12 PM, Taylor R Campbell
On Mon, Nov 24, 2014 at 11:16 PM, Taylor R Campbell
wrote:
>Date: Mon, 24 Nov 2014 15:26:21 +0900
>From: Masao Uebayashi
>
>(I'm trying, but I can't follow up all mails soon, because I need more
>than x2 energy & time to write English than you.)
>
> I understand! I will keep this
Date: Tue, 25 Nov 2014 00:42:58 +0900
From: Masao Uebayashi
On Mon, Nov 24, 2014 at 11:16 PM, Taylor R Campbell
wrote:
>Date: Mon, 24 Nov 2014 15:26:21 +0900
>From: Masao Uebayashi
>
>I think pserialize_read_{enter,exit}() should explicitly call
>KPREE
On Tue, Nov 25, 2014 at 12:48 AM, Taylor R Campbell
wrote:
>Date: Tue, 25 Nov 2014 00:42:58 +0900
>From: Masao Uebayashi
>
>On Mon, Nov 24, 2014 at 11:16 PM, Taylor R Campbell
> wrote:
>>Date: Mon, 24 Nov 2014 15:26:21 +0900
>>From: Masao Uebayashi
>>
>>
On Mon, Nov 24, 2014 at 11:23 AM, Masao Uebayashi wrote:
> http://marc.info/?t=14167055271&r=1&w=2
>
> Following the ideas raised in that thread:
Thank you for the information.
>
> - Allocate callout_t dynamically. struct ifnet only has a pointer to struct
> callout, which will not be rea
On Mon, Nov 24, 2014 at 3:26 PM, Masao Uebayashi wrote:
> (I'm trying, but I can't follow up all mails soon, because I need more
> than x2 energy & time to write English than you.)
>
> On Mon, Nov 24, 2014 at 12:12 PM, Taylor R Campbell
> wrote:
>>Date: Mon, 24 Nov 2014 11:23:28 +0900
>>F
On Mon, Nov 24, 2014 at 5:26 PM, matthew green wrote:
>
> could you also fix netstat, etc., to use sysctl to obtain this
> info, while you're at it, so we can provide a stable ABI for this
> such that changing internel kernel information doesn't matter.
I'm putting the task in my todo list. I'm n
On Tue, Nov 25, 2014 at 7:18 PM, Ryota Ozaki wrote:
> Hmm, if_flags.
>
> http://mail-index.netbsd.org/tech-net/2009/01/27/msg000985.html
>
> Do we have to care about kvm(3) users (i.e., netstat) as well as
> the callout_t issue?
For new flags, you can reuse if__pad1.
> I don't understand how kvm
I think I can help sysctl'ification, but let's check in this callout
thing first.
On Tue, Nov 25, 2014 at 10:02 PM, Masao Uebayashi wrote:
> On Tue, Nov 25, 2014 at 7:18 PM, Ryota Ozaki wrote:
>> Hmm, if_flags.
>>
>> http://mail-index.netbsd.org/tech-net/2009/01/27/msg000985.html
>>
>> Do we have to care about kvm(3) users (i.e., netstat) as well as
>> the callout_t issue?
>
>
On Wed, Nov 26, 2014 at 12:29 AM, Ryota Ozaki wrote:
> Yes, we can extend if_flags to int by unifying with if__pad1, and it works
> for the kernel and for old userland programs on little endian machines.
> However, it doesn't work for old userland programs on big endian machines,
> IIUC. An old us
On Wed, Nov 26, 2014 at 12:50 AM, Masao Uebayashi wrote:
> On Wed, Nov 26, 2014 at 12:29 AM, Ryota Ozaki wrote:
>> Yes, we can extend if_flags to int by unifying with if__pad1, and it works
>> for the kernel and for old userland programs on little endian machines.
>> However, it doesn't work for
On Wed, Nov 26, 2014 at 1:31 AM, Ryota Ozaki wrote:
>> I'd change if__pad1 to if_flags2 and define IFF2_SLOWTIMO_MPSAFE etc.
>
> Hmm, I think it's a last resort. I don't want to do something like
> (ifp->if_flags & IFF_XXX) && (ifp->if_flags2 & IFF_XXX2) if possible.
>
> I'm thinking more to avoid
On Wed, Nov 26, 2014 at 11:24 AM, Masao Uebayashi wrote:
> On Wed, Nov 26, 2014 at 1:31 AM, Ryota Ozaki wrote:
>>> I'd change if__pad1 to if_flags2 and define IFF2_SLOWTIMO_MPSAFE etc.
>>
>> Hmm, I think it's a last resort. I don't want to do something like
>> (ifp->if_flags & IFF_XXX) && (ifp->i
On Wed, Nov 26, 2014 at 11:58 AM, Ryota Ozaki wrote:
> On Wed, Nov 26, 2014 at 11:24 AM, Masao Uebayashi wrote:
>> On Wed, Nov 26, 2014 at 1:31 AM, Ryota Ozaki wrote:
I'd change if__pad1 to if_flags2 and define IFF2_SLOWTIMO_MPSAFE etc.
>>>
>>> Hmm, I think it's a last resort. I don't want
On Wed, Nov 26, 2014 at 1:02 PM, Ryota Ozaki wrote:
> Here is a latest patch:
> http://www.netbsd.org/~ozaki-r/watchdog-callout-per-if.diff
OK from me.
> So my remaining concern is whether we can embed callout_t with _KERNEL
> which I proposed earlier. If the answer is no, the above patch that
>
On Wed, Nov 26, 2014 at 2:43 PM, Masao Uebayashi wrote:
> On Wed, Nov 26, 2014 at 1:02 PM, Ryota Ozaki wrote:
>> Here is a latest patch:
>> http://www.netbsd.org/~ozaki-r/watchdog-callout-per-if.diff
>
> OK from me.
>
>> So my remaining concern is whether we can embed callout_t with _KERNEL
>> wh
> > Good point. There must be a way for drivers to declare if
> > CALLOUT_MPSAFE or not. Need to extend if_flags.
>
> Hmm, if_flags.
>
> http://mail-index.netbsd.org/tech-net/2009/01/27/msg000985.html
>
> Do we have to care about kvm(3) users (i.e., netstat) as well as
> the callout_t issue?
Ryota Ozaki writes:
> On Mon, Nov 24, 2014 at 5:26 PM, matthew green wrote:
> >
> > could you also fix netstat, etc., to use sysctl to obtain this
> > info, while you're at it, so we can provide a stable ABI for this
> > such that changing internel kernel information doesn't matter.
>
> I'm putt
I overlooked you've back out the struct callout * dynamic allocation
part. Can you make it use kmem as I showed?
On Wed, Nov 26, 2014 at 5:09 PM, Masao Uebayashi wrote:
> I overlooked you've back out the struct callout * dynamic allocation
> part. Can you make it use kmem as I showed?
Hmm, I'll do so, although I don't yet understand why the pointer version
is better than the embedded version.
ozaki-r
On Wed, Nov 26, 2014 at 5:53 PM, Ryota Ozaki wrote:
> On Wed, Nov 26, 2014 at 5:09 PM, Masao Uebayashi wrote:
>> I overlooked you've back out the struct callout * dynamic allocation
>> part. Can you make it use kmem as I showed?
>
> Hmm, I'll do so, although I don't yet understand why the pointe
On Wed, Nov 26, 2014 at 5:58 PM, Masao Uebayashi wrote:
> On Wed, Nov 26, 2014 at 5:53 PM, Ryota Ozaki wrote:
>> On Wed, Nov 26, 2014 at 5:09 PM, Masao Uebayashi wrote:
>>> I overlooked you've back out the struct callout * dynamic allocation
>>> part. Can you make it use kmem as I showed?
>>
>>
On Wed, Nov 26, 2014 at 5:07 PM, matthew green wrote:
>
>> > Good point. There must be a way for drivers to declare if
>> > CALLOUT_MPSAFE or not. Need to extend if_flags.
>>
>> Hmm, if_flags.
>>
>> http://mail-index.netbsd.org/tech-net/2009/01/27/msg000985.html
>>
>> Do we have to care about kv
On Tue, Nov 18, 2014 at 11:38 AM, Ryota Ozaki wrote:
> On Mon, Nov 17, 2014 at 11:39 PM, Masao Uebayashi wrote:
>> On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki wrote:
>>> - http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
>>
>> There are many copyout()'s while IFNET_LOCK() is taken.
>
> I'm trying
Hi,
I found an defect of ifnet object initializations.
- if_attach and if_alloc_sadl is called in each
interface XXXattach function
- if_alloc_sadl may be called via XXX_ifattach
(e.g., ether_ifattach)
- if_attach initializes an ifnet object, but
the initialization is incomplete and if_
On Mon, Dec 1, 2014 at 7:59 PM, Ryota Ozaki wrote:
> Hi,
>
> I found an defect of ifnet object initializations.
>
> - if_attach and if_alloc_sadl is called in each
> interface XXXattach function
> - if_alloc_sadl may be called via XXX_ifattach
> (e.g., ether_ifattach)
> - if_attach initial
Hi,
A new patch of ifnet + pserialize has come:
http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
(or https://github.com/ozaki-r/netbsd-src/tree/psz-ifnet
to see individual commits)
IFNET_LOCK is now used at only three places and there is
no sleep/block operation inside the critical sections.
m_recl
65 matches
Mail list logo
