Hi, David Chinner wrote: > Can you please split this into two patches - one which introduces the > generic functionality *without* the timeout stuff, and a second patch > that introduces the timeouts. OK. I will send the split patches in subsequent mails. > I think this timeout stuff is dangerous - it adds significant > complexity and really does not protect against anything that can't > be done in userspace. i.e. If your system is running well enough > for the timer to fire and unfreeze the filesystem, it's running well > enough for you to do "freeze X; sleep Y; unfreeze X". If the process is terminated at "sleep Y" by an unexpected accident (e.g. signals), the filesystem will be left frozen. So, I think the timeout is needed to unfreeze more definitely. > FWIW, there is nothing to guarantee that the filesystem has finished > freezing when the timeout fires (it's not uncommon to see > freeze_bdev() taking *minutes*) and unfreezing in the middle of a > freeze operation will cause problems - either for the filesystem > in the middle of a freeze operation, or for whatever is freezing the > filesystem to get a consistent image..... Do you mention the freeze_bdev()'s hang? The salvage target of my timeout is freeze process's accident as below. - It is killed before calling the unfreeze ioctl - It causes a deadlock by accessing the frozen filesystem So the delayed work for the timeout is set after all of freeze operations in freeze_bdev() in my patches. I think the filesystem dependent code (write_super_lockfs operation) should be implemented not to cause a hang. Cheers, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
