Messages in this thread | | | Date | Sat, 14 Dec 2013 15:23:24 -0500 | From | Tejun Heo <> | Subject | Re: Writeback threads and freezable |
| |
Hello, Dave.
On Sat, Dec 14, 2013 at 12:53:43PM +1100, Dave Chinner wrote: > That's the fundamental problem here - device removal asks the device > to fsync the filesystem on top of the device that was just removed. > The simple way to trigger this is to pull a device from underneath > an active filesystem (e.g. user unplugs a USB device without first > unmounting it). There are many years worth of bug reports showing > that this attempt by the device removal code to sync the filesystem > leads to deadlocks.
What are you suggesting? Implementing separate warm and hot unplug paths? That makes no sense whatsoever. Device hot unplug is just a sub operation of general device unplug which should be able to succeed whether the underlying device is failing IOs or not.
Another thing to consider is that you want to excercise your error handling path as often as possible. IOs could still fail even during device warm unplugs. If anything atop the driver can't handle that, that's a bug and we want to fix them. The fact that we have bug reports going back years on the subject simply means that we've been traditionally lousy with error handling throughout the layers - those were the occassions where we fixed actual error handling bugs, hot unplug or not. Special casing hot unplug would just make existing error handling bugs more difficult to find.
> It's simply not a valid thing to do - just how is the filesystem > supposed to sync to a non-existent device?
By issuing all IOs and then handling the failures appropriately. Exactly just as it would handle *ANY* io errors. This actually is the simplest thing to do to drain the pipe - pushing things down all the way and fail them at single point. What's the alternative? Implement quick exit path at each layer? That's just silly and error-prone.
> I've raised this each time a user reports it over the past few years > and never been able to convince any to fix the filesystem > re-entrancy problem device removal causes. Syncing the filesystem
Well, it isn't a good idea.
> will require taking locks that are by IO in progress, and so can't > make progress until the IO is completed, but that can't happen > until the error handling completes the sync of the filesystem....
IOs in progress shouldn't ever be stalled by filesystem sync. That's a massive layering violation. Where and how does that happen?
> Preventing fs/io re-entrancy from contexts where we might be holding > locks is the same reason we have GFP_NOFS and GFP_NOIO for memory > allocation: re-entering a filesystem or IO subsystem whenever we are > holding locks or serialised context in the fs/io path can deadlock > the fs/io path. > > IOWs, syncing the filesystem from the device delete code is just > plain wrong. That's what needs fixing - removing the cause of > re-entrancy, not the workqueue or writeback code...
No, the wrong thing there is having IO failures depending on filesystem sync. Nothing else.
> > Ideas? > > Fix the device delete error handling not to re-enter the > filesystem and IO path. It's just wrong.
Nope, bad idea.
Thanks.
-- tejun
| |