Re: [Patch 1/2] cciss: fix for 2TB support
> On Fri, 23 Feb 2007 14:52:29 -0600 "Mike Miller (OS Dev)" <[EMAIL PROTECTED]> > wrote: > On Thu, Feb 22, 2007 at 04:06:41PM -0600, James Bottomley wrote: > > On Thu, 2007-02-22 at 16:02 -0600, Mike Miller (OS Dev) wrote: > > > Will this patch for my patch work for now? > > > > Yes, I think that should be fine ... it's only a theoretical worry; at > > the moment sector_t is unsigned ... but just in case. > > > > James > > > > > Andrew, > Are you waiting for a new patch from me? Or is my patch's patch sufficient? > It looked OK. But I'm travelling at present, will get back into things late next week. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, Feb 22, 2007 at 04:06:41PM -0600, James Bottomley wrote: > On Thu, 2007-02-22 at 16:02 -0600, Mike Miller (OS Dev) wrote: > > Will this patch for my patch work for now? > > Yes, I think that should be fine ... it's only a theoretical worry; at > the moment sector_t is unsigned ... but just in case. > > James > > Andrew, Are you waiting for a new patch from me? Or is my patch's patch sufficient? -- mikem - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, Feb 22, 2007 at 04:06:41PM -0600, James Bottomley wrote: On Thu, 2007-02-22 at 16:02 -0600, Mike Miller (OS Dev) wrote: Will this patch for my patch work for now? Yes, I think that should be fine ... it's only a theoretical worry; at the moment sector_t is unsigned ... but just in case. James Andrew, Are you waiting for a new patch from me? Or is my patch's patch sufficient? -- mikem - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Fri, 23 Feb 2007 14:52:29 -0600 Mike Miller (OS Dev) [EMAIL PROTECTED] wrote: On Thu, Feb 22, 2007 at 04:06:41PM -0600, James Bottomley wrote: On Thu, 2007-02-22 at 16:02 -0600, Mike Miller (OS Dev) wrote: Will this patch for my patch work for now? Yes, I think that should be fine ... it's only a theoretical worry; at the moment sector_t is unsigned ... but just in case. James Andrew, Are you waiting for a new patch from me? Or is my patch's patch sufficient? It looked OK. But I'm travelling at present, will get back into things late next week. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, 2007-02-22 at 16:02 -0600, Mike Miller (OS Dev) wrote: > Will this patch for my patch work for now? Yes, I think that should be fine ... it's only a theoretical worry; at the moment sector_t is unsigned ... but just in case. James - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, Feb 22, 2007 at 03:41:24PM -0600, James Bottomley wrote:
> On Thu, 2007-02-22 at 13:24 -0800, Andrew Morton wrote:
> > > On Thu, 22 Feb 2007 10:51:23 -0600 "Mike Miller (OS Dev)" <[EMAIL
> > > PROTECTED]> wrote:
> > > On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
> > > > On Wed, 21 Feb 2007 15:10:39 -0600 "Mike Miller (OS Dev)" <[EMAIL
> > > > PROTECTED]> wrote:
> > > >
> > > > > Patch 1/2
> > > > > + if (total_size == 0x) {
> > > >
> > > > I seem to remember having already questioned this. total_size is
> > > > sector_t, which
> > > > can be either 32-bit or 64-bit. Are you sure that comparison works as
> > > > intended in both cases?
> > > >
> > > >
> > > > > + if(total_size == 0x) {
> > > > > cciss_read_capacity_16(cntl_num, i, 0,
> > > > > _size, _size);
> > > > > hba[cntl_num]->cciss_read = CCISS_READ_16;
> > > >
> > > > Here too.
> > > It has worked in all of the configs I've tested. Should I change it from
> > > sector_t to a
> > > __64? I have not tested all possible configs.
> > >
> >
> > I'd suggest using -1: that just works.
>
> Actually, no, that won't work.
>
> This is a SCSI heuristic for determining when to use the 16 byte version
> of the read capacity command. The 10 byte command can only return 32
> bits of information (this is in sectors, so it returns up to 2TB of
> bytes).
>
> The heuristic requirement is that if the size is exactly 0x then
> you should try the 16 byte command (which can return 64 bits of
> information). If that fails then you assume the 0xfff is a real
> size otherwize, you assume it was truncated and take the real result
> from the 16 byte command.
>
> You can see a far more elaborate version of this in operation in
> sd.c:sd_read_capacity().
>
> The only thing I'd suggest is to use 0xULL as the constant to
> prevent sign extension issues.
>
> James
>
>
Will this patch for my patch work for now?
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 1abf1f5..a1f1d9f 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1303,7 +1303,7 @@ static void cciss_update_drive_info(int
/* if read_capacity returns all F's this volume is >2TB in size */
/* so we switch to 16-byte CDB's for all read/write ops */
- if (total_size == 0x) {
+ if (total_size == 0xULL) {
cciss_read_capacity_16(ctlr, drv_index, 1,
_size, _size);
h->cciss_read = CCISS_READ_16;
@@ -3129,7 +3129,7 @@ #endif/* CCISS_DEBUG */
/* If read_capacity returns all F's the logical is >2TB */
/* so we switch to 16-byte CDBs for all read/write ops */
- if(total_size == 0x) {
+ if(total_size == 0xULL) {
cciss_read_capacity_16(cntl_num, i, 0,
_size, _size);
hba[cntl_num]->cciss_read = CCISS_READ_16;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, 2007-02-22 at 13:24 -0800, Andrew Morton wrote:
> > On Thu, 22 Feb 2007 10:51:23 -0600 "Mike Miller (OS Dev)" <[EMAIL
> > PROTECTED]> wrote:
> > On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
> > > On Wed, 21 Feb 2007 15:10:39 -0600 "Mike Miller (OS Dev)" <[EMAIL
> > > PROTECTED]> wrote:
> > >
> > > > Patch 1/2
> > > > + if (total_size == 0x) {
> > >
> > > I seem to remember having already questioned this. total_size is
> > > sector_t, which
> > > can be either 32-bit or 64-bit. Are you sure that comparison works as
> > > intended in both cases?
> > >
> > >
> > > > + if(total_size == 0x) {
> > > > cciss_read_capacity_16(cntl_num, i, 0,
> > > > _size, _size);
> > > > hba[cntl_num]->cciss_read = CCISS_READ_16;
> > >
> > > Here too.
> > It has worked in all of the configs I've tested. Should I change it from
> > sector_t to a
> > __64? I have not tested all possible configs.
> >
>
> I'd suggest using -1: that just works.
Actually, no, that won't work.
This is a SCSI heuristic for determining when to use the 16 byte version
of the read capacity command. The 10 byte command can only return 32
bits of information (this is in sectors, so it returns up to 2TB of
bytes).
The heuristic requirement is that if the size is exactly 0x then
you should try the 16 byte command (which can return 64 bits of
information). If that fails then you assume the 0xfff is a real
size otherwize, you assume it was truncated and take the real result
from the 16 byte command.
You can see a far more elaborate version of this in operation in
sd.c:sd_read_capacity().
The only thing I'd suggest is to use 0xULL as the constant to
prevent sign extension issues.
James
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [Patch 1/2] cciss: fix for 2TB support
> -Original Message-
> From: Mike Miller (OS Dev) [mailto:[EMAIL PROTECTED]
>
> Andrew,
> Using this test program and changing the type of x to int,
> long, long long signed and unsigned the comparison always
> worked on x86, x86_64, and ia64. It looks to me like the
> comparsion will always do what we expect. Unless you see some
> other problem.
>
>
> #include
>
> int main(int argc, char *argv[])
> {
> unsigned long long x;
>
> x = 0x;
>
> printf(sizeof(x) == 8 ?
> "x = %lld, sizeof(x) = %d\n" :
> "x = %ld, sizeof(x) = %d\n", x, sizeof(x));
> if (x == 0x)
> printf("equal\n");
> else
> printf("not equal\n");
>
> }
>
> -- mikem
>
BTW: also changed x to be 8 f's, 16 f's, and 8 and 8 as shown.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
> On Thu, 22 Feb 2007 10:51:23 -0600 "Mike Miller (OS Dev)" <[EMAIL PROTECTED]>
> wrote:
> On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
> > On Wed, 21 Feb 2007 15:10:39 -0600 "Mike Miller (OS Dev)" <[EMAIL
> > PROTECTED]> wrote:
> >
> > > Patch 1/2
> > > + if (total_size == 0x) {
> >
> > I seem to remember having already questioned this. total_size is sector_t,
> > which
> > can be either 32-bit or 64-bit. Are you sure that comparison works as
> > intended in both cases?
> >
> >
> > > + if(total_size == 0x) {
> > > cciss_read_capacity_16(cntl_num, i, 0,
> > > _size, _size);
> > > hba[cntl_num]->cciss_read = CCISS_READ_16;
> >
> > Here too.
> It has worked in all of the configs I've tested. Should I change it from
> sector_t to a
> __64? I have not tested all possible configs.
>
I'd suggest using -1: that just works.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
> >
> > + if (total_size == 0x) {
>
> I seem to remember having already questioned this. total_size is sector_t,
> which
> can be either 32-bit or 64-bit. Are you sure that comparison works as
> intended in both cases?
>
>
> > + if(total_size == 0x) {
> > cciss_read_capacity_16(cntl_num, i, 0,
> > _size, _size);
> > hba[cntl_num]->cciss_read = CCISS_READ_16;
>
> Here too.
Andrew,
Using this test program and changing the type of x to int, long, long long
signed and
unsigned the comparison always worked on x86, x86_64, and ia64. It looks to me
like
the comparsion will always do what we expect. Unless you see some other problem.
#include
int main(int argc, char *argv[])
{
unsigned long long x;
x = 0x;
printf(sizeof(x) == 8 ?
"x = %lld, sizeof(x) = %d\n" :
"x = %ld, sizeof(x) = %d\n", x, sizeof(x));
if (x == 0x)
printf("equal\n");
else
printf("not equal\n");
}
-- mikem
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
> On Wed, 21 Feb 2007 15:10:39 -0600 "Mike Miller (OS Dev)" <[EMAIL PROTECTED]>
> wrote:
>
> > Patch 1/2
> >
> > This patch changes the way we determine if a logical volume is larger than
> > 2TB. The
> > original test looked for a total_size of 0. Originally we added 1 to the
> > total_size.
> > That would make our read_capacity return size 0 for >2TB lv's. We assumed
> > that we
> > could not have a lv size of 0 so it seemed OK until we were in a clustered
> > system. The
> > backup node would see a size of 0 due to the reservation on the drive. That
> > caused
> > the driver to switch to 16-byte CDB's which are not supported on older
> > controllers.
> > After that everything was broken.
> > It may seem petty but I don't see the value in trying to determine if the
> > LBA is
> > beyond the 2TB boundary. That's why when we switch we use 16-byte CDB's for
> > all
> > read/write operations.
> > Please consider this for inclusion.
> >
> > ...
> >
> > + if (total_size == 0x) {
>
> I seem to remember having already questioned this. total_size is sector_t,
> which
> can be either 32-bit or 64-bit. Are you sure that comparison works as
> intended in both cases?
>
>
> > + if(total_size == 0x) {
> > cciss_read_capacity_16(cntl_num, i, 0,
> > _size, _size);
> > hba[cntl_num]->cciss_read = CCISS_READ_16;
>
> Here too.
It has worked in all of the configs I've tested. Should I change it from
sector_t to a
__64? I have not tested all possible configs.
-- mikem
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
On Wed, 21 Feb 2007 15:10:39 -0600 Mike Miller (OS Dev) [EMAIL PROTECTED]
wrote:
Patch 1/2
This patch changes the way we determine if a logical volume is larger than
2TB. The
original test looked for a total_size of 0. Originally we added 1 to the
total_size.
That would make our read_capacity return size 0 for 2TB lv's. We assumed
that we
could not have a lv size of 0 so it seemed OK until we were in a clustered
system. The
backup node would see a size of 0 due to the reservation on the drive. That
caused
the driver to switch to 16-byte CDB's which are not supported on older
controllers.
After that everything was broken.
It may seem petty but I don't see the value in trying to determine if the
LBA is
beyond the 2TB boundary. That's why when we switch we use 16-byte CDB's for
all
read/write operations.
Please consider this for inclusion.
...
+ if (total_size == 0x) {
I seem to remember having already questioned this. total_size is sector_t,
which
can be either 32-bit or 64-bit. Are you sure that comparison works as
intended in both cases?
+ if(total_size == 0x) {
cciss_read_capacity_16(cntl_num, i, 0,
total_size, block_size);
hba[cntl_num]-cciss_read = CCISS_READ_16;
Here too.
It has worked in all of the configs I've tested. Should I change it from
sector_t to a
__64? I have not tested all possible configs.
-- mikem
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
+ if (total_size == 0x) {
I seem to remember having already questioned this. total_size is sector_t,
which
can be either 32-bit or 64-bit. Are you sure that comparison works as
intended in both cases?
+ if(total_size == 0x) {
cciss_read_capacity_16(cntl_num, i, 0,
total_size, block_size);
hba[cntl_num]-cciss_read = CCISS_READ_16;
Here too.
Andrew,
Using this test program and changing the type of x to int, long, long long
signed and
unsigned the comparison always worked on x86, x86_64, and ia64. It looks to me
like
the comparsion will always do what we expect. Unless you see some other problem.
#include stdio.h
int main(int argc, char *argv[])
{
unsigned long long x;
x = 0x;
printf(sizeof(x) == 8 ?
x = %lld, sizeof(x) = %d\n :
x = %ld, sizeof(x) = %d\n, x, sizeof(x));
if (x == 0x)
printf(equal\n);
else
printf(not equal\n);
}
-- mikem
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, 22 Feb 2007 10:51:23 -0600 Mike Miller (OS Dev) [EMAIL PROTECTED]
wrote:
On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
On Wed, 21 Feb 2007 15:10:39 -0600 Mike Miller (OS Dev) [EMAIL
PROTECTED] wrote:
Patch 1/2
+ if (total_size == 0x) {
I seem to remember having already questioned this. total_size is sector_t,
which
can be either 32-bit or 64-bit. Are you sure that comparison works as
intended in both cases?
+ if(total_size == 0x) {
cciss_read_capacity_16(cntl_num, i, 0,
total_size, block_size);
hba[cntl_num]-cciss_read = CCISS_READ_16;
Here too.
It has worked in all of the configs I've tested. Should I change it from
sector_t to a
__64? I have not tested all possible configs.
I'd suggest using -1: that just works.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [Patch 1/2] cciss: fix for 2TB support
-Original Message-
From: Mike Miller (OS Dev) [mailto:[EMAIL PROTECTED]
Andrew,
Using this test program and changing the type of x to int,
long, long long signed and unsigned the comparison always
worked on x86, x86_64, and ia64. It looks to me like the
comparsion will always do what we expect. Unless you see some
other problem.
#include stdio.h
int main(int argc, char *argv[])
{
unsigned long long x;
x = 0x;
printf(sizeof(x) == 8 ?
x = %lld, sizeof(x) = %d\n :
x = %ld, sizeof(x) = %d\n, x, sizeof(x));
if (x == 0x)
printf(equal\n);
else
printf(not equal\n);
}
-- mikem
BTW: also changed x to be 8 f's, 16 f's, and 8 and 8 as shown.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, 2007-02-22 at 13:24 -0800, Andrew Morton wrote:
On Thu, 22 Feb 2007 10:51:23 -0600 Mike Miller (OS Dev) [EMAIL
PROTECTED] wrote:
On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
On Wed, 21 Feb 2007 15:10:39 -0600 Mike Miller (OS Dev) [EMAIL
PROTECTED] wrote:
Patch 1/2
+ if (total_size == 0x) {
I seem to remember having already questioned this. total_size is
sector_t, which
can be either 32-bit or 64-bit. Are you sure that comparison works as
intended in both cases?
+ if(total_size == 0x) {
cciss_read_capacity_16(cntl_num, i, 0,
total_size, block_size);
hba[cntl_num]-cciss_read = CCISS_READ_16;
Here too.
It has worked in all of the configs I've tested. Should I change it from
sector_t to a
__64? I have not tested all possible configs.
I'd suggest using -1: that just works.
Actually, no, that won't work.
This is a SCSI heuristic for determining when to use the 16 byte version
of the read capacity command. The 10 byte command can only return 32
bits of information (this is in sectors, so it returns up to 2TB of
bytes).
The heuristic requirement is that if the size is exactly 0x then
you should try the 16 byte command (which can return 64 bits of
information). If that fails then you assume the 0xfff is a real
size otherwize, you assume it was truncated and take the real result
from the 16 byte command.
You can see a far more elaborate version of this in operation in
sd.c:sd_read_capacity().
The only thing I'd suggest is to use 0xULL as the constant to
prevent sign extension issues.
James
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, Feb 22, 2007 at 03:41:24PM -0600, James Bottomley wrote:
On Thu, 2007-02-22 at 13:24 -0800, Andrew Morton wrote:
On Thu, 22 Feb 2007 10:51:23 -0600 Mike Miller (OS Dev) [EMAIL
PROTECTED] wrote:
On Wed, Feb 21, 2007 at 07:14:27PM -0800, Andrew Morton wrote:
On Wed, 21 Feb 2007 15:10:39 -0600 Mike Miller (OS Dev) [EMAIL
PROTECTED] wrote:
Patch 1/2
+ if (total_size == 0x) {
I seem to remember having already questioned this. total_size is
sector_t, which
can be either 32-bit or 64-bit. Are you sure that comparison works as
intended in both cases?
+ if(total_size == 0x) {
cciss_read_capacity_16(cntl_num, i, 0,
total_size, block_size);
hba[cntl_num]-cciss_read = CCISS_READ_16;
Here too.
It has worked in all of the configs I've tested. Should I change it from
sector_t to a
__64? I have not tested all possible configs.
I'd suggest using -1: that just works.
Actually, no, that won't work.
This is a SCSI heuristic for determining when to use the 16 byte version
of the read capacity command. The 10 byte command can only return 32
bits of information (this is in sectors, so it returns up to 2TB of
bytes).
The heuristic requirement is that if the size is exactly 0x then
you should try the 16 byte command (which can return 64 bits of
information). If that fails then you assume the 0xfff is a real
size otherwize, you assume it was truncated and take the real result
from the 16 byte command.
You can see a far more elaborate version of this in operation in
sd.c:sd_read_capacity().
The only thing I'd suggest is to use 0xULL as the constant to
prevent sign extension issues.
James
Will this patch for my patch work for now?
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 1abf1f5..a1f1d9f 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1303,7 +1303,7 @@ static void cciss_update_drive_info(int
/* if read_capacity returns all F's this volume is 2TB in size */
/* so we switch to 16-byte CDB's for all read/write ops */
- if (total_size == 0x) {
+ if (total_size == 0xULL) {
cciss_read_capacity_16(ctlr, drv_index, 1,
total_size, block_size);
h-cciss_read = CCISS_READ_16;
@@ -3129,7 +3129,7 @@ #endif/* CCISS_DEBUG */
/* If read_capacity returns all F's the logical is 2TB */
/* so we switch to 16-byte CDBs for all read/write ops */
- if(total_size == 0x) {
+ if(total_size == 0xULL) {
cciss_read_capacity_16(cntl_num, i, 0,
total_size, block_size);
hba[cntl_num]-cciss_read = CCISS_READ_16;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Thu, 2007-02-22 at 16:02 -0600, Mike Miller (OS Dev) wrote: Will this patch for my patch work for now? Yes, I think that should be fine ... it's only a theoretical worry; at the moment sector_t is unsigned ... but just in case. James - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Wed, 21 Feb 2007 15:10:39 -0600 "Mike Miller (OS Dev)" <[EMAIL PROTECTED]>
wrote:
> Patch 1/2
>
> This patch changes the way we determine if a logical volume is larger than
> 2TB. The
> original test looked for a total_size of 0. Originally we added 1 to the
> total_size.
> That would make our read_capacity return size 0 for >2TB lv's. We assumed
> that we
> could not have a lv size of 0 so it seemed OK until we were in a clustered
> system. The
> backup node would see a size of 0 due to the reservation on the drive. That
> caused
> the driver to switch to 16-byte CDB's which are not supported on older
> controllers.
> After that everything was broken.
> It may seem petty but I don't see the value in trying to determine if the LBA
> is
> beyond the 2TB boundary. That's why when we switch we use 16-byte CDB's for
> all
> read/write operations.
> Please consider this for inclusion.
>
> ...
>
> + if (total_size == 0x) {
I seem to remember having already questioned this. total_size is sector_t,
which
can be either 32-bit or 64-bit. Are you sure that comparison works as
intended in both cases?
> + if(total_size == 0x) {
> cciss_read_capacity_16(cntl_num, i, 0,
> _size, _size);
> hba[cntl_num]->cciss_read = CCISS_READ_16;
Here too.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 1/2] cciss: fix for 2TB support
On Wed, 21 Feb 2007 15:10:39 -0600 Mike Miller (OS Dev) [EMAIL PROTECTED]
wrote:
Patch 1/2
This patch changes the way we determine if a logical volume is larger than
2TB. The
original test looked for a total_size of 0. Originally we added 1 to the
total_size.
That would make our read_capacity return size 0 for 2TB lv's. We assumed
that we
could not have a lv size of 0 so it seemed OK until we were in a clustered
system. The
backup node would see a size of 0 due to the reservation on the drive. That
caused
the driver to switch to 16-byte CDB's which are not supported on older
controllers.
After that everything was broken.
It may seem petty but I don't see the value in trying to determine if the LBA
is
beyond the 2TB boundary. That's why when we switch we use 16-byte CDB's for
all
read/write operations.
Please consider this for inclusion.
...
+ if (total_size == 0x) {
I seem to remember having already questioned this. total_size is sector_t,
which
can be either 32-bit or 64-bit. Are you sure that comparison works as
intended in both cases?
+ if(total_size == 0x) {
cciss_read_capacity_16(cntl_num, i, 0,
total_size, block_size);
hba[cntl_num]-cciss_read = CCISS_READ_16;
Here too.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
