Re: [Bugme-new] [Bug 9901] New: kernel panic in stex modules (?)
On Wed, 06 Feb 2008 12:26:39 -0600
James Bottomley <[EMAIL PROTECTED]> wrote:
> On Wed, 2008-02-06 at 10:15 -0800, Andrew Morton wrote:
> > On Wed, 6 Feb 2008 09:40:15 -0800 (PST) [EMAIL PROTECTED] wrote:
> >
> > > http://bugzilla.kernel.org/show_bug.cgi?id=9901
> > >
> > >Summary: kernel panic in stex modules (?)
> > >Product: IO/Storage
> > >Version: 2.5
> > > KernelVersion: 2.6.24
> > > Platform: All
> > > OS/Version: Linux
> > > Tree: Mainline
> > > Status: NEW
> > > Severity: normal
> > > Priority: P1
> > > Component: Serial ATA
> > > AssignedTo: [EMAIL PROTECTED]
> > > ReportedBy: [EMAIL PROTECTED]
> > >
> > >
> > > Latest working kernel version: 2.6.23-r6
> > > Earliest failing kernel version: 2.6.24
> > > Distribution: Gentoo
> > > Hardware Environment: Core2D E6600, Asus p5B Dlx, 2G DDR2 667, Promise ST
> > > EX4350
> > > Software Environment: GCC 4.2.3/4.1.2, CFLAGS="-O2"
> > >
> > > Problem Description:
> > > The problem is frequent kernel panics within the same module. Can't say
> > > what it
> > > is, but looks like it is related to dma and promise driver.
> > > The first culprit, the memory, is ok, 8 hours of memtest passed without
> > > errors.
> > > Before, kernel 2.6.23-gentoo-r6, compiled with GCC 4.1.2 worked just
> > > fine, then
> > > after upgrade to 4.2.2 th bug appeared. Upgrade to 2.6.24 didn't solve the
> > > problem. Switching back to GCC 4.1.2 made things better for a moment,
> > > crashes
> > > became less frequent and I thought compiler was the cause. But today
> > > system
> > > crashed again with same symptoms.
> > > Sorry, but I can't save crash log, so I'll provide screen "shot":
> > > http://img238.imageshack.us/my.php?image=p2030030ki1.jpg
> > >
> > > Steps to reproduce:
> > > Boot, start FTP-server, load RAID with heavy input, in some hours it will
> > > crash. With pure reads system can run several days, heavy write load
> > > kills it
> > > much too easier.
> > >
> >
> > The supertrak driver has regressed in 2.6.24. And
> >
> > commit 9cb83c7529d929c00f37d821daed1942a1b20602
> > Author: FUJITA Tomonori <[EMAIL PROTECTED]>
> > Date: Tue Oct 16 11:24:32 2007 +0200
> >
> > [SCSI] add use_sg_chaining option to scsi_host_template
> >
> > looks a likely candidate.
> >
> > And this:
> >
> > commit d3f46f39b7092594b498abc12f0c73b0b9913bde
> > Author: James Bottomley <[EMAIL PROTECTED]>
> > Date: Tue Jan 15 11:11:46 2008 -0600
> >
> > [SCSI] remove use_sg_chaining
> >
> > from 2.6.25 looks to be a likely fix for it. Should it be backported?
>
> If the patch you identify is the culprit, mine can't be the fix ... and
> it should also be present in git head.
>
> The BUG_ON is here: isn't it?
>
> static inline void
> dma_unmap_sg(struct device *hwdev, struct scatterlist *sg, int nents,
>int direction)
> {
> BUG_ON(!valid_dma_direction(direction));
> ^^^
> dma_ops->unmap_sg(hwdev, sg, nents, direction);
> }
>
> stex only does scsi_dma_unmap(), so something looks to have tampered
> with the cmnd->sc_data_direction somehow ... and I can't see how.
Surely, someone changes the cmnd->sc_data_direction, or else we should
be hit by dma_map_sg before dma_unmap_sg:
static inline int
dma_map_sg(struct device *hwdev, struct scatterlist *sg, int nents, int
direction)
{
BUG_ON(!valid_dma_direction(direction));
return dma_ops->map_sg(hwdev, sg, nents, direction);
}
-
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] Marvell 6440 SAS/SATA driver
Ke Wei wrote: Added support for hotplug and wide port. Signed-off-by: Ke Wei <[EMAIL PROTECTED]> --- drivers/scsi/mvsas.c | 445 ++ 1 files changed, 339 insertions(+), 106 deletions(-) Technically speaking, everything is looking great so far. We need to correct one process problem, and we should be able to get your work into the 'mvsas' branch of scsi-misc-2.6.git... and soon thereafter hopefully into the official upstream kernel. The process problem is... when making revisions, we need to get the full patch each time. In this case, each patch should be diff'd against the current version in the 'mvsas' branch: version 0.1. The succession of emails would look like this: Email #1: Subject: [PATCH] mvsas: make it work Convert the skeleton mvsas driver into a real, working driver. Currently, the following works: SAS, SAS expanders, SAS wide ports SATA devices, SATAPI Signed-off-by: Ke Wei <[EMAIL PROTECTED]> And then James, myself, other reviewers reply. Or, you add some new features like hotplugging. Each time, you must regenerate a full patch against the most git repository revision: If a previous patch of yours, version "0.2", has been applied to git, then you would create your patch against version 0.2. If a previous patch of yours has NOT yet been applied to git, then you would create your patch against version 0.1. Thus, in this case, version 0.1 is in 'mvsas' branch, so your second email would then be Email #2: Subject: [PATCH v2] mvsas: make it work Convert the skeleton mvsas driver into a real, working driver. Currently, the following works: SAS, SAS expanders, SAS wide ports SATA devices, SATAPI Signed-off-by: Ke Wei <[EMAIL PROTECTED]> --- Changes since last posting (version 0.2): - fix coding problem And then you continue your work, and add another revision while everyone else is sleeping, the third email would look like: Email #3: Subject: [PATCH v3] mvsas: make it work Convert the skeleton mvsas driver into a real, working driver. Currently, the following works: SAS, SAS expanders, SAS wide ports SATA devices, SATAPI Signed-off-by: Ke Wei <[EMAIL PROTECTED]> --- Changes since last posting (version 0.3): - add hotplug support - add wide port support Changes since last posting (version 0.2): - fix coding problem Thus, you always create a patch against the most recent source code in the git repository. It is common for patches to go through a few revisions on the mailing list, before it is applied to the git repository. So anyway... send a patch against the latest #mvsas (version 0.1), and that patch should go in rapidly. Thanks! And keep up the good work, Jeff - 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: [PATCHSET #upstream] block/libata: update and use block layer padding and draining
Tejun Heo wrote: This patchset updates block layer padding and draining support and make libata use it. It's based on James Bottomley's initial work and, of the five, the last two patches are from James with some modifications. Please read the following thread for more info. http://thread.gmane.org/gmane.linux.scsi/37185 This patchset is on top of upstream (a6af42fc9a12165136d82206ad52f18c5955ce87) + kill-n_iter-and-fix-fsl patch [1] ACK patchset... lets definitely get these fixes upstream. Once Jens is happy, I would prefer the merge the lot upstream, if that is OK with everyone involved? Jeff - 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] bugfix for an underflow condition in usb storage & isd200.c
On Wed, 2008-02-06 at 18:25 -0500, Alan Stern wrote: > On Wed, 6 Feb 2008, James Bottomley wrote: > > > > What we're talking about is a routine that provides drivers a simple > > > way to access the data in a scatter-gather buffer (which may lie in > > > highmem or otherwise not be easily reachable). The idea is that some > > > commands are emulated by the driver rather than carried out by the > > > device, and the driver needs some way to stick the results in the > > > transfer buffer. > > > > Isn't that what scsi_kmap_sg() is designed for ... or do you need > > something slightly different? > > I don't know -- I've never heard of scsi_kmap_sg(). And it doesn't > appear to exist anywhere in my kernel source. > > Did you mean scsi_kmap_atomic_sg()? Yes .. I replied from memory rather than looking in the source. > That appears to do only part of > what usb_stor_access_xfer_buf() does. In fact, all it does is map a > single page. Um, it does everything you asked about above. It's designed as a helper for SCSI devices that need to modify data for automatically generated returns (RAID devices and INQUIRY strings for instance). It's also used by some of the less well automated devices that might need to do an effective PIO feed on DMA engine halts. It allows you to address a sg list by offset and length (although it will adjust the length if there's a mapping problem). Since the only way to guarantee access to highmem is by kmap_atomic (which has limited slots), any API that does this sort of thing is necessarily page based and does single page mappings at a time. Why don't you tell me what you think is missing rather than me having to dig around in the usb sources to try to work it out. 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: [PATCHSET #upstream] block/libata: update and use block layer padding and draining
On Tue, 2008-02-05 at 16:53 +0900, Tejun Heo wrote: > This patchset updates block layer padding and draining support and > make libata use it. It's based on James Bottomley's initial work and, > of the five, the last two patches are from James with some > modifications. > > Please read the following thread for more info. > > http://thread.gmane.org/gmane.linux.scsi/37185 > > This patchset is on top of > > upstream (a6af42fc9a12165136d82206ad52f18c5955ce87) > + kill-n_iter-and-fix-fsl patch [1] This certainly fixes the SATAPI panic on aic94xx (finally got the machine with it plugged into the expanders to boot). 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] bugfix for an underflow condition in usb storage & isd200.c
On Wed, 6 Feb 2008, James Bottomley wrote: > > What we're talking about is a routine that provides drivers a simple > > way to access the data in a scatter-gather buffer (which may lie in > > highmem or otherwise not be easily reachable). The idea is that some > > commands are emulated by the driver rather than carried out by the > > device, and the driver needs some way to stick the results in the > > transfer buffer. > > Isn't that what scsi_kmap_sg() is designed for ... or do you need > something slightly different? I don't know -- I've never heard of scsi_kmap_sg(). And it doesn't appear to exist anywhere in my kernel source. Did you mean scsi_kmap_atomic_sg()? That appears to do only part of what usb_stor_access_xfer_buf() does. In fact, all it does is map a single page. 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
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Wed, 2008-02-06 at 17:18 -0500, Alan Stern wrote: > On Wed, 6 Feb 2008, Matthew Dharm wrote: > > > Maybe this is a crazy question, but... > > > > Why is this not in the SCSI core? > > Or even in the block core? > > > It's hardly USB-specific, and I'm > > willing to bet that there are other HCDs (at least spb2) which need to do > > this sort of thing... > > James, do you have any idea? > > What we're talking about is a routine that provides drivers a simple > way to access the data in a scatter-gather buffer (which may lie in > highmem or otherwise not be easily reachable). The idea is that some > commands are emulated by the driver rather than carried out by the > device, and the driver needs some way to stick the results in the > transfer buffer. Isn't that what scsi_kmap_sg() is designed for ... or do you need something slightly different? 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: Kernel Panic in MPT SAS on 2.6.24 (and 2.6.23.14, 2.6.23.9)
On Wed, 6 Feb 2008 22:04:26 +0100
Maximilian Wilhelm <[EMAIL PROTECTED]> wrote:
> Hi!
>
> While installing my new firewall I got the following kernel panic in
> the MPT SAS driver which I need for the disks.
>
> The first kernel I bootet was 2.6.23.14 which did panic so I tried a
> 2.6.24 which panics, too. Our usual FAI kernel (2.6.23.9) is also
> affected.
>
> If there is any information you may need to track this down, please
> let me know.
>
> I've put the .config to http://files.rfc2324.org/mptsas_panic/2.6.24-config
> to limit the size of this mail.
>
> ...
>
> ide-floppy driver 0.99.newide
> aic94xx: Adaptec aic94xx SAS/SATA driver version 1.0.3 loaded
> megaraid cmm: 2.20.2.7 (Release Date: Sun Jul 16 00:01:03 EST 2006)
> megaraid: 2.20.5.1 (Release Date: Thu Nov 16 15:32:35 EST 2006)
> megasas: 00.00.03.10-rc5 Thu May 17 10:09:32 PDT 2007
> Driver 'sd' needs updating - please use bus_type methods
> Fusion MPT base driver 3.04.06
> Copyright (c) 1999-2007 LSI Corporation
> Fusion MPT SAS Host driver 3.04.06
> mptbase: ioc0: Initiating bringup
> ioc0: LSISAS1068E B3: Capabilities={Initiator}
> scsi0 : ioc0: LSISAS1068E B3, FwRev=00142e00h, Ports=1, MaxQ=511, IRQ=16
> scsi 0:0:0:0: Direct-Access SEAGATE ST973402SS S207 PQ: 0 ANSI: 5
> scsi 0:0:1:0: Direct-Access SEAGATE ST973402SS S207 PQ: 0 ANSI: 5
> BUG: unable to handle kernel NULL pointer dereference at virtual address
> 0010
> printing eip: c02c0b38 *pde =
> Oops: [#1] SMP
> Modules linked in:
>
> Pid: 1, comm: swapper Not tainted (2.6.24 #1)
> EIP: 0060:[] EFLAGS: 00010246 CPU: 1
> EIP is at mptsas_probe_expander_phys+0x51/0x4a2
> EAX: 0010 EBX: f7457ec0 ECX: f7c3fd9c EDX: 0004
> ESI: f7fe7800 EDI: f7fe7800 EBP: f7fe7904 ESP: f7c3fe18
> DS: 007b ES: 007b FS: 00d8 GS: SS: 0068
> Process swapper (pid: 1, ti=f7c3e000 task=f7c22ab0 task.ti=f7c3e000)
> Stack: 00200200 fffefd74 c02b9cc8 f7fe7800 c04c5280
> f7c3fecc
>376b1000 0001 00100100 00200200
>
>00200200 fffefd74 c02b9cc8 f7fe7800 c04c5280 f7c3fe8c 376b1000
> 0001
> Call Trace:
> [] mpt_timer_expired+0x0/0x5c
> [] mpt_timer_expired+0x0/0x5c
> [] ide_wait_cmd+0x90/0xa0
> [] mptsas_probe+0x38a/0x40b
> [] sysfs_create_link+0xb7/0xf9
> [] pci_device_probe+0x36/0x57
> [] driver_probe_device+0xde/0x15c
> [] klist_next+0x4b/0x6b
> [] __driver_attach+0x0/0x79
> [] __driver_attach+0x46/0x79
> [] bus_for_each_dev+0x33/0x55
> [] driver_attach+0x16/0x18
> [] __driver_attach+0x0/0x79
> [] bus_add_driver+0x6d/0x197
> [] __pci_register_driver+0x48/0x74
> [] mptsas_init+0xbf/0xd6
> [] kernel_init+0x140/0x2a2
> [] ret_from_fork+0x6/0x1c
> [] kernel_init+0x0/0x2a2
> [] kernel_init+0x0/0x2a2
> [] kernel_thread_helper+0x7/0x10
> ===
> Code: 85 c0 0f 84 68 04 00 00 8b 54 24 1c 8b 02 89 04 24 31 c9 89 da 89 f8 e8
> 2b f2 ff ff 89 44 24 2c 85 c0 8b 43 0c 0f 85 39 04 00 00 <0f> b7 00 8b 74 24
> 1c 89 06 8d 87 24 05 00 00 89 44 24 20 e8 5b
> EIP: [] mptsas_probe_expander_phys+0x51/0x4a2 SS:ESP 0068:f7c3fe18
> ---[ end trace 50b3e7147499e641 ]---
> Kernel panic - not syncing: Attempted to kill init!
>
Thanks. Cc's added...
-
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] bugfix for an underflow condition in usb storage & isd200.c
On Wed, 6 Feb 2008, Matthew Dharm wrote: > Maybe this is a crazy question, but... > > Why is this not in the SCSI core? Or even in the block core? > It's hardly USB-specific, and I'm > willing to bet that there are other HCDs (at least spb2) which need to do > this sort of thing... James, do you have any idea? What we're talking about is a routine that provides drivers a simple way to access the data in a scatter-gather buffer (which may lie in highmem or otherwise not be easily reachable). The idea is that some commands are emulated by the driver rather than carried out by the device, and the driver needs some way to stick the results in the transfer buffer. 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
Re: [PATCH 1/1] aacraid: do not set valid bit in sense information
--- On Wed, 2/6/08, Salyzyn, Mark <[EMAIL PROTECTED]> wrote:
> From: Salyzyn, Mark <[EMAIL PROTECTED]>
> Subject: [PATCH 1/1] aacraid: do not set valid bit in sense information
> To: "linux-scsi@vger.kernel.org"
> Cc: "Tony Battersby" <[EMAIL PROTECTED]>, "[EMAIL PROTECTED]" <[EMAIL
> PROTECTED]>, "Mike Snitzer" <[EMAIL PROTECTED]>
> Date: Wednesday, February 6, 2008, 1:54 PM
> Luben Tuikov [mailto:[EMAIL PROTECTED] sez:
> > Just as in your case and Tony's case, which I
> presume
> > uses the same RAID firmware vendor, it would've
> > probably been better if the RAID firmware vendor
> > fixed the firmware to not set the VALID bit if the
> > INFORMATION field is not valid.
>
> Point taken regarding the aacraid driver. Dropped the VALID
> bit, and then did some cleanup/simplification of the
> set_sense procedure and the associated parameters. Mike did
> some preliminary tests when the VALID bit was dropped before
> the 'Re: [PATCH] [SCSI] sd: make error handling more
> robust' patches came on the scene. The change in the
> SCSI subsystem does make this enclosed aacraid patch
> unnecessary, so this aacraid patch is merely post battle
> ground cleanup. If the simplification is an issue,
> repugnant, too much for a back-port to the stable trees or
> clouds the point, this patch could be happily distilled
> down to:
Sounds good.
Luben
>
> diff -ru a/drivers/scsi/aacraid/aachba.c
> b/drivers/scsi/aacraid/aachba.c
> --- a/drivers/scsi/aacraid/aachba.c 2008-02-06
> 16:26:45.834938955 -0500
> +++ b/drivers/scsi/aacraid/aachba.c 2008-02-06
> 16:32:01.109035329 -0500
> @@ -865,7 +865,7 @@
> u32 residue)
> {
> -sense_buf[0] = 0xF0;/* Sense data valid, err
> code 70h (current error) */
> +sense_buf[0] = 0x70;/* Sense data invalid, err
> code 70h (current error) */
> sense_buf[1] = 0; /* Segment number, always
> zero */
>
> if (incorrect_length) {
>
> This patch is against current scsi-misc-2.6
>
> ObligatoryDisclaimer: Please accept my condolences
> regarding Outlook's handling of patch attachments.
> Please use the attached patch and not any inline content.
>
> Signed-off-by: Mark Salyzyn <[EMAIL PROTECTED]>
>
> drivers/scsi/aacraid/aachba.c | 81
> +++---
> 1 file changed, 30 insertions(+), 51 deletions(-)
>
> Sincerely -- Mark Salyzyn
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] aacraid: do not set valid bit in sense information
Luben Tuikov [mailto:[EMAIL PROTECTED] sez:
> Just as in your case and Tony's case, which I presume
> uses the same RAID firmware vendor, it would've
> probably been better if the RAID firmware vendor
> fixed the firmware to not set the VALID bit if the
> INFORMATION field is not valid.
Point taken regarding the aacraid driver. Dropped the VALID bit, and then did
some cleanup/simplification of the set_sense procedure and the associated
parameters. Mike did some preliminary tests when the VALID bit was dropped
before the 'Re: [PATCH] [SCSI] sd: make error handling more robust' patches
came on the scene. The change in the SCSI subsystem does make this enclosed
aacraid patch unnecessary, so this aacraid patch is merely post battle ground
cleanup. If the simplification is an issue, repugnant, too much for a back-port
to the stable trees or clouds the point, this patch could be happily distilled
down to:
diff -ru a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
--- a/drivers/scsi/aacraid/aachba.c 2008-02-06 16:26:45.834938955 -0500
+++ b/drivers/scsi/aacraid/aachba.c 2008-02-06 16:32:01.109035329 -0500
@@ -865,7 +865,7 @@
u32 residue)
{
-sense_buf[0] = 0xF0;/* Sense data valid, err code 70h (current
error) */
+sense_buf[0] = 0x70;/* Sense data invalid, err code 70h (current
error) */
sense_buf[1] = 0; /* Segment number, always zero */
if (incorrect_length) {
This patch is against current scsi-misc-2.6
ObligatoryDisclaimer: Please accept my condolences regarding Outlook's handling
of patch attachments. Please use the attached patch and not any inline content.
Signed-off-by: Mark Salyzyn <[EMAIL PROTECTED]>
drivers/scsi/aacraid/aachba.c | 81 +++---
1 file changed, 30 insertions(+), 51 deletions(-)
Sincerely -- Mark Salyzyn
aacraid_set_sense.patch
Description: aacraid_set_sense.patch
[PATCH 1/1] aacraid: add optional MSI support.
Added support for MSI utilizing the aacraid.msi=1 parameter. This patch adds
some localized or like-minded janitor fixes. Since the default is disabled,
there is no impact on the code paths unless the customer wishes to experiment
with the MSI performance.
This patch is against current scsi-misc-2.6
ObligatoryDisclaimer: Please accept my condolences regarding Outlook's handling
of patch attachments. Please use the attached file to patch, the inlined patch
is a hand filtered diff -rub unusable to patch but usable for inspection
clarity.
Signed-off-by: Mark Salyzyn <[EMAIL PROTECTED]>
drivers/scsi/aacraid/aachba.c | 56 ++---
drivers/scsi/aacraid/aacraid.h |2 +
drivers/scsi/aacraid/linit.c | 32 ---
drivers/scsi/aacraid/rx.c |5 ++-
drivers/scsi/aacraid/sa.c |4 +-
5 files changed, 66 insertions(+), 33 deletions(-)
diff -rub a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
--- a/drivers/scsi/aacraid/aachba.c 2008-02-06 15:33:31.356341806 -0500
+++ b/drivers/scsi/aacraid/aachba.c 2008-02-06 15:52:33.988699503 -0500
@@ -144,51 +144,77 @@
*/
static int nondasd = -1;
-static int aac_cache = 0;
+static int aac_cache;
static int dacmode = -1;
-
+int aac_msi;
int aac_commit = -1;
int startup_timeout = 180;
int aif_timeout = 120;
@@ -159,6 +153,9 @@
MODULE_PARM_DESC(dacmode, "Control whether dma addressing is using 64 bit DAC.
0=off, 1=on");
module_param_named(commit, aac_commit, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(commit, "Control whether a COMMIT_CONFIG is issued to the
adapter for foreign arrays.\nThis is typically needed in systems that do not
have a BIOS. 0=off, 1=on");
+module_param_named(msi, aac_msi, int, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(msi, "IRQ handling."
+ " 0=PIC(default), 1=MSI, 2=MSI-X(unsupported, uses MSI)");
module_param(startup_timeout, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(startup_timeout, "The duration of time in seconds to wait for
adapter to have it's kernel up and\nrunning. This is typically adjusted for
large systems that do not have a BIOS.");
module_param(aif_timeout, int, S_IRUGO|S_IWUSR);
@@ -181,7 +184,7 @@
module_param(expose_physicals, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(expose_physicals, "Expose physical components of the arrays.
- 1=protect 0=off, 1=on");
-int aac_reset_devices = 0;
+int aac_reset_devices;
module_param_named(reset_devices, aac_reset_devices, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(reset_devices, "Force an adapter reset at initialization.");
diff -rub a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
--- a/drivers/scsi/aacraid/aacraid.h2008-02-06 15:33:31.357341679 -0500
+++ b/drivers/scsi/aacraid/aacraid.h2008-02-06 15:44:11.908258253 -0500
@@ -1026,6 +1026,7 @@
u8 raw_io_64;
u8 printf_enabled;
u8 in_reset;
+ u8 msi;
};
#define aac_adapter_interrupt(dev) \
@@ -1881,6 +1882,7 @@
extern int aif_timeout;
extern int expose_physicals;
extern int aac_reset_devices;
+extern int aac_msi;
extern int aac_commit;
extern int update_interval;
extern int check_interval;
diff -rub a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
--- a/drivers/scsi/aacraid/linit.c 2008-02-06 15:33:31.358341553 -0500
+++ b/drivers/scsi/aacraid/linit.c 2008-02-06 15:44:11.908258253 -0500
@@ -1039,6 +1039,8 @@
aac_send_shutdown(aac);
aac_adapter_disable_int(aac);
free_irq(aac->pdev->irq, aac);
+ if (aac->msi)
+ pci_disable_msi(aac->pdev);
}
static int __devinit aac_probe_one(struct pci_dev *pdev,
diff -rub a/drivers/scsi/aacraid/rx.c b/drivers/scsi/aacraid/rx.c
--- a/drivers/scsi/aacraid/rx.c 2008-02-06 15:33:31.359341426 -0500
+++ b/drivers/scsi/aacraid/rx.c 2008-02-06 15:44:11.909258127 -0500
@@ -625,8 +625,11 @@
if (aac_init_adapter(dev) == NULL)
goto error_iounmap;
aac_adapter_comm(dev, dev->comm_interface);
- if (request_irq(dev->scsi_host_ptr->irq, dev->a_ops.adapter_intr,
+ dev->msi = aac_msi && !pci_enable_msi(dev->pdev);
+ if (request_irq(dev->pdev->irq, dev->a_ops.adapter_intr,
IRQF_SHARED|IRQF_DISABLED, "aacraid", dev) < 0) {
+ if (dev->msi)
+ pci_disable_msi(dev->pdev);
printk(KERN_ERR "%s%d: Interrupt unavailable.\n",
name, instance);
goto error_iounmap;
diff -rub a/drivers/scsi/aacraid/sa.c b/drivers/scsi/aacraid/sa.c
--- a/drivers/scsi/aacraid/sa.c 2008-02-06 15:33:31.359341426 -0500
+++ b/drivers/scsi/aacraid/sa.c 2008-02-06 15:44:11.909258127 -0500
@@ -385,7 +385,7 @@
if(aac_init_adapter(dev) == NULL)
goto error_irq;
- if (request_irq(dev->scsi_host_ptr->irq, dev->a_ops.adapter_intr,
+ if (request_irq(dev->pde
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Wed, Feb 06, 2008 at 03:23:39PM -0500, Alan Stern wrote: > On Tue, 5 Feb 2008, Matthew Dharm wrote: > > > Six of one and a half-dozen of the other. All we're arguing over is the > > definition of "correct behavior" here. You want to change the API so that > > overrun is acceptable and handled; I prefer calling it a Bad Thing(tm). > > > > We both agree that the code shouldn't run off the end of the s-g list. > > > > Since you've already committed to updating the patch, then we can do it > > your way. Just make sure it's very very clear in the comments. > > Okay, here's my version. It makes some significant changes to the > interface for usb_stor_access_xfer_buf() -- in particular, the context > information is now stored in an opaque structure rather than in ad-hoc > local variables. All the callers are updated to use the new interface. Maybe this is a crazy question, but... Why is this not in the SCSI core? It's hardly USB-specific, and I'm willing to bet that there are other HCDs (at least spb2) which need to do this sort of thing... Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver YOU SEE!!?? It's like being born with only one nipple! -- Erwin User Friendly, 10/19/1998 pgpLv1iX6JiHu.pgp Description: PGP signature
Re: [PATCH] sr: update to follow tray status correctly
James Bottomley wrote: However, I think you're right, the vanilla TUR does eat NOT_READY for removable media, which CDs are. Does this fix it? Works great, thanks! Daniel - 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 3/9] scsi_dh: scsi handling of REQ_LB_OP_TRANSITION
On Wed, 2008-02-06 at 11:00 -0800, Mike Anderson wrote:
> James Bottomley <[EMAIL PROTECTED]> wrote:
> >
> > On Fri, 2008-02-01 at 14:00 -0600, Mike Christie wrote:
> > > Chandra Seetharaman wrote:
> > > > @@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
> > > > static void scsi_softirq_done(struct request *rq)
> > > > {
> > > > struct scsi_cmnd *cmd = rq->completion_data;
> > > > - unsigned long wait_for = (cmd->allowed + 1) *
> > > > cmd->timeout_per_command;
> > > > int disposition;
> > > > + struct request_queue *q;
> > > > + unsigned long wait_for, flags;
> > > >
> > > > + if (blk_linux_request(rq)) {
> > > > + q = rq->q;
> > > > + spin_lock_irqsave(q->queue_lock, flags);
> > > > + /*
> > > > +* we always return 1 and the caller should
> > > > +* check rq->errors for the complete status
> > > > +*/
> > > > + end_that_request_last(rq, 1);
> > > > + spin_unlock_irqrestore(q->queue_lock, flags);
> > > > + return;
> > > > + }
> > > > +
> > > > +
> > > > + wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > > > INIT_LIST_HEAD(&cmd->eh_entry);
> > > >
> > > .
> > >
> > > > +
> > > > /*
> > > > * Function:scsi_request_fn()
> > > > *
> > > > @@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
> > > > * accept it.
> > > > */
> > > > req = elv_next_request(q);
> > > > - if (!req || !scsi_dev_queue_ready(q, sdev))
> > > > + if (!req)
> > > > + break;
> > > > +
> > > > + /*
> > > > +* We do not account for linux blk req in the device
> > > > +* or host busy accounting because it is not necessarily
> > > > +* a scsi command that is sent to some object. The lower
> > > > +* level can translate it into a request/scsi_cmnd, if
> > > > +* necessary, and then queue that up using
> > > > REQ_TYPE_BLOCK_PC.
> > > > +*/
> > > > + if (blk_linux_request(req)) {
> > > > + blkdev_dequeue_request(req);
> > > > + scsi_execute_blk_linux_cmd(req);
> > > > + continue;
> > > > + }
> > > > +
> > > > + if (!scsi_dev_queue_ready(q, sdev))
> > > > break;
> > >
> > > I think these two pieces are one of the reasons I have not pushed the
> > > patches. I thought the completion and execution pieces here are a little
> > > ugly and seem to just wedge themselves in where they want to be.
> > >
> > > Is there any way to make the insertion of non-scsi commands more common?
> > > Do we have the code for being able to send requests directly to
> > > something like a fc rport done? Could we maybe inject these special
> > > commands to the hw handler using something similar to how bsg would send
> > > non scsi commands to weird objects (objects like rport, sessions, and
> > > not devices we traditionally associated with queues like scsi_devices).
> > > Just a thought with no code :) that is why the ugly code existed still :)
> >
> > We sort of do. The bsg code in scsi_transport_sas to send SMP frames to
> > expander devices would be an example of non-scsi commands going via a
> > mechanism other than being encapsulated in SCSI. I don't know if that's
> > the complete solution in this case, but you could investigate it.
>
> I looked at the bsg code in scsi_transport_sas and all I see it doing is
> calling blk_init_queue to set the request_fn. The request_fn
> (*smp_request) just processes one cmd_type. Is there code is another tree
> that has more processing?
No ... that's it. It's designed to expose a frame driven SMP
communication channel to expanders via a block tap.
Part of the problem seems to be that your current code is very much
trying to do this in-band. A block tap like the SMP handlers are
effectively out of band
> A idea to allow for more control / flexibility cmd_type handlers could be
> added inside request_fn, prep_rq_fn, softirq_done_fn.
>
> I thought about this being at a higher level in the block layer, but it
> would be hard to handle the request_fn cleanly at the high level. The
> localized change would reduce impact on users who do not want or need per
> cmd_type handlers.
But this type of thinking does lead to a lot of apparent nastiness
inside your actual handlers. Trying to do all of this in-band has you
doing a lot of callback driven async I/O stuff using
blk_execute_rq_nowait(). It might be a lot cleaner to do it out of band
on a thread using the standard waiting interfaces.
James
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo inf
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Tue, 5 Feb 2008, Matthew Dharm wrote:
> Six of one and a half-dozen of the other. All we're arguing over is the
> definition of "correct behavior" here. You want to change the API so that
> overrun is acceptable and handled; I prefer calling it a Bad Thing(tm).
>
> We both agree that the code shouldn't run off the end of the s-g list.
>
> Since you've already committed to updating the patch, then we can do it
> your way. Just make sure it's very very clear in the comments.
Okay, here's my version. It makes some significant changes to the
interface for usb_stor_access_xfer_buf() -- in particular, the context
information is now stored in an opaque structure rather than in ad-hoc
local variables. All the callers are updated to use the new interface.
Functionally it's almost the same as before. It's more careful about
not letting callers overrun the transfer buffer, and it can now set the
SCSI residue when a copy is finished.
This patch applies to 2.6.24. If it's acceptable I'll send a patch for
2.6.25-rc1 (once it has been released), complete with Changelog and
a Signed-off-by: line.
Note that this has been compile-tested only! Verification that it
works okay would be appreciated.
Alan Stern
Index: 2.6.24/drivers/usb/storage/protocol.h
===
--- 2.6.24.orig/drivers/usb/storage/protocol.h
+++ 2.6.24/drivers/usb/storage/protocol.h
@@ -48,13 +48,23 @@ extern void usb_stor_ufi_command(struct
extern void usb_stor_transparent_scsi_command(struct scsi_cmnd*,
struct us_data*);
-/* struct scsi_cmnd transfer buffer access utilities */
+/* Utility routines for accessing data in scsi_cmnd transfer buffers */
enum xfer_buf_dir {TO_XFER_BUF, FROM_XFER_BUF};
+struct xfer_buf_params {
+ struct scsi_cmnd*srb;
+ struct scatterlist *sg;
+ enum xfer_buf_dir dir;
+ unsigned intoffset;
+ unsigned intcount;
+};
+
+extern void usb_stor_init_xfer_buf(struct xfer_buf_params *xbp,
+ struct us_data *us, enum xfer_buf_dir dir);
extern unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,
- unsigned int buflen, struct scsi_cmnd *srb, struct scatterlist **,
- unsigned int *offset, enum xfer_buf_dir dir);
+ unsigned int buflen, struct xfer_buf_params *xbp);
+extern void usb_stor_finish_xfer_buf(struct xfer_buf_params *xbp);
extern void usb_stor_set_xfer_buf(unsigned char *buffer,
- unsigned int buflen, struct scsi_cmnd *srb);
+ unsigned int buflen, struct us_data *us);
#endif
Index: 2.6.24/drivers/usb/storage/protocol.c
===
--- 2.6.24.orig/drivers/usb/storage/protocol.c
+++ 2.6.24/drivers/usb/storage/protocol.c
@@ -148,33 +148,66 @@ void usb_stor_transparent_scsi_command(s
* Scatter-gather transfer buffer access routines
***/
-/* Copy a buffer of length buflen to/from the srb's transfer buffer.
- * (Note: for scatter-gather transfers (srb->use_sg > 0), srb->request_buffer
- * points to a list of s-g entries and we ignore srb->request_bufflen.
- * For non-scatter-gather transfers, srb->request_buffer points to the
- * transfer buffer itself and srb->request_bufflen is the buffer's length.)
- * Update the *index and *offset variables so that the next copy will
- * pick up from where this one left off. */
+/* usb_stor_init_xfer_buf - initialize an xfer_buf_params structure
+ *
+ * Store in @xbp the values needed for accessing a transfer buffer
+ * through several calls to usb_stor_access_xfer_buf(). The sg
+ * and offset members are initialized to point to the start of the
+ * transfer buffer for @us->srb.
+ */
+void usb_stor_init_xfer_buf(struct xfer_buf_params *xbp,
+ struct us_data *us, enum xfer_buf_dir dir)
+{
+ xbp->srb = us->srb;
+ xbp->sg = (struct scatterlist *) xbp->srb->request_buffer;
+ xbp->dir = dir;
+ xbp->offset = 0;
+ xbp->count = 0;
+}
+/* usb_stor_access_xfer_buf - copy data to/from a scsi_cmnd's transfer buffer
+ *
+ * Copy data between @buffer and @xbp->srb's transfer buffer. Data
+ * can be copied incrementally through successive calls to this routine;
+ * each copy will pick up from the location in the transfer buffer where
+ * the previous call left off.
+ *
+ * The direction of the copy is as specified in usb_stor_init_xfer_buf(),
+ * which must have been called earlier.
+ *
+ * The amount of data to copy is given by @buflen, but it is limited
+ * by @xbp->srb->request_bufflen as well as by the total length of
+ * the scatter-gather buffers (if @xbp->srb uses scatter-gather).
+ * It's not an error for the caller to try copying too much data;
+ * when that happens only the data that fits will be copied.
+ *
+ * The sg and offset members of @xbp are updated to point to the next
+ * locatio
Re: ABORT_TASK defined in aic94xx_sas.h
--- On Wed, 2/6/08, Boaz Harrosh <[EMAIL PROTECTED]> wrote:
> include/scsi/scsi.h as a definition:
> #define ABORT_TASK 0x0d
>
> on the other hand drivers/scsi/aic94xx/aic94xx_sas.h has:
> #define ABORT_TASK 0x03
>
> am I right in thinking that aic94xx_sas.h is wrong in
> polluting the global name-space?
>
> If you ask me aic94xx_sas.h is a global name-space
> minefield
LOL, "global name-space minefield" -- that's funny.
Yeah, I didn't think the aic94xx_sas.h would be
used the way it is (or that the SAS Stack would become
a "pimple" as opposed to a layer as originally defined).
In macro name definitions, I tried to stay as close
as possible to the specs of the chip.
Anyway, I ack this patch. See below.
(Maybe scsi.h::ABORT_TASK should go away for the better
defined scsi.h::TMF_ABORT_TASK.)
>
> (This gives me problems when trying to pull in scsi_eh.h
> into
> aic94xx source files)
>
> perhaps:
> ---
> From: Boaz Harrosh <[EMAIL PROTECTED]>
> Date: Wed, 6 Feb 2008 15:35:37 +0200
> Subject: [PATCH] aic94xx_sas: avoid conflict with scsi.h
>
> drivers/scsi/aic94xx/aic94xx_sas.h would redefine
> ABORT_TASK
> as a different value.
>
> Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
Acked-by: Luben Tuikov <[EMAIL PROTECTED]>
Luben
> ---
> drivers/scsi/aic94xx/aic94xx_sas.h |2 +-
> drivers/scsi/aic94xx/aic94xx_tmf.c |2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/aic94xx/aic94xx_sas.h
> b/drivers/scsi/aic94xx/aic94xx_sas.h
> index fa7c529..912e6b7 100644
> --- a/drivers/scsi/aic94xx/aic94xx_sas.h
> +++ b/drivers/scsi/aic94xx/aic94xx_sas.h
> @@ -292,7 +292,7 @@ struct scb_header {
> #define INITIATE_SSP_TASK 0x00
> #define INITIATE_LONG_SSP_TASK 0x01
> #define INITIATE_BIDIR_SSP_TASK 0x02
> -#define ABORT_TASK 0x03
> +#define SCB_ABORT_TASK 0x03
> #define INITIATE_SSP_TMF0x04
> #define SSP_TARG_GET_DATA 0x05
> #define SSP_TARG_GET_DATA_GOOD 0x06
> diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c
> b/drivers/scsi/aic94xx/aic94xx_tmf.c
> index 87b2f6e..b52124f 100644
> --- a/drivers/scsi/aic94xx/aic94xx_tmf.c
> +++ b/drivers/scsi/aic94xx/aic94xx_tmf.c
> @@ -369,7 +369,7 @@ int asd_abort_task(struct sas_task
> *task)
> return -ENOMEM;
> scb = ascb->scb;
>
> - scb->header.opcode = ABORT_TASK;
> + scb->header.opcode = SCB_ABORT_TASK;
>
> switch (task->task_proto) {
> case SAS_PROTOCOL_SATA:
> --
> 1.5.3.3
>
> -
> 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] sr: update to follow tray status correctly
On Wed, 2008-02-06 at 17:09 +, Daniel Drake wrote:
> Hi James,
>
> James Bottomley wrote:
> > It's been a while with no status on this one, so I corrected the patch
> > based on my original input. The attached is what I think is the best
> > way of doing this (I've replaced the home grown test_unit_ready routine
> > with the SCSI one and updated some of the sense check conditions).
>
> Many thanks for following up on this. The user has not responded to my
> request for him to test your patch, so I decided to try it myself. I'm
> using Linus git as of today, I noticed the patch is merged.
>
> It doesn't work for me. On unpatched 2.6.24, I get the behaviour
> previously described - the ioctl always gives CDS_TRAY_OPEN regardless
> of tray status. On 2.6.24-git which includes your patch, I instead
> always get CDS_DISC_OK. I have confirmed that on 2 systems this is
> because scsi_test_unit_ready() is returning 0.
>
> Before I dig further, does this work for you? I have attached the test
> program I am using.
Heh, you assume I have a CD easily to hand in my testing setup. But,
unfortunately it requires a bit of manual intervention to set up the
state ...
However, I think you're right, the vanilla TUR does eat NOT_READY for
removable media, which CDs are. Does this fix it?
James
---
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 50ba492..1332a5c 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -163,6 +163,29 @@ static void scsi_cd_put(struct scsi_cd *cd)
mutex_unlock(&sr_ref_mutex);
}
+/* identical to scsi_test_unit_ready except that it doesn't
+ * eat the NOT_READY returns for removable media */
+int sr_test_unit_ready(struct scsi_device *sdev, struct scsi_sense_hdr *sshdr)
+{
+ int retries = MAX_RETRIES;
+ int the_result;
+ u8 cmd[] = {TEST_UNIT_READY, 0, 0, 0, 0, 0 };
+
+ /* issue TEST_UNIT_READY until the initial startup UNIT_ATTENTION
+* conditions are gone, or a timeout happens
+*/
+ do {
+ the_result = scsi_execute_req (sdev, cmd, DMA_NONE, NULL,
+ 0, sshdr, SR_TIMEOUT,
+ retries--);
+
+ } while (retries > 0 &&
+(!scsi_status_is_good(the_result) ||
+ (scsi_sense_valid(sshdr) &&
+ sshdr->sense_key == UNIT_ATTENTION)));
+ return the_result;
+}
+
/*
* This function checks to see if the media has been changed in the
* CDROM drive. It is possible that we have already sensed a change,
@@ -185,8 +208,7 @@ static int sr_media_change(struct cdrom_device_info *cdi,
int slot)
}
sshdr = kzalloc(sizeof(*sshdr), GFP_KERNEL);
- retval = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES,
- sshdr);
+ retval = sr_test_unit_ready(cd->device, sshdr);
if (retval || (scsi_sense_valid(sshdr) &&
/* 0x3a is medium not present */
sshdr->asc == 0x3a)) {
@@ -733,10 +755,8 @@ static void get_capabilities(struct scsi_cd *cd)
{
unsigned char *buffer;
struct scsi_mode_data data;
- unsigned char cmd[MAX_COMMAND_SIZE];
struct scsi_sense_hdr sshdr;
- unsigned int the_result;
- int retries, rc, n;
+ int rc, n;
static const char *loadmech[] =
{
@@ -758,23 +778,8 @@ static void get_capabilities(struct scsi_cd *cd)
return;
}
- /* issue TEST_UNIT_READY until the initial startup UNIT_ATTENTION
-* conditions are gone, or a timeout happens
-*/
- retries = 0;
- do {
- memset((void *)cmd, 0, MAX_COMMAND_SIZE);
- cmd[0] = TEST_UNIT_READY;
-
- the_result = scsi_execute_req (cd->device, cmd, DMA_NONE, NULL,
- 0, &sshdr, SR_TIMEOUT,
- MAX_RETRIES);
-
- retries++;
- } while (retries < 5 &&
-(!scsi_status_is_good(the_result) ||
- (scsi_sense_valid(&sshdr) &&
- sshdr.sense_key == UNIT_ATTENTION)));
+ /* eat unit attentions */
+ sr_test_unit_ready(cd->device, &sshdr);
/* ask for mode page 0x2a */
rc = scsi_mode_sense(cd->device, 0, 0x2a, buffer, 128,
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 81fbc0b..1e144df 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -61,6 +61,7 @@ int sr_select_speed(struct cdrom_device_info *cdi, int speed);
int sr_audio_ioctl(struct cdrom_device_info *, unsigned int, void *);
int sr_is_xa(Scsi_CD *);
+int sr_test_unit_ready(struct scsi_device *sdev, struct scsi_sense_hdr *sshdr);
/* sr_vendor.c */
void sr_vendor_init(Scsi_CD *);
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index d5cebff..ae87d08 100644
--- a/drivers/scsi/sr_ioctl.c
+
Re: [PATCH 3/9] scsi_dh: scsi handling of REQ_LB_OP_TRANSITION
James Bottomley <[EMAIL PROTECTED]> wrote:
>
> On Fri, 2008-02-01 at 14:00 -0600, Mike Christie wrote:
> > Chandra Seetharaman wrote:
> > > @@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
> > > static void scsi_softirq_done(struct request *rq)
> > > {
> > > struct scsi_cmnd *cmd = rq->completion_data;
> > > - unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > > int disposition;
> > > + struct request_queue *q;
> > > + unsigned long wait_for, flags;
> > >
> > > + if (blk_linux_request(rq)) {
> > > + q = rq->q;
> > > + spin_lock_irqsave(q->queue_lock, flags);
> > > + /*
> > > + * we always return 1 and the caller should
> > > + * check rq->errors for the complete status
> > > + */
> > > + end_that_request_last(rq, 1);
> > > + spin_unlock_irqrestore(q->queue_lock, flags);
> > > + return;
> > > + }
> > > +
> > > +
> > > + wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
> > > INIT_LIST_HEAD(&cmd->eh_entry);
> > >
> > .
> >
> > > +
> > > /*
> > > * Function:scsi_request_fn()
> > > *
> > > @@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
> > >* accept it.
> > >*/
> > > req = elv_next_request(q);
> > > - if (!req || !scsi_dev_queue_ready(q, sdev))
> > > + if (!req)
> > > + break;
> > > +
> > > + /*
> > > + * We do not account for linux blk req in the device
> > > + * or host busy accounting because it is not necessarily
> > > + * a scsi command that is sent to some object. The lower
> > > + * level can translate it into a request/scsi_cmnd, if
> > > + * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
> > > + */
> > > + if (blk_linux_request(req)) {
> > > + blkdev_dequeue_request(req);
> > > + scsi_execute_blk_linux_cmd(req);
> > > + continue;
> > > + }
> > > +
> > > + if (!scsi_dev_queue_ready(q, sdev))
> > > break;
> >
> > I think these two pieces are one of the reasons I have not pushed the
> > patches. I thought the completion and execution pieces here are a little
> > ugly and seem to just wedge themselves in where they want to be.
> >
> > Is there any way to make the insertion of non-scsi commands more common?
> > Do we have the code for being able to send requests directly to
> > something like a fc rport done? Could we maybe inject these special
> > commands to the hw handler using something similar to how bsg would send
> > non scsi commands to weird objects (objects like rport, sessions, and
> > not devices we traditionally associated with queues like scsi_devices).
> > Just a thought with no code :) that is why the ugly code existed still :)
>
> We sort of do. The bsg code in scsi_transport_sas to send SMP frames to
> expander devices would be an example of non-scsi commands going via a
> mechanism other than being encapsulated in SCSI. I don't know if that's
> the complete solution in this case, but you could investigate it.
I looked at the bsg code in scsi_transport_sas and all I see it doing is
calling blk_init_queue to set the request_fn. The request_fn
(*smp_request) just processes one cmd_type. Is there code is another tree
that has more processing?
A idea to allow for more control / flexibility cmd_type handlers could be
added inside request_fn, prep_rq_fn, softirq_done_fn.
I thought about this being at a higher level in the block layer, but it
would be hard to handle the request_fn cleanly at the high level. The
localized change would reduce impact on users who do not want or need per
cmd_type handlers.
A SCSI example might be something like:
static void scsi_softirq_done(struct request *rq)
{
...
sdev->cmd_type_handler[rq->cmd_type]->softirq_done(rq)
...
}
int scsi_prep_fn(struct request_queue *q, struct request *req)
{
...
sdev->cmd_type_handler[rq->cmd_type]->prep_fn(req)
...
}
static void scsi_request_fn(struct request_queue *q)
{
...
sdev->cmd_type_handler[rq->cmd_type]->request_fn(req)
...
}
This is just moving the code inside the cmd_type "if" checks, but it may
reduce the number of cmd_type "if" checks in some paths (if we make
multiple decisions based on cmd_type). On init of the sdev default
handlers would be installed.
-andmike
--
Michael Anderson
[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: [Bugme-new] [Bug 9901] New: kernel panic in stex modules (?)
On Wed, 2008-02-06 at 10:15 -0800, Andrew Morton wrote:
> On Wed, 6 Feb 2008 09:40:15 -0800 (PST) [EMAIL PROTECTED] wrote:
>
> > http://bugzilla.kernel.org/show_bug.cgi?id=9901
> >
> >Summary: kernel panic in stex modules (?)
> >Product: IO/Storage
> >Version: 2.5
> > KernelVersion: 2.6.24
> > Platform: All
> > OS/Version: Linux
> > Tree: Mainline
> > Status: NEW
> > Severity: normal
> > Priority: P1
> > Component: Serial ATA
> > AssignedTo: [EMAIL PROTECTED]
> > ReportedBy: [EMAIL PROTECTED]
> >
> >
> > Latest working kernel version: 2.6.23-r6
> > Earliest failing kernel version: 2.6.24
> > Distribution: Gentoo
> > Hardware Environment: Core2D E6600, Asus p5B Dlx, 2G DDR2 667, Promise ST
> > EX4350
> > Software Environment: GCC 4.2.3/4.1.2, CFLAGS="-O2"
> >
> > Problem Description:
> > The problem is frequent kernel panics within the same module. Can't say
> > what it
> > is, but looks like it is related to dma and promise driver.
> > The first culprit, the memory, is ok, 8 hours of memtest passed without
> > errors.
> > Before, kernel 2.6.23-gentoo-r6, compiled with GCC 4.1.2 worked just fine,
> > then
> > after upgrade to 4.2.2 th bug appeared. Upgrade to 2.6.24 didn't solve the
> > problem. Switching back to GCC 4.1.2 made things better for a moment,
> > crashes
> > became less frequent and I thought compiler was the cause. But today system
> > crashed again with same symptoms.
> > Sorry, but I can't save crash log, so I'll provide screen "shot":
> > http://img238.imageshack.us/my.php?image=p2030030ki1.jpg
> >
> > Steps to reproduce:
> > Boot, start FTP-server, load RAID with heavy input, in some hours it will
> > crash. With pure reads system can run several days, heavy write load kills
> > it
> > much too easier.
> >
>
> The supertrak driver has regressed in 2.6.24. And
>
> commit 9cb83c7529d929c00f37d821daed1942a1b20602
> Author: FUJITA Tomonori <[EMAIL PROTECTED]>
> Date: Tue Oct 16 11:24:32 2007 +0200
>
> [SCSI] add use_sg_chaining option to scsi_host_template
>
> looks a likely candidate.
>
> And this:
>
> commit d3f46f39b7092594b498abc12f0c73b0b9913bde
> Author: James Bottomley <[EMAIL PROTECTED]>
> Date: Tue Jan 15 11:11:46 2008 -0600
>
> [SCSI] remove use_sg_chaining
>
> from 2.6.25 looks to be a likely fix for it. Should it be backported?
If the patch you identify is the culprit, mine can't be the fix ... and
it should also be present in git head.
The BUG_ON is here: isn't it?
static inline void
dma_unmap_sg(struct device *hwdev, struct scatterlist *sg, int nents,
int direction)
{
BUG_ON(!valid_dma_direction(direction));
^^^
dma_ops->unmap_sg(hwdev, sg, nents, direction);
}
stex only does scsi_dma_unmap(), so something looks to have tampered
with the cmnd->sc_data_direction somehow ... and I can't see how.
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][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
James Bottomley wrote:
> On Mon, 2008-02-04 at 23:36 +0100, Roel Kluin wrote:
>> Note the duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
>> - if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
>> + if (SCpnt->sc_data_direction == DMA_TO_DEVICE) {
>>cpp->xdir = DTD_IN;
>>return;
>>}
>> else if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
>
> Well spotted, this is definitely a huge bug in the driver.
>
> However, I think on closer examination that DTD_IN actually means
> transfer from device to host, so DMA_FROM_DEVICE is correct for that
> case. It should be DMA_TO_DEVICE in the else if() piece.
>
> Could you correct and resend the patch and I'll apply it.
Thanks for your review.
---
Direction of data transfer 'DMA_FROM_DEVICE' was tested twice. DTD_OUT
means transfer from host to device. This should occur when the
direction of data transfer (sc_data_direction) is 'DMA_TO_DEVICE'.
Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
index 662c004..58d7eee 100644
--- a/drivers/scsi/u14-34f.c
+++ b/drivers/scsi/u14-34f.c
@@ -1215,9 +1215,9 @@ static void scsi_to_dev_dir(unsigned int i, unsigned int
j) {
if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
cpp->xdir = DTD_IN;
return;
}
- else if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
+ else if (SCpnt->sc_data_direction == DMA_TO_DEVICE) {
cpp->xdir = DTD_OUT;
return;
}
else if (SCpnt->sc_data_direction == DMA_NONE) {
-
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: [Bugme-new] [Bug 9901] New: kernel panic in stex modules (?)
On Wed, 6 Feb 2008 09:40:15 -0800 (PST) [EMAIL PROTECTED] wrote: > http://bugzilla.kernel.org/show_bug.cgi?id=9901 > >Summary: kernel panic in stex modules (?) >Product: IO/Storage >Version: 2.5 > KernelVersion: 2.6.24 > Platform: All > OS/Version: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: Serial ATA > AssignedTo: [EMAIL PROTECTED] > ReportedBy: [EMAIL PROTECTED] > > > Latest working kernel version: 2.6.23-r6 > Earliest failing kernel version: 2.6.24 > Distribution: Gentoo > Hardware Environment: Core2D E6600, Asus p5B Dlx, 2G DDR2 667, Promise ST > EX4350 > Software Environment: GCC 4.2.3/4.1.2, CFLAGS="-O2" > > Problem Description: > The problem is frequent kernel panics within the same module. Can't say what > it > is, but looks like it is related to dma and promise driver. > The first culprit, the memory, is ok, 8 hours of memtest passed without > errors. > Before, kernel 2.6.23-gentoo-r6, compiled with GCC 4.1.2 worked just fine, > then > after upgrade to 4.2.2 th bug appeared. Upgrade to 2.6.24 didn't solve the > problem. Switching back to GCC 4.1.2 made things better for a moment, crashes > became less frequent and I thought compiler was the cause. But today system > crashed again with same symptoms. > Sorry, but I can't save crash log, so I'll provide screen "shot": > http://img238.imageshack.us/my.php?image=p2030030ki1.jpg > > Steps to reproduce: > Boot, start FTP-server, load RAID with heavy input, in some hours it will > crash. With pure reads system can run several days, heavy write load kills it > much too easier. > The supertrak driver has regressed in 2.6.24. And commit 9cb83c7529d929c00f37d821daed1942a1b20602 Author: FUJITA Tomonori <[EMAIL PROTECTED]> Date: Tue Oct 16 11:24:32 2007 +0200 [SCSI] add use_sg_chaining option to scsi_host_template looks a likely candidate. And this: commit d3f46f39b7092594b498abc12f0c73b0b9913bde Author: James Bottomley <[EMAIL PROTECTED]> Date: Tue Jan 15 11:11:46 2008 -0600 [SCSI] remove use_sg_chaining from 2.6.25 looks to be a likely fix for it. Should it be backported? - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] aacraid: pci_set_dma_max_seg_size opened up for late model controllers (take 2)
Merde, excuse my english. No idea how those spaces snuck in, a last minute vi fumble finger?! Updated patch enclosed. My apologies for the gaff! Signed-off-by: Mark Salyzyn <[EMAIL PROTECTED]> drivers/scsi/aacraid/linit.c | 42 -- 1 file changed, 20 insertions(+), 22 deletions(-) Sincerely -- Mark Salyzyn > -Original Message- > From: James Bottomley [mailto:[EMAIL PROTECTED] > Sent: Wednesday, February 06, 2008 12:55 PM > To: Salyzyn, Mark > Cc: FUJITA Tomonori; [EMAIL PROTECTED]; > [EMAIL PROTECTED]; [EMAIL PROTECTED]; > [EMAIL PROTECTED]; [EMAIL PROTECTED]; 'linux-scsi@vger.kernel.org' > Subject: Re: [PATCH 1/1] aacraid: pci_set_dma_max_seg_size > opened up for late model controllers > > On Wed, 2008-02-06 at 09:00 -0800, Salyzyn, Mark wrote: > > This patch depends on 'RE: [patch 048/265] iommu sg > merging: aacraid: use pci_set_dma_max_seg_size' and ensures > that the modern adapters get a maximum sg segment size on par > with the maximum transfer size. Added some localized janitor > fixes to the discussion patch I used with Fujita. > > > > FUJITA Tomonori [mailto:[EMAIL PROTECTED] sez: > > > I think that setting the proper maximum segment size for the late > > > model cards (as you did above) makes sense. > > > > This attached patch is against current linux-2.6. It will > not apply to current scsi-misc-2.6 without the above > mentioned dependency. > > > > ObligatoryDisclaimer: Please accept my condolences > regarding Outlook's handling of patch attachments. Please use > the attached file to patch > > > > Signed-off-by: Mark Salyzyn <[EMAIL PROTECTED]> > > OK, I have this, but someone really needs to do a little more > checkpatch > love: > > [EMAIL PROTECTED]> ./scripts/checkpatch.pl ~/tmp.mail > ERROR: use tabs not spaces > #44: FILE: drivers/scsi/aacraid/linit.c:1138: > + ^I^Ishost->sg_tablesize = 34;$ > > ERROR: use tabs not spaces > #45: FILE: drivers/scsi/aacraid/linit.c:1139: > + ^I^Ishost->max_sectors = (shost->sg_tablesize * 8) + 112;$ > > ERROR: use tabs not spaces > #54: FILE: drivers/scsi/aacraid/linit.c:1144: > + ^I^Ishost->sg_tablesize = 17;$ > > ERROR: use tabs not spaces > #55: FILE: drivers/scsi/aacraid/linit.c:1145: > + ^I^Ishost->max_sectors = (shost->sg_tablesize * 8) + 112;$ > > ERROR: use tabs not spaces > #60: FILE: drivers/scsi/aacraid/linit.c:1150: > + ^I^I^I(shost->max_sectors << 9) : 65536);$ > > total: 5 errors, 0 warnings, 44 lines checked > > Your patch has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > James aacraid_pci_set_dma1.patch Description: aacraid_pci_set_dma1.patch
Re: Integration of SCST in the mainstream Linux kernel
James Bottomley wrote: On Tue, 2008-02-05 at 21:59 +0300, Vladislav Bolkhovitin wrote: Hmm, how can one write to an mmaped page and don't touch it? I meant from user space ... the writes are done inside the kernel. Sure, the mmap() approach agreed to be unpractical, but could you elaborate more on this anyway, please? I'm just curious. Do you think about implementing a new syscall, which would put pages with data in the mmap'ed area? No, it has to do with the way invalidation occurs. When you mmap a region from a device or file, the kernel places page translations for that region into your vm_area. The regions themselves aren't backed until faulted. For write (i.e. incoming command to target) you specify the write flag and send the area off to receive the data. The gather, expecting the pages to be overwritten, backs them with pages marked dirty but doesn't fault in the contents (unless it already exists in the page cache). The kernel writes the data to the pages and the dirty pages go back to the user. msync() flushes them to the device. The disadvantage of all this is that the handle for the I/O if you will is a virtual address in a user process that doesn't actually care to see the data. non-x86 architectures will do flushes/invalidates on this address space as the I/O occurs. I more or less see, thanks. But (1) pages still needs to be mmaped to the user space process before the data transmission, i.e. they must be zeroed before being mmaped, which isn't much faster, than data copy, and (2) I suspect, it would be hard to make it race free, e.g. if another process would want to write to the same area simultaneously However, as Linus has pointed out, this discussion is getting a bit off topic. No, that isn't off topic. We've just proved that there is no good way to implement zero-copy cached I/O for STGT. I see the only practical way for that, proposed by FUJITA Tomonori some time ago: duplicating Linux page cache in the user space. But will you like it? Well, there's no real evidence that zero copy or lack of it is a problem yet. The performance improvement from zero copy can be easily estimated, knowing the link throughput and data copy throughput, which are about the same for 20Gbps links (I did that few e-mail ago). Vlad - 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/1] aacraid: pci_set_dma_max_seg_size opened up for late model controllers
On Wed, 2008-02-06 at 09:00 -0800, Salyzyn, Mark wrote: > This patch depends on 'RE: [patch 048/265] iommu sg merging: aacraid: use > pci_set_dma_max_seg_size' and ensures that the modern adapters get a maximum > sg segment size on par with the maximum transfer size. Added some localized > janitor fixes to the discussion patch I used with Fujita. > > FUJITA Tomonori [mailto:[EMAIL PROTECTED] sez: > > I think that setting the proper maximum segment size for the late > > model cards (as you did above) makes sense. > > This attached patch is against current linux-2.6. It will not apply to > current scsi-misc-2.6 without the above mentioned dependency. > > ObligatoryDisclaimer: Please accept my condolences regarding Outlook's > handling of patch attachments. Please use the attached file to patch > > Signed-off-by: Mark Salyzyn <[EMAIL PROTECTED]> OK, I have this, but someone really needs to do a little more checkpatch love: [EMAIL PROTECTED]> ./scripts/checkpatch.pl ~/tmp.mail ERROR: use tabs not spaces #44: FILE: drivers/scsi/aacraid/linit.c:1138: + ^I^Ishost->sg_tablesize = 34;$ ERROR: use tabs not spaces #45: FILE: drivers/scsi/aacraid/linit.c:1139: + ^I^Ishost->max_sectors = (shost->sg_tablesize * 8) + 112;$ ERROR: use tabs not spaces #54: FILE: drivers/scsi/aacraid/linit.c:1144: + ^I^Ishost->sg_tablesize = 17;$ ERROR: use tabs not spaces #55: FILE: drivers/scsi/aacraid/linit.c:1145: + ^I^Ishost->max_sectors = (shost->sg_tablesize * 8) + 112;$ ERROR: use tabs not spaces #60: FILE: drivers/scsi/aacraid/linit.c:1150: + ^I^I^I(shost->max_sectors << 9) : 65536);$ total: 5 errors, 0 warnings, 44 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 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][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
On Mon, 2008-02-04 at 23:36 +0100, Roel Kluin wrote:
> It should be like this I guess? this patch was not yet tested, please
> confirm.
> --
> Note the duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
>
> from Documentation/DMA-API.txt:
> DMA_TO_DEVICE = PCI_DMA_TODEVICE data is going from the
> memory to the device
> DMA_FROM_DEVICE = PCI_DMA_FROMDEVICEdata is coming from
> the device to the
>
> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
> ---
> diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
> index 662c004..1e704f9 100644
> --- a/drivers/scsi/u14-34f.c
> +++ b/drivers/scsi/u14-34f.c
> @@ -1208,15 +1208,15 @@ static void scsi_to_dev_dir(unsigned int i, unsigned
> int j) {
>};
>
> struct mscp *cpp;
> struct scsi_cmnd *SCpnt;
>
> cpp = &HD(j)->cp[i]; SCpnt = cpp->SCpnt;
>
> - if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
> + if (SCpnt->sc_data_direction == DMA_TO_DEVICE) {
>cpp->xdir = DTD_IN;
>return;
>}
> else if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
>cpp->xdir = DTD_OUT;
>return;
>}
Well spotted, this is definitely a huge bug in the driver.
However, I think on closer examination that DTD_IN actually means
transfer from device to host, so DMA_FROM_DEVICE is correct for that
case. It should be DMA_TO_DEVICE in the else if() piece.
Could you correct and resend the patch and I'll apply it.
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] sr: update to follow tray status correctly
Hi James,
James Bottomley wrote:
It's been a while with no status on this one, so I corrected the patch
based on my original input. The attached is what I think is the best
way of doing this (I've replaced the home grown test_unit_ready routine
with the SCSI one and updated some of the sense check conditions).
Many thanks for following up on this. The user has not responded to my
request for him to test your patch, so I decided to try it myself. I'm
using Linus git as of today, I noticed the patch is merged.
It doesn't work for me. On unpatched 2.6.24, I get the behaviour
previously described - the ioctl always gives CDS_TRAY_OPEN regardless
of tray status. On 2.6.24-git which includes your patch, I instead
always get CDS_DISC_OK. I have confirmed that on 2 systems this is
because scsi_test_unit_ready() is returning 0.
Before I dig further, does this work for you? I have attached the test
program I am using.
Thanks!
Daniel
#include
#include
#include
#include
#include
#include
int main(void)
{
int fd = open("/dev/sr0", O_RDONLY | O_NONBLOCK);
int ret;
if (fd < 0) {
perror("open");
return 1;
}
ret = ioctl(fd, CDROM_DRIVE_STATUS, 0);
printf("ioctl result %d\n", ret);
switch(ret) {
case CDS_NO_INFO: printf("CDS_NO_INFO\n"); break;
case CDS_NO_DISC: printf("CDS_NO_DISC\n"); break;
case CDS_TRAY_OPEN: printf("CDS_TRAY_OPEN\n"); break;
case CDS_DRIVE_NOT_READY: printf("CDS_DRIVE_NOT_READY\n");
break;
case CDS_DISC_OK: printf("CDS_DISC_OK\n"); break;
default: printf("Unknown\n"); break;
}
return 0;
}
Re: [PATCH 1/24][RFC] scsi_eh: Define new API for sense handling
On Tue, 2008-02-05 at 17:43 +0200, Boaz Harrosh wrote: > On Mon, Feb 04 2008 at 19:33 +0200, James Bottomley <[EMAIL PROTECTED]> wrote: > > On Mon, 2008-02-04 at 17:30 +0200, Boaz Harrosh wrote: > >> This patch defines a new API for sense handling. All drivers will > >> be converted to this API, before the sense handling implementation will > >> change. API is as follows: > >> > >> void scsi_eh_cpy_sense(struct scsi_cmnd *cmd, void* sense, > >> unsigned sense_bytes); > >> To be used by drivers, when they have sense-bits > >> and wants to send them to upper layer. Max size > >> need not be a concern, If upper layer does not have > >> enough space it will be automatically truncated. > >> > >> u8 *scsi_make_sense(struct scsi_cmnd *cmd); > >> To be used by drivers, and scsi-midlayer. Returns a DMA-able > >> sense buffer. Must be returned by scsi_return_sense(). It should > >> never fail if .pre_allocate_sense && .sense_buffsize in host > >> template where properly set. > >> the buffer is of shost->sense_buffsize long. > >> > >> void *scsi_return_sense(struct scsi_cmnd *cmd, u8 *sb); > >> Frees and returns the sense to the upper layer, > >> copying only what's necessary. > >> > >> void scsi_eh_reset_sense(struct scsi_cmnd *cmd) > >> Should not be used or necessary. > >> > >> const u8 *scsi_sense(struct scsi_cmnd *cmd) > >> Used by ULDs and for inspecting the returned sense, can not > >> be modified. It is only valid after a call to > >> scsi_eh_cpy_sense() or a call to scsi_return_sense(). Before > >> that it will/should return an empty buffer. > >> > >> New members at scsi host template: > >> .sense_buffsize - if a driver calls scsi_make_sense() or > >> scsi_eh_prep_cmnd(), This value should be none > >> zero indicating the max sense size, the driver > >> supports. In most cases it should be > >> SCSI_SENSE_BUFFERSIZE. > >> If this value is zero the driver will only call > >> scsi_eh_cpy_sense(). > >> > >> .pre_allocate_sense - if a Driver calls scsi_make_sense() > >> in .queuecommand for every cmnd, this > >> should be set to true. In which case > >> scsi_make_sense() will not fail because > >> midlayer will fail the command allocation. > >> If the drivers calls scsi_eh_prep_cmnd() > >> then sense_buffsize is not Zero but this > >> here is set to false. > > > > My initial reaction to this is that you're doing too many contortions to > > ensure something we don't particularly care about: whether we can > > allocate a sense buffer atomically or not. > > I hope that now, once you've actually seen the implementation, my > motivation is clearer. Perhaps I explained it badly, but the actual code > is pretty simple and contortions is not how I would describe it. The API > above is just a way for drivers to say how they intend to behave, and the > midlayer will accommodate easily. None of the solutions are hard and they > are all simpler then what exists today. The only added complexity introduced > is the initial choice. Well, actually now it's clearer that this is at least three patch sets rolled into one: 1. Alter the sense allocation method (The one I'm not keen on) 2. Allow the sense buffer size to be increased 3. A block change to allow variable sense lengths. So, confining the remarks to 1. only Your API could do with a bit of name tuning. scsi_make_sense() is definitely an API certainly linux people have been yearning after for a while ... I'm just not sure you satisfy them in that regard ... The usual API naming would either be _get/_put based or _alloc/_free based. > > > > What all this code should be doing is simply allocating the sense buffer > > in scsi_eh_prep_cmnd() using tomo's existing slab (and GFP_ATOMIC) > > This is what we are doing. Only allocating the sense buffer in the very > unlikely event of the call to scsi_eh_prep_cmnd(). So we are in agreement > here. No, you're not ... you're preallocating in the command get for most hosts that don't have an existing sense buffer in their descriptor structure. For the others, I think the logic is flawed ... see below. > > if > > that fails, we need a return from scsi_eh_prep_cmnd() telling us. At > > that point, the driver should abandon the auto request sense attempt and > > instead just return the CC/UA without the DRIVER_SENSE bit set which > > will trigger the eh to collect the sense for us. > > > > This is a nightmare and a serious regression. It will cause an IO deadlock > in the event of an IO error during an IO-to-free-memory scenari
Re: Integration of SCST in the mainstream Linux kernel
> Sorry, but I'm afraid you got this wrong. When the iSER transport is > used instead of TCP, all data is sent via RDMA, including unsolicited > data. If you have look at the iSER implementation in the Linux kernel > (source files under drivers/infiniband/ulp/iser), you will see that > all data is transferred via RDMA and not via TCP/IP. I think the confusion here is caused by a slight misuse of the term "RDMA". It is true that all data is always transported over an InfiniBand connection when iSER is used, but not all such transfers are one-sided RDMA operations; some data can be transferred using send/receive operations. - R. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] aacraid: pci_set_dma_max_seg_size opened up for late model controllers
This patch depends on 'RE: [patch 048/265] iommu sg merging: aacraid: use
pci_set_dma_max_seg_size' and ensures that the modern adapters get a maximum sg
segment size on par with the maximum transfer size. Added some localized
janitor fixes to the discussion patch I used with Fujita.
FUJITA Tomonori [mailto:[EMAIL PROTECTED] sez:
> I think that setting the proper maximum segment size for the late
> model cards (as you did above) makes sense.
This attached patch is against current linux-2.6. It will not apply to current
scsi-misc-2.6 without the above mentioned dependency.
ObligatoryDisclaimer: Please accept my condolences regarding Outlook's handling
of patch attachments. Please use the attached file to patch
Signed-off-by: Mark Salyzyn <[EMAIL PROTECTED]>
drivers/scsi/aacraid/linit.c | 28 +---
1 file changed, 13 insertions(+), 15 deletions(-)
diff -ru a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
--- a/drivers/scsi/aacraid/linit.c 2008-02-06 11:35:55.111631591 -0500
+++ b/drivers/scsi/aacraid/linit.c 2008-02-06 11:52:11.095208736 -0500
@@ -1130,31 +1130,29 @@
if (error < 0)
goto out_deinit;
- if (!(aac->adapter_info.options & AAC_OPT_NEW_COMM)) {
- error = pci_set_dma_max_seg_size(pdev, 65536);
- if (error)
- goto out_deinit;
- }
-
/*
* Lets override negotiations and drop the maximum SG limit to 34
*/
if ((aac_drivers[index].quirks & AAC_QUIRK_34SG) &&
- (aac->scsi_host_ptr->sg_tablesize > 34)) {
- aac->scsi_host_ptr->sg_tablesize = 34;
- aac->scsi_host_ptr->max_sectors
- = (aac->scsi_host_ptr->sg_tablesize * 8) + 112;
+ (shost->sg_tablesize > 34)) {
+ shost->sg_tablesize = 34;
+ shost->max_sectors = (shost->sg_tablesize * 8) + 112;
}
if ((aac_drivers[index].quirks & AAC_QUIRK_17SG) &&
- (aac->scsi_host_ptr->sg_tablesize > 17)) {
- aac->scsi_host_ptr->sg_tablesize = 17;
- aac->scsi_host_ptr->max_sectors
- = (aac->scsi_host_ptr->sg_tablesize * 8) + 112;
+ (shost->sg_tablesize > 17)) {
+ shost->sg_tablesize = 17;
+ shost->max_sectors = (shost->sg_tablesize * 8) + 112;
}
+ error = pci_set_dma_max_seg_size(pdev,
+ (aac->adapter_info.options & AAC_OPT_NEW_COMM) ?
+ (shost->max_sectors << 9) : 65536);
+ if (error)
+ goto out_deinit;
+
/*
-* Firware printf works only with older firmware.
+* Firmware printf works only with older firmware.
*/
if (aac_drivers[index].quirks & AAC_QUIRK_34SG)
aac->printf_enabled = 1;
aacraid_pci_set_dma.patch
Description: aacraid_pci_set_dma.patch
Re: Integration of SCST in the mainstream Linux kernel
On Feb. 06, 2008, 14:16 +0200, "Bart Van Assche" <[EMAIL PROTECTED]> wrote: > On Feb 5, 2008 6:01 PM, Erez Zilber <[EMAIL PROTECTED]> wrote: >> Using such large values for FirstBurstLength will give you poor >> performance numbers for WRITE commands (with iSER). FirstBurstLength >> means how much data should you send as unsolicited data (i.e. without >> RDMA). It means that your WRITE commands were sent without RDMA. > > Sorry, but I'm afraid you got this wrong. When the iSER transport is > used instead of TCP, all data is sent via RDMA, including unsolicited > data. If you have look at the iSER implementation in the Linux kernel > (source files under drivers/infiniband/ulp/iser), you will see that > all data is transferred via RDMA and not via TCP/IP. Regardless of what the current implementation is, the behavior you (Bart) describe seems to disagree with http://www.ietf.org/rfc/rfc5046.txt. Benny > > Bart Van Assche. > - - 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: ABORT_TASK defined in aic94xx_sas.h
On Wed, 2008-02-06 at 17:34 +0200, Boaz Harrosh wrote: > On Wed, Feb 06 2008 at 17:13 +0200, James Bottomley <[EMAIL PROTECTED]> wrote: > > On Wed, 2008-02-06 at 15:38 +0200, Boaz Harrosh wrote: > >> include/scsi/scsi.h as a definition: > >> #define ABORT_TASK 0x0d > >> > >> on the other hand drivers/scsi/aic94xx/aic94xx_sas.h has: > >> #define ABORT_TASK 0x03 > >> > >> am I right in thinking that aic94xx_sas.h is wrong in > >> polluting the global name-space? > >> > >> If you ask me aic94xx_sas.h is a global name-space minefield > >> > >> (This gives me problems when trying to pull in scsi_eh.h into > >> aic94xx source files) > > > > Well, no, not in those terms. The global namespace exists in shared > > headers which it's a little hard to argue that aic94xx_sas.h is, being > > unusable by anything other than a single driver. > > > > It is correct to say that include/scsi/scsi.h is polluting the global > > namespace, because that is pulled into a large section of the kernel. > > > > The message code #defines in scsi.h are a horrible mess of SPI message > > defines and task management function defines each of which should > > arguably have a SPI_ and TMF_ global namespace discriminator (and the > > SPI_ ones be shovelled off into the SPI transport class header). > > > > However, this looks like a reasonable hack. > > > > James > > > > > Point taken, you are right. > > Please see that you approve of the name I gave it, > I just got it from the nearest comment so I'm not > even sure if it's related. Yes, it's a sequencer control block opcode, so prefixing it with SCB_ is reasonable. 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: ABORT_TASK defined in aic94xx_sas.h
On Wed, Feb 06 2008 at 17:13 +0200, James Bottomley <[EMAIL PROTECTED]> wrote: > On Wed, 2008-02-06 at 15:38 +0200, Boaz Harrosh wrote: >> include/scsi/scsi.h as a definition: >> #define ABORT_TASK 0x0d >> >> on the other hand drivers/scsi/aic94xx/aic94xx_sas.h has: >> #define ABORT_TASK 0x03 >> >> am I right in thinking that aic94xx_sas.h is wrong in >> polluting the global name-space? >> >> If you ask me aic94xx_sas.h is a global name-space minefield >> >> (This gives me problems when trying to pull in scsi_eh.h into >> aic94xx source files) > > Well, no, not in those terms. The global namespace exists in shared > headers which it's a little hard to argue that aic94xx_sas.h is, being > unusable by anything other than a single driver. > > It is correct to say that include/scsi/scsi.h is polluting the global > namespace, because that is pulled into a large section of the kernel. > > The message code #defines in scsi.h are a horrible mess of SPI message > defines and task management function defines each of which should > arguably have a SPI_ and TMF_ global namespace discriminator (and the > SPI_ ones be shovelled off into the SPI transport class header). > > However, this looks like a reasonable hack. > > James > > Point taken, you are right. Please see that you approve of the name I gave it, I just got it from the nearest comment so I'm not even sure if it's related. Thanks Boaz - 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: diskdump fails on x3850 (x86_64)
On Wed, 2008-02-06 at 19:56 +0530, Chandru wrote: > We looked at the code and there seem to be atleast two functions that > may be required in the driver to support diskdump, dump_poll and > dump_sanity_check as part of Scsi_Host_Template structure. Different > drivers have their own way of implementing these routines in the driver > code. Hence we wanted to ask if the current developers of adp94xx can > provide implementation for these routines. The driver version that we > have seems to be adp94xx - '1.0.8-13' Erm, you'll need to take this up with Red Hat and Adaptec support. The adp94xx isn't the in-kernel driver, it's one those companies have supplied to RHEL4. If you can reproduce the problem with the in-kernel aic94xx driver then sure, we could take a look at it. 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: ABORT_TASK defined in aic94xx_sas.h
On Wed, 2008-02-06 at 15:38 +0200, Boaz Harrosh wrote: > include/scsi/scsi.h as a definition: > #define ABORT_TASK 0x0d > > on the other hand drivers/scsi/aic94xx/aic94xx_sas.h has: > #define ABORT_TASK 0x03 > > am I right in thinking that aic94xx_sas.h is wrong in > polluting the global name-space? > > If you ask me aic94xx_sas.h is a global name-space minefield > > (This gives me problems when trying to pull in scsi_eh.h into > aic94xx source files) Well, no, not in those terms. The global namespace exists in shared headers which it's a little hard to argue that aic94xx_sas.h is, being unusable by anything other than a single driver. It is correct to say that include/scsi/scsi.h is polluting the global namespace, because that is pulled into a large section of the kernel. The message code #defines in scsi.h are a horrible mess of SPI message defines and task management function defines each of which should arguably have a SPI_ and TMF_ global namespace discriminator (and the SPI_ ones be shovelled off into the SPI transport class header). However, this looks like a reasonable hack. 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: diskdump fails on x3850 (x86_64)
We looked at the code and there seem to be atleast two functions that may be required in the driver to support diskdump, dump_poll and dump_sanity_check as part of Scsi_Host_Template structure. Different drivers have their own way of implementing these routines in the driver code. Hence we wanted to ask if the current developers of adp94xx can provide implementation for these routines. The driver version that we have seems to be adp94xx - '1.0.8-13' Regards, Chandru Ciju Rajan K wrote: Hello folks, Diskdump on x3850 with adp94xx driver is not working (Redhat Enterprise Linux 4.6). 'Service diskdump restart' fails. Is there any plan to support it? Thanks Ciju - 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: Integration of SCST in the mainstream Linux kernel
Bart Van Assche wrote: On Feb 5, 2008 6:50 PM, Jeff Garzik <[EMAIL PROTECTED]> wrote: For remotely accessing data, iSCSI+fs is quite simply more overhead than a networked fs. With iSCSI you are doing local VFS -> local blkdev -> network whereas a networked filesystem is local VFS -> network There are use cases than can be solved better via iSCSI and a filesystem than via a network filesystem. One such use case is when deploying a virtual machine whose data is stored on a network server: in that case there is only one user of the data (so there are no locking issues) and filesystem and block device each run in another operating system: the filesystem runs inside the virtual machine and iSCSI either runs in the hypervisor or in the native OS. Hence the diskless root fs configuration I referred to in multiple emails... whoopee, you reinvented NFS root with quotas :) Jeff - 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
ABORT_TASK defined in aic94xx_sas.h
include/scsi/scsi.h as a definition:
#define ABORT_TASK 0x0d
on the other hand drivers/scsi/aic94xx/aic94xx_sas.h has:
#define ABORT_TASK 0x03
am I right in thinking that aic94xx_sas.h is wrong in
polluting the global name-space?
If you ask me aic94xx_sas.h is a global name-space minefield
(This gives me problems when trying to pull in scsi_eh.h into
aic94xx source files)
perhaps:
---
From: Boaz Harrosh <[EMAIL PROTECTED]>
Date: Wed, 6 Feb 2008 15:35:37 +0200
Subject: [PATCH] aic94xx_sas: avoid conflict with scsi.h
drivers/scsi/aic94xx/aic94xx_sas.h would redefine ABORT_TASK
as a different value.
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
drivers/scsi/aic94xx/aic94xx_sas.h |2 +-
drivers/scsi/aic94xx/aic94xx_tmf.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_sas.h
b/drivers/scsi/aic94xx/aic94xx_sas.h
index fa7c529..912e6b7 100644
--- a/drivers/scsi/aic94xx/aic94xx_sas.h
+++ b/drivers/scsi/aic94xx/aic94xx_sas.h
@@ -292,7 +292,7 @@ struct scb_header {
#define INITIATE_SSP_TASK 0x00
#define INITIATE_LONG_SSP_TASK 0x01
#define INITIATE_BIDIR_SSP_TASK 0x02
-#define ABORT_TASK 0x03
+#define SCB_ABORT_TASK 0x03
#define INITIATE_SSP_TMF0x04
#define SSP_TARG_GET_DATA 0x05
#define SSP_TARG_GET_DATA_GOOD 0x06
diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c
b/drivers/scsi/aic94xx/aic94xx_tmf.c
index 87b2f6e..b52124f 100644
--- a/drivers/scsi/aic94xx/aic94xx_tmf.c
+++ b/drivers/scsi/aic94xx/aic94xx_tmf.c
@@ -369,7 +369,7 @@ int asd_abort_task(struct sas_task *task)
return -ENOMEM;
scb = ascb->scb;
- scb->header.opcode = ABORT_TASK;
+ scb->header.opcode = SCB_ABORT_TASK;
switch (task->task_proto) {
case SAS_PROTOCOL_SATA:
--
1.5.3.3
-
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: Integration of SCST in the mainstream Linux kernel
On Feb 5, 2008 6:01 PM, Erez Zilber <[EMAIL PROTECTED]> wrote: > > Using such large values for FirstBurstLength will give you poor > performance numbers for WRITE commands (with iSER). FirstBurstLength > means how much data should you send as unsolicited data (i.e. without > RDMA). It means that your WRITE commands were sent without RDMA. Sorry, but I'm afraid you got this wrong. When the iSER transport is used instead of TCP, all data is sent via RDMA, including unsolicited data. If you have look at the iSER implementation in the Linux kernel (source files under drivers/infiniband/ulp/iser), you will see that all data is transferred via RDMA and not via TCP/IP. Bart Van Assche. - 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: Integration of SCST in the mainstream Linux kernel
On Feb 5, 2008 6:50 PM, Jeff Garzik <[EMAIL PROTECTED]> wrote: > For remotely accessing data, iSCSI+fs is quite simply more overhead than > a networked fs. With iSCSI you are doing > > local VFS -> local blkdev -> network > > whereas a networked filesystem is > > local VFS -> network There are use cases than can be solved better via iSCSI and a filesystem than via a network filesystem. One such use case is when deploying a virtual machine whose data is stored on a network server: in that case there is only one user of the data (so there are no locking issues) and filesystem and block device each run in another operating system: the filesystem runs inside the virtual machine and iSCSI either runs in the hypervisor or in the native OS. Bart Van Assche. - 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
