Skip to contentSkip to conversation

Copy i_flags to gfs2 inode flags on write

PATCH In review Jan Kara 2 participants 3 messages Apr 23, 2007 – Apr 26, 2007

lore
Share & tools
lore

Jan Kara

author
#0 lore raw

PATCH 1/1 Copy i_flags to gfs2 inode flags on write

Hi,

attached is a patch to copy i_flags to GFS2 specific i_di_di_flags. The
patch is compile-tested only. Steven, does it look OK to you?

Honza

Signature · 9 lines
Jan Kara <[email protected]>
SuSE CR Labs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gfs2-2.6.21-rc6-propagate_flags.diff
Type: text/x-patch
Size: 4621 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20070423/d9dceb72/attachment.bin>
#1 lore raw

Hi,

Quoted 7 lines · Jan Kara

Hi,

attached is a patch to copy i_flags to GFS2 specific i_di_di_flags. The
patch is compile-tested only. Steven, does it look OK to you?

Honza

I can't see why we need this at the moment... what other interface is there aside from the ioctl() for setting these flags?

Also:

        diff -rupX /home/jack/.kerndiffexclude linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.c linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.c
        ---
        linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.c    2007-04-10 17:09:55.000000000 +0200
        +++
        linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.c     2007-04-23 20:10:55.000000000 +0200
        @@ -207,6 +207,7 @@ static int gfs2_get_flags(struct file *f
                if (error)
                        return error;
         
        +       gfs2_get_inode_flags(ip);
                fsflags = fsflags_cvt(gfs2_to_fsflags,
        ip->i_di.di_flags);

Why do you call gfs2_get_inode_flags() here? There is no reason that the gfs2 flags will not match the vfs inode flags at this point that I can see (its a bug if they don't match). It is not valid to change the flags while only holding a read lock and when outside of a transaction anyway.

        diff -rupX /home/jack/.kerndiffexclude linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.h linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.h
        ---
        linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.h    2007-02-07 12:03:23.000000000 +0100
        +++
        linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.h     2007-04-23 20:11:43.000000000 +0200
        @@ -18,6 +18,7 @@ extern int gfs2_internal_read(struct gfs
                                      struct file_ra_state *ra_state,
                                      char *buf, loff_t *pos, unsigned
        size);
         extern void gfs2_set_inode_flags(struct inode *inode);
        +extern void gfs2_get_inode_flags(struct gfs2_inode *inode);

Please use *ip for struct gfs2_inode and *inode for struct inode in common with the rest of the code as it helps avoid confusion,

Steve.

author
#2 lore raw

Hi,

Quoted 8 lines · Steven Whitehouse
Quoted 5 lines · Jan Kara

attached is a patch to copy i_flags to GFS2 specific i_di_di_flags. The
patch is compile-tested only. Steven, does it look OK to you?

Honza

I can't see why we need this at the moment... what other interface is there aside from the ioctl() for setting these flags?

Hm, VFS quota subsystem sets flags on quota files. Given that GFS2
doesn't use VFS quota, you are right that currently there's noone who could change the flags in GFS2 inode. But still I'd find it more correct to store the flags, especially if it does not cost you much. So do you agree with the idea of the patch?

Quoted 15 lines

Also:

        diff -rupX /home/jack/.kerndiffexclude linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.c linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.c
        ---
        linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.c    2007-04-10 17:09:55.000000000 +0200
        +++
        linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.c     2007-04-23 20:10:55.000000000 +0200
        @@ -207,6 +207,7 @@ static int gfs2_get_flags(struct file *f
                if (error)
                        return error;
         
        +       gfs2_get_inode_flags(ip);
                fsflags = fsflags_cvt(gfs2_to_fsflags,
        ip->i_di.di_flags);
Quoted 1 line

Why do you call gfs2_get_inode_flags() here? There is no reason that the

I guess what I have written above explains it...

Quoted 3 lines

gfs2 flags will not match the vfs inode flags at this point that I can see (its a bug if they don't match). It is not valid to change the flags while only holding a read lock and when outside of a transaction anyway.

Oh, right. So just merging GFS2 flags with VFS inode flags to some
temporary variable should fix the problem.

Quoted 12 lines
        diff -rupX /home/jack/.kerndiffexclude linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.h linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.h
        ---
        linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.h    2007-02-07 12:03:23.000000000 +0100
        +++
        linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.h     2007-04-23 20:11:43.000000000 +0200
        @@ -18,6 +18,7 @@ extern int gfs2_internal_read(struct gfs
                                      struct file_ra_state *ra_state,
                                      char *buf, loff_t *pos, unsigned
        size);
         extern void gfs2_set_inode_flags(struct inode *inode);
        +extern void gfs2_get_inode_flags(struct gfs2_inode *inode);
Quoted 2 lines

Please use *ip for struct gfs2_inode and *inode for struct inode in common with the rest of the code as it helps avoid confusion,

Ups, I swear I wanted to ;). But somehow "inode" crept in...

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SuSE CR Labs