From: Matthew Wilcox on
On Tue, Jun 01, 2010 at 09:49:51AM -0400, Chris Mason wrote:
> > I agree that a block based retry would close all the holes ... it just
> > doesn't look elegant to me that the fs will already be repeating the I/O
> > if it changed the page and so will block.
>
> We might not ever repeat the IO. We might change the page, write it,
> change it again, truncate the file and toss the page completely.

Why does it matter that it was never written in that case?

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Chris Mason on
On Tue, Jun 01, 2010 at 10:29:30AM -0600, Matthew Wilcox wrote:
> On Tue, Jun 01, 2010 at 09:49:51AM -0400, Chris Mason wrote:
> > > I agree that a block based retry would close all the holes ... it just
> > > doesn't look elegant to me that the fs will already be repeating the I/O
> > > if it changed the page and so will block.
> >
> > We might not ever repeat the IO. We might change the page, write it,
> > change it again, truncate the file and toss the page completely.
>
> Why does it matter that it was never written in that case?

It matters is the storage layer is going to wait around for the block to
be written again with a correct crc.

Unless there is trim + DIF, but that's a lot of plates to spin just for
a basic implementation.

-chris

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: James Bottomley on
On Tue, 2010-06-01 at 12:47 -0400, Chris Mason wrote:
> On Tue, Jun 01, 2010 at 10:29:30AM -0600, Matthew Wilcox wrote:
> > On Tue, Jun 01, 2010 at 09:49:51AM -0400, Chris Mason wrote:
> > > > I agree that a block based retry would close all the holes ... it just
> > > > doesn't look elegant to me that the fs will already be repeating the I/O
> > > > if it changed the page and so will block.
> > >
> > > We might not ever repeat the IO. We might change the page, write it,
> > > change it again, truncate the file and toss the page completely.
> >
> > Why does it matter that it was never written in that case?
>
> It matters is the storage layer is going to wait around for the block to
> be written again with a correct crc.

Actually, I wasn't advocating that. I think block should return a guard
mismatch error. I think somewhere in filesystem writeout is the place
to decide whether the error was self induced or systematic. For self
induced errors (as long as we can detect them) I think we can just
forget about it ... if the changed page is important, the I/O request
gets repeated (modulo the problem of too great a frequency of changes
leading to us never successfully writing it) or it gets dropped because
the file was truncated or the data deleted for some other reason.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Chris Mason on
On Tue, Jun 01, 2010 at 04:54:53PM +0000, James Bottomley wrote:
> On Tue, 2010-06-01 at 12:47 -0400, Chris Mason wrote:
> > On Tue, Jun 01, 2010 at 10:29:30AM -0600, Matthew Wilcox wrote:
> > > On Tue, Jun 01, 2010 at 09:49:51AM -0400, Chris Mason wrote:
> > > > > I agree that a block based retry would close all the holes ... it just
> > > > > doesn't look elegant to me that the fs will already be repeating the I/O
> > > > > if it changed the page and so will block.
> > > >
> > > > We might not ever repeat the IO. We might change the page, write it,
> > > > change it again, truncate the file and toss the page completely.
> > >
> > > Why does it matter that it was never written in that case?
> >
> > It matters is the storage layer is going to wait around for the block to
> > be written again with a correct crc.
>
> Actually, I wasn't advocating that. I think block should return a guard
> mismatch error. I think somewhere in filesystem writeout is the place
> to decide whether the error was self induced or systematic.

In that case the io error goes to the async page writeback bio-endio
handlers. We don't have a reference on the inode and no ability to
reliably restart the IO, but we can set a bit on the address space
indicating that somewhere, sometime in the past we had an IO error.

> For self
> induced errors (as long as we can detect them) I think we can just
> forget about it ... if the changed page is important, the I/O request
> gets repeated (modulo the problem of too great a frequency of changes
> leading to us never successfully writing it) or it gets dropped because
> the file was truncated or the data deleted for some other reason.

Sorry, how can we tell the errors that are self induced from the evil
bit flipping cable induced errors?

-chris

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nick Piggin on
On Tue, Jun 01, 2010 at 02:09:05PM -0400, Chris Mason wrote:
> On Tue, Jun 01, 2010 at 04:54:53PM +0000, James Bottomley wrote:
> > On Tue, 2010-06-01 at 12:47 -0400, Chris Mason wrote:
> > > On Tue, Jun 01, 2010 at 10:29:30AM -0600, Matthew Wilcox wrote:
> > > > On Tue, Jun 01, 2010 at 09:49:51AM -0400, Chris Mason wrote:
> > > > > > I agree that a block based retry would close all the holes ... it just
> > > > > > doesn't look elegant to me that the fs will already be repeating the I/O
> > > > > > if it changed the page and so will block.
> > > > >
> > > > > We might not ever repeat the IO. We might change the page, write it,
> > > > > change it again, truncate the file and toss the page completely.
> > > >
> > > > Why does it matter that it was never written in that case?
> > >
> > > It matters is the storage layer is going to wait around for the block to
> > > be written again with a correct crc.
> >
> > Actually, I wasn't advocating that. I think block should return a guard
> > mismatch error. I think somewhere in filesystem writeout is the place
> > to decide whether the error was self induced or systematic.
>
> In that case the io error goes to the async page writeback bio-endio
> handlers. We don't have a reference on the inode and no ability to
> reliably restart the IO, but we can set a bit on the address space
> indicating that somewhere, sometime in the past we had an IO error.
>
> > For self
> > induced errors (as long as we can detect them) I think we can just
> > forget about it ... if the changed page is important, the I/O request
> > gets repeated (modulo the problem of too great a frequency of changes
> > leading to us never successfully writing it) or it gets dropped because
> > the file was truncated or the data deleted for some other reason.
>
> Sorry, how can we tell the errors that are self induced from the evil
> bit flipping cable induced errors?

Block layer should retry it with bounce pages. That would be a lot nicer
than forcing all upper layers to avoid the problem.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/