Re: [PATCH 1/1] scsicam_getgeo_odd_sector_size

[Brian King <brking@us.ibm.com>]

James Bottomley wrote:
> On Wed, 2004-10-13 at 14:55, Brian King wrote:
> 
>>James Bottomley wrote:
>>
>>>How does the attached work for you?
>>
>>Not very well. It looks like it blows up in create_buffers() with this patch.
>>Since the block size (522) is not a multiple of the host page size, we end up
>>decrementing offset to a negative number in create_buffers, which then hits a BUG()
>>in set_bh_page. It looks to me like the block layer assumes the device block size
>>is a multiple of PAGE_SIZE, in more than one place. Here is the latest backtrace:
> 
> 
> OK it seems that we get to bogus block size confusion before we see the
> device's zero size.  The least nasty fix seems to be to set a power of
> two blocksize as well as zeroing the capacity for this problem.
> 
> How does the attached fare?

Much better. I no longer get an oops. However, I do get the following confusing
console output since we are lying to the layers above regarding the sector size:

sdl : unsupported sector size 522.
SCSI device sdl: 0 512-byte hdwr sectors (0 MB)

As far as I am concerned, this isn't a showstopper for this patch,
just an observation.

Thanks

-Brian

> 
> James
> 
> ===== drivers/scsi/sd.c 1.160 vs edited =====
> --- 1.160/drivers/scsi/sd.c	2004-09-19 11:55:03 -05:00
> +++ edited/drivers/scsi/sd.c	2004-10-13 16:29:37 -05:00
> @@ -1125,6 +1125,13 @@
>  		 * For this reason, we leave the thing in the table.
>  		 */
>  		sdkp->capacity = 0;
> +		/*
> +		 * set a bogus sector size so the normal read/write
> +		 * logic in the block layer will eventually refuse any
> +		 * request on this device without tripping over power
> +		 * of two sector size assumptions
> +		 */
> +		sector_size = 512;
>  	}
>  	{
>  		/*
> ===== drivers/scsi/scsicam.c 1.16 vs edited =====
> --- 1.16/drivers/scsi/scsicam.c	2004-06-19 09:45:02 -05:00
> +++ edited/drivers/scsi/scsicam.c	2004-10-13 10:38:25 -05:00
> @@ -29,10 +29,11 @@
>  	unsigned char *res = kmalloc(66, GFP_KERNEL);
>  	if (res) {
>  		struct block_device *bdev = dev->bd_contains;
> -		struct buffer_head *bh = __bread(bdev, 0, block_size(bdev));
> -		if (bh) {
> -			memcpy(res, bh->b_data + 0x1be, 66);
> -			brelse(bh);
> +		Sector sect;
> +		void *data = read_dev_sector(bdev, 0, &sect);
> +		if (data) {
> +			memcpy(res, data + 0x1be, 66);
> +			put_dev_sector(sect);
>  		} else {
>  			kfree(res);
>  			res = NULL;
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center