On Fri, Mar 28, 2008 at 06:01:45PM +0900, Takashi Sato wrote:
quoted text
> 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.

At which point you run "unfreeze /mnt/fs" and unfreeze it.

If you've got a script that fails in the middle of an operation that
freezes the filesystem, then the error handling of that script
needs to unfreeze the filesystem. The kernel does not need to do
this.

quoted text
> So, I think the timeout is needed to unfreeze more definitely.

No, it can be handled by userspace perfectly well.

quoted text
> > 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?

If freeze_bdev() hangs, then you've got a buggy filesystem and far
more problems to worry about than undoing the freeze. It's likely
you're going to need a reboot to unwedge then hung filesystem.....

quoted text
> The salvage target of my timeout is freeze process's accident as below.
> - It is killed before calling the unfreeze ioctl

Can be fixed from userspace.

quoted text
> - It causes a deadlock by accessing the frozen filesystem

Application bug. Undeadlock it by running "unfreeze /mnt/fs"....

FWIW, DM is quite capable of freezing the filesystem, snapshotting
it and then unfreezing it without hanging, crashing or having nasty
stuff in general happen. We've used 'xfs_freeze -f /mnt/fs;
do_something; xfs_freeze -u /mnt/fs' for years without having
problems with freeze hanging, application deadlocks, etc.....

... And if something has gone wrong during the freeze, it is far,
far better to leave the filesystem stuck in a frozen state than to
unfreeze it and allow it to be damaged further. If you get stuck or
a script gets killed in the middle of execution, then an admin needs
to look at the problem immediately. Just timing out and unfreezing
is about the worst thing you can do because it allows problems
(corruptions, errors, etc) to be propagated and potentially make
things worse before an admin can intervene and fix things up....

Basically, I don't want to have to deal with the "snapshot
image corrupt" bug reports that will come from user
misuse/misunderstanding of the "freeze timeout". It's hard enough
tracking down these sorts of problems without throwing in the
"freeze timed out before completion" possibility that guarantees
a non-consistent snapshot image.....

/me points to the ASSERT_ALWAYS() in xfs_attr_quiesce() that ensures
we get bug reports when the filesystem is still being actively
modified when the freeze "completes".

quoted text
> 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.

And that should already be the case. If write_super_lockfs() hangs,
then you've got a filesystem bug ;)

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/