Re: [PATCH]: ufs: truncate should allocate block for last byte

From: Andrew Morton
Date: Wed Jun 28 2006 - 15:21:21 EST


On Wed, 28 Jun 2006 19:24:50 +0400
Evgeniy Dushistov <dushistov@xxxxxxx> wrote:

> On Wed, Jun 28, 2006 at 04:50:29AM -0700, Andrew Morton wrote:
> > On Wed, 28 Jun 2006 13:38:51 +0400
> > > + if (unlikely(!page->mapping || !page_has_buffers(page))) {
> > > + unlock_page(page);
> > > + page_cache_release(page);
> > > + goto try_again;/*we really need these buffers*/
> > > + }
> > > +out:
> > > + return page;
> > > +}
> >
> > I think there's a (preexisting) problem here. When one thread is executing
> > ufs_get_locked_page() while a second thread is running truncate().
> >
> > If truncate got to the page first, truncate_complete_page() will mark the
> > page !uptodate and will later unlock it. Now this function gets the page
> > lock and emits a printk (bad) and assumes -EIO (worse).
> >
> > That scenario might not be possible because of i_mutex coverage, dunno.
> >
> I suppose this is possible because of
> a)page may be mapped to hole
> b)sys_msync doesn't use i_mutex
> c)in case of block allocation we can call ufs_get_locked_page

OK.

> > But if it _is_ possible, it can be simply fixed by doing
> >
> But you added such check "!page->mapping" into ufs_get_locked_page,
> is it not enough?

That is what I was proposing, here:

> > lock_page(page);
> > + if (page->mapping == NULL) {
> > + /* truncate() got there first */
> > + page_cache_release(page);
> > + goto try_again;
> > + }

-
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/