Re: 2.6.2-rc2 nfsd+xfs spins in i_size_read()

[Andrew Morton <akpm@osdl.org>]

Miquel van Smoorenburg <miquels@cistron.nl> wrote:
>
> On Sat, 31 Jan 2004 00:13:16, Andrew Morton wrote:
> > Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > On Fri, Jan 30, 2004 at 02:34:59PM -0800, Andrew Morton wrote:
> > > > If two CPUs hit i_size_write() at the same time we have a bug.  That
> > > > function requires that the caller provide external serialisation, via i_sem.
> > > 
> > > O_APPEND|O_DIRECT writes could do that under XFS..
> > 
> > Sigh.
> >
> > diff -puN mm/filemap.c~i_size_write-check mm/filemap.c
> > --- 25/mm/filemap.c~i_size_write-check	Fri Jan 30 15:10:23 2004
> > +++ 25-akpm/mm/filemap.c	Fri Jan 30 15:11:41 2004
> > @@ -2010,3 +2010,18 @@ out:
> >  }
> >  
> >  EXPORT_SYMBOL_GPL(generic_file_direct_IO);
> > +
> > +void i_size_write_check(struct inode *inode)
> > +{
> > +	static int count = 0;
> > +
> > +	if (down_trylock(&inode->i_sem) == 0) {
> > +		if (count < 10) {
> 
> You want to set this to 100 at least, since at boot time the message
> happens _often_ even without XFS.

fun.

> It's caused by sysfs:

OK, sysfs_symlink() needs i_sem.

> Also, bd_set_size runs unlocked:

I don't expect we'll ever see a race over the blockdev's i_size in there. 
It might be simplest to just do a straight assignment.  I'll work that with
viro.

> But the XFS problem appears to be vn_revalidate which calls i_size_write()
> without holding i_sem:

There's your bug.