Re: oops w/ 2.6.2-mm1 on ppc32

[Jens Axboe <axboe@suse.de>]

On Sun, Feb 15 2004, Andrew Morton wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> > On Sun, 2004-02-15 at 18:41, Marc Heckmann wrote:
> > > It happened while the machine was waking up from sleep. There were no
> > > UDF or ISO filesystems mounted at the time, in fact, there wasn't even
> > > a cd in the drive. The "autorun" process was running though (polls the
> > > cdrom drive, to see if a disc has been inserted...). There were some
> > > request timeouts on the cdrom drive (hdc) just before, it went to
> > > sleep (system was idle at the time, I wasn't even at home).
> > > 
> > > Here is the kernel output before and after the machine went to sleep. The Oops
> > > is at the bottom.
> > 
> > Looks like CD went berserk, and something didn't deal with the
> > error correctly... I don't know those code path in there
> > very well... Can you paste more of the ide-cd errors,
> > those are weird.
> 
> Note that isofs_fill_super() calls sb_bread() before setting the blocksize.
> For this it is relying on blockdev.bd_block_size being set up
> appropriately.
> 
> Which all tends to imply that the underlying queue's ->hardsect_size is
> very wrong.
> 
> The code which is responsible for setting up the queue's hardsect_size
> appears to live in cdrom_read_toc():
> 
> 	/* Check to see if the existing data is still valid.
> 	   If it is, just return. */
> 	(void) cdrom_check_status(drive, sense);
> 
> 	if (CDROM_STATE_FLAGS(drive)->toc_valid)
> 		return 0;
> 
> 	/* Try to get the total cdrom capacity and sector size. */
> 	stat = cdrom_read_capacity(drive, &toc->capacity, &sectors_per_frame,
> 				   sense);
> 	if (stat)
> 		toc->capacity = 0x1fffff;
> 
> 	set_capacity(drive->disk, toc->capacity * sectors_per_frame);
> 	blk_queue_hardsect_size(drive->queue,
> 				sectors_per_frame << SECTOR_BITS);
> 
> I'm wondering about that `return 0;' in there.  That will return "success"
> even though we haven't set up half the things which should have been set
> up.
> 
> Jens, should we be returning some sort of error code there?

I'll have a look to see if it can go wrong, but ->toc_valid should never
be set if the hardsector stuff etc hasn't been set up yet.

-- 
Jens Axboe