Re: 2.6.8.1-mm2

[Hans Reiser]

Rik van Riel wrote:

>  
>
> > reiser4-perthread-pages.patch
> >    
>
> If a task exits unexpectedly, it will leak the reserved pages.
> This memory leak wants fixing...
>
> Also, why the !in_interrupt() test in perthread_pages_alloc() ?
> Surely this function shouldn't be called from interrupts, since
> it is a general purpose pool of pages.
>
>  
>
> > reiser4-radix-tree-tag.patch
> >    
>
> Just a nitpick here, could we rename PAGECACHE_TAG_FS_SPECIFIC
> to PAGECACHE_TAG_FS_PRIVATE, since we're using the name "private"
> in half a number of other places for the exact same purpose ?
>
>  
>
> > reiser4-radix_tree_lookup_slot.patch
> >    
>
> Having reiserfs dig into the radix tree looks like a layering
> violation to me.  If there is a real need to replace pagecache
> pages with other pages in the radix tree, maybe we should have
> a function to do that in the pagecache code, leaving reiserfs
> to call things at the right abstraction level ?
>
> I see a potential for race conditions when reiserfs changes a
> page which write has just looked up, and what about mmap?
> Even if the code is safe now, this is bound to result in a
> maintenance nightmare down the road.
>
>  
>
> > reiser4-truncate_inode_pages_range.patch
> >    
>
> This has the same race issue as any of the "hole punch"
> patches that have been floating around in the past.  The
> truncate path has some (subtle!) race prevention that
> depends on the nopage functions not searching past i_size,
> but this hole punch code doesn't.
>
> I am not convinced this is SMP safe.
>
> cheers,
>
> Rik
>  
Thanks very much for identifying some races and leaks for us to look 
at.  Zam is the head of our Races and Leaks Department;-), so I will let 
him comment.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/