Re: [PATCH scsi-misc-2.6 08/08] scsi: fix hot unplug sequence
On Fri, 25 Mar 2005, James Bottomley wrote: > On Fri, 2005-03-25 at 14:38 +0900, Tejun Heo wrote: > > We have users of scsi_do_req() other than scsi_wait_req() and they > > use different done() functions to do different things. I've checked > > other done functions and none uses contents inside the passed > > scsi_cmnd, so using a dummy command should be okay with them. Am I > > missing something here? > > Well ... the other users are supposed to be going away. They're > actually all coded wrongly in some way or other ... perhaps I should > speed up the process. > I have seen you mention this several times now and I am getting more and more worried. The reason is that scsi_wait_req() is a synchronous interface and it does not allow a driver to do this: - send a request - do other useful things/let the user do useful work - wait for completion before starting another request I fully agree that doing done() correctly _is_ a problem, especially when the SCSI subsystem evolves and the high-level driver writers do not follow the development closely enough. One solution to these problems would be to let the drivers still use scsi_do_req() and their own done() function, but create two (three) helpers: - one to be called at the beginning of done(); it would do what needs to be done here but lets the driver to do some special things of its own if necessary - one to be called to wait for the request to finish (- one to do scsi_ro_req() and the things necessary before these) Having these helpers would isolate the user of the SCSI subsystem from the internals. scsi_wait_req() should call these functions and no additional maintenance would be needed for this additional asynchronous interface. The current drivers may not do any work in done() that could not be done later but there is one patch pending where this happens: the st performance statistics patch needs to get the time stamp when the SCSI command is processed. -- Kai - 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] rm unused scan delay var
Is FC_SCSI_SCAN_DELAY used by a FC driver that is
not yet in mainline? This patch just deletes it
if not since no one else is.
Signed-off-by: Mike Chrisite <[EMAIL PROTECTED]>
--- scsi-misc-2.6.orig/include/scsi/scsi_transport_fc.h 2005-03-25 21:35:06.0 -0800
+++ scsi-misc-2.6.test/include/scsi/scsi_transport_fc.h 2005-03-25 22:01:49.0 -0800
@@ -212,9 +212,6 @@ struct fc_rport { /* aka fc_starget_attr
#define rport_to_shost(r) \
dev_to_shost(r->dev.parent)
-#define FC_SCSI_SCAN_DELAY (1 * HZ) /* 1 second delay */
-
-
/*
* FC SCSI Target Attributes
*
[PATCH] fix fc class work queue usage
According to this article http://lwn.net/Articles/125930/, "When
cancel_delayed_work() returns zero, it means that the delayed
work request was fired off before the call; it might, in fact,
be running on another CPU when the cancel attempt is made".
If it is successful, it returns a nonzero value. Tracing
through cancel_delayed_work's timer usage would seem to confirm
this. The fc class today though performs a flush_scheduled_work,
when the return value is nonzero instead of zero.
Also it appears the fc class will use flush_scheduled_work to
flush the work from the shost_work_q when it should be using
flush_workqueue(shost->work_q) (flush_scheduled_work() only
flushes the default, keventd_wq, work queue).
The attached patch adds a scsi_flush_work function for
scsi_transport_fc to use and it fixes the cancel_delayed_work()
test to detect when to flush the work queues correctly (it
also only calls cancel_delayed_work when the work is queued
as delayed (scan_work is not delayed). The patch has only been
compile tested since I am away from any FC HW for a while.
Signed-off-by: Mike Chrisite <[EMAIL PROTECTED]>
diff -aurp scsi-misc-2.6.orig/drivers/scsi/hosts.c scsi-misc-2.6.test/drivers/scsi/hosts.c
--- scsi-misc-2.6.orig/drivers/scsi/hosts.c 2005-03-25 21:34:30.0 -0800
+++ scsi-misc-2.6.test/drivers/scsi/hosts.c 2005-03-25 21:40:32.0 -0800
@@ -443,3 +443,20 @@ int scsi_queue_work(struct Scsi_Host *sh
}
EXPORT_SYMBOL_GPL(scsi_queue_work);
+/**
+ * scsi_flush_work - Flush a Scsi_Host's workqueue.
+ * @shost: Pointer to Scsi_Host.
+ **/
+void scsi_flush_work(struct Scsi_Host *shost)
+{
+ if (!shost->work_q) {
+ printk(KERN_ERR
+ "ERROR: Scsi host '%s' attempted to flush scsi-work, "
+ "when no workqueue created.\n", shost->hostt->name);
+ dump_stack();
+ return;
+ }
+
+ flush_workqueue(shost->work_q);
+}
+EXPORT_SYMBOL_GPL(scsi_flush_work);
diff -aurp scsi-misc-2.6.orig/drivers/scsi/scsi_transport_fc.c scsi-misc-2.6.test/drivers/scsi/scsi_transport_fc.c
--- scsi-misc-2.6.orig/drivers/scsi/scsi_transport_fc.c 2005-03-25 21:34:36.0 -0800
+++ scsi-misc-2.6.test/drivers/scsi/scsi_transport_fc.c 2005-03-25 21:53:36.0 -0800
@@ -1375,12 +1375,14 @@ EXPORT_SYMBOL(fc_remote_port_add);
static void
fc_rport_tgt_remove(struct fc_rport *rport)
{
+ struct Scsi_Host *shost = rport_to_shost(rport);
+
scsi_target_unblock(&rport->dev);
/* Stop anything on the workq */
- if (cancel_delayed_work(&rport->dev_loss_work) ||
- cancel_delayed_work(&rport->scan_work))
+ if (!cancel_delayed_work(&rport->dev_loss_work))
flush_scheduled_work();
+ scsi_flush_work(shost);
scsi_remove_target(&rport->dev);
}
@@ -1625,7 +1627,7 @@ fc_remote_port_unblock(struct fc_rport *
* failure as the state machine state change will validate the
* transaction.
*/
- if (cancel_delayed_work(work))
+ if (!cancel_delayed_work(work))
flush_scheduled_work();
if (rport->port_state == FC_PORTSTATE_OFFLINE)
diff -aurp scsi-misc-2.6.orig/include/scsi/scsi_host.h scsi-misc-2.6.test/include/scsi/scsi_host.h
--- scsi-misc-2.6.orig/include/scsi/scsi_host.h 2005-03-25 21:34:47.0 -0800
+++ scsi-misc-2.6.test/include/scsi/scsi_host.h 2005-03-25 21:25:06.0 -0800
@@ -590,6 +590,7 @@ static inline struct Scsi_Host *dev_to_s
}
extern int scsi_queue_work(struct Scsi_Host *, struct work_struct *);
+extern void scsi_flush_work(struct Scsi_Host *);
extern struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *, int);
extern int __must_check scsi_add_host(struct Scsi_Host *, struct device *);
Re: [PATCH 6/7] - MPT FUSION - SPLITTING SCSI HOST DRIVERS
On Thu, 2005-03-24 at 16:57 -0700, Moore, Eric Dean wrote:
> +static struct device_attribute mptscsih_queue_depth_attr = {
> + .attr = {
> + .name = "queue_depth",
> + .mode = S_IWUSR,
> + },
> + .store = mpt_core_store_queue_depth,
> +};
But in the original which you're removing, this was implemented via the
change_queue_depth API.
It looks like the patches you're posting are actually an older version
of the fusion driver. Do you have the split done on a current copy?
Thanks,
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 1/7] - MPT FUSION - SPLITTING SCSI HOST DRIVERS
On Thu, 2005-03-24 at 16:56 -0700, Moore, Eric Dean wrote: > + > config FUSION_FC > - tristate "Fusion MPT (base + ScsiHost) drivers for FC" > - depends on PCI && SCSI > + tristate "Fusion MPT (ScsiHost) drivers for FC" This rejects completely in Kconfig. Could you check your base for the diffs ... there's no FUSION_FC parameter in the vanilla kernel. Thanks, 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: The latest megaraid_mbox 2.20.4.5 is hung
Hi Seokmann, When failing in megaraid_queue_command(), is not scsi_cmnd(io request) lost? I think that I fix this issue. Please check the code. (See attached file: linux-2.6.12-rc1-megaraid-queuecommand-error-midlayer-retry-fix.patch) The patching order is: - linux-2.6.12-rc1-megaraid-isr-quiescent-race-fix.patch - linux-2.6.12-rc1-megaraid-outstanding_cmds-atomic.patch - linux-2.6.12-rc1-megaraid-queuecommand-error-midlayer-retry-fix.patch BTW, Are the max number of commands that FW is treatable at the same time 128? For the megaraid2 driver, it was 127. Which is correct? I continue the driver's debugging further. Thanks -- Haruo linux-2.6.12-rc1-megaraid-queuecommand-error-midlayer-retry-fix.patch Description: linux-2.6.12-rc1-megaraid-queuecommand-error-midlayer-retry-fix.patch
Re: [PATCH scsi-misc-2.6 08/08] scsi: fix hot unplug sequence
On Sat, 2005-03-26 at 06:43 +0900, Tejun Heo wrote: > 1. Allocate scsi_request and request (two are linked) This can't be done because the scsi_cmnd's are allocated specially (slab with reserve pool). 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 scsi-misc-2.6 08/08] scsi: fix hot unplug sequence
Hello, James. James Bottomley wrote: > On Fri, 2005-03-25 at 14:38 +0900, Tejun Heo wrote: > >> We have users of scsi_do_req() other than scsi_wait_req() and they >>use different done() functions to do different things. I've checked >>other done functions and none uses contents inside the passed >>scsi_cmnd, so using a dummy command should be okay with them. Am I >>missing something here? > > > Well ... the other users are supposed to be going away. They're > actually all coded wrongly in some way or other ... perhaps I should > speed up the process. Sounds great. :-) >> Oh, and I would really appreciate if you can fill me in / give a >>pointer about the scsi_request/scsi_cmnd distinction. > > The block layer speaks in terms of requests and the scsi layers in terms > of commands. The scsi_request_fn() actually associates a request with a > command. However, since SCSI uses the block layer for queueing, all the > internal scsi command submit paths have to use requests. This is what a > scsi_request is. The reason for the special casing is that we can't use > the normal REQ_CMD or REQ_BLOCK_PC paths because they need ULD > initialisation and back end processing. What I meant was we could just use scsi_cmnd instead of scsi_request for commands. Currently, we do the following for special commands. 1. Allocate scsi_request and request (two are linked) 2. Initialize scsi_request as needed 3. queue the request 4. the request is dispatched 5. scsi_cmnd is initialized from scsi_request 6. scsi_cmnd is executed 7. result code and sense copied back to scsi_request 8. request is completed Instead, we can 1. Allocate scsi_cmnd and request (two are linked) 2. Initialize scsi_cmnd as needed 3. queue the request 4. the request is dispatched 5. scsi_cmnd is executed 6. request is completed As the latter seemed more straight-forward to me, I was wondering if there were reasons that I wasn't aware of. Thanks. -- tejun - 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: megaraid driver (proposed patch)
Hello,
After a first attempt of discussion on lkml, I'd like to get your
feedback on the following points.
I've noticed that since recent kernel versions, it's not possible
anymore to use simultaneously new and old megaraid driver.
It seems to have been introduced by that changeset:
http://linux.bkbits.net:8080/linux-2.5/diffs/drivers/scsi/megaraid/Kconfig.megar
[EMAIL PROTECTED]|src/.|src/drivers|src/drivers/scsi|src/drivers/scsi/mega
raid|hist/drivers/scsi/megaraid/Kconfig.megaraid
It particularly makes life of people developing kernel for distro
difficult as it forces them to drop support for legacy hardware which is
working just fine with 2.6, or to patch their own kernel build.
As well it prevents simultaneous usage of new and old cards in the same
system.
As notes by James Bottomley and Christoph Hellwig the best thing to do
is probably to remove support for cards in the old driver that are still
supported by the new driver.
I'd like to propose a patch but looking at the code I have the following
question: the card id 101E:1960 seems to be supported by the new driver
as per the header of megaraid_mbox.c but in reality I was unable to boot
my system which uses that precise card with the new driver.
Are they mistakes in the header file ? Should they be removed from the
new driver (if they don't work) or from the old if they work ?
My current status is the following (just for discussion)
diff -Nru drivers/scsi/megaraid/Kconfig.megaraid
drivers.new/scsi/megaraid/Kconfig.megaraid
--- drivers/scsi/megaraid/Kconfig.megaraid Fri Mar 25 20:34:06 2005
+++ drivers.new/scsi/megaraid/Kconfig.megaraid Fri Mar 25 20:53:37 2005
@@ -64,15 +64,12 @@
To compile this driver as a module, choose M here: the
module will be called megaraid_mbox
-if MEGARAID_NEWGEN=n
config MEGARAID_LEGACY
tristate "LSI Logic Legacy MegaRAID Driver"
depends on PCI && SCSI
help
This driver supports the LSI MegaRAID 418, 428, 438, 466, 762,
490
- and 467 SCSI host adapters. This driver also support the all
U320
- RAID controllers
+ and 467 SCSI host adapters.
To compile this driver as a module, choose M here: the
module will be called megaraid
-endif
diff -Nru drivers/scsi/megaraid.c drivers.new/scsi/megaraid.c
--- drivers/scsi/megaraid.c Fri Mar 25 20:28:29 2005
+++ drivers.new/scsi/megaraid.c Fri Mar 25 20:57:25 2005
@@ -5033,12 +5033,6 @@
}
static struct pci_device_id megaraid_pci_tbl[] = {
- {PCI_VENDOR_ID_DELL, PCI_DEVICE_ID_DISCOVERY,
- PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- {PCI_VENDOR_ID_DELL, PCI_DEVICE_ID_PERC4_DI,
- PCI_ANY_ID, PCI_ANY_ID, 0, 0, BOARD_64BIT},
- {PCI_VENDOR_ID_LSI_LOGIC, PCI_DEVICE_ID_PERC4_QC_VERDE,
- PCI_ANY_ID, PCI_ANY_ID, 0, 0, BOARD_64BIT},
{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID2,
diff -Nru drivers/scsi/megaraid.h drivers.new/scsi/megaraid.h
--- drivers/scsi/megaraid.h Fri Mar 25 20:28:25 2005
+++ drivers.new/scsi/megaraid.h Fri Mar 25 20:57:03 2005
@@ -73,10 +73,6 @@
#define PCI_DEVICE_ID_AMI_MEGARAID30x1960
#endif
-#define PCI_DEVICE_ID_DISCOVERY0x000E
-#define PCI_DEVICE_ID_PERC4_DI 0x000F
-#define PCI_DEVICE_ID_PERC4_QC_VERDE 0x0407
-
/* Sub-System Vendor IDs */
#defineAMI_SUBSYS_VID 0x101E
#define DELL_SUBSYS_VID0x1028
--
Linux Solution Consultant / Open Source Evangelist \HP C&I EMEA ISG
HP/Intel Solution Center http://hpintelco.net Hewlett-Packard Grenoble/France
Des infos sur Linux? http://www.HyPer-Linux.org http://www.hp.com/linux
La musique ancienne? http://www.musique-ancienne.org http://www.medieval.org
-
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 scsi-misc-2.6 08/08] scsi: fix hot unplug sequence
On Fri, 2005-03-25 at 14:38 +0900, Tejun Heo wrote: > We have users of scsi_do_req() other than scsi_wait_req() and they > use different done() functions to do different things. I've checked > other done functions and none uses contents inside the passed > scsi_cmnd, so using a dummy command should be okay with them. Am I > missing something here? Well ... the other users are supposed to be going away. They're actually all coded wrongly in some way or other ... perhaps I should speed up the process. > Oh, and I would really appreciate if you can fill me in / give a > pointer about the scsi_request/scsi_cmnd distinction. The block layer speaks in terms of requests and the scsi layers in terms of commands. The scsi_request_fn() actually associates a request with a command. However, since SCSI uses the block layer for queueing, all the internal scsi command submit paths have to use requests. This is what a scsi_request is. The reason for the special casing is that we can't use the normal REQ_CMD or REQ_BLOCK_PC paths because they need ULD initialisation and back end processing. 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
mdadm output -- please explain
as far as i know, there are only 6 devices in this array, yet i see 'Total Devices : 7' i also see a failed devices, yet at the bottom, none of the devices are marked as faulty. here's the pertinent line from /proc/mdstat: md0 : active raid5 sdp[5] sdo[4] sdn[3] sdm[2] sdj[1] sdi[0] 716872960 blocks level 5, 128k chunk, algorithm 2 [6/6] [UU] i'm not having any problems with the array, this just wasn't what i expected to see. could someone please explain? -rui #> mdadm --detail /dev/md0 /dev/md0: Version : 00.90.00 Creation Time : Mon Mar 21 16:59:41 2005 Raid Level : raid5 Array Size : 716872960 (683.66 GiB 734.08 GB) Device Size : 143374592 (136.73 GiB 146.82 GB) Raid Devices : 6 Total Devices : 7 Preferred Minor : 0 Persistence : Superblock is persistent Update Time : Tue Mar 22 07:55:58 2005 State : dirty, no-errors Active Devices : 6 Working Devices : 6 Failed Devices : 1 Spare Devices : 0 Layout : left-symmetric Chunk Size : 128K Number Major Minor RaidDevice State 0 8 1280 active sync /dev/sdi 1 8 1441 active sync /dev/sdj 2 8 1922 active sync /dev/sdm 3 8 2083 active sync /dev/sdn 4 8 2244 active sync /dev/sdo 5 8 2405 active sync /dev/sdp UUID : 0dd747ec:22a09ffb:debe6f31:78f4c769 Events : 0.4 - 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] fix fusion breakage with multiple PCI domains
Any comments on the patch below? Eric, sorry I didn't sent this
to you before -- maybe you'd consider the MAINTAINERS patch below
as well?
I also suggested pci_get_slot() as a possibility for rewriting
the function below (mpt_detect_bound_ports()). But that's a
bit more than I want to do at the moment. We need a fix for
RHEL4 and SLES9, and as a non-expert in mpt fusion, I'm more
comfortable advocating the simple change below.
On Thu, 2005-03-17 at 13:00 -0700, Bjorn Helgaas wrote:
> mpt_detect_bound_ports(): Don't assume that two devices with the same
> dev->bus->number are on the same bus. With multiple PCI domains,
> many buses may have the same number.
>
> Signed-off-by: Bjorn Helgaas <[EMAIL PROTECTED]>
>
> = drivers/message/fusion/mptbase.c 1.40 vs edited =
> --- 1.40/drivers/message/fusion/mptbase.c 2005-03-13 16:30:09 -07:00
> +++ edited/drivers/message/fusion/mptbase.c 2005-03-17 12:46:57 -07:00
> @@ -1834,14 +1834,14 @@
>
> match_lo = pdev->devfn-1;
> match_hi = pdev->devfn+1;
> - dprintk((MYIOC_s_INFO_FMT "PCI bus/devfn=%x/%x, searching for devfn
> match on %x or %x\n",
> - ioc->name, pdev->bus->number, pdev->devfn, match_lo,
> match_hi));
> + dprintk((MYIOC_s_INFO_FMT "PCI device %s devfn=%x/%x, searching for
> devfn match on %x or %x\n",
> + ioc->name, pci_name(pdev), pdev->devfn, match_lo,
> match_hi));
>
> list_for_each_entry(ioc_srch, &ioc_list, list) {
> struct pci_dev *_pcidev = ioc_srch->pcidev;
>
> if ((_pcidev->device == pdev->device) &&
> - (_pcidev->bus->number == pdev->bus->number) &&
> + (_pcidev->bus == pdev->bus) &&
> (_pcidev->devfn == match_lo || _pcidev->devfn == match_hi)
> ) {
> /* Paranoia checks */
> if (ioc->alt_ioc != NULL) {
>
= MAINTAINERS 1.291 vs edited =
--- 1.291/MAINTAINERS 2005-03-13 16:29:59 -07:00
+++ edited/MAINTAINERS 2005-03-25 09:05:50 -07:00
@@ -1449,6 +1449,12 @@
L: linux-scsi@vger.kernel.org
S: Maintained
+LSILOGIC FC9XX/53C1030/53C1035 FUSION MPT DRIVER
+P: Eric Moore
+M: [EMAIL PROTECTED]
+L: linux-scsi@vger.kernel.org
+S: Maintained
+
M68K ARCHITECTURE
P: Geert Uytterhoeven
M: [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: [PATCH as494] Add a scsi_device flag for RETRY_HWERROR
On Fri, 25 Mar 2005, Douglas Gilbert wrote: > Alan Stern wrote: > > James: > > > > It turns out that a bunch of USB-IDE converters make the mistake of > > returning SK = 04 (Hardware Error) whenever the IDE device signals any > > sort of error, without bothering to distinguish recoverable from > > non-recoverable errors. > > Alan, > The sense key of HARDWARE ERROR is a superset of MEDIUM ERROR. > SBC-2 treats them as synonymous. If it is a "real" medium > error then the LBA of the first (i.e. lowest address) bad > block should be placed in the "info" field and the "valid" > bit should be set. If this is done the block layer does > its job well. I don't have a copy of SBC-2. The (very old) document I do have and the code in scsi_error.c both treat HARDWARE_ERROR as nonrecoverable and MEDIUM_ERROR as recoverable. So in a sense the patch does go some way towards treating one as a superset of the other. Were the errors in question "real" medium errors? I don't know -- maybe not. There wasn't much information available. > Also for both hardware and medium errors the "sense key > specific" field (if SKSV=1) reports the actual retry > count. The read/write retry count is set in the "read > write error recovery" mode page. Anyways if the disk > has already retried reading the bad block 64 times (say) > what is the point of the mid level retrying?? For the devices I've had to deal with, it's not obvious that the disk has already retried 64 times. (And the error-recovery mode page may not exist either; remember these are slightly buggy USB adaptors trying to pretend that an IDE drive is a SCSI device.) All I know for certain is that the first time the command was issued the we got back HARDWARE_ERROR and then on a retry the command succeeded. (The IBM drive that started this whole thing is a different story. But regardless, it's clear that having these patches helps with some devices and doesn't hurt others.) Alan Stern - 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
Kill superfluos newline in dmesg output of aic7xxx
Hi, my dmesg looks like this: scsi0 : Adaptec AIC7XXX EISA/VLB/PCI SCSI HBA DRIVER, Rev 6.2.36 aic7860: Single Channel A, SCSI Id=7, 3/253 SCBs (scsi0:A:0): 10.000MB/s transfers (10.000MHz, offset 15) Vendor: SEAGATE Model: ST318436LWRev: 0010 To avoid the empty line I think this patch is needed. Eike Signed-off-by: Rolf Eike Beer <[EMAIL PROTECTED]> diff -aupr linux-2.6.11/drivers/scsi/aic7xxx/aic7xxx_osm.c linux-2.6.12-rc1/drivers/scsi/aic7xxx/aic7xxx_osm.c --- linux-2.6.11/drivers/scsi/aic7xxx/aic7xxx_osm.c 2005-03-02 08:37:48.0 +0100 +++ linux-2.6.12-rc1/drivers/scsi/aic7xxx/aic7xxx_osm.c 2005-03-25 09:57:01.0 +0100 @@ -932,7 +932,6 @@ ahc_linux_info(struct Scsi_Host *host) strcat(bp, ""); ahc_controller_info(ahc, ahc_info); strcat(bp, ahc_info); - strcat(bp, "\n"); return (bp); } - 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] Reduce stack usage in sg.c
> Mostly looks OK to me. However, I would rather see patches
> against the current -bk instead of 2.6.x.y.
Point noted. Dug around for info on BK. Cloned
http://linux.bkbits.net/linux-2.6 and updated with pull from
bk://linux-scsi.bkbits.net/scsi-misc-2.6. Generated bk patch with -hdu
option. Hope this new little BK grasshoper got it right :)
> This patch section looks suspect to me:
> Notice that the second '+' sign begins an illegal construct,
> probably missing an 'if' line or 2.
> Or the '+' should be '-'.
>
> > while (1) {
> > - res = 0;/* following is a macro that beats
> > race condition */
> > + res = 0; /* following macro beats race condition */
> > __wait_event_interruptible(sfp->read_wait,
> > - (sdp->detached || (srp = sg_get_rq_mark(sfp,
> > req_pack_id))),
> > -res);
> > - if (sdp->detached)
> > - return -ENODEV;
> > + (sdp->detached ||
> > + (srp = sg_get_rq_mark(sfp, req_pack_id))),
> > + res);
> > + if (sdp->detached) {
I adjusted long lines in and around my changes to fit in 80 column
editor. The second "+" is not a statement but the second argument of
the __wait_event_interruptible macro. So this is verified good.
Reworked patch follows.
Regards,
Rayan
Summary: Reduce stack usage in sg_read() and sg_ioctl()
Signed-off-by: Yum Rayan <[EMAIL PROTECTED]>
diff -Nru a/drivers/scsi/sg.c b/drivers/scsi/sg.c
--- a/drivers/scsi/sg.c 2005-03-24 00:36:05 -08:00
+++ b/drivers/scsi/sg.c 2005-03-24 00:36:05 -08:00
@@ -330,14 +330,13 @@
static ssize_t
sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
{
- int res;
Sg_device *sdp;
Sg_fd *sfp;
Sg_request *srp;
int req_pack_id = -1;
- struct sg_header old_hdr;
- sg_io_hdr_t new_hdr;
+ struct sg_header *old_hdr;
sg_io_hdr_t *hp;
+ int retval = 0;
if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
return -ENXIO;
@@ -345,99 +344,131 @@
sdp->disk->disk_name, (int) count));
if (!access_ok(VERIFY_WRITE, buf, count))
return -EFAULT;
+ old_hdr = kmalloc(SZ_SG_HEADER, GFP_KERNEL);
+ if (!old_hdr)
+ return -ENOMEM;
if (sfp->force_packid && (count >= SZ_SG_HEADER)) {
- if (__copy_from_user(&old_hdr, buf, SZ_SG_HEADER))
- return -EFAULT;
- if (old_hdr.reply_len < 0) {
+ if (__copy_from_user(old_hdr, buf, SZ_SG_HEADER)) {
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
+ if (old_hdr->reply_len < 0) {
if (count >= SZ_SG_IO_HDR) {
- if (__copy_from_user
- (&new_hdr, buf, SZ_SG_IO_HDR))
- return -EFAULT;
- req_pack_id = new_hdr.pack_id;
+ sg_io_hdr_t *new_hdr;
+ new_hdr = kmalloc(SZ_SG_IO_HDR, GFP_KERNEL);
+ if (!new_hdr) {
+ retval = -ENOMEM;
+ goto free_old_hdr;
+ }
+ retval =__copy_from_user
+ (new_hdr, buf, SZ_SG_IO_HDR);
+ req_pack_id = new_hdr->pack_id;
+ kfree(new_hdr);
+ if (retval) {
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
}
} else
- req_pack_id = old_hdr.pack_id;
+ req_pack_id = old_hdr->pack_id;
}
srp = sg_get_rq_mark(sfp, req_pack_id);
if (!srp) { /* now wait on packet to arrive */
- if (sdp->detached)
- return -ENODEV;
- if (filp->f_flags & O_NONBLOCK)
- return -EAGAIN;
+ if (sdp->detached) {
+ retval = -ENODEV;
+ goto free_old_hdr;
+ }
+ if (filp->f_flags & O_NONBLOCK) {
+ retval = -EAGAIN;
+ goto free_old_hdr;
+ }
while (1) {
- res = 0;/* following is a macro that beats race
condition */
+ retval = 0; /* following macro beats race condition */
__wait
