ext3: prevent reread after write IO error

This patch fixes the similar bug fixed by commit 95450f5a.

If a directory is modified, its data block is journaled as metadata and finally written back to the right place. Now, we assume a transient write erorr happens on that writeback. Uptodate flag of the buffer is cleared by write error, so next access on the buffer causes a reread from disk. This means it breaks the filesystems consistency.

This patch fixes the similar bug fixed by commit 95450f5a.

If a directory is modified, its data block is journaled as metadata and finally written back to the right place. Now, we assume a transient write erorr happens on that writeback. Uptodate flag of the buffer is cleared by write error, so next access on the buffer causes a reread from disk. This means it breaks the filesystems consistency.

To prevent old directory data from being reread, this patch set uptodate flag again in the case of after write error before issuing the read operation. The write error on the directory's data block is detected at the time of journal checkpointing or discarded if a rewrite by another modification succeeds, so no problem.

Similarly, this kind of consistency breakage can be caused by a transient write error on a bitmap block.

I tested this patch by using fault injection approach.

By the way, I think the right fix is to keep uptodate flag on write error, but it gives a big impact. We have to confirm whether over 200 buffer_uptodate's are used for real uptodate check or write error check. For now, I adopt the quick-fix solution.

Signed-off-by Hidehiro Kawai <[email protected]>

---
 fs/ext3/balloc.c |   12 ++++++++++++
 fs/ext3/inode.c  |   13 +++++++++++++
 fs/ext3/namei.c  |   15 ++++++++++++++-
 3 files changed, 39 insertions(+), 1 deletions(-)

fs/ext3/balloc.c +12 -0

index 27967f9..5dc5ccf 100644
@@ -156,6 +156,18 @@ read_block_bitmap(struct super_block *sb, unsigned int block_group)
 	if (likely(bh_uptodate_or_lock(bh)))
 		return bh;
 
+	/*
+	 * uptodate flag may have been cleared by a previous (transient)
+	 * write IO error.  In this case, we don't want to reread its
+	 * old on-disk data.  Actually the buffer has the latest data,
+	 * so set uptodate flag again.
+	 */
+	if (buffer_write_io_error(bh)) {
+		set_buffer_uptodate(bh);
+		unlock_buffer(bh);
+		return bh;
+	}
+
 	if (bh_submit_read(bh) < 0) {
 		brelse(bh);
 		ext3_error(sb, __func__,

fs/ext3/inode.c +13 -0

index 455e6e6..67d7849 100644
@@ -1077,10 +1077,23 @@ struct buffer_head *ext3_bread(handle_t *handle, struct inode *inode,
 		return bh;
 	if (buffer_uptodate(bh))
 		return bh;
+
+	/*
+	 * uptodate flag may have been cleared by a previous (transient)
+	 * write IO error.  In this case, we don't want to reread its
+	 * old on-disk data.  Actually the buffer has the latest data,
+	 * so set uptodate flag again.
+	 */
+	if (buffer_write_io_error(bh)) {
+		set_buffer_uptodate(bh);
+		return bh;
+	}
+
 	ll_rw_block(READ_META, 1, &bh);
 	wait_on_buffer(bh);
 	if (buffer_uptodate(bh))
 		return bh;
+
 	put_bh(bh);
 	*err = -EIO;
 	return NULL;

fs/ext3/namei.c +14 -1

index 7b0e44f..7ed8e45 100644
@@ -909,7 +909,20 @@ restart:
 				num++;
 				bh = ext3_getblk(NULL, dir, b++, 0, &err);
 				bh_use[ra_max] = bh;
-				if (bh)
+				if (!bh || buffer_uptodate(bh))
+					continue;
+
+				/*
+				 * uptodate flag may have been cleared by a
+				 * previous (transient) write IO error.  In
+				 * this case, we don't want to reread its
+				 * old on-disk data.  Actually the buffer
+				 * has the latest data, so set uptodate flag
+				 * again.
+				 */
+				if (buffer_write_io_error(bh))
+					set_buffer_uptodate(bh);
+				else
 					ll_rw_block(READ_META, 1, &bh);
 			}
 		}

Signature · 8 lines

1.6.0.3



-- 
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center

Quoted 4 lines

By the way, I think the right fix is to keep uptodate flag on write error, but it gives a big impact. We have to confirm whether over 200 buffer_uptodate's are used for real uptodate check or write error check. For now, I adopt the quick-fix solution.

Yes that needs to be solved. I also looked into it and it's too much work
to do it in a one big sweep. But I think we could do the conversion filesystem by filesystem - see below.
Admittedly, I don't like your solution very much. It looks strange to
check write_io_error when *reading* the buffer and I'm afraid we could introduce bugs e.g. by clearing write_io_error bit so that ext3_bread would then fail to detect the error condition or by some other code deciding to read the buffer from disk via other function than just ext3_bread. So I think it would be much better to set back uptodate flag shortly after the

failed write or not clear it at all.
Now here's how I think we could achieve that without having to change all
other filesystems: We could create a new superblock flag which would mean that "this filesystem handles write_io_error and doesn't want to clear uptodate flag". Filesystems with this capability would set this flag when calling get_sb_bdev. And if write IO fails we check this flag (via bh->b_bdev->bd_super->s_flags) and clear / not clear uptodate flag accordingly. What do you think?

Quoted 9 lines · Hidehiro Kawai

This patch fixes the similar bug fixed by commit 95450f5a.

If a directory is modified, its data block is journaled as metadata and finally written back to the right place. Now, we assume a transient write erorr happens on that writeback. Uptodate flag of the buffer is cleared by write error, so next access on the buffer causes a reread from disk. This means it breaks the filesystems consistency.

Quoted 8 lines

To prevent old directory data from being reread, this patch set uptodate flag again in the case of after write error before issuing the read operation. The write error on the directory's data block is detected at the time of journal checkpointing or discarded if a rewrite by another modification succeeds, so no problem.

Similarly, this kind of consistency breakage can be caused by a transient write error on a bitmap block.

Good catch, that's indeed a problem.

Quoted 2 lines

I tested this patch by using fault injection approach.

Quoted 4 lines

By the way, I think the right fix is to keep uptodate flag on write error, but it gives a big impact. We have to confirm whether over 200 buffer_uptodate's are used for real uptodate check or write error check. For now, I adopt the quick-fix solution.

Yes that needs to be solved. I also looked into it and it's too much work
to do it in a one big sweep. But I think we could do the conversion filesystem by filesystem - see below.

Admittedly, I don't like your solution very much. It looks strange to
check write_io_error when *reading* the buffer and I'm afraid we could introduce bugs e.g. by clearing write_io_error bit so that ext3_bread would then fail to detect the error condition or by some other code deciding to read the buffer from disk via other function than just ext3_bread. So I think it would be much better to set back uptodate flag shortly after the failed write or not clear it at all.
Now here's how I think we could achieve that without having to change all

other filesystems: We could create a new superblock flag which would mean that "this filesystem handles write_io_error and doesn't want to clear uptodate flag". Filesystems with this capability would set this flag when calling get_sb_bdev. And if write IO fails we check this flag (via bh->b_bdev->bd_super->s_flags) and clear / not clear uptodate flag accordingly. What do you think?
I know it's more work than your quick fix but it should fix all these
problems for ext3 once for all and it would be much cleaner...

Quoted 94 lines

Signed-off-by Hidehiro Kawai <[email protected]>

---
 fs/ext3/balloc.c |   12 ++++++++++++
 fs/ext3/inode.c  |   13 +++++++++++++
 fs/ext3/namei.c  |   15 ++++++++++++++-
 3 files changed, 39 insertions(+), 1 deletions(-)

fs/ext3/balloc.c +12 -0

index 27967f9..5dc5ccf 100644
@@ -156,6 +156,18 @@ read_block_bitmap(struct super_block *sb, unsigned int block_group)
 	if (likely(bh_uptodate_or_lock(bh)))
 		return bh;
 
+	/*
+	 * uptodate flag may have been cleared by a previous (transient)
+	 * write IO error.  In this case, we don't want to reread its
+	 * old on-disk data.  Actually the buffer has the latest data,
+	 * so set uptodate flag again.
+	 */
+	if (buffer_write_io_error(bh)) {
+		set_buffer_uptodate(bh);
+		unlock_buffer(bh);
+		return bh;
+	}
+
 	if (bh_submit_read(bh) < 0) {
 		brelse(bh);
 		ext3_error(sb, __func__,

fs/ext3/inode.c +13 -0

index 455e6e6..67d7849 100644
@@ -1077,10 +1077,23 @@ struct buffer_head *ext3_bread(handle_t *handle, struct inode *inode,
 		return bh;
 	if (buffer_uptodate(bh))
 		return bh;
+
+	/*
+	 * uptodate flag may have been cleared by a previous (transient)
+	 * write IO error.  In this case, we don't want to reread its
+	 * old on-disk data.  Actually the buffer has the latest data,
+	 * so set uptodate flag again.
+	 */
+	if (buffer_write_io_error(bh)) {
+		set_buffer_uptodate(bh);
+		return bh;
+	}
+
 	ll_rw_block(READ_META, 1, &bh);
 	wait_on_buffer(bh);
 	if (buffer_uptodate(bh))
 		return bh;
+
 	put_bh(bh);
 	*err = -EIO;
 	return NULL;

fs/ext3/namei.c +14 -1

index 7b0e44f..7ed8e45 100644
@@ -909,7 +909,20 @@ restart:
 				num++;
 				bh = ext3_getblk(NULL, dir, b++, 0, &err);
 				bh_use[ra_max] = bh;
-				if (bh)
+				if (!bh || buffer_uptodate(bh))
+					continue;
+
+				/*
+				 * uptodate flag may have been cleared by a
+				 * previous (transient) write IO error.  In
+				 * this case, we don't want to reread its
+				 * old on-disk data.  Actually the buffer
+				 * has the latest data, so set uptodate flag
+				 * again.
+				 */
+				if (buffer_write_io_error(bh))
+					set_buffer_uptodate(bh);
+				else
 					ll_rw_block(READ_META, 1, &bh);
 			}
 		}

Signature · 8 lines

1.6.0.3



-- 
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

We also need to remove some ClearPageUptodate calls I think (similar issues), so keep those in mind too. Unfortunately it looks like there are also a lot of filesystem specific tests of PageUptodate... but you could also move those under the new compatibility s_flag.

I don't know of a really good way to inject and test filesystem errors. Make request failures causes most fs to quickly go readonly or have bigger problems. If you're careful like try to only fail read IOs for data, or only fail write IOs not involved in integrity or journal operations, then test programs just tend to abort pretty quickly. Does anyone know of anything more systematic?

Quoted 11 lines · Nick Piggin

We also need to remove some ClearPageUptodate calls I think (similar issues), so keep those in mind too. Unfortunately it looks like there are also a lot of filesystem specific tests of PageUptodate... but you could also move those under the new compatibility s_flag.

I don't know of a really good way to inject and test filesystem errors. Make request failures causes most fs to quickly go readonly or have bigger problems. If you're careful like try to only fail read IOs for data, or only fail write IOs not involved in integrity or journal operations, then test programs just tend to abort pretty quickly. Does anyone know of anything more systematic?

This might be a good time to bring up IO error behaviour again. I got into some debates I think on Andi's hwpoison thread a while back, but probably not appropriate thread to find a real solution to this.

The problem we have now is that IO error semantics are not well defined. It is hard to even enumerate all the issues.

read IOs
how to retry? appropriate defaults should happen at the block layer I think. Should retry behaviour be tunable by the mm/fs, or should that be coded explicitly as submission retry loops? Either way does imply there is either similar defaults for all types (or maybe classes) of drivers, or some way to query/set this.

It would be nice to be able to set fs/driver behaviour from userspace too, in a generic (not driver or fs specific way). But defaults should be reasonable and similar between all, I guess.

write IOs
This is more interesting. How to handle write IO errors. In my opinion we must not invalidate the data before an IO error is returned to somebody (whether it be fsync or a synchronous write syscall).

Any
earlier and the app just gets RAW consistency randomly violated. And I think it is important to treat IO errors as transparently as possible until the error can be detected.

I happen to think that actually we should go further and not invalidate the data at all. This makes implementation simpler, and also allows us to retry writes like we can retry reads. It's also problematic to throw out errors at that point because *sync syscalls coming from elsewhere could result in loss of error reporting (think, sys_sync).

If we go this way, we probably need another syscall and fs helper call to invalidate the dirty data when we give up on retries. truncate_range probably not appropriate because it is much harder to implement and maybe we want to try to get at the most recent data that is on disk.

Also do we need to think about O_SYNC or -o sync type of writes that are implemented via writeback cache? We could invalidate the dirtied cache ASAP, which would leave a window where a concurrent read can see first new, then old data. It would also kind of break the above scheme in case the pagecache was already dirty via a descriptor without O_SYNC. It might just make sense to leave the pagecache dirty. Either way it should be documented I think.

Do we even care enough to bother thinking about this now? (serious question)

Quoted 9 lines · Nick Piggin

The problem we have now is that IO error semantics are not well defined. It is hard to even enumerate all the issues.

read IOs
how to retry? appropriate defaults should happen at the block layer I think. Should retry behaviour be tunable by the mm/fs, or should that be coded explicitly as submission retry loops? Either way does imply there is either similar defaults for all types (or maybe classes) of drivers, or some way to query/set this.

It's more complex than that - there are classes of errors to consider as well. e.g transient vs permanent.

Transient is from stuff like FC path failures - failover can take up to 240s to occur, and then the IO will generally complete successfully. Permanent errors are those that involve data loss e.g bad sectors on single disks or on degraded RAID devices.

Quoted 3 lines

It would be nice to be able to set fs/driver behaviour from userspace too, in a generic (not driver or fs specific way). But defaults should be reasonable and similar between all, I guess.

I don't think generic handling is really possible - filesystems may have different ways of recovering e.g. duplicate copies of data or

metadata or internal ECC that can be used to recovery the bad region. Also, depending where the error occurs, the filesystem might need to shutdown to be repaired....

Quoted 4 lines

write IOs
This is more interesting. How to handle write IO errors. In my opinion we must not invalidate the data before an IO error is returned to somebody (whether it be fsync or a synchronous write syscall).

We already pass the error via mapping_set_error() calls when the error occurs and checking in it filemap_fdatawait_range(). However, where we check the error we've lost all context and what range the error occurred on. I don't see any easy way to track such an error for later invalidation except maybe by a new radix tree tag. That would allow later invalidation of only the specific range the error was reported from.

Quoted 11 lines

Any
earlier and the app just gets RAW consistency randomly violated. And I think it is important to treat IO errors as transparently as possible until the error can be detected.

I happen to think that actually we should go further and not invalidate the data at all. This makes implementation simpler, and also allows us to retry writes like we can retry reads. It's also problematic to throw out errors at that point because *sync syscalls coming from elsewhere could result in loss of error reporting (think, sys_sync).

The worst problem with this is what happens when you can't write back to the filesystem because of IO errors, but you still allow more incoming writes? It's not far from IO error to running out of memory and deadlocking....

Quoted 4 lines

If we go this way, we probably need another syscall and fs helper call to invalidate the dirty data when we give up on retries. truncate_range probably not appropriate because it is much harder to implement and maybe we want to try to get at the most recent data that is on disk.

First we need to track what needs invalidating...

Quoted 7 lines

Also do we need to think about O_SYNC or -o sync type of writes that are implemented via writeback cache? We could invalidate the dirtied cache ASAP, which would leave a window where a concurrent read can see first new, then old data. It would also kind of break the above scheme in case the pagecache was already dirty via a descriptor without O_SYNC. It might just make sense to leave the pagecache dirty. Either way it should be documented I think.

How to handle this comes down to the type of error that occurred. In the case of permanent error, the second read after the invalidation probably should return EIO because you have no idea whether what is on disk is the old, the new, some combination of the two or some other random or stale garbage....

Quoted 1 line

Do we even care enough to bother thinking about this now? (serious question)

It's a damn hard problem and many of the details are filesystem specific. However, if we want high grade reliability from our systems then we have to tackle these problems at some point in time.

FWIW, I started to document some of what I've just been talking (from a XFS metadata reliability context) about a year and a half ago. The relevant section is here:

http://xfs.org/index.php/Reliable_Detection_and_Repair_of_Metadata_Corruption#Exception_Handling

Though the entire page is probably somewhat relevant. I only got as far as documenting methods for handling transient and permanent read errors, and the TODO includes handling:

- Transient write error
- Permanent write error
- Corrupted data on read
- Corrupted data on write (detected during guard calculation)

- I/O timeouts

- Memory corruption

Quoted 14 lines · Dave Chinner

Quoted 9 lines · Nick Piggin

The problem we have now is that IO error semantics are not well defined. It is hard to even enumerate all the issues.

read IOs
how to retry? appropriate defaults should happen at the block layer I think. Should retry behaviour be tunable by the mm/fs, or should that be coded explicitly as submission retry loops? Either way does imply there is either similar defaults for all types (or maybe classes) of drivers, or some way to query/set this.

It's more complex than that - there are classes of errors to consider as well. e.g transient vs permanent.

Quoted 4 lines

Transient is from stuff like FC path failures - failover can take up to 240s to occur, and then the IO will generally complete successfully. Permanent errors are those that involve data loss e.g bad sectors on single disks or on degraded RAID devices.

Yes. Is this something that should be visible above the block layer though? If it is known transient, should it remain uncompleted until it is successful?

Known permanent errors yes could avoid any need for retries. Leaving cases where the lower layers don't really know (in which case we'd maybe want to leave it to userspace or a userspace-set policy).

Quoted 6 lines

Quoted 3 lines

It would be nice to be able to set fs/driver behaviour from userspace too, in a generic (not driver or fs specific way). But defaults should be reasonable and similar between all, I guess.

I don't think generic handling is really possible - filesystems may have different ways of recovering e.g. duplicate copies of data or

For write errors, you could also do block re-allocation, which would be fun.

Quoted 3 lines

metadata or internal ECC that can be used to recovery the bad region. Also, depending where the error occurs, the filesystem might need to shutdown to be repaired....

Definitely there will be filesystem specific issues. But I mean that some common things could be specified (like how long / how many times to retry failed requests).

Quoted 5 lines

Quoted 4 lines

write IOs
This is more interesting. How to handle write IO errors. In my opinion we must not invalidate the data before an IO error is returned to somebody (whether it be fsync or a synchronous write syscall).

Quoted 7 lines

We already pass the error via mapping_set_error() calls when the error occurs and checking in it filemap_fdatawait_range(). However, where we check the error we've lost all context and what range the error occurred on. I don't see any easy way to track such an error for later invalidation except maybe by a new radix tree tag. That would allow later invalidation of only the specific range the error was reported from.

If we always leave the error pages / buffers as dirty and uptodate, then we can walk the radix tree dirty bits. IO errors are only really reported by syncing calls anyway which walk dirty bits already.

If we wanted a purely querying syscall, it probably doesn't need to so so performance critical as to require a new tag rather than just checking PageError on the dirty pages.

Quoted 12 lines

Quoted 11 lines

Any
earlier and the app just gets RAW consistency randomly violated. And I think it is important to treat IO errors as transparently as possible until the error can be detected.

I happen to think that actually we should go further and not invalidate the data at all. This makes implementation simpler, and also allows us to retry writes like we can retry reads. It's also problematic to throw out errors at that point because *sync syscalls coming from elsewhere could result in loss of error reporting (think, sys_sync).

Quoted 4 lines

The worst problem with this is what happens when you can't write back to the filesystem because of IO errors, but you still allow more incoming writes? It's not far from IO error to running out of memory and deadlocking....

Again, keeping pages dirty so we'll start synchronous dirty pagecache throttling eventually.

That could cause problems of its own as well, but I don't know what else we can do. I don't think we can throw out the dirty data by default (the errors might be transient). It could be a policy, maybe.

Quoted 6 lines

Quoted 4 lines

If we go this way, we probably need another syscall and fs helper call to invalidate the dirty data when we give up on retries. truncate_range probably not appropriate because it is much harder to implement and maybe we want to try to get at the most recent data that is on disk.

First we need to track what needs invalidating...

Well by this I just mean the dirty, unwritten pagecache and its associated fs private structures. For errors in filesystem metadata yes it is a lot harder. I guess filesystems simply need to check and handle errors on a case by case basis.

Quoted 8 lines

Quoted 7 lines

Also do we need to think about O_SYNC or -o sync type of writes that are implemented via writeback cache? We could invalidate the dirtied cache ASAP, which would leave a window where a concurrent read can see first new, then old data. It would also kind of break the above scheme in case the pagecache was already dirty via a descriptor without O_SYNC. It might just make sense to leave the pagecache dirty. Either way it should be documented I think.

Quoted 5 lines

How to handle this comes down to the type of error that occurred. In the case of permanent error, the second read after the invalidation probably should return EIO because you have no idea whether what is on disk is the old, the new, some combination of the two or some other random or stale garbage....

I'm not sure if that is important because you would have the same problems if the read was not preceded by a write (or if the write came from previous boot, or a different machine etc).

If we want to catch IO errors not detected by the block layer, it really needs a complete solution, in the fs.

Quoted 5 lines

FWIW, I started to document some of what I've just been talking (from a XFS metadata reliability context) about a year and a half ago. The relevant section is here:

http://xfs.org/index.php/Reliable_Detection_and_Repair_of_Metadata_Corruption#Exception_Handling

OK, interesting. Yes a document is needed.

Quoted 8 lines

Though the entire page is probably somewhat relevant. I only got as far as documenting methods for handling transient and permanent read errors, and the TODO includes handling:

- Transient write error
- Permanent write error
- Corrupted data on read
- Corrupted data on write (detected during guard calculation)

We do want to start by making this as _simple_ as possible. Even the existing rudimentary error reporting by the block layer is not used in a consistent way (or at all, in many cases).

So I think squashing corrupted data errors into transient/permanent errors (at least to start with) could be a good idea.

Quoted 1 line

- I/O timeouts

Different from transient/permanent error cases?

Quoted 1 line

- Memory corruption

Yes this needs support, which I've talked about in hwpoison discussions. Currently (or last time I checked) it just causes corrupted dirty pagecache to appear as an IO error. IMO this is wrong -- the fs or the app might retry the write, or try to re-allocate things and write that data elsewhere in the case of EIO, which is totally wrong for memory corruption.

For write errors, you could also do block re-allocation, which would be fun.

Hi,

Quoted 2 lines · Nick Piggin

For write errors, you could also do block re-allocation, which would be fun.

Yes it would. (-:

FWIW, Windows does this with Microsoft's NTFS driver. When a write fails due to a bad block, the block is marked as bad (recorded in the bad cluster list and marked as allocated in the in-use bitmap so no-one tries to allocate it), a new block is allocated, inode metadata is updated to reflect the change in the logical to physical block map of the file the block belongs to, and the write is then re-tried to its new location.

I have never bothered implementing it in NTFS on Linux partially because there doesn't seem any obvious way to do it inside the file system. I think the VFS and/or the block layer would have to offer help there in some way. What I mean for example is that if ->writepage fails then the failure is only detected inside the asynchronous i/o completion handler at which point the page is not locked any more, it is marked as being under writeback, and we are in IRQ context (or something) and thus it is not easy to see how we can from there get to doing all the above needed actions that require memory allocations, disk i/o, etc... I suppose a separate thread could do it where we just schedule the work to be done. But problem with that is that that work later on might fail so we can't simply

Best regards,

Anton

Quoted 33 lines · Anton Altaparmakov

Hi,

Quoted 4 lines · Nick Piggin

For write errors, you could also do block re-allocation, which would be fun.

Yes it would.  (-:

FWIW, Windows does this with Microsoft's NTFS driver.  When a write fails due to a bad block, the block is marked as bad (recorded in the bad cluster list and marked as allocated in the in-use bitmap so no-one tries to allocate it), a new block is allocated, inode metadata is updated to reflect the change in the logical to physical block map of the file the block belongs to, and the write is then re-tried to its new location.

I have never bothered implementing it in NTFS on Linux partially because there doesn't seem any obvious way to do it inside the file system.  I think the VFS and/or the block layer would have to offer help there in some way. What I mean for example is that if ->writepage fails then the failure is only detected inside the asynchronous i/o completion handler at which point the page is not locked any more, it is marked as being under writeback, and we are in IRQ context (or something) and thus it is not easy to see how we can from there get to doing all the above needed actions that require memory allocations, disk i/o, etc...  I suppose a separate thread could do it where we just schedule the work to be done.  But problem with that is that that work later on might fail so we can't simply pretend the block was written successfully yet we do not want to report an error or the upper layers would pick it up even though we hopefully will correct it in due course...

Quoted 4 lines

Best regards,

       Anton

For permanent write errors, I would expect any modern drive to do a sector remapping internally. We should never need to track this kind of information for any modern device that I know of (S-ATA, SAS, SSD's and raid arrays should all handle this).

Would not seem to be worth the complexity.

Also keep in mind that retrying IO errors is not always a good thing

For permanent write errors, I would expect any modern drive to do a sector remapping internally. We should never need to track this kind of information for any modern device that I know of (S-ATA, SAS, SSD's and raid arrays should all handle this).

Quoted 5 lines · Ric Wheeler

For permanent write errors, I would expect any modern drive to do a sector remapping internally. We should never need to track this kind of information for any modern device that I know of (S-ATA, SAS, SSD's and raid arrays should all handle this).

... and if the device is run out of all of its blocks in its spare blocks pool, it's probably well past the time to replace said disk.

BTW, I really liked Dave Chinner's summary of the issues involved; I ran into Kawai-san last week at Linux.conf.au, and we discussed pretty much the same thing over lunch. (i.e., that it's a hard problem, and in some cases we need to retry the writes, such as a transient FC path problem --- but some kind of write throttling is critical or we could end up choking the VM due to too many pages getting dirtied and no way of cleaning them.)

- Ted

Quoted 5 lines · Ric Wheeler

For permanent write errors, I would expect any modern drive to do a sector remapping internally. We should never need to track this kind of information for any modern device that I know of (S-ATA, SAS, SSD's and raid arrays should all handle this).

... and if the device is run out of all of its blocks in its spare blocks pool, it's probably well past the time to replace said disk.

BTW, I really liked Dave Chinner's summary of the issues involved; I ran into Kawai-san last week at Linux.conf.au, and we discussed pretty much the same thing over lunch. (i.e., that it's a hard problem, and in some cases we need to retry the writes, such as a transient FC path problem --- but some kind of write throttling is critical or we could end up choking the VM due to too many pages getting dirtied and no way of cleaning them.)

- Ted

Quoted 5 lines · Ric Wheeler

For permanent write errors, I would expect any modern drive to do a sector remapping internally. We should never need to track this kind of information for any modern device that I know of (S-ATA, SAS, SSD's and raid arrays should all handle this).

... and if the device is run out of all of its blocks in its spare blocks pool, it's probably well past the time to replace said disk.

BTW, I really liked Dave Chinner's summary of the issues involved; I ran into Kawai-san last week at Linux.conf.au, and we discussed pretty much the same thing over lunch. (i.e., that it's a hard problem, and in some cases we need to retry the writes, such as a transient FC path problem --- but some kind of write throttling is critical or we could end up choking the VM due to too many pages getting dirtied and no way of cleaning them.)

Quoted 5 lines · Ric Wheeler

For permanent write errors, I would expect any modern drive to do a sector remapping internally. We should never need to track this kind of information for any modern device that I know of (S-ATA, SAS, SSD's and raid arrays should all handle this).

... and if the device is run out of all of its blocks in its spare blocks pool, it's probably well past the time to replace said disk.

BTW, I really liked Dave Chinner's summary of the issues involved; I ran into Kawai-san last week at Linux.conf.au, and we discussed pretty much the same thing over lunch. (i.e., that it's a hard problem, and in some cases we need to retry the writes, such as a transient FC path problem --- but some kind of write throttling is critical or we could end up choking the VM due to too many pages getting dirtied and no way of cleaning them.)

Quoted 18 lines · On 01/25/2010 12:47 PM, [email protected] wrote:

Quoted 5 lines · Ric Wheeler

For permanent write errors, I would expect any modern drive to do a sector remapping internally. We should never need to track this kind of information for any modern device that I know of (S-ATA, SAS, SSD's and raid arrays should all handle this).

... and if the device is run out of all of its blocks in its spare blocks pool, it's probably well past the time to replace said disk.

BTW, I really liked Dave Chinner's summary of the issues involved; I ran into Kawai-san last week at Linux.conf.au, and we discussed pretty much the same thing over lunch. (i.e., that it's a hard problem, and in some cases we need to retry the writes, such as a transient FC path problem --- but some kind of write throttling is critical or we could end up choking the VM due to too many pages getting dirtied and no way of cleaning them.)

Quoted 1 line

- Ted

Also note that retrying writes (or reads for that matter) often are counter productive. For those of us who have suffered with trying to migrate data off of an old, failing disk onto a new, shiny one, excessive retries can be painful...

Quoted 10 lines · On Mon, Jan 25, 2010 at 12:47:23PM -0500, [email protected] wrote:

Quoted 5 lines · Ric Wheeler

For permanent write errors, I would expect any modern drive to do a sector remapping internally. We should never need to track this kind of information for any modern device that I know of (S-ATA, SAS, SSD's and raid arrays should all handle this).

... and if the device is run out of all of its blocks in its spare blocks pool, it's probably well past the time to replace said disk.

Quoted 7 lines

BTW, I really liked Dave Chinner's summary of the issues involved; I ran into Kawai-san last week at Linux.conf.au, and we discussed pretty much the same thing over lunch. (i.e., that it's a hard problem, and in some cases we need to retry the writes, such as a transient FC path problem --- but some kind of write throttling is critical or we could end up choking the VM due to too many pages getting dirtied and no way of cleaning them.)

Well I just don't think we can ever discard them by default.

Therefore
we must default to not discarding them, therefore we need to solve or work around the dirty page congestion problem some how.