Messages in this thread | | | Date | Wed, 12 Aug 2009 09:46:11 +0200 | From | Andi Kleen <> | Subject | Re: [PATCH] [16/19] HWPOISON: Enable .remove_error_page for migration aware file systems |
| |
On Wed, Aug 12, 2009 at 11:49:56AM +0900, Hidehiro Kawai wrote: > > I don't think there's much we can do if the application doesn't > > check for IO errors properly. What would you do if it doesn't > > check for IO errors at all? If it checks for IO errors it simply > > has to check for them on all IO operations -- if they do > > they will detect hwpoison errors correctly too. > > I believe it's not uncommon for applications to do buffered write > and then exit without fsync(). And I think it's difficult to > preclude such applications and commands from the system perfectly.
That's true, but for anything mission critical you would expect them to use some transactional mechanism, either with O_SYNC or fsync(). Otherwise they always risk data loss anyways.
> > It's unclear to me this special mode is really desirable. > > Does it bring enough value to the user to justify the complexity > > of another exotic option? The case is relatively exotic, > > as in dirty write cache that is mapped to a file. > > > > Try to explain it in documentation and you see how ridiculous it sounds; u > > it simply doesn't have clean semantics > > > > ("In case you have applications with broken error IO handling on > > your mission critical system ...") > > Generally, dropping unwritten dirty page caches is considered to be > risky. So the "panic on IO error" policy has been used as usual > practice for some systems. I just suggested that we adopted > this policy into machine check errors.
Hmm, what we could possibly do -- as followon patches -- would be to let error_remove_page check the per file system panic-on-io-error super block setting for dirty pages and panic in this case too. Unfortunately this setting is currently per file system, not generic, so it would need to be a fs specific check (or the flag would need to be moved into a generic fs superblock field first)
I think that would be relatively clean semantics wise. Would you be interested in working on patches for that?
> Another option is to introduce "ignore all" policy instead of > panicking at the beginig of memory_failure(). Perhaps it finally > causes SRAR machine check, and then kernel will panic or a process > will be killed. Anyway, this is a topic for the next stage.
The problem is memory_failure() would then need to start distingushing between AR=1 and AR=0 which it doesn't today.
It could be done, but would need some more work.
> > If you want to have improved IO error handling feel free to > > submit it separately. I agree this area could use some work. > > But it probably needs more design work first. > > Well, this patch set itself looks good to me. > I also looked into the other patches, I couldn't find any > problems (although I'm not good judge of reviewing). > > Reviewed-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Thanks for your review and your comments.
-Andi -- ak@linux.intel.com -- Speaking for myself only.
| |