Re: increase softint_bytes

[6bone]

On Thu, 16 Nov 2017, Masanobu SAITOH wrote:


Hi, all.

Some device drivers now allocate a lot of softints.
See:

http://mail-index.netbsd.org/current-users/2017/11/09/msg032581.html

To avoid this panic, I wrote the following patch:

http://www.netbsd.org/~msaitoh/softint-20171116-0.dif



I tested the patch. Now the dump comes in another place.

https://suse.uni-leipzig.de/crash/crash-with-patch.jpg

Regards
Uwe

Re: increase softint_bytes

[Masanobu SAITOH]

Hi, all.

On 2017/11/17 15:35, Masanobu SAITOH wrote:

Hi, mrg.

On 2017/11/17 5:05, matthew green wrote:

Masanobu SAITOH writes:

   Hi, all.

   Some device drivers now allocate a lot of softints.
See:

http://mail-index.netbsd.org/current-users/2017/11/09/msg032581.html

To avoid this panic, I wrote the following patch:

http://www.netbsd.org/~msaitoh/softint-20171116-0.dif

Summary:

- Increase the default size from 8192 bytes to 32768 bytes.
- Add new option SOFTINT_BYTES to change the value.
- Add two new read-only sysctls kern.softint.{max,count}

Any comment?


can't this be fixed by making it dynamic?


  It's not easy because the return value of softint_establish() is
made from this area's address. As you know, the value is keep by each driver.


 I'm sorry. I misread kern_softint.c. The return value is not directly point to
the area, but it's offset of the area, so it would be easy to resize it.

 I'll try to modify the code to do auto-resize.






 ie, fix it properly so
that it never has this problem again -- if in 10 years we're adding
5x as many softints as today, we'll hit it again.

i also don't see why the crash can't be fixed to become an error.


The code is:

ip_init(void)
{
    const struct protosw *pr;

    in_init();
    sysctl_net_inet_ip_setup(NULL);

    pr = pffindproto(PF_INET, IPPROTO_RAW, SOCK_RAW);
    KASSERT(pr != NULL);

    ip_pktq = pktq_create(IFQ_MAXLEN, ipintr, NULL);
    KASSERT(ip_pktq != NULL); <== This.






the code tries to avoid adding more than it can, but soemthing must
be wrong for the crash to occur anyway.


  I think this KASSERT() is reasonable.


  ie, at worst we'd fix it
to return an error instead of crashing.

if that is done, the sysctl exports seem unneccesary as well.


bridge, agr, strip, sl, stf, ppp, l2tp, gre and maybe some other
pseudo interfaces which is created by ifconfig command allocate
softint. Some of them may be created tens, hundreds or more.
So I think it's worth to check the current number.




thanks.


.mrg.







--
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

Re: increase softint_bytes

[Manuel Bouyer]

On Fri, Nov 17, 2017 at 03:35:48PM +0900, Masanobu SAITOH wrote:
> bridge, agr, strip, sl, stf, ppp, l2tp, gre and maybe some other
> pseudo interfaces which is created by ifconfig command allocate
> softint. Some of them may be created tens, hundreds or more.
> So I think it's worth to check the current number.

I have dom0 Xen with about 50 vlans and 50 bridges at work.
They're stuck to netbsd-6 because netbsd-7 runs out of softints
with that many interfaces.

As I understand it, things changed in this area in netbsd-8 and 
it should support that many vlans/bridges, but don't put the barrier too low.

-- 
Manuel Bouyer 
 NetBSD: 26 ans d'experience feront toujours la difference
--

Re: increase softint_bytes

[Martin Husemann]

On Fri, Nov 17, 2017 at 03:43:57PM +0900, Masanobu SAITOH wrote:
>  Each driver which call softint_establish() keep the return value, so
> it's very hard without big modification.

We could make the "void *cookie" returned by softint_establish(9) an
index (internally) instead (w/o changing ABI or API), so the value
would stay valid on reallocations of the array.

The realllocation could be handled lazy per cpu (that is: note the new
target size globaly, have the local cpu handle the reallocation at
next time a softint is established/disesatablished/scheduled).

But maybe I'm overlooking something.

Martin