[PATCH 1/10] VFS: Fix error handling of write_super_lockfs/unlockfs

[PATCH 1/10] VFS: Fix error handling of write_super_lockfs/unlockfs

[Takashi Sato]

I've changed the type of write_super_lockfs and unlockfs from "void" to
"int" so that they can return an error. 

Signed-off-by: Takashi Sato <t-sato@yk.jp.nec.com>
Signed-off-by: Masayuki Hamaguchi <m-hamaguchi@ys.jp.nec.com>
---
 Documentation/filesystems/Locking |    4 ++--
 Documentation/filesystems/vfs.txt |    4 ++--
 include/linux/fs.h                |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff -uprN -X linux-2.6.27-rc7.org/Documentation/dontdiff linux-2.6.27-rc7.org/Documentation/filesystems/Locking linux-2
.6.27-rc7-lockfs/Documentation/filesystems/Locking
--- linux-2.6.27-rc7.org/Documentation/filesystems/Locking	2008-09-22 07:29:55.000000000 +0900
+++ linux-2.6.27-rc7-lockfs/Documentation/filesystems/Locking	2008-09-22 09:53:22.000000000 +0900
@@ -97,8 +97,8 @@ prototypes:
 	void (*put_super) (struct super_block *);
 	void (*write_super) (struct super_block *);
 	int (*sync_fs)(struct super_block *sb, int wait);
-	void (*write_super_lockfs) (struct super_block *);
-	void (*unlockfs) (struct super_block *);
+	int (*write_super_lockfs) (struct super_block *);
+	int (*unlockfs) (struct super_block *);
 	int (*statfs) (struct dentry *, struct kstatfs *);
 	int (*remount_fs) (struct super_block *, int *, char *);
 	void (*clear_inode) (struct inode *);
diff -uprN -X linux-2.6.27-rc7.org/Documentation/dontdiff linux-2.6.27-rc7.org/Documentation/filesystems/vfs.txt linux-2
.6.27-rc7-lockfs/Documentation/filesystems/vfs.txt
--- linux-2.6.27-rc7.org/Documentation/filesystems/vfs.txt	2008-09-22 07:29:55.000000000 +0900
+++ linux-2.6.27-rc7-lockfs/Documentation/filesystems/vfs.txt	2008-09-22 09:53:22.000000000 +0900
@@ -210,8 +210,8 @@ struct super_operations {
         void (*put_super) (struct super_block *);
         void (*write_super) (struct super_block *);
         int (*sync_fs)(struct super_block *sb, int wait);
-        void (*write_super_lockfs) (struct super_block *);
-        void (*unlockfs) (struct super_block *);
+        int (*write_super_lockfs) (struct super_block *);
+        int (*unlockfs) (struct super_block *);
         int (*statfs) (struct dentry *, struct kstatfs *);
         int (*remount_fs) (struct super_block *, int *, char *);
         void (*clear_inode) (struct inode *);
diff -uprN -X linux-2.6.27-rc7.org/Documentation/dontdiff linux-2.6.27-rc7.org/include/linux/fs.h linux-2.6.27-rc7-lockf
s/include/linux/fs.h
--- linux-2.6.27-rc7.org/include/linux/fs.h	2008-09-22 07:29:55.000000000 +0900
+++ linux-2.6.27-rc7-lockfs/include/linux/fs.h	2008-09-22 09:53:46.000000000 +0900
@@ -1314,8 +1314,8 @@ struct super_operations {
 	void (*put_super) (struct super_block *);
 	void (*write_super) (struct super_block *);
 	int (*sync_fs)(struct super_block *sb, int wait);
-	void (*write_super_lockfs) (struct super_block *);
-	void (*unlockfs) (struct super_block *);
+	int (*write_super_lockfs) (struct super_block *);
+	int (*unlockfs) (struct super_block *);
 	int (*statfs) (struct dentry *, struct kstatfs *);
 	int (*remount_fs) (struct super_block *, int *, char *);
 	void (*clear_inode) (struct inode *);

Re: [PATCH 1/10] VFS: Fix error handling of write_super_lockfs/unlockfs

[Christoph Hellwig]

On Mon, Sep 22, 2008 at 07:55:26PM +0900, Takashi Sato wrote:
> I've changed the type of write_super_lockfs and unlockfs from "void" to
> "int" so that they can return an error. 

Returning an error from the freeze operation makes sense, but for the
unfreeze I don't see the point.  You must however change all existing
instances to actually return a value (even if it's always 0 for now)
to avoid breaking git bisect.

If you touch all instances anyway, it would be nice to rename them
to freeze / unfreze as the current names are more confusing.

Re: [PATCH 1/10] VFS: Fix error handling ofwrite_super_lockfs/unlockfs

[Takashi Sato]

Hi,

> On Mon, Sep 22, 2008 at 07:55:26PM +0900, Takashi Sato wrote:
>> I've changed the type of write_super_lockfs and unlockfs from "void" to
>> "int" so that they can return an error.
>
> Returning an error from the freeze operation makes sense, but for the
> unfreeze I don't see the point.  You must however change all existing
> instances to actually return a value (even if it's always 0 for now)
> to avoid breaking git bisect.

I thought unlockfs should return an error because ext3_unlockfs()
might cause I/O error in writing a super block.
But it is an internal error and the unfreezing succeeds.
So I will consider returning 0.

> If you touch all instances anyway, it would be nice to rename them
> to freeze / unfreze as the current names are more confusing.

I will consider renaming.

Cheers, Takashi

[PATCH 1/10] VFS: Fix error handling of write_super_lockfs/unlockfs

[Takashi Sato]

I've changed the type of write_super_lockfs and unlockfs from "void" to
"int" so that they can return an error 
and renamed write_super_lockfs/unlockfs of the super block operation
freeze_fs/unfreeze_fs to avoid a confusion.


Signed-off-by: Takashi Sato <t-sato@yk.jp.nec.com>
Signed-off-by: Masayuki Hamaguchi <m-hamaguchi@ys.jp.nec.com>
---
 Documentation/filesystems/Locking |    8 ++++----
 Documentation/filesystems/vfs.txt |    8 ++++----
 fs/buffer.c                       |    8 ++++----
 include/linux/fs.h                |    4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

diff -uprN -X linux-2.6.27-rc7/Documentation/dontdiff linux-2.6.27-rc7/Documentation/filesystems/Locking linux-2.6.27-rc
7-lockfs/Documentation/filesystems/Locking
--- linux-2.6.27-rc7/Documentation/filesystems/Locking	2008-09-22 07:29:55.000000000 +0900
+++ linux-2.6.27-rc7-lockfs/Documentation/filesystems/Locking	2008-09-26 18:47:47.000000000 +0900
@@ -97,8 +97,8 @@ prototypes:
 	void (*put_super) (struct super_block *);
 	void (*write_super) (struct super_block *);
 	int (*sync_fs)(struct super_block *sb, int wait);
-	void (*write_super_lockfs) (struct super_block *);
-	void (*unlockfs) (struct super_block *);
+	int (*freeze_fs) (struct super_block *);
+	int (*unfreeze_fs) (struct super_block *);
 	int (*statfs) (struct dentry *, struct kstatfs *);
 	int (*remount_fs) (struct super_block *, int *, char *);
 	void (*clear_inode) (struct inode *);
@@ -119,8 +119,8 @@ delete_inode:		no
 put_super:		yes	yes	no
 write_super:		no	yes	read
 sync_fs:		no	no	read
-write_super_lockfs:	?
-unlockfs:		?
+freeze_fs:		?
+unfreeze_fs:		?
 statfs:			no	no	no
 remount_fs:		yes	yes	maybe		(see below)
 clear_inode:		no
diff -uprN -X linux-2.6.27-rc7/Documentation/dontdiff linux-2.6.27-rc7/Documentation/filesystems/vfs.txt linux-2.6.27-rc
7-lockfs/Documentation/filesystems/vfs.txt
--- linux-2.6.27-rc7/Documentation/filesystems/vfs.txt	2008-09-22 07:29:55.000000000 +0900
+++ linux-2.6.27-rc7-lockfs/Documentation/filesystems/vfs.txt	2008-09-26 18:43:49.000000000 +0900
@@ -210,8 +210,8 @@ struct super_operations {
         void (*put_super) (struct super_block *);
         void (*write_super) (struct super_block *);
         int (*sync_fs)(struct super_block *sb, int wait);
-        void (*write_super_lockfs) (struct super_block *);
-        void (*unlockfs) (struct super_block *);
+        int (*freeze_fs) (struct super_block *);
+        int (*unfreeze_fs) (struct super_block *);
         int (*statfs) (struct dentry *, struct kstatfs *);
         int (*remount_fs) (struct super_block *, int *, char *);
         void (*clear_inode) (struct inode *);
@@ -270,11 +270,11 @@ or bottom half).
   	a superblock. The second parameter indicates whether the method
 	should wait until the write out has been completed. Optional.
 
-  write_super_lockfs: called when VFS is locking a filesystem and
+  freeze_fs: called when VFS is locking a filesystem and
   	forcing it into a consistent state.  This method is currently
   	used by the Logical Volume Manager (LVM).
 
-  unlockfs: called when VFS is unlocking a filesystem and making it writable
+  unfreeze_fs: called when VFS is unlocking a filesystem and making it writable
   	again.
 
   statfs: called when the VFS needs to get filesystem statistics. This
diff -uprN -X linux-2.6.27-rc7/Documentation/dontdiff linux-2.6.27-rc7/fs/buffer.c linux-2.6.27-rc7-lockfs/fs/buffer.c
--- linux-2.6.27-rc7/fs/buffer.c	2008-09-22 07:29:55.000000000 +0900
+++ linux-2.6.27-rc7-lockfs/fs/buffer.c	2008-09-26 18:41:12.000000000 +0900
@@ -214,8 +214,8 @@ struct super_block *freeze_bdev(struct b
 
 		sync_blockdev(sb->s_bdev);
 
-		if (sb->s_op->write_super_lockfs)
-			sb->s_op->write_super_lockfs(sb);
+		if (sb->s_op->freeze_fs)
+			sb->s_op->freeze_fs(sb);
 	}
 
 	sync_blockdev(bdev);
@@ -235,8 +235,8 @@ void thaw_bdev(struct block_device *bdev
 	if (sb) {
 		BUG_ON(sb->s_bdev != bdev);
 
-		if (sb->s_op->unlockfs)
-			sb->s_op->unlockfs(sb);
+		if (sb->s_op->unfreeze_fs)
+			sb->s_op->unfreeze_fs(sb);
 		sb->s_frozen = SB_UNFROZEN;
 		smp_wmb();
 		wake_up(&sb->s_wait_unfrozen);
diff -uprN -X linux-2.6.27-rc7/Documentation/dontdiff linux-2.6.27-rc7/include/linux/fs.h linux-2.6.27-rc7-lockfs/includ
e/linux/fs.h
--- linux-2.6.27-rc7/include/linux/fs.h	2008-09-22 07:29:55.000000000 +0900
+++ linux-2.6.27-rc7-lockfs/include/linux/fs.h	2008-09-26 18:44:27.000000000 +0900
@@ -1314,8 +1314,8 @@ struct super_operations {
 	void (*put_super) (struct super_block *);
 	void (*write_super) (struct super_block *);
 	int (*sync_fs)(struct super_block *sb, int wait);
-	void (*write_super_lockfs) (struct super_block *);
-	void (*unlockfs) (struct super_block *);
+	int (*freeze_fs) (struct super_block *);
+	int (*unfreeze_fs) (struct super_block *);
 	int (*statfs) (struct dentry *, struct kstatfs *);
 	int (*remount_fs) (struct super_block *, int *, char *);
 	void (*clear_inode) (struct inode *);