[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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.





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]