ext3 freeze feature

/*
+ * ext3_add_freeze_timeout - Add timeout for ext3 freeze.
+ *
+ * @sbi : ext3 super block
+ * @timeout_msec : timeout period
+ *
+ * Add the delayed work for ext3 freeze timeout
+ * to the delayed work queue.
+ */

include/linux/ext3_fs_sb.h 2008-01-22 18:20:33.000000000 +0900 renamed from include/linux/ext3_fs_sb.h 2008-01-16 13:22:48.000000000 +0900 +39 -0

+void ext3_add_freeze_timeout(struct ext3_sb_info *sbi,
+                       long timeout_msec)
+{
+       s64 timeout_jiffies = msecs_to_jiffies(timeout_msec);
+
+       /*
+        * setup freeze timeout function
+        */
+       INIT_DELAYED_WORK(&sbi->s_freeze_timeout, ext3_freeze_timeout);
+
+       /* set delayed work queue */
+       cancel_delayed_work(&sbi->s_freeze_timeout);
+       schedule_delayed_work(&sbi->s_freeze_timeout, timeout_jiffies);
+}
+
+/*
+ * ext3_del_freeze_timeout - Delete timeout for ext3 freeze.
+ *
+ * @sbi        : ext3 super block
+ *
+ * Delete the delayed work for ext3 freeze timeout
+ * from the delayed work queue.
+ */
+void ext3_del_freeze_timeout(struct ext3_sb_info *sbi)
+{
+       if (delayed_work_pending(&sbi->s_freeze_timeout))
+               cancel_delayed_work(&sbi->s_freeze_timeout);
+}
 
+/*
+ * ext3_freeze_timeout - Thaw the filesystem.
+ *
+ * @work       : work queue (delayed_work.work)
+ *
+ * Called by the delayed work when elapsing the timeout period.
+ * Thaw the filesystem.
+ */
+static void ext3_freeze_timeout(struct work_struct *work)
+{
+       struct ext3_sb_info *sbi = container_of(work,

I am also wondering whether we should have system call(s) for these:

fs/ext3/ioctl.c flags_err:

Quoted 3 lines · Takashi Sato

fs/ext3/ioctl.c +2 -0

+       case EXT3_IOC_FREEZE: {
 
+       case EXT3_IOC_THAW: {

And just convert XFS to use them too?

Quoted 8 lines

I am also wondering whether we should have system call(s) for these:

fs/ext3/ioctl.c flags_err:

Quoted 3 lines · Takashi Sato

fs/ext3/ioctl.c +2 -0

+       case EXT3_IOC_FREEZE: {
 
+       case EXT3_IOC_THAW: {

And just convert XFS to use them too?

I think it is reasonable to implement it as the generic system call, as you said. Does XFS folks think so?

Hi,

Quoted 5 lines

I am also wondering whether we should have system call(s) for these:

Quoted 1 line · Takashi Sato

+ case EXT3_IOC_FREEZE: {

Quoted 3 lines

Quoted 1 line

+ case EXT3_IOC_THAW: {

And just convert XFS to use them too?

I think it is reasonable to implement it as the generic system call, as you said.
Does XFS folks think so?

Hi,

Currently, ext3 doesn't have the freeze feature which suspends write requests. So, we cannot get a backup which keeps the filesystem's consistency with the storage device's features (snapshot, replication) while it is mounted.
In many case, a commercial filesystems (e.g. VxFS) has the freeze feature and it would be used to get the consistent backup.

So I am planning on implementing the ioctl of the freeze feature for ext3. I think we can get the consistent backup with the following steps.
1. Freeze the filesystem with ioctl.

2. Separate the replication volume or get the snapshot with the storage device's feature.
3. Unfreeze the filesystem with ioctl.

From my experience if delay is more than 4-5 seconds whole system becomes

4. Get the backup from the separated replication volume or the snapshot.

The usage of the ioctl is as below.
int ioctl(int fd, int cmd, long *timeval)
fd: The file descriptor of the mountpoint.
cmd: EXT3_IOC_FREEZE for the freeze or EXT3_IOC_THAW for the unfreeze.
timeval: The timeout value expressed in seconds.
If it's 0, the timeout isn't set.
Return value: 0 if the operation succeeds. Otherwise, -1.

I have made sure that write requests were suspended with the experimental patch for this feature and attached it in this mail.

The points of the implementation are followings.
- Add calls of the freeze function (freeze_bdev) and the unfreeze function (thaw_bdev) in ext3_ioctl().

- ext3_freeze_timeout() which calls the unfreeze function (thaw_bdev) is registered to the delayed work queue to unfreeze the filesystem automatically after the lapse of the specified time.

Any comments are very welcome.

Signed-off-by Takashi Sato <[email protected]>

---
diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/fs/ext3/ioctl.c linux-2.6.24-rc8-freeze/fs/ext3/ioctl.c

fs/ext3/ioctl.c +0 -0

@@ -254,6 +254,42 @@ flags_err:
 		return err;

fs/ext3/ioctl.c +29 -0

+		/* arg(sec) to tick value */
+		get_user(timeout_sec, (long __user *) arg);
+		timeout_msec = timeout_sec * 1000;
+		if (timeout_msec < 0)
+			return -EINVAL;
+
+		/* Freeze */
+		freeze_bdev(inode->i_sb->s_bdev);
+
+		/* set up unfreeze timer */
+		if (timeout_msec > 0)
+			ext3_add_freeze_timeout(EXT3_SB(inode->i_sb),
+						timeout_msec);
+		return 0;
+	}
+	case EXT3_IOC_THAW: {
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		if (inode->i_sb->s_frozen == SB_UNFROZEN)
+			return -EINVAL;
+
+		/* delete unfreeze timer */
+		ext3_del_freeze_timeout(EXT3_SB(inode->i_sb));
+
+		/* Unfreeze */
+		thaw_bdev(inode->i_sb->s_bdev, inode->i_sb);
+
+		return 0;
+	}
 
 	default:

return -ENOTTY;
diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/fs/ext3/super.c linux-2.6.24-rc8-freeze/fs/ext3/super.c

fs/ext3/super.c +61 -1

@@ -63,6 +63,7 @@ static int ext3_statfs (struct dentry * 
 static void ext3_unlockfs(struct super_block *sb);
 static void ext3_write_super (struct super_block * sb);
 static void ext3_write_super_lockfs(struct super_block *sb);
+static void ext3_freeze_timeout(struct work_struct *work);
 
 /*
  * Wrappers for journal_start/end.

@@ -323,6 +324,44 @@ void ext3_update_dynamic_rev(struct supe
 }
 
 /*
+ * ext3_add_freeze_timeout - Add timeout for ext3 freeze.
+ *
+ * @sbi			: ext3 super block
+ * @timeout_msec	: timeout period
+ *
+ * Add the delayed work for ext3 freeze timeout
+ * to the delayed work queue.
+ */
+void ext3_add_freeze_timeout(struct ext3_sb_info *sbi,
+			long timeout_msec)
+{
+	s64 timeout_jiffies = msecs_to_jiffies(timeout_msec);
+
+	/*
+	 * setup freeze timeout function
+	 */
+	INIT_DELAYED_WORK(&sbi->s_freeze_timeout, ext3_freeze_timeout);
+
+	/* set delayed work queue */
+	cancel_delayed_work(&sbi->s_freeze_timeout);
+	schedule_delayed_work(&sbi->s_freeze_timeout, timeout_jiffies);
+}
+
+/*
+ * ext3_del_freeze_timeout - Delete timeout for ext3 freeze.
+ *
+ * @sbi	: ext3 super block
+ *
+ * Delete the delayed work for ext3 freeze timeout
+ * from the delayed work queue.
+ */
+void ext3_del_freeze_timeout(struct ext3_sb_info *sbi)
+{
+	if (delayed_work_pending(&sbi->s_freeze_timeout))
+		cancel_delayed_work(&sbi->s_freeze_timeout);
+}
+
+/*
  * Open the external journal device
  */
 static struct block_device *ext3_blkdev_get(dev_t dev)

@@ -2367,10 +2406,31 @@ static void ext3_unlockfs(struct super_b
 		EXT3_SET_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER);
 		ext3_commit_super(sb, EXT3_SB(sb)->s_es, 1);
 		unlock_super(sb);
-		journal_unlock_updates(EXT3_SB(sb)->s_journal);
+		journal_unlock_updates_if_needed(EXT3_SB(sb)->s_journal);
 	}
 }
 
+/*
+ * ext3_freeze_timeout - Thaw the filesystem.
+ *
+ * @work	: work queue (delayed_work.work)
+ *
+ * Called by the delayed work when elapsing the timeout period.
+ * Thaw the filesystem.
+ */
+static void ext3_freeze_timeout(struct work_struct *work)
+{
+	struct ext3_sb_info *sbi = container_of(work,
+				struct ext3_sb_info,
+				s_freeze_timeout.work);
+	struct super_block *sb = get_super_block(sbi);
+
+	BUG_ON(sb == NULL);
+
+	if (sb->s_frozen != SB_UNFROZEN)
+		thaw_bdev(sb->s_bdev, sb);
+}
+
 static int ext3_remount (struct super_block * sb, int * flags, char * data)
 {
 	struct ext3_super_block * es;

diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/fs/jbd/journal.c linux-2.6.24-rc8-freeze/fs/jbd/journal.c

fs/jbd/journal.c +1 -0

@@ -46,6 +46,7 @@ EXPORT_SYMBOL(journal_extend);
 EXPORT_SYMBOL(journal_stop);
 EXPORT_SYMBOL(journal_lock_updates);
 EXPORT_SYMBOL(journal_unlock_updates);
+EXPORT_SYMBOL(journal_unlock_updates_if_needed);
 EXPORT_SYMBOL(journal_get_write_access);
 EXPORT_SYMBOL(journal_get_create_access);
 EXPORT_SYMBOL(journal_get_undo_access);

diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/fs/jbd/transaction.c linux-2.6.24-rc8-freeze/fs/jbd/transaction.c

fs/jbd/transaction.c +23 -0

@@ -485,6 +485,29 @@ void journal_unlock_updates (journal_t *
 	wake_up(&journal->j_wait_transaction_locked);
 }
 
+/**
+ * journal_unlock_updates_if_needed	 - release barrier if needed.
+ *
+ * @journal:  Journal to release the barrier on.
+ *
+ * Release a transaction barrier obtained if barrier count is not 0.
+ * Should be called without the journal lock held.
+ */
+void journal_unlock_updates_if_needed(journal_t *journal)
+{
+	spin_lock(&journal->j_state_lock);
+
+	if (!journal->j_barrier_count) {
+		spin_unlock(&journal->j_state_lock);
+		return;
+	}
+
+	--journal->j_barrier_count;
+	spin_unlock(&journal->j_state_lock);
+	mutex_unlock(&journal->j_barrier);
+	wake_up(&journal->j_wait_transaction_locked);
+}
+
 /*
  * Report any unexpected dirty buffers which turn up.  Normally those
  * indicate an error, but they can occur if the user is running (say)

diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/fs/super.c linux-2.6.24-rc8-freeze/fs/super.c

fs/super.c +27 -0

@@ -950,3 +950,30 @@ struct vfsmount *kern_mount_data(struct 
 }
 
 EXPORT_SYMBOL_GPL(kern_mount_data);
+
+/**
+ * get_super_block - get super_block
+ * @s_fs_info	: filesystem dependent information
+ * 		  (super_block.s_fs_info)
+ *
+ * Get super_block which holds s_fs_info from super_blocks.
+ * get_super_block() returns a pointer of super block or
+ * %NULL if it have failed.
+ */
+struct super_block *get_super_block(void *s_fs_info)
+{
+	struct super_block *sb;
+
+	spin_lock(&sb_lock);
+	sb = sb_entry(super_blocks.prev);
+	for (; sb != sb_entry(&super_blocks);
+				sb = sb_entry(sb->s_list.prev)) {
+		if (sb->s_fs_info == s_fs_info) {
+			spin_unlock(&sb_lock);
+			return sb;
+		}
+	}
+	spin_unlock(&sb_lock);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(get_super_block);

diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/include/linux/ext3_fs.h linux-2.6.24-rc8-freeze/include/linux/ext3_fs.h

include/linux/ext3_fs.h +5 -0

@@ -225,6 +225,8 @@ struct ext3_new_group_data {
 #endif
 #define EXT3_IOC_GETRSVSZ		_IOR('f', 5, long)
 #define EXT3_IOC_SETRSVSZ		_IOW('f', 6, long)
+#define EXT3_IOC_FREEZE			_IOW('f', 9, long)
+#define EXT3_IOC_THAW			_IOW('f', 10, long)
 
 /*
  * ioctl commands in 32 bit emulation

@@ -864,6 +866,9 @@ extern void ext3_abort (struct super_blo
 extern void ext3_warning (struct super_block *, const char *, const char *, ...)
 	__attribute__ ((format (printf, 3, 4)));
 extern void ext3_update_dynamic_rev (struct super_block *sb);
+extern void ext3_add_freeze_timeout(struct ext3_sb_info *sbi,
+			long timeout_msec);
+extern void ext3_del_freeze_timeout(struct ext3_sb_info *sbi);
 
 #define ext3_std_error(sb, errno)				\
 do {								\

diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/include/linux/ext3_fs_sb.h linux-2.6.24-rc8-freeze/include/linux/ext3_fs_sb.h

include/linux/ext3_fs_sb.h +2 -0

@@ -81,6 +81,8 @@ struct ext3_sb_info {
 	char *s_qf_names[MAXQUOTAS];		/* Names of quota files with journalled quota */
 	int s_jquota_fmt;			/* Format of quota to use */
 #endif
+	/* Delayed work for freeze */
+	struct delayed_work s_freeze_timeout;
 };
 
 #endif	/* _LINUX_EXT3_FS_SB */

diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/include/linux/fs.h linux-2.6.24-rc8-freeze/include/linux/fs.h

include/linux/fs.h +1 -0

@@ -2095,6 +2095,7 @@ struct ctl_table;
 int proc_nr_files(struct ctl_table *table, int write, struct file *filp,
 		  void __user *buffer, size_t *lenp, loff_t *ppos);
 
+extern struct super_block *get_super_block(void *s_fs_info);
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_FS_H */

diff -uprN -X linux-2.6.24-rc8/Documentation/dontdiff linux-2.6.24-rc8/include/linux/jbd.h linux-2.6.24-rc8-freeze/include/linux/jbd.h

include/linux/jbd.h +1 -1

@@ -905,6 +905,7 @@ extern int	 journal_stop(handle_t *);
 extern int	 journal_flush (journal_t *);
 extern void	 journal_lock_updates (journal_t *);
 extern void	 journal_unlock_updates (journal_t *);
+extern void	 journal_unlock_updates_if_needed(journal_t *);
 
 extern journal_t * journal_init_dev(struct block_device *bdev,
 				struct block_device *fs_dev,
 
-

To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

First of all Linux already have at least one open-source(dm-snap), and several commercial snapshot solutions.

You have to realize what delay between 1-3 stages have to be minimal. for example dm-snap perform it only for explicit journal flushing. From my experience if delay is more than 4-5 seconds whole system becomes unstable.

The other approach would be to say, "oh well, the freeze ioctl is inherently dangerous, and root is allowed to himself in the foot, so who cares". :-)

But it was this concern which is why ext3 never exported freeze functionality to userspace, even though other commercial filesystems do support this. It wasn't that it wasn't considered, but the concern about whether or not it was sufficiently safe to make available.

And I do agree that we probably should just implement this in filesystem independent way, in which case all of the filesystems that support this already have super_operations functions write_super_lockfs() and unlockfs().

Quoted 4 lines

But it was this concern which is why ext3 never exported freeze functionality to userspace, even though other commercial filesystems do support this. It wasn't that it wasn't considered, but the concern about whether or not it was sufficiently safe to make available.

What's the safety concern; that the admin will forget to unfreeze?

I'm also not sure I see the point of the timeout in the original patch; either you are done snapshotting and ready to unfreeze, or you're not; 1, or 2, or 3 seconds doesn't really matter. When you're done, you're done, and you can only unfreeze then. Shouldn't this be done programmatically, and not with some pre-determined timeout?

Quoted 6 lines · Eric Sandeen

Quoted 4 lines

But it was this concern which is why ext3 never exported freeze functionality to userspace, even though other commercial filesystems do support this. It wasn't that it wasn't considered, but the concern about whether or not it was sufficiently safe to make available.

What's the safety concern; that the admin will forget to unfreeze?

That the admin would manage to deadlock him/herself and wedge up the whole system...

Quoted 5 lines

I'm also not sure I see the point of the timeout in the original patch; either you are done snapshotting and ready to unfreeze, or you're not; 1, or 2, or 3 seconds doesn't really matter. When you're done, you're done, and you can only unfreeze then. Shouldn't this be done programmatically, and not with some pre-determined timeout?

This is only a guess, but I suspect it was a fail-safe in case the admin did manage to deadlock him/herself.

I would think a better approach would be to make the filesystem unfreeze if the file descriptor that was used to freeze the filesystem is closed, and then have explicit deadlock detection that kills the process doing the freeze, at which point the filesystem unlocks and the system can recover.

What you *could* do is to start putting processes to sleep if they attempt to write to the frozen filesystem, and then detect the deadlock case where the process holding the file descriptor used to freeze the filesystem gets frozen because it attempted to write to the filesystem --- at which point it gets some kind of signal (which defaults to killing the process), and the filesystem is unfrozen and as part of the unfreeze you wake up all of the processes that were put to sleep for touching the frozen filesystem.

Quoted 8 lines

What you *could* do is to start putting processes to sleep if they attempt to write to the frozen filesystem, and then detect the deadlock case where the process holding the file descriptor used to freeze the filesystem gets frozen because it attempted to write to the filesystem --- at which point it gets some kind of signal (which defaults to killing the process), and the filesystem is unfrozen and as part of the unfreeze you wake up all of the processes that were put to sleep for touching the frozen filesystem.

I don't think close() usually writes to journal and the deadlock occurs. Is there the special case which close() writes to journal in case of getting signal?

and then detect the
deadlock case where the process holding the file descriptor used to freeze the filesystem gets frozen because it attempted to write to the filesystem --- at which point it gets some kind of signal (which defaults to killing the process), and the filesystem is unfrozen and as part of the unfreeze you wake up all of the processes that were put to sleep for touching the frozen filesystem.

What you *could* do is to start putting processes to sleep if they attempt to write to the frozen filesystem, and then detect the deadlock case where the process holding the file descriptor used to freeze the filesystem gets frozen because it attempted to write to the filesystem --- at which point it gets some kind of signal (which defaults to killing the process), and the filesystem is unfrozen and as part of the unfreeze you wake up all of the processes that were put to sleep for touching the frozen filesystem.

The other approach would be to say, "oh well, the freeze ioctl is inherently dangerous, and root is allowed to himself in the foot, so who cares". :-)

And I do agree that we probably should just implement this in filesystem independent way, in which case all of the filesystems that support this already have super_operations functions write_super_lockfs() and unlockfs().

So if this is done using a new system call, there should be no filesystem-specific changes needed, and all filesystems which support those super_operations method functions would be able to provide this functionality to the new system call.

OK I would like to implement the freeze feature on VFS as the filesystem independent ioctl so that it can be available on filesystems that have already had write_super_lockfs() and unlockfs().
The usage for the freeze ioctl is the following.
int ioctl(int fd, int FIFREEZE, long *timeval);
fd:file descriptor of mountpoint
FIFREEZE:request cord for freeze
timeval:timeout period (second)

And the unfreeze ioctl is the following.
int ioctl(int fd, int FITHAW, NULL);
fd:file descriptor of mountpoint
FITHAW:Request cord for unfreeze

You may as well make the common ioctl the same as the XFS version, both by number and parameters, so that applications which already understand the XFS ioctl will work on other filesystems.

Quoted 3 lines · Andreas Dilger

You may as well make the common ioctl the same as the XFS version, both by number and parameters, so that applications which already understand the XFS ioctl will work on other filesystems.

Yes. In facy you should be able to lift the implementations of XFS_IOC_FREEZE and XFS_IOC_THAW to generic code, there's nothing XFS-specific in there.

From Documentation/ioctl-number.txt:

Hi,

Quoted 7 lines · Christoph Hellwig

Quoted 3 lines · Andreas Dilger

You may as well make the common ioctl the same as the XFS version, both by number and parameters, so that applications which already understand the XFS ioctl will work on other filesystems.

Yes. In facy you should be able to lift the implementations of XFS_IOC_FREEZE and XFS_IOC_THAW to generic code, there's nothing XFS-specific in there.

According to Documentation/ioctl-number.txt, XFS_IOC_XXXs (_IOWR('X', aa, bb)) are defined for XFS like below. From Documentation/ioctl-number.txt:
----------------------------------------------------------------------------
Code Seq# Include File Comments
========================================================
: :
'X' all linux/xfs_fs.h
----------------------------------------------------------------------------

So XFS_IOC_FREEZE and XFS_IOC_THAW cannot be lifted to generic code simply.

I think we should create new generic numbers for freeze and thaw like FIBMAP as followings.
linux/fs.h:
#define FIFREEZE _IO(0x00,3)
#define FITHAW _IO(0x00,4)

And xfs_freeze calls XFS_IOC_FREEZE with a magic number 1, but what is 1?

Instead, I'd like the sec to timeout on freeze API in order to thaw the filesystem automatically. It can prevent a filesystem from staying frozen forever.
(Because a freezer may cause a deadlock by accessing the frozen filesystem.)

Any comments are very welcome.

Cheers, Takashi

#define FS_IOC_GETFLAGS _IOR('f', 1, long)
#define FS_IOC_SETFLAGS _IOW('f', 2, long)

as generic vfs ioctls. These ioctls started out as EXT2_IOC_SETFLAGS/EXT2_IOC_GETFLAGS but they were generically useful, other filesystems picked them up, and they were "elevated" to the vfs.

Quoted 1 line

And xfs_freeze calls XFS_IOC_FREEZE with a magic number 1, but what is 1?

Looks like it's called "level" but it's probably a holdover, it doesn't look like it's used.

Quoted 4 lines

Instead, I'd like the sec to timeout on freeze API in order to thaw the filesystem automatically. It can prevent a filesystem from staying frozen forever.
(Because a freezer may cause a deadlock by accessing the frozen filesystem.)

I'm still not very comfortable with the timeout; if you un-freeze on a timer, how do you know that the work for which you needed the fileystem frozen is complete? How would you know if your snapshot was good if there's a possibility that the fs unfroze while it was being taken?

Quoted 9 lines

Quoted 4 lines

Instead, I'd like the sec to timeout on freeze API in order to thaw the filesystem automatically. It can prevent a filesystem from staying frozen forever.
(Because a freezer may cause a deadlock by accessing the frozen filesystem.)

I'm still not very comfortable with the timeout; if you un-freeze on a timer, how do you know that the work for which you needed the fileystem frozen is complete? How would you know if your snapshot was good if there's a possibility that the fs unfroze while it was being taken?

And how about adding the new ioctl to reset the timeval like below? (Dmitri proposed this idea before.)
int ioctl(int fd, int FIFREEZE_RESET_TIMEOUT, long *timeval);
fd:file descriptor of mountpoint FIFREEZE_RESET_TIMEOUT:request code for reset of timeout period timeval:new timeout period
This is useful for the application to set the timeval more accurately. For example, the freezer resets the timeval 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 can recognize that at the next reset of timeval.

o Elevate XFS ioctl numbers (XFS_IOC_FREEZE and XFS_IOC_THAW) to the VFS As Andreas Dilger and Christoph Hellwig advised me, I have elevated them to include/linux/fs.h as below.

    #define FIFREEZE        _IOWR('X', 119, int)

　 #define FITHAW _IOWR('X', 120, int)
The ioctl numbers used by XFS applications don't need to be changed. But my following ioctl for the freeze needs the parameter as the timeout period. So if XFS applications don't want the timeout feature as the current implementation, the parameter needs to be changed 1 (level?) into 0.

Quoted 10 lines · Takashi Sato

o Elevate XFS ioctl numbers (XFS_IOC_FREEZE and XFS_IOC_THAW) to the VFS As Andreas Dilger and Christoph Hellwig advised me, I have elevated them to include/linux/fs.h as below.

    #define FIFREEZE        _IOWR('X', 119, int)

　 #define FITHAW _IOWR('X', 120, int)
The ioctl numbers used by XFS applications don't need to be changed. But my following ioctl for the freeze needs the parameter as the timeout period. So if XFS applications don't want the timeout feature as the current implementation, the parameter needs to be changed 1 (level?) into 0.

So, existing xfs applications calling the xfs ioctl now will behave differently, right? We can only keep the same ioctl number if the calling semantics are the same. Keeping the same number but changing the semantics is harmful, IMHO....

Quoted 11 lines

Quoted 10 lines

o Elevate XFS ioctl numbers (XFS_IOC_FREEZE and XFS_IOC_THAW) to the VFS As Andreas Dilger and Christoph Hellwig advised me, I have elevated them to include/linux/fs.h as below.

    #define FIFREEZE        _IOWR('X', 119, int)

　 #define FITHAW _IOWR('X', 120, int)
The ioctl numbers used by XFS applications don't need to be changed. But my following ioctl for the freeze needs the parameter as the timeout period. So if XFS applications don't want the timeout feature as the current implementation, the parameter needs to be changed 1 (level?) into 0.

Quoted 4 lines

So, existing xfs applications calling the xfs ioctl now will behave differently, right? We can only keep the same ioctl number if the calling semantics are the same. Keeping the same number but changing the semantics is harmful, IMHO....

Do we know what this parameter was supposed to mean?

We could special case "1" if needed to keep compatibility (documenting this clearly), either making it == 0, or some very long timeout (1h or whatever). A relatively minor wart I think.

So XFS_IOC_FREEZE and XFS_IOC_THAW cannot be lifted to generic code simply. I think we should create new generic numbers for freeze and thaw

And xfs_freeze calls XFS_IOC_FREEZE with a magic number 1, but what is 1?

Instead, I'd like the sec to timeout on freeze API in order to thaw the filesystem automatically. It can prevent a filesystem from staying frozen forever.
(Because a freezer may cause a deadlock by accessing the frozen filesystem.)

P.S. Oh yeah, it should be noted that freezing at the filesystem layer does *not* guarantee that changes to the block device aren't happening via mmap()'ed files. The LVM needs to freeze writes the block device level if it wants to guarantee a completely stable snapshot image. So the proposed patch doens't quite give you those guarantees, if that was the intended goal.

The points of the implementation are followings.
- Add calls of the freeze function (freeze_bdev) and the unfreeze function (thaw_bdev) in ext3_ioctl().

- ext3_freeze_timeout() which calls the unfreeze function (thaw_bdev) is registered to the delayed work queue to unfreeze the filesystem automatically after the lapse of the specified time.

fs/ext3/ioctl.c +1 -0

+	case EXT3_IOC_FREEZE: {

(quoted diff) +2 -0

+		if (inode->i_sb->s_frozen != SB_UNFROZEN)
+			return -EINVAL;

fs/ext3/ioctl.c +1 -0

+		freeze_bdev(inode->i_sb->s_bdev);

fs/ext3/ioctl.c +5 -0

+	case EXT3_IOC_THAW: {
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		if (inode->i_sb->s_frozen == SB_UNFROZEN)
+			return -EINVAL;

Quoted 7 lines · Takashi Sato

The points of the implementation are followings.
- Add calls of the freeze function (freeze_bdev) and the unfreeze function (thaw_bdev) in ext3_ioctl().

- ext3_freeze_timeout() which calls the unfreeze function (thaw_bdev) is registered to the delayed work queue to unfreeze the filesystem automatically after the lapse of the specified time.

Seems like pointless complexity to me - what happens if a timeout occurs while the filsystem is still freezing?

It's not uncommon for a freeze to take minutes if memory is full of dirty data that needs to be flushed out, esp. if dm-snap is doing COWs for every write issued....

That's inherently unsafe - you can have multiple unfreezes running in parallel which seriously screws with the bdev semaphore count that is used to lock the device due to doing multiple up()s for every down.

Your timeout thingy guarantee that at some point you will get multiple up()s occuring due to the timer firing racing with a thaw ioctl.

If this interface is to be more widely exported, then it needs a complete revamp of the bdev is locked while it is frozen so that there is no chance of a double up() ever occuring on the bd_mount_sem due to racing thaws.....