On Thu, Jun 30, 2011 at 12:00:13PM +1000, Dave Chinner wrote:
> > - if (iohead)
> > - xfs_cancel_ioend(iohead);
> > -
> > - if (err == -EAGAIN)
> > - goto redirty;
> > -
>
> Should this EAGAIN handling be dealt with in the removing-the-non-
> blocking-mode patch?
Probably.
> > + ret = write_cache_pages(mapping, wbc, __xfs_vm_writepage, &ctx);
> > +
> > + if (ctx.iohead) {
> > + if (ret)
> > + xfs_cancel_ioend(ctx.iohead);
> > + else
> > + xfs_submit_ioend(wbc, ctx.iohead);
> > + }
>
> I think this error handling does not work. If we have put pages into
> the ioend (i.e. successful ->writepage calls) and then have a
> ->writepage call fail, we'll get all the pages under writeback (i.e.
> those on the ioend) remain in that state, and not ever get written
> back (so move into the clean state) or redirtied (so written again
> later)
>
> xfs_cancel_ioend() was only ever called for the first page sent down
> to ->writepage, and on error that page was redirtied separately.
> Hence it doesn't handle this case at all as it never occurs in the
> existing code.
>
> I'd suggest that regardless of whether an error is returned here,
> the existence of ctx.iohead indicates a valid ioend that needs to be
> submitted....
Ok. That would also solve the problem of the trylock failures. I'll
see how we can deal with it nicely.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
