Re: ramdisk corruption problems - was: RE: pivot_root and initrd kern el panic woes

[Andrea Arcangeli <andrea@suse.de>]

On Fri, Jan 04, 2002 at 11:53:59PM -0800, Andrew Morton wrote:
> Andrea Arcangeli wrote:
> > 
> > I diffed a more advanced version of it. It's not very well tested.
> > 
> > [ your rd.c patch ]
> >
> 
> Your patch is working OK for me.  I made two changes:
> 
> - s/PAGE_SIZE/PAGE_CACHE_SIZE/ in ramdisk_updatepage()

ok

> 
> - I think there's an SMP race in rd_blkdev_pagecache_IO() - it looks up
>   the underlying page in the pagecache and if it is present, it simply
>   proceeds, assuming that the page is uptodate.   But another CPU could have
>   just added the page and may be in the middle of initialising it.
>   So I changed rd_blkdev_pagecache_IO() to always lock the page.  It
>   got simpler.

certainly it makes smp simpler agreed, so also ramdisk_updatepage always
runs on locked pages.

the reason it was not locking down the page is that before the
ramdisk_aops was introduced, a read via /dev/ram? would lock down the
pagecache page, and then start the ll_rw_block on the pagecache page,
but with the page locked, so it would deadlock with an unconditional
grab_cache_page. Now with the ramdisk_aops it should be ok because the physical
address space I/O never uses the ->make_request_fn so it shouldn't
recurse on the page lock any longer.

> Please review - I'm trying to use the rd driver to test the truncate+ENOSPC

your patch seems all right to me now (of course with ramdisk_updatepage
run unconditionally). (I'd only set -ENOMEM in the fast path, so the
oom branch becomes an out of line goto, didn't checked the asm generated
though)

Andrea