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

[Valdis . Kletnieks]

(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@xxxxxxxxx> 
> 
> ---
> 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: pgp00000.pgp
Description: PGP signature