2008-09-22 10:56:34

by Takashi Sato

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

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 <[email protected]>
Signed-off-by: Masayuki Hamaguchi <[email protected]>
---
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 *);

2008-09-22 10:59:56

by Christoph Hellwig

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

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.

2008-09-22 12:52:09

by Takashi Sato

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

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