Skip to contentSkip to conversation

Remove i_mode passing from NFS File Handle

PATCH New S. Wendy Cheng 2 participants 3 messages Jun 26, 2007 – Jun 27, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 3/3 Remove i_mode passing from NFS File Handle

GFS2 has been passing i_mode within NFS File Handle. Other than the wrong assumption that there is always room for this extra 16 bit value, the current gfs2_get_dentry doesn't really need the i_mode to work correctly. Note that GFS2 NFS code does go thru the same lookup code path as direct file access route (where the mode is obtained from name lookup) but gfs2_get_dentry() is coded for different purpose. It is not used during lookup time. It is part of the file access procedure call. When the call is invoked, if on-disk inode is not in-memory, it has to be read-in. This makes i_mode passing a useless overhead.

-- Wendy

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 03_gfs2_specsfs.patch
Type: text/x-patch
Size: 8615 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20070625/a9a908a6/attachment.bin>

#1 lore raw

PATCH 3/3 Remove i_mode passing from NFS File Handle

Hi,

Quoted 10 lines · S. Wendy Cheng

GFS2 has been passing i_mode within NFS File Handle. Other than the wrong assumption that there is always room for this extra 16 bit value, the current gfs2_get_dentry doesn't really need the i_mode to work correctly. Note that GFS2 NFS code does go thru the same lookup code path as direct file access route (where the mode is obtained from name lookup) but gfs2_get_dentry() is coded for different purpose. It is not used during lookup time. It is part of the file access procedure call. When the call is invoked, if on-disk inode is not in-memory, it has to be read-in. This makes i_mode passing a useless overhead.

Quoted 2 lines

-- Wendy

As per the second patch in the series, please avoid adding new _host structures since the plan is to eliminate them over time. Also some other comments:

- } else if (S_ISLNK(mode)) {
- inode->i_op = &gfs2_symlink_iops;
- } else {
- inode->i_op = &gfs2_dev_iops;
- }
+ (void) gfs2_set_iop(inode);
^^^^^ cast not needed

-       ip = GFS2_I(inode);
        igrab(inode);

+ ip = GFS2_I(inode);

The above makes no difference... the dentry has a ref on the inode at this point in time, so that the igrab will always succeed.

+
+ /* Pick up the works we bypass in gfs2_inode_lookup */
+ if (inode->i_state & I_NEW) {
+ inode->i_mode = GFS2_I(inode)->i_inode.i_mode;
^^^^ This appears to assign the value of the field to
itself
+ (void) gfs2_set_iop(inode);
^^^^ again the cast isn't required
+ }
+

I probably need to think about this a bit more too.... but its now 3:36am by my body clock and I'm not thinking too straight, so I'll have to leave that for tomorrow,

Steve.