[Cluster-devel] [GFS2 PATCH 3/3] Remove i_mode passing from NFS File Handle

[Cluster-devel] [GFS2 PATCH 3/3] Remove i_mode passing from NFS File Handle

[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.

-- 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>

[Cluster-devel] [GFS2 PATCH 3/3] Remove i_mode passing from NFS File Handle

[Steven Whitehouse]

Hi,

On Mon, 2007-06-25 at 21:15 -0400, S. Wendy Cheng wrote:
> 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
> 

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.

[Cluster-devel] [GFS2 PATCH 3/3] Remove i_mode passing from NFS File Handle

[Wendy Cheng]

Revised patch ... Wendy

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