Re: 2.6.24-rc5-mm1 - wonky disk cache and CDROM behavior...

Valdis.Kletnieks
2007-12-18T15:14:59Z

[-- Attachment #1: Type: text/plain, Size: 2583 bytes --]

(Adding Dave Howells, his name is on iget-stop-isofs-from-using-read_inode.patch)

On Tue, 18 Dec 2007 10:37:32 +0800, Dave Young said:

> > I don't mind it failing the mount, but the oops seems excessive. I suspect
> > that *somewhere* in that stack trace, we're wanting something like a
> >
> > if (!foo_ptr)
> > return -EIO;
> >
> > but I admit not being competent enough to decide where that should be.
> >
>
> Hi,
> Could you please try the below patch:
>
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>
> ---
> fs/isofs/inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

With that patch applied, I took the ISO image (which I ended up reading on
another machine and copying over the net to get a complete usable image),
and dd'ed just the first 150M into another file, and tried to loopback mount
it. And I got:

# mount -o ro,loop /path/to/cd.partial.image /mnt/loop
mount: wrong fs type, bad option, bad superblock on /dev/loop0,
missing codepage or helper program, or other error
In some cases useful info is found in syslog - try
dmesg | tail or so

And my dmesg says:

[ 33.622073] ISO 9660 Extensions: Microsoft Joliet Level 3
[ 33.622125] attempt to access beyond end of device
[ 33.622129] loop0: rw=0, want=1284500, limit=300000
[ 33.622133] ISOFS: unable to read i-node block
Here is where we would oops before - now it errors out more reasonably:
[ 33.622140] ISOFS: changing to secondary root
[ 33.622148] attempt to access beyond end of device
[ 33.622151] loop0: rw=0, want=1284508, limit=300000
[ 33.622155] ISOFS: unable to read i-node block
[ 33.622159] isofs_fill_super: get root inode failed

So that fixes *this* bug. I looked in the -rc5-mm1 broken-out/, saw
the vast multitudes of 'iget-stop-<foofs>-from-using' patches, and decided
that somebody else will probably have to audit them for sanity.

In the iget-* series, there's some 184 'return ERR_PTR(-E<FOO>);' for some FOO,
and 3 other uses:

% grep ERR_PTR iget* | grep -v return
iget-stop-isofs-from-using-read_inode.patch:+ inode = ERR_PTR(ret);
iget-stop-jfs-from-using-iget-and-read_inode-try.patch:+ parent = ERR_PTR(-ENOMEM);
iget-stop-jfs-from-using-iget-and-read_inode-try.patch:- parent = ERR_PTR(-EACCES);
iget-stop-jfs-from-using-iget-and-read_inode-try.patch:- parent = ERR_PTR(-ENOMEM);

isofs is the only place we don't return a constant 'ERR_PTR(-EFOO)', but
cast somebody else's return value. I wish I understood what that tells us. ;)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]