[Cluster-devel] [GFS2 PATCH 1/3] Obtaining no_formal_ino from directory entry

[Cluster-devel] [GFS2 PATCH 1/3] Obtaining no_formal_ino from directory entry

[S. Wendy Cheng]

GFS2 lookup code doesn't ask for inode shared glock. This implies during 
in-memory inode creation for existing file, GFS2 will not disk-read in 
the inode contents. This leaves no_formal_ino un-initialized during 
lookup time. The un-initialized no_formal_ino is subsequently encoded 
into file handle. Clients will get ESTALE error whenever it tries to 
access these files.

-- Wendy

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

[Cluster-devel] [GFS2 PATCH 2/3] Obtaining no_formal_ino from directory entry

[S. Wendy Cheng]

Sorry, correct subject line from [GFS2 PATCH 1/3] to [GFS2 PATCH 2/3] ...

> GFS2 lookup code doesn't ask for inode shared glock. This implies 
> during in-memory inode creation for existing file, GFS2 will not 
> disk-read in the inode contents. This leaves no_formal_ino 
> un-initialized during lookup time. The un-initialized no_formal_ino is 
> subsequently encoded into file handle. Clients will get ESTALE error 
> whenever it tries to access these files.
>
> -- Wendy
>
>------------------------------------------------------------------------
>
> Signed-off-by: S. Wendy Cheng <wcheng@redhat.com>
>
> dir.c        |    6 ++++--
> inode.c      |   13 ++++++++-----
> inode.h      |    3 ++-
> ops_export.c |    2 +-
> ops_fstype.c |   19 ++++++++++++-------
> rgrp.c       |   10 +++++-----
> 6 files changed, 32 insertions(+), 21 deletions(-)
>
>--- gfs2-nfs1/fs/gfs2/inode.h	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/inode.h	2007-06-23 01:24:22.000000000 -0400
>@@ -47,7 +47,8 @@ static inline void gfs2_inum_out(const s
> 
> 
> void gfs2_inode_attr_in(struct gfs2_inode *ip);
>-struct inode *gfs2_inode_lookup(struct super_block *sb, u64 no_addr, unsigned type);
>+struct inode *gfs2_inode_lookup(struct super_block *sb, 
>+				struct gfs2_inum_host *ino, unsigned type);
> struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr);
> 
> int gfs2_inode_refresh(struct gfs2_inode *ip);
>--- gfs2-nfs1/fs/gfs2/dir.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/dir.c	2007-06-23 01:24:22.000000000 -0400
>@@ -1493,13 +1493,15 @@ struct inode *gfs2_dir_search(struct ino
> 	struct buffer_head *bh;
> 	struct gfs2_dirent *dent;
> 	struct inode *inode;
>+	struct gfs2_inum_host ino;
> 
> 	dent = gfs2_dirent_search(dir, name, gfs2_dirent_find, &bh);
> 	if (dent) {
> 		if (IS_ERR(dent))
> 			return ERR_PTR(PTR_ERR(dent));
>-		inode = gfs2_inode_lookup(dir->i_sb,
>-					  be64_to_cpu(dent->de_inum.no_addr),
>+		ino.no_formal_ino = be64_to_cpu(dent->de_inum.no_formal_ino);
>+		ino.no_addr = be64_to_cpu(dent->de_inum.no_addr);
>+		inode = gfs2_inode_lookup(dir->i_sb, &ino,
> 					  be16_to_cpu(dent->de_type));
> 		brelse(bh);
> 		return inode;
>--- gfs2-nfs1/fs/gfs2/inode.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/inode.c	2007-06-23 01:24:22.000000000 -0400
>@@ -86,9 +86,11 @@ static struct inode *gfs2_iget(struct su
>  * Returns: A VFS inode, or an error
>  */
> 
>-struct inode *gfs2_inode_lookup(struct super_block *sb, u64 no_addr, unsigned int type)
>+struct inode *gfs2_inode_lookup(struct super_block *sb, 
>+				struct gfs2_inum_host *ino,
>+				unsigned int type)
> {
>-	struct inode *inode = gfs2_iget(sb, no_addr);
>+	struct inode *inode = gfs2_iget(sb, ino->no_addr);
> 	struct gfs2_inode *ip = GFS2_I(inode);
> 	struct gfs2_glock *io_gl;
> 	int error;
>@@ -100,13 +102,14 @@ struct inode *gfs2_inode_lookup(struct s
> 		struct gfs2_sbd *sdp = GFS2_SB(inode);
> 		umode_t mode;
> 		inode->i_private = ip;
>+		ip->i_no_formal_ino = ino->no_formal_ino;
> 
>-		error = gfs2_glock_get(sdp, no_addr, &gfs2_inode_glops, CREATE, &ip->i_gl);
>+		error = gfs2_glock_get(sdp, ino->no_addr, &gfs2_inode_glops, CREATE, &ip->i_gl);
> 		if (unlikely(error))
> 			goto fail;
> 		ip->i_gl->gl_object = ip;
> 
>-		error = gfs2_glock_get(sdp, no_addr, &gfs2_iopen_glops, CREATE, &io_gl);
>+		error = gfs2_glock_get(sdp, ino->no_addr, &gfs2_iopen_glops, CREATE, &io_gl);
> 		if (unlikely(error))
> 			goto fail_put;
> 
>@@ -915,7 +918,7 @@ struct inode *gfs2_createi(struct gfs2_h
> 	if (error)
> 		goto fail_gunlock2;
> 
>-	inode = gfs2_inode_lookup(dir->i_sb, inum.no_addr, IF2DT(mode));
>+	inode = gfs2_inode_lookup(dir->i_sb, &inum, IF2DT(mode));
> 	if (IS_ERR(inode))
> 		goto fail_gunlock2;
> 
>--- gfs2-nfs1/fs/gfs2/ops_export.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/ops_export.c	2007-06-23 23:43:45.000000000 -0400
>@@ -245,7 +245,7 @@ static struct dentry *gfs2_get_dentry(st
> 	gfs2_glock_dq_uninit(&rgd_gh);
> 	gfs2_glock_dq_uninit(&ri_gh);
> 
>-	inode = gfs2_inode_lookup(sb, inum->no_addr, fh_obj->imode);
>+	inode = gfs2_inode_lookup(sb, inum, fh_obj->imode);
> 	if (!inode)
> 		goto fail;
> 	if (IS_ERR(inode)) {
>--- gfs2-nfs1/fs/gfs2/ops_fstype.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/ops_fstype.c	2007-06-23 23:42:38.000000000 -0400
>@@ -234,16 +234,16 @@ fail:
> }
> 
> static inline struct inode *gfs2_lookup_root(struct super_block *sb,
>-					     u64 no_addr)
>+					     struct gfs2_inum_host *ino)
> {
>-	return gfs2_inode_lookup(sb, no_addr, DT_DIR);
>+	return gfs2_inode_lookup(sb, ino, DT_DIR);
> }
> 
> static int init_sb(struct gfs2_sbd *sdp, int silent, int undo)
> {
> 	struct super_block *sb = sdp->sd_vfs;
> 	struct gfs2_holder sb_gh;
>-	u64 no_addr;
>+	struct gfs2_inum_host ino;
> 	struct inode *inode;
> 	int error = 0;
> 
>@@ -286,10 +286,12 @@ static int init_sb(struct gfs2_sbd *sdp,
> 	sb_set_blocksize(sb, sdp->sd_sb.sb_bsize);
> 
> 	/* Get the root inode */
>-	no_addr = sdp->sd_sb.sb_root_dir.no_addr;
>+	ino.no_formal_ino = 0;
> 	if (sb->s_type == &gfs2meta_fs_type)
>-		no_addr = sdp->sd_sb.sb_master_dir.no_addr;
>-	inode = gfs2_lookup_root(sb, no_addr);
>+		ino.no_addr = sdp->sd_sb.sb_master_dir.no_addr;
>+	else
>+		ino.no_addr = sdp->sd_sb.sb_root_dir.no_addr;
>+	inode = gfs2_lookup_root(sb, &ino);
> 	if (IS_ERR(inode)) {
> 		error = PTR_ERR(inode);
> 		fs_err(sdp, "can't read in root inode: %d\n", error);
>@@ -442,11 +444,14 @@ static int init_inodes(struct gfs2_sbd *
> 	int error = 0;
> 	struct gfs2_inode *ip;
> 	struct inode *inode;
>+	struct gfs2_inum_host ino;
> 
> 	if (undo)
> 		goto fail_qinode;
> 
>-	inode = gfs2_lookup_root(sdp->sd_vfs, sdp->sd_sb.sb_master_dir.no_addr);
>+	ino.no_formal_ino = 0;
>+	ino.no_addr = sdp->sd_sb.sb_master_dir.no_addr;
>+	inode = gfs2_lookup_root(sdp->sd_vfs, &ino);
> 	if (IS_ERR(inode)) {
> 		error = PTR_ERR(inode);
> 		fs_err(sdp, "can't read in master directory: %d\n", error);
>--- gfs2-nfs1/fs/gfs2/rgrp.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/rgrp.c	2007-06-23 01:24:22.000000000 -0400
>@@ -861,18 +861,18 @@ static struct inode *try_rgrp_unlink(str
> {
> 	struct inode *inode;
> 	u32 goal = 0;
>-	u64 ino;
>+	struct gfs2_inum_host ino = {.no_formal_ino = 0};
> 
> 	for(;;) {
> 		goal = rgblk_search(rgd, goal, GFS2_BLKST_UNLINKED,
> 				    GFS2_BLKST_UNLINKED);
> 		if (goal == 0)
> 			return 0;
>-		ino = goal + rgd->rd_data0;
>-		if (ino <= *last_unlinked)
>+		ino.no_addr = goal + rgd->rd_data0;
>+		if (ino.no_addr <= *last_unlinked)
> 			continue;
>-		*last_unlinked = ino;
>-		inode = gfs2_inode_lookup(rgd->rd_sbd->sd_vfs, ino, DT_UNKNOWN);
>+		*last_unlinked = ino.no_addr;
>+		inode = gfs2_inode_lookup(rgd->rd_sbd->sd_vfs, &ino, DT_UNKNOWN);
> 		if (!IS_ERR(inode))
> 			return inode;
> 	}
>
>  
>

[Cluster-devel] [GFS2 PATCH 1/3] Obtaining no_formal_ino from directory entry

[Steven Whitehouse]

Hi,

On Mon, 2007-06-25 at 21:14 -0400, S. Wendy Cheng wrote:
> GFS2 lookup code doesn't ask for inode shared glock. This implies during 
> in-memory inode creation for existing file, GFS2 will not disk-read in 
> the inode contents. This leaves no_formal_ino un-initialized during 
> lookup time. The un-initialized no_formal_ino is subsequently encoded 
> into file handle. Clients will get ESTALE error whenever it tries to 
> access these files.
> 
> -- Wendy
> 

Generally this looks good. Please don't add back the _host structure
though - just add an extra no_formal_ino argument to the lookup function
which is a u64,

Steve.

[Cluster-devel] [GFS2 PATCH 2/3] Obtaining no_formal_ino from directory entry

[S. Wendy Cheng]

Steven Whitehouse wrote:

>Hi,
>
>On Mon, 2007-06-25 at 21:14 -0400, S. Wendy Cheng wrote:
>  
>
>>GFS2 lookup code doesn't ask for inode shared glock. This implies during 
>>in-memory inode creation for existing file, GFS2 will not disk-read in 
>>the inode contents. This leaves no_formal_ino un-initialized during 
>>lookup time. The un-initialized no_formal_ino is subsequently encoded 
>>into file handle. Clients will get ESTALE error whenever it tries to 
>>access these files.
>>
>>-- Wendy
>>
>>    
>>
>
>Generally this looks good. Please don't add back the _host structure
>though - just add an extra no_formal_ino argument to the lookup function
>which is a u64,
>  
>

The gfs2_inode_lookup() parameter set would be too long. How about this 
... we will never use DT_WHT (bit 14) anyway so let's define:

struct inode *gfs2_inode_lookup(struct super_block *sb,
                                unsigned type,   // if invoked from 
gfs2_get_dentry by NFSD
                                                          // type would 
be DT_WHT.
                                u64 no_addr,
                                u64 no_formal_ino);

Then we would save one parameter in patch #3 that will use DT_WHT as 
nfsbypass param ? Patch enclosed.

-- Wendy



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

[Cluster-devel] [GFS2 PATCH 2/3] Obtaining no_formal_ino from directory entry

[S. Wendy Cheng]

Grr... there is a typo ... If we all agree with DT_WHT type, will 
re-submit the patch early tomorrow morning.. maybe we should hop on 
#sistina irc around 9:30 am EST to speed up the discussion ? Need to get 
this done and tested out (on RHEL 5.1) by noon tomorrow..

Kicked off the new kernel build now. G'night !

-- Wendy

> Steven Whitehouse wrote:
>
>> Hi,
>>
>> On Mon, 2007-06-25 at 21:14 -0400, S. Wendy Cheng wrote:
>>  
>>
>>> GFS2 lookup code doesn't ask for inode shared glock. This implies 
>>> during in-memory inode creation for existing file, GFS2 will not 
>>> disk-read in the inode contents. This leaves no_formal_ino 
>>> un-initialized during lookup time. The un-initialized no_formal_ino 
>>> is subsequently encoded into file handle. Clients will get ESTALE 
>>> error whenever it tries to access these files.
>>>
>>> -- Wendy
>>>
>>>   
>>
>>
>> Generally this looks good. Please don't add back the _host structure
>> though - just add an extra no_formal_ino argument to the lookup function
>> which is a u64,
>>  
>>
>
> The gfs2_inode_lookup() parameter set would be too long. How about 
> this ... we will never use DT_WHT (bit 14) anyway so let's define:
>
> struct inode *gfs2_inode_lookup(struct super_block *sb,
>                                unsigned type,   // if invoked from 
> gfs2_get_dentry by NFSD
>                                                          // type would 
> be DT_WHT.
>                                u64 no_addr,
>                                u64 no_formal_ino);
>
> Then we would save one parameter in patch #3 that will use DT_WHT as 
> nfsbypass param ? Patch enclosed.
>
> -- Wendy
>
>
>
>------------------------------------------------------------------------
>
> Signed-off-by: S. Wendy Cheng <wcheng@redhat.com>
>
> dir.c        |    7 ++++---
> inode.c      |   12 +++++++++---
> inode.h      |    3 ++-
> ops_export.c |    4 +++-
> ops_fstype.c |    2 +-
> rgrp.c       |   11 ++++++-----
> 6 files changed, 25 insertions(+), 14 deletions(-)
>
>--- gfs2-nfs1/fs/gfs2/inode.h	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/inode.h	2007-06-27 00:17:41.000000000 -0400
>@@ -47,7 +47,8 @@ static inline void gfs2_inum_out(const s
> 
> 
> void gfs2_inode_attr_in(struct gfs2_inode *ip);
>-struct inode *gfs2_inode_lookup(struct super_block *sb, u64 no_addr, unsigned type);
>+struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned type, 
>+				u64 no_addr, u64 no_formal_ino);
> struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr);
> 
> int gfs2_inode_refresh(struct gfs2_inode *ip);
>--- gfs2-nfs1/fs/gfs2/inode.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/inode.c	2007-06-27 00:23:05.000000000 -0400
>@@ -86,9 +86,12 @@ static struct inode *gfs2_iget(struct su
>  * Returns: A VFS inode, or an error
>  */
> 
>-struct inode *gfs2_inode_lookup(struct super_block *sb, u64 no_addr, unsigned int type)
>+struct inode *gfs2_inode_lookup(struct super_block *sb, 
>+				unsigned int type,
>+				u64 no_addr,
>+				u64 no_formal_ino)
> {
>-	struct inode *inode = gfs2_iget(sb, no_addr);
>+	struct inode *inode = gfs2_iget(sb, ino->no_addr);
> 	struct gfs2_inode *ip = GFS2_I(inode);
> 	struct gfs2_glock *io_gl;
> 	int error;
>@@ -100,6 +103,7 @@ struct inode *gfs2_inode_lookup(struct s
> 		struct gfs2_sbd *sdp = GFS2_SB(inode);
> 		umode_t mode;
> 		inode->i_private = ip;
>+		ip->i_no_formal_ino = no_formal_ino;
> 
> 		error = gfs2_glock_get(sdp, no_addr, &gfs2_inode_glops, CREATE, &ip->i_gl);
> 		if (unlikely(error))
>@@ -915,7 +919,9 @@ struct inode *gfs2_createi(struct gfs2_h
> 	if (error)
> 		goto fail_gunlock2;
> 
>-	inode = gfs2_inode_lookup(dir->i_sb, inum.no_addr, IF2DT(mode));
>+	inode = gfs2_inode_lookup(dir->i_sb, IF2DT(mode),
>+					inum.no_addr,
>+					inum.no_formal_ino);
> 	if (IS_ERR(inode))
> 		goto fail_gunlock2;
> 
>--- gfs2-nfs1/fs/gfs2/dir.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/dir.c	2007-06-27 00:19:22.000000000 -0400
>@@ -1498,9 +1498,10 @@ struct inode *gfs2_dir_search(struct ino
> 	if (dent) {
> 		if (IS_ERR(dent))
> 			return ERR_PTR(PTR_ERR(dent));
>-		inode = gfs2_inode_lookup(dir->i_sb,
>-					  be64_to_cpu(dent->de_inum.no_addr),
>-					  be16_to_cpu(dent->de_type));
>+		inode = gfs2_inode_lookup(dir->i_sb, 
>+				be16_to_cpu(dent->de_type),
>+				be64_to_cpu(dent->de_inum.no_addr),
>+				be64_to_cpu(dent->de_inum.no_formal_ino);
> 		brelse(bh);
> 		return inode;
> 	}
>--- gfs2-nfs1/fs/gfs2/ops_export.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/ops_export.c	2007-06-27 00:24:38.000000000 -0400
>@@ -245,7 +245,9 @@ static struct dentry *gfs2_get_dentry(st
> 	gfs2_glock_dq_uninit(&rgd_gh);
> 	gfs2_glock_dq_uninit(&ri_gh);
> 
>-	inode = gfs2_inode_lookup(sb, inum->no_addr, fh_obj->imode);
>+	inode = gfs2_inode_lookup(sb, fh_obj->imode,
>+					inum->no_addr,
>+					inum->no_formal_ino);
> 	if (!inode)
> 		goto fail;
> 	if (IS_ERR(inode)) {
>--- gfs2-nfs1/fs/gfs2/ops_fstype.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/ops_fstype.c	2007-06-27 00:30:22.000000000 -0400
>@@ -236,7 +236,7 @@ fail:
> static inline struct inode *gfs2_lookup_root(struct super_block *sb,
> 					     u64 no_addr)
> {
>-	return gfs2_inode_lookup(sb, no_addr, DT_DIR);
>+	return gfs2_inode_lookup(sb, DT_DIR, no_addr, 0);
> }
> 
> static int init_sb(struct gfs2_sbd *sdp, int silent, int undo)
>--- gfs2-nfs1/fs/gfs2/rgrp.c	2007-06-22 22:11:42.000000000 -0400
>+++ gfs2-nfs2/fs/gfs2/rgrp.c	2007-06-27 00:31:44.000000000 -0400
>@@ -861,18 +861,19 @@ static struct inode *try_rgrp_unlink(str
> {
> 	struct inode *inode;
> 	u32 goal = 0;
>-	u64 ino;
>+	u64 no_addr;
> 
> 	for(;;) {
> 		goal = rgblk_search(rgd, goal, GFS2_BLKST_UNLINKED,
> 				    GFS2_BLKST_UNLINKED);
> 		if (goal == 0)
> 			return 0;
>-		ino = goal + rgd->rd_data0;
>-		if (ino <= *last_unlinked)
>+		no_addr = goal + rgd->rd_data0;
>+		if (no_addr <= *last_unlinked)
> 			continue;
>-		*last_unlinked = ino;
>-		inode = gfs2_inode_lookup(rgd->rd_sbd->sd_vfs, ino, DT_UNKNOWN);
>+		*last_unlinked = ino.no_addr;
>+		inode = gfs2_inode_lookup(rgd->rd_sbd->sd_vfs, DT_UNKNOWN,
>+					no_addr, 0);
> 		if (!IS_ERR(inode))
> 			return inode;
> 	}
>  
>

[Cluster-devel] [GFS2 PATCH 2/3] Obtaining no_formal_ino from directory entry

[Steven Whitehouse]

Hi,

On Wed, 2007-06-27 at 01:15 -0400, S. Wendy Cheng wrote:
> Steven Whitehouse wrote:
> 
> >Hi,
> >
> >On Mon, 2007-06-25 at 21:14 -0400, S. Wendy Cheng wrote:
> >  
> >
> >>GFS2 lookup code doesn't ask for inode shared glock. This implies during 
> >>in-memory inode creation for existing file, GFS2 will not disk-read in 
> >>the inode contents. This leaves no_formal_ino un-initialized during 
> >>lookup time. The un-initialized no_formal_ino is subsequently encoded 
> >>into file handle. Clients will get ESTALE error whenever it tries to 
> >>access these files.
> >>
> >>-- Wendy
> >>
> >>    
> >>
> >
> >Generally this looks good. Please don't add back the _host structure
> >though - just add an extra no_formal_ino argument to the lookup function
> >which is a u64,
> >  
> >
> 
> The gfs2_inode_lookup() parameter set would be too long. How about this 
> ... we will never use DT_WHT (bit 14) anyway so let's define:
> 
> struct inode *gfs2_inode_lookup(struct super_block *sb,
>                                 unsigned type,   // if invoked from 
> gfs2_get_dentry by NFSD
>                                                           // type would 
> be DT_WHT.
>                                 u64 no_addr,
>                                 u64 no_formal_ino);
> 
> Then we would save one parameter in patch #3 that will use DT_WHT as 
> nfsbypass param ? Patch enclosed.
> 
Ugh! I'd rather not misuse the DT_ types. We can just use DT_UNKNOWN I
think as we can tell the difference between NFS and unlinked recovery
simply by looking to see if no_formal_ino is non-zero or not (since zero
can never be a valid no_formal_ino). Do you think thats ok?

Steve.

> -- Wendy
> 
> 
> 

[Cluster-devel] [GFS2 PATCH 2/3] Obtaining no_formal_ino from directory entry

[Wendy Cheng]

Revised patch .... Wendy



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

[Cluster-devel] [GFS2 PATCH 2/3] Obtaining no_formal_ino from directory entry

[Steven Whitehouse]

Hi,

On Wed, 2007-06-27 at 17:07 -0400, Wendy Cheng wrote:
> Revised patch .... Wendy
> 
> 
> 

Both patches applied. Please check your patches to ensure you don't add
trailing whitespace in future. Thanks,

Steve.