Re: Oops in a driver while using SLUB as a SLAB allocator

[Hugh Dickins <hugh@veritas.com>]

On Mon, 25 Jun 2007, Christoph Lameter wrote:
> On Mon, 25 Jun 2007, Hugh Dickins wrote:
> > 
> > I didn't claim that flush_dcache_page(virt_to_page(virt)) is not expected
> > to work.  I claim that flush_dcache_page is expected to be a noop rather
> > than an oops on a kmalloced page.
> 
> There are no kmalloced pages. There is only kmalloced memory. You allocate 
> pages from the page allocator. Its a layering violation to expect a page
> struct operation on a slab object to work.

Oh, thanks so much for resolving my confusion on that.

> 
> It is not okay to use a page cache function on a slab object. The slab 
> object does not fulfill the alignment requirements of a page cache page
> nor does it  have a compatible page struct content as a page cache page. 
> This can only cause trouble.
> 
> The problem is that the code is allocating some slab memory and then 
> determines the page struct pointer and then hands it back to the DMA 
> layer?
> 
> > > Ok. I think your patch is fine as a quick fix for 2.6.22. I am a bit 
> > > uneasy with that given that its in such a broadly used function while its 
> > > only use is to enable flush_dcache_page to work. But we need the general 
> > > issue taken care of after 2.6.22.
> > 
> > What general issue?
> 
> How to flush slab objects in a reliable way.
> 
> > Please see Documentation/cachetlb.txt: flush_dcache_page is about
> > pagecache pages mapped into userspace.  We don't use kmalloc for those,
> > but we do sometimes need to flush_dcache_page in places which commonly
> > deal with pagecache pages, but sometimes handle kmalloc'ed buffers too.
> > Luckily we don't have to deal with buffers in which the first page is
> > kmalloced and the next comes from pagecache.
> 
> This gets crazier and crazier. flush_dcache_page is for pages not for
> allocated buffers via kmalloc. So this has nothing to do with any need
> to flush slab objects?
> 
> Sometimes we do this and then we do that? So someone played loose
> ball with the slab, was successful and that makes it right now?
> 
> We need the VM_BUG_ON in include/linux/mm.h to stop this nonsense.
> 
> The "sometimes we have kmalloced buffers" locations need to be fixed.

I've said enough, I'd better leave it to others to deter you or not
from fiddling around pointlessly here.

But Andrew, please cancel my Acked-by to mm's
add-vm_bug_on-in-case-someone-uses-page_mapping-on-a-slab-page.patch

Thanks,
Hugh
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/