* [PATCH] fusion: streamline ->queuecommand
@ 2004-10-02 8:13 Christoph Hellwig
2004-10-02 13:39 ` Matthew Wilcox
2004-10-21 9:21 ` Christoph Hellwig
0 siblings, 2 replies; 20+ messages in thread
From: Christoph Hellwig @ 2004-10-02 8:13 UTC (permalink / raw)
To: Emoore; +Cc: linux-scsi
--- 1.50/drivers/message/fusion/mptscsih.c 2004-09-29 01:58:39 +02:00
+++ edited/drivers/message/fusion/mptscsih.c 2004-10-01 12:45:04 +02:00
@@ -1863,11 +1844,7 @@
u32 cmd_len;
int my_idx;
int ii;
- int rc;
- int did_errcode;
- int issueCmd;
- did_errcode = 0;
hd = (MPT_SCSI_HOST *) SCpnt->device->host->hostdata;
target = SCpnt->device->id;
lun = SCpnt->device->lun;
@@ -1966,84 +1943,78 @@
/* Now add the SG list
* Always have a SGE even if null length.
*/
- rc = SUCCESS;
if (datalen == 0) {
/* Add a NULL SGE */
mptscsih_add_sge((char *)&pScsiReq->SGL, MPT_SGE_FLAGS_SSIMPLE_READ | 0,
(dma_addr_t) -1);
} else {
/* Add a 32 or 64 bit SGE */
- rc = mptscsih_AddSGE(hd->ioc, SCpnt, pScsiReq, my_idx);
+ if (mptscsih_AddSGE(hd->ioc, SCpnt, pScsiReq, my_idx) != SUCCESS)
+ goto fail;
}
-
- if (rc == SUCCESS) {
- hd->ScsiLookup[my_idx] = SCpnt;
- SCpnt->host_scribble = NULL;
-
- /* SCSI specific processing */
- issueCmd = 1;
- if (hd->is_spi) {
- int dvStatus = hd->ioc->spi_data.dvStatus[target];
-
- if (dvStatus || hd->ioc->spi_data.forceDv) {
+ hd->ScsiLookup[my_idx] = SCpnt;
+ SCpnt->host_scribble = NULL;
#ifdef MPTSCSIH_ENABLE_DOMAIN_VALIDATION
- if ((dvStatus & MPT_SCSICFG_NEED_DV) ||
- (hd->ioc->spi_data.forceDv & MPT_SCSICFG_NEED_DV)) {
- unsigned long lflags;
- /* Schedule DV if necessary */
- spin_lock_irqsave(&dvtaskQ_lock, lflags);
- if (!dvtaskQ_active) {
- dvtaskQ_active = 1;
- spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
- INIT_WORK(&mptscsih_dvTask, mptscsih_domainValidation, (void *) hd);
-
- schedule_work(&mptscsih_dvTask);
- } else {
- spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
- }
- hd->ioc->spi_data.forceDv &= ~MPT_SCSICFG_NEED_DV;
- }
-
- /* Trying to do DV to this target, extend timeout.
- * Wait to issue until flag is clear
- */
- if (dvStatus & MPT_SCSICFG_DV_PENDING) {
- mod_timer(&SCpnt->eh_timeout, jiffies + 40 * HZ);
- issueCmd = 0;
+ if (hd->is_spi) {
+ int dvStatus = hd->ioc->spi_data.dvStatus[target];
+ int issueCmd = 1;
+
+ if (dvStatus || hd->ioc->spi_data.forceDv) {
+
+ if ((dvStatus & MPT_SCSICFG_NEED_DV) ||
+ (hd->ioc->spi_data.forceDv & MPT_SCSICFG_NEED_DV)) {
+ unsigned long lflags;
+ /* Schedule DV if necessary */
+ spin_lock_irqsave(&dvtaskQ_lock, lflags);
+ if (!dvtaskQ_active) {
+ dvtaskQ_active = 1;
+ spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
+ INIT_WORK(&mptscsih_dvTask, mptscsih_domainValidation, (void *) hd);
+
+ schedule_work(&mptscsih_dvTask);
+ } else {
+ spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
}
+ hd->ioc->spi_data.forceDv &= ~MPT_SCSICFG_NEED_DV;
+ }
- /* Set the DV flags.
- */
- if (dvStatus & MPT_SCSICFG_DV_NOT_DONE)
- mptscsih_set_dvflags(hd, pScsiReq);
-#endif
+ /* Trying to do DV to this target, extend timeout.
+ * Wait to issue until flag is clear
+ */
+ if (dvStatus & MPT_SCSICFG_DV_PENDING) {
+ mod_timer(&SCpnt->eh_timeout, jiffies + 40 * HZ);
+ issueCmd = 0;
}
- }
-#ifdef MPTSCSIH_DBG_TIMEOUT
- if (hd->ioc->timeout_cnt < hd->ioc->timeout_maxcnt) {
- foo_to[hd->ioc->timeout_cnt] = SCpnt;
- hd->ioc->timeout_cnt++;
- //mod_timer(&SCpnt->eh_timeout, jiffies + hd->ioc->timeout_delta);
- issueCmd = 0;
- printk(MYIOC_s_WARN_FMT
- "to pendingQ: (sc=%p, mf=%p, time=%ld)\n",
- hd->ioc->name, SCpnt, mf, jiffies);
+ /* Set the DV flags.
+ */
+ if (dvStatus & MPT_SCSICFG_DV_NOT_DONE)
+ mptscsih_set_dvflags(hd, pScsiReq);
+
+ if (!issueCmd)
+ goto fail;
}
+ }
#endif
- if (issueCmd) {
- mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
- dmfprintk((MYIOC_s_INFO_FMT "Issued SCSI cmd (%p) mf=%p idx=%d\n",
- hd->ioc->name, SCpnt, mf, my_idx));
- DBG_DUMP_REQUEST_FRAME(mf)
- } else
- goto fail;
- } else
+#ifdef MPTSCSIH_DBG_TIMEOUT
+ if (hd->ioc->timeout_cnt < hd->ioc->timeout_maxcnt) {
+ foo_to[hd->ioc->timeout_cnt] = SCpnt;
+ hd->ioc->timeout_cnt++;
+ //mod_timer(&SCpnt->eh_timeout, jiffies + hd->ioc->timeout_delta);
+ printk(MYIOC_s_WARN_FMT
+ "to pendingQ: (sc=%p, mf=%p, time=%ld)\n",
+ hd->ioc->name, SCpnt, mf, jiffies);
goto fail;
+ }
+#endif
+ mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
+ dmfprintk((MYIOC_s_INFO_FMT "Issued SCSI cmd (%p) mf=%p idx=%d\n",
+ hd->ioc->name, SCpnt, mf, my_idx));
+ DBG_DUMP_REQUEST_FRAME(mf)
return 0;
fail:
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-02 8:13 Christoph Hellwig
@ 2004-10-02 13:39 ` Matthew Wilcox
2004-10-02 14:49 ` Christoph Hellwig
2004-10-21 9:21 ` Christoph Hellwig
1 sibling, 1 reply; 20+ messages in thread
From: Matthew Wilcox @ 2004-10-02 13:39 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Emoore, linux-scsi
On Sat, Oct 02, 2004 at 10:13:55AM +0200, Christoph Hellwig wrote:
> --- 1.50/drivers/message/fusion/mptscsih.c 2004-09-29 01:58:39 +02:00
> +++ edited/drivers/message/fusion/mptscsih.c 2004-10-01 12:45:04 +02:00
> @@ -1863,11 +1844,7 @@
>
> #ifdef MPTSCSIH_ENABLE_DOMAIN_VALIDATION
> + int dvStatus = hd->ioc->spi_data.dvStatus[target];
> + int issueCmd = 1;
> +
> + if (dvStatus || hd->ioc->spi_data.forceDv) {
> +
> + if ((dvStatus & MPT_SCSICFG_NEED_DV) ||
> + (hd->ioc->spi_data.forceDv & MPT_SCSICFG_NEED_DV)) {
> + unsigned long lflags;
> + /* Schedule DV if necessary */
Of course, at some point, Fusion should be switched over to use the
scsi_transport_spi domain validation code ... not urgent, just an
item for the long-term todo list.
--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-02 13:39 ` Matthew Wilcox
@ 2004-10-02 14:49 ` Christoph Hellwig
0 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2004-10-02 14:49 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: Christoph Hellwig, Emoore, linux-scsi
On Sat, Oct 02, 2004 at 02:39:15PM +0100, Matthew Wilcox wrote:
> Of course, at some point, Fusion should be switched over to use the
> scsi_transport_spi domain validation code ... not urgent, just an
> item for the long-term todo list.
absolutely. Also the dv threaad handling in fusion is rather bogus,
and moving to the genric one would fix that nicely.
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
@ 2004-10-04 21:33 Moore, Eric Dean
2004-10-04 21:57 ` James Bottomley
2004-10-06 15:41 ` Christoph Hellwig
0 siblings, 2 replies; 20+ messages in thread
From: Moore, Eric Dean @ 2004-10-04 21:33 UTC (permalink / raw)
To: Christoph Hellwig, Matthew Wilcox; +Cc: Christoph Hellwig, linux-scsi
On Saturday, October 02, 2004 8:50 AM, Christoph Hellwig wrote:
> On Sat, Oct 02, 2004 at 02:39:15PM +0100, Matthew Wilcox wrote:
> > Of course, at some point, Fusion should be switched over to use the
> > scsi_transport_spi domain validation code ... not urgent, just an
> > item for the long-term todo list.
>
> absolutely. Also the dv threaad handling in fusion is rather bogus,
> and moving to the genric one would fix that nicely.
>
I don't recommend removing the domain validation code in place of the
generic.
Points:
(1) We have three levels of DV(Domain Validation): none, basic, and
enahanced.
OEM Customers can set their own level of DV which is stored in nvram data,
and
passed to the driver and read thru config pages.
(2) MPT driver is sending negotiation through firmware config device pages 0
and 1. Is there
another way to send negotiation {sdtr and ppr } messages directly to scsi
devices in
a generic API? I have not looked at the generic DV, so I'm not for sure how
this would
be handled.
(3) How will a generic negotiation be done for raid volumes? The MPT driver
supports RAID levels
0, 1, and 0+1. The driver does DV via RAID passthru to the individual
devices.
(4) The MPT driver has several work arounds in DV path to take advantage of
the
full inquiries that are passed as part of its alogirthm. The problem in
older kernels
were some distro's such as SuSE was only sending 36 bytes of inquiry data
during Scaning the bus,
and we are needing 57 bytes to determine if device has IU and QAS set, as
well as detecting
if the devices is a safte device. In addition, with IU set or not, the
driver will know whether 64 or 256 luns
can be scanned for.
(5) We only do DV for SCSI HBA's. DV is not done for Fiber or SAS, which is
also handled by this driver.
(6) DV algorithm works, and well tested, and accepted by our OEMS. Nothing
is broke. Why break it?
Eric Moore
LSI Logic
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
2004-10-04 21:33 [PATCH] fusion: streamline ->queuecommand Moore, Eric Dean
@ 2004-10-04 21:57 ` James Bottomley
2004-10-06 15:41 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: James Bottomley @ 2004-10-04 21:57 UTC (permalink / raw)
To: Eric Dean Moore
Cc: Christoph Hellwig, Matthew Wilcox, Christoph Hellwig,
SCSI Mailing List
On Mon, 2004-10-04 at 16:33, Moore, Eric Dean wrote:
> Points:
> (1) We have three levels of DV(Domain Validation): none, basic, and
> enahanced.
> OEM Customers can set their own level of DV which is stored in nvram data,
> and
> passed to the driver and read thru config pages.
By basic and enhanced I presume you mean read only vs device write echo
buffer tests?
You can chose not to invoke spi_dv_device(), so we can do none and
enhanced (whether we do enhanced or basic depends on device support for
the echo buffer).
> (2) MPT driver is sending negotiation through firmware config device pages 0
> and 1. Is there
> another way to send negotiation {sdtr and ppr } messages directly to scsi
> devices in
> a generic API? I have not looked at the generic DV, so I'm not for sure how
> this would
> be handled.
No, the API assumes you supply the functions for setting and retrieving
the transport parameters. How you implement this is at your option.
> (3) How will a generic negotiation be done for raid volumes? The MPT driver
> supports RAID levels
> 0, 1, and 0+1. The driver does DV via RAID passthru to the individual
> devices.
As long as real scsi_device devices exist for the passthrough, it would
also be able to do dv for those.
> (4) The MPT driver has several work arounds in DV path to take advantage of
> the
> full inquiries that are passed as part of its alogirthm. The problem in
> older kernels
> were some distro's such as SuSE was only sending 36 bytes of inquiry data
> during Scaning the bus,
> and we are needing 57 bytes to determine if device has IU and QAS set, as
> well as detecting
> if the devices is a safte device. In addition, with IU set or not, the
> driver will know whether 64 or 256 luns
> can be scanned for.
This is obsolete and needs removing ... the mid layer does a 2 phase
inquiry and sets the capability support based on this. No device should
be doing inquiry snooping.
> (5) We only do DV for SCSI HBA's. DV is not done for Fiber or SAS, which is
> also handled by this driver.
DV is part of the SPI transport class only ... it's not an option for
the others.
> (6) DV algorithm works, and well tested, and accepted by our OEMS. Nothing
> is broke. Why break it?
Because sharing code and thus increasing its usage is good.
James
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
@ 2004-10-05 22:38 Moore, Eric Dean
2004-10-05 23:11 ` James Bottomley
0 siblings, 1 reply; 20+ messages in thread
From: Moore, Eric Dean @ 2004-10-05 22:38 UTC (permalink / raw)
To: James Bottomley, Stephens, Larry, Shirron, Stephen,
Gibbons, Terry
Cc: Christoph Hellwig, Matthew Wilcox, Christoph Hellwig,
SCSI Mailing List
On Monday, October 04, 2004 3:57 PM, James Bottomley wrote:
> > (3) How will a generic negotiation be done for raid
> volumes? The MPT driver
> > supports RAID levels
> > 0, 1, and 0+1. The driver does DV via RAID passthru to the
> individual
> > devices.
>
> As long as real scsi_device devices exist for the
> passthrough, it would
> also be able to do dv for those.
The physical devices hiddend in a RAID array are
not going to be exported as real scsi_devices to the layer
above the LLD. Only the virtual volume represting the RAID array will be.
Therefore it will be impossible for DV to be handled from a layer
outside the LLD for RAID arrays.
>
> > (4) The MPT driver has several work arounds in DV path to
> take advantage of
> > the
> > full inquiries that are passed as part of its alogirthm.
> The problem in
> > older kernels
> > were some distro's such as SuSE was only sending 36 bytes
> of inquiry data
> > during Scaning the bus,
> > and we are needing 57 bytes to determine if device has IU
> and QAS set, as
> > well as detecting
> > if the devices is a safte device. In addition, with IU set
> or not, the
> > driver will know whether 64 or 256 luns
> > can be scanned for.
>
> This is obsolete and needs removing ... the mid layer does a 2 phase
> inquiry and sets the capability support based on this. No
> device should
> be doing inquiry snooping.
Can we assume that all linux distributions provide at least 57 bytes of
inquiry data
during the scsi bus scan? I don't know if Suse has solved this, but they
were
in 2.4 kernel only sending 36 bytes of inquiry data.
>
> > (5) We only do DV for SCSI HBA's. DV is not done for Fiber
> or SAS, which is
> > also handled by this driver.
>
> DV is part of the SPI transport class only ... it's not an option for
> the others.
>
> > (6) DV algorithm works, and well tested, and accepted by
> our OEMS. Nothing
> > is broke. Why break it?
>
> Because sharing code and thus increasing its usage is good.
That is not a risk that LSI Logic is willing to take;
currently there is not a 2.7 kernel testbed for this.
Having someone outside LSI blindly posting a patch removing our
DV and not having throughly tested could have grave results.
The DV currently in place has been thru many test cycles in our own
test labs, and these drivers are well accepted by our customer base.
Other LSI employees copied on this email can comment futher on that.
Eric
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
2004-10-05 22:38 Moore, Eric Dean
@ 2004-10-05 23:11 ` James Bottomley
2004-10-05 23:37 ` Patrick Mansfield
0 siblings, 1 reply; 20+ messages in thread
From: James Bottomley @ 2004-10-05 23:11 UTC (permalink / raw)
To: Eric Dean Moore
Cc: Stephens, Larry, Shirron, Stephen, Gibbons, Terry,
Christoph Hellwig, Matthew Wilcox, Christoph Hellwig,
SCSI Mailing List
On Tue, 2004-10-05 at 17:38, Moore, Eric Dean wrote:
> Can we assume that all linux distributions provide at least 57 bytes of
> inquiry data
> during the scsi bus scan? I don't know if Suse has solved this, but they
> were
> in 2.4 kernel only sending 36 bytes of inquiry data.
2.6 always does a two phase inquiry, the minimum first and then the full
inquiry string. We pick the negotiation flags you're looking for out of
the inquiry data and save them away in the scsi_device structure, so you
don't even have to parse the saved inquiry data.
> > > (6) DV algorithm works, and well tested, and accepted by
> > our OEMS. Nothing
> > > is broke. Why break it?
> >
> > Because sharing code and thus increasing its usage is good.
>
> That is not a risk that LSI Logic is willing to take;
> currently there is not a 2.7 kernel testbed for this.
> Having someone outside LSI blindly posting a patch removing our
> DV and not having throughly tested could have grave results.
> The DV currently in place has been thru many test cycles in our own
> test labs, and these drivers are well accepted by our customer base.
> Other LSI employees copied on this email can comment futher on that.
I'm not forcing people to use it ... yet.
However, the attitude that "my driver works and I've tested it, so I'm
not going to change it" is in a large measure what got the SCSI subsytem
into its pariah status in 2.4: too many in-driver work arounds and no
thought to fixing the problems in the generic code.
If the benefits of sharing common code in an open source model aren't
apparent then consider this: the last three critical Adaptec bugs were
all in the domain validation code; and they thought their driver was
"thoroughly tested" too.
James
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-05 23:11 ` James Bottomley
@ 2004-10-05 23:37 ` Patrick Mansfield
2004-10-06 0:48 ` James Bottomley
0 siblings, 1 reply; 20+ messages in thread
From: Patrick Mansfield @ 2004-10-05 23:37 UTC (permalink / raw)
To: James Bottomley
Cc: Eric Dean Moore, Stephens, Larry, Shirron, Stephen,
Gibbons, Terry, Christoph Hellwig, Matthew Wilcox,
Christoph Hellwig, SCSI Mailing List
On Tue, Oct 05, 2004 at 06:11:22PM -0500, James Bottomley wrote:
> However, the attitude that "my driver works and I've tested it, so I'm
> not going to change it" is in a large measure what got the SCSI subsytem
> into its pariah status in 2.4: too many in-driver work arounds and no
> thought to fixing the problems in the generic code.
Yes, we want drivers to move forward, but with our current model, we don't
have any 2.6.x kernel.org stable kernels.
When we first went to this model I thought it did not matter, you could
just get stable kernels from a distro, but now I see problems with it.
If code changes for a mainline kernel scsi driver, there is no place to
keep a stable driver as part of a stable kernel.org series.
Driver maintainers are getting requests to backport current 2.6.x code to
distributions in order to support stable kernel series, else they have to
(effectively) maintain a driver version for each distro.
When we had 2.4 stable and 2.5 development, there was a single repository
for stable kernels, and one for development. Maintainers did not have to
track distros, the distros could (generally) pull from the latest 2.4.x
tree to sync up with the latest driver fixes.
-- Patrick Mansfield
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-05 23:37 ` Patrick Mansfield
@ 2004-10-06 0:48 ` James Bottomley
0 siblings, 0 replies; 20+ messages in thread
From: James Bottomley @ 2004-10-06 0:48 UTC (permalink / raw)
To: Patrick Mansfield
Cc: Eric Dean Moore, Stephens, Larry, Shirron, Stephen,
Gibbons, Terry, Christoph Hellwig, Matthew Wilcox,
Christoph Hellwig, SCSI Mailing List
On Tue, 2004-10-05 at 18:37, Patrick Mansfield wrote:
> On Tue, Oct 05, 2004 at 06:11:22PM -0500, James Bottomley wrote:
> Yes, we want drivers to move forward, but with our current model, we don't
> have any 2.6.x kernel.org stable kernels.
>From the API point of view, that's correct
> When we first went to this model I thought it did not matter, you could
> just get stable kernels from a distro, but now I see problems with it.
>
> If code changes for a mainline kernel scsi driver, there is no place to
> keep a stable driver as part of a stable kernel.org series.
True, but on the other hand no-one really keeps their stable drivers in
vanilla 2.4 either.
> Driver maintainers are getting requests to backport current 2.6.x code to
> distributions in order to support stable kernel series, else they have to
> (effectively) maintain a driver version for each distro.
This is normal.
> When we had 2.4 stable and 2.5 development, there was a single repository
> for stable kernels, and one for development. Maintainers did not have to
> track distros, the distros could (generally) pull from the latest 2.4.x
> tree to sync up with the latest driver fixes.
This isn't really the case. In the 2.4 timeframe, distribution kernels
were positively awash with backports from 2.5 (and generally different
backports or implementations too) ... you only have to look at something
like the adaptec driver with it's redhat specific pieces to appreciate
this.
A distribution is a snapshot, so there'll always be a backport issue for
vendors who interact directly with distribution kernels. However, the
difference between sles 9 and the current 2.6 is a lot less than between
sles 8 and 2.5. So, I accept that there's backport work to be done but
I don't accept necessarily that the situation is worse than what we had
in 2.4 where the amount of distribution patches were bigger and no
vendor tracked the 2.5 tree.
In theory, the vendor promise of "upstream first" means that the vendor
kernels should be tracking 2.6 more closely and thus the backports
should be easier ... and the requirement is that the vendor feature be
in vanilla 2.6 first.
James
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
@ 2004-10-06 14:00 Gibbons, Terry
2004-10-06 14:13 ` Arjan van de Ven
2004-10-06 14:25 ` Matthew Wilcox
0 siblings, 2 replies; 20+ messages in thread
From: Gibbons, Terry @ 2004-10-06 14:00 UTC (permalink / raw)
To: 'James Bottomley', Moore, Eric Dean
Cc: Stephens, Larry, Shirron, Stephen, Matthew Wilcox,
Christoph Hellwig, SCSI Mailing List
Good Day,
As a development manager, I'm responsible for technical ingenuity as well as
customer satisfaction. My responsibilities require that I look at issues
from a business point of view as well as a technical point of view.
LSI Logic supports 80% of the server market that is shipping Ultra320 SCSI.
We supply a free hardware based RAID integrated into our U320 devices. The
firmware was written three years ago. The firmware, running behind an
integrated processor, does not allow the host system to see any peripheral
it chooses to hide. The firmware chooses to hide all physical disks of a
RAID volume. Therefore, no outside entity, can talk to any disk drive
"hidden" by our volume management code in the firmware...unless the proper
Fusion-MPT (tm) message is sent to the firmware. Given the age of the
firmware (stable but vulnerable and shipped on millions of systems) and the
lack of external memory to add code, the firmware won't change.
Therefore, the technical facts are that if Domain Validation is removed this
will cause our customers field upgrade problems, new system integration test
problems, manufacturing test problems, and perhaps no capability at all to
get drives in an array faster than ASYNC. I know the latter will happen on a
drive swap.
A change in DV code will result in lost time to LSI, Red Hat, SuSE, and all
of LSI's customers as there will be absolutely no choice but to force LSI
driver patches for DV through Red Hat and SuSE (at a minimum).
Now, I challenge all of you. Ultra320 SCSI was fully released over two and a
half years ago. With SAS and SATA being the new technologies, what's the
benefit in changing a technology that is peaking and soon to be replaced?
Regards,
Terry Gibbons
LSI Logic Corp
Software Development Manager
-----Original Message-----
From: James Bottomley [mailto:James.Bottomley@SteelEye.com]
Sent: Tuesday, October 05, 2004 5:11 PM
To: Eric Dean Moore
Cc: Stephens, Larry; Shirron, Stephen; Gibbons, Terry; Christoph
Hellwig; Matthew Wilcox; Christoph Hellwig; SCSI Mailing List
Subject: RE: [PATCH] fusion: streamline ->queuecommand
On Tue, 2004-10-05 at 17:38, Moore, Eric Dean wrote:
> Can we assume that all linux distributions provide at least 57 bytes of
> inquiry data
> during the scsi bus scan? I don't know if Suse has solved this, but they
> were
> in 2.4 kernel only sending 36 bytes of inquiry data.
2.6 always does a two phase inquiry, the minimum first and then the full
inquiry string. We pick the negotiation flags you're looking for out of
the inquiry data and save them away in the scsi_device structure, so you
don't even have to parse the saved inquiry data.
> > > (6) DV algorithm works, and well tested, and accepted by
> > our OEMS. Nothing
> > > is broke. Why break it?
> >
> > Because sharing code and thus increasing its usage is good.
>
> That is not a risk that LSI Logic is willing to take;
> currently there is not a 2.7 kernel testbed for this.
> Having someone outside LSI blindly posting a patch removing our
> DV and not having throughly tested could have grave results.
> The DV currently in place has been thru many test cycles in our own
> test labs, and these drivers are well accepted by our customer base.
> Other LSI employees copied on this email can comment futher on that.
I'm not forcing people to use it ... yet.
However, the attitude that "my driver works and I've tested it, so I'm
not going to change it" is in a large measure what got the SCSI subsytem
into its pariah status in 2.4: too many in-driver work arounds and no
thought to fixing the problems in the generic code.
If the benefits of sharing common code in an open source model aren't
apparent then consider this: the last three critical Adaptec bugs were
all in the domain validation code; and they thought their driver was
"thoroughly tested" too.
James
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
2004-10-06 14:00 Gibbons, Terry
@ 2004-10-06 14:13 ` Arjan van de Ven
2004-10-06 14:25 ` Matthew Wilcox
1 sibling, 0 replies; 20+ messages in thread
From: Arjan van de Ven @ 2004-10-06 14:13 UTC (permalink / raw)
To: Gibbons, Terry
Cc: 'James Bottomley', Moore, Eric Dean, Stephens, Larry,
Shirron, Stephen, Matthew Wilcox, Christoph Hellwig,
SCSI Mailing List
[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]
On Wed, 2004-10-06 at 16:00, Gibbons, Terry wrote:
> A change in DV code will result in lost time to LSI, Red Hat, SuSE, and all
> of LSI's customers as there will be absolutely no choice but to force LSI
> driver patches for DV through Red Hat and SuSE (at a minimum).
I personally object to your terminology here. LSI cannot force any
patches through Red Hat, and I suspect the same holds for SuSE. LSI can
request patch inclusion which Red Hat or SuSE may or may not decide to
do. I can't speak for SuSE but RH seems to not be a great fan of huge
driver patches that deviate from what is available and tested upstream.
> Now, I challenge all of you. Ultra320 SCSI was fully released over two and a
> half years ago. With SAS and SATA being the new technologies, what's the
> benefit in changing a technology that is peaking and soon to be replaced?
If you are unmaintaining the driver just say so. If you're not, please
consider the fact that we have to work with your code, and that we have
to keep making sure your driver works with "our" scsi stack. Previously
the linux scsi layer became a really big mess this way because drivers
just never changed and no real innovation was possible as a result.
Times have changed, the scsi layer now is actively maintained and moving
forward both in code quality, reliability and functionality. Your
statement seems to suggest you want to hold back linux scsi....
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
@ 2004-10-06 14:23 Gibbons, Terry
2004-10-06 14:30 ` Christoph Hellwig
2004-10-06 15:47 ` James Bottomley
0 siblings, 2 replies; 20+ messages in thread
From: Gibbons, Terry @ 2004-10-06 14:23 UTC (permalink / raw)
To: 'arjanv@redhat.com'
Cc: 'James Bottomley', Moore, Eric Dean, Stephens, Larry,
Shirron, Stephen, Matthew Wilcox, Christoph Hellwig,
SCSI Mailing List
I stand corrected. Our customers will force driver patches through Red Hat
and SuSE. They buy these products and keep Linux distributors in business.
Do you expect our customers to take huge hits to their bottom line because
DV will no longer execute properly on LSI devices?
Maybe we should change the focus. What are your suggestions for making sure
that every major server vendor isn't negatively impacted by changes to DV?
As for your last paragraph, I'm saying that there are more important and
pressing issues to be tackled than one issue on an old technology.
Terry
-----Original Message-----
From: Arjan van de Ven [mailto:arjanv@redhat.com]
Sent: Wednesday, October 06, 2004 8:14 AM
To: Gibbons, Terry
Cc: 'James Bottomley'; Moore, Eric Dean; Stephens, Larry; Shirron,
Stephen; Matthew Wilcox; Christoph Hellwig; SCSI Mailing List
Subject: RE: [PATCH] fusion: streamline ->queuecommand
<< File: signature.asc >> On Wed, 2004-10-06 at 16:00, Gibbons, Terry
wrote:
> A change in DV code will result in lost time to LSI, Red Hat, SuSE, and
all
> of LSI's customers as there will be absolutely no choice but to force LSI
> driver patches for DV through Red Hat and SuSE (at a minimum).
I personally object to your terminology here. LSI cannot force any
patches through Red Hat, and I suspect the same holds for SuSE. LSI can
request patch inclusion which Red Hat or SuSE may or may not decide to
do. I can't speak for SuSE but RH seems to not be a great fan of huge
driver patches that deviate from what is available and tested upstream.
> Now, I challenge all of you. Ultra320 SCSI was fully released over two and
a
> half years ago. With SAS and SATA being the new technologies, what's the
> benefit in changing a technology that is peaking and soon to be replaced?
If you are unmaintaining the driver just say so. If you're not, please
consider the fact that we have to work with your code, and that we have
to keep making sure your driver works with "our" scsi stack. Previously
the linux scsi layer became a really big mess this way because drivers
just never changed and no real innovation was possible as a result.
Times have changed, the scsi layer now is actively maintained and moving
forward both in code quality, reliability and functionality. Your
statement seems to suggest you want to hold back linux scsi....
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-06 14:00 Gibbons, Terry
2004-10-06 14:13 ` Arjan van de Ven
@ 2004-10-06 14:25 ` Matthew Wilcox
1 sibling, 0 replies; 20+ messages in thread
From: Matthew Wilcox @ 2004-10-06 14:25 UTC (permalink / raw)
To: Gibbons, Terry
Cc: 'James Bottomley', Moore, Eric Dean, Stephens, Larry,
Shirron, Stephen, Matthew Wilcox, Christoph Hellwig,
SCSI Mailing List
On Wed, Oct 06, 2004 at 08:00:31AM -0600, Gibbons, Terry wrote:
> Good Day,
>
> As a development manager, I'm responsible for technical ingenuity as well as
> customer satisfaction. My responsibilities require that I look at issues
> from a business point of view as well as a technical point of view.
Good Day,
As a programmer, I'm responsible for caring about more than just my
little bit of the kernel. My responsibilities require that I look at
issues from a wider perspective, not just how it affects my driver.
> LSI Logic supports 80% of the server market that is shipping Ultra320 SCSI.
I once owned a guineau pig named Doris.
> We supply a free hardware based RAID integrated into our U320 devices. The
> firmware was written three years ago. The firmware, running behind an
> integrated processor, does not allow the host system to see any peripheral
> it chooses to hide. The firmware chooses to hide all physical disks of a
> RAID volume. Therefore, no outside entity, can talk to any disk drive
> "hidden" by our volume management code in the firmware...unless the proper
> Fusion-MPT (tm) message is sent to the firmware. Given the age of the
> firmware (stable but vulnerable and shipped on millions of systems) and the
> lack of external memory to add code, the firmware won't change.
I think your lack of actually reading the code shines through here. Fusion
gets to continue to send whatever messages it likes with the generic DV
code. The sym2 code still manufactures its own PPR/SDTR/WDTR packets.
It just does so based on what the DV code tells it to do rather than
making its own decisions about how to negotiate with devices.
> Now, I challenge all of you. Ultra320 SCSI was fully released over two and a
> half years ago. With SAS and SATA being the new technologies, what's the
> benefit in changing a technology that is peaking and soon to be replaced?
We update drivers all the time that are for hardware that was obsolete
10 years ago. This is what stops Linux from degenerating into an
unmaintainable heap of spaghetti code.
--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-06 14:23 Gibbons, Terry
@ 2004-10-06 14:30 ` Christoph Hellwig
2004-10-06 15:47 ` James Bottomley
1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2004-10-06 14:30 UTC (permalink / raw)
To: Gibbons, Terry
Cc: 'arjanv@redhat.com', 'James Bottomley',
Moore, Eric Dean, Stephens, Larry, Shirron, Stephen,
Matthew Wilcox, SCSI Mailing List
On Wed, Oct 06, 2004 at 08:23:23AM -0600, Gibbons, Terry wrote:
> I stand corrected. Our customers will force driver patches through Red Hat
> and SuSE. They buy these products and keep Linux distributors in business.
> Do you expect our customers to take huge hits to their bottom line because
> DV will no longer execute properly on LSI devices?
You're totally missing the point. No one wants to remove DV
functionality. We want a single implementation of the DV mechanism in
the SPI transport class instead of various buggy copies in individual
drivers.
No may I kindly suggest that you stop trolling on this list and let your
staff handle this issue. I'm pretty sure we can work out a solution
that suits everyone with Eric if we get the nonsensical managment issues
out of the loop.
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
@ 2004-10-06 14:46 Gibbons, Terry
2004-10-06 14:58 ` Matthew Wilcox
0 siblings, 1 reply; 20+ messages in thread
From: Gibbons, Terry @ 2004-10-06 14:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: 'arjanv@redhat.com', 'James Bottomley',
Moore, Eric Dean, Stephens, Larry, Shirron, Stephen,
Matthew Wilcox, SCSI Mailing List
Cristoph, Matthew,
I agree that I really shouldn't be on this thread. I am not a Linux expert
and I haven't read this code.
I was copied because the very people that will resolve the issue, namely
Eric and Larry, included me as to bring support to their position that they
have a BIG problem and solutions aren't being mutually agreed upon.
Therefore, I'm the one chosen to take the unpopular stance of challenging
the status quo and asking for solutions as opposed to pushback on every
stance LSI has put forth.
Terry
-----Original Message-----
From: Christoph Hellwig [mailto:hch@lst.de]
Sent: Wednesday, October 06, 2004 8:30 AM
To: Gibbons, Terry
Cc: 'arjanv@redhat.com'; 'James Bottomley'; Moore, Eric Dean; Stephens,
Larry; Shirron, Stephen; Matthew Wilcox; SCSI Mailing List
Subject: Re: [PATCH] fusion: streamline ->queuecommand
On Wed, Oct 06, 2004 at 08:23:23AM -0600, Gibbons, Terry wrote:
> I stand corrected. Our customers will force driver patches through Red Hat
> and SuSE. They buy these products and keep Linux distributors in business.
> Do you expect our customers to take huge hits to their bottom line because
> DV will no longer execute properly on LSI devices?
You're totally missing the point. No one wants to remove DV
functionality. We want a single implementation of the DV mechanism in
the SPI transport class instead of various buggy copies in individual
drivers.
No may I kindly suggest that you stop trolling on this list and let your
staff handle this issue. I'm pretty sure we can work out a solution
that suits everyone with Eric if we get the nonsensical managment issues
out of the loop.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-06 14:46 Gibbons, Terry
@ 2004-10-06 14:58 ` Matthew Wilcox
0 siblings, 0 replies; 20+ messages in thread
From: Matthew Wilcox @ 2004-10-06 14:58 UTC (permalink / raw)
To: Gibbons, Terry
Cc: Christoph Hellwig, 'arjanv@redhat.com',
'James Bottomley', Moore, Eric Dean, Stephens, Larry,
Shirron, Stephen, Matthew Wilcox, SCSI Mailing List
On Wed, Oct 06, 2004 at 08:46:11AM -0600, Gibbons, Terry wrote:
> I was copied because the very people that will resolve the issue, namely
> Eric and Larry, included me as to bring support to their position that they
> have a BIG problem and solutions aren't being mutually agreed upon.
>
> Therefore, I'm the one chosen to take the unpopular stance of challenging
> the status quo and asking for solutions as opposed to pushback on every
> stance LSI has put forth.
All I said was:
> Of course, at some point, Fusion should be switched over to use the
> scsi_transport_spi domain validation code ... not urgent, just an
> item for the long-term todo list.
I see the remainder of the thread as being pushback from _LSI_ on the
direction that the rest of the SCSI people have decided to go.
If you're not part of the solution ...
--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-04 21:33 [PATCH] fusion: streamline ->queuecommand Moore, Eric Dean
2004-10-04 21:57 ` James Bottomley
@ 2004-10-06 15:41 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2004-10-06 15:41 UTC (permalink / raw)
To: Moore, Eric Dean
Cc: Christoph Hellwig, Matthew Wilcox, Christoph Hellwig, linux-scsi
On Mon, Oct 04, 2004 at 05:33:41PM -0400, Moore, Eric Dean wrote:
> (3) How will a generic negotiation be done for raid volumes? The MPT driver
> supports RAID levels
> 0, 1, and 0+1. The driver does DV via RAID passthru to the individual
> devices.
I've taken a quick look at fusion in that respect. I think the cleanest
thing would be to add "virtual" scsi_device structures for raid physical
disks, and set the BLIST_NO_ULD_ATTACH flag so we don't get the disk
driver to attach to it. This would have the added benefit of beeing
able to use the SG_IO passthrough interface even for physical disk in
a raid volume. To make this work we'd obviously have to add some code
to ->queuecommand to handle this special case.
I think all the other questions have been answered already.
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
2004-10-06 14:23 Gibbons, Terry
2004-10-06 14:30 ` Christoph Hellwig
@ 2004-10-06 15:47 ` James Bottomley
1 sibling, 0 replies; 20+ messages in thread
From: James Bottomley @ 2004-10-06 15:47 UTC (permalink / raw)
To: Gibbons, Terry
Cc: 'arjanv@redhat.com', Eric Dean Moore, Stephens, Larry,
Shirron, Stephen, Matthew Wilcox, Christoph Hellwig,
SCSI Mailing List
On Wed, 2004-10-06 at 09:23, Gibbons, Terry wrote:
> Maybe we should change the focus. What are your suggestions for making sure
> that every major server vendor isn't negatively impacted by changes to DV?
Well, since every statment from you and your engineers is that you
haven't looked through the generic DV code, I suggest you do so if you
want to participate meaningfully in the discussion. Saying point blank
you won't use any replacement whatever it looks like is hardly conducive
to a measured debate.
After you look through it, any technical criticisms or requests for
missing features would be welcome.
You appear to be assuming that you can't suggest changes to the generic
DV code, which isn't the case. The object of making the code generic is
to avoid all the pitfalls that the other drivers fell into trying to
roll their own while creating the best code for all of them.
As has been said before no-one gets a veto on open source code. Your
choice is either to participate in the evolution of the SCSI subsystem
or not to. However, the evolution will continue, whatever you decide.
The thing I know it's hardest to appreciate, coming from the computing
industry myself, is that maintaining a driver doesn't just make you
responsible for that driver alone but also for improving the subsystem
it sits in and the rest of the kernel that surrounds it. If you find a
bug, it's always tempting to apply a workaround to your driver when what
is really needed is a fix for the subsystem itself. However, in
exchange for thinking of the kernel and the subsystem as well as just
your driver you get a genuine franchise in how the kernel and the
subsystem evolve ... most industry maintainers find that to be more than
worth the extra effort.
James
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fusion: streamline ->queuecommand
2004-10-02 8:13 Christoph Hellwig
2004-10-02 13:39 ` Matthew Wilcox
@ 2004-10-21 9:21 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2004-10-21 9:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Emoore, linux-scsi
ping?
On Sat, Oct 02, 2004 at 10:13:55AM +0200, Christoph Hellwig wrote:
>
> --- 1.50/drivers/message/fusion/mptscsih.c 2004-09-29 01:58:39 +02:00
> +++ edited/drivers/message/fusion/mptscsih.c 2004-10-01 12:45:04 +02:00
> @@ -1863,11 +1844,7 @@
> u32 cmd_len;
> int my_idx;
> int ii;
> - int rc;
> - int did_errcode;
> - int issueCmd;
>
> - did_errcode = 0;
> hd = (MPT_SCSI_HOST *) SCpnt->device->host->hostdata;
> target = SCpnt->device->id;
> lun = SCpnt->device->lun;
> @@ -1966,84 +1943,78 @@
> /* Now add the SG list
> * Always have a SGE even if null length.
> */
> - rc = SUCCESS;
> if (datalen == 0) {
> /* Add a NULL SGE */
> mptscsih_add_sge((char *)&pScsiReq->SGL, MPT_SGE_FLAGS_SSIMPLE_READ | 0,
> (dma_addr_t) -1);
> } else {
> /* Add a 32 or 64 bit SGE */
> - rc = mptscsih_AddSGE(hd->ioc, SCpnt, pScsiReq, my_idx);
> + if (mptscsih_AddSGE(hd->ioc, SCpnt, pScsiReq, my_idx) != SUCCESS)
> + goto fail;
> }
>
> -
> - if (rc == SUCCESS) {
> - hd->ScsiLookup[my_idx] = SCpnt;
> - SCpnt->host_scribble = NULL;
> -
> - /* SCSI specific processing */
> - issueCmd = 1;
> - if (hd->is_spi) {
> - int dvStatus = hd->ioc->spi_data.dvStatus[target];
> -
> - if (dvStatus || hd->ioc->spi_data.forceDv) {
> + hd->ScsiLookup[my_idx] = SCpnt;
> + SCpnt->host_scribble = NULL;
>
> #ifdef MPTSCSIH_ENABLE_DOMAIN_VALIDATION
> - if ((dvStatus & MPT_SCSICFG_NEED_DV) ||
> - (hd->ioc->spi_data.forceDv & MPT_SCSICFG_NEED_DV)) {
> - unsigned long lflags;
> - /* Schedule DV if necessary */
> - spin_lock_irqsave(&dvtaskQ_lock, lflags);
> - if (!dvtaskQ_active) {
> - dvtaskQ_active = 1;
> - spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
> - INIT_WORK(&mptscsih_dvTask, mptscsih_domainValidation, (void *) hd);
> -
> - schedule_work(&mptscsih_dvTask);
> - } else {
> - spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
> - }
> - hd->ioc->spi_data.forceDv &= ~MPT_SCSICFG_NEED_DV;
> - }
> -
> - /* Trying to do DV to this target, extend timeout.
> - * Wait to issue until flag is clear
> - */
> - if (dvStatus & MPT_SCSICFG_DV_PENDING) {
> - mod_timer(&SCpnt->eh_timeout, jiffies + 40 * HZ);
> - issueCmd = 0;
> + if (hd->is_spi) {
> + int dvStatus = hd->ioc->spi_data.dvStatus[target];
> + int issueCmd = 1;
> +
> + if (dvStatus || hd->ioc->spi_data.forceDv) {
> +
> + if ((dvStatus & MPT_SCSICFG_NEED_DV) ||
> + (hd->ioc->spi_data.forceDv & MPT_SCSICFG_NEED_DV)) {
> + unsigned long lflags;
> + /* Schedule DV if necessary */
> + spin_lock_irqsave(&dvtaskQ_lock, lflags);
> + if (!dvtaskQ_active) {
> + dvtaskQ_active = 1;
> + spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
> + INIT_WORK(&mptscsih_dvTask, mptscsih_domainValidation, (void *) hd);
> +
> + schedule_work(&mptscsih_dvTask);
> + } else {
> + spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
> }
> + hd->ioc->spi_data.forceDv &= ~MPT_SCSICFG_NEED_DV;
> + }
>
> - /* Set the DV flags.
> - */
> - if (dvStatus & MPT_SCSICFG_DV_NOT_DONE)
> - mptscsih_set_dvflags(hd, pScsiReq);
> -#endif
> + /* Trying to do DV to this target, extend timeout.
> + * Wait to issue until flag is clear
> + */
> + if (dvStatus & MPT_SCSICFG_DV_PENDING) {
> + mod_timer(&SCpnt->eh_timeout, jiffies + 40 * HZ);
> + issueCmd = 0;
> }
> - }
>
> -#ifdef MPTSCSIH_DBG_TIMEOUT
> - if (hd->ioc->timeout_cnt < hd->ioc->timeout_maxcnt) {
> - foo_to[hd->ioc->timeout_cnt] = SCpnt;
> - hd->ioc->timeout_cnt++;
> - //mod_timer(&SCpnt->eh_timeout, jiffies + hd->ioc->timeout_delta);
> - issueCmd = 0;
> - printk(MYIOC_s_WARN_FMT
> - "to pendingQ: (sc=%p, mf=%p, time=%ld)\n",
> - hd->ioc->name, SCpnt, mf, jiffies);
> + /* Set the DV flags.
> + */
> + if (dvStatus & MPT_SCSICFG_DV_NOT_DONE)
> + mptscsih_set_dvflags(hd, pScsiReq);
> +
> + if (!issueCmd)
> + goto fail;
> }
> + }
> #endif
>
> - if (issueCmd) {
> - mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
> - dmfprintk((MYIOC_s_INFO_FMT "Issued SCSI cmd (%p) mf=%p idx=%d\n",
> - hd->ioc->name, SCpnt, mf, my_idx));
> - DBG_DUMP_REQUEST_FRAME(mf)
> - } else
> - goto fail;
> - } else
> +#ifdef MPTSCSIH_DBG_TIMEOUT
> + if (hd->ioc->timeout_cnt < hd->ioc->timeout_maxcnt) {
> + foo_to[hd->ioc->timeout_cnt] = SCpnt;
> + hd->ioc->timeout_cnt++;
> + //mod_timer(&SCpnt->eh_timeout, jiffies + hd->ioc->timeout_delta);
> + printk(MYIOC_s_WARN_FMT
> + "to pendingQ: (sc=%p, mf=%p, time=%ld)\n",
> + hd->ioc->name, SCpnt, mf, jiffies);
> goto fail;
> + }
> +#endif
>
> + mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
> + dmfprintk((MYIOC_s_INFO_FMT "Issued SCSI cmd (%p) mf=%p idx=%d\n",
> + hd->ioc->name, SCpnt, mf, my_idx));
> + DBG_DUMP_REQUEST_FRAME(mf)
> return 0;
>
> fail:
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
---end quoted text---
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] fusion: streamline ->queuecommand
@ 2004-10-21 14:58 Moore, Eric Dean
0 siblings, 0 replies; 20+ messages in thread
From: Moore, Eric Dean @ 2004-10-21 14:58 UTC (permalink / raw)
To: Christoph Hellwig, Christoph Hellwig; +Cc: linux-scsi
Christoph - This patch and the other patch "kill FusionInitCalled" are fine.
These two patches are currently incorporated into a driver currently being
tested in the labs. I anticpate a posting a new release of the mpt driver
early next week having SAS support.
Eric Moore
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, October 21, 2004 3:22 AM
> To: Christoph Hellwig
> Cc: Emoore@lsil.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH] fusion: streamline ->queuecommand
>
>
> ping?
>
> On Sat, Oct 02, 2004 at 10:13:55AM +0200, Christoph Hellwig wrote:
> >
> > --- 1.50/drivers/message/fusion/mptscsih.c 2004-09-29
> 01:58:39 +02:00
> > +++ edited/drivers/message/fusion/mptscsih.c
> 2004-10-01 12:45:04 +02:00
> > @@ -1863,11 +1844,7 @@
> > u32 cmd_len;
> > int my_idx;
> > int ii;
> > - int rc;
> > - int did_errcode;
> > - int issueCmd;
> >
> > - did_errcode = 0;
> > hd = (MPT_SCSI_HOST *) SCpnt->device->host->hostdata;
> > target = SCpnt->device->id;
> > lun = SCpnt->device->lun;
> > @@ -1966,84 +1943,78 @@
> > /* Now add the SG list
> > * Always have a SGE even if null length.
> > */
> > - rc = SUCCESS;
> > if (datalen == 0) {
> > /* Add a NULL SGE */
> > mptscsih_add_sge((char *)&pScsiReq->SGL,
> MPT_SGE_FLAGS_SSIMPLE_READ | 0,
> > (dma_addr_t) -1);
> > } else {
> > /* Add a 32 or 64 bit SGE */
> > - rc = mptscsih_AddSGE(hd->ioc, SCpnt, pScsiReq, my_idx);
> > + if (mptscsih_AddSGE(hd->ioc, SCpnt, pScsiReq,
> my_idx) != SUCCESS)
> > + goto fail;
> > }
> >
> > -
> > - if (rc == SUCCESS) {
> > - hd->ScsiLookup[my_idx] = SCpnt;
> > - SCpnt->host_scribble = NULL;
> > -
> > - /* SCSI specific processing */
> > - issueCmd = 1;
> > - if (hd->is_spi) {
> > - int dvStatus =
> hd->ioc->spi_data.dvStatus[target];
> > -
> > - if (dvStatus || hd->ioc->spi_data.forceDv) {
> > + hd->ScsiLookup[my_idx] = SCpnt;
> > + SCpnt->host_scribble = NULL;
> >
> > #ifdef MPTSCSIH_ENABLE_DOMAIN_VALIDATION
> > - if ((dvStatus & MPT_SCSICFG_NEED_DV) ||
> > -
> (hd->ioc->spi_data.forceDv & MPT_SCSICFG_NEED_DV)) {
> > - unsigned long lflags;
> > - /* Schedule DV if necessary */
> > -
> spin_lock_irqsave(&dvtaskQ_lock, lflags);
> > - if (!dvtaskQ_active) {
> > - dvtaskQ_active = 1;
> > -
> spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
> > -
> INIT_WORK(&mptscsih_dvTask, mptscsih_domainValidation, (void *) hd);
> > -
> > -
> schedule_work(&mptscsih_dvTask);
> > - } else {
> > -
> spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
> > - }
> > -
> hd->ioc->spi_data.forceDv &= ~MPT_SCSICFG_NEED_DV;
> > - }
> > -
> > - /* Trying to do DV to this
> target, extend timeout.
> > - * Wait to issue until flag is clear
> > - */
> > - if (dvStatus & MPT_SCSICFG_DV_PENDING) {
> > -
> mod_timer(&SCpnt->eh_timeout, jiffies + 40 * HZ);
> > - issueCmd = 0;
> > + if (hd->is_spi) {
> > + int dvStatus = hd->ioc->spi_data.dvStatus[target];
> > + int issueCmd = 1;
> > +
> > + if (dvStatus || hd->ioc->spi_data.forceDv) {
> > +
> > + if ((dvStatus & MPT_SCSICFG_NEED_DV) ||
> > + (hd->ioc->spi_data.forceDv &
> MPT_SCSICFG_NEED_DV)) {
> > + unsigned long lflags;
> > + /* Schedule DV if necessary */
> > +
> spin_lock_irqsave(&dvtaskQ_lock, lflags);
> > + if (!dvtaskQ_active) {
> > + dvtaskQ_active = 1;
> > +
> spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
> > +
> INIT_WORK(&mptscsih_dvTask, mptscsih_domainValidation, (void *) hd);
> > +
> > + schedule_work(&mptscsih_dvTask);
> > + } else {
> > +
> spin_unlock_irqrestore(&dvtaskQ_lock, lflags);
> > }
> > + hd->ioc->spi_data.forceDv &=
> ~MPT_SCSICFG_NEED_DV;
> > + }
> >
> > - /* Set the DV flags.
> > - */
> > - if (dvStatus & MPT_SCSICFG_DV_NOT_DONE)
> > -
> mptscsih_set_dvflags(hd, pScsiReq);
> > -#endif
> > + /* Trying to do DV to this target,
> extend timeout.
> > + * Wait to issue until flag is clear
> > + */
> > + if (dvStatus & MPT_SCSICFG_DV_PENDING) {
> > + mod_timer(&SCpnt->eh_timeout,
> jiffies + 40 * HZ);
> > + issueCmd = 0;
> > }
> > - }
> >
> > -#ifdef MPTSCSIH_DBG_TIMEOUT
> > - if (hd->ioc->timeout_cnt < hd->ioc->timeout_maxcnt) {
> > - foo_to[hd->ioc->timeout_cnt] = SCpnt;
> > - hd->ioc->timeout_cnt++;
> > - //mod_timer(&SCpnt->eh_timeout, jiffies
> + hd->ioc->timeout_delta);
> > - issueCmd = 0;
> > - printk(MYIOC_s_WARN_FMT
> > - "to pendingQ: (sc=%p, mf=%p,
> time=%ld)\n",
> > - hd->ioc->name, SCpnt, mf, jiffies);
> > + /* Set the DV flags.
> > + */
> > + if (dvStatus & MPT_SCSICFG_DV_NOT_DONE)
> > + mptscsih_set_dvflags(hd, pScsiReq);
> > +
> > + if (!issueCmd)
> > + goto fail;
> > }
> > + }
> > #endif
> >
> > - if (issueCmd) {
> > - mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
> > - dmfprintk((MYIOC_s_INFO_FMT "Issued
> SCSI cmd (%p) mf=%p idx=%d\n",
> > - hd->ioc->name, SCpnt,
> mf, my_idx));
> > - DBG_DUMP_REQUEST_FRAME(mf)
> > - } else
> > - goto fail;
> > - } else
> > +#ifdef MPTSCSIH_DBG_TIMEOUT
> > + if (hd->ioc->timeout_cnt < hd->ioc->timeout_maxcnt) {
> > + foo_to[hd->ioc->timeout_cnt] = SCpnt;
> > + hd->ioc->timeout_cnt++;
> > + //mod_timer(&SCpnt->eh_timeout, jiffies +
> hd->ioc->timeout_delta);
> > + printk(MYIOC_s_WARN_FMT
> > + "to pendingQ: (sc=%p, mf=%p, time=%ld)\n",
> > + hd->ioc->name, SCpnt, mf, jiffies);
> > goto fail;
> > + }
> > +#endif
> >
> > + mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
> > + dmfprintk((MYIOC_s_INFO_FMT "Issued SCSI cmd (%p) mf=%p
> idx=%d\n",
> > + hd->ioc->name, SCpnt, mf, my_idx));
> > + DBG_DUMP_REQUEST_FRAME(mf)
> > return 0;
> >
> > fail:
> > -
> > To unsubscribe from this list: send the line "unsubscribe
> linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> ---end quoted text---
>
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2004-10-21 14:59 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-04 21:33 [PATCH] fusion: streamline ->queuecommand Moore, Eric Dean
2004-10-04 21:57 ` James Bottomley
2004-10-06 15:41 ` Christoph Hellwig
-- strict thread matches above, loose matches on Subject: below --
2004-10-21 14:58 Moore, Eric Dean
2004-10-06 14:46 Gibbons, Terry
2004-10-06 14:58 ` Matthew Wilcox
2004-10-06 14:23 Gibbons, Terry
2004-10-06 14:30 ` Christoph Hellwig
2004-10-06 15:47 ` James Bottomley
2004-10-06 14:00 Gibbons, Terry
2004-10-06 14:13 ` Arjan van de Ven
2004-10-06 14:25 ` Matthew Wilcox
2004-10-05 22:38 Moore, Eric Dean
2004-10-05 23:11 ` James Bottomley
2004-10-05 23:37 ` Patrick Mansfield
2004-10-06 0:48 ` James Bottomley
2004-10-02 8:13 Christoph Hellwig
2004-10-02 13:39 ` Matthew Wilcox
2004-10-02 14:49 ` Christoph Hellwig
2004-10-21 9:21 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).