2001-02-11 12:37:49

by Nick Urbanik

[permalink] [raw]
Subject: 2.4.2-pre3 compile error in 6pack.c

Dear folks,

2.4.2-pre3 doesn't compile with 6pack as a module; I had to disable it;
now it compiles (and so far, works fine).
kgcc -D__KERNEL__ -I/home/nicku/linux/include -Wall -Wstrict-prototypes
-O2 -fomit-frame-pointer -fno-strict-aliasing -pipe -march=i686
-DMODULE -DMODVERSIONS -include
/home/nicku/linux/include/linux/modversions.h -c -o 6pack.o 6pack.c
6pack.c: In function `sixpack_init_driver':
6pack.c:714: `KMALLOC_MAXSIZE' undeclared (first use in this function)
6pack.c:714: (Each undeclared identifier is reported only once
6pack.c:714: for each function it appears in.)
make[2]: *** [6pack.o] Error 1
make[2]: Leaving directory `/home/nicku/linux/drivers/net/hamradio'
make[1]: *** [_modsubdir_net/hamradio] Error 2
make[1]: Leaving directory `/home/nicku/linux/drivers'
make: *** [_mod_drivers] Error 2
{later:]
$ find linux -type f | xargs grep KMALLOC_MAXSIZE
linux/drivers/net/hamradio/6pack.c: if (sixpack_maxdev *
sizeof(void*) >= KMALLOC_MAXSIZE) {
1005 $ uname -r
2.4.2-pre3

--
Nick Urbanik, Dept. of Computing and Mathematics
Hong Kong Institute of Vocational Education (Tsing Yi)
email: [email protected]
Tel: (852) 2436 8576, (852) 2436 8579 Fax: (852) 2435 1406
pgp ID: 7529555D fingerprint: 53 B6 6D 73 52 EE 1F EE EC F8 21 98 45 1C 23 7B




2001-02-11 17:14:07

by Alan

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

> 2.4.2-pre3 doesn't compile with 6pack as a module; I had to disable it;
> now it compiles (and so far, works fine).

It has a slight dependancy on -ac right now.

KMALLOC_MAXSIZE is the alloc size limit - 131072. It checks this as kmalloc
now panics if called with an oversize request


2001-02-11 20:00:19

by Jeff Garzik

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

Alan Cox wrote:
>
> > 2.4.2-pre3 doesn't compile with 6pack as a module; I had to disable it;
> > now it compiles (and so far, works fine).
>
> It has a slight dependancy on -ac right now.
>
> KMALLOC_MAXSIZE is the alloc size limit - 131072. It checks this as kmalloc
> now panics if called with an oversize request

Would it be costly/reasonable to have kmalloc -not- panic if given a
too-large size? Principle of Least Surprises says it should return NULL
at the very least.

Jeff


--
Jeff Garzik | "You see, in this world there's two kinds of
Building 1024 | people, my friend: Those with loaded guns
MandrakeSoft | and those who dig. You dig." --Blondie

2001-02-11 20:09:00

by David Weinehall

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

On Sun, Feb 11, 2001 at 02:59:13PM -0500, Jeff Garzik wrote:
> Alan Cox wrote:
> >
> > > 2.4.2-pre3 doesn't compile with 6pack as a module; I had to disable it;
> > > now it compiles (and so far, works fine).
> >
> > It has a slight dependancy on -ac right now.
> >
> > KMALLOC_MAXSIZE is the alloc size limit - 131072. It checks this as kmalloc
> > now panics if called with an oversize request
>
> Would it be costly/reasonable to have kmalloc -not- panic if given a
> too-large size? Principle of Least Surprises says it should return NULL
> at the very least.

It's on purpose; to find the erroneous drivers.


/David Weinehall
_ _
// David Weinehall <[email protected]> /> Northern lights wander \\
// Project MCA Linux hacker // Dance across the winter sky //
\> http://www.acc.umu.se/~tao/ </ Full colour fire </

2001-02-11 20:15:51

by Jeff Garzik

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

David Weinehall wrote:
>
> On Sun, Feb 11, 2001 at 02:59:13PM -0500, Jeff Garzik wrote:
> > Alan Cox wrote:
> > >
> > > > 2.4.2-pre3 doesn't compile with 6pack as a module; I had to disable it;
> > > > now it compiles (and so far, works fine).
> > >
> > > It has a slight dependancy on -ac right now.
> > >
> > > KMALLOC_MAXSIZE is the alloc size limit - 131072. It checks this as kmalloc
> > > now panics if called with an oversize request
> >
> > Would it be costly/reasonable to have kmalloc -not- panic if given a
> > too-large size? Principle of Least Surprises says it should return NULL
> > at the very least.
>
> It's on purpose; to find the erroneous drivers.

Oh good grief. You will never find all such drivers. Dynamic memory
allocation is by its definition dynamic. Alloc size is often selected
at runtime based on a variety of factors.

printk a message and fail the call. Don't panic.

We have a system in place to notify calls when kmalloc fails -- return
value. Use that, it's what its there for...

Jeff


--
Jeff Garzik | "You see, in this world there's two kinds of
Building 1024 | people, my friend: Those with loaded guns
MandrakeSoft | and those who dig. You dig." --Blondie

2001-02-11 20:26:32

by Manfred Spraul

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

Jeff Garzik wrote:
>
>
> printk a message and fail the call. Don't panic.
>

Perhaps add a compile time warning, similar to __bad_udelay();

The BUG is a bad idea.

--
Manfred

2001-02-11 20:30:33

by Alan

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

> > Would it be costly/reasonable to have kmalloc -not- panic if given a
> > too-large size? Principle of Least Surprises says it should return NULL
> > at the very least.
>
> It's on purpose; to find the erroneous drivers.

Unfortunately Linus forgot to provide a way to check if a kmalloc is too
large so the drivers cannot work around it. Dave put an incredibly ugly
constant assumption in af_unix for this and no doubt more will follow.

So -ac added the constant

Alan

2001-02-11 20:34:12

by Alan

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

> > printk a message and fail the call. Don't panic.
>
> Perhaps add a compile time warning, similar to __bad_udelay();
> The BUG is a bad idea.

They are all dynamic allocations

2001-02-11 20:39:32

by Manfred Spraul

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

Alan Cox wrote:
>
> > > Would it be costly/reasonable to have kmalloc -not- panic if given a
> > > too-large size? Principle of Least Surprises says it should return NULL
> > > at the very least.
> >
> > It's on purpose; to find the erroneous drivers.
>
> Unfortunately Linus forgot to provide a way to check if a kmalloc is too
> large so the drivers cannot work around it. Dave put an incredibly ugly
> constant assumption in af_unix for this and no doubt more will follow.
>
> So -ac added the constant
>
What about removing the BUG?

I means all drivers should be aware that kmalloc() > 16 kB fail quite
often.
kmalloc() over 128 kB always fail.

Do you really prefer if drivers contain a

static inline void* safe_kmalloc(size, flags)
{
if(size > LIMIT)
return NULL;
return kmalloc(size, flags);
}

--
Manfred

2001-02-11 20:44:42

by Alan

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

> Do you really prefer if drivers contain a
>
> static inline void* safe_kmalloc(size, flags)
> {
> if(size > LIMIT)
> return NULL;
> return kmalloc(size, flags);
> }

It isnt that simple. Look at af_unix.c for example. It needs to know the
maximum safe request size to set values up and is prepared to accept
smaller values if that fails

2001-02-11 21:09:49

by Manfred Spraul

[permalink] [raw]
Subject: Re: 2.4.2-pre3 compile error in 6pack.c

Alan Cox wrote:
>
> > Do you really prefer if drivers contain a
> >
> > static inline void* safe_kmalloc(size, flags)
> > {
> > if(size > LIMIT)
> > return NULL;
> > return kmalloc(size, flags);
> > }
>
> It isnt that simple. Look at af_unix.c for example. It needs to know the
> maximum safe request size to set values up and is prepared to accept
> smaller values if that fails
>

Ok, I just downloaded -ac9.

Hmm.
What about removing -16 instead of increasing it to 64?
The slab allocator is perfect for power of 2 allocations!
The slab descriptors are stored outside in seperate buffers.

And why KMALLOC_SIZE/2?
"Keep 2 messages in ..."?


Btw, sock_alloc_send_skb() (net/core.c) still uses the wrong allocation
mode for "size":

GFP_BUFFER both sleeps and uses the atomic queue.

skb = alloc_skb(size, sk->allocation & (~__GFP_WAIT));

--
Manfred