[PATCH] utilities: add helper functions for safe 64-bit integer operations as 32-bit halves
From: John Anthony Kazos Jr. <[EMAIL PROTECTED]> Add helper functions "upper_32_bits" and "lower_32_bits" to to allow 64-bit integers to be separated into their 32-bit upper and lower halves without promoting integers, without stretching sign bits, and without generating compiler warnings when used with any integer not greater than 64 bits wide. High-order bits are assumed to be zero for integers with fewer than 64 of them. Signed-off-by: John Anthony Kazos Jr. <[EMAIL PROTECTED]> --- Using these functions with signed quantities is an error, especially if you read a 32-bit quantity from disk that happens to have the high bit set into an int on a 32-bit machine, then use it with a function taking a u64 which screws your data. When switching to using these functions, it's a good opportunity to check for these signedness errors. (Haven't we learned anything over the past decades of computing about assuming that one little bit doesn't matter?) Not sure exactly whom the maintainer is for this, so I added [EMAIL PROTECTED] It's certainly not limited to one subsystem anymore, and converting the whole kernel to this could be a good step for readability and correctness across architectures of any word size. --- linux-2.6.21-rc7-git4.orig/include/linux/kernel.h 2007-04-20 20:22:13.0 -0400 +++ linux-2.6.21-rc7-git4.mod/include/linux/kernel.h2007-04-20 20:37:41.0 -0400 @@ -40,6 +40,23 @@ extern const char linux_proc_banner[]; #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d)) #define roundup(x, y) x) + ((y) - 1)) / (y)) * (y)) +/** + * lower_32_bits, upper_32_bits - separate the halves of a 64-bit integer + * @n: the integer to separate + * + * Separate a 64-bit integer into its upper and lower 32-bit halves. + * Designed to avoid integer promotions and compiler warnings when used + * with smaller integers, in which case the missing bits are assumed to + * be zero. Designed to treat integers as unsigned whether or not they + * really are. (If you are using these with signed integers, your code + * is almost certainly wrong. The cast is good for people too lazy to + * type "unsigned" in their code, since breaking things is bad.) + * + * These assume the integer used is NOT greater than 64 bits wide. + */ +#define upper_32_bits(n) (sizeof(n) == 8 ? (u64)(n) >> 32 : 0) +#define lower_32_bits(n) (sizeof(n) == 8 ? (u32)(n) : (n)) + #defineKERN_EMERG "<0>" /* system is unusable */ #defineKERN_ALERT "<1>" /* action must be taken immediately */ #defineKERN_CRIT "<2>" /* critical conditions */ - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cciss: Fix warnings during compilation under 32bitenvironment
On Fri, 20 Apr 2007, Andrew Morton wrote: > On Fri, 20 Apr 2007 16:20:59 -0400 > James Bottomley <[EMAIL PROTECTED]> wrote: > > > On Fri, 2007-04-20 at 12:30 -0700, Andrew Morton wrote: > > > On Fri, 20 Apr 2007 14:50:06 -0400 > > > James Bottomley <[EMAIL PROTECTED]> wrote: > > > > > > > > CONFIG_LBD=y gives us an additional 3kb of instructions on i386 > > > > > allnoconfig. Other architectures might do less well. It's not a huge > > > > > difference, but that's the way in which creeping bloatiness happens. > > > > > > > > OK, sure, but if we really care about this saving, then unconditionally > > > > casting to u64 is therefore wrong as well ... this is starting to open > > > > quite a large can of worms ... > > > > > > > > For the record, if we have to do this, I fancy sector_upper_32() ... we > > > > should already have some similar accessor for dma_addr_t as well. > > > > > > hm. How about this? > > > > > > --- a/include/linux/kernel.h~upper-32-bits > > > +++ a/include/linux/kernel.h > > > @@ -40,6 +40,17 @@ extern const char linux_proc_banner[]; > > > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d)) > > > #define roundup(x, y) x) + ((y) - 1)) / (y)) * (y)) > > > > > > +/** > > > + * upper_32_bits - return bits 32-63 of a number > > > + * @n: the number we're accessing > > > + * > > > + * A basic shift-right of a 64- or 32-bit quantity. Use this to suppress > > > + * the "right shift count >= width of type" warning when that quantity is > > > + * 32-bits. > > > + */ > > > +#define upper_32_bits(n) (((u64)(n)) >> 32) > > > > Won't this have the unwanted side effect of promoting everything in a > > calculation to long long on 32 bit platforms, even if n was only 32 > > bits? > > bummer. > > > > + > > > + > > > #define KERN_EMERG "<0>" /* system is unusable > > > */ > > > #define KERN_ALERT "<1>" /* action must be taken immediately > > > */ > > > #define KERN_CRIT "<2>" /* critical conditions > > > */ > > > _ > > > > > > It seems to generate the desired code. I avoided Alan's ((n >> 31) >> 1) > > > trick because it'll generate peculiar results with signed 64-bit > > > quantities. > > > > I've seen the trick done similarly with ((n >> 16) >> 16) which > > shouldn't have the issue. > > That works if we know the caller is treating the return value as 32 bits, > but we don't know that. > > If we have > > #define upper_32_bits(x) ((x >> 16) >> 16) > > then > > upper_32_bits(0x) > > will return 0x if it's treated as 32-bits, but it'll return > 0x if the caller is using 64-bits. > > I spose > > #define upper_32_bits(x) ((u32)((x >> 16) >> 16)) > > will do the trick. What about this? #define upper_32_bits(x) (sizeof(x) == 8 ? (u64)(x) >> 32 : 0) The u64 cast prevents the sign bit from being carried over and therefore eliminates the need for a subsequent cast to u32 since the upper 32 of the result will be 0. Shouldn't be any case where an integer gets promoted if 64 bits is the largest possible promotion. Assuming, of course, I'm not an idiot. Which I somewhat frequently am. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] megaraid: update version reported by MEGAIOC_QDRVRVER
ACK. Thank you David. Regards, Sumant -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of David Milburn Sent: Thursday, April 19, 2007 10:10 AM To: [EMAIL PROTECTED] Cc: linux-scsi@vger.kernel.org; [EMAIL PROTECTED] Subject: [PATCH] megaraid: update version reported by MEGAIOC_QDRVRVER Update the driver version reported by MEGAIOC_QDRVRVER to match LSI_COMMON_MOD_VERSION. Signed-off-by: David Milburn <[EMAIL PROTECTED]> --- drivers/scsi/megaraid/megaraid_mm.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c index f33a678..e075a52 100644 --- a/drivers/scsi/megaraid/megaraid_mm.c +++ b/drivers/scsi/megaraid/megaraid_mm.c @@ -60,7 +60,7 @@ EXPORT_SYMBOL(mraid_mm_unregister_adp); EXPORT_SYMBOL(mraid_mm_adapter_app_handle); static int majorno; -static uint32_t drvr_ver = 0x02200206; +static uint32_t drvr_ver = 0x02200207; static int adapters_count_g; static struct list_head adapters_list_g; - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cciss: Fix warnings during compilation under 32bitenvironment
On Fri, 20 Apr 2007 16:20:59 -0400 James Bottomley <[EMAIL PROTECTED]> wrote: > On Fri, 2007-04-20 at 12:30 -0700, Andrew Morton wrote: > > On Fri, 20 Apr 2007 14:50:06 -0400 > > James Bottomley <[EMAIL PROTECTED]> wrote: > > > > > > CONFIG_LBD=y gives us an additional 3kb of instructions on i386 > > > > allnoconfig. Other architectures might do less well. It's not a huge > > > > difference, but that's the way in which creeping bloatiness happens. > > > > > > OK, sure, but if we really care about this saving, then unconditionally > > > casting to u64 is therefore wrong as well ... this is starting to open > > > quite a large can of worms ... > > > > > > For the record, if we have to do this, I fancy sector_upper_32() ... we > > > should already have some similar accessor for dma_addr_t as well. > > > > hm. How about this? > > > > --- a/include/linux/kernel.h~upper-32-bits > > +++ a/include/linux/kernel.h > > @@ -40,6 +40,17 @@ extern const char linux_proc_banner[]; > > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d)) > > #define roundup(x, y) x) + ((y) - 1)) / (y)) * (y)) > > > > +/** > > + * upper_32_bits - return bits 32-63 of a number > > + * @n: the number we're accessing > > + * > > + * A basic shift-right of a 64- or 32-bit quantity. Use this to suppress > > + * the "right shift count >= width of type" warning when that quantity is > > + * 32-bits. > > + */ > > +#define upper_32_bits(n) (((u64)(n)) >> 32) > > Won't this have the unwanted side effect of promoting everything in a > calculation to long long on 32 bit platforms, even if n was only 32 > bits? bummer. > > + > > + > > #defineKERN_EMERG "<0>" /* system is unusable > > */ > > #defineKERN_ALERT "<1>" /* action must be taken immediately > > */ > > #defineKERN_CRIT "<2>" /* critical conditions > > */ > > _ > > > > It seems to generate the desired code. I avoided Alan's ((n >> 31) >> 1) > > trick because it'll generate peculiar results with signed 64-bit > > quantities. > > I've seen the trick done similarly with ((n >> 16) >> 16) which > shouldn't have the issue. That works if we know the caller is treating the return value as 32 bits, but we don't know that. If we have #define upper_32_bits(x) ((x >> 16) >> 16) then upper_32_bits(0x) will return 0x if it's treated as 32-bits, but it'll return 0x if the caller is using 64-bits. I spose #define upper_32_bits(x) ((u32)((x >> 16) >> 16)) will do the trick. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2]: PCI Error Recovery: Symbios SCSI First Failure
Implement the so-called "first failure data capture" (FFDC) for the
symbios PCI error recovery. After a PCI error event is reported,
the driver requests that MMIO be enabled. Once enabled, it
then reads and dumps assorted status registers, and concludes
by requesting the usual reset sequence.
(includes a whitespace fix for bad indentation).
Signed-off-by: Linas Vepstas <[EMAIL PROTECTED]>
drivers/scsi/sym53c8xx_2/sym_glue.c | 15 +++
drivers/scsi/sym53c8xx_2/sym_glue.h |1 +
drivers/scsi/sym53c8xx_2/sym_hipd.c | 18 ++
3 files changed, 30 insertions(+), 4 deletions(-)
Index: linux-2.6.21-rc4-git4/drivers/scsi/sym53c8xx_2/sym_glue.c
===
--- linux-2.6.21-rc4-git4.orig/drivers/scsi/sym53c8xx_2/sym_glue.c
2007-04-20 12:52:01.0 -0500
+++ linux-2.6.21-rc4-git4/drivers/scsi/sym53c8xx_2/sym_glue.c 2007-04-20
15:25:35.0 -0500
@@ -1987,6 +1987,20 @@ static pci_ers_result_t sym2_io_error_de
disable_irq(pdev->irq);
pci_disable_device(pdev);
+ /* Request that MMIO be enabled, so register dump can be taken. */
+ return PCI_ERS_RESULT_CAN_RECOVER;
+}
+
+/**
+ * sym2_io_slot_dump -- Enable MMIO and dump debug registers
+ * @pdev: pointer to PCI device
+ */
+static pci_ers_result_t sym2_io_slot_dump (struct pci_dev *pdev)
+{
+ struct sym_hcb *np = pci_get_drvdata(pdev);
+
+ sym_dump_registers(np);
+
/* Request a slot reset. */
return PCI_ERS_RESULT_NEED_RESET;
}
@@ -2241,6 +2255,7 @@ MODULE_DEVICE_TABLE(pci, sym2_id_table);
static struct pci_error_handlers sym2_err_handler = {
.error_detected = sym2_io_error_detected,
+ .mmio_enabled = sym2_io_slot_dump,
.slot_reset = sym2_io_slot_reset,
.resume = sym2_io_resume,
};
Index: linux-2.6.21-rc4-git4/drivers/scsi/sym53c8xx_2/sym_glue.h
===
--- linux-2.6.21-rc4-git4.orig/drivers/scsi/sym53c8xx_2/sym_glue.h
2007-04-20 12:15:07.0 -0500
+++ linux-2.6.21-rc4-git4/drivers/scsi/sym53c8xx_2/sym_glue.h 2007-04-20
15:21:31.0 -0500
@@ -270,5 +270,6 @@ void sym_xpt_async_bus_reset(struct sym_
void sym_xpt_async_sent_bdr(struct sym_hcb *np, int target);
int sym_setup_data_and_start (struct sym_hcb *np, struct scsi_cmnd *csio,
struct sym_ccb *cp);
void sym_log_bus_error(struct sym_hcb *np);
+void sym_dump_registers(struct sym_hcb *np);
#endif /* SYM_GLUE_H */
Index: linux-2.6.21-rc4-git4/drivers/scsi/sym53c8xx_2/sym_hipd.c
===
--- linux-2.6.21-rc4-git4.orig/drivers/scsi/sym53c8xx_2/sym_hipd.c
2007-04-20 12:18:59.0 -0500
+++ linux-2.6.21-rc4-git4/drivers/scsi/sym53c8xx_2/sym_hipd.c 2007-04-20
15:18:01.0 -0500
@@ -1180,10 +1180,10 @@ static void sym_log_hard_error(struct sy
scr_to_cpu((int) *(u32 *)(script_base + script_ofs)));
}
-printf ("%s: regdump:", sym_name(np));
-for (i=0; i<24;i++)
-printf (" %02x", (unsigned)INB_OFF(np, i));
-printf (".\n");
+ printf ("%s: regdump:", sym_name(np));
+ for (i=0; i<24;i++)
+ printf (" %02x", (unsigned)INB_OFF(np, i));
+ printf (".\n");
/*
* PCI BUS error.
@@ -1192,6 +1192,16 @@ static void sym_log_hard_error(struct sy
sym_log_bus_error(np);
}
+void sym_dump_registers(struct sym_hcb *np)
+{
+ u_short sist;
+ u_char dstat;
+
+ sist = INW(np, nc_sist);
+ dstat = INB(np, nc_dstat);
+ sym_log_hard_error(np, sist, dstat);
+}
+
static struct sym_chip sym_dev_table[] = {
{PCI_DEVICE_ID_NCR_53C810, 0x0f, "810", 4, 8, 4, 64,
FE_ERL}
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2]: PCI Error Recovery: Symbios SCSI base support
Hi Matthew,
After a long hiatus, I took another stab at pci error recovery
for the symbios. This is very nearly the same patch as before,
with only an update to enable MWI, and to support chip workarounds.
I think I've addressed all the other issues that came up. Thus,
again, I'll ask that the patch go in (for 2.6.22 of course).
To recap the only outstanding issue:
>> @@ -657,6 +657,10 @@ static irqreturn_t sym53c8xx_intr(int ir
>> + /* Avoid spinloop trying to handle interrupts on frozen device */
>> + if (pci_channel_offline(np->s.device))
>> + return IRQ_HANDLED;
>
>Just wondering ... should we really be returning HANDLED? What if the
>IRQ is shared? Will the hardware de-assert the level interrupt when it
>puts the device in reset (ie is this a transitory glitch?), or do we
>have to cope with a screaming interrupt?
This routine *always* returns HANDLED anyway, so this patch does
not change semantics. For a symbios device plugged into a shared
irq line, this is a problem with or without my patch.
Yes, irq's will typically scream until handled. Yes, the device
reset will eventually clear the irq, assuming the system doesn't
deadlock on a screaming irq.
--linas
Here's the formal changelog entry:
Various PCI bus errors can be signaled by newer PCI controllers.
This patch adds the PCI error recovery callbacks to the Symbios
SCSI device driver. The patch has been tested, and appears to
work well.
Signed-off-by: Linas Vepstas <[EMAIL PROTECTED]>
--
drivers/scsi/sym53c8xx_2/sym_glue.c | 136
drivers/scsi/sym53c8xx_2/sym_glue.h |4 +
drivers/scsi/sym53c8xx_2/sym_hipd.c |6 +
3 files changed, 146 insertions(+)
Index: linux-2.6.21-rc4-git4/drivers/scsi/sym53c8xx_2/sym_glue.c
===
--- linux-2.6.21-rc4-git4.orig/drivers/scsi/sym53c8xx_2/sym_glue.c
2007-04-20 12:07:38.0 -0500
+++ linux-2.6.21-rc4-git4/drivers/scsi/sym53c8xx_2/sym_glue.c 2007-04-20
12:52:01.0 -0500
@@ -657,6 +657,10 @@ static irqreturn_t sym53c8xx_intr(int ir
unsigned long flags;
struct sym_hcb *np = (struct sym_hcb *)dev_id;
+ /* Avoid spinloop trying to handle interrupts on frozen device */
+ if (pci_channel_offline(np->s.device))
+ return IRQ_HANDLED;
+
if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("[");
spin_lock_irqsave(np->s.host->host_lock, flags);
@@ -726,6 +730,20 @@ static int sym_eh_handler(int op, char *
dev_warn(&cmd->device->sdev_gendev, "%s operation started.\n", opname);
+ /* We may be in an error condition because the PCI bus
+* went down. In this case, we need to wait until the
+* PCI bus is reset, the card is reset, and only then
+* proceed with the scsi error recovery. There's no
+* point in hurrying; take a leisurely wait.
+*/
+#define WAIT_FOR_PCI_RECOVERY 35
+ if (pci_channel_offline(np->s.device)) {
+ int finished_reset = wait_for_completion_timeout(
+ &np->s.io_reset_wait, WAIT_FOR_PCI_RECOVERY*HZ);
+ if (!finished_reset)
+ return SCSI_FAILED;
+ }
+
spin_lock_irq(host->host_lock);
/* This one is queued in some place -> to wait for completion */
FOR_EACH_QUEUED_ELEMENT(&np->busy_ccbq, qp) {
@@ -1510,6 +1528,7 @@ static struct Scsi_Host * __devinit sym_
np->maxoffs = dev->chip.offset_max;
np->maxburst= dev->chip.burst_max;
np->myaddr = dev->host_id;
+ init_completion(&np->s.io_reset_wait);
/*
* Edit its name.
@@ -1948,6 +1967,116 @@ static void __devexit sym2_remove(struct
attach_count--;
}
+/**
+ * sym2_io_error_detected() -- called when PCI error is detected
+ * @pdev: pointer to PCI device
+ * @state: current state of the PCI slot
+ */
+static pci_ers_result_t sym2_io_error_detected (struct pci_dev *pdev,
+ enum pci_channel_state state)
+{
+ struct sym_hcb *np = pci_get_drvdata(pdev);
+
+ /* If slot is permanently frozen, turn everything off */
+ if (state == pci_channel_io_perm_failure) {
+ sym2_remove(pdev);
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
+ init_completion(&np->s.io_reset_wait);
+ disable_irq(pdev->irq);
+ pci_disable_device(pdev);
+
+ /* Request a slot reset. */
+ return PCI_ERS_RESULT_NEED_RESET;
+}
+
+/**
+ * sym2_reset_workarounds -- hardware-specific work-arounds
+ *
+ * This routine is similar to sym_set_workarounds(), except
+ * that, at this point, we already know that the device was
+ * succesfully intialized at least once before, and so most
+ * of the steps taken there are un-needed here.
+ */
+static void sym2_reset_workarounds (struct pci_dev *pdev)
+{
+ u_char revision;
+ u_short status_reg;
+
Re: qla2xxx hba crashes with older 2310 cards
On Fri, 2007-04-20 at 13:24 -0700, David Miller wrote: > From: Robert Peterson <[EMAIL PROTECTED]> > Date: Fri, 20 Apr 2007 10:40:30 -0500 > > > I've seen some chatter about the qla2xxx driver but not paid attention, so > > I'm sorry if this is a known issue. I've got an older qlogic hba, and > > recent > > drivers don't seem to play nice with it. I've got the latest firmware from > > qlogic's web site. I'm using a 2.6.21-rc6 kernel from Steve Whitehouse's > > -nmw git tree. Reverting to an older driver (but same kernel) and it works. > > The current driver gives this: > > Yes, known problem, I'm sorry these guys broke the driver for > you as well, please see this thread: > > http://marc.info/?l=linux-kernel&m=117671067701124&w=2 > > This was really a stupid change to make. OK,OK, we heard you the first time ... the maintainers will try to fix this in a manner acceptable to all concerned ... could we try to cool down the public traductions now? James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: qla2xxx hba crashes with older 2310 cards
From: Robert Peterson <[EMAIL PROTECTED]> Date: Fri, 20 Apr 2007 10:40:30 -0500 > I've seen some chatter about the qla2xxx driver but not paid attention, so > I'm sorry if this is a known issue. I've got an older qlogic hba, and recent > drivers don't seem to play nice with it. I've got the latest firmware from > qlogic's web site. I'm using a 2.6.21-rc6 kernel from Steve Whitehouse's > -nmw git tree. Reverting to an older driver (but same kernel) and it works. > The current driver gives this: Yes, known problem, I'm sorry these guys broke the driver for you as well, please see this thread: http://marc.info/?l=linux-kernel&m=117671067701124&w=2 This was really a stupid change to make. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cciss: Fix warnings during compilation under 32bitenvironment
On Fri, 2007-04-20 at 12:30 -0700, Andrew Morton wrote: > On Fri, 20 Apr 2007 14:50:06 -0400 > James Bottomley <[EMAIL PROTECTED]> wrote: > > > > CONFIG_LBD=y gives us an additional 3kb of instructions on i386 > > > allnoconfig. Other architectures might do less well. It's not a huge > > > difference, but that's the way in which creeping bloatiness happens. > > > > OK, sure, but if we really care about this saving, then unconditionally > > casting to u64 is therefore wrong as well ... this is starting to open > > quite a large can of worms ... > > > > For the record, if we have to do this, I fancy sector_upper_32() ... we > > should already have some similar accessor for dma_addr_t as well. > > hm. How about this? > > --- a/include/linux/kernel.h~upper-32-bits > +++ a/include/linux/kernel.h > @@ -40,6 +40,17 @@ extern const char linux_proc_banner[]; > #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d)) > #define roundup(x, y) x) + ((y) - 1)) / (y)) * (y)) > > +/** > + * upper_32_bits - return bits 32-63 of a number > + * @n: the number we're accessing > + * > + * A basic shift-right of a 64- or 32-bit quantity. Use this to suppress > + * the "right shift count >= width of type" warning when that quantity is > + * 32-bits. > + */ > +#define upper_32_bits(n) (((u64)(n)) >> 32) Won't this have the unwanted side effect of promoting everything in a calculation to long long on 32 bit platforms, even if n was only 32 bits? > + > + > #define KERN_EMERG "<0>" /* system is unusable > */ > #define KERN_ALERT "<1>" /* action must be taken immediately > */ > #define KERN_CRIT "<2>" /* critical conditions > */ > _ > > It seems to generate the desired code. I avoided Alan's ((n >> 31) >> 1) > trick because it'll generate peculiar results with signed 64-bit > quantities. I've seen the trick done similarly with ((n >> 16) >> 16) which shouldn't have the issue. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cciss: Fix warnings during compilation under 32bitenvironment
On Fri, 20 Apr 2007 14:50:06 -0400 James Bottomley <[EMAIL PROTECTED]> wrote: > > CONFIG_LBD=y gives us an additional 3kb of instructions on i386 > > allnoconfig. Other architectures might do less well. It's not a huge > > difference, but that's the way in which creeping bloatiness happens. > > OK, sure, but if we really care about this saving, then unconditionally > casting to u64 is therefore wrong as well ... this is starting to open > quite a large can of worms ... > > For the record, if we have to do this, I fancy sector_upper_32() ... we > should already have some similar accessor for dma_addr_t as well. hm. How about this? --- a/include/linux/kernel.h~upper-32-bits +++ a/include/linux/kernel.h @@ -40,6 +40,17 @@ extern const char linux_proc_banner[]; #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d)) #define roundup(x, y) x) + ((y) - 1)) / (y)) * (y)) +/** + * upper_32_bits - return bits 32-63 of a number + * @n: the number we're accessing + * + * A basic shift-right of a 64- or 32-bit quantity. Use this to suppress + * the "right shift count >= width of type" warning when that quantity is + * 32-bits. + */ +#define upper_32_bits(n) (((u64)(n)) >> 32) + + #defineKERN_EMERG "<0>" /* system is unusable */ #defineKERN_ALERT "<1>" /* action must be taken immediately */ #defineKERN_CRIT "<2>" /* critical conditions */ _ It seems to generate the desired code. I avoided Alan's ((n >> 31) >> 1) trick because it'll generate peculiar results with signed 64-bit quantities. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cciss: Fix warnings during compilation under 32bitenvironment
On Fri, 2007-04-20 at 11:43 -0700, Andrew Morton wrote:
> On Fri, 20 Apr 2007 09:10:41 -0400
> James Bottomley <[EMAIL PROTECTED]> wrote:
>
> > On Thu, 2007-04-19 at 22:20 -0700, Andrew Morton wrote:
> > > On Thu, 19 Apr 2007 16:27:26 - "Cameron, Steve" <[EMAIL PROTECTED]>
> > > wrote:
> > > >
> > > > Something like
> > > >
> > > > if (sizeof(blah) > 4) {
> > > >do all the assignments with shifts
> > > > }
> > > >
> > > > might be slighly better since the CDB is already zeroed
> > > > by cmd_alloc() and doesn't need to be zeroed a 2nd time.
> > > >
> > > > -- steve
> > > >
> > > > -Original Message-
> > > > From: James Bottomley [mailto:[EMAIL PROTECTED]
> > > > Sent: Thu 4/19/2007 11:22 AM
> > > > To: Miller, Mike (OS Dev)
> > > > Cc: Hisashi Hifumi; [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL
> > > > PROTECTED]; linux-scsi@vger.kernel.org; Cameron, Steve
> > > > Subject: RE: [PATCH] cciss: Fix warnings during compilation under
> > > > 32bitenvironment
> > > >
> > > > On Thu, 2007-04-19 at 16:12 +, Miller, Mike (OS Dev) wrote:
> > > > > > > Nak. You still haven't told where you saw these warnings. What
> > > > > > > compiler are you using? I do not see these in my 32-bit
> > > > > > > environment.
> > > > > >
> > > > > > I think it's seen with CONFIG_LBD=n on 32 bits
> > > > > >
> > > > > > In that configuration, sector_t is a u32 (it's u64 even on 32
> > > > > > bits with CONFIG_LBD=y). The proposed code change is a
> > > > > > simple cut and paste from the sd driver.
> > > > >
> > > > > Isn't there a better way than testing each one?
> > > >
> > > > It's not such a bad option. The sizeof() test is compile time
> > > > determinable, so the compiler simply zeros the fields in the
> > > > CONFIG_LBD=n case and does the shift for CONFIG_LBD=y. It certainly
> > > > never compiles to four inline condition checks.
> > > >
> > >
> > > Boy you guys make a mess of a nice email trail :(
> > >
> > >
> > > --- linux-2.6.21-rc7.org/drivers/block/cciss.c2007-04-17
> > > 16:36:02.0 +0900
> > > +++ linux-2.6.21-rc7/drivers/block/cciss.c2007-04-17
> > > 16:25:53.0 +0900
> > > @@ -2552,10 +2552,10 @@ static void do_cciss_request(request_que
> > > } 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[2]= sizeof(start_blk) > 4 ? (start_blk >> 56) &
> > > 0xff : 0;//MSB
> > > + c->Request.CDB[3]= sizeof(start_blk) > 4 ? (start_blk >> 48) &
> > > 0xff : 0;
> > > + c->Request.CDB[4]= sizeof(start_blk) > 4 ? (start_blk >> 40) &
> > > 0xff : 0;
> > > + c->Request.CDB[5]= sizeof(start_blk) > 4 ? (start_blk >> 32) &
> > > 0xff : 0;
> > > c->Request.CDB[6]= (start_blk >> 24) & 0xff;
> > > c->Request.CDB[7]= (start_blk >> 16) & 0xff;
> > > c->Request.CDB[8]= (start_blk >> 8) & 0xff;
> > >
> > > This is not the first time we've hit this problem and presumably it won't
> > > be the last time.
> > >
> > > Could we do something like
> > >
> > > #if (BITS_PER_LONG > 32) || defined(CONFIG_LBD)
> > > #define sector_upper_32(sector) ((sector) >> 32)
> > > #else
> > > #define sector_upper_32(sector) (0)
> > > #endif
> > >
> > > and then cciss can do
> > >
> > > - c->Request.CDB[2]= start_blk >> 56;
> > > + c->Request.CDB[2]= sector_upper_32(start_blk) >> 24;
> > >
> > > which will do the right thing.
> >
> > Sure, we could do that. The slight problem is that we don't have
> > general agreement in the kernel how to handle sector_t. So, the only
> > consumer in scsi, sd, does the sizeof(block) ? thing. If you look in
> > libata you'll see that it unconditionally uses a u64 for picking up the
> > value of sector_t so the shift is never invalid ... if we're going to
> > start making macros for handling this, we probably need to ask the
> > janitors to fix all of our existing code to use them ... or we could
> > just let sleeping dogs lie ..
>
> Let's decide how we _should_ do it, implement that, then teach cciss about
> it, then try to ensure that new code uses the approved interfaces. Over
> time, people may (or may not) migrate existing code over to the new
> interfaces.
>
> They may also merge new code which uses open-coded hand-rolled stuff, too :(
>
> > Is there even a valid use for CONFIG_LBD=n anymore, anyway?
>
> Lots and lots and lots of systems don't have any disks larger than 2TB.
>
> CONFIG_LBD=y gives us an additional 3kb of instructions on i386
> allnoconfig. Other architectures might do less well. It's not a huge
> difference, but that's the way in which creeping bloatiness happens.
OK, sure, but if we really care about this saving, then unconditionally
cas
Re: [PATCH] cciss: Fix warnings during compilation under 32bitenvironment
On Fri, 20 Apr 2007 09:10:41 -0400
James Bottomley <[EMAIL PROTECTED]> wrote:
> On Thu, 2007-04-19 at 22:20 -0700, Andrew Morton wrote:
> > On Thu, 19 Apr 2007 16:27:26 - "Cameron, Steve" <[EMAIL PROTECTED]>
> > wrote:
> > >
> > > Something like
> > >
> > > if (sizeof(blah) > 4) {
> > >do all the assignments with shifts
> > > }
> > >
> > > might be slighly better since the CDB is already zeroed
> > > by cmd_alloc() and doesn't need to be zeroed a 2nd time.
> > >
> > > -- steve
> > >
> > > -Original Message-
> > > From: James Bottomley [mailto:[EMAIL PROTECTED]
> > > Sent: Thu 4/19/2007 11:22 AM
> > > To: Miller, Mike (OS Dev)
> > > Cc: Hisashi Hifumi; [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL
> > > PROTECTED]; linux-scsi@vger.kernel.org; Cameron, Steve
> > > Subject: RE: [PATCH] cciss: Fix warnings during compilation under
> > > 32bitenvironment
> > >
> > > On Thu, 2007-04-19 at 16:12 +, Miller, Mike (OS Dev) wrote:
> > > > > > Nak. You still haven't told where you saw these warnings. What
> > > > > > compiler are you using? I do not see these in my 32-bit environment.
> > > > >
> > > > > I think it's seen with CONFIG_LBD=n on 32 bits
> > > > >
> > > > > In that configuration, sector_t is a u32 (it's u64 even on 32
> > > > > bits with CONFIG_LBD=y). The proposed code change is a
> > > > > simple cut and paste from the sd driver.
> > > >
> > > > Isn't there a better way than testing each one?
> > >
> > > It's not such a bad option. The sizeof() test is compile time
> > > determinable, so the compiler simply zeros the fields in the
> > > CONFIG_LBD=n case and does the shift for CONFIG_LBD=y. It certainly
> > > never compiles to four inline condition checks.
> > >
> >
> > Boy you guys make a mess of a nice email trail :(
> >
> >
> > --- linux-2.6.21-rc7.org/drivers/block/cciss.c 2007-04-17
> > 16:36:02.0 +0900
> > +++ linux-2.6.21-rc7/drivers/block/cciss.c 2007-04-17 16:25:53.0
> > +0900
> > @@ -2552,10 +2552,10 @@ static void do_cciss_request(request_que
> > } 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[2]= sizeof(start_blk) > 4 ? (start_blk >> 56) &
> > 0xff : 0;//MSB
> > + c->Request.CDB[3]= sizeof(start_blk) > 4 ? (start_blk >> 48) &
> > 0xff : 0;
> > + c->Request.CDB[4]= sizeof(start_blk) > 4 ? (start_blk >> 40) &
> > 0xff : 0;
> > + c->Request.CDB[5]= sizeof(start_blk) > 4 ? (start_blk >> 32) &
> > 0xff : 0;
> > c->Request.CDB[6]= (start_blk >> 24) & 0xff;
> > c->Request.CDB[7]= (start_blk >> 16) & 0xff;
> > c->Request.CDB[8]= (start_blk >> 8) & 0xff;
> >
> > This is not the first time we've hit this problem and presumably it won't
> > be the last time.
> >
> > Could we do something like
> >
> > #if (BITS_PER_LONG > 32) || defined(CONFIG_LBD)
> > #define sector_upper_32(sector) ((sector) >> 32)
> > #else
> > #define sector_upper_32(sector) (0)
> > #endif
> >
> > and then cciss can do
> >
> > - c->Request.CDB[2]= start_blk >> 56;
> > + c->Request.CDB[2]= sector_upper_32(start_blk) >> 24;
> >
> > which will do the right thing.
>
> Sure, we could do that. The slight problem is that we don't have
> general agreement in the kernel how to handle sector_t. So, the only
> consumer in scsi, sd, does the sizeof(block) ? thing. If you look in
> libata you'll see that it unconditionally uses a u64 for picking up the
> value of sector_t so the shift is never invalid ... if we're going to
> start making macros for handling this, we probably need to ask the
> janitors to fix all of our existing code to use them ... or we could
> just let sleeping dogs lie ..
Let's decide how we _should_ do it, implement that, then teach cciss about
it, then try to ensure that new code uses the approved interfaces. Over
time, people may (or may not) migrate existing code over to the new
interfaces.
They may also merge new code which uses open-coded hand-rolled stuff, too :(
> Is there even a valid use for CONFIG_LBD=n anymore, anyway?
Lots and lots and lots of systems don't have any disks larger than 2TB.
CONFIG_LBD=y gives us an additional 3kb of instructions on i386
allnoconfig. Other architectures might do less well. It's not a huge
difference, but that's the way in which creeping bloatiness happens.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: InitIO SCSI: Volunteers required
On Friday 20 April 2007 11:38:49 Alan Cox wrote: > I'm looking for some testers for a revamp of the initio driver. No real > code changes other than to hopefully stop it exploding on load on 64bit, > but a major reorganisation, commenting and "de-windowsification" so the > code is actually readable and I can do the pci_find_device to > pci_get_device transitions. I once had such a card flying around, feel free to attach it to me and I can see if I find the card in some box over the weekend :-) Yours, -- René Rebe - ExactCODE GmbH - Europe, Germany, Berlin Geschäftsführer: Susanne Klaus, René Rebe Sitz: Berlin, Amtsgericht Charlottenburg HRB 105 123 B USt-IdNr.: DE251602478 http://exactcode.de | http://t2-project.org | http://rene.rebe.name - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cciss: Fix warnings during compilation under 32bitenvironment
> > #if (BITS_PER_LONG > 32) || defined(CONFIG_LBD) > > #define sector_upper_32(sector) ((sector) >> 32) > > #else > > #define sector_upper_32(sector) (0) > > #endif Gak Just do sector_upper_32(sector) (((sector) >> 31) >> 1) and lose all the ifdefs, Alan - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cciss: Fix warnings during compilation under 32bitenvironment
On Thu, 2007-04-19 at 22:20 -0700, Andrew Morton wrote:
> On Thu, 19 Apr 2007 16:27:26 - "Cameron, Steve" <[EMAIL PROTECTED]> wrote:
> >
> > Something like
> >
> > if (sizeof(blah) > 4) {
> >do all the assignments with shifts
> > }
> >
> > might be slighly better since the CDB is already zeroed
> > by cmd_alloc() and doesn't need to be zeroed a 2nd time.
> >
> > -- steve
> >
> > -Original Message-
> > From: James Bottomley [mailto:[EMAIL PROTECTED]
> > Sent: Thu 4/19/2007 11:22 AM
> > To: Miller, Mike (OS Dev)
> > Cc: Hisashi Hifumi; [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL
> > PROTECTED]; linux-scsi@vger.kernel.org; Cameron, Steve
> > Subject: RE: [PATCH] cciss: Fix warnings during compilation under
> > 32bitenvironment
> >
> > On Thu, 2007-04-19 at 16:12 +, Miller, Mike (OS Dev) wrote:
> > > > > Nak. You still haven't told where you saw these warnings. What
> > > > > compiler are you using? I do not see these in my 32-bit environment.
> > > >
> > > > I think it's seen with CONFIG_LBD=n on 32 bits
> > > >
> > > > In that configuration, sector_t is a u32 (it's u64 even on 32
> > > > bits with CONFIG_LBD=y). The proposed code change is a
> > > > simple cut and paste from the sd driver.
> > >
> > > Isn't there a better way than testing each one?
> >
> > It's not such a bad option. The sizeof() test is compile time
> > determinable, so the compiler simply zeros the fields in the
> > CONFIG_LBD=n case and does the shift for CONFIG_LBD=y. It certainly
> > never compiles to four inline condition checks.
> >
>
> Boy you guys make a mess of a nice email trail :(
>
>
> --- linux-2.6.21-rc7.org/drivers/block/cciss.c2007-04-17
> 16:36:02.0 +0900
> +++ linux-2.6.21-rc7/drivers/block/cciss.c2007-04-17 16:25:53.0
> +0900
> @@ -2552,10 +2552,10 @@ static void do_cciss_request(request_que
> } 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[2]= sizeof(start_blk) > 4 ? (start_blk >> 56) &
> 0xff : 0;//MSB
> + c->Request.CDB[3]= sizeof(start_blk) > 4 ? (start_blk >> 48) &
> 0xff : 0;
> + c->Request.CDB[4]= sizeof(start_blk) > 4 ? (start_blk >> 40) &
> 0xff : 0;
> + c->Request.CDB[5]= sizeof(start_blk) > 4 ? (start_blk >> 32) &
> 0xff : 0;
> c->Request.CDB[6]= (start_blk >> 24) & 0xff;
> c->Request.CDB[7]= (start_blk >> 16) & 0xff;
> c->Request.CDB[8]= (start_blk >> 8) & 0xff;
>
> This is not the first time we've hit this problem and presumably it won't
> be the last time.
>
> Could we do something like
>
> #if (BITS_PER_LONG > 32) || defined(CONFIG_LBD)
> #define sector_upper_32(sector) ((sector) >> 32)
> #else
> #define sector_upper_32(sector) (0)
> #endif
>
> and then cciss can do
>
> - c->Request.CDB[2]= start_blk >> 56;
> + c->Request.CDB[2]= sector_upper_32(start_blk) >> 24;
>
> which will do the right thing.
Sure, we could do that. The slight problem is that we don't have
general agreement in the kernel how to handle sector_t. So, the only
consumer in scsi, sd, does the sizeof(block) ? thing. If you look in
libata you'll see that it unconditionally uses a u64 for picking up the
value of sector_t so the shift is never invalid ... if we're going to
start making macros for handling this, we probably need to ask the
janitors to fix all of our existing code to use them ... or we could
just let sleeping dogs lie ..
Is there even a valid use for CONFIG_LBD=n anymore, anyway?
James
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
InitIO SCSI: Volunteers required
I'm looking for some testers for a revamp of the initio driver. No real code changes other than to hopefully stop it exploding on load on 64bit, but a major reorganisation, commenting and "de-windowsification" so the code is actually readable and I can do the pci_find_device to pci_get_device transitions. Alan - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
mpt: sync offset errors?
System: linux-2.6.21rc3 3 53c1030 2 times 53c1030 PCI-X Fusion-MPT Dual Ultra320 SCSI version: 08 1 time version: 07 After adding another disk array I'm getting some kernel: mptbase: ioc1: LogInfo(0x1104): F/W: SYNC Offset Error mptbase: ioc4: LogInfo(0x1104): F/W: SYNC Offset Error (both are the new disk cage) But the system seems to be working as expected (performance o.k.) What could be the reason of that? Should I be afraid? Gerhard Schneider -- Gerhard Schneider Institute of Lightweight Design and e-Mail:[EMAIL PROTECTED] Structural Biomechanics (E317) Tel.: +43 1 58801 31716 Vienna University of Technology / Austria Fax:+43 1 58801 31799 A-1040 Wien, Gusshausstrasse 27-29 http://www.ilsb.tuwien.ac.at/~gs/ signature.asc Description: OpenPGP digital signature
