* [PATCH] cciss: Fix warnings (and bug on 1TB discs)
@ 2006-10-20 3:23 Matthew Wilcox
2006-10-20 3:27 ` Matthew Wilcox
0 siblings, 1 reply; 3+ messages in thread
From: Matthew Wilcox @ 2006-10-20 3:23 UTC (permalink / raw)
To: Mike Miller; +Cc: iss_storagedev, linux-scsi
CCISS was producing warnings about shifts being greater than the size
of the type and pointers being of incompatible type. Turns out this
is because it's calling do_div on a 32-bit quantity. Upon further
investigation, the sector_t total_size is being assigned to an int,
and then we're calling do_div on that int. Obviously, sector_div is
called for here, and I took the chance to refactor the code a little.
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index dcccaf2..bc66026 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1923,7 +1923,6 @@ static void cciss_geometry_inquiry(int c
{
int return_code;
unsigned long t;
- unsigned long rem;
memset(inq_buff, 0, sizeof(InquiryData_struct));
if (withirq)
@@ -1939,26 +1938,23 @@ static void cciss_geometry_inquiry(int c
printk(KERN_WARNING
"cciss: reading geometry failed, volume "
"does not support reading geometry\n");
- drv->block_size = block_size;
- drv->nr_blocks = total_size;
drv->heads = 255;
drv->sectors = 32; // Sectors per track
- t = drv->heads * drv->sectors;
- drv->cylinders = total_size;
- rem = do_div(drv->cylinders, t);
} else {
- drv->block_size = block_size;
- drv->nr_blocks = total_size;
drv->heads = inq_buff->data_byte[6];
drv->sectors = inq_buff->data_byte[7];
drv->cylinders = (inq_buff->data_byte[4] & 0xff) << 8;
drv->cylinders += inq_buff->data_byte[5];
drv->raid_level = inq_buff->data_byte[8];
- t = drv->heads * drv->sectors;
- if (t > 1) {
- drv->cylinders = total_size;
- rem = do_div(drv->cylinders, t);
- }
+ }
+ drv->block_size = block_size;
+ drv->nr_blocks = total_size;
+ t = drv->heads * drv->sectors;
+ if (t > 1) {
+ unsigned rem = sector_div(total_size, t);
+ if (rem)
+ total_size++;
+ drv->cylinders = total_size;
}
} else { /* Get geometry failed */
printk(KERN_WARNING "cciss: reading geometry failed\n");
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] cciss: Fix warnings (and bug on 1TB discs)
2006-10-20 3:23 [PATCH] cciss: Fix warnings (and bug on 1TB discs) Matthew Wilcox
@ 2006-10-20 3:27 ` Matthew Wilcox
2006-10-20 14:53 ` Miller, Mike (OS Dev)
0 siblings, 1 reply; 3+ messages in thread
From: Matthew Wilcox @ 2006-10-20 3:27 UTC (permalink / raw)
To: Mike Miller; +Cc: iss_storagedev, linux-scsi
On Thu, Oct 19, 2006 at 09:23:36PM -0600, Matthew Wilcox wrote:
> CCISS was producing warnings about shifts being greater than the size
> of the type and pointers being of incompatible type. Turns out this
> is because it's calling do_div on a 32-bit quantity. Upon further
> investigation, the sector_t total_size is being assigned to an int,
> and then we're calling do_div on that int. Obviously, sector_div is
> called for here, and I took the chance to refactor the code a little.
Oops, forgot:
Signed-off-by: Matthew Wilcox <matthew@wil.cx>
> diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
> index dcccaf2..bc66026 100644
> --- a/drivers/block/cciss.c
> +++ b/drivers/block/cciss.c
> @@ -1923,7 +1923,6 @@ static void cciss_geometry_inquiry(int c
> {
> int return_code;
> unsigned long t;
> - unsigned long rem;
>
> memset(inq_buff, 0, sizeof(InquiryData_struct));
> if (withirq)
> @@ -1939,26 +1938,23 @@ static void cciss_geometry_inquiry(int c
> printk(KERN_WARNING
> "cciss: reading geometry failed, volume "
> "does not support reading geometry\n");
> - drv->block_size = block_size;
> - drv->nr_blocks = total_size;
> drv->heads = 255;
> drv->sectors = 32; // Sectors per track
> - t = drv->heads * drv->sectors;
> - drv->cylinders = total_size;
> - rem = do_div(drv->cylinders, t);
> } else {
> - drv->block_size = block_size;
> - drv->nr_blocks = total_size;
> drv->heads = inq_buff->data_byte[6];
> drv->sectors = inq_buff->data_byte[7];
> drv->cylinders = (inq_buff->data_byte[4] & 0xff) << 8;
> drv->cylinders += inq_buff->data_byte[5];
> drv->raid_level = inq_buff->data_byte[8];
> - t = drv->heads * drv->sectors;
> - if (t > 1) {
> - drv->cylinders = total_size;
> - rem = do_div(drv->cylinders, t);
> - }
> + }
> + drv->block_size = block_size;
> + drv->nr_blocks = total_size;
> + t = drv->heads * drv->sectors;
> + if (t > 1) {
> + unsigned rem = sector_div(total_size, t);
> + if (rem)
> + total_size++;
> + drv->cylinders = total_size;
> }
> } else { /* Get geometry failed */
> printk(KERN_WARNING "cciss: reading geometry failed\n");
> -
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] cciss: Fix warnings (and bug on 1TB discs)
2006-10-20 3:27 ` Matthew Wilcox
@ 2006-10-20 14:53 ` Miller, Mike (OS Dev)
0 siblings, 0 replies; 3+ messages in thread
From: Miller, Mike (OS Dev) @ 2006-10-20 14:53 UTC (permalink / raw)
To: Matthew Wilcox, Andrew Morton, Jens Axboe
Cc: ISS StorageDev, linux-scsi, linux-kernel
>
> On Thu, Oct 19, 2006 at 09:23:36PM -0600, Matthew Wilcox wrote:
> > CCISS was producing warnings about shifts being greater
> than the size
> > of the type and pointers being of incompatible type. Turns
> out this
> > is because it's calling do_div on a 32-bit quantity. Upon further
> > investigation, the sector_t total_size is being assigned to an int,
> > and then we're calling do_div on that int. Obviously,
> sector_div is
> > called for here, and I took the chance to refactor the code
> a little.
>
> Oops, forgot:
> Signed-off-by: Matthew Wilcox <matthew@wil.cx>
Acked-by: Mike Miller <mike.miller@hp.com>
>
> > diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c index
> > dcccaf2..bc66026 100644
> > --- a/drivers/block/cciss.c
> > +++ b/drivers/block/cciss.c
> > @@ -1923,7 +1923,6 @@ static void cciss_geometry_inquiry(int c {
> > int return_code;
> > unsigned long t;
> > - unsigned long rem;
> >
> > memset(inq_buff, 0, sizeof(InquiryData_struct));
> > if (withirq)
> > @@ -1939,26 +1938,23 @@ static void cciss_geometry_inquiry(int c
> > printk(KERN_WARNING
> > "cciss: reading geometry failed, volume "
> > "does not support reading geometry\n");
> > - drv->block_size = block_size;
> > - drv->nr_blocks = total_size;
> > drv->heads = 255;
> > drv->sectors = 32; // Sectors per track
> > - t = drv->heads * drv->sectors;
> > - drv->cylinders = total_size;
> > - rem = do_div(drv->cylinders, t);
> > } else {
> > - drv->block_size = block_size;
> > - drv->nr_blocks = total_size;
> > drv->heads = inq_buff->data_byte[6];
> > drv->sectors = inq_buff->data_byte[7];
> > drv->cylinders =
> (inq_buff->data_byte[4] & 0xff) << 8;
> > drv->cylinders += inq_buff->data_byte[5];
> > drv->raid_level = inq_buff->data_byte[8];
> > - t = drv->heads * drv->sectors;
> > - if (t > 1) {
> > - drv->cylinders = total_size;
> > - rem = do_div(drv->cylinders, t);
> > - }
> > + }
> > + drv->block_size = block_size;
> > + drv->nr_blocks = total_size;
> > + t = drv->heads * drv->sectors;
> > + if (t > 1) {
> > + unsigned rem = sector_div(total_size, t);
> > + if (rem)
> > + total_size++;
> > + drv->cylinders = total_size;
> > }
> > } else { /* Get geometry failed */
> > printk(KERN_WARNING "cciss: reading geometry failed\n");
> > -
> > 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2006-10-20 14:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-20 3:23 [PATCH] cciss: Fix warnings (and bug on 1TB discs) Matthew Wilcox
2006-10-20 3:27 ` Matthew Wilcox
2006-10-20 14:53 ` Miller, Mike (OS Dev)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox