Re: SLAB vs. pci_alloc_xxx in usb-uhci patch

[David Brownell <david-b@pacbell.net>]

> > (1) CONFIG_SLAB_DEBUG breaks the documented
> > requirement that the slab cache return adequately aligned
> > data ...
> 
> adequately aligned for the _cpu_, not for some controllers. It's neither
> documented that HW_CACHEALIGN aligns to 16 byte boundaries

It's documented in mm/slab.c (line 612 in my ac7+tweaks).
"Aligns to hardware cache line."  The only cacheline define
I've found is in <asm/cache.h> for L1_CACHE_BYTES,
which is often more than 16 (not on x86).

If you don't like the patch I forwarded, then please submit
one that changes the kmem_cache_create() API spec ...
one or the other is needed.  (I'd not change that API!!)


> nor that kmalloc uses HW_CACHEALIGN. ...
>
> > + /* redzoning would break cache alignment requirements */
> > + if (flags & SLAB_HWCACHE_ALIGN)
> > +  flags &= ~SLAB_RED_ZONE;
> 
> The problem is that you've just disabled red zoning for kmalloc.   And
> kmalloc is the only case where redzoning is important: ...

If kmalloc wants to get auto redzoning, then I think it shouldn't be
using SLAB_HWCACHE_ALIGN when CONFIG_SLAB_DEBUG.
That'd be another simple fix.  (Or, make it use some new flag that's
just a performance hint that can be ignored for debugging.)

 
> I think everyone agrees that (2) correct fix.

I was saying that there were two bugs (two fixes needed), and you're
saying that there's only one ... despite the evidence of the API spec.
But you could persuade me there's a third bug:  kmalloc misuse of
that kmem_cache API.


> I see 2 temporary workarounds: either your patch or
> 
> + #ifdef CONFIG_SLAB_DEBUG
> + #error
> + #endif
> 
> in uhci.c.

Better in linux/drivers/usb/Config.in instead.  All the host controller
drivers rely on the kmem_cache_create() API spec to be followed.
(Even the OHCI driver, when using kmalloc not kmem_cache.)

- Dave