Re: BAD_SG_DMA panic in aha1542
Alan Cox wrote: > > As before, no problems using the sda hard disk (which is the boot drive): > > everything works reliably until I touch the cdrom drive. > > A little quiet contemplation and gnome number 387 suggests trying the > following > (and providing more detailed information such as the last message printed > before > the DMA message). Stuff a BUG() before the panic in BAD_DMA (aha1542.c) if > needed > to get a good trace. > > Please report success/failure/change. Can do. I don't have access to the machine on weekends, so it will be at least Monday before I can give this a whirl. Thanks! -- --- Bob Tracy WTO + WIPO = DMCA? http://www.anti-dmca.org [EMAIL PROTECTED] --- - 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: BAD_SG_DMA panic in aha1542
James Bottomley wrote: > On Fri, 2007-04-27 at 16:47 -0500, Bob Tracy wrote: > > I previously reported an ISA DMA issue for the 2.6.12 kernel. The issue > > persists through at least 2.6.18. SCSI controller is an Adaptec > > AHA-1542B (ISA). > > > > The action "mount -t iso9660 /dev/scd0 /mnt/cdrom -r" > > > > produces > > > > (cdrom detection messages as various modules autoload, then...) > > Knowing what these messages are is would be helpful; it tells me what > point in the initialisation it got to. Sorry about that... I'm running the DSL-N distribution (based on Knoppix), and having to transcribe the log messages by hand from the console, i.e., there's no logfile to cut-and-paste from :-(. I don't have access to the machine except on weekdays, but I'll repeat the crash first thing Monday morning and copy everything that's there... > I'm interested. > > This is clearly a use_sg==1 path that has failed to bounce the buffer > for some reason ... and I was contemplating eliminating the GFP_DMA from > our sr driver because I thought the block bouncing had it covered. > > It might also be helpful to apply this patch. It should give a stack > trace of the problem command and not immediately panic the box. I'll throw together a 2.6.21 kernel with this patch and give it a try. Again, it will be at least Monday before you hear back from me on this. Thanks! -- --- Bob Tracy WTO + WIPO = DMCA? http://www.anti-dmca.org [EMAIL PROTECTED] --- - 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: BAD_SG_DMA panic in aha1542
On Fri, 2007-04-27 at 16:47 -0500, Bob Tracy wrote:
> I previously reported an ISA DMA issue for the 2.6.12 kernel. The issue
> persists through at least 2.6.18. SCSI controller is an Adaptec
> AHA-1542B (ISA).
>
> The action "mount -t iso9660 /dev/scd0 /mnt/cdrom -r"
>
> produces
>
> (cdrom detection messages as various modules autoload, then...)
Knowing what these messages are is would be helpful; it tells me what
point in the initialisation it got to.
> sgpnt[0:1] page c1ee5af0/0x1ee5af0 length 32
> Kernel panic - not syncing: Buffer at physical address > 16 Mb used for
> aha1542
>
> As before, no problems using the sda hard disk (which is the boot drive):
> everything works reliably until I touch the cdrom drive.
>
> I'll be happy to assist with the debugging, but the system with the
> aha1542 has no development facilities, i.e., I'll have to build test
> kernels on a different system, and turnaround is going to be slow :-(.
I'm interested.
This is clearly a use_sg==1 path that has failed to bounce the buffer
for some reason ... and I was contemplating eliminating the GFP_DMA from
our sr driver because I thought the block bouncing had it covered.
It might also be helpful to apply this patch. It should give a stack
trace of the problem command and not immediately panic the box.
Thanks,
James
diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
index 1d239f6..4ee7d99 100644
--- a/drivers/scsi/aha1542.c
+++ b/drivers/scsi/aha1542.c
@@ -75,7 +75,7 @@ static void BAD_SG_DMA(Scsi_Cmnd * SCpnt,
/*
* Not safe to continue.
*/
- panic("Buffer at physical address > 16Mb used for aha1542");
+ WARN_ON(1);
}
#include
@@ -725,8 +725,12 @@ static int aha1542_queuecommand(Scsi_Cmnd * SCpnt, void
(*done) (Scsi_Cmnd *))
panic("Fod fight!");
};
any2scsi(cptr[i].dataptr, SCSI_SG_PA(&sgpnt[i]));
- if (SCSI_SG_PA(&sgpnt[i]) + sgpnt[i].length - 1 >
ISA_DMA_THRESHOLD)
+ if (SCSI_SG_PA(&sgpnt[i]) + sgpnt[i].length - 1 >
ISA_DMA_THRESHOLD) {
BAD_SG_DMA(SCpnt, sgpnt, SCpnt->use_sg, i);
+ SCpnt->result = DID_ERROR << 16;
+ done(SCpnt);
+ return 0;
+ }
any2scsi(cptr[i].datalen, sgpnt[i].length);
};
any2scsi(ccb[mbo].datalen, SCpnt->use_sg * sizeof(struct
chain));
-
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: BAD_SG_DMA panic in aha1542
> As before, no problems using the sda hard disk (which is the boot drive):
> everything works reliably until I touch the cdrom drive.
A little quiet contemplation and gnome number 387 suggests trying the following
(and providing more detailed information such as the last message printed before
the DMA message). Stuff a BUG() before the panic in BAD_DMA (aha1542.c) if
needed
to get a good trace.
Please report success/failure/change.
--- drivers/scsi/sr_ioctl.c~2007-04-27 22:53:33.885035256 +0100
+++ drivers/scsi/sr_ioctl.c 2007-04-27 22:53:33.885035256 +0100
@@ -187,9 +187,10 @@
struct scsi_sense_hdr sshdr;
int result, err = 0, retries = 0;
struct request_sense *sense = cgc->sense;
-
+ void *zebedee = cgc->buffer;
+
SDev = cd->device;
-
+
if (!sense) {
sense = kmalloc(SCSI_SENSE_BUFFERSIZE, GFP_KERNEL);
if (!sense) {
@@ -197,7 +198,22 @@
goto out;
}
}
-
+
+ if (cgc->buflen && cd->device->host->unchecked_isa_dma) {
+ switch(cgc->data_direction) {
+ case DMA_NONE:
+ break;
+ case DMA_FROM_DEVICE:
+ case DMA_TO_DEVICE: /* Boing said Zebedee */
+ zebedee = kmalloc(cgc->buflen,
GFP_KERNEL|GFP_DMA);
+ if (zebedee ==NULL) {
+ err = -ENOMEM;
+ goto out;
+ }
+ }
+ if (cgc->data_direction == DMA_TO_DEVICE)
+ memcpy(zebedee, cgc->buffer, cgc->buflen);
+ }
retry:
if (!scsi_block_when_processing_errors(SDev)) {
err = -ENODEV;
@@ -206,10 +222,13 @@
memset(sense, 0, sizeof(*sense));
result = scsi_execute(SDev, cgc->cmd, cgc->data_direction,
- cgc->buffer, cgc->buflen, (char *)sense,
+ zebedee, cgc->buflen, (char *)sense,
cgc->timeout, IOCTL_RETRIES, 0);
scsi_normalize_sense((char *)sense, sizeof(*sense), &sshdr);
+
+ if (zebedeee != cgc->buffer && cgc->data_direction == DMA_FROM_DEVICE)
+ memcpy(cgc->buffer, zebedee, cgc->buflen);
/* Minimal error checking. Ignore cases we know about, and report the
rest. */
if (driver_byte(result) != 0) {
-
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] esp_scsi.c: fix compilation
From: Alexey Dobriyan <[EMAIL PROTECTED]> Date: Sat, 28 Apr 2007 02:09:01 +0400 > irqreturn.h for irqreturn_t and dma_addr_t being u128 warnings ;-) > > Signed-off-by: Alexey Dobriyan <[EMAIL PROTECTED]> I'll apply this, thanks. - 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] esp_scsi.c: fix compilation
irqreturn.h for irqreturn_t and dma_addr_t being u128 warnings ;-)
Signed-off-by: Alexey Dobriyan <[EMAIL PROTECTED]>
---
drivers/scsi/esp_scsi.c |9 +
1 file changed, 5 insertions(+), 4 deletions(-)
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1706,17 +1707,17 @@ again:
if (!dma_len) {
printk(KERN_ERR PFX "esp%d: DMA length is zero!\n",
esp->host->unique_id);
- printk(KERN_ERR PFX "esp%d: cur adr[%08x] len[%08x]\n",
+ printk(KERN_ERR PFX "esp%d: cur adr[%08llx]
len[%08x]\n",
esp->host->unique_id,
- esp_cur_dma_addr(ent, cmd),
+ (unsigned long long)esp_cur_dma_addr(ent, cmd),
esp_cur_dma_len(ent, cmd));
esp_schedule_reset(esp);
return 0;
}
- esp_log_datastart("ESP: start data addr[%08x] len[%u] "
+ esp_log_datastart("ESP: start data addr[%08llx] len[%u] "
"write(%d)\n",
- dma_addr, dma_len, write);
+ (unsigned long long)dma_addr, dma_len, write);
esp->ops->send_dma_cmd(esp, dma_addr, dma_len, dma_len,
write, ESP_CMD_DMA | ESP_CMD_TI);
-
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] aacraid: Initialize rx/rkt function pointers before calling them
Salyzyn, Mark wrote: > As an option for a patch (later), what was the actual value of the > Munit.OIMR register (on the x3550 and the x3650 please, just in case)? 0xF. --D - 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
BAD_SG_DMA panic in aha1542
I previously reported an ISA DMA issue for the 2.6.12 kernel. The issue persists through at least 2.6.18. SCSI controller is an Adaptec AHA-1542B (ISA). The action "mount -t iso9660 /dev/scd0 /mnt/cdrom -r" produces (cdrom detection messages as various modules autoload, then...) sgpnt[0:1] page c1ee5af0/0x1ee5af0 length 32 Kernel panic - not syncing: Buffer at physical address > 16 Mb used for aha1542 As before, no problems using the sda hard disk (which is the boot drive): everything works reliably until I touch the cdrom drive. I'll be happy to assist with the debugging, but the system with the aha1542 has no development facilities, i.e., I'll have to build test kernels on a different system, and turnaround is going to be slow :-(. Thanks in advance for helping me get this old machine working again. No issues with 2.4 kernels. I have no idea about 2.5 kernels and 2.6 kernels prior to 2.6.12. As for why I didn't report this before now, the aha1542b was in my parts bin until I cobbled a system together approx. two weeks ago, mostly to see if a useful system could still be had using legacy hardware and modern GNU/Linux software. I'm happy to report the answer is mostly "yes". -- --- Bob Tracy WTO + WIPO = DMCA? http://www.anti-dmca.org [EMAIL PROTECTED] --- - 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: [RFC PATCH]: Rewritten ESP driver, porters needed!
From: Christoph Hellwig <[EMAIL PROTECTED]> Date: Fri, 27 Apr 2007 16:16:58 +0100 > aic7xxx might not be the best driver to look at either :) In practice > a softirq has short enough latency so this doesn't matter, but you > should probably benchmark it on your hardware. 53x700.c which is > the most modern scsi driver and is in about the same hardware class > as the esp gets away without using internal queues, but it also > has a nice internal buffer where commands can just be put into slots > for later execution. This driver runs on machines with 10MHz cpus (sun4c), the delay to the softirq surely matters for those guys :-) > Btw, using the block layer tcq code and the scsi wrappers for it > would be nice aswell. Hmm, I though I was doing so by using the interfaces found in include/scsi/scsi_tcq.h which I am doing. Oh, you're suggesting to use scsi_find_tag() for reconnect handling? Do I have to do anything special for that to work or can I just add calls to it right now? - 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] aacraid: Initialize rx/rkt function pointers before calling them
Maybe we should consider something like: dev->a_ops.adapter_sync_cmd = rx_sync_cmd; dev->a_ops.adapter_enable_int = aac_rx_disable_interrupt; -dev->OIMR = status = rx_readb (dev, MUnit.OIMR); -if status & 0xff) != 0xff) || reset_devices) && +if (reset_devices && !aac_rx_restart_adapter(dev, 0)) ++restart; /* * Check to see if the board panic'd while booting. As an option for a patch (later), what was the actual value of the Munit.OIMR register (on the x3550 and the x3650 please, just in case)? The BIOS should not be enabling any interrupts, I will be checking if they have needed to in some variants(IBM?) for customization or chip(Aurora interface?) reasons... Sincerely -- Mark Salyzyn > -Original Message- > From: Darrick J. Wong [mailto:[EMAIL PROTECTED] > Sent: Friday, April 27, 2007 2:11 PM > To: Salyzyn, Mark > Cc: linux-scsi@vger.kernel.org; Alexis Bruemmer; Vivek Goyal; > Judith Lebzelter > Subject: Re: [PATCH] aacraid: Initialize rx/rkt function > pointers before calling them > > > Salyzyn, Mark wrote: > > > In my unit tests of aacraid_kexec_5.patch, restart was not > called for > > normal operations. If you are just doing a normal boot, > what conditions > > are causing restart to be called in your case? Is it a warm restart? > > Some kind of operation that leaves the Adapter in an > initialized state, > > or a bug in the driver making sure that interrupts are disabled when > > shut down. Inquiring minds want to know! > > This is a normal boot of a "Serveraid 8k-l" on an IBM x3550. One > wrinkle in the configuration is that the system is booted off the > network, though I don't see how that would affect the aacraid's state. > It looks like the MUnit.OIMR test just after the "Failure to > reset here > is an option..." comment is succeeding. The crash seems to happen > regardless of whether we had just done a warm or cold boot. > The option > ROM had run during POST, if that makes any difference. No kexec/kdump > have been configured. For that matter, neither kexec nor kdump have > ever been run in the lifetime of the machine. > > Also observed on an IBM x3650. > > --D > - 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] aacraid: Initialize rx/rkt function pointers before calling them
Salyzyn, Mark wrote: > In my unit tests of aacraid_kexec_5.patch, restart was not called for > normal operations. If you are just doing a normal boot, what conditions > are causing restart to be called in your case? Is it a warm restart? > Some kind of operation that leaves the Adapter in an initialized state, > or a bug in the driver making sure that interrupts are disabled when > shut down. Inquiring minds want to know! This is a normal boot of a "Serveraid 8k-l" on an IBM x3550. One wrinkle in the configuration is that the system is booted off the network, though I don't see how that would affect the aacraid's state. It looks like the MUnit.OIMR test just after the "Failure to reset here is an option..." comment is succeeding. The crash seems to happen regardless of whether we had just done a warm or cold boot. The option ROM had run during POST, if that makes any difference. No kexec/kdump have been configured. For that matter, neither kexec nor kdump have ever been run in the lifetime of the machine. Also observed on an IBM x3650. --D - 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] fc_transport: fix sysfs deadlock on vport delete
When the vport attribute "delete" is used to delete the vport, sysfs deadlocks waiting for the write to complete, which is waiting for the sysfs teardown to complete. Moved this effort to a work_q element. Took the opportunity to make some other cosmetic changes: - removed tabs in Doc file - replaced with expanded spaces - minor copyright text and author text updates - removed a bunch of trailing whitespace -- james s Signed-off-by: James Smart <[EMAIL PROTECTED]> diff -upNr a/Documentation/scsi/scsi_fc_transport.txt b/Documentation/scsi/scsi_fc_transport.txt --- a/Documentation/scsi/scsi_fc_transport.txt 2007-04-20 21:47:20.0 -0400 +++ b/Documentation/scsi/scsi_fc_transport.txt 2007-04-20 22:26:45.0 -0400 @@ -119,67 +119,67 @@ Vport Attributes: The new fc_vport class object has the following attributes - node_name: Read_Only + node_name: Read_Only The WWNN of the vport - port_name: Read_Only + port_name: Read_Only The WWPN of the vport - roles:Read_Only + roles: Read_Only Indicates the FC4 roles enabled on the vport. - symbolic_name:Read_Write + symbolic_name: Read_Write A string, appended to the driver's symbolic port name string, which is registered with the switch to identify the vport. For example, a hypervisor could set this string to "Xen Domain 2 VM 5 Vport 2", and this set of identifiers can be seen on switch management screens to identify the port. - vport_delete: Write_Only + vport_delete: Write_Only When written with a "1", will tear down the vport. - vport_disable:Write_Only + vport_disable: Write_Only When written with a "1", will transition the vport to a disabled. state. The vport will still be instantiated with the Linux kernel, but it will not be active on the FC link. When written with a "0", will enable the vport. - vport_last_state: Read_Only + vport_last_state: Read_Only Indicates the previous state of the vport. See the section below on "Vport States". - vport_state: Read_Only + vport_state: Read_Only Indicates the state of the vport. See the section below on "Vport States". - vport_type: Read_Only + vport_type:Read_Only Reflects the FC mechanism used to create the virtual port. Only NPIV is supported currently. For the fc_host class object, the following attributes are added for vports: - max_npiv_vports: Read_Only + max_npiv_vports: Read_Only Indicates the maximum number of NPIV-based vports that the driver/adapter can support on the fc_host. - npiv_vports_inuse: Read_Only + npiv_vports_inuse: Read_Only Indicates how many NPIV-based vports have been instantiated on the fc_host. - vport_create: Write_Only + vport_create: Write_Only A "simple" create interface to instantiate a vport on an fc_host. A ":" string is written to the attribute. The transport then instantiates the vport object and calls the LLDD to create the vport with the role of FCP_Initiator. Each WWN is specified as 16 hex characters and may *not* contain any prefixes (e.g. 0x, x, etc). - vport_delete: Write_Only + vport_delete: Write_Only A "simple" delete interface to teardown a vport. A ":" - string is written to the attribute. The transport will locate the - vport on the fc_host with the same WWNs and tear it down. Each WWN - is specified as 16 hex characters and may *not* contain any prefixes - (e.g. 0x, x, etc). +string is written to the attribute. The transport will locate the +vport on the fc_host with
Re: [RFC PATCH]: Rewritten ESP driver, porters needed!
Sorry for the late reply. I'll stay in this thead despite the new version beeing posted to not lose the context. On Tue, Apr 24, 2007 at 01:44:40PM -0700, David Miller wrote: > I did all of this, and it's fine, but there is one site which is much > less pleasant, device reconnect. > > With the esp_target_data->lun[] mapping the lookup during device > reconnect was O(1), now I have to use __scsi_device_lookup_by_target() > which is O(num_active_luns). > > In fact the efficiency of that lookup was why I did the data > structures the way I did in the first place. > > But anyways this is cleaner for now and I doubt it matters for > the setups people have with this chip. The import bit is to have the 1:1 kmalloc/kfree pairing in slave_alloc and slave_destroy to have consistant lifetime rules. Using ->hostdata is just ad tad cleaner ontop. And given the number of targets an SPI bus has, aswell as the number of luns seen on them the complexity should not be a problem at all - if this was needed in a hotish path in a fibrechannel drivers things would be different :) > > > > > > + esp_maybe_execute_command(esp); > > > > You still have internal queueing in the driver, and I think this > > is avoidable. Instead you should just try to directly issue > > the command and return SCSI_MLQUEUE_DEVICE_BUSY/SCSI_MLQUEUE_EH_RETRY > > if you can't do it at this point. The midlayer keeps proper per-lun > > and per-host busy counters to call into ->queuecommand once the > > next command returned and the lun/host is not busy anymore. > > Hmmm, OK. But which of those two error codes should I use? SCSI_MLQUEUE_DEVICE_BUSY if you couldn't execute it due to a lun/target-level resource shortage, or SCSI_MLQUEUE_HOST_BUSY if it's a host-level or global resource shortage. The SCSI_MLQUEUE_EH_RETRY above was a typo no one notices and should not be returned at all. > Actually I don't think it's avoidable. I set cmd_per_lun to "2" > and in this way when a command completes I'll always be able to > immediately issue another command for the same device even if > disconnect is disabled. > > Otherwise I have to wait for the softirq to run (after ->scsi_done()) > and then the scsi mid-layer to feed me another command. Meanwhile > the scsi bus will be idle when it could be instead running commands. > > That is the reason behind this. It's not simply for queueing up at > ->queuecommand(). FWIW, I learned this technique from the aic7xxx > driver :-) aic7xxx might not be the best driver to look at either :) In practice a softirq has short enough latency so this doesn't matter, but you should probably benchmark it on your hardware. 53x700.c which is the most modern scsi driver and is in about the same hardware class as the esp gets away without using internal queues, but it also has a nice internal buffer where commands can just be put into slots for later execution. Btw, using the block layer tcq code and the scsi wrappers for it would be nice aswell. > unique_id is only 32-bits, the MMIO address on sparc64 systems often > only differ in the upper 32bits, in fact I do remember one big sparc64 > box where the ESP register addresses only differed like that. > > That's why I implemented it this way, I did look at other drivers > and consider going the MMIO route but I simply can't :-) As I said the value really doesn't matter. We should probably get rid of it and generate an uniqueue enough value in the midlayer instead to keep the legacy API users happy. - 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][REPOST] FC Transport support for vports based on NPIV
The repost does not change anything. It simply updates the patch so it can be applied cleanly after the "make all rports wait dev_loss_tmo" patch which was just posted. -- This patch provides support for FC virtual ports based on NPIV. For information on the interfaces and design, please read the Documentation/scsi/scsi_fc_transport.txt file enclosed within the patch. The RFC was originally posted here: http://marc.info/?l=linux-scsi&m=117226959918393&w=2 Changes from the initial RFC: - Bug fix: needed a transport_class_unregister() for the vport class - Create a symlink to the vport in the shost device if it is not the parent of the vport. - Made symbolic name writable so it can be set after creation - Made the temporary fc_vport_identifiers struct private to the transport. - Deleted the vport_id field from the vport. I couldn't find any good use for it (and symname is a good replacement). - Made the vport_state and vport_last_state "private" attributes. Added the fc_vport_set_state() helper function to manage state transitions - Updated vport_create() to allow a vport to be created in a disabled state. - Added INITIALIZING and FAILED vport states - Added VPCERR_xxx defines for errors to be returned from vport_create() - Created a Documentation/scsi/scsi_fc_transport.txt file that describes the interfaces and expected LLDD behaviors. -- james Signed-off-by: James Smart <[EMAIL PROTECTED]> - 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: [2.6 patch] drivers/scsi/nsp32.c: remove kernel 2.4 code
On Fri, Apr 27, 2007 at 10:55:54AM -0400, Robert P. J. Day wrote: > On Thu, 26 Apr 2007, James Bottomley wrote: > > > Personally, I don't like to see 2.4 and 2.6 in a new driver, and > > will tend to try to force it to be 2.6 only. For an existing > > driver, I tend to be much more tolerant: removing the huge gobs of > > code to achieve 2.6 only is usually a bit disruptive on both the > > driver and the maintainer > > > > > But if a driver is no longer actually maintained for both kernels > > > these checks become useless (and there quickly arised > > > unconditional 2.6-only code in such a driver) and can be removed. > > > > This driver is maintained by > > > > Yokota Hiroshi <[EMAIL PROTECTED]> > > GOTO Masanori <[EMAIL PROTECTED]> > > > > As it says in the header. It was last modified in May 2006, so it > > is maintained under the somewhat elastic standards of SCSI. I've > > cc'd them to see what they think. > > while we're on the subject, what's the policy on supporting kernel > version selection *within* the 2.5 series? as in: > > $ grep -r "KERNEL_VERSION(2,5" * > drivers/scsi/pcmcia/nsp_cs.h:#if (LINUX_VERSION_CODE >= > KERNEL_VERSION(2,5,74)) > drivers/scsi/pcmcia/nsp_cs.c:#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0)) > drivers/scsi/pcmcia/nsp_cs.c:#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,5,2)) > drivers/scsi/pcmcia/nsp_cs.c:#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,5,73)) > ... etc etc ... > > granted, this doesn't happen in a lot of files (almost of them > SCSI-related), but is it official policy to support code based on its > release number in the 2.5 series of releases? unless you have a good > reason, wouldn't it make more sense to compare against (2,6,0) rather > than, say, (2,5,73)? just an observation. Besides the fact that I sent a patch to remove the compat code from this driver, it simply doesn't matter whether to compare with (2,5,73) or (2,6,0), so there's no reason for changing it. > rday cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - 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: [2.6 patch] drivers/scsi/nsp32.c: remove kernel 2.4 code
On Thu, 26 Apr 2007, James Bottomley wrote: > Personally, I don't like to see 2.4 and 2.6 in a new driver, and > will tend to try to force it to be 2.6 only. For an existing > driver, I tend to be much more tolerant: removing the huge gobs of > code to achieve 2.6 only is usually a bit disruptive on both the > driver and the maintainer > > > But if a driver is no longer actually maintained for both kernels > > these checks become useless (and there quickly arised > > unconditional 2.6-only code in such a driver) and can be removed. > > This driver is maintained by > > Yokota Hiroshi <[EMAIL PROTECTED]> > GOTO Masanori <[EMAIL PROTECTED]> > > As it says in the header. It was last modified in May 2006, so it > is maintained under the somewhat elastic standards of SCSI. I've > cc'd them to see what they think. while we're on the subject, what's the policy on supporting kernel version selection *within* the 2.5 series? as in: $ grep -r "KERNEL_VERSION(2,5" * drivers/scsi/pcmcia/nsp_cs.h:#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,74)) drivers/scsi/pcmcia/nsp_cs.c:#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0)) drivers/scsi/pcmcia/nsp_cs.c:#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,5,2)) drivers/scsi/pcmcia/nsp_cs.c:#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,5,73)) ... etc etc ... granted, this doesn't happen in a lot of files (almost of them SCSI-related), but is it official policy to support code based on its release number in the 2.5 series of releases? unless you have a good reason, wouldn't it make more sense to compare against (2,6,0) rather than, say, (2,5,73)? just an observation. rday -- Robert P. J. Day Linux Consulting, Training and Annoying Kernel Pedantry Waterloo, Ontario, CANADA http://fsdev.net/wiki/index.php?title=Main_Page - 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][REPOST] fc_transport: make all rports wait dev_loss_tmo before removing them
Per the comment in the change - it's not always prudent to immediately
remove the rport upon first notice of a disconnect. Make all rports
wait dev_loss_tmo before being deleted (and each could have a separate
dev_loss_tmo value).
The original post was:
http://marc.info/?l=linux-scsi&m=117392196006703&w=2
The repost contains the following changes:
- Bug fix in fc_starget_delete(). Dev_loss_tmo_callbk() was called prior to
tearing down the target. The callback is to be the last thing called, as
it tells the LLDD that the rport is completely finished and can be torn
down. Rework so that terminate_rport_io() is called to terminate the
outstanding io. Isolated work so it's is simply "starget" work.
- Fix holes in original patch. There were code paths that did not expect
the dev_loss_tmo timer to be running for the non-fcp rports.
- Bug Fix: the transport wasn't protecting against a LLDD calling
fc_remote_port_delete() back-to-back. Thus, the dev_loss_tmo timer
could be restarted such that it fires after the rport had been deleted.
Validate rport state before starting the timer.
-- james s
Signed-off-by: James Smart <[EMAIL PROTECTED]>
diff -upNr a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
--- a/drivers/scsi/scsi_transport_fc.c 2007-03-30 21:14:18.0 -0500
+++ b/drivers/scsi/scsi_transport_fc.c 2007-04-19 20:56:21.0 -0400
@@ -1718,31 +1718,12 @@ fc_starget_delete(struct work_struct *wo
struct fc_rport *rport =
container_of(work, struct fc_rport, stgt_delete_work);
struct Scsi_Host *shost = rport_to_shost(rport);
- unsigned long flags;
struct fc_internal *i = to_fc_internal(shost->transportt);
- /*
-* Involve the LLDD if possible. All io on the rport is to
-* be terminated, either as part of the dev_loss_tmo callback
-* processing, or via the terminate_rport_io function.
-*/
- if (i->f->dev_loss_tmo_callbk)
- i->f->dev_loss_tmo_callbk(rport);
- else if (i->f->terminate_rport_io)
+ /* Involve the LLDD if possible to terminate all io on the rport. */
+ if (i->f->terminate_rport_io)
i->f->terminate_rport_io(rport);
- spin_lock_irqsave(shost->host_lock, flags);
- if (rport->flags & FC_RPORT_DEVLOSS_PENDING) {
- spin_unlock_irqrestore(shost->host_lock, flags);
- if (!cancel_delayed_work(&rport->fail_io_work))
- fc_flush_devloss(shost);
- if (!cancel_delayed_work(&rport->dev_loss_work))
- fc_flush_devloss(shost);
- spin_lock_irqsave(shost->host_lock, flags);
- rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
- }
- spin_unlock_irqrestore(shost->host_lock, flags);
-
scsi_remove_target(&rport->dev);
}
@@ -1760,6 +1741,7 @@ fc_rport_final_delete(struct work_struct
struct device *dev = &rport->dev;
struct Scsi_Host *shost = rport_to_shost(rport);
struct fc_internal *i = to_fc_internal(shost->transportt);
+ unsigned long flags;
/*
* if a scan is pending, flush the SCSI Host work_q so that
@@ -1768,13 +1750,37 @@ fc_rport_final_delete(struct work_struct
if (rport->flags & FC_RPORT_SCAN_PENDING)
scsi_flush_work(shost);
+ /* involve the LLDD to terminate all pending i/o */
+ if (i->f->terminate_rport_io)
+ i->f->terminate_rport_io(rport);
+
+ /*
+* Cancel any outstanding timers. These should really exist
+* only when rmmod'ing the LLDD and we're asking for
+* immediate termination of the rports
+*/
+ spin_lock_irqsave(shost->host_lock, flags);
+ if (rport->flags & FC_RPORT_DEVLOSS_PENDING) {
+ spin_unlock_irqrestore(shost->host_lock, flags);
+ if (!cancel_delayed_work(&rport->fail_io_work))
+ fc_flush_devloss(shost);
+ if (!cancel_delayed_work(&rport->dev_loss_work))
+ fc_flush_devloss(shost);
+ spin_lock_irqsave(shost->host_lock, flags);
+ rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
+ }
+ spin_unlock_irqrestore(shost->host_lock, flags);
+
/* Delete SCSI target and sdevs */
if (rport->scsi_target_id != -1)
fc_starget_delete(&rport->stgt_delete_work);
- else if (i->f->dev_loss_tmo_callbk)
+
+ /*
+* Notify the driver that the rport is now dead. The LLDD will
+* also guarantee that any communication to the rport is terminated
+*/
+ if (i->f->dev_loss_tmo_callbk)
i->f->dev_loss_tmo_callbk(rport);
- else if (i->f->terminate_rport_io)
- i->f->terminate_rport_io(rport);
transport_remove_device(dev);
device_del(dev);
@@ -1963,8 +1969,6 @@ fc_remote_port_add(struct Scsi_Host *sho
RE: [PATCH] aacraid: Initialize rx/rkt function pointers before calling them
Just wait a second ... We are not supposed to be calling aac_rx_restart_adapter unless the Adapter has it's interrupts enabled (from a previous driver load in the same warm session such as a kexec) or if the kernel reset_devices flag is set (which is a mandatory kernel flag during kdump or kexec). This restart of the Adapter is really for kexec and kdump issues and should not be triggered elsewhere. In my unit tests of aacraid_kexec_5.patch, restart was not called for normal operations. If you are just doing a normal boot, what conditions are causing restart to be called in your case? Is it a warm restart? Some kind of operation that leaves the Adapter in an initialized state, or a bug in the driver making sure that interrupts are disabled when shut down. Inquiring minds want to know! Sincerely -- Mark Salyzyn > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Salyzyn, Mark > Sent: Friday, April 27, 2007 8:47 AM > To: Darrick J. Wong; linux-scsi@vger.kernel.org > Cc: Alexis Bruemmer > Subject: RE: [PATCH] aacraid: Initialize rx/rkt function > pointers before calling them > > > Reject, this is already covered in aacraid_kexec_5.patch and again > separately in aacraid_kexec_fix.patch. > > Sincerely -- Mark Salyzyn > > > -Original Message- > > From: Darrick J. Wong [mailto:[EMAIL PROTECTED] > > Sent: Thursday, April 26, 2007 6:58 PM > > To: linux-scsi@vger.kernel.org > > Cc: Salyzyn, Mark; Alexis Bruemmer > > Subject: [PATCH] aacraid: Initialize rx/rkt function pointers > > before calling them > > > > > > Commit 8418852d11f0bbaeebeedd4243560d8fdc85410d to scsi-misc > > resulted in > > the substitution of calls to rx_sync_cmd with a function pointer > > abstraction. aac_rx_restart_adapter requires a pointer to > a sync_cmd > > function, which is not set up before its first invocation. > > That causes > > the driver to crash at startup. Move the initializers (we need both > > rx_sync_cmd and enable_int pointers) further up to proceed the > > restart_adapter call. > > > > Signed-off-by: Darrick J. Wong <[EMAIL PROTECTED]> > > --- > > > > drivers/scsi/aacraid/rx.c |4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/scsi/aacraid/rx.c b/drivers/scsi/aacraid/rx.c > > index 0c71315..b7810d6 100644 > > --- a/drivers/scsi/aacraid/rx.c > > +++ b/drivers/scsi/aacraid/rx.c > > @@ -537,6 +537,8 @@ int _aac_rx_init(struct aac_dev *dev) > > printk(KERN_WARNING "%s: unable to map > > adapter.\n", name); > > goto error_iounmap; > > } > > + dev->a_ops.adapter_sync_cmd = rx_sync_cmd; > > + aac_adapter_comm(dev, AAC_COMM_PRODUCER); > > > > /* Failure to reset here is an option ... */ > > dev->OIMR = status = rx_readb (dev, MUnit.OIMR); > > @@ -598,7 +600,6 @@ int _aac_rx_init(struct aac_dev *dev) > > dev->a_ops.adapter_interrupt = aac_rx_interrupt_adapter; > > dev->a_ops.adapter_disable_int = aac_rx_disable_interrupt; > > dev->a_ops.adapter_notify = aac_rx_notify_adapter; > > - dev->a_ops.adapter_sync_cmd = rx_sync_cmd; > > dev->a_ops.adapter_check_health = aac_rx_check_health; > > dev->a_ops.adapter_restart = aac_rx_restart_adapter; > > > > @@ -606,7 +607,6 @@ int _aac_rx_init(struct aac_dev *dev) > > * First clear out all interrupts. Then enable > > the one's that we > > * can handle. > > */ > > - aac_adapter_comm(dev, AAC_COMM_PRODUCER); > > aac_adapter_disable_int(dev); > > rx_writel(dev, MUnit.ODR, 0x); > > aac_adapter_enable_int(dev); > > > - > 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] aacraid: Initialize rx/rkt function pointers before calling them
Reject, this is already covered in aacraid_kexec_5.patch and again separately in aacraid_kexec_fix.patch. Sincerely -- Mark Salyzyn > -Original Message- > From: Darrick J. Wong [mailto:[EMAIL PROTECTED] > Sent: Thursday, April 26, 2007 6:58 PM > To: linux-scsi@vger.kernel.org > Cc: Salyzyn, Mark; Alexis Bruemmer > Subject: [PATCH] aacraid: Initialize rx/rkt function pointers > before calling them > > > Commit 8418852d11f0bbaeebeedd4243560d8fdc85410d to scsi-misc > resulted in > the substitution of calls to rx_sync_cmd with a function pointer > abstraction. aac_rx_restart_adapter requires a pointer to a sync_cmd > function, which is not set up before its first invocation. > That causes > the driver to crash at startup. Move the initializers (we need both > rx_sync_cmd and enable_int pointers) further up to proceed the > restart_adapter call. > > Signed-off-by: Darrick J. Wong <[EMAIL PROTECTED]> > --- > > drivers/scsi/aacraid/rx.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/aacraid/rx.c b/drivers/scsi/aacraid/rx.c > index 0c71315..b7810d6 100644 > --- a/drivers/scsi/aacraid/rx.c > +++ b/drivers/scsi/aacraid/rx.c > @@ -537,6 +537,8 @@ int _aac_rx_init(struct aac_dev *dev) > printk(KERN_WARNING "%s: unable to map > adapter.\n", name); > goto error_iounmap; > } > + dev->a_ops.adapter_sync_cmd = rx_sync_cmd; > + aac_adapter_comm(dev, AAC_COMM_PRODUCER); > > /* Failure to reset here is an option ... */ > dev->OIMR = status = rx_readb (dev, MUnit.OIMR); > @@ -598,7 +600,6 @@ int _aac_rx_init(struct aac_dev *dev) > dev->a_ops.adapter_interrupt = aac_rx_interrupt_adapter; > dev->a_ops.adapter_disable_int = aac_rx_disable_interrupt; > dev->a_ops.adapter_notify = aac_rx_notify_adapter; > - dev->a_ops.adapter_sync_cmd = rx_sync_cmd; > dev->a_ops.adapter_check_health = aac_rx_check_health; > dev->a_ops.adapter_restart = aac_rx_restart_adapter; > > @@ -606,7 +607,6 @@ int _aac_rx_init(struct aac_dev *dev) >* First clear out all interrupts. Then enable > the one's that we >* can handle. >*/ > - aac_adapter_comm(dev, AAC_COMM_PRODUCER); > aac_adapter_disable_int(dev); > rx_writel(dev, MUnit.ODR, 0x); > aac_adapter_enable_int(dev); > - 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]: Final ESP driver rewrite
On Apr 27 2007 01:24, David Miller wrote: >> On Apr 27 2007 00:28, David Miller wrote: >> >> >This is the patch I intend to send to Linus via my >> >sparc-2.6 tree. >> > >> >Thanks to everyone for the review and testing! >> > >> >commit cd9ad58d4061494e7fdd70ded7bcf2418daf356a >> >Author: David S. Miller <[EMAIL PROTECTED]> >> >Date: Thu Apr 26 21:19:23 2007 -0700 >> >> Would not it be better to delete the file (à la `svn rm` first), then readd >> it? > >What I did in GIT was remove drivers/scsi/esp.[ch] then add >drivers/scsi/esp_scsi.[ch] and drivers/scsi/sun_esp.c > >And that is reflected exactly in the patch, so what's the >suggestion? Well the patch looks like it's done in one commit (rather than two - delete+add), but that's probably just because it's the diff over both commits. All fine. Jan -- - 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]: Final ESP driver rewrite
From: Jan Engelhardt <[EMAIL PROTECTED]> Date: Fri, 27 Apr 2007 10:33:30 +0200 (MEST) > Well the patch looks like it's done in one commit (rather than two - > delete+add), but that's probably just because it's the diff over both commits. I didn't do it in multiple commits, I did it in one. If I do it in two commits, the driver doesn't exist at a point in the history and this screws things up for people trying to bisect who have that hardware. - 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]: Final ESP driver rewrite
From: Jan Engelhardt <[EMAIL PROTECTED]> Date: Fri, 27 Apr 2007 09:57:34 +0200 (MEST) > > On Apr 27 2007 00:28, David Miller wrote: > > >This is the patch I intend to send to Linus via my > >sparc-2.6 tree. > > > >Thanks to everyone for the review and testing! > > > >commit cd9ad58d4061494e7fdd70ded7bcf2418daf356a > >Author: David S. Miller <[EMAIL PROTECTED]> > >Date: Thu Apr 26 21:19:23 2007 -0700 > > Would not it be better to delete the file (à la `svn rm` first), then readd > it? What I did in GIT was remove drivers/scsi/esp.[ch] then add drivers/scsi/esp_scsi.[ch] and drivers/scsi/sun_esp.c And that is reflected exactly in the patch, so what's the suggestion? - 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]: Final ESP driver rewrite
On Apr 27 2007 00:28, David Miller wrote: >This is the patch I intend to send to Linus via my >sparc-2.6 tree. > >Thanks to everyone for the review and testing! > >commit cd9ad58d4061494e7fdd70ded7bcf2418daf356a >Author: David S. Miller <[EMAIL PROTECTED]> >Date: Thu Apr 26 21:19:23 2007 -0700 Would not it be better to delete the file (à la `svn rm` first), then readd it? Jan -- - 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
