The timeout feature is added to freeze ioctl.  And new ioctl
to reset the timeout period is added.
o Freeze the filesystem
  int ioctl(int fd, int FIFREEZE, long *timeout_sec)
    fd: The file descriptor of the mountpoint
    FIFREEZE: request code for the freeze
    timeout_sec: the timeout period in seconds
             If it's 0 or 1, the timeout isn't set.
             This special case of "1" is implemented to keep
             the compatibility with XFS applications.
    Return value: 0 if the operation succeeds. Otherwise, -1

o Reset the timeout period
  int ioctl(int fd, int FIFREEZE_RESET_TIMEOUT, long *timeout_sec)
    fd:file descriptor of mountpoint
    FIFREEZE_RESET_TIMEOUT: request code for reset of timeout period
    timeout_sec: new timeout period in seconds
    Return value: 0 if the operation succeeds. Otherwise, -1
    Error number: If the filesystem has already been unfrozen,
                  errno is set to EINVAL.

Signed-off-by: Takashi Sato <t-sato@yk.jp.nec.com>
Signed-off-by: Masayuki Hamaguchi <m-hamaguchi@ys.jp.nec.com>
---
 drivers/md/dm.c             |    2 -
 fs/block_dev.c              |    2 +
 fs/buffer.c                 |   16 +++++++--
 fs/ioctl.c                  |   77 ++++++++++++++++++++++++++++++++++++++++++--
 fs/super.c                  |   57 ++++++++++++++++++++++++++++++++
 fs/xfs/xfs_fsops.c          |    2 -
 include/linux/buffer_head.h |    3 +
 include/linux/fs.h          |   10 +++++
 8 files changed, 160 insertions(+), 9 deletions(-)

diff -uprN -X linux-2.6.27-rc2.org/Documentation/dontdiff linux-2.6.27-rc2-xfs/drivers/md/dm.c linux-2.6.27-rc2-timeout/
drivers/md/dm.c
--- linux-2.6.27-rc2-xfs/drivers/md/dm.c	2008-08-07 09:00:27.000000000 +0900
+++ linux-2.6.27-rc2-timeout/drivers/md/dm.c	2008-08-07 09:14:30.000000000 +0900
@@ -1451,7 +1451,7 @@ static int lock_fs(struct mapped_device 
 
 	WARN_ON(md->frozen_sb);
 
-	md->frozen_sb = freeze_bdev(md->suspended_bdev);
+	md->frozen_sb = ...
[ message continues ]

On Mon, 18 Aug 2008 21:28:56 +0900

I don't think the changelogs actually explained why this feature is
being added?

Which userspace tools are expected to send these ioctls?  Something in

This duplicates quite a bit of code from ioctl_freeze().  Can this be


I don't particularly like the names of these new global symbols.  The
kernel already has a "freezer" thing, part of power-management. 
Introducing another one is a bit confusing.

otoh, freezer seems to have consistently used "freezer", so the 'r'
arguable saves us.

Still, I'd have thought that "fsfreeze" would have been a clearer, more

So if the calling task is keventd via run_workqueue() then
delayed_work_pending() should return false due to run_workqueue()
ordering, so we avoid the deadlock.

Seems a bit racy if some other process starts the delayed-work while
this function is running but I guess the new semaphore prevents that.

Perhaps cancel_delayed_work_sync() shouldn't hang up if called from the
work handler?


--

[ message continues ]

Currently the only surspace using freeze and thaw is xfs_freeze from
xfsprogs, which would work for various other filesystems that implement
->write_super_lockfs now instead of just XFS with patch 1.

The freeze stuff in this third patch isn't and won't be used by
xfs_freeze and doesn't make all that much sense (and we already had a
lot of previous discussion on this..)

--

[ message continues ]

I don't understand this patch, but the code above looks strange to me...

Let's suppose del_freeze_timeout() is called by ioctl_thaw()->thaw_bdev().
Now,

	IF delayed_work_pending() == T

		we can deadlock if the timer expires before
		cancel_delayed_work_sync() cancels it?
		in that case we are going to wait for this work,
		but freeze_timeout()->thaw_bdev() will block
		on ->bd_freeze_sem, no?

	ELSE

		we don't really flush the work, it is possible
		the timer has already expired and the work
		is pending. It will run later.

Perhaps this all is correct, but in that case, why can't we just do

	void del_freeze_timeout(struct block_device *bdev)
	{
		cancel_delayed_work(&bdev->bd_freeze_timeout);
	}


This is trivial,

	--- kernel/workqueue.c
	+++ kernel/workqueue.c
	@@ -516,6 +516,9 @@ static void wait_on_cpu_work(struct cpu_
		struct wq_barrier barr;
		int running = 0;
	 
	+	if (cwq->thread == current)
	+		return;
	+
		spin_lock_irq(&cwq->lock);
		if (unlikely(cwq->current_work == work)) {
			insert_wq_barrier(cwq, &barr, cwq->worklist.next);

but do we really need this?

We have a similar hack in flush_cpu_workqueue(), and we are going
to kill it once we fix the callers.

I dunno.

Oleg.

--

[ message continues ]

Hi, Andrew and Oleg.


I will fix the changelogs as following.

-----------------------------------------------------------
The timeout feature is added to "freeze ioctl" to solve a deadlock
when the freezer accesses a frozen filesystem. And new ioctl
to reset the timeout period is added to extend the timeout period.
For example, the freezer resets the timeout period to 10 seconds every 5
seconds.  In this approach, even if the freezer causes a deadlock by
accessing the frozen filesystem, it will be solved by the timeout
in 10 seconds and the freezer will be able to recognize that
at the next reset of timeout period.

I think the management software for the storage device



I will rename the names of these global symbols.

I think so.

In my current implementation,
the delayed work task calls thaw_bdev() to unfreeze a filesystem
and it calls del_freeze_timeout().  So, the deadlock occurs.
But I've found that the delayed work task doesn't need to call
del_freeze_timeout() because it is removed
from the delayed work queue automatically after the work finishes.
So I will fix thaw_bdev() so that it doesn't call
del_freeze_timeout() only when it's called by the delayed work task.
And I will delete delayed_work_pending() in del_freeze_timeout().

Cheers, Takashi
--

[ message continues ]