2007-12-18 04:48:51

by Bret Towe

[permalink] [raw]
Subject: xfs mknod regression

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


2007-12-18 13:47:10

by David Chinner

[permalink] [raw]
Subject: Re: xfs mknod regression

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

2007-12-18 17:36:52

by Christoph Hellwig

[permalink] [raw]
Subject: Re: xfs mknod regression


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 <[email protected]>

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

2007-12-19 00:38:18

by David Chinner

[permalink] [raw]
Subject: Re: xfs mknod regression

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 <[email protected]>
>
> 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

2007-12-19 02:46:39

by Bret Towe

[permalink] [raw]
Subject: Re: xfs mknod regression

On Dec 18, 2007 9:36 AM, Christoph Hellwig <[email protected]> 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 <[email protected]>

tested the patch on 2 machines and works fine for me

Thanks

2007-12-19 23:45:41

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: xfs mknod regression

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 <[email protected]>
> >
> > 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