* cciss: warning: right shift count >= width of type
@ 2007-08-12 0:28 Jesper Juhl
2007-08-12 0:56 ` Jesper Juhl
2007-08-12 1:21 ` Rene Herman
0 siblings, 2 replies; 9+ messages in thread
From: Jesper Juhl @ 2007-08-12 0:28 UTC (permalink / raw)
To: linux-scsi; +Cc: Linux Kernel Mailing List, Jesper Juhl
Hi,
I've been building some randconfig kernels lately and I've noticed
this in a few builds :
drivers/block/cciss.c:2614: warning: right shift count >= width of type
drivers/block/cciss.c:2615: warning: right shift count >= width of type
drivers/block/cciss.c:2616: warning: right shift count >= width of type
The code in question is this :
static void do_cciss_request(struct request_queue *q)
{
...
sector_t start_blk;
...
c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB
c->Request.CDB[3]= (start_blk >> 48) & 0xff;
c->Request.CDB[4]= (start_blk >> 40) & 0xff;
...
}
The problem stems from these lines in include/linux/types.h :
...
#ifdef CONFIG_LBD
typedef u64 sector_t;
#else
typedef unsigned long sector_t;
#endif
...
So on a 32bit arch without CONFIG_LBD, sector_t is going to be 32 bits wide.
Thus it seems gcc is absolutely right in complaining about those
56, 48 & 40 bit shifts, since they are indeed wider than the type
in the "!CONFIG_LBD on a 32bit arch" case.
I must admit that I have no idear what the proper way to deal with
that is, so I'll just report it so hopefully someone else can fix it.
By the way; I'm building current Linus git tree, head at commit
ac07860264bd2b18834d3fa3be47032115524cea
Kind regards,
Jesper Juhl
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: cciss: warning: right shift count >= width of type 2007-08-12 0:28 cciss: warning: right shift count >= width of type Jesper Juhl @ 2007-08-12 0:56 ` Jesper Juhl 2007-08-12 1:25 ` [PATCH] " Rene Herman 2007-08-12 1:21 ` Rene Herman 1 sibling, 1 reply; 9+ messages in thread From: Jesper Juhl @ 2007-08-12 0:56 UTC (permalink / raw) To: linux-scsi Cc: Linux Kernel Mailing List, Jesper Juhl, Mike Miller, iss_storagedev (whoops, forgot to add maintainer to Cc - now added) On 12/08/07, Jesper Juhl <jesper.juhl@gmail.com> wrote: > Hi, > > I've been building some randconfig kernels lately and I've noticed > this in a few builds : > > drivers/block/cciss.c:2614: warning: right shift count >= width of type > drivers/block/cciss.c:2615: warning: right shift count >= width of type > drivers/block/cciss.c:2616: warning: right shift count >= width of type > > The code in question is this : > > static void do_cciss_request(struct request_queue *q) > { > ... > sector_t start_blk; > ... > c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB > c->Request.CDB[3]= (start_blk >> 48) & 0xff; > c->Request.CDB[4]= (start_blk >> 40) & 0xff; > ... > } > > > The problem stems from these lines in include/linux/types.h : > ... > #ifdef CONFIG_LBD > typedef u64 sector_t; > #else > typedef unsigned long sector_t; > #endif > ... > > So on a 32bit arch without CONFIG_LBD, sector_t is going to be 32 bits wide. > Thus it seems gcc is absolutely right in complaining about those > 56, 48 & 40 bit shifts, since they are indeed wider than the type > in the "!CONFIG_LBD on a 32bit arch" case. > > > I must admit that I have no idear what the proper way to deal with > that is, so I'll just report it so hopefully someone else can fix it. > > By the way; I'm building current Linus git tree, head at commit > ac07860264bd2b18834d3fa3be47032115524cea > > > Kind regards, > Jesper Juhl > > > -- Jesper Juhl <jesper.juhl@gmail.com> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] Re: cciss: warning: right shift count >= width of type 2007-08-12 0:56 ` Jesper Juhl @ 2007-08-12 1:25 ` Rene Herman 2007-08-12 1:54 ` Rene Herman 0 siblings, 1 reply; 9+ messages in thread From: Rene Herman @ 2007-08-12 1:25 UTC (permalink / raw) To: Jesper Juhl Cc: linux-scsi, Linux Kernel Mailing List, Mike Miller, iss_storagedev [-- Attachment #1: Type: text/plain, Size: 1829 bytes --] On 08/12/2007 02:56 AM, Jesper Juhl wrote: > (whoops, forgot to add maintainer to Cc - now added) Ehm... too late... > On 12/08/07, Jesper Juhl <jesper.juhl@gmail.com> wrote: >> Hi, >> >> I've been building some randconfig kernels lately and I've noticed >> this in a few builds : >> >> drivers/block/cciss.c:2614: warning: right shift count >= width of type >> drivers/block/cciss.c:2615: warning: right shift count >= width of type >> drivers/block/cciss.c:2616: warning: right shift count >= width of type >> >> The code in question is this : >> >> static void do_cciss_request(struct request_queue *q) >> { >> ... >> sector_t start_blk; >> ... >> c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB >> c->Request.CDB[3]= (start_blk >> 48) & 0xff; >> c->Request.CDB[4]= (start_blk >> 40) & 0xff; >> ... >> } >> >> >> The problem stems from these lines in include/linux/types.h : >> ... >> #ifdef CONFIG_LBD >> typedef u64 sector_t; >> #else >> typedef unsigned long sector_t; >> #endif >> ... >> >> So on a 32bit arch without CONFIG_LBD, sector_t is going to be 32 bits wide. >> Thus it seems gcc is absolutely right in complaining about those >> 56, 48 & 40 bit shifts, since they are indeed wider than the type >> in the "!CONFIG_LBD on a 32bit arch" case. >> >> >> I must admit that I have no idear what the proper way to deal with >> that is, so I'll just report it so hopefully someone else can fix it. >> >> By the way; I'm building current Linus git tree, head at commit >> ac07860264bd2b18834d3fa3be47032115524cea Well, given that it's explicitly treating start_blk as a 64-bit value, the proper fix is probably to cast start_blk to an actual (guaranteed) 64-bit value. Untested, uncompiled, and someone else may disagree: Rene. [-- Attachment #2: cciss.diff --] [-- Type: text/plain, Size: 1174 bytes --] diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c index 5acc6c4..fa2eec8 100644 --- a/drivers/block/cciss.c +++ b/drivers/block/cciss.c @@ -2609,13 +2609,13 @@ static void do_cciss_request(request_queue_t *q) } else { c->Request.CDBLen = 16; c->Request.CDB[1]= 0; - c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB - c->Request.CDB[3]= (start_blk >> 48) & 0xff; - c->Request.CDB[4]= (start_blk >> 40) & 0xff; - c->Request.CDB[5]= (start_blk >> 32) & 0xff; - c->Request.CDB[6]= (start_blk >> 24) & 0xff; - c->Request.CDB[7]= (start_blk >> 16) & 0xff; - c->Request.CDB[8]= (start_blk >> 8) & 0xff; + c->Request.CDB[2]= ((u64)start_blk >> 56) & 0xff; //MSB + c->Request.CDB[3]= ((u64)start_blk >> 48) & 0xff; + c->Request.CDB[4]= ((u64)start_blk >> 40) & 0xff; + c->Request.CDB[5]= ((u64)start_blk >> 32) & 0xff; + c->Request.CDB[6]= ((u64)start_blk >> 24) & 0xff; + c->Request.CDB[7]= ((u64)start_blk >> 16) & 0xff; + c->Request.CDB[8]= ((u64)start_blk >> 8) & 0xff; c->Request.CDB[9]= start_blk & 0xff; c->Request.CDB[10]= (creq->nr_sectors >> 24) & 0xff; c->Request.CDB[11]= (creq->nr_sectors >> 16) & 0xff; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Re: cciss: warning: right shift count >= width of type 2007-08-12 1:25 ` [PATCH] " Rene Herman @ 2007-08-12 1:54 ` Rene Herman 0 siblings, 0 replies; 9+ messages in thread From: Rene Herman @ 2007-08-12 1:54 UTC (permalink / raw) To: Jesper Juhl Cc: linux-scsi, Linux Kernel Mailing List, Mike Miller, iss_storagedev On 08/12/2007 03:25 AM, Rene Herman wrote: > On 08/12/2007 02:56 AM, Jesper Juhl wrote: > >> (whoops, forgot to add maintainer to Cc - now added) > > Ehm... too late... Useless followup though -- hp.com rejects me as it feels the SPF neutral results gmail sends due to me not using the gmail SMTP server and/or web interface is something it wants to listen to. I see you are @gmail as well, which depending on how you use it might mean your are also not able to reach @hp. If anyone can get HP to fix their mail setup, that'd be useful. Rene. > >> On 12/08/07, Jesper Juhl <jesper.juhl@gmail.com> wrote: >>> Hi, >>> >>> I've been building some randconfig kernels lately and I've noticed >>> this in a few builds : >>> >>> drivers/block/cciss.c:2614: warning: right shift count >= width of type >>> drivers/block/cciss.c:2615: warning: right shift count >= width of type >>> drivers/block/cciss.c:2616: warning: right shift count >= width of type >>> >>> The code in question is this : >>> >>> static void do_cciss_request(struct request_queue *q) >>> { >>> ... >>> sector_t start_blk; >>> ... >>> c->Request.CDB[2]= (start_blk >> 56) & >>> 0xff; //MSB >>> c->Request.CDB[3]= (start_blk >> 48) & 0xff; >>> c->Request.CDB[4]= (start_blk >> 40) & 0xff; >>> ... >>> } >>> >>> >>> The problem stems from these lines in include/linux/types.h : >>> ... >>> #ifdef CONFIG_LBD >>> typedef u64 sector_t; >>> #else >>> typedef unsigned long sector_t; >>> #endif >>> ... >>> >>> So on a 32bit arch without CONFIG_LBD, sector_t is going to be 32 >>> bits wide. >>> Thus it seems gcc is absolutely right in complaining about those >>> 56, 48 & 40 bit shifts, since they are indeed wider than the type >>> in the "!CONFIG_LBD on a 32bit arch" case. >>> >>> >>> I must admit that I have no idear what the proper way to deal with >>> that is, so I'll just report it so hopefully someone else can fix it. >>> >>> By the way; I'm building current Linus git tree, head at commit >>> ac07860264bd2b18834d3fa3be47032115524cea > > Well, given that it's explicitly treating start_blk as a 64-bit value, > the proper fix is probably to cast start_blk to an actual (guaranteed) > 64-bit value. Untested, uncompiled, and someone else may disagree: > > Rene. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] Re: cciss: warning: right shift count >= width of type 2007-08-12 0:28 cciss: warning: right shift count >= width of type Jesper Juhl 2007-08-12 0:56 ` Jesper Juhl @ 2007-08-12 1:21 ` Rene Herman 2007-08-12 6:58 ` Al Viro 1 sibling, 1 reply; 9+ messages in thread From: Rene Herman @ 2007-08-12 1:21 UTC (permalink / raw) To: Jesper Juhl; +Cc: linux-scsi, Linux Kernel Mailing List [-- Attachment #1: Type: text/plain, Size: 1661 bytes --] On 08/12/2007 02:28 AM, Jesper Juhl wrote: > I've been building some randconfig kernels lately and I've noticed > this in a few builds : > > drivers/block/cciss.c:2614: warning: right shift count >= width of type > drivers/block/cciss.c:2615: warning: right shift count >= width of type > drivers/block/cciss.c:2616: warning: right shift count >= width of type > > The code in question is this : > > static void do_cciss_request(struct request_queue *q) > { > ... > sector_t start_blk; > ... > c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB > c->Request.CDB[3]= (start_blk >> 48) & 0xff; > c->Request.CDB[4]= (start_blk >> 40) & 0xff; > ... > } > > > The problem stems from these lines in include/linux/types.h : > ... > #ifdef CONFIG_LBD > typedef u64 sector_t; > #else > typedef unsigned long sector_t; > #endif > ... > > So on a 32bit arch without CONFIG_LBD, sector_t is going to be 32 bits wide. > Thus it seems gcc is absolutely right in complaining about those > 56, 48 & 40 bit shifts, since they are indeed wider than the type > in the "!CONFIG_LBD on a 32bit arch" case. > > > I must admit that I have no idear what the proper way to deal with > that is, so I'll just report it so hopefully someone else can fix it. > > By the way; I'm building current Linus git tree, head at commit > ac07860264bd2b18834d3fa3be47032115524cea Well, given that it's explicitly treating start_blk as a 64-bit value, the proper fix is probably to cast start_blk to an actual (guaranteed) 64-bit value. Untested, uncompiled, and someone else may disagree: Rene. [-- Attachment #2: cciss.diff --] [-- Type: text/plain, Size: 1174 bytes --] diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c index 5acc6c4..fa2eec8 100644 --- a/drivers/block/cciss.c +++ b/drivers/block/cciss.c @@ -2609,13 +2609,13 @@ static void do_cciss_request(request_queue_t *q) } else { c->Request.CDBLen = 16; c->Request.CDB[1]= 0; - c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB - c->Request.CDB[3]= (start_blk >> 48) & 0xff; - c->Request.CDB[4]= (start_blk >> 40) & 0xff; - c->Request.CDB[5]= (start_blk >> 32) & 0xff; - c->Request.CDB[6]= (start_blk >> 24) & 0xff; - c->Request.CDB[7]= (start_blk >> 16) & 0xff; - c->Request.CDB[8]= (start_blk >> 8) & 0xff; + c->Request.CDB[2]= ((u64)start_blk >> 56) & 0xff; //MSB + c->Request.CDB[3]= ((u64)start_blk >> 48) & 0xff; + c->Request.CDB[4]= ((u64)start_blk >> 40) & 0xff; + c->Request.CDB[5]= ((u64)start_blk >> 32) & 0xff; + c->Request.CDB[6]= ((u64)start_blk >> 24) & 0xff; + c->Request.CDB[7]= ((u64)start_blk >> 16) & 0xff; + c->Request.CDB[8]= ((u64)start_blk >> 8) & 0xff; c->Request.CDB[9]= start_blk & 0xff; c->Request.CDB[10]= (creq->nr_sectors >> 24) & 0xff; c->Request.CDB[11]= (creq->nr_sectors >> 16) & 0xff; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Re: cciss: warning: right shift count >= width of type 2007-08-12 1:21 ` Rene Herman @ 2007-08-12 6:58 ` Al Viro 2007-08-12 19:55 ` Rene Herman 2007-08-12 20:32 ` James Bottomley 0 siblings, 2 replies; 9+ messages in thread From: Al Viro @ 2007-08-12 6:58 UTC (permalink / raw) To: Rene Herman; +Cc: Jesper Juhl, linux-scsi, Linux Kernel Mailing List On Sun, Aug 12, 2007 at 03:21:57AM +0200, Rene Herman wrote: > @@ -2609,13 +2609,13 @@ static void do_cciss_request(request_queue_t *q) > } else { > c->Request.CDBLen = 16; > c->Request.CDB[1]= 0; > - c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB > - c->Request.CDB[3]= (start_blk >> 48) & 0xff; > - c->Request.CDB[4]= (start_blk >> 40) & 0xff; > - c->Request.CDB[5]= (start_blk >> 32) & 0xff; > - c->Request.CDB[6]= (start_blk >> 24) & 0xff; > - c->Request.CDB[7]= (start_blk >> 16) & 0xff; > - c->Request.CDB[8]= (start_blk >> 8) & 0xff; > + c->Request.CDB[2]= ((u64)start_blk >> 56) & 0xff; //MSB > + c->Request.CDB[3]= ((u64)start_blk >> 48) & 0xff; > + c->Request.CDB[4]= ((u64)start_blk >> 40) & 0xff; > + c->Request.CDB[5]= ((u64)start_blk >> 32) & 0xff; > + c->Request.CDB[6]= ((u64)start_blk >> 24) & 0xff; > + c->Request.CDB[7]= ((u64)start_blk >> 16) & 0xff; > + c->Request.CDB[8]= ((u64)start_blk >> 8) & 0xff; put_unaligned(cpu_to_be64(start_blk), &c->Request.CDB[2]); which is what's happening here anyway. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Re: cciss: warning: right shift count >= width of type 2007-08-12 6:58 ` Al Viro @ 2007-08-12 19:55 ` Rene Herman 2007-08-12 20:32 ` James Bottomley 1 sibling, 0 replies; 9+ messages in thread From: Rene Herman @ 2007-08-12 19:55 UTC (permalink / raw) To: Al Viro; +Cc: Jesper Juhl, linux-scsi, Linux Kernel Mailing List [-- Attachment #1: Type: text/plain, Size: 1142 bytes --] On 08/12/2007 08:58 AM, Al Viro wrote: > On Sun, Aug 12, 2007 at 03:21:57AM +0200, Rene Herman wrote: >> + c->Request.CDB[2]= ((u64)start_blk >> 56) & 0xff; //MSB >> + c->Request.CDB[3]= ((u64)start_blk >> 48) & 0xff; >> + c->Request.CDB[4]= ((u64)start_blk >> 40) & 0xff; >> + c->Request.CDB[5]= ((u64)start_blk >> 32) & 0xff; >> + c->Request.CDB[6]= ((u64)start_blk >> 24) & 0xff; >> + c->Request.CDB[7]= ((u64)start_blk >> 16) & 0xff; >> + c->Request.CDB[8]= ((u64)start_blk >> 8) & 0xff; > > put_unaligned(cpu_to_be64(start_blk), &c->Request.CDB[2]); > > which is what's happening here anyway. Well, yes. There are a few more of these in the driver and this wants a maintainer (whom I can't reach @hp.com) comment. Is that 16-bit one for CCISS_READ_10 really right? Either: put_unaligned(cpu_to_be32(creq->nr_sectors), &c->Request.CDB[6]); or possibly: put_unaligned(cpu_to_be16(creq->nr_sectors), &c->Request.CDB[8]); would look less surprising than the current 16-bit in 24-bit thing and the "sect >> 24" comment there seems to imply the first? (the implcit downcasting in that case is fine, right?) Rene. [-- Attachment #2: cciss.diff --] [-- Type: text/plain, Size: 3028 bytes --] diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c index 5acc6c4..9fb6b3c 100644 --- a/drivers/block/cciss.c +++ b/drivers/block/cciss.c @@ -40,6 +40,7 @@ #include <linux/blktrace_api.h> #include <asm/uaccess.h> #include <asm/io.h> +#include <asm/unaligned.h> #include <linux/dma-mapping.h> #include <linux/blkdev.h> @@ -1711,10 +1712,7 @@ static int fill_cmd(CommandList_struct *c, __u8 cmd, int ctlr, void *buff, size_ c->Request.Type.Direction = XFER_READ; c->Request.Timeout = 0; c->Request.CDB[0] = cmd; - c->Request.CDB[6] = (size >> 24) & 0xFF; //MSB - c->Request.CDB[7] = (size >> 16) & 0xFF; - c->Request.CDB[8] = (size >> 8) & 0xFF; - c->Request.CDB[9] = size & 0xFF; + put_unaligned(cpu_to_be32(size), &c->Request.CDB[6]); break; case CCISS_READ_CAPACITY: @@ -1735,12 +1733,7 @@ static int fill_cmd(CommandList_struct *c, __u8 cmd, int ctlr, void *buff, size_ c->Request.Timeout = 0; c->Request.CDB[0] = cmd; c->Request.CDB[1] = 0x10; - c->Request.CDB[10] = (size >> 24) & 0xFF; - c->Request.CDB[11] = (size >> 16) & 0xFF; - c->Request.CDB[12] = (size >> 8) & 0xFF; - c->Request.CDB[13] = size & 0xFF; - c->Request.Timeout = 0; - c->Request.CDB[0] = cmd; + put_unaligned(cpu_to_be32(size), &c->Request.CDB[10]); break; case CCISS_CACHE_FLUSH: c->Request.CDBLen = 12; @@ -2598,29 +2591,15 @@ static void do_cciss_request(request_queue_t *q) if (likely(blk_fs_request(creq))) { if(h->cciss_read == CCISS_READ_10) { c->Request.CDB[1] = 0; - c->Request.CDB[2] = (start_blk >> 24) & 0xff; //MSB - c->Request.CDB[3] = (start_blk >> 16) & 0xff; - c->Request.CDB[4] = (start_blk >> 8) & 0xff; - c->Request.CDB[5] = start_blk & 0xff; - c->Request.CDB[6] = 0; // (sect >> 24) & 0xff; MSB - c->Request.CDB[7] = (creq->nr_sectors >> 8) & 0xff; - c->Request.CDB[8] = creq->nr_sectors & 0xff; + put_unaligned(cpu_to_be32(start_blk), &c->Request.CDB[2]); + c->Request.CDB[6] = 0; + put_unaligned(cpu_to_be16(creq->nr_sectors), &c->Request.CDB[7]); c->Request.CDB[9] = c->Request.CDB[11] = c->Request.CDB[12] = 0; } else { c->Request.CDBLen = 16; c->Request.CDB[1]= 0; - c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB - c->Request.CDB[3]= (start_blk >> 48) & 0xff; - c->Request.CDB[4]= (start_blk >> 40) & 0xff; - c->Request.CDB[5]= (start_blk >> 32) & 0xff; - c->Request.CDB[6]= (start_blk >> 24) & 0xff; - c->Request.CDB[7]= (start_blk >> 16) & 0xff; - c->Request.CDB[8]= (start_blk >> 8) & 0xff; - c->Request.CDB[9]= start_blk & 0xff; - c->Request.CDB[10]= (creq->nr_sectors >> 24) & 0xff; - c->Request.CDB[11]= (creq->nr_sectors >> 16) & 0xff; - c->Request.CDB[12]= (creq->nr_sectors >> 8) & 0xff; - c->Request.CDB[13]= creq->nr_sectors & 0xff; + put_unaligned(cpu_to_be64(start_blk), &c->Request.CDB[2]); + put_unaligned(cpu_to_be32(creq->nr_sectors), &c->Request.CDB[10]); c->Request.CDB[14] = c->Request.CDB[15] = 0; } } else if (blk_pc_request(creq)) { ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Re: cciss: warning: right shift count >= width of type 2007-08-12 6:58 ` Al Viro 2007-08-12 19:55 ` Rene Herman @ 2007-08-12 20:32 ` James Bottomley 2007-08-12 23:08 ` Rene Herman 1 sibling, 1 reply; 9+ messages in thread From: James Bottomley @ 2007-08-12 20:32 UTC (permalink / raw) To: Al Viro Cc: Rene Herman, Jesper Juhl, linux-scsi, Linux Kernel Mailing List, Mike.miller On Sun, 2007-08-12 at 07:58 +0100, Al Viro wrote: > On Sun, Aug 12, 2007 at 03:21:57AM +0200, Rene Herman wrote: > > @@ -2609,13 +2609,13 @@ static void do_cciss_request(request_queue_t *q) > > } else { > > c->Request.CDBLen = 16; > > c->Request.CDB[1]= 0; > > - c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB > > - c->Request.CDB[3]= (start_blk >> 48) & 0xff; > > - c->Request.CDB[4]= (start_blk >> 40) & 0xff; > > - c->Request.CDB[5]= (start_blk >> 32) & 0xff; > > - c->Request.CDB[6]= (start_blk >> 24) & 0xff; > > - c->Request.CDB[7]= (start_blk >> 16) & 0xff; > > - c->Request.CDB[8]= (start_blk >> 8) & 0xff; > > + c->Request.CDB[2]= ((u64)start_blk >> 56) & 0xff; //MSB > > + c->Request.CDB[3]= ((u64)start_blk >> 48) & 0xff; > > + c->Request.CDB[4]= ((u64)start_blk >> 40) & 0xff; > > + c->Request.CDB[5]= ((u64)start_blk >> 32) & 0xff; > > + c->Request.CDB[6]= ((u64)start_blk >> 24) & 0xff; > > + c->Request.CDB[7]= ((u64)start_blk >> 16) & 0xff; > > + c->Request.CDB[8]= ((u64)start_blk >> 8) & 0xff; > > put_unaligned(cpu_to_be64(start_blk), &c->Request.CDB[2]); > > which is what's happening here anyway. Well ... this was debated a while ago ad nauseam: http://marc.info/?t=117699555300010 The main objection to what you propose is that it forces the u64 coercion even in the 32 bit start_blk case. The preferred solution as a result of that debate was simply to us a macro Andrew introduced: upper_32_bits() Which will silently replace zero in the non LBD case. I actually thought this had already been done. James James ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Re: cciss: warning: right shift count >= width of type 2007-08-12 20:32 ` James Bottomley @ 2007-08-12 23:08 ` Rene Herman 0 siblings, 0 replies; 9+ messages in thread From: Rene Herman @ 2007-08-12 23:08 UTC (permalink / raw) To: James Bottomley Cc: Al Viro, Jesper Juhl, linux-scsi, Linux Kernel Mailing List, Mike.miller On 08/12/2007 10:32 PM, James Bottomley wrote: > On Sun, 2007-08-12 at 07:58 +0100, Al Viro wrote: >> On Sun, Aug 12, 2007 at 03:21:57AM +0200, Rene Herman wrote: >>> @@ -2609,13 +2609,13 @@ static void do_cciss_request(request_queue_t *q) >>> } else { >>> c->Request.CDBLen = 16; >>> c->Request.CDB[1]= 0; >>> - c->Request.CDB[2]= (start_blk >> 56) & 0xff; //MSB >>> - c->Request.CDB[3]= (start_blk >> 48) & 0xff; >>> - c->Request.CDB[4]= (start_blk >> 40) & 0xff; >>> - c->Request.CDB[5]= (start_blk >> 32) & 0xff; >>> - c->Request.CDB[6]= (start_blk >> 24) & 0xff; >>> - c->Request.CDB[7]= (start_blk >> 16) & 0xff; >>> - c->Request.CDB[8]= (start_blk >> 8) & 0xff; >>> + c->Request.CDB[2]= ((u64)start_blk >> 56) & 0xff; //MSB >>> + c->Request.CDB[3]= ((u64)start_blk >> 48) & 0xff; >>> + c->Request.CDB[4]= ((u64)start_blk >> 40) & 0xff; >>> + c->Request.CDB[5]= ((u64)start_blk >> 32) & 0xff; >>> + c->Request.CDB[6]= ((u64)start_blk >> 24) & 0xff; >>> + c->Request.CDB[7]= ((u64)start_blk >> 16) & 0xff; >>> + c->Request.CDB[8]= ((u64)start_blk >> 8) & 0xff; >> put_unaligned(cpu_to_be64(start_blk), &c->Request.CDB[2]); >> >> which is what's happening here anyway. > > Well ... this was debated a while ago ad nauseam: > > http://marc.info/?t=117699555300010 > > The main objection to what you propose is that it forces the u64 > coercion even in the 32 bit start_blk case. The preferred solution as a > result of that debate was simply to us a macro Andrew introduced: > > upper_32_bits() > > Which will silently replace zero in the non LBD case. I actually > thought this had already been done. I see. Will assume it's somewhere in the pipeline. Rene. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-08-12 23:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-08-12 0:28 cciss: warning: right shift count >= width of type Jesper Juhl 2007-08-12 0:56 ` Jesper Juhl 2007-08-12 1:25 ` [PATCH] " Rene Herman 2007-08-12 1:54 ` Rene Herman 2007-08-12 1:21 ` Rene Herman 2007-08-12 6:58 ` Al Viro 2007-08-12 19:55 ` Rene Herman 2007-08-12 20:32 ` James Bottomley 2007-08-12 23:08 ` Rene Herman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).