xfs mknod regression

xfs mknod regression

[Bret Towe]

I hit a bug in 2.6.24-rc looks to be in 2.6.23 also so not sure how
long it's been there
with an xfs filesystem pbuilder has an issue using device files it
makes for chroot
the mknod command looks to work fine the file is created
however when attempting to use one of the created files you see
something like the below

ghoststar dev # echo "hi" > null
bash: null: No such device or address
ghoststar dev # ls -l null
crw-rw-rw- 1 root root 1, 3 2007-12-17 20:34 null
ghoststar dev # ls -l /dev/null
crw-rw-rw- 1 root root 1, 3 2007-10-05 17:29 /dev/null
ghoststar dev # echo "hi" > /dev/null
ghoststar dev #

I have not done any bisecting yet if needed I can narrow it down
the minor work around I found was to just mount an ext3 filesystem
where pbuilder builds

Re: xfs mknod regression

[David Chinner]

On Mon, Dec 17, 2007 at 08:48:40PM -0800, Bret Towe wrote:
> I hit a bug in 2.6.24-rc looks to be in 2.6.23 also so not sure how
> long it's been there
> with an xfs filesystem pbuilder has an issue using device files it
> makes for chroot
> the mknod command looks to work fine the file is created
> however when attempting to use one of the created files you see
> something like the below
> 
> ghoststar dev # echo "hi" > null
> bash: null: No such device or address
> ghoststar dev # ls -l null
> crw-rw-rw- 1 root root 1, 3 2007-12-17 20:34 null
> ghoststar dev # ls -l /dev/null
> crw-rw-rw- 1 root root 1, 3 2007-10-05 17:29 /dev/null
> ghoststar dev # echo "hi" > /dev/null
> ghoststar dev #

Ok. Works on 2.6.22. doesn't work on latest xfsdev. I'll
look into it.

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: xfs mknod regression

[Christoph Hellwig]

This was broken by my '[XFS] simplify xfs_create/mknod/symlink prototype',
which assigned the re-shuffled ondisk dev_t back to the rdev variable in
xfs_vn_mknod.  Because of that i_rdev is set to the ondisk dev_t instead
of the linux dev_t later down the function.

Fortunately the fix for it is trivial:  we can just remove the
assignment because xfs_revalidate_inode has done the proper job before
unlocking the inode.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_iops.c	2007-12-18 18:23:32.000000000 +0100
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c	2007-12-18 18:23:43.000000000 +0100
@@ -345,9 +345,7 @@ xfs_vn_mknod(
 		ASSERT(vp);
 		ip = vn_to_inode(vp);
 
-		if (S_ISCHR(mode) || S_ISBLK(mode))
-			ip->i_rdev = rdev;
-		else if (S_ISDIR(mode))
+		if (S_ISDIR(mode))
 			xfs_validate_fields(ip);
 		d_instantiate(dentry, ip);
 		xfs_validate_fields(dir);

Re: xfs mknod regression

[David Chinner]

On Tue, Dec 18, 2007 at 05:36:42PM +0000, Christoph Hellwig wrote:
> 
> This was broken by my '[XFS] simplify xfs_create/mknod/symlink prototype',
> which assigned the re-shuffled ondisk dev_t back to the rdev variable in
> xfs_vn_mknod.  Because of that i_rdev is set to the ondisk dev_t instead
> of the linux dev_t later down the function.
> 
> Fortunately the fix for it is trivial:  we can just remove the
> assignment because xfs_revalidate_inode has done the proper job before
> unlocking the inode.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_iops.c	2007-12-18 18:23:32.000000000 +0100
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c	2007-12-18 18:23:43.000000000 +0100
> @@ -345,9 +345,7 @@ xfs_vn_mknod(
>  		ASSERT(vp);
>  		ip = vn_to_inode(vp);
>  
> -		if (S_ISCHR(mode) || S_ISBLK(mode))
> -			ip->i_rdev = rdev;
> -		else if (S_ISDIR(mode))
> +		if (S_ISDIR(mode))
>  			xfs_validate_fields(ip);
>  		d_instantiate(dentry, ip);
>  		xfs_validate_fields(dir);

Thanks for this, Christoph - I'll run some tests on it and check it in.

Rafael - this is a regression introduced in 2.6.24-rc1 if you want to
track it.

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: xfs mknod regression

[Bret Towe]

On Dec 18, 2007 9:36 AM, Christoph Hellwig <hch@infradead.org> wrote:
>
> This was broken by my '[XFS] simplify xfs_create/mknod/symlink prototype',
> which assigned the re-shuffled ondisk dev_t back to the rdev variable in
> xfs_vn_mknod.  Because of that i_rdev is set to the ondisk dev_t instead
> of the linux dev_t later down the function.
>
> Fortunately the fix for it is trivial:  we can just remove the
> assignment because xfs_revalidate_inode has done the proper job before
> unlocking the inode.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

tested the patch on 2 machines and works fine for me

Thanks

Re: xfs mknod regression

[Rafael J. Wysocki]

On Wednesday, 19 of December 2007, David Chinner wrote:
> On Tue, Dec 18, 2007 at 05:36:42PM +0000, Christoph Hellwig wrote:
> > 
> > This was broken by my '[XFS] simplify xfs_create/mknod/symlink prototype',
> > which assigned the re-shuffled ondisk dev_t back to the rdev variable in
> > xfs_vn_mknod.  Because of that i_rdev is set to the ondisk dev_t instead
> > of the linux dev_t later down the function.
> > 
> > Fortunately the fix for it is trivial:  we can just remove the
> > assignment because xfs_revalidate_inode has done the proper job before
> > unlocking the inode.
> > 
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c
> > ===================================================================
> > --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_iops.c	2007-12-18 18:23:32.000000000 +0100
> > +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c	2007-12-18 18:23:43.000000000 +0100
> > @@ -345,9 +345,7 @@ xfs_vn_mknod(
> >  		ASSERT(vp);
> >  		ip = vn_to_inode(vp);
> >  
> > -		if (S_ISCHR(mode) || S_ISBLK(mode))
> > -			ip->i_rdev = rdev;
> > -		else if (S_ISDIR(mode))
> > +		if (S_ISDIR(mode))
> >  			xfs_validate_fields(ip);
> >  		d_instantiate(dentry, ip);
> >  		xfs_validate_fields(dir);
> 
> Thanks for this, Christoph - I'll run some tests on it and check it in.
> 
> Rafael - this is a regression introduced in 2.6.24-rc1 if you want to
> track it.

I do, thanks.

Added to the list, http://bugzilla.kernel.org/show_bug.cgi?id=9608 .
I'll close it when the fix is upstream.

Thanks,
Rafael