Re: [PATCH 27/30] blk_end_request: changing scsi (take 4)
On Tue, 2007-12-11 at 17:52 -0500, Kiyoshi Ueda wrote: This patch converts scsi mid-layer to use blk_end_request interfaces. Related 'uptodate' arguments are converted to 'error'. As a result, the interface of internal function, scsi_end_request(), is changed. This looks fine, as far as it goes. One of the things that might actually be useful in future is as well as communicating the error, the ability to say more information about what happened. For instance dm-mp needs to know whether the error is transport or device related (the former being retryable on a different path). 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: broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
On Wed, 2007-12-12 at 11:16 -0800, Andrew Morton wrote: On Wed, 12 Dec 2007 14:43:42 +0100 Anders Henke [EMAIL PROTECTED] wrote: Am 12.12.2007 schrieb Miquel van Smoorenburg: On Wed, 2007-12-12 at 03:38 -0800, Andrew Morton wrote: On Wed, 12 Dec 2007 11:58:41 +0100 Anders Henke [EMAIL PROTECTED] wrote: Hi, I'd like to let you now that my boxes are running a 32-bit kernel, so the 64-bit-uncleanliness shouldn't apply to my boxes; however, http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch fixed the issue on my testbox. I took a clean 2.6.23, applied patch, recompiled the kernel, reboot: works. What a huge patch :( We already reverted the offening patch so I assume that 2.6.24-rc5 is working for you? I guess we need to look at restoring dpt_i2o: convert to SCSI hotplug model and then absorbing what Miquel has done there. This was just a patch I had lying around, if it worked it would confirm my suspicion, which it has. The minimal patch which is suitable for 2.6.23-stable and 2.6.24 would be the attached one-liner. The dpt_i2o: convert to SCSI hotplug model patch could be restored then. (if the list eats the attachment, it's also available here: http://www.miquels.cistron.nl/linux/linux-2.6.23+24-dpt_i2o-dma64.patch ) Anders, does this one-liner patch work for you ? Got it - and it works! I took a clean 2.6.23, applied the patch, recompiled the kernel and rebooted my testbox: came up with the fresh-compiled kernel (verified by uname -a). That looks appropriate for 2.6.23.x: --- linux-2.6.23.9.orig/drivers/scsi/dpt_i2o.c2007-11-26 18:51:43.0 +0100 +++ linux-2.6.23.9/drivers/scsi/dpt_i2o.c 2007-12-12 13:21:05.0 +0100 @@ -905,8 +905,7 @@ } pci_set_master(pDev); - if (pci_set_dma_mask(pDev, DMA_64BIT_MASK) - pci_set_dma_mask(pDev, DMA_32BIT_MASK)) + if (pci_set_dma_mask(pDev, DMA_32BIT_MASK)) return -EINVAL; Yes, this has to be in ... the mptr filling the scatterlist on the current driver is only a u32 and so will silently truncate. base_addr0_phys = pci_resource_start(pDev,0); However it is a bit mystifying that 55d9fcf57ba5ec427544fca7abc335cf3da78160 would cause a dma mask problem (isn't it?) The scsi people might want to restore 55d9fcf57ba5ec427544fca7abc335cf3da78160 and then apply Miquel's patch on top for 2.6.24, or do it for 2.6.25? I think it's a bit late in the game for 2.6.24, so I'm happy to leave the hotplug reverted. We'll try adding back hotplug plus this for 2.6.25 I think. 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] dpt_i2o: don't set DMA_64BIT_MASK [was: Re: [stable] broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)]
On Thu, 2007-12-13 at 11:11 +0100, Miquel van Smoorenburg wrote: According to Greg KH: So, what should be added to 2.6.23-stable then? And, can I get a real changelog entry for it? This is suitable for both 2.6.23.x and 2.6.24-rc5 : linux-2.6-dpt_i2o-no-dma64.patch Actually, this one's already queued: http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=a066b307861238c1970310579c0bc2fe8c8dca51 James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi device recovery
On Wed, 2007-12-12 at 18:54 +0100, Bernd Schubert wrote: [Hmm, resending since mail after more than 30min still not on the ML, maybe the attachment was too large? I have uploaded the log to http://www.pci.uni-heidelberg.de/tc/usr/bernd/downloads/scsi/kern.log.1] On Wednesday 12 December 2007 16:59:36 James Bottomley wrote: On Wed, 2007-12-12 at 15:36 +0100, Bernd Schubert wrote: On Wednesday 12 December 2007 14:39:27 Matthew Wilcox wrote: On Wed, Dec 12, 2007 at 01:54:14PM +0100, Bernd Schubert wrote: below is a patch introducing device recovery, trying to prevent i/o errors when a DID_NO_CONNECT or SOFT_ERROR does happen. Why doesn't the regular scsi_eh do what you need? First of all, it is presently simply not called when the two errors above do happen. This could be changed, of course. Erm, I think you'll find the error handler does activate on DID_SOFT_ERROR. It causes a retry via the eh. DID_NO_CONNECT is an Dec 7 23:48:45 beo-96 kernel: [94605.297924] sd 2:0:5:0: [sdd] Result: hostbyte=DID_SOFT_ERROR driverbyte=DRIVER_OK,SUGGEST_OK Dec 7 23:48:45 beo-96 kernel: [94605.297932] end_request: I/O error, dev sdd, sector 7706802052 Dec 7 23:48:45 beo-96 kernel: [94605.297937] raid5:md5: read error not correctable (sector 871932472 on sdd3). This is some type of ioc internal error. What we do on DID_SOFT_ERROR is retry for the usual number of times up to the timeout limit. Unfortunately, the retries are fixed at SD_MAX_RETRIES in sd.c. Without diagnosing what's going wrong in the fusion, it's impossible to say if this is reasonable, but your fusion is signalling ioc errors (firmware errors). Full log attached. immediate error with no eh intervention because it means that the target went away. Handling this as a retryable error isn't an option because it will interfere with hotplug. Then we need a sysfs flag one can set to manually enable eh for these devices on DID_NO_CONNECT. No, because that will seriously damage a lot of other systems. The DID_NO_CONNECT looks to be a genuine reselection issue caused by a device out of spec on the bus. The SPI standard says a device should respond in 250ms, which is what most HBA's take as the default selection timeout. I'd say for the device you have, you need to increase this. Unfortunately doing this for the fusion is some type of mode page setting, I think, but I don't have the doc in front of me. I'd be amenable to putting the selection timeout as a parameter in the spi transport class, since others might find it valuable occasionally to control. Secondly, I think scsi_eh is in most cases doing too much. We are fighting with flaky Infortrend boxes here, and scsi_eh sometimes manages to crash their scsi channels. In most cases it is sufficient to stall any io to the device and then to resume. But that's basically the default behaviour of the error handler (stall then resume). For most scsi devices one probably doesn't need a suspend time or it can be very small, this still needs to become configurable via sysfs. You mean a wait time beyond what the error handler currently does (basically it waits for the quiesce, begins error handling and then sends a test unit ready when it finishes before restarting). In deh just waits on the first error and then only does a DV. For these infortrend devices, thats mostly sufficient. Thirdly, scsi_eh doesn't give up, in most cases, when the scsi channel of a Infortrend box crashed, it tried forever to recover. To improve this is still on my todo list. Could you send traces for this. I thought the error handler had been fixed over the last few years always to terminate. If there's a case where it doesn't, this needs fixing. I'm attaching the syslog, this is 2.6.22 + additional printks, dump_stack()'s and msleep()'s. At 03:59:36 the system finally went into wait_for_completion(), similar to the everything in wait_for_completion, what is my system doing? thread. This looks like a genuine bug. I missed the thread, since my email system went off line while I was on holiday for two weeks. The symptoms look to be lost commands, but I can't see why from the traces. There's a known bug where we can hang in domain validation because of a resource starvation issue, but I know of none where everything hangs just after error recovery completes. 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: QUEUE_FLAG_CLUSTER: not working in 2.6.24 ?
On Thu, 2007-12-13 at 11:42 -0700, Matthew Wilcox wrote: On Thu, Dec 13, 2007 at 01:37:59PM -0500, Mark Lord wrote: The problem is, the block layer *never* sends an SG entry larger than 8192 bytes, and even that size is exceptionally rare. Nearly all I/O segments are 4096 bytes, so I never see a single I/O larger than 512KB (128 * 4096). If I patch various parts of block and SCSI, this limit doesn't budge, but when I change the hardware PRD limit in libata, it scales by exactly whatever I set the new value to. This tells me that adjacent I/O segments are not being combined. I thought that QUEUE_FLAG_CLUSTER (aka. SCSI host .use_clustering=1) should result in adjacent single pages being combined into larger physical segments? I was recently debugging a driver and noticed that consecutive pages in an sg list are in the reverse order. ie first you get page 918, then 917, 916, 915, 914, etc. I vaguely remember James having patches to correct this, but maybe they weren't merged? Yes, they were ... it was actually Bill Irwin's patch. The old problem was that we fault allocations in reverse order (because we were taking from the end of the zone list). I can't remember when his patches went in, but it was several years ago. After they did, I was getting a 33% chance of physical merging (as opposed to zero before). Probably someone redid the vm or the zones without understanding this and we've gone back to the original position. 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: QUEUE_FLAG_CLUSTER: not working in 2.6.24 ?
On Thu, 2007-12-13 at 14:02 -0800, Andrew Morton wrote: On Thu, 13 Dec 2007 21:09:59 +0100 Jens Axboe [EMAIL PROTECTED] wrote: OK, it's a vm issue, cc linux-mm and probable culprit. I have tens of thousand backward pages after a boot - IOW, bvec-bv_page is the page before bvprv-bv_page, not reverse. So it looks like that bug got reintroduced. Bill Irwin fixed this a couple of years back: changed the page allocator so that it mostly hands out pages in ascending physical-address order. I guess we broke that, quite possibly in Mel's page allocator rework. It would help if you could provide us with a simple recipe for demonstrating this problem, please. The simple way seems to be to malloc a large area, touch every page and then look at the physical pages assigned ... they now mostly seem to be descending in physical address. 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 22/30] sg: nopage
On Thu, 2007-12-13 at 16:14 -0800, [EMAIL PROTECTED] wrote: From: Nick Piggin [EMAIL PROTECTED] Convert SG from nopage to fault. Doug, could you look this over and test it, please? 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 07/24] arm: scsi convert to accessors and !use_sg cleanup
On Tue, 2007-09-18 at 17:04 +0200, Boaz Harrosh wrote:
On Wed, Sep 12 2007 at 10:42 +0300, Russell King [EMAIL PROTECTED] wrote:
On Wed, Sep 12, 2007 at 02:55:19AM +0300, Boaz Harrosh wrote:
- if (SCpnt-request_bufflen != len)
+ if (scsi_bufflen(SCpnt) != len) {
+ WARN_ON(1);
NAK. The call trace generally doesn't provide any additional information
on the cause of the error.
In my opinion this can not happen any more. If it does, I want to see that
it is not through the regular scsi-ml .queuecommand mechanism.
But if you insist than sure I will remove it.
printk(KERN_WARNING scsi%d.%c: bad request buffer
length %d, should be %ld\n,
SCpnt-device-host-host_no,
- '0' + SCpnt-device-id, SCpnt-request_bufflen,
len);
- SCpnt-request_bufflen = len;
+ '0' + SCpnt-device-id, scsi_bufflen(SCpnt),
len);
+ }
#endif
} else {
- SCpnt-SCp.ptr = (unsigned char *)SCpnt-request_buffer;
- SCpnt-SCp.this_residual = SCpnt-request_bufflen;
- SCpnt-SCp.phase = SCpnt-request_bufflen;
- }
-
- /*
- * If the upper SCSI layers pass a buffer, but zero length,
- * we aren't interested in the buffer pointer.
- */
- if (SCpnt-SCp.this_residual == 0 SCpnt-SCp.ptr) {
-#if 0 //def BELT_AND_BRACES
- printk(KERN_WARNING scsi%d.%c: zero length buffer passed for
- command , SCpnt-host-host_no, '0' + SCpnt-target);
- __scsi_print_command(SCpnt-cmnd);
-#endif
SCpnt-SCp.ptr = NULL;
+ SCpnt-SCp.this_residual = 0;
+ SCpnt-SCp.phase = 0;
}
}
Also NAK. This was added due to bad behaviour of the SCSI layer and
was found to be necessary.
No! This check is no longer Relevant. The master if() is on bufflen() now,
and only than do we ever set SCp.ptr. The else will always set both to Zero.
(Which is what you want)
In any way this check is done in scsi-ml, and since 2.6.18 only scsi-ml
can allocate and issue commands. All other sources of commands where removed.
All upper layers issue requests now.
Russell, could you respond to this, please? Boaz's points seem valid to
me and this conversion must be done soon otherwise these drivers will
break.
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: INITIO scsi driver fails to work properly
On Mon, 2007-12-17 at 14:36 +, Alan Cox wrote: On Mon, 17 Dec 2007 16:40:53 +0200 Boaz Harrosh [EMAIL PROTECTED] wrote: On Mon, Dec 17 2007 at 15:05 +0200, Alan Cox [EMAIL PROTECTED] wrote: initio doesn't seem to have a maintainer... Are you able to identify any earlier kernel which worked OK? Maybe it's a new device? If you can get the `lspci -vvxx' output for that device we can take a look. If I remember rightly the fixes for this went into the scsi tree a couple of months ago. The patch is in the -mm tree as well. No idea why its gotten stuck as an obvious one liner. Alan - You mean this one: http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commitdiff;h=ba2c270154cc90c9a8bfc45b7bed4cca78c75aaf It's only queued for 2.6.25 via scsi-misc. I have found another bug. (See other mail in thread). I Will wait for testing and submit a proper patch. That one yes - which really should have gone straight into the main tree as the initio driver has been broken all the time it sits queued for future patches. It can't make the problem any worse - the driver does not work. Well, the change log isn't very committal for rush me immediately into main line plus, as far as I could dig out, there was no confirmation that it actually worked. This way, I can now say please try the current -mm kernel to the bug reporter and we get to see if this fixes the problem. 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: 2.6.24-rc5: tape drive not responding
On Mon, 2007-12-17 at 13:43 -0800, Andrew Morton wrote: On Mon, 17 Dec 2007 16:02:02 -0500 John Stoffel [EMAIL PROTECTED] wrote: Just to confirm, the propsed patch to st.c fixes the issue with 2.6.24-rc5 as well at 2.6.24-rc5-mm1 with access to my DLT tape drives. err, what patch to st.c? That's this one: http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commit;h=acdd0b1c371b2fbb4b6110a51ba69cb0af9e6f45 So it seems that 2.6.24 (and presumably 2.6.23?) need Not 2.6.23 .. the scatterlist changes causing the st problems are local to 2.6.24. 1: Alan's initio: fix conflict when loading driver (currently stocuk in git-scsi-misc) Yes, I'm moving this into scsi-rc-fixes 2: Boaz's initio: initio_build_scb() fix (my name for it) And applying this ... although I'd still appreciate confirmation from someone that the initio driver works after this. 3: The mystery st.c fix. yes? 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
[GIT PATCH] SCSI bug fixes for 2.6.24-rc5
This patch series is a sequence of late arriving bug fixes, mainly in
drivers but also one debugging scatterlist BUG_ON() in the tape ULD.
The initio bug fixes are definitely required, we just don't have
confirmation that they're the whole fix yet, so an additional fix for
initio may be necessary.
The patch is available at:
master.kernel.org:/pub/scm/linux/kernel/git/jejb/scsi-rc-fixes-2.6.git
The short changelog is:
Alan Cox (1):
initio: fix conflict when loading driver
Boaz Harrosh (1):
initio: bugfix for accessors patch
FUJITA Tomonori (1):
st: fix kernel BUG at include/linux/scatterlist.h:59!
James Bottomley (1):
dpt_i2o: driver is only 32 bit so don't set 64 bit DMA mask
Tony Battersby (2):
sym53c8xx: fix irq X: nobody cared regression
sym53c8xx: fix free_irq() regression
The diffstat:
dpt_i2o.c |3 +--
initio.c |2 ++
st.c |1 +
sym53c8xx_2/sym_glue.c |2 +-
sym53c8xx_2/sym_hipd.c |2 +-
5 files changed, 6 insertions(+), 4 deletions(-)
The complete patch is attached below.
James
---
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 70f48a1..b31d1c9 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -906,8 +906,7 @@ static int adpt_install_hba(struct scsi_host_template* sht,
struct pci_dev* pDev
}
pci_set_master(pDev);
- if (pci_set_dma_mask(pDev, DMA_64BIT_MASK)
- pci_set_dma_mask(pDev, DMA_32BIT_MASK))
+ if (pci_set_dma_mask(pDev, DMA_32BIT_MASK))
return -EINVAL;
base_addr0_phys = pci_resource_start(pDev,0);
diff --git a/drivers/scsi/initio.c b/drivers/scsi/initio.c
index 4c4465d..01bf018 100644
--- a/drivers/scsi/initio.c
+++ b/drivers/scsi/initio.c
@@ -2616,6 +2616,7 @@ static void initio_build_scb(struct initio_host * host,
struct scsi_ctrl_blk * c
scsi_for_each_sg(cmnd, sglist, cblk-sglen, i) {
sg-data = cpu_to_le32((u32)sg_dma_address(sglist));
total_len += sg-len =
cpu_to_le32((u32)sg_dma_len(sglist));
+ ++sg;
}
cblk-buflen = (scsi_bufflen(cmnd) total_len) ?
@@ -2867,6 +2868,7 @@ static int initio_probe_one(struct pci_dev *pdev,
}
host = (struct initio_host *)shost-hostdata;
memset(host, 0, sizeof(struct initio_host));
+ host-addr = pci_resource_start(pdev, 0);
if (!request_region(host-addr, 256, i91u)) {
printk(KERN_WARNING initio: I/O port range 0x%x is busy.\n,
host-addr);
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 98dfd6e..328c47c 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -3611,6 +3611,7 @@ static struct st_buffer *
tb-dma = need_dma;
tb-buffer_size = got;
+ sg_init_table(tb-sg, max_sg);
return tb;
}
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c
b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 0f74aba..9e0908d 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1243,7 +1243,7 @@ static void sym_free_resources(struct sym_hcb *np, struct
pci_dev *pdev)
* Free O/S specific resources.
*/
if (pdev-irq)
- free_irq(pdev-irq, np);
+ free_irq(pdev-irq, np-s.host);
if (np-s.ioaddr)
pci_iounmap(pdev, np-s.ioaddr);
if (np-s.ramaddr)
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c
b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 463f119..254bdae 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -2791,7 +2791,7 @@ irqreturn_t sym_interrupt(struct Scsi_Host *shost)
istat = INB(np, nc_istat);
if (istat INTF) {
OUTB(np, nc_istat, (istat SIGP) | INTF | np-istat_sem);
- istat = INB(np, nc_istat); /* DUMMY READ */
+ istat |= INB(np, nc_istat); /* DUMMY READ */
if (DEBUG_FLAGS DEBUG_TINY) printf (F );
sym_wakeup_done(np);
}
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: INITIO scsi driver fails to work properly
On Wed, 2007-12-19 at 06:29 -0700, Matthew Wilcox wrote: On Wed, Dec 19, 2007 at 10:48:27AM +0200, Filippos Papadopoulos wrote: Would it be better to open a bug report at bugzilla? No, it wouldn't. Bugzilla is a place where bug reports go to be ignored. Witness 9370 where despite my best efforts to move discussion to the mailing list, it's been thoroughly ignored because the original reporte insists on posting additional information there instead of to the mailing list. That's a bit harsh on bugzilla. It is of use to people whose job it is to track outstanding bugs. However, Matthew is completely correct, it's useless for getting bugs fixed *if* the information isn't on the mailing list. The reason for using mailing list is the more eyes principle: if you email linux-scsi, all the SCSI experts will see it, not just the one email listed as owner in bugzilla. Likewise, as the bug goes through analysis, if it turns out to be in a different area, that areas mailing list can be added to the Cc list. So, to get the best of both worlds, file a bugzilla and note the bugid. Then email a complete report to the relevant list, but add [BUG bugid] to the subject line and cc [EMAIL PROTECTED] If you do this, bugzilla will keep track of the entire discussion as it progresses and allow those who track bugs through bugzilla to get a pretty accurate idea of the status. You should never need to touch bugzilla again once the initial bug report is filed: all future information flow is via the mailing lists. 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: INITIO scsi driver fails to work properly
On Thu, 2007-12-20 at 01:32 -0800, Natalie Protasevich wrote: On Dec 19, 2007 9:05 AM, Matthew Wilcox [EMAIL PROTECTED] wrote: On Wed, Dec 19, 2007 at 10:50:40AM -0600, James Bottomley wrote: So, to get the best of both worlds, file a bugzilla and note the bugid. Then email a complete report to the relevant list, but add [BUG bugid] to the subject line and cc [EMAIL PROTECTED] If you do this, bugzilla will keep track of the entire discussion as it progresses and allow those who track bugs through bugzilla to get a pretty accurate idea of the status. You should never need to touch bugzilla again once the initial bug report is filed: all future information flow is via the mailing lists. The problem is that it appears to the casual observer as if they can then add information to the bug through the web interface. But that information will never be forwarded to the mailing list. Unless there's a way of marking bugs as 'unchangable through the web interface' or 'all messages appended to this bug need to be forwarded', Bugzilla just doesn't fit our needs. The Debian BTS fits our way of working much better. Perhaps somebody should investigate a migration. This is excellent observation by Matthew and James. There is no magic in bugzilla not being loved, it is just not the right set of features for effective work on a problem. It doesn't support multiple developer' collaboration well. This distaste is not universal, since some people don't have a problem with bugzilla as is, maybe those who tend to work on problems alone... But making it to be a workable tool for everyone is definitely worth it. Any other favorite bugzillas that are nice to work with and that have the advantage above? We have actually been trying for over two years to get bugzilla fixed so that it suits our email and list publishing workflow for fixing bugs. I surmise that 90% of our problems with bugzilla could be solved if it simply tipped a SCSI bug report onto the SCSI list when it was created in such a way that all replies were gathered back into bugzilla. Unfortunately, no-one who maintains our bugzilla has actually been able to make this happen. The other 10% of the problem is that bugzilla doesn't seem to have a way properly to integrate people who insist on using its web interface to reply into the email flow. 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: smp_utils 0.93 beta, adds sgv4 interface
On Fri, 2007-12-21 at 10:10 -0500, Douglas Gilbert wrote: James Bottomley wrote: On Wed, 2007-12-19 at 23:46 -0500, Douglas Gilbert wrote: smp_utils is a package of command line utilities for invoking SMP functions to monitor and manage SAS expanders. SMP is the Serial Attached SCSI (SAS) Management Protocol (SMP). A SAS Host Bus Adapter (HBA) includes a SMP initiator (along with a SSP and STP initiator). A SAS expander contains a SMP target. Previous versions have supported two interfaces: the mptctl pass through used by MPT Fusion SAS HBAs and the smp_portal sysfs attribute pass through used by Luben's aic94xx driver. This beta adds preliminary support for an interface based on the scsi generic version 4 (sgv4) interface. It works, apart from a few shortcomings, with the MPT Fusion SAS driver found in lk 2.6.24-rc4 . My aic94xx based hardware refuses to work with the mainline driver (but works with Luben's driver) so I'm unable to test it with the sgv4 interface. I would be grateful if someone would test it and tell me how it fairs. Since the sgv4 interface essentially talks to the linux SAS transport layer, other SAS HBAs may work as well. I'm pleased to report that it works just fine for me with my aic94xx setup and the sgv4 interface and a couple of LSI expanders (which, unfortunately don't take all the commands ... it looks like you're using the SAS2.0 set). Good. Yes, I am tracking the SAS-2 drafts but they do keep backward compatibility with SAS-1.1 and SAS-1 . See the COVERAGE file in the smp_utils tarball. I'm not aware that LSI have released any SAS-2 expanders. When manufacturers start talking about zoning in their expanders they should be supporting at least some of the extra SAS-2 expander functionality. Yes ... I've no idea when (if) I'll get SAS-2 expanders. I'll also plug in my mpt1068 and see if I can get it to function through the sgv4 interface as well. One of the things which might be useful would be for SAS hosts to respond to SMP requests so they could also be managed with these tools. To do this, we'd have to put a SMP interpreter for them inside the transport class, probably just for a minimal set of SMP frames ... what do people think about this? The MPT Fusion SAS HBAs already supports a virtual SMP target on the HBA itself. Try: smp_rep_manufacturer /sys/class/bsg/sas_host4 where sas_host4 is a MPT SAS HBA. This virtual SMP target is not visible to other devices in the same SAS domain. I believe this allows access to the sideband signals (SGPIO) on a wide internal cable. Some enclosure management chips (e.g. MG9072) offer SES-2 target functionality on a SGPIO interface. So are you proposing that other SAS HBAs (e.g. aic94xx) should add code to their drivers so that the SMP READ and WRITE GPIO REGISTER functions control those SGPIO sideband signals on a wide internal connector? Or did you have something else in mind ... The way I was planning to implement it was to stick an SMP interpreter in libsas (sort of like the way the fusion does it by using the firmware page) and make it translate to the existing phy functions. However, as best as I can discover (at least according to the docs I have) the aic94xx has no external SGPIO bus (although perhaps someone with better documentation knows better?), so support would only be over an expander link (that also means we have no SGPIO support in libsas itself, either). 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: INITIO scsi driver fails to work properly
On Fri, 2007-12-21 at 14:30 -0500, Chuck Ebbert wrote: On 12/19/2007 03:48 AM, Filippos Papadopoulos wrote: On Dec 17, 2007 2:18 PM, Boaz Harrosh [EMAIL PROTECTED] wrote: I have found one problem. Please try patch [2] below and report. If it still fails try to enable debugging by setting with patch [1] these values at top of drivers/scsi/initio.c. And send dmsgs. Boaz I tried patch[2] (addition of sg++) at 2.6.24-rc5-mm1 but the system hangs after some seconds when the initio driver loads. I will try patch[1] next week to see what happens. Would it be better to open a bug report at bugzilla? There is also a Fedora bug report against 2.6.23. The user has applied commit e9e42faf47255274a1ed0b9bf1c46118023ec5fa from 2.6.24-rc plus the two additional fixes under discussion and it hangs for him too. https://bugzilla.redhat.com/show_bug.cgi?id=390531 It really sounds like there's some problem applying the patches. The consistent report throughout is this one: initio: I/O port range 0x0 is busy. Which should be fixed by 99f1f534922a2f2251ba05b14657a1c62882a80e. I didn't actually find that in the bug thread anywhere, but maybe I missed 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 - 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 7/7] sg_ring: convert core ATA code to sg_ring.
On Wed, 2007-12-26 at 17:36 +0900, Tejun Heo wrote: (PS, I haven't followed the sg chaining discussion. Why is sg chaining an optional feature? Performance overhead on low end machines?) The idea of SG chaining is to allow drivers that wish to take advantage of it to increase their transfer lengths beyond MAX_HW_SEGMENTS*PAGE_SIZE by using chaining. However, drivers that stay below MAX_HW_SEGMENTS for the scatterlist length don't need to be altered. The ultimate goal (well, perhaps more wish) is to have all drivers converted, so SCSI can use something small for the default scatterlist sizing and dump all the sglist mempool stuff (although this may never be reached). 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
[PATCH] libsas: add host SMP processing
This patch allows the sas host device to accept SMP commands. This
brings the libsas attached hosts on to a par with the firmware based
ones like mptsas.
James
diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
index c01a40d..18f33cd 100644
--- a/drivers/scsi/libsas/Kconfig
+++ b/drivers/scsi/libsas/Kconfig
@@ -38,6 +38,15 @@ config SCSI_SAS_ATA
Builds in ATA support into libsas. Will necessitate
the loading of libata along with libsas.
+config SCSI_SAS_HOST_SMP
+ bool Support for SMP interpretation for SAS hosts
+ default y
+ depends on SCSI_SAS_LIBSAS
+ help
+ Allows sas hosts to receive SMP frames. Selecting this
+ option builds an SMP interpreter into libsas. Say
+ N here if you want to save the few kb this consumes.
+
config SCSI_SAS_LIBSAS_DEBUG
bool Compile the SAS Domain Transport Attributes in debug mode
default y
diff --git a/drivers/scsi/libsas/Makefile b/drivers/scsi/libsas/Makefile
index fd387b9..60d6e93 100644
--- a/drivers/scsi/libsas/Makefile
+++ b/drivers/scsi/libsas/Makefile
@@ -35,3 +35,4 @@ libsas-y += sas_init.o \
sas_expander.o \
sas_scsi_host.o
libsas-$(CONFIG_SCSI_SAS_ATA) += sas_ata.o
+libsas-$(CONFIG_SCSI_SAS_HOST_SMP) += sas_host_smp.o
\ No newline at end of file
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 27674fe..76555b1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost, struct
sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
- __FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
type = rphy-identify.device_type;
if (type != SAS_EDGE_EXPANDER_DEVICE
diff --git a/drivers/scsi/libsas/sas_host_smp.c
b/drivers/scsi/libsas/sas_host_smp.c
new file mode 100644
index 000..5b9370a
--- /dev/null
+++ b/drivers/scsi/libsas/sas_host_smp.c
@@ -0,0 +1,247 @@
+/*
+ * Serial Attached SCSI (SAS) Expander discovery and configuration
+ *
+ * Copyright (C) 2007 James E.J. Bottomley
+ * [EMAIL PROTECTED]
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 only.
+ */
+#include linux/scatterlist.h
+#include linux/blkdev.h
+
+#include sas_internal.h
+
+#include scsi/scsi_transport.h
+#include scsi/scsi_transport_sas.h
+#include ../scsi_sas_internal.h
+
+static void sas_host_smp_discover(struct sas_ha_struct *sas_ha, u8 *resp_data,
+ u8 phy_id)
+{
+ struct sas_phy *phy;
+ struct sas_rphy *rphy;
+
+ if (phy_id = sas_ha-num_phys) {
+ resp_data[2] = SMP_RESP_NO_PHY;
+ return;
+ }
+ resp_data[2] = SMP_RESP_FUNC_ACC;
+
+ phy = sas_ha-sas_phy[phy_id]-phy;
+ resp_data[9] = phy_id;
+ resp_data[13] = phy-negotiated_linkrate;
+ memcpy(resp_data + 16, sas_ha-sas_addr, SAS_ADDR_SIZE);
+ memcpy(resp_data + 24, sas_ha-sas_phy[phy_id]-attached_sas_addr,
+ SAS_ADDR_SIZE);
+ resp_data[40] = (phy-minimum_linkrate 4) |
+ phy-minimum_linkrate_hw;
+ resp_data[41] = (phy-maximum_linkrate 4) |
+ phy-maximum_linkrate_hw;
+
+ if (!sas_ha-sas_phy[phy_id]-port)
+ return;
+
+ rphy = sas_ha-sas_phy[phy_id]-port-port_dev-rphy;
+ resp_data[12] = rphy-identify.device_type 4;
+ resp_data[14] = rphy-identify.initiator_port_protocols;
+ resp_data[15] = rphy-identify.target_port_protocols;
+}
+
+static void sas_report_phy_sata(struct sas_ha_struct *sas_ha, u8 *resp_data,
+ u8 phy_id)
+{
+ struct sas_rphy *rphy;
+ struct dev_to_host_fis *fis;
+ int i;
+
+ if (phy_id = sas_ha-num_phys) {
+ resp_data[2] = SMP_RESP_NO_PHY;
+ return;
+ }
+
+ resp_data[2] = SMP_RESP_PHY_NO_SATA;
+
+ if (!sas_ha-sas_phy[phy_id]-port)
+ return;
+
+ rphy = sas_ha-sas_phy[phy_id]-port-port_dev-rphy;
+ fis = (struct dev_to_host_fis *)
+ sas_ha-sas_phy[phy_id]-port-port_dev-frame_rcvd;
+ if (rphy-identify.target_port_protocols != SAS_PROTOCOL_SATA)
+ return;
+
+ resp_data[2] = SMP_RESP_FUNC_ACC;
+ resp_data[9] = phy_id;
+ memcpy(resp_data + 16, sas_ha-sas_phy[phy_id]-attached_sas_addr,
+ SAS_ADDR_SIZE);
+
+ /* check to see if we have a valid d2h fis */
+ if (fis-fis_type != 0x34)
+
Re: [PATCH] libsas: add host SMP processing
On Sat, 2007-12-29 at 14:24 +0900, FUJITA Tomonori wrote:
From: James Bottomley [EMAIL PROTECTED]
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost, struct
sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
- __FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
I have one related question.
Currently, bsg doesn't return an error to user space since I had no
idea how to convert errors such as EINVAL and ENOMEM into
driver_status, transport_status, and device_status in struct sg_io_v4.
I think that it's confusing that bsg don't return an error even if SMP
requests aren't sent (e.g. devices are offline).
Do we need to map errors to the current error code in scsi/scsi.h
(like DID_*) or define a new one for SMP?
Neither, I think ... the DID codes are only for things that actually
pass through the SCSI stack. The way you implemented the smp functions
in bsg, they're direct queue handlers themselves (Incidentally, that's
another point about this: I think almost every use of bsg like this is
going to be for SG_IO only, so it makes sense to move the actual queue
handler into bsg, since they'll all share it).
The attached is the simplest patch that implements this. However, it
unfortunately can't be applied yet ... the current SMP tools send
receive buffers too large and libsas actually returns a data underrun
error (which is now propagated).
I'll fix it with a series of patches transferring the protocol handler
into bsg, adding error handling and correcting libsas.
James
diff --git a/block/bsg.c b/block/bsg.c
index 8e181ab..fbf0be3 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -837,6 +837,7 @@ static long bsg_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
struct bsg_device *bd = file-private_data;
int __user *uarg = (int __user *) arg;
+ int ret;
switch (cmd) {
/*
@@ -889,12 +890,13 @@ static long bsg_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
if (rq-next_rq)
bidi_bio = rq-next_rq-bio;
blk_execute_rq(bd-queue, NULL, rq, 0);
+ ret = rq-errors;
blk_complete_sgv4_hdr_rq(rq, hdr, bio, bidi_bio);
if (copy_to_user(uarg, hdr, sizeof(hdr)))
return -EFAULT;
- return 0;
+ return ret;
}
/*
* block device ioctls
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index 87e786d..f2149d0 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -173,6 +173,7 @@ static void sas_smp_request(struct request_queue *q, struct
Scsi_Host *shost,
handler = to_sas_internal(shost-transportt)-f-smp_handler;
ret = handler(shost, rphy, req);
+ req-errors = ret;
spin_lock_irq(q-queue_lock);
-
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] libsas: add host SMP processing
On Sun, 2007-12-30 at 01:06 +0900, FUJITA Tomonori wrote:
On Sat, 29 Dec 2007 09:44:32 -0600
James Bottomley [EMAIL PROTECTED] wrote:
On Sat, 2007-12-29 at 14:24 +0900, FUJITA Tomonori wrote:
From: James Bottomley [EMAIL PROTECTED]
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost,
struct sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
- __FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
I have one related question.
Currently, bsg doesn't return an error to user space since I had no
idea how to convert errors such as EINVAL and ENOMEM into
driver_status, transport_status, and device_status in struct sg_io_v4.
I think that it's confusing that bsg don't return an error even if SMP
requests aren't sent (e.g. devices are offline).
Do we need to map errors to the current error code in scsi/scsi.h
(like DID_*) or define a new one for SMP?
Neither, I think ... the DID codes are only for things that actually
pass through the SCSI stack. The way you implemented the smp functions
in bsg, they're direct queue handlers themselves (Incidentally, that's
another point about this: I think almost every use of bsg like this is
going to be for SG_IO only, so it makes sense to move the actual queue
handler into bsg, since they'll all share it).
The attached is the simplest patch that implements this. However, it
unfortunately can't be applied yet ... the current SMP tools send
receive buffers too large and libsas actually returns a data underrun
error (which is now propagated).
bsg read/write interface doens't return errors in this way (compatible
with sg3 read/write interface). If we support only SG_IO for non SCSI
request/response protocols, then that's fine.
OK, guilty of disliking the read/write interface (and therefore not
thinking about it). However, it's easy to fix. How about this? Note,
I think returning errors when they occur is more important than
preserving compatibility and swallowing the error somewhere, especially
as the bsg v3 interface *only* dealt with SCSI, so this is more like an
extension to non-scsi error returning. But if I'd had my way, bsg
wouldn't have had a read/write interface, so I'm happy to do whatever
people who actually use it want.
James
diff --git a/block/bsg.c b/block/bsg.c
index 8e181ab..69b0a9d 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -445,6 +445,15 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq,
struct sg_io_v4 *hdr,
else
hdr-dout_resid = rq-data_len;
+ /*
+* If the request generated a negative error number, return it
+* (providing we aren't already returning an error); if it's
+* just a protocol response (i.e. non negative), that gets
+* processed above.
+*/
+ if (!ret rq-errors 0)
+ ret = rq-errors;
+
blk_rq_unmap_user(bio);
blk_put_request(rq);
@@ -837,6 +846,7 @@ static long bsg_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
struct bsg_device *bd = file-private_data;
int __user *uarg = (int __user *) arg;
+ int ret;
switch (cmd) {
/*
@@ -889,12 +899,12 @@ static long bsg_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
if (rq-next_rq)
bidi_bio = rq-next_rq-bio;
blk_execute_rq(bd-queue, NULL, rq, 0);
- blk_complete_sgv4_hdr_rq(rq, hdr, bio, bidi_bio);
+ ret = blk_complete_sgv4_hdr_rq(rq, hdr, bio, bidi_bio);
if (copy_to_user(uarg, hdr, sizeof(hdr)))
return -EFAULT;
- return 0;
+ return ret;
}
/*
* block device ioctls
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index 87e786d..f2149d0 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -173,6 +173,7 @@ static void sas_smp_request(struct request_queue *q, struct
Scsi_Host *shost,
handler = to_sas_internal(shost-transportt)-f-smp_handler;
ret = handler(shost, rphy, req);
+ req-errors = ret;
spin_lock_irq(q-queue_lock);
-
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] libsas: don't treat underrun as an error on SMP tasks
On Sun, 2007-12-30 at 19:34 +0900, FUJITA Tomonori wrote:
On Sat, 29 Dec 2007 11:49:53 -0600
James Bottomley [EMAIL PROTECTED] wrote:
All SMP tasks sent through bsg generate messages like:
sas: smp_execute_task: task to dev 500605b01450 response: 0x0 status
0x81
Three times (because the task gets retried). Firstly, don't retry
either overrun or underrun (the data buffer isn't going to change size)
and secondly, just report the underrun but don't set an error for it.
This is necessary so bsg can report back the residual.
James
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 76555b1..1578059 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -109,6 +109,16 @@ static int smp_execute_task(struct domain_device *dev,
void *req, int req_size,
task-task_status.stat == SAM_GOOD) {
res = 0;
break;
+ } if (task-task_status.resp == SAS_TASK_COMPLETE
+ task-task_status.stat == SAS_DATA_UNDERRUN) {
+ /* no error, but return the number of bytes of
+* underrun */
+ res = task-task_status.residual;
+ break;
+ } if (task-task_status.resp == SAS_TASK_COMPLETE
+ task-task_status.stat == SAS_DATA_OVERRUN) {
+ res = -EMSGSIZE;
+ break;
} else {
SAS_DPRINTK(%s: task to dev %016llx response: 0x%x
status 0x%x\n, __FUNCTION__,
@@ -1924,6 +1934,11 @@ int sas_smp_handler(struct Scsi_Host *shost, struct
sas_rphy *rphy,
ret = smp_execute_task(dev, bio_data(req-bio), req-data_len,
bio_data(rsp-bio), rsp-data_len);
+ if (ret 0) {
+ /* positive number is the untransferred residual */
+ rsp-data_len = ret;
+ ret = 0;
+ }
Would be better to update dout_resid too (on sucess, we can set
req-data_len to zero, I think)?
Yes, I'll add that.
Here's a patch to do the same thing for mpt sas, an updated version
of:
http://marc.info/?l=linux-scsim=119811872823947w=2
And this.
James
--
From: FUJITA Tomonori [EMAIL PROTECTED]
Subject: [PATCH] mpt fusion: mptsas_smp_handler updates resid
This patch fixes mptsas_smp_handler to update both din_resid or
dout_resid on success. bsg can report back the residual.
Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
drivers/message/fusion/mptsas.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index e4c94f9..f77b329 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1343,6 +1343,8 @@ static int mptsas_smp_handler(struct Scsi_Host *shost,
struct sas_rphy *rphy,
smprep = (SmpPassthroughReply_t *)ioc-sas_mgmt.reply;
memcpy(req-sense, smprep, sizeof(*smprep));
req-sense_len = sizeof(*smprep);
+ req-data_len = 0;
+ rsp-data_len -= smprep-ResponseDataLength;
} else {
printk(MYIOC_s_ERR_FMT %s: smp passthru reply failed to be
returned\n,
ioc-name, __FUNCTION__);
-
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] libsas: don't use made up error codes
This is bad for two reasons:
1. If they're returned to outside applications, no-one knows what
they mean.
2. Eventually they'll clash with the ever expanding standard error
codes.
The problem error code in question is ETASK. I've replaced this by
ECOMM (communications error on send) a network error code that seems to
most closely relay what ETASK meant.
James
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 0829b55..adc47d4 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -500,7 +500,7 @@ static int sas_execute_task(struct sas_task *task, void
*buffer, int size,
goto ex_err;
}
wait_for_completion(task-completion);
- res = -ETASK;
+ res = -ECOMM;
if (task-task_state_flags SAS_TASK_STATE_ABORTED) {
int res2;
SAS_DPRINTK(task aborted, flags:0x%x\n,
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 8aeaad9..aefd865 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -96,7 +96,7 @@ static int smp_execute_task(struct domain_device *dev, void
*req, int req_size,
}
wait_for_completion(task-completion);
- res = -ETASK;
+ res = -ECOMM;
if ((task-task_state_flags SAS_TASK_STATE_ABORTED)) {
SAS_DPRINTK(smp task timed out or aborted\n);
i-dft-lldd_abort_task(task);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 93248cd..a075f13 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -91,8 +91,6 @@ enum discover_event {
/* -- Expander Devices -- */
-#define ETASK 0xFA
-
#define to_dom_device(_obj) container_of(_obj, struct domain_device, dev_obj)
#define to_dev_attr(_attr) container_of(_attr, struct domain_dev_attribute,\
attr)
-
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] libsas: don't use made up error codes
On Mon, 2007-12-31 at 13:25 -0800, Darrick J. Wong wrote: On Sun, Dec 30, 2007 at 12:37:31PM -0600, James Bottomley wrote: This is bad for two reasons: 1. If they're returned to outside applications, no-one knows what they mean. 2. Eventually they'll clash with the ever expanding standard error codes. The problem error code in question is ETASK. I've replaced this by ECOMM (communications error on send) a network error code that seems to most closely relay what ETASK meant. Yay, cleanups :) Don't get your hopes up ... this was just one that unavoidably crossed my attention path while I was doing something else. I'm not going out looking for these things ... mainly for fear of finding them in large quantities. 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
[PATCH] libata: eliminate the home grown dma padding in favour of that provided by the block layer
ATA requires that all DMA transfers begin and end on word boundaries.
Because of this, a large amount of machinery grew up in ide to adjust
scatterlists on this basis. However, as of 2.5, the block layer has a
dma_alignment variable which ensures both the beginning and length of a
DMA transfer are aligned on the dma_alignment boundary. Although the
block layer does adjust the beginning of the transfer to ensure this
happens, it doesn't actually adjust the length, it merely makes sure
that space is allocated for transfers beyond the declared length. The
upshot of this is that scatterlists may be padded to any size between
the actual length and the length adjusted to the dma_alignment safely
knowing that memory is allocated in this region.
Right at the moment, SCSI takes the default dma_aligment which is on a
512 byte boundary. Note that this aligment only applies to transfers
coming in from user space. However, since all kernel allocations are
automatically aligned on a minimum of 32 byte boundaries, it is safe to
adjust them in this manner as well.
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
This also fixes a current panic in libsas with SATAPI devices. I was
trying to trace a bad interaction between the libata padding and the new
sglist code which was the root cause, and ended up concluding that the
whole libata padding thing was redundant.
In a future improvement, I think we can relax SCSI dma_alignemnt (it's
causing almost every user space command to be copied instead of directly
passed through) and allow devices and transports to build up their own
requirements instead.
James
drivers/ata/ahci.c|5 -
drivers/ata/libata-core.c | 126 +++---
drivers/ata/libata-scsi.c | 18 +-
drivers/ata/pata_icside.c |8 --
drivers/ata/sata_fsl.c| 17 -
drivers/ata/sata_mv.c |5 -
drivers/ata/sata_nv.c |2
drivers/ata/sata_promise.c|2
drivers/ata/sata_qstor.c |2
drivers/ata/sata_sil24.c |5 -
drivers/scsi/ipr.c|4 -
drivers/scsi/libsas/sas_ata.c |4 -
include/linux/libata.h| 35 ---
13 files changed, 25 insertions(+), 208 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 54f38c2..4dd318d 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1965,16 +1965,11 @@ static int ahci_port_start(struct ata_port *ap)
struct ahci_port_priv *pp;
void *mem;
dma_addr_t mem_dma;
- int rc;
pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
if (!pp)
return -ENOMEM;
- rc = ata_pad_alloc(ap, dev);
- if (rc)
- return rc;
-
mem = dmam_alloc_coherent(dev, AHCI_PORT_PRIV_DMA_SZ, mem_dma,
GFP_KERNEL);
if (!mem)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 4753a18..97db82f 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -60,6 +60,8 @@
#include linux/io.h
#include scsi/scsi.h
#include scsi/scsi_cmnd.h
+#include scsi/scsi_device.h
+#include scsi/scsi_dbg.h
#include scsi/scsi_host.h
#include linux/libata.h
#include asm/semaphore.h
@@ -4464,7 +4466,6 @@ void ata_sg_clean(struct ata_queued_cmd *qc)
struct ata_port *ap = qc-ap;
struct scatterlist *sg = qc-__sg;
int dir = qc-dma_dir;
- void *pad_buf = NULL;
WARN_ON(!(qc-flags ATA_QCFLAG_DMAMAP));
WARN_ON(sg == NULL);
@@ -4474,34 +4475,14 @@ void ata_sg_clean(struct ata_queued_cmd *qc)
VPRINTK(unmapping %u sg elements\n, qc-n_elem);
- /* if we padded the buffer out to 32-bit bound, and data
-* xfer direction is from-device, we must copy from the
-* pad buffer back into the supplied buffer
-*/
- if (qc-pad_len !(qc-tf.flags ATA_TFLAG_WRITE))
- pad_buf = ap-pad + (qc-tag * ATA_DMA_PAD_SZ);
-
if (qc-flags ATA_QCFLAG_SG) {
if (qc-n_elem)
dma_unmap_sg(ap-dev, sg, qc-n_elem, dir);
- /* restore last sg */
- sg_last(sg, qc-orig_n_elem)-length += qc-pad_len;
- if (pad_buf) {
- struct scatterlist *psg = qc-pad_sgent;
- void *addr = kmap_atomic(sg_page(psg), KM_IRQ0);
- memcpy(addr + psg-offset, pad_buf, qc-pad_len);
- kunmap_atomic(addr, KM_IRQ0);
- }
} else {
if (qc-n_elem)
dma_unmap_single(ap-dev,
sg_dma_address(sg[0]), sg_dma_len(sg[0]),
dir);
- /* restore sg */
- sg-length += qc-pad_len;
- if (pad_buf)
- memcpy(qc-buf_virt + sg-length - qc-pad_len,
- pad_buf, qc-pad_len
Re: [PATCH 2/2] relax scsi dma alignment
On Tue, 2008-01-01 at 11:08 -0500, Pete Wyckoff wrote:
[EMAIL PROTECTED] wrote on Mon, 31 Dec 2007 16:43 -0600:
This patch relaxes the default SCSI DMA alignment from 512 bytes to 4
bytes. I remember from previous discussions that usb and firewire have
sector size alignment requirements, so I upped their alignments in the
respective slave allocs.
The reason for doing this is so that we don't get such a huge amount of
copy overhead in bio_copy_user() for udev. (basically all inquiries it
issues can now be directly mapped).
Great change. Here's another patch. It allows a queue
dma_alignment of 0 bytes, for drivers that can move data
at byte granularity.
Two drivers try to turn off DMA alignment restrictions by
setting the dma_alignment to zero:
drivers/scsi/iscsi_tcp.c: blk_queue_dma_alignment(sdev-request_queue, 0);
drivers/scsi/scsi_tgt_lib.c: blk_queue_dma_alignment(q, 0);
But they end up doing copies due to the way that queue_dma_alignment()
returns 511 in this case.
-- Pete
---
From 90ee6d61ad71a024815eee2b416edb40c6b01256 Mon Sep 17 00:00:00 2001
From: Pete Wyckoff [EMAIL PROTECTED]
Date: Tue, 1 Jan 2008 10:23:02 -0500
Subject: [PATCH] block: allow queue dma_alignment of zero
Let queue_dma_alignment return 0 if it was specifically set to 0.
This permits devices with no particular alignment restrictions to
use arbitrary user space buffers without copying.
Signed-off-by: Pete Wyckoff [EMAIL PROTECTED]
---
include/linux/blkdev.h |7 +--
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index aa3090c..eea31c2 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -860,12 +860,7 @@ static inline int bdev_hardsect_size(struct block_device
*bdev)
static inline int queue_dma_alignment(struct request_queue *q)
{
- int retval = 511;
-
- if (q q-dma_alignment)
- retval = q-dma_alignment;
-
- return retval;
+ return q ? q-dma_alignment : 511;
}
/* assumes size 256 */
This is certainly a possible patch. What assurance do you have that it
doesn't upset block devices who set zero assuming they get the default
alignment?
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: [Bugme-new] [Bug 9674] New: Oops during rmmod'ing modeuls sdhci, sr_mod, ricoh_mmc, mmc_core
On Tue, 2008-01-01 at 18:10 -0800, Andrew Morton wrote: On Tue, 1 Jan 2008 14:55:45 -0800 (PST) [EMAIL PROTECTED] wrote: http://bugzilla.kernel.org/show_bug.cgi?id=9674 Summary: Oops during rmmod'ing modeuls sdhci, sr_mod, ricoh_mmc, mmc_core Guys, this is a very recent regression. Could you please take a look, see if it's due to mmc, block or scsi changes? There's not a lot of information to go on. The stack trace looks bogus, so I guess the kernel is compiled without a frame pointer. However, it does look like the initial insertion of sr_mod is going through and it generates a command which gets into scsi_request_fn and then indirects through a bogus queueucommand pointer. What's the actual underlying device the cdrom is attached to? There's no real changes to SCSI in this area from 2.6.24-rc4 ... however, the reinsertion is suggestive, it's like the removal is retriggering a module request for some reason. 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: [Bugme-new] [Bug 9674] New: Oops during rmmod'ing modeuls sdhci, sr_mod, ricoh_mmc, mmc_core
On Wed, 2008-01-02 at 13:21 +0100, Rafael J. Wysocki wrote: On Wednesday, 2 of January 2008, James Bottomley wrote: On Tue, 2008-01-01 at 18:10 -0800, Andrew Morton wrote: On Tue, 1 Jan 2008 14:55:45 -0800 (PST) [EMAIL PROTECTED] wrote: http://bugzilla.kernel.org/show_bug.cgi?id=9674 Summary: Oops during rmmod'ing modeuls sdhci, sr_mod, ricoh_mmc, mmc_core Guys, this is a very recent regression. Could you please take a look, see if it's due to mmc, block or scsi changes? There's not a lot of information to go on. The stack trace looks bogus, so I guess the kernel is compiled without a frame pointer. The bug report has been updated with a stack trace from a kernel compiled with a frame pointer. Please have a look. Please, please don't do this. Filing something in bugzilla is tantamount to putting it in the file and forget folder. The reason I cc'd the SCSI mailing list and asked for more details is so that we get the email flow that might trigger direct interaction between the reporter and someone on the list who recognised the symptoms. Let me say again, catagorically, that if you want to give a bug the best chance of being fixed, the correct flow of information is: file a bugzilla and note the bugid. Then email a complete report to the relevant list, but add [BUG bugid] to the subject line and cc [EMAIL PROTECTED] If you do this, bugzilla will keep track of the entire discussion as it progresses and allow those who track bugs through bugzilla to get a pretty accurate idea of the status. You should never need to touch bugzilla again once the initial bug report is filed: all future information flow is via the mailing lists. Also, using urls unless for historical purposes is also a killer. Many people travel, and their MO is to download the email and read it on the plane/train/whatever. If you embed a url containing critical information, the email gets marked as read, but since I can't get to the information, nothing happens. Then it gets forgotten. This is the relevant piece of information that should have been on the mailing list: [ 101.359083] Unable to handle kernel paging request at 88021cc0 RIP: [ 101.359092] [88021cc0] [ 101.359099] PGD 203067 PUD 207063 PMD 3d34a067 PTE 0 [ 101.359108] Oops: 0010 [1] PREEMPT SMP [ 101.359115] CPU 0 [ 101.359118] Modules linked in: sr_mod tcp_westwood ipt_REJECT xt_state iptable_filter ipt_owner ipt_MASQUERADE xt_tcpudp xt_multiport iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack ip_tables x_tables iwl3945 ricoh_mmc cdrom [ 101.359150] Pid: 4496, comm: modprobe Not tainted 2.6.24-rc6-git7 #5 [ 101.359154] RIP: 0010:[88021cc0] [88021cc0] [ 101.359159] RSP: 0018:81002b457970 EFLAGS: 00010086 [ 101.359163] RAX: 88021cc0 RBX: 81003f1627e0 RCX: 81003f023b38 [ 101.359167] RDX: RSI: 810030efd000 RDI: 81003f1627e0 [ 101.359171] RBP: 81002b4579b8 R08: 0001 R09: 0001 [ 101.359175] R10: R11: R12: 810030efd000 [ 101.359179] R13: 81002b457988 R14: 0010 R15: 00010010 [ 101.359185] FS: 2adcf7ea0b00() GS:80733000() knlGS: [ 101.359189] CS: 0010 DS: ES: CR0: 8005003b [ 101.359193] CR2: 88021cc0 CR3: 2b497000 CR4: 06e0 [ 101.359197] DR0: DR1: DR2: [ 101.359201] DR3: DR6: 0ff0 DR7: 0400 [ 101.359206] Process modprobe (pid: 4496, threadinfo 81002b456000, task 81003de1ef50) [ 101.359210] Stack: 80333a98 0086 81003f023af8 810030efd000 [ 101.359221] 81003f1627e0 81003f023800 81003e31c000 81003f1627e0 [ 101.359230] 81002b457a08 803fe7e1 81003f023b38 [ 101.359237] Call Trace: [ 101.359248] [80333a98] elv_next_request+0xe8/0x180 [ 101.359256] [803fe7e1] scsi_request_fn+0x71/0x380 [ 101.359264] [803375b8] __generic_unplug_device+0x28/0x30 [ 101.359270] [80337623] blk_execute_rq_nowait+0x63/0xb0 [ 101.359276] [80339113] blk_execute_rq+0x73/0xe0 [ 101.359283] [80337775] get_request_wait+0x25/0x120 [ 101.359288] [80337896] blk_get_request+0x26/0x80 [ 101.359296] [803fe5b2] scsi_execute+0xe2/0x110 [ 101.359301] [803fe661] scsi_execute_req+0x81/0xf0 [ 101.359312] [8800d713] :sr_mod:sr_probe+0x1e3/0x630 [ 101.359323] [803c8d01] driver_probe_device+0xa1/0x1c0 [ 101.359329] [803c8ff5] __driver_attach+0xe5/0xf0 [ 101.359334] [803c8f10] __driver_attach+0x0/0xf0 [ 101.359342] [803c7ee3] bus_for_each_dev+0x53/0x80 [ 101.359348] [803c8b3c] driver_attach+0x1c/0x20 [ 101.359353] [803c8305
Re: [Bugme-new] [Bug 9674] New: Oops during rmmod'ing modeuls sdhci, sr_mod, ricoh_mmc, mmc_core
On Wed, 2008-01-02 at 10:49 -0500, Pete Wyckoff wrote:
[EMAIL PROTECTED] wrote on Tue, 01 Jan 2008 21:24 -0600:
On Tue, 2008-01-01 at 18:10 -0800, Andrew Morton wrote:
On Tue, 1 Jan 2008 14:55:45 -0800 (PST) [EMAIL PROTECTED] wrote:
http://bugzilla.kernel.org/show_bug.cgi?id=9674
Summary: Oops during rmmod'ing modeuls sdhci, sr_mod,
ricoh_mmc,
mmc_core
Guys, this is a very recent regression. Could you please take a look, see
if it's due to mmc, block or scsi changes?
There's not a lot of information to go on. The stack trace looks bogus,
so I guess the kernel is compiled without a frame pointer. However, it
does look like the initial insertion of sr_mod is going through and it
generates a command which gets into scsi_request_fn and then indirects
through a bogus queueucommand pointer.
Bogus prep_rq_fn actually.
What's the actual underlying device the cdrom is attached to?
There's no real changes to SCSI in this area from 2.6.24-rc4 ...
however, the reinsertion is suggestive, it's like the removal is
retriggering a module request for some reason.
Here's a guess. When sr_mod is removed, it looks like the request
queue prep_rq_fn is still pointing to the now nonexistent
sr_prep_fn. This may have been due to a commit that went in early
2.6.24:
commit 7f9a6bc4e9d59e7fcf03ed23f60cd81ca5d80b65
Author: James Bottomley [EMAIL PROTECTED]
Date: Sat Aug 4 10:06:25 2007 -0500
[SCSI] move ULD attachment into the prep function
One of the intents of the block prep function was to allow ULDs to use
it for preprocessing. The original SCSI model was to have a single prep
function and add a pointer indirect filter to build the necessary
commands. This patch reverses that, does away with the init_command
field of the scsi_driver structure and makes ULDs attach directly to the
prep function instead. The value is really that it allows us to begin
to separate the ULDs from the SCSI mid layer (as long as they don't use
any core functions---which is hard at the moment---a ULD doesn't even
need SCSI to bind).
Acked-by: Jens Axboe [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
When the module is re-inserted, it does a few SCSI commands before
setting up the new prep_rq_fn, presumably hitting this bogus
pointer.
One fix would be to have sr remember the original prep function and
restore it in sr_kref_release. Sd and a few other drivers have this
issue. Ide-cd bothers to set prep_rq_fn to NULL as it releases
the device.
Bingo .. that's why we ask the list, thanks Pete!
I don't think the fix is the correct one, but I've attached what I think
is the correct fix (basically, there's a bus callback we can use to
ensure the right thing always gets done rather than relying on drivers
doing it in their own release methods, that way they can't forget).
The reason it was showing up in -rc4 I suspect is because something was
structurally altering the module stack, and the address that sr_mod was
loaded was changed, so the prep_fn as Pete said was pointing into bogus
address space.
The way to trigger this bug 100% of the time is to rmmod sr_mod and then
send an inquiry (or another command) to the device using the sg node.
James
---
Index: BUILD-2.6/drivers/scsi/scsi_lib.c
===
--- BUILD-2.6.orig/drivers/scsi/scsi_lib.c 2008-01-01 10:13:33.0
-0600
+++ BUILD-2.6/drivers/scsi/scsi_lib.c 2008-01-02 10:17:51.0 -0600
@@ -1324,7 +1324,7 @@ int scsi_prep_return(struct request_queu
}
EXPORT_SYMBOL(scsi_prep_return);
-static int scsi_prep_fn(struct request_queue *q, struct request *req)
+int scsi_prep_fn(struct request_queue *q, struct request *req)
{
struct scsi_device *sdev = q-queuedata;
int ret = BLKPREP_KILL;
Index: BUILD-2.6/drivers/scsi/scsi_priv.h
===
--- BUILD-2.6.orig/drivers/scsi/scsi_priv.h 2007-11-03 09:08:46.0
-0500
+++ BUILD-2.6/drivers/scsi/scsi_priv.h 2008-01-02 10:20:09.0 -0600
@@ -74,6 +74,9 @@ extern struct request_queue *scsi_alloc_
extern void scsi_free_queue(struct request_queue *q);
extern int scsi_init_queue(void);
extern void scsi_exit_queue(void);
+struct request_queue;
+struct request;
+extern int scsi_prep_fn(struct request_queue *, struct request *);
/* scsi_proc.c */
#ifdef CONFIG_SCSI_PROC_FS
Index: BUILD-2.6/drivers/scsi/scsi_sysfs.c
===
--- BUILD-2.6.orig/drivers/scsi/scsi_sysfs.c2007-11-03 10:08:02.0
-0500
+++ BUILD-2.6/drivers/scsi/scsi_sysfs.c 2008-01-02 10:31:33.0 -0600
@@ -373,12 +373,24 @@ static int scsi_bus_resume(struct device
return err;
}
+static int scsi_bus_remove
[PATCH] scsi_sysfs: restore prep_fn when ULD is removed
A recent bug report:
http://bugzilla.kernel.org/show_bug.cgi?id=9674
Was caused because the ULDs now set their own prep functions, but don't
necessarily reset the prep function back to the SCSI default when they
are removed. This leads to panics if commands are sent to the device
after the module is removed because the prep_fn is still pointing to the
old module code. The fix for this is to implement a bus remove method
that resets the prep_fn pointer correctly before calling the driver
specific prep_fn.
James
P.S. The patch in the bug report is slightly wrong. It doesn't include
the piece to call the ULD specific remove function.
Index: BUILD-2.6/drivers/scsi/scsi_lib.c
===
--- BUILD-2.6.orig/drivers/scsi/scsi_lib.c 2008-01-01 10:13:33.0
-0600
+++ BUILD-2.6/drivers/scsi/scsi_lib.c 2008-01-02 10:17:51.0 -0600
@@ -1324,7 +1324,7 @@ int scsi_prep_return(struct request_queu
}
EXPORT_SYMBOL(scsi_prep_return);
-static int scsi_prep_fn(struct request_queue *q, struct request *req)
+int scsi_prep_fn(struct request_queue *q, struct request *req)
{
struct scsi_device *sdev = q-queuedata;
int ret = BLKPREP_KILL;
Index: BUILD-2.6/drivers/scsi/scsi_priv.h
===
--- BUILD-2.6.orig/drivers/scsi/scsi_priv.h 2007-11-03 09:08:46.0
-0500
+++ BUILD-2.6/drivers/scsi/scsi_priv.h 2008-01-02 10:20:09.0 -0600
@@ -74,6 +74,9 @@ extern struct request_queue *scsi_alloc_
extern void scsi_free_queue(struct request_queue *q);
extern int scsi_init_queue(void);
extern void scsi_exit_queue(void);
+struct request_queue;
+struct request;
+extern int scsi_prep_fn(struct request_queue *, struct request *);
/* scsi_proc.c */
#ifdef CONFIG_SCSI_PROC_FS
Index: BUILD-2.6/drivers/scsi/scsi_sysfs.c
===
--- BUILD-2.6.orig/drivers/scsi/scsi_sysfs.c2007-11-03 10:08:02.0
-0500
+++ BUILD-2.6/drivers/scsi/scsi_sysfs.c 2008-01-02 10:58:17.0 -0600
@@ -373,12 +373,29 @@ static int scsi_bus_resume(struct device
return err;
}
+static int scsi_bus_remove(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ struct scsi_device *sdev = to_scsi_device(dev);
+ int err = 0;
+
+ /* reset the prep_fn back to the default since the
+* driver may have altered it and it's being removed */
+ blk_queue_prep_rq(sdev-request_queue, scsi_prep_fn);
+
+ if (drv drv-remove)
+ err = drv-remove(dev);
+
+ return 0;
+}
+
struct bus_type scsi_bus_type = {
.name = scsi,
.match = scsi_bus_match,
.uevent = scsi_bus_uevent,
.suspend= scsi_bus_suspend,
.resume = scsi_bus_resume,
+ .remove = scsi_bus_remove,
};
int scsi_sysfs_register(void)
-
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] libata: eliminate the home grown dma padding in favour of that provided by the block layer
On Thu, 2008-01-03 at 16:58 +0900, FUJITA Tomonori wrote:
On Mon, 31 Dec 2007 15:56:08 -0600
James Bottomley [EMAIL PROTECTED] wrote:
@@ -1080,24 +1073,15 @@ extern void ata_port_pbar_desc(struct ata_port *ap,
int bar, ssize_t offset,
static inline struct scatterlist *
ata_qc_first_sg(struct ata_queued_cmd *qc)
{
- qc-n_iter = 0;
if (qc-n_elem)
return qc-__sg;
- if (qc-pad_len)
- return qc-pad_sgent;
return NULL;
}
static inline struct scatterlist *
ata_qc_next_sg(struct scatterlist *sg, struct ata_queued_cmd *qc)
{
- if (sg == qc-pad_sgent)
- return NULL;
- if (++qc-n_iter qc-n_elem)
- return sg_next(sg);
- if (qc-pad_len)
- return qc-pad_sgent;
- return NULL;
+ return sg_next(sg);
}
#define ata_for_each_sg(sg, qc) \
How about removing ata_qc_first_sg and ata_qc_next_sg completely?
Now we can just replace ata_qc_next_sg with sg_next. qc-__sg seems to
be always initialized to NULL so we can remove ata_qc_first_sg too.
Sure ... I assumed (without actually looking) that the inlines were
there because they were an API used throughout the drivers. Actually,
grep tells me they're only used in the ata_for_each_sg macro, so this
patch looks good.
Thanks,
James
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 4f6404c..2774882 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1054,25 +1054,8 @@ extern void ata_port_pbar_desc(struct ata_port *ap,
int bar, ssize_t offset,
const char *name);
#endif
-/*
- * qc helpers
- */
-static inline struct scatterlist *
-ata_qc_first_sg(struct ata_queued_cmd *qc)
-{
- if (qc-n_elem)
- return qc-__sg;
- return NULL;
-}
-
-static inline struct scatterlist *
-ata_qc_next_sg(struct scatterlist *sg, struct ata_queued_cmd *qc)
-{
- return sg_next(sg);
-}
-
#define ata_for_each_sg(sg, qc) \
- for (sg = ata_qc_first_sg(qc); sg; sg = ata_qc_next_sg(sg, qc))
+ for (sg = qc-__sg; sg; sg = sg_next(sg))
static inline unsigned int ata_tag_valid(unsigned int tag)
{
-
To unsubscribe from this list: send the line unsubscribe linux-ide 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: advansys broken (at least) on ARM and MIPS
On Thu, 2008-01-03 at 08:43 +0100, Martin Michlmayr wrote: Commit 9d511a4b29de6764931343d03e493f2e04df0271 removed the depends on BROKEN || X86_32 line from advansys' Kconfig entry. I'm not sure that was such a great idea - the module doesn't compile at least on ARM and MIPS: ARM: CC [M] drivers/scsi/advansys.o drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not properly converted to the DMA API drivers/scsi/advansys.c: In function ‘AdvBuildCarrierFreelist’: drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) ... drivers/scsi/advansys.c: In function ‘advansys_get_sense_buffer_dma’: drivers/scsi/advansys.c:9885: error: implicit declaration of function ‘dma_cache_sync’ That error doesn't look to tbe the fault of the driver. dma_cache_sync() is a piece of the DMA API it looks like ARM doesn't implement? James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Remove of old NCR53C9x/esp family of drivers
On Thu, 2008-01-03 at 17:40 +0200, Boaz Harrosh wrote: As recommended by Christoph Hellwig. There is no use of Fixing these drivers, since there is a much simpler and modern esp infrastructure with David Miller's esp_scsi - Remove all driver files dependent on NCR53C9x.c deleted:drivers/scsi/NCR53C9x.c deleted:drivers/scsi/NCR53C9x.h deleted:drivers/scsi/blz1230.c deleted:drivers/scsi/blz2060.c deleted:drivers/scsi/cyberstorm.c deleted:drivers/scsi/cyberstormII.c deleted:drivers/scsi/dec_esp.c deleted:drivers/scsi/fastlane.c deleted:drivers/scsi/mac_esp.c deleted:drivers/scsi/mca_53c9x.c deleted:drivers/scsi/oktagon_esp.c deleted:drivers/scsi/oktagon_io.S deleted:drivers/scsi/sun3x_esp.c - Remove above list from drivers/scsi/Kconfig drivers/scsi/Makefile OK, I'll split this into four pieces for scsi-pending, since there are three separate interest groups with signoffs to collect (MCA, m68k and alpha) plus the core removal. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Remove of old NCR53C9x/esp family of drivers
On Thu, 2008-01-03 at 20:05 +0100, Geert Uytterhoeven wrote: On Thu, 3 Jan 2008, James Bottomley wrote: On Thu, 2008-01-03 at 17:40 +0200, Boaz Harrosh wrote: As recommended by Christoph Hellwig. There is no use of Fixing these drivers, since there is a much simpler and modern esp infrastructure with David Miller's esp_scsi - Remove all driver files dependent on NCR53C9x.c deleted:drivers/scsi/NCR53C9x.c deleted:drivers/scsi/NCR53C9x.h deleted:drivers/scsi/blz1230.c deleted:drivers/scsi/blz2060.c deleted:drivers/scsi/cyberstorm.c deleted:drivers/scsi/cyberstormII.c deleted:drivers/scsi/dec_esp.c deleted:drivers/scsi/fastlane.c deleted:drivers/scsi/mac_esp.c deleted:drivers/scsi/mca_53c9x.c deleted:drivers/scsi/oktagon_esp.c deleted:drivers/scsi/oktagon_io.S deleted:drivers/scsi/sun3x_esp.c - Remove above list from drivers/scsi/Kconfig drivers/scsi/Makefile OK, I'll split this into four pieces for scsi-pending, since there are three separate interest groups with signoffs to collect (MCA, m68k and alpha) plus the core removal. Anybody who can look into converting the m68k NCR53C9x drivers and has hardware to test (some of) them? I don't think we can afford losing one third of our SCSI drivers... You can use the following as guidance: commit 5ff263667798946abc15314eae3f341345877d7a Author: Thomas Bogendoerfer [EMAIL PROTECTED] Date: Tue May 22 17:03:44 2007 -0700 [SCSI] jazz_esp: Converted to use esp_core. Use new esp_scsi for JAZZ SCSI host adapter driver I can also offer help to anyone who tries this. It's also a good opportunity to let die drivers that have no committed users. Just to be clear on why we're doing this: the NCR53C9x driver on which these are all based is in a pretty horrendous state of repair. The esp_scsi one is much nicer, actually nicely tested and has a host of features the old driver didn't. However, the principle driving force is the conversion of the SCSI subsystem to the sg_list accessors. esp_scsi is already coverted. NCR53C9x looks like a nasty job. Thus, the moment the conversion patch goes in, all your drivers will break. However, since breakage excites a whole bunch of kernel compile checkers (and lands me with a flood of email), I'm prepared to remove them to prevent this from happening ... unless we can get them converted over to esp_scsi. I'll put the removal in the scsi-pending tree, so it won't be picked up by -mm, but we need to get this situation resolved by 2.6.25 at the latest. 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
[GIT PATCH] another SCSI critical bug fix for 2.6.24-rc6
They just keep coming out of the woodwork, sorry. This one's in the
SCSI over RDMA Protocol transport class, so it's causing SCSI over
infiniband installations to oops on shutdown. I'm told that their are
now nearly as many SCSI over IB users as their are voyager users, so,
since it's nearly mainstream, we need to fix the problem urgently ...
The patch is available here:
master.kernel.org:/pub/scm/linux/kernel/git/jejb/scsi-rc-fixes-2.6.git
I've just attached it below as well.
James
---
From 911833440b498e3e4fe2f12c1ae2bd44400c7004 Mon Sep 17 00:00:00 2001
From: Dave Dillow [EMAIL PROTECTED]
Date: Thu, 3 Jan 2008 21:34:49 -0500
Subject: [SCSI] SRP transport: only remove our own entries
The SCSI SRP transport class currently iterates over all children
devices of the host that is being removed in srp_remove_host(). However,
not all of those children were created by the SRP transport, and
removing them will cause corruption and an oops when their creator tries
to remove them.
Signed-off-by: David Dillow [EMAIL PROTECTED]
Acked-by: FUJITA Tomonori [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/scsi_transport_srp.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/scsi_transport_srp.c
b/drivers/scsi/scsi_transport_srp.c
index 44a340b..65c584d 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -265,7 +265,8 @@ EXPORT_SYMBOL_GPL(srp_rport_del);
static int do_srp_rport_del(struct device *dev, void *data)
{
- srp_rport_del(dev_to_rport(dev));
+ if (scsi_is_srp_rport(dev))
+ srp_rport_del(dev_to_rport(dev));
return 0;
}
--
1.5.3.7
-
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] tgt: Use scsi_init_io instead of scsi_alloc_sgtable
On Thu, 2007-12-13 at 13:44 +0200, Boaz Harrosh wrote:
- If we export scsi_init_io()/scsi_release_buffers() instead of
scsi_{alloc,free}_sgtable() from scsi_lib than tgt code is
much more insulated from scsi_lib changes. As a bonus it will
also gain bidi capability when it comes.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
Acked-by: FUJITA Tomonori [EMAIL PROTECTED]
I'm afraid the reversion of -done removal broke the application of this
yet again ... could you respin.
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 0/7] sg_ring: a ring of scatterlist arrays
On Wed, 2007-12-19 at 17:31 +1100, Rusty Russell wrote: This patch series is the start of my attempt to simplify and make explicit the chained scatterlist logic. It's not complete: my SATA box boots and seems happy, but all the other users of SCSI need to be updated and checked. But I've gotten far enough to believe it's worth persuing. Sorry for the delay in looking at this, I was busy with Holidays and things. When I compare sg_ring with the current sg_chain (and later sg_table) implementations, I'm actually struck by how similar they are. The other thing I note is that the problem you're claiming to solve with sg_ring (the ability to add extra scatterlists to the front or the back of an existing one) is already solved with sg_chain, so the only real advantage of sg_ring was that it contains explicit counts, which sg_table (in -mm) also introduces. The other differences are that sg_ring only allows adding at the front or back of an existing sg_ring, it doesn't allow splicing at any point like sg_chain does, so I'd say it's less functional (not that I actually want anyone ever to do this, of course ...) The final point is that sg_ring requires a two level traversal: ring list then scatterlist, whereas sg_chain only requires a single level traversal. I grant that we can abstract out the traversal into something that would make users think they're only doing a single level, but I don't see what the extra level really buys us. The above analysis seems to suggest that sg_chain is simpler and has more functionality than sg_ring, unless I've missed anything? The only thing missing from sg_chain perhaps is an accessor function that does the splicing, which I can easily construct if you want to try it out in virtio. 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
[PATCH] add missing transport configure points for target and host
While trying to convert the SPI transport class to attribute groups, I discovered that we don't actually have any transport configure points for either the target or the host. This patch adds these missing transport class triggers. The host one is simply done after the add, the target one tries to be more clever and add it after devices may have been placed on the target (so the device configure will have set up the target parameters). James --- diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index ada72af..1dc165a 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -1489,6 +1489,7 @@ struct scsi_device *__scsi_add_device(struct Scsi_Host *shost, uint channel, if (scsi_host_scan_allowed(shost)) scsi_probe_and_add_lun(starget, lun, NULL, sdev, 1, hostdata); mutex_unlock(shost-scan_mutex); + transport_configure_device(starget-dev); scsi_target_reap(starget); put_device(starget-dev); @@ -1569,6 +1570,7 @@ static void __scsi_scan_target(struct device *parent, unsigned int channel, out_reap: /* now determine if the target has any children at all * and if not, nuke it */ + transport_configure_device(starget-dev); scsi_target_reap(starget); put_device(starget-dev); diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 00b3866..ed83cdb 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1018,6 +1018,7 @@ int scsi_sysfs_add_host(struct Scsi_Host *shost) } transport_register_device(shost-shost_gendev); + transport_configure_device(shost-shost_gendev); return 0; } - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi_transport_spi: convert to attribute groups
This conversion makes full use of the is_visible() callback on attribute
groups. Now, each device appears only with its capability flags in the
transport class directory. Previously each device appeared with the
capability of the host, so this is a functionality improvement.
Converting to attribute groups allows us to sweep away most of the home
grown #defines that were effectively doing the same thing.
James
---
This depends on:
[PATCH] sysfs: add filter function to groups
[PATCH] fix the sysfs_add_file_to_group interfaces
[PATCH] attribute_container: update to use the group interface
[PATCH] add missing transport configure points for target and host
Greg and Kay, there's a nasty point in the code where I'd like to use
the -EEXIST return of sysfs_add_file_to_group() to indicate the file is
already there, however, this also dumps a stack trace and would frighten
users ... can we get rid of the printk and the WARN_ON(1)?
diff --git a/drivers/scsi/scsi_transport_spi.c
b/drivers/scsi/scsi_transport_spi.c
index 4df21c9..1fb6031 100644
--- a/drivers/scsi/scsi_transport_spi.c
+++ b/drivers/scsi/scsi_transport_spi.c
@@ -52,13 +52,6 @@
struct spi_internal {
struct scsi_transport_template t;
struct spi_function_template *f;
- /* The actual attributes */
- struct class_device_attribute private_attrs[SPI_NUM_ATTRS];
- /* The array of null terminated pointers to attributes
-* needed by scsi_sysfs.c */
- struct class_device_attribute *attrs[SPI_NUM_ATTRS + SPI_OTHER_ATTRS +
1];
- struct class_device_attribute private_host_attrs[SPI_HOST_ATTRS];
- struct class_device_attribute *host_attrs[SPI_HOST_ATTRS + 1];
};
#define to_spi_internal(tmpl) container_of(tmpl, struct spi_internal, t)
@@ -174,17 +167,20 @@ static int spi_host_setup(struct transport_container *tc,
struct device *dev,
return 0;
}
+static int spi_host_configure(struct transport_container *tc,
+ struct device *dev,
+ struct class_device *cdev);
+
static DECLARE_TRANSPORT_CLASS(spi_host_class,
spi_host,
spi_host_setup,
NULL,
- NULL);
+ spi_host_configure);
static int spi_host_match(struct attribute_container *cont,
struct device *dev)
{
struct Scsi_Host *shost;
- struct spi_internal *i;
if (!scsi_is_host_device(dev))
return 0;
@@ -194,11 +190,13 @@ static int spi_host_match(struct attribute_container
*cont,
!= spi_host_class.class)
return 0;
- i = to_spi_internal(shost-transportt);
-
- return i-t.host_attrs.ac == cont;
+ return shost-transportt-host_attrs.ac == cont;
}
+static int spi_target_configure(struct transport_container *tc,
+ struct device *dev,
+ struct class_device *cdev);
+
static int spi_device_configure(struct transport_container *tc,
struct device *dev,
struct class_device *cdev)
@@ -300,8 +298,10 @@ store_spi_transport_##field(struct class_device *cdev,
const char *buf, \
struct Scsi_Host *shost = dev_to_shost(starget-dev.parent);\
struct spi_internal *i = to_spi_internal(shost-transportt);\
\
+ if (!i-f-set_##field) \
+ return -EINVAL; \
val = simple_strtoul(buf, NULL, 0); \
- i-f-set_##field(starget, val);\
+ i-f-set_##field(starget, val);\
return count; \
}
@@ -317,6 +317,8 @@ store_spi_transport_##field(struct class_device *cdev,
const char *buf, \
struct spi_transport_attrs *tp \
= (struct spi_transport_attrs *)starget-starget_data; \
\
+ if (i-f-set_##field) \
+ return -EINVAL; \
val = simple_strtoul(buf, NULL, 0); \
if (val tp-max_##field) \
val = tp-max_##field; \
@@ -327,14 +329,14 @@ store_spi_transport_##field(struct class_device *cdev,
const char *buf, \
#define spi_transport_rd_attr(field, format_string)\
spi_transport_show_function(field, format_string) \
spi_transport_store_function(field, format_string) \
-static
[PATCH] sr: update to follow tray status correctly
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).
It seems to work for me.
James
---
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 57b5263..8f31b41 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -67,8 +67,6 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM);
#define SR_DISKS 256
-#define MAX_RETRIES3
-#define SR_TIMEOUT (30 * HZ)
#define SR_CAPABILITIES \
(CDC_CLOSE_TRAY|CDC_OPEN_TRAY|CDC_LOCK|CDC_SELECT_SPEED| \
CDC_SELECT_DISC|CDC_MULTI_SESSION|CDC_MCN|CDC_MEDIA_CHANGED| \
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 0d04e28..81fbc0b 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -20,6 +20,9 @@
#include linux/genhd.h
#include linux/kref.h
+#define MAX_RETRIES3
+#define SR_TIMEOUT (30 * HZ)
+
struct scsi_device;
/* The CDROM is fairly slow, so we need a little extra time */
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index e1589f9..d5cebff 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -275,18 +275,6 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
/* -- */
/* interface to cdrom.c */
-static int test_unit_ready(Scsi_CD *cd)
-{
- struct packet_command cgc;
-
- memset(cgc, 0, sizeof(struct packet_command));
- cgc.cmd[0] = GPCMD_TEST_UNIT_READY;
- cgc.quiet = 1;
- cgc.data_direction = DMA_NONE;
- cgc.timeout = IOCTL_TIMEOUT;
- return sr_do_ioctl(cd, cgc);
-}
-
int sr_tray_move(struct cdrom_device_info *cdi, int pos)
{
Scsi_CD *cd = cdi-handle;
@@ -310,14 +298,46 @@ int sr_lock_door(struct cdrom_device_info *cdi, int lock)
int sr_drive_status(struct cdrom_device_info *cdi, int slot)
{
+ struct scsi_cd *cd = cdi-handle;
+ struct scsi_sense_hdr sshdr;
+ struct media_event_desc med;
+
if (CDSL_CURRENT != slot) {
/* we have no changer support */
return -EINVAL;
}
- if (0 == test_unit_ready(cdi-handle))
+ if (0 == scsi_test_unit_ready(cd-device, SR_TIMEOUT, MAX_RETRIES,
+ sshdr))
return CDS_DISC_OK;
- return CDS_TRAY_OPEN;
+ if (!cdrom_get_media_event(cdi, med)) {
+ if (med.media_present)
+ return CDS_DISC_OK;
+ else if (med.door_open)
+ return CDS_TRAY_OPEN;
+ else
+ return CDS_NO_DISC;
+ }
+
+ /*
+* 0x04 is format in progress .. but there must be a disc present!
+*/
+ if (sshdr.sense_key == NOT_READY sshdr.asc == 0x04)
+ return CDS_DISC_OK;
+
+ /*
+* If not using Mt Fuji extended media tray reports,
+* just return TRAY_OPEN since ATAPI doesn't provide
+* any other way to detect this...
+*/
+ if (scsi_sense_valid(sshdr)
+ /* 0x3a is medium not present */
+ sshdr.asc == 0x3a)
+ return CDS_NO_DISC;
+ else
+ return CDS_TRAY_OPEN;
+
+ return CDS_DRIVE_NOT_READY;
}
int sr_disk_status(struct cdrom_device_info *cdi)
-
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: An MCA ESP driver
On Wed, 2007-08-15 at 14:55 -0700, David Miller wrote:
From: Matthew Wilcox [EMAIL PROTECTED]
Date: Wed, 15 Aug 2007 11:26:00 -0600
On Tue, Aug 07, 2007 at 12:26:04AM -0700, David Miller wrote:
- struct sbus_dma *dma;
+ union {
+ struct sbus_dma *sbus_dma;
+ unsigned intx86_dma;
+ };
};
Feel free to make this a void *dma_cookie or similar.
It's private to the bus front-end.
Hi Dave,
Could I just clarify; would you prefer it to be a void *? I prefer the
anonymous union that I have there right now, but I'm not particularly
attached to it. In particular, I don't really care to be casting ints
(x86) to pointers, but that's a matter of personal taste.
Alternatively, you could remove this member entirely, and
make the front-end driver allocate a private area at the
end of struct esp to use for whatever purpose it likes.
I'm mostly ambivalent, but if the member stays it should be
some generic type rather than anything front-end specific
like it is now.
Just a check up on this: Matthew were you ever going to complete the
mca_94x conversion? It's quite topical because it would be another
example driver for the m68k people to look at.
If not, I can probably complete the bits you haven't yet done, but it
would be nice to know.
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] tgt: Use scsi_init_io instead of scsi_alloc_sgtable
On Sun, 2008-01-06 at 19:08 +0200, Boaz Harrosh wrote:
On Sat, Jan 05 2008 at 1:02 +0200, James Bottomley [EMAIL PROTECTED] wrote:
On Thu, 2007-12-13 at 13:44 +0200, Boaz Harrosh wrote:
- If we export scsi_init_io()/scsi_release_buffers() instead of
scsi_{alloc,free}_sgtable() from scsi_lib than tgt code is
much more insulated from scsi_lib changes. As a bonus it will
also gain bidi capability when it comes.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
Acked-by: FUJITA Tomonori [EMAIL PROTECTED]
I'm afraid the reversion of -done removal broke the application of this
yet again ... could you respin.
Thanks,
James
It looks like the revert is to be reverted ;)
I have rebased, but should I wait a while to
see if they're needed?
Yes, let's just wait a while to see what the outcome is ...
James
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][SCSI] megaraid: Convert from scsi.h to scsi.h (and friends)
On Sun, 2008-01-06 at 20:03 +0100, Richard Knutsson wrote: Convert glue-include scsi.h to scsi.h (and friends). (binary sizes) allyesconfig: before: 260132 after: 260048 allmodconfig: before: 261740 after: 261656 Signed-off-by: Richard Knutsson [EMAIL PROTECTED] --- Do not have the hardware, but since it compiles I hope it is alright. diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c index 66c6520..9f1e2c5 100644 --- a/drivers/scsi/megaraid.c +++ b/drivers/scsi/megaraid.c @@ -48,8 +48,9 @@ #include linux/dma-mapping.h #include scsi/scsicam.h -#include scsi.h +#include scsi/scsi_cmnd.h #include scsi/scsi_host.h +#include scsi.h I'm afraid this is pretty much wrong. The scsi.h being referred to here is the local scsi.h in the build directory, so it should be included as a string. For #include, ... means begin the search in the local directory (what is wanted here) and ... means begin the search starting with the predefined include paths. The rest of the patch looks like a spurious (unrelated and undescribed) downcasing of TRUE and FALSE. 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 0/7] sg_ring: a ring of scatterlist arrays
On Mon, 2008-01-07 at 15:38 +1100, Rusty Russell wrote: On Sunday 06 January 2008 02:31:12 James Bottomley wrote: On Wed, 2007-12-19 at 17:31 +1100, Rusty Russell wrote: This patch series is the start of my attempt to simplify and make explicit the chained scatterlist logic. It's not complete: my SATA box boots and seems happy, but all the other users of SCSI need to be updated and checked. But I've gotten far enough to believe it's worth persuing. Sorry for the delay in looking at this, I was busy with Holidays and things. Thankyou for your consideration. When I compare sg_ring with the current sg_chain (and later sg_table) implementations, I'm actually struck by how similar they are. I agree, they're solving the same problem. It is possible that the pain of change is no longer worthwhile, but I hate to see us give up on this. We're adding complexity without making it harder to misuse. We're always open to new APIs (or more powerful and expanded old ones). The way we've been doing the sg_chain conversion is to slide API layers into the drivers so sg_chain becomes a simple API flip when we turn it on. Unfortunately sg_ring doesn't quite fit nicely into this. The other thing I note is that the problem you're claiming to solve with sg_ring (the ability to add extra scatterlists to the front or the back of an existing one) is already solved with sg_chain, so the only real advantage of sg_ring was that it contains explicit counts, which sg_table (in -mm) also introduces. I just converted virtio using latest Linus for fair comparison Erm, but that's not really a fair comparison; you need the sg_table code in git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-2.6-block.git branch sg as well. , and it's still pretty ugly. sg_ring needs more work to de-scsi it. sg_table is almost sg_ring except it retains all the chaining warts. But we hit the same problems: 1) sg_chain loses information. The clever chain packaging makes reading easy, but manipulation is severely limited. You can append to your own chains by padding, but not someone elses. This works for SCSI, but what about the rest of us? And don't even think of joining mapped chains: it will almost work. OK, but realistically some of your criticisms are way outside of the design intent. Scatterlists (and now sg_chains) are the way the block subsystem hands pages to its underlying block devices. We do this through two APIs: blk_rq_map_sg() to take a request and place all the pages into a scatterlist respecting the merging parameters and dma_map_sg() which takes the scatterlist and produces an arch specific DMA mapped scatterlist reusing the old list. The requirement is that dma_unmap_sg return the chain to its former state (so block device may map, and unmap between congestion waits to avoid running systems out of scarce IOMMU mappings), but there's not even a parallel blk_rq_unmap_sg() beacuse the scatterlist is assumed disposable by the block driver after the request completes. By someone elses I'm assuming you mean a chain in a stacked block driver? You are actually allowed to transform these ... depending on who does the dma mapping, of course, if the lower driver (yours) does DMA mapping, you can do anything you want. If the upper driver does, then you have to preserve idempotence. There have never until now been any requirements to join already dma_map_sg() converted scatterlists ... that would wreak havoc with the way we reuse the list plus damage the idempotence of map/unmap. What is the use case you have for this? 2) sg_chain's end marker is only for reading non-dma elements, not for mapped chains, nor for writing into chains. Plus it's a duplicate of the num arg, which is still passed around for efficiency. (virtio had to implement count_sg() because I didn't want those two redundant num args). 3) By overloading struct scatterlist, it's invisible what code has been converted to chains, and which hasn't. Too clever by half! No it's not ... that's the whole point. Code not yet converted to use the API accessors is by definition unable to use chaining. Code converted to use the accessors by design doesn't care (and so is converted to chains). Look at sg_chain(): it claims to join two scatterlists, but it doesn't. It assumes that prv is an array, not a chain. Because you've overloaded an existing type, this happens everywhere. Try passing skb_to_sgvec a chained skb. sg_ring would not have required any change to existing drivers, just those that want to use long sg lists. And then it's damn obvious which are which. Which, by definition, would have been pretty much all of them. Another driving factor is that the mempool allocation of scatterlists is suboptimal in terms of their bucket size, so we were looking to tune that once the chaining code was in. The guess is that we'd probalby end up with a uniform size
Re: Multipath failover handling (Was: Re: 2.6.24-rc3-mm1)
On Mon, 2008-01-07 at 15:05 +0100, Hannes Reinecke wrote: James Bottomley wrote: On Fri, 2007-12-14 at 10:00 +0100, Hannes Reinecke wrote: James Bottomley wrote: On Mon, 2007-11-26 at 22:15 -0800, Andrew Morton wrote: OK, thanks. I'll assume that James and Hannes have this in hand (or will have, by mid-week) and I won't do anything here. Just to confirm what I think I'm going to be doing: rebasing the scsi-misc tree to remove this commit: commit 8655a546c83fc43f0a73416bbd126d02de7ad6c0 Author: Hannes Reinecke [EMAIL PROTECTED] Date: Tue Nov 6 09:23:40 2007 +0100 [SCSI] Do not requeue requests if REQ_FAILFAST is set And its allied fix ups: commit 983289045faa96fba8841d3c51b98bb8623d9504 Author: James Bottomley [EMAIL PROTECTED] Date: Sat Nov 24 19:47:25 2007 +0200 [SCSI] fix up REQ_FASTFAIL not to fail when state is QUIESCE commit 9dd15a13b332e9f5c8ee752b1ccd9b84cb5bdf17 Author: James Bottomley [EMAIL PROTECTED] Date: Sat Nov 24 19:55:53 2007 +0200 [SCSI] fix domain validation to work again James Or just apply my latest patch (cf Undo __scsi_kill_request). The main point is that we shouldn't retry requests with FAILFAST set when the queue is blocked. AFAICS only FC and iSCSI transports set the queue to blocked, and use this to indicate a loss of connection. So any retry with queue blocked is futile. I still don't think this is the right approach. For link up/down events, those are direct pathing events and should be signalled along a kernel notifier, not by mucking with the SCSI state machine. Of course they will be signalled. And eventually we should patch up mutltipath-tools to read the exising events from the uevent socket. But even with that patch there is a quite largish window during which IOs will be sent to the blocked device, and hence will be stuck in the request queue until the timer expires. But the assumption your code makes is that if REQ_FAILFAST is set then it's a dm request ... and that's not true. The code in question negatively impacts other users of REQ_FAILFAST. For every user other than dm, the right thing to do is to wait out the block. However, there's still devloss_tmo to consider ... even in multipath, I don't think you want to signal path failure until devloss_tmo has fired otherwise you'll get too many transient up/down events which damage performance if the array has an expensive failover model. Yes. But currently we have a very high failover latency as we always have to wait for the requeued commands to time-out. Hence we're damaging performance on arrays with inexpensive failover. If it's a either/or choice between the two that's showing our current approach to multi-path is broken. The other problem is what to do with in-flight commands at the time the link went down. With your current patch, they're still stuck until they time out ... surely there needs to be some type of recovery mechanism for these? Well, the in-flight commands are owned by the HBA driver, which should have the proper code to terminate / return those commands with the appriopriate codes. They will then be rescheduled and will be caught like 'normal' IO requests. But my point is that if a driver goes blocked, those commands will be forced to wait the blocked timeout anyway, so your proposed patch does nothing to improve the case for dm anyway ... you only avoid commands stuck when a device goes blocked if by chance its request queue was empty. 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] sym8xx_2: Fixes call to wait_for_completion_timeout() with NULL argument
On Mon, 2008-01-07 at 22:56 +0100, Krzysztof Helt wrote: From: Krzysztof Helt [EMAIL PROTECTED] This patch fixes call to wait_for_completion_timeout() with NULL argument. That doesn't seem to be at all what your patch is doing. I can't see any case in the old code where wait_for_completion_timeout() could be called with a NULL that you fix. What it seems you are doing is altering the code to eliminate the finished_reset variable. 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] sym8xx_2: kill compilation warning
On Tue, 2008-01-08 at 07:40 +0100, Krzysztof Helt wrote: On Mon, 07 Jan 2008 16:39:35 -0600 James Bottomley [EMAIL PROTECTED] wrote: On Mon, 2008-01-07 at 22:56 +0100, Krzysztof Helt wrote: From: Krzysztof Helt [EMAIL PROTECTED] This patch fixes call to wait_for_completion_timeout() with NULL argument. That doesn't seem to be at all what your patch is doing. I can't see any case in the old code where wait_for_completion_timeout() could be called with a NULL that you fix. What it seems you are doing is altering the code to eliminate the finished_reset variable. I am sorry for the mess. I put the wrong description of the patch. Here is the correct one: --- The purpose of the patch is to kill the compilation warning with gcc-4.1.1 on sparc64: sym_glue.c: In function sym_eh_handler: sym_glue.c:612: warning: io_reset may be used uninitialized in this function This patch also eliminates the finished_reset variable. OK, ordinarily we don't change kernel code for what is clearly a compiler bug (as in other versions of gcc 4.1 don't produce this warning because they see the tie between the two variables). However, now that I actually look at this code, it has two clear bugs in it: 1. the if (!sym_data-io_reset). That variable is only ever filled by a stack based completion. If we find it non empty it means this code has been entered twice and we have a severe problem, so that should just become a BUG_ON(!sym_data-io_reset) 2. sym_data-io_reset should be set to NULL before the routine is exited otherwise the PCI recovery code could end up completing what will be a bogus pointer into the stack. The vaule of sym_data-io_reset looks like it should remain current across the lock, so there's no need to save it in a variable (the sym2_io_resume() routine also shouldn't be setting it to NULL, and the complete_all should be complete---just because we can only have a single thread waiting for the completion otherwise all hell would break loose). So, fix these issues (and quietly change the code not to produce the compile warning on sparc64) and it can go in. 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: Buggy Firewire bridge 'Prolific PL3507'
On Tue, 2008-01-08 at 18:56 +0100, Stefan Richter wrote: Matthew Wilcox wrote: On Tue, Jan 08, 2008 at 02:43:34PM +0100, Hannes Reinecke wrote: I have here a buggy firewire bridge (Prolific PL3507) which requires that each 'INQUIRY' command is followed by a 'READ CAPACITY' command. Otherwise any read will return invalid data (the payload is preceded by 36 empty bytes). How to fix this? Sure one could add a hack to sbp2.c to always issue a READ CAPACITY after INQUIRY, but this somehow feels wrong ... There's only one place in the scsi stack that issues INQUIRY -- scsi_scan.c [1]. sd is going to issue READ CAPACITY before it issues any READ commands, so I don't see where you're having this problem. Is it with some program issuing INQUIRY through SG_IO or something? It's hald or something like that. Not to hijack the thread, but this is getting to be a broken record. I was dealing only a few days ago with a camera presenting as a mass storage device that was then crashing and going offline. I looked at the dmesg trace which showed that the SCSI layer completes its probing successfully, and said it was some other extraneous command issued from user level causing the crash. Sure enough, stopping hald fixed the camera so it functioned as a mass storage device properly. The bug report is here: https://bugs.launchpad.net/ubuntu/+source/hal/+bug/180472 What is the point of having SCSI be so careful in its probing and setup so as not to annoy these devices, and then have hald or another standard component blithely go and wreck the device by issuing unwarranted SCSI commands? Can we please stop hald from issuing SCSI commands ... we should have the infrastructure in place now that renders this unnecessary ... unless there's still some information it needs that we're not providing? 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] qla2xxx: Fix endianness of task management response code
On Tue, 2012-09-18 at 15:10 -0700, Roland Dreier wrote: From: Roland Dreier rol...@purestorage.com The qla2xxx firmware actually expects the task management response code in a CTIO IOCB with SCSI status mode 1 to be in little-endian byte order, ie the response code should be the first byte in the sense_data[] array. Is this also true on Big Endian Hardware? Because the fix you have assumes that the TIO IOCB with SCSI status mode 1 should be CPU endian ... that doesn't look right since this is passed directly over the PCI bus (and the PCI bus is little endian), so shouldn't the correct fix be to replace cpu_to_be32 with cpu_to_le32? James The old code erroneously byte-swapped the response code, which puts it in the wrong place on the wire and leads to initiators thinking every task management request succeeds (since they see 0 in the byte where they look for the response code). Cc: Chad Dupuis chad.dup...@qlogic.com Cc: Arun Easi arun.e...@qlogic.com Signed-off-by: Roland Dreier rol...@purestorage.com --- drivers/scsi/qla2xxx/qla_target.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 5b30132..41b74ba 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -1403,7 +1403,7 @@ static void qlt_24xx_send_task_mgmt_ctio(struct scsi_qla_host *ha, ctio-u.status1.scsi_status = __constant_cpu_to_le16(SS_RESPONSE_INFO_LEN_VALID); ctio-u.status1.response_len = __constant_cpu_to_le16(8); - ((uint32_t *)ctio-u.status1.sense_data)[0] = cpu_to_be32(resp_code); + ctio-u.status1.sense_data[0] = resp_code; qla2x00_start_iocbs(ha, ha-req); } -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/6] ZPODD patches
On Wed, 2012-09-19 at 16:03 +0800, Aaron Lu wrote: Hi James, May I know if this patchset will enter v3.7? Sigh, well, I was hoping to persuade the PM people to sort this out first. The first observation is that all this looks to be too specific. ZPO may be ACPI specific, but the property it abstracts: whether the particular device is powered off or not is generic and probably should be known at the generic PM level. Nothing actually really cares about how we power off the device until you get all the way down to the ACPI controller. I think we could do this with a couple of flags sitting inside struct device itself: one for pm state and capabilities defined at a generic level and one for device specific pm state. The latter would be for things like the door lock information which is very specific to CDs (although not specific to SCSI CDs). Alternatively, even if we can't use these capabilities at the generic pm level, we still need an internal state set of flags because power state stuff traverses the stack and struct device is the only universal object in that stack. So I definitely think all of the sdev flags should become either generic or specific flags in device. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/6] ZPODD patches
On Wed, 2012-09-19 at 14:50 +0200, Rafael J. Wysocki wrote: On Wednesday, September 19, 2012, James Bottomley wrote: On Wed, 2012-09-19 at 16:03 +0800, Aaron Lu wrote: Hi James, May I know if this patchset will enter v3.7? Sigh, well, I was hoping to persuade the PM people to sort this out first. The first observation is that all this looks to be too specific. ZPO may be ACPI specific, but the property it abstracts: whether the particular device is powered off or not is generic and probably should be known at the generic PM level. Nothing actually really cares about how we power off the device until you get all the way down to the ACPI controller. I think we could do this with a couple of flags sitting inside struct device itself: one for pm state and capabilities defined at a generic level and one for device specific pm state. The latter would be for things like the door lock information which is very specific to CDs (although not specific to SCSI CDs). Alternatively, even if we can't use these capabilities at the generic pm level, we still need an internal state set of flags because power state stuff traverses the stack and struct device is the only universal object in that stack. So I definitely think all of the sdev flags should become either generic or specific flags in device. Well, the problem is that it is kind of irrelevant to the core whether or not the given device can be powered off. Moreover, the actual meaning of what power off means depends on the platform (it may be an individual device state or a power domain state, for instance). Also, the set of available low-power states depends on the platform (or the bus type) and generally cannot be universally represented and there are low-power states that aren't power off per se, but still require the device state to be restored when putting it back into full power. So I don't insist on it being generic, but we do need somewhere to hang the state. We've discussed that for a few times and each time we've ended up agreeing that struct device is not the right place to store this information (for example, PCI stores it in struct pci_dev, USB has its own rules etc.). So, here's the problem this causes. In SCSI, lower level devices have no access to the drivers (to which the upper layer structures are tied), so we have no way to go from device/scsi device to the scsi_disk structure say. This means that a lot of device specific PM stuff tends to have flags in scsi_device just so we can get access to it. A flag in device would allow us to carry the information farther (say to struct cdrom for instance). I'll have a look at the patchset again and see what can be done about this. Thanks, James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qla2xxx: Correct loop_id_map allocation-size and usage.
On Thu, 2012-09-20 at 03:39 -0400, Saurav Kashyap wrote: From: Andrew Vasquez andrew.vasq...@qlogic.com Original code incorrectly assigned LOOPID_MAP_SIZE to be the allocation size in bytes rather than total bit size. Additionally corrected code to check for bit-allocation failure in qla2x00_find_new_loop_id(). Signed-off-by: Andrew Vasquez andrew.vasq...@qlogic.com Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com --- drivers/scsi/qla2xxx/qla_def.h |2 +- drivers/scsi/qla2xxx/qla_init.c |4 ++-- drivers/scsi/qla2xxx/qla_os.c |3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) This is correcting a bug introduced by commit 07f2a8c179aa81075a161e6d0c522aa1289d994f Author: Chad Dupuis chad.dup...@qlogic.com Date: Wed Aug 22 14:21:00 2012 -0400 [SCSI] qla2xxx: Use bitmap to store loop_id's for fcports. Isn't it? So rather than introducing a bug and later fixing it, we should just merge the two, since it's not yet upstream, right? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Discard merging / Write same support v5
On Thu, 2012-09-20 at 14:33 +0200, Jens Axboe wrote: On 09/18/2012 06:19 PM, Martin K. Petersen wrote: I've been beating on this patch kit for the last few weeks with a bunch of different devices. I have also tested extensively with Shaohua Li's MD RAID0 discard support. The only functional change is support for discard and write same bio/request merging. Other than that there were a few tweaks for later kernels by both Mike and me. I put a writesame5 branch on: git://git.kernel.org/pub/scm/linux/kernel/git/mkp/linux.git It's against block/for-3.7/core. [PATCH 1/8] block: Clean up special command handling logic [PATCH 2/8] block: Consolidate command flag and queue limit checks [PATCH 3/8] block: Implement support for WRITE SAME [PATCH 4/8] block: Make blkdev_issue_zeroout use WRITE SAME [PATCH 5/8] block: ioctl to zero block ranges [PATCH 6/8] scsi: Add a report opcode helper [PATCH 7/8] sd: Permit merged discard requests [PATCH 8/8] sd: Implement support for WRITE SAME Thanks Martin, I queued up 1..5 in for-3.7/core. I'm assuming James will pick up the SCSI bits? Yes, but Linus gave me a rocket about post merge trees. Can you do this on a separate branch and I'll build on top of that? Thanks, James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qla2xxx: Fix endianness of task management response code
On Wed, 2012-09-19 at 07:09 -0700, Roland Dreier wrote: On Wed, Sep 19, 2012 at 12:59 AM, James Bottomley james.bottom...@hansenpartnership.com wrote: Is this also true on Big Endian Hardware? Because the fix you have assumes that the TIO IOCB with SCSI status mode 1 should be CPU endian ... that doesn't look right since this is passed directly over the PCI bus (and the PCI bus is little endian), so shouldn't the correct fix be to replace cpu_to_be32 with cpu_to_le32? After my patch the assignment is to a u8, and that byte is in the right place in memory. So I don't think there's any endianness bug. The data in status1 appears to get used a word at a time ... what about the other three bytes you don't set; are they guaranteed to be zero? (in which case this works, it just looks wrong from the way the thing is used in the rest of the code). James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] scsi_lib: rate-limit the error message from failing commands
On Fri, 2012-09-21 at 15:15 -0400, Mike Snitzer wrote: On Thu, Aug 30, 2012 at 7:25 PM, Jens Axboe ax...@kernel.dk wrote: On 2012-08-30 14:06, Yi Zou wrote: [ Jens/James, This is a rather old rate limt patch but never gets picked up in upstream, so I am resending it here as v3, with some minor changes so it is directly applicable to the current linux-2.6.git tree. I don't have that many LUNs that the original issue was reported but the patch was already reviewed and acked in the past, I did retest w/ stress I/O on 4 LUNs I have right now. Original thread can be found at: http://comments.gmane.org/gmane.linux.scsi/73497 http://www.open-fcoe.org/patchwork/patch/2436/ Can you please pull this patch in? it's been hanging there for a long time. Original patch description is below. I've pulled in the block bit. James, Do you intend to pick up the scsi_lib.c change? Possibly, if someone resends. The original v3 patch isn't actually on the list that I can find. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] SCSI fixes for 3.6-rc6
This is a set of four essential fixes: two oops related (bnx2i,
virtio-scsi), one data corruption related (hpsa) and one failure to boot
due to interrupt routing issues (mpt2ss).
The patch is available here:
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes
The short changelog is:
Eddie Wai (1):
bnx2i: Fixed NULL ptr deference for 1G bnx2 Linux iSCSI offload
Stephen M. Cameron (1):
hpsa: fix handling of protocol error
Wang Sen (1):
scsi: virtio-scsi: Fix address translation failure of HighMem pages used
by sg list
sreekanth.re...@lsi.com (1):
mpt2sas: Fix for issue - Unable to boot from the drive connected to HBA
And the diffstat:
drivers/scsi/bnx2i/bnx2i_hwi.c | 3 +++
drivers/scsi/hpsa.c | 3 ++-
drivers/scsi/mpt2sas/mpt2sas_base.c | 7 +++
drivers/scsi/virtio_scsi.c | 2 +-
4 files changed, 13 insertions(+), 2 deletions(-)
Full diff is appended below.
James
---
diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c b/drivers/scsi/bnx2i/bnx2i_hwi.c
index 33d6630..91eec60 100644
--- a/drivers/scsi/bnx2i/bnx2i_hwi.c
+++ b/drivers/scsi/bnx2i/bnx2i_hwi.c
@@ -1264,6 +1264,9 @@ int bnx2i_send_fw_iscsi_init_msg(struct bnx2i_hba *hba)
int rc = 0;
u64 mask64;
+ memset(iscsi_init, 0x00, sizeof(struct iscsi_kwqe_init1));
+ memset(iscsi_init2, 0x00, sizeof(struct iscsi_kwqe_init2));
+
bnx2i_adjust_qp_size(hba);
iscsi_init.flags =
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 796482b..2b4261c 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1315,8 +1315,9 @@ static void complete_scsi_command(struct CommandList *cp)
}
break;
case CMD_PROTOCOL_ERR:
+ cmd-result = DID_ERROR 16;
dev_warn(h-pdev-dev, cp %p has
- protocol error \n, cp);
+ protocol error\n, cp);
break;
case CMD_HARDWARE_ERR:
cmd-result = DID_ERROR 16;
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index b25757d..9d5a56c 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1209,6 +1209,13 @@ _base_check_enable_msix(struct MPT2SAS_ADAPTER *ioc)
u16 message_control;
+ /* Check whether controller SAS2008 B0 controller,
+ if it is SAS2008 B0 controller use IO-APIC instead of MSIX */
+ if (ioc-pdev-device == MPI2_MFGPAGE_DEVID_SAS2008
+ ioc-pdev-revision == 0x01) {
+ return -EINVAL;
+ }
+
base = pci_find_capability(ioc-pdev, PCI_CAP_ID_MSIX);
if (!base) {
dfailprintk(ioc, printk(MPT2SAS_INFO_FMT msix not
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index c7030fb..3e79a2f 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -331,7 +331,7 @@ static void virtscsi_map_sgl(struct scatterlist *sg,
unsigned int *p_idx,
int i;
for_each_sg(table-sgl, sg_elem, table-nents, i)
- sg_set_buf(sg[idx++], sg_virt(sg_elem), sg_elem-length);
+ sg[idx++] = *sg_elem;
*p_idx = idx;
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 47781] ata2: exception Emask 0x10 SAct 0x0 SErr 0x4050002 action 0xe frozen
On Sun, 2012-09-23 at 21:07 +, bugzilla-dae...@bugzilla.kernel.org wrote: https://bugzilla.kernel.org/show_bug.cgi?id=47781 Alan a...@lxorguk.ukuu.org.uk changed: What|Removed |Added Component|IDE |SCSI AssignedTo|io_...@kernel-bugs.osdl.org |linux-scsi@vger.kernel.org I'm a bit mystified by this reassignment. This looks like a pure ata problem to me; what makes you think it's SCSI? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH scsi] Add NULL checking of return value from scsi_cmd_to_driver()
On Mon, 2012-09-24 at 13:30 +0800, Li Zhong wrote: Just noticed that after commit 919f797, it is possible that scsi_cmd_to_driver() returns NULL. This patch adds the NULL checking for drv returned from the above function. Maybe it is not possible at run time, but from the code itself, we'd better have this check? There's not much point having a check that never trips, unless it's an assert, in which case a NULL deref does that. All it does is add pointless instructions to the critical path. only REQ_TYPE_BLOCK_PC commands can be submitted without a driver, so the check above would seem to preclude that. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH scsi] Add NULL checking of return value from scsi_cmd_to_driver()
On Mon, 2012-09-24 at 15:03 +0800, Li Zhong wrote: On Mon, 2012-09-24 at 09:44 +0400, James Bottomley wrote: On Mon, 2012-09-24 at 13:30 +0800, Li Zhong wrote: Just noticed that after commit 919f797, it is possible that scsi_cmd_to_driver() returns NULL. This patch adds the NULL checking for drv returned from the above function. Maybe it is not possible at run time, but from the code itself, we'd better have this check? There's not much point having a check that never trips, unless it's an assert, in which case a NULL deref does that. All it does is add pointless instructions to the critical path. only REQ_TYPE_BLOCK_PC commands can be submitted without a driver, so the check above would seem to preclude that. Hi James, Thank you, it sounds reasonable to me. Let's drop it. Well, there is another thing you might do: The path length of scsi_cmd_to_driver() increased a lot thanks to 18a4d0a22ed6 it might be worth getting it back to what it was (this looks to be doable with the same != REQ_TYPE_BLOCK_PC test in the error handler. Plus, I think it fixes a bug where you get different behaviours from REQ_TYPE_BLOCK_PC commands when a driver is and isn't attached (I've cc'd Martin to see what he thinks). James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] st: remove st_mutex
On Mon, 2012-09-10 at 15:36 -0700, Lee Duncan wrote: From: Hannes Reinecke h...@suse.com The st_mutex was created when the BKL was removed, and prevents simultaneous st_open calls. It is better to protect just the necessary data. Signed-off-by: Hannes Reinecke h...@suse.com Reviewed-by: Lee Duncan ldun...@suse.com That should be Signed-off-by since you sent the patch to me. Signoffs have to follow the chain of transmission. I've assumed this was just a mistype. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH scsi] Add NULL checking of return value from scsi_cmd_to_driver()
On Mon, 2012-09-24 at 17:25 +0800, Li Zhong wrote:
On Mon, 2012-09-24 at 11:35 +0400, James Bottomley wrote:
On Mon, 2012-09-24 at 15:03 +0800, Li Zhong wrote:
On Mon, 2012-09-24 at 09:44 +0400, James Bottomley wrote:
On Mon, 2012-09-24 at 13:30 +0800, Li Zhong wrote:
Just noticed that after commit 919f797, it is possible that
scsi_cmd_to_driver() returns NULL. This patch adds the NULL checking
for drv
returned from the above function.
Maybe it is not possible at run time, but from the code itself, we'd
better
have this check?
There's not much point having a check that never trips, unless it's an
assert, in which case a NULL deref does that. All it does is add
pointless instructions to the critical path. only REQ_TYPE_BLOCK_PC
commands can be submitted without a driver, so the check above would
seem to preclude that.
Hi James,
Thank you, it sounds reasonable to me. Let's drop it.
Well, there is another thing you might do: The path length of
scsi_cmd_to_driver() increased a lot thanks to 18a4d0a22ed6 it might be
worth getting it back to what it was (this looks to be doable with the
same != REQ_TYPE_BLOCK_PC test in the error handler. Plus, I think it
fixes a bug where you get different behaviours from REQ_TYPE_BLOCK_PC
commands when a driver is and isn't attached (I've cc'd Martin to see
what he thinks).
Hi James,
Do you mean something like this:
Pretty much, yes.
James
===
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index de2337f..c1b05a8 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -789,7 +789,6 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd,
unsigned char *cmnd,
int cmnd_size, int timeout, unsigned sense_bytes)
{
struct scsi_device *sdev = scmd-device;
- struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd);
struct Scsi_Host *shost = sdev-host;
DECLARE_COMPLETION_ONSTACK(done);
unsigned long timeleft;
@@ -845,8 +844,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd,
unsigned char *cmnd,
scsi_eh_restore_cmnd(scmd, ses);
- if (sdrv sdrv-eh_action)
- rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
+ if (scmd-request-cmd_type != REQ_TYPE_BLOCK_PC) {
+ struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd);
+ if (sdrv-eh_action)
+ rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
+ }
return rtn;
}
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index ac06cc5..377df4a 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -134,16 +134,7 @@ struct scsi_cmnd {
static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
{
- struct scsi_driver **sdp;
-
- if (!cmd-request-rq_disk)
- return NULL;
-
- sdp = (struct scsi_driver **)cmd-request-rq_disk-private_data;
- if (!sdp)
- return NULL;
-
- return *sdp;
+ return *(struct scsi_driver **)cmd-request-rq_disk-private_data;
}
extern struct scsi_cmnd *scsi_get_command(struct scsi_device *, gfp_t);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI PATCH] sd: max-retries becomes configurable
On Tue, 2012-09-25 at 01:21 -0400, Jeff Garzik wrote: On 09/25/2012 12:06 AM, James Bottomley wrote: On Mon, 2012-09-24 at 17:00 -0400, Jeff Garzik wrote: drivers/scsi/sd.c |4 drivers/scsi/sd.h |2 +- 2 files changed, 5 insertions(+), 1 deletion(-) I'm not opposed in principle to doing this (except that it should be a sysfs parameter like all our other controls), but what's the reasoning behind needing it changed? vendor hat on Periodically turns up as a useful field sledgehammer for solving problems, until the real problem is found and fixed. Got tired of a very similar patch manually bouncing around the hey, pssst, this worked for me backchannel IT network. /red hat I'm asking because the general consensus from the device guys is that we should never retry unless the device or the transport tells us to (and then we shouldn't count the retries). A long time ago we used to get spurious command failures from retry exhaustion on QUEUE_FULL or BUSY, but since we switched those to being purely timeout based, I thought the problem had gone away and I'm curious to know what guise it resurfaced in. Can you be more specific about sysfs location? A runtime-writable (via sysfs!) module parameter for a module-wide default seemed appropriate. Well, if it's really important, the same thing should happen with retries as happened with timeout (it became a request_queue property), but it could be hacked as a struct scsi_disk one with a corresponding entry in sd_dis_attrs. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/6] ZPODD patches
On Tue, 2012-09-25 at 16:18 +0800, Aaron Lu wrote:
A example patch would be something like the following, I didn't seperate
these ACPI calls in sr.c as this is just a concept proof, if this is the
right thing to do, I will separate them into another file sr-acpi.c and
make empty stubs for them in sr.h for systems do not have ACPI configured.
Apart from the needed separation to compile in the !ACPI case
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index ef72682..94d17f1 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -46,6 +46,7 @@
#include linux/mutex.h
#include linux/slab.h
#include linux/pm_runtime.h
+#include linux/acpi.h
#include asm/uaccess.h
#include scsi/scsi.h
@@ -57,6 +58,8 @@
#include scsi/scsi_host.h
#include scsi/scsi_ioctl.h /* For the door lock/unlock commands */
+#include acpi/acpi_bus.h
+
#include scsi_logging.h
#include sr.h
@@ -212,8 +220,8 @@ static int sr_resume(struct device *dev)
scsi_test_unit_ready(cd-device, SR_TIMEOUT, MAX_RETRIES, sshdr);
/* If user wakes up the ODD, eject the tray */
- if (cd-device-need_eject) {
- cd-device-need_eject = 0;
+ if (cd-need_eject) {
+ cd-need_eject = false;
/* But only for tray type ODD when door is not locked */
if (!(cd-cdi.mask CDC_CLOSE_TRAY) !cd-door_locked)
sr_tray_move(cd-cdi, 1);
@@ -704,6 +711,58 @@ static void sr_release(struct cdrom_device_info *cdi)
}
+static void sr_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
+{
+ struct device *dev = context;
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+
+ if (event == ACPI_NOTIFY_DEVICE_WAKE pm_runtime_suspended(dev)) {
+ cd-need_eject = true;
+ pm_runtime_resume(dev);
+ }
+}
+
+static void sr_acpi_add_pm_notifier(struct device *dev)
+{
+ struct acpi_device *acpi_dev;
+ acpi_handle handle;
+ acpi_status status;
+
+ handle = dev-archdata.acpi_handle;
This is a complete no-no. archdata is defined to be specific to the
architecture it's supposed to be opaque to non-arch code. You'll find
that only x86 and ia64 defines an acpi_handle there. This will
instantly fail to compile on non intel. If you need the handle, it
should be obtained via some accessor like dev_to_acpi_handle() which
will allow this to continue to function when, say, arm acquires ACPI.
I've got to say, this looks like a fault in ACPI itself. If the handles
are 1:1 with struct device, then why not have all the functions taking
handles take the device instead and leave the embedded handle safely in
the architecture specific code?
James
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH scsi] Add NULL checking of return value from scsi_cmd_to_driver()
On Wed, 2012-09-26 at 22:02 -0400, Martin K. Petersen wrote: James == James Bottomley james.bottom...@hansenpartnership.com writes: James Plus, I think it fixes a bug where you get different behaviours James from REQ_TYPE_BLOCK_PC commands when a driver is and isn't James attached (I've cc'd Martin to see what he thinks). I'm fine with having the eh action be triggered for FS requests only, if that's what you're asking? Sort of ... I was thinking do it for all non REQ_TYPE_BLOCK_PC commands (which includes flush and other strange types), but if you think it should only be for REQ_TYPE_FS, that's fine too. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI PATCH] sd: max-retries becomes configurable
On Wed, 2012-09-26 at 22:20 -0400, Martin K. Petersen wrote: James == James Bottomley james.bottom...@hansenpartnership.com writes: James On Mon, 2012-09-24 at 17:00 -0400, Jeff Garzik wrote: drivers/scsi/sd.c | 4 drivers/scsi/sd.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) James I'm not opposed in principle to doing this (except that it should James be a sysfs parameter like all our other controls), Now that we're in that department. I never got any feedback on the following patch. Hannes told me in person that he felt the eh_timeout belonged in scsi_device and not in the request queue. Whereas I favored making it a block layer tunable despite currently only being used by SCSI. Any opinions? request_queue makes more sense to me because there was once a plan to move all our timeout processing to block. I think it got stalled somewhere, but this would act as a reminder. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH scsi] Short the path length of scsi_cmd_to_driver()
On Thu, 2012-09-27 at 13:43 -0400, Martin K. Petersen wrote:
Li == Li Zhong zh...@linux.vnet.ibm.com writes:
@@ -845,8 +844,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd,
unsigned char *cmnd,
scsi_eh_restore_cmnd(scmd, ses);
-if (sdrv sdrv-eh_action)
-rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
+if (scmd-request-cmd_type == REQ_TYPE_FS) {
+struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd);
+if (sdrv-eh_action)
+rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
+}
return rtn;
}
My only concern is whether our device lifetime rules guarantee that the
ULD is always attached when we service an error handling command?
Yes, they are. We can only get REQ_TYPE_FS through a filesystem, which
must be mounted on a block device, which is provided by the ULD. You
can't unmount with outstanding I/O (which people complain about when it
goes into error handling, I'll admit), so the ULD has to stay bound.
James
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH scsi] Add NULL checking of return value from scsi_cmd_to_driver()
On Thu, 2012-09-27 at 13:41 -0400, Martin K. Petersen wrote: James == James Bottomley james.bottom...@hansenpartnership.com writes: I'm fine with having the eh action be triggered for FS requests only, if that's what you're asking? James Sort of ... I was thinking do it for all non REQ_TYPE_BLOCK_PC James commands (which includes flush and other strange types), but if James you think it should only be for REQ_TYPE_FS, that's fine too. I'm ok either way. OK, let's mirror the code in scsi.c then, so != REQ_TYPE_BLOCK_PC. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] virtio-scsi fixes for 3.6
On Mon, 2012-10-01 at 15:11 +0200, Paolo Bonzini wrote: Il 26/07/2012 15:28, Paolo Bonzini ha scritto: James, patch 1 fixes scanning of LUNs whose number is greater than 255. QEMU passes a max_lun of 16383 (because it uses SAM numbering) but in Linux it must become 32768 (because LUNs above 255 are relocated to 16640). Patch 2 is a resubmission of the patch for online resizing of virtio-scsi LUNs, which needs to be rebased. LUNs above 255 now work for all of scanning, hotplug, hotunplug and resize. Thanks, Paolo Paolo Bonzini (2): virtio-scsi: fix LUNs greater than 255 virtio-scsi: support online resizing of disks drivers/scsi/virtio_scsi.c | 37 +++-- include/linux/virtio_scsi.h |2 ++ 2 files changed, 37 insertions(+), 2 deletions(-) Ping, are these patches going into 3.7? They're 3.7 candidates yes (enhancements certainly aren't 3.6). I seem to have become lost with the virtio-scsi updates since what I have marked for inclusion is a patch series that's a partial intersection with this. I'll flush my queue for virto-scsi, please resend all the missing patches you want in 3.7. Thanks, James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 48241] New: oops when setting up LVM
On Wed, 2012-10-03 at 15:04 +, bugzilla-dae...@bugzilla.kernel.org wrote: https://bugzilla.kernel.org/show_bug.cgi?id=48241 Summary: oops when setting up LVM Product: IO/Storage Version: 2.5 Kernel Version: 3.6.0-next-20121003 Platform: All OS/Version: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: SCSI AssignedTo: linux-scsi@vger.kernel.org ReportedBy: daniel.san...@pobox.com Regression: No Created an attachment (id=81921) -- (https://bugzilla.kernel.org/attachment.cgi?id=81921) image of oops The image says the RIP is at kthread_data + 0xb That implies something went wrong within the workqueue or kthread systems, I've cc'd linux-kernel, but it's a bit of a vague thing to go on and could conceivably be a hardware issue (or some weird thread interaction in linux-next). The first question would be does it happen in vanilla 3.6? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: standards revisions
On Sat, 2012-10-06 at 23:11 -0700, Nicholas A. Bellinger wrote: On Sat, 2012-10-06 at 21:49 -0400, Christoph Hellwig wrote: Currenly all non-pscsi bakcneds report their standards version as SPC 2 via -get_device_rev. No, the proper on-the-wire bits to signal SPC-3 compliance are already being returned by virtual backend drivers within standard INQUIRY payload data. Notice the comment: root@tifa:/usr/src/target-pending.git# grep SCSI_SPC_2 drivers/target/*.c drivers/target/target_core_file.c:return SCSI_SPC_2; /* Returns SPC-3 in Initiator Data */ drivers/target/target_core_iblock.c: return SCSI_SPC_2; /* Returns SPC-3 in Initiator Data */ drivers/target/target_core_rd.c: return SCSI_SPC_2; /* Returns SPC-3 in Initiator Data */ That's causing confusion, I think! In addition to putting it into the inquirty data in spc_emulate_inquiry_std we also use it in two places to check on the version of the persistent reservation and alua support. What is the benefit of supporting the old-style SCSI 2 reservations and ALUA support when they can't be used at all with the virtual backends, and can only be used in the corner case emulated ALUA/PR support for pscsi? It's the include/scsi/scsi.h SCSI_3 + SCSI_SPC_* version definition names that are incorrect: #define SCSI_UNKNOWN0 #define SCSI_1 1 #define SCSI_1_CCS 2 #define SCSI_2 3 #define SCSI_3 4/* SPC */ #define SCSI_SPC_2 5 #define SCSI_SPC_3 6 from spc4r30 section 6.4.2 Standard INQUIRY data, Table 142 -- Version: 00h The device server does not claim conformance to any standard. 01h to 02h Obsolete 03h The device server complies to ANSI INCITS 301-1997 (a withdrawn standard). 04h The device server complies to ANSI INCITS 351-2001 (SPC-2). 05h The device server complies to ANSI INCITS 408-2005 (SPC-3). 06h The device server complies to this standard. How about the following patch to fix the long-standing incorrect name usage of these three scsi.h defines..? Because it's not incorrect. Your confusion is that the SCSI_ constants should match the inquiry data ... they shouldn't. Look where we set scsi_level in scsi_scan.c:708-713. We have to fit an extra identifier for SCSI_1_CCS so the scsi_level doesn't actually match the inquiry data; it's incremented by 1 for SCSI_3 and above. SCSI_3 does exist, by the way, it was defined in the first version of SPC and there are some devices which conform to it. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, resend] Fix race between starved list processing and device removal
On Mon, 2012-09-24 at 15:14 +0200, Bart Van Assche wrote: Avoid that the sdev reference count can drop to zero before the queue is run by scsi_run_queue(). Also avoid that the sdev reference count can drop to zero in the same function by invoking __blk_run_queue(). [...] if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0) @@ -977,6 +978,10 @@ void __scsi_remove_device(struct scsi_device *sdev) blk_cleanup_queue(sdev-request_queue); cancel_work_sync(sdev-requeue_work); + spin_lock_irqsave(shost-host_lock, flags); + list_del(sdev-starved_entry); + spin_unlock_irqrestore(shost-host_lock, flags); + This hunk doesn't make much sense. It seems to be orthogonal to the problem listed in the changelog and this action is done on last put anyway. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Request for improved commit tracking between fcoe and scsi trees
On Wed, 2012-10-03 at 15:23 -0400, Neil Horman wrote: James, Robert- I've been doing lots of backports of FCoE code to the RHEL tree these last few months, and I've noticed something fairly irritating, and I was wondering if you two could help me out with it (in fact you two are the only two which can). I noticed that commits which are accepted into the FCoE tree that get passed upstream through the scsi tree have their commit hashes altered. I can't find any examples currently, due to the fact that you, Robert, have recently re-cloned your git tree at open-fcoe.org, so all this nastiness has been covered up currently, but if things don't change, this issue will quickly resurface. Regardless, This makes it _really_ difficult to track a given patchs' traversal between trees upstream, and makes my life as a distro subsystem maintainer fairly painful. Normally I would just live with it, but I can't see any reason why it should be this way, given that git can easily prevent this with a pull. James, Robert, could you two please work out a way to provide commit hash consistency between your trees? It would make mine (and I'm sure many other people's) lives, much easier. I'm reluctant to commit to any tracking process that relies on stable commit ids simply because they're illusory in git: if we find a bug in a commit (or even worse a bisection failure) in a tree, we fix it there, which causes the commit id to change. The way I do this type of tracking is via the Subject: line ... why can't you use that? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Request for improved commit tracking between fcoe and scsi trees
On Thu, 2012-10-04 at 20:25 +, Love, Robert W wrote: On 10/3/2012 12:23 PM, Neil Horman wrote: James, Robert- I've been doing lots of backports of FCoE code to the RHEL tree these last few months, and I've noticed something fairly irritating, and I was wondering if you two could help me out with it (in fact you two are the only two which can). I noticed that commits which are accepted into the FCoE tree that get passed upstream through the scsi tree have their commit hashes altered. I can't find any examples currently, due to the fact that you, Robert, have recently re-cloned your git tree at open-fcoe.org, so all this nastiness has been covered up currently, but if things don't change, this issue will quickly resurface. Regardless, This makes it _really_ difficult to track a given patchs' traversal between trees upstream, and makes my life as a distro subsystem maintainer fairly painful. Normally I would just live with it, but I can't see any reason why it should be this way, given that git can easily prevent this with a pull. James, Robert, could you two please work out a way to provide commit hash consistency between your trees? It would make mine (and I'm sure many other people's) lives, much easier. I had included pull URLs in the covermails of my updates, but I haven't lately. I will make sure to do that from now on. Actually, I'm happy to do a pull based process with signed tags going forwards. However: Bart had a complaint about a misspelling in a commit message of a patch in my last update. I just resent that three patch series with the corrected commit message. I included a signed-tag to pull from in the covermail. That change changed the commit id and gives a graphic illustration of why any tracking process based on git commit ids is wrong. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 0/20][SCSI] mpt3sas: Driver submission v01.100.00.00
On Sat, 2012-09-29 at 19:48 +0530, sreekanth.re...@lsi.com wrote: This is new scsi lld device driver from LSI supporting the SAS 3.0 standard. Here is list of new 12gb host controllers: LSI SAS3004 LSI SAS3008 LSI SAS3108 Signed-off-by: Sreekanth Reddy sreekanth.re...@lsi.com Reviewed-by: Nagalakshmi Nandigama nagalakshmi.nandig...@lsi.com This doesn't seem to compile: drivers/scsi/mpt3sas/mpt3sas_scsih.c: In function ‘_scsih_sas_broadcast_primitive_event’: drivers/scsi/mpt3sas/mpt3sas_scsih.c:5368:2: error: ‘event_data’ undeclared (first use in this function) drivers/scsi/mpt3sas/mpt3sas_scsih.c:5368:2: note: each undeclared identifier is reported only once for each function it appears in drivers/scsi/mpt3sas/mpt3sas_ctl.c: In function ‘mpt3sas_ctl_reset_handler’: drivers/scsi/mpt3sas/mpt3sas_ctl.c:475:3: error: expected ‘)’ before ‘MPT3SAS_FMT9’ make[3]: *** [drivers/scsi/mpt3sas/mpt3sas_scsih.o] Error 1 make[3]: *** Waiting for unfinished jobs make[3]: *** [drivers/scsi/mpt3sas/mpt3sas_ctl.o] Error 1 make[2]: *** [drivers/scsi/mpt3sas] Error 2 make[2]: *** Waiting for unfinished jobs make[1]: *** [drivers/scsi] Error 2 make: *** [drivers] Error 2 Did you miss something in the split? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: standards revisions
On Sun, 2012-10-07 at 11:11 -0700, Nicholas A. Bellinger wrote: On Sun, 2012-10-07 at 09:16 +0100, James Bottomley wrote: On Sat, 2012-10-06 at 23:11 -0700, Nicholas A. Bellinger wrote: On Sat, 2012-10-06 at 21:49 -0400, Christoph Hellwig wrote: Currenly all non-pscsi bakcneds report their standards version as SPC 2 via -get_device_rev. No, the proper on-the-wire bits to signal SPC-3 compliance are already being returned by virtual backend drivers within standard INQUIRY payload data. Notice the comment: root@tifa:/usr/src/target-pending.git# grep SCSI_SPC_2 drivers/target/*.c drivers/target/target_core_file.c:return SCSI_SPC_2; /* Returns SPC-3 in Initiator Data */ drivers/target/target_core_iblock.c: return SCSI_SPC_2; /* Returns SPC-3 in Initiator Data */ drivers/target/target_core_rd.c: return SCSI_SPC_2; /* Returns SPC-3 in Initiator Data */ That's causing confusion, I think! In addition to putting it into the inquirty data in spc_emulate_inquiry_std we also use it in two places to check on the version of the persistent reservation and alua support. What is the benefit of supporting the old-style SCSI 2 reservations and ALUA support when they can't be used at all with the virtual backends, and can only be used in the corner case emulated ALUA/PR support for pscsi? It's the include/scsi/scsi.h SCSI_3 + SCSI_SPC_* version definition names that are incorrect: #define SCSI_UNKNOWN0 #define SCSI_1 1 #define SCSI_1_CCS 2 #define SCSI_2 3 #define SCSI_3 4/* SPC */ #define SCSI_SPC_2 5 #define SCSI_SPC_3 6 from spc4r30 section 6.4.2 Standard INQUIRY data, Table 142 -- Version: 00h The device server does not claim conformance to any standard. 01h to 02h Obsolete 03h The device server complies to ANSI INCITS 301-1997 (a withdrawn standard). 04h The device server complies to ANSI INCITS 351-2001 (SPC-2). 05h The device server complies to ANSI INCITS 408-2005 (SPC-3). 06h The device server complies to this standard. How about the following patch to fix the long-standing incorrect name usage of these three scsi.h defines..? Because it's not incorrect. Your confusion is that the SCSI_ constants should match the inquiry data ... they shouldn't. No, my current confusion is stemming from why it's OK for SCSI_SPC_[2,3] constant names+values to not match what is actually defined in SPC-4 drafts for what target INQUIRY emulation bits end up going 'over the wire'. OK, I don't understand what you didn't get about the explanation below. But the gist is it's not a constant representing inquiry[2]7; it's an ordered set of enumerations representing gross capabilities abstracted into sdev-scsi_level. Look where we set scsi_level in scsi_scan.c:708-713. We have to fit an extra identifier for SCSI_1_CCS so the scsi_level doesn't actually match the inquiry data; it's incremented by 1 for SCSI_3 and above. The extra increment by 1 is required by some Linux/SCSI client implementation dependent requirements, yes..? Not at all, no. It's defined and used in the mid-layer. ULDs may consult it or even (once in the case of usb) modify scsi_level, but they don't use it for directly altering inquiry data. SCSI_3 does exist, by the way, it was defined in the first version of SPC and there are some devices which conform to it. Regardless, SCSI_SPC_[2,3] - SCSI_SPC[3,4] constants for target-core + scsi-core should still be re-named to avoid the extra confusion this introduces for folks reading code. I'm not convinced there is confusion; this use of enumerated levelling goes back to 2.2 and no-one else has had a problem with it. You're the only one whose had an issue and it does seem to be because you didn't bother reading the comment above their definitions in the header file which does explain what's going on. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/6] scsi: sr: support runtime pm
On Mon, 2012-10-08 at 17:27 +0800, Aaron Lu wrote: On Sun, Sep 30, 2012 at 03:43:27PM -0400, Alan Stern wrote: On Sun, 30 Sep 2012, Jeff Garzik wrote: The simple fact of only ZPODD devices out there are ATA is not the decision-maker for where the code should live. It is more a question where ZPODD belongs in the device/command set model currently employed. I don't really accept this argument. It's a little like saying: The tty layer uses ioctl commands to control RS232 line settings; therefore RS232 settings should be handled in the VFS layer as part of the ioctl core. Regardless, according to Aaron the ZP power-off stuff is currently implemented only in ACPI, tied more closely to the ATA layer than the SCSI layer (though not part of either). It is not part of the SCSI spec in any form. The mechanism of SATA ODD zero power model is specified in Mount Fuji spec v8 section 15 describing what the ODD needs support, how to sense if the ODD is ready to be powered off and on power up what needs to be done, etc. And the actual power off and wakeup is implemented in ACPI and SATA. Now it's true that determining whether a device is in the right state for power to be removed involves sending a TEST UNIT READY command, which is of course a SCSI command. This seems to be incidental to the overall scheme, however. I need to add that, there are 2 schemes to sense if the ODD is ready to be powered off: 1 the one I used here, by checking if the door is closed and no media inside with test_unit_ready; 2 some ZP capable ODDs can report zero power ready(ZPReady) event to host when queried, eliminating the need of host checking the conditions. The way I read the standard is that ZP ODD is a hack to try and get to off and ZPready is the same hack but integrated into the standardised power management states (and hence available to normal power saving). The standard even implies ZP ODD is a less desirable hack by recommending the use of ZPready. The ZPready method, being an extension of the usual SCSI power management model, is pretty much what we support today (especially as the whole thing is timer driven from values in the mode page and happens pretty much invisibly to us). Since the object is to make this as painless as possible, why don't we just implement ZPODD the way the spec recommends? i.e. emulate the timers at an incredibly low level and intercept and emulate the non-disk commands listed in table 321. I bet, in Linux, since we moved basically to GET_EVENT_STATUS_NOTIFICATION, that's the only one that actually needs an emulation. The state model seems to work if you snoop the polled media event, so you wait for no media, then set your internal timer, stop it if we get a media change and power off the device after interval expiry. Thereafter you emulate media event with no change keeping the device powered off. If a disc gets inserted or the eject button is pressed, you see that via the SATA PHY events (so wake the drive) and if the Upper Layer sends an unexpected command, you also power on the drive. That way all of this should be nicely containable within SATA/ACPI. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [V5 PATCH 0/8] csiostor: Chelsio FCoE offload driver submission
On Mon, 2012-10-08 at 15:37 +0530, Naresh Kumar Inna wrote: Please let me know if you are expecting any more changes to this driver to resume its review. I have addressed all review comments as of the last series of patches (below). No, I don't think so, but please understand that the review talent is all busy concentrating on the merge window for at least the next week and a bit, so I won't be able to get them to pay attention until 3.7-rc1 is released. Thanks, James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] arcmsr: Re-name the HBA Type
On Wed, 2012-10-03 at 20:39 +0800, NickCheng wrote:
From: Nick Cheng nick.ch...@areca.com.tw
Replace the nameing, hba, hbb and hbc, with hbaA, hbaB abd hbaC respectively
Signed-off-by: Nick Cheng nick.ch...@areca.com.tw
---
diff -uprN -X linux-vanilla/Documentation/dontdiff
linux-vanilla//drivers/scsi/arcmsr/arcmsr.h
linux-development//drivers/scsi/arcmsr/arcmsr.h
--- linux-vanilla//drivers/scsi/arcmsr/arcmsr.h 2012-10-03
18:29:18.030657090 +0800
+++ linux-development//drivers/scsi/arcmsr/arcmsr.h 2012-10-03
18:43:58.542648536 +0800
This patch is linebroken and can't be applied. Whatever mail tool
you're using must have broken the lines. Please see
Documentation/email-clients.txt
About how to fix this and resubmit.
Thanks,
James
@@ -51,7 +51,7 @@ struct device_attribute;
#else
#define ARCMSR_MAX_FREECCB_NUM 320
#endif
-#define ARCMSR_DRIVER_VERSION Driver Version 1.20.00.15
2010/08/05
+#define ARCMSR_DRIVER_VERSION Driver Version 1.20.00.15
2012/09/30
#define ARCMSR_SCSI_INITIATOR_ID
255
#define ARCMSR_MAX_XFER_SECTORS
512
#define ARCMSR_MAX_XFER_SECTORS_B
4096
diff -uprN -X linux-vanilla/Documentation/dontdiff
linux-vanilla//drivers/scsi/arcmsr/arcmsr_hba.c
linux-development//drivers/scsi/arcmsr/arcmsr_hba.c
--- linux-vanilla//drivers/scsi/arcmsr/arcmsr_hba.c 2012-10-03
18:29:18.030657090 +0800
+++ linux-development//drivers/scsi/arcmsr/arcmsr_hba.c 2012-10-03
18:43:58.542648536 +0800
@@ -61,6 +61,7 @@
#include linux/aer.h
#include asm/dma.h
#include asm/io.h
+#include asm/system.h
#include asm/uaccess.h
#include scsi/scsi_host.h
#include scsi/scsi.h
@@ -95,16 +96,16 @@ static void arcmsr_iop_init(struct Adapt
static void arcmsr_free_ccb_pool(struct AdapterControlBlock *acb);
static u32 arcmsr_disable_outbound_ints(struct AdapterControlBlock *acb);
static void arcmsr_stop_adapter_bgrb(struct AdapterControlBlock *acb);
-static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb);
-static void arcmsr_flush_hbb_cache(struct AdapterControlBlock *acb);
+static void arcmsr_hbaA_flush_cache(struct AdapterControlBlock *acb);
+static void arcmsr_hbaB_flush_cache(struct AdapterControlBlock *acb);
static void arcmsr_request_device_map(unsigned long pacb);
-static void arcmsr_request_hba_device_map(struct AdapterControlBlock *acb);
-static void arcmsr_request_hbb_device_map(struct AdapterControlBlock *acb);
-static void arcmsr_request_hbc_device_map(struct AdapterControlBlock *acb);
+static void arcmsr_hbaA_request_device_map(struct AdapterControlBlock
*acb);
+static void arcmsr_hbaB_request_device_map(struct AdapterControlBlock
*acb);
+static void arcmsr_hbaC_request_device_map(struct AdapterControlBlock
*acb);
static void arcmsr_message_isr_bh_fn(struct work_struct *work);
static bool arcmsr_get_firmware_spec(struct AdapterControlBlock *acb);
static void arcmsr_start_adapter_bgrb(struct AdapterControlBlock *acb);
-static void arcmsr_hbc_message_isr(struct AdapterControlBlock *pACB);
+static void arcmsr_hbaC_message_isr(struct AdapterControlBlock *pACB);
static void arcmsr_hardware_reset(struct AdapterControlBlock *acb);
static const char *arcmsr_info(struct Scsi_Host *);
static irqreturn_t arcmsr_interrupt(struct AdapterControlBlock *acb);
@@ -308,7 +309,7 @@ static void arcmsr_define_adapter_type(s
}
}
-static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock
*acb)
+static uint8_t arcmsr_hbaA_wait_msgint_ready(struct AdapterControlBlock
*acb)
{
struct MessageUnit_A __iomem *reg = acb-pmuA;
int i;
@@ -326,7 +327,7 @@ static uint8_t arcmsr_hba_wait_msgint_re
return false;
}
-static uint8_t arcmsr_hbb_wait_msgint_ready(struct AdapterControlBlock
*acb)
+static uint8_t arcmsr_hbaB_wait_msgint_ready(struct AdapterControlBlock
*acb)
{
struct MessageUnit_B *reg = acb-pmuB;
int i;
@@ -346,7 +347,7 @@ static uint8_t arcmsr_hbb_wait_msgint_re
return false;
}
-static uint8_t arcmsr_hbc_wait_msgint_ready(struct AdapterControlBlock
*pACB)
+static uint8_t arcmsr_hbaC_wait_msgint_ready(struct AdapterControlBlock
*pACB)
{
struct MessageUnit_C *phbcmu = (struct MessageUnit_C *)pACB-pmuC;
int i;
@@ -364,13 +365,13 @@ static uint8_t arcmsr_hbc_wait_msgint_re
return false;
}
-static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
+static void arcmsr_hbaA_flush_cache(struct AdapterControlBlock *acb)
{
struct MessageUnit_A __iomem *reg = acb-pmuA;
int retry_count = 30;
writel(ARCMSR_INBOUND_MESG0_FLUSH_CACHE, reg-inbound_msgaddr0);
do {
- if (arcmsr_hba_wait_msgint_ready(acb))
+ if (arcmsr_hbaA_wait_msgint_ready(acb))
break;
else {
retry_count--;
@@ -380,13 +381,13 @@ static void arcmsr_flush_hba_cache(struc
Re: [PATCH 1/5] arcmsr: Re-name the HBA Type
On Tue, 2012-10-09 at 21:23 +0800, NickCheng wrote: From: Nick Cheng nick.ch...@areca.com.tw Replace the nameing, hba, hbb and hbc, with hbaA, hbaB abd hbaC respectively Signed-off-by: Nick Cheng nick.ch...@areca.com.tw This is still malformed in exactly the same way diff -uprN -X linux-vanilla/Documentation/dontdiff linux-vanilla//drivers/scsi/arcmsr/arcmsr.h linux-development//drivers/scsi/arcmsr/arcmsr.h --- linux-vanilla//drivers/scsi/arcmsr/arcmsr.h 2012-10-03 There's a spurious carriage return here which kills the patch applicability 18:29:18.030657090 +0800 James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/6] scsi: sr: support runtime pm
On Tue, 2012-10-09 at 15:20 +0800, Aaron Lu wrote: On 10/08/2012 06:21 PM, James Bottomley wrote: On Mon, 2012-10-08 at 17:27 +0800, Aaron Lu wrote: On Sun, Sep 30, 2012 at 03:43:27PM -0400, Alan Stern wrote: On Sun, 30 Sep 2012, Jeff Garzik wrote: The simple fact of only ZPODD devices out there are ATA is not the decision-maker for where the code should live. It is more a question where ZPODD belongs in the device/command set model currently employed. I don't really accept this argument. It's a little like saying: The tty layer uses ioctl commands to control RS232 line settings; therefore RS232 settings should be handled in the VFS layer as part of the ioctl core. Regardless, according to Aaron the ZP power-off stuff is currently implemented only in ACPI, tied more closely to the ATA layer than the SCSI layer (though not part of either). It is not part of the SCSI spec in any form. The mechanism of SATA ODD zero power model is specified in Mount Fuji spec v8 section 15 describing what the ODD needs support, how to sense if the ODD is ready to be powered off and on power up what needs to be done, etc. And the actual power off and wakeup is implemented in ACPI and SATA. Now it's true that determining whether a device is in the right state for power to be removed involves sending a TEST UNIT READY command, which is of course a SCSI command. This seems to be incidental to the overall scheme, however. I need to add that, there are 2 schemes to sense if the ODD is ready to be powered off: 1 the one I used here, by checking if the door is closed and no media inside with test_unit_ready; 2 some ZP capable ODDs can report zero power ready(ZPReady) event to host when queried, eliminating the need of host checking the conditions. The way I read the standard is that ZP ODD is a hack to try and get to Thanks for your time. off and ZPready is the same hack but integrated into the standardised power management states (and hence available to normal power saving). The standard even implies ZP ODD is a less desirable hack by recommending the use of ZPready. There are ZPODDs not supporting ZPready and I want to support them so the sense scheme 1 is used. Right, but what I'm saying is that ZPODD looks like a hack until ZPready is fairly universally implemented. ZPready makes far more sense since it's integrated into the usual SCSI power management, so ZPODD should have a limited shelf life. The ZPready method, being an extension of the usual SCSI power management model, is pretty much what we support today (especially as the whole thing is timer driven from values in the mode page and happens pretty much invisibly to us). Since the object is to make this as painless as possible, why don't we just implement ZPODD the way the spec recommends? i.e. emulate the timers at an incredibly low level and intercept and emulate the non-disk commands listed in table 321. I bet, in Linux, since we moved basically to GET_EVENT_STATUS_NOTIFICATION, that's the only one that actually needs an emulation. The state model seems to work if you snoop the polled media event, so you wait for no media, then set your internal timer, stop it if we get a media change and power off the device after interval expiry. Thereafter you emulate media event with no change keeping the device powered off. If a disc gets inserted or the eject button is pressed, you see that via the SATA PHY events (so wake the drive) and if the Upper Layer sends an unexpected command, you also power on the drive. That way all of this should be nicely containable within SATA/ACPI. Thanks for the suggestion, it is really something that I've never thought of :-) But I was hoping to use the runtime pm framework to support ZPODD. Well, the runtime pm framework doesn't support the current SCSI power management states within the drive, that's why it makes sense to treat what is essentially a hack to them in the same manner. With your suggestion, I don't know how to do this. Maybe I can set the scsi device representing the ODD to runtime suspended once I decided to power it off and resume it when I power it up. But there is a problem, that I'm setting a scsi device's runtime status in ATA layer, this doesn't feel right. And someday, someone may want to add runtime pm support for sr and the runtime status of the scsi device will be messed. No, if we ever actually supported the standard power management states, you'd simply be intercepting the SCSI commands that forced the state transitions (START_STOP_UNIT) and act when yours was forced. The reason I want to use runtime PM is, when the scsi device is runtime suspended, its ancestor devices will have a chance to enter runtime suspend state, e.g. the sata controller. But this, I think, is why it looks odd. You're implementing a lower state than standard SCSI power
Re: [PATCH] sd: Reshuffle init_sd to avoid crash
On Wed, 2012-10-10 at 10:36 +0200, Hannes Reinecke wrote: scsi_register_driver will register a prep_fn() function, which in turn migh need to use the sd_cdp_pool for DIF. Which hasn't been initialised at this point, leading to a crash. So reshuffle the init_sd() and exit_sd() paths to have the driver registered last. Signed-off-by: Joel D. Diaz joeld...@us.ibm.com Signed-off-by: Hannes Reinecke h...@suse.de Explain what these signoffs mean, since the patch is sent with you as the author. Thanks, James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sd: Reshuffle init_sd to avoid crash
On Wed, 2012-10-10 at 12:01 +0200, Hannes Reinecke wrote: On 10/10/2012 11:55 AM, James Bottomley wrote: On Wed, 2012-10-10 at 10:36 +0200, Hannes Reinecke wrote: scsi_register_driver will register a prep_fn() function, which in turn migh need to use the sd_cdp_pool for DIF. Which hasn't been initialised at this point, leading to a crash. So reshuffle the init_sd() and exit_sd() paths to have the driver registered last. Signed-off-by: Joel D. Diaz joeld...@us.ibm.com Signed-off-by: Hannes Reinecke h...@suse.de Explain what these signoffs mean, since the patch is sent with you as the author. Ah. Sorry. Joel Diaz send the patch relative to SLES11 SP2, and I ported / send it to mainline. So yeah, Joel is actually the author. Should I send an updated patch? That's OK, I can fix this up on the fly (this time). James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] scsi: make struct scsi_varlen_cdb_hdr packed
On Thu, 2012-10-11 at 10:15 +0100, James Hogan wrote:
The struct scsi_varlen_cdb_hdr is expected to be exactly 10 bytes when
used in struct osd_cdb_head, but it isn't marked as packed. Some
architectures will round the struct size up which triggers BUILD_BUG_ON
compile errors in osd_initiator.c when the outer structs are unexpected
sizes. This is fixed by marking struct scsi_varlen_cdb_hdr as __packed.
What actual problem have you encountered? The structure is {u8[8], u16}
which is naturally packed on every architecture I know about. I've even
built osd_initiator without problem on parisc, which has some of the
most rigid alignment rules I've seen.
James
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Drivers: scsi
On Wed, 2012-10-24 at 09:25 -0700, K. Y. Srinivasan wrote: When the low level driver returns SCSI_MLQUEUE_DEVICE_BUSY, how is the command retried; I suspect the retry is done after some delay. Delay depends mainly on I/O pressure and the unplug timer in the block layer. Is this delay programmable? If the device state changes, can the low level driver notify upper layers that it can now handle the command that it had failed earlier with SCSI_MLQUEUE_DEVICE_BUSY. In theory, you can call blk_run_queue() from the event handler that sees the device become ready. I don't think any driver actually does this, but I can't see it would cause any problem. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] aic7xxx_old: silence GCC warnings
On Mon, 2012-10-29 at 11:36 +0100, Paul Bolle wrote: On Fri, 2012-09-21 at 11:28 +0200, Paul Bolle wrote: Building the aic7xxx_old driver triggers these GCC warnings: drivers/scsi/aic7xxx_old.c:7901:5: warning: case value '257' not in enumerated type 'ahc_chip' [-Wswitch] drivers/scsi/aic7xxx_old.c:7898:5: warning: case value '513' not in enumerated type 'ahc_chip' [-Wswitch] drivers/scsi/aic7xxx_old.c:8517:5: warning: case value '257' not in enumerated type 'ahc_chip' [-Wswitch] drivers/scsi/aic7xxx_old.c:8510:5: warning: case value '513' not in enumerated type 'ahc_chip' [-Wswitch] Fix these warnings by adopting the idiom used elsewhere in this driver. Since AHC_EISA and AHC_VL are only ever set for AHC_AIC7770 this fix should not lead to any functional change. Signed-off-by: Paul Bolle pebo...@tiscali.nl --- 0) I noticed these warnings while building v3.6-rc6 on current Fedora 17, using Fedora's default config. Identical warnings can be seen while building v3.7-rc3. What's the status of my patch? Did anyone found some time to have a look at it? the aic7xxx_old driver is in deep maintenance; I don't think anyone can actually test changes to it anymore, so we just keep it around unchanged for the odd really old card that can't be driven by the current driver. For this reason, we tend only to do tested bug fixes to it because no-one will notice if any error is introduced. mvsas has a maintainer: poke them harder and aic94xx is currently maintainerless, so it will depend on someone finding the time to test. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 02/11] ata: zpodd: Add CONFIG_SATA_ZPODD
On Mon, 2012-10-29 at 17:01 +0800, Aaron Lu wrote: Added a new config CONFIG_SATA_ZPODD, which is ued to support SATA based zero power ODD. It depends on ACPI, and selects BLK_DEV_SR as the implementation of ZPODD depends on SCSI sr driver. 2 new files are added, which will be used to host ZPODD related code. They are empty for this commit. I think you actually need to combine this with patch 3 to have it compile ... James Signed-off-by: Aaron Lu aaron...@intel.com --- drivers/ata/Kconfig | 12 drivers/ata/Makefile | 1 + 2 files changed, 13 insertions(+) create mode 100644 drivers/ata/sata_zpodd.c create mode 100644 drivers/ata/sata_zpodd.h diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index e08d322..2cdecee 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -58,6 +58,18 @@ config ATA_ACPI You can disable this at kernel boot time by using the option libata.noacpi=1 +config SATA_ZPODD + bool SATA Zero Power ODD Support + depends on ACPI + select BLK_DEV_SR + default n + help + This option adds support for SATA ZPODD. It requires both + ODD and the platform support, and if enabled, will automatically + power on/off the ODD when certain condition is satisfied. This + does not impact user's experience of the ODD, only power is saved + when ODD is not in use(i.e. no disc inside). + config SATA_PMP bool SATA Port Multiplier support default y diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile index 9329daf..a5120ff 100644 --- a/drivers/ata/Makefile +++ b/drivers/ata/Makefile @@ -107,3 +107,4 @@ libata-y := libata-core.o libata-scsi.o libata-eh.o libata-transport.o libata-$(CONFIG_ATA_SFF) += libata-sff.o libata-$(CONFIG_SATA_PMP)+= libata-pmp.o libata-$(CONFIG_ATA_ACPI)+= libata-acpi.o +obj-$(CONFIG_SATA_ZPODD) += sata_zpodd.o diff --git a/drivers/ata/sata_zpodd.c b/drivers/ata/sata_zpodd.c new file mode 100644 index 000..e69de29 diff --git a/drivers/ata/sata_zpodd.h b/drivers/ata/sata_zpodd.h new file mode 100644 index 000..e69de29 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 10/11] scsi: sr: support (un)block events
On Mon, 2012-10-29 at 17:01 +0800, Aaron Lu wrote:
2 interfaces are added to block/unblock events for the disk sr manages.
This is used by SATA ZPODD, when ODD is runtime powered off, the events
poll is no longer needed so better be blocked. And once powered on,
events poll will be unblocked.
These 2 interfaces are needed here because SATA layer does not have
access to the gendisk structure sr manages.
I'm afraid this is a nasty layering violation. You can't have a low
level driver have knowledge of a call back in an upper layer one (in
this case sr_block_events).
This is all done it looks like because of the problem of getting access
to the scsi_cd structure from libata, but it's not the right way to
solve it.
I also don't really think you need to do any blocking or unblocking. As
I said in the previous thread, just fake the events the standard tells
you to, so userspace can probe to its heart's content and you can keep
the device powered off.
James
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/scsi/Makefile | 1 +
drivers/scsi/sr_zpodd.c | 21 +
drivers/scsi/sr_zpodd.h | 9 +
3 files changed, 31 insertions(+)
create mode 100644 drivers/scsi/sr_zpodd.c
create mode 100644 drivers/scsi/sr_zpodd.h
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 888f73a..474efe2 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -177,6 +177,7 @@ sd_mod-objs := sd.o
sd_mod-$(CONFIG_BLK_DEV_INTEGRITY) += sd_dif.o
sr_mod-objs := sr.o sr_ioctl.o sr_vendor.o
+sr_mod-$(CONFIG_SATA_ZPODD) += sr_zpodd.o
ncr53c8xx-flags-$(CONFIG_SCSI_ZALON) \
:= -DCONFIG_NCR53C8XX_PREFETCH -DSCSI_NCR_BIG_ENDIAN \
-DCONFIG_SCSI_NCR53C8XX_NO_WORD_TRANSFERS
diff --git a/drivers/scsi/sr_zpodd.c b/drivers/scsi/sr_zpodd.c
new file mode 100644
index 000..0686e8c
--- /dev/null
+++ b/drivers/scsi/sr_zpodd.c
@@ -0,0 +1,21 @@
+#include linux/module.h
+#include linux/genhd.h
+#include linux/cdrom.h
+#include sr.h
+
+bool sr_block_events(struct device *dev)
+{
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+ if (cd)
+ return disk_try_block_events(cd-disk);
+ return false;
+}
+EXPORT_SYMBOL(sr_block_events);
+
+void sr_unblock_events(struct device *dev)
+{
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+ if (cd)
+ disk_unblock_events(cd-disk);
+}
+EXPORT_SYMBOL(sr_unblock_events);
diff --git a/drivers/scsi/sr_zpodd.h b/drivers/scsi/sr_zpodd.h
new file mode 100644
index 000..49c6a55
--- /dev/null
+++ b/drivers/scsi/sr_zpodd.h
@@ -0,0 +1,9 @@
+#ifndef __SR_ZPODD_H__
+#define __SR_ZPODD_H__
+
+#include linux/device.h
+
+extern bool sr_block_events(struct device *dev);
+extern void sr_unblock_events(struct device *dev);
+
+#endif
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 10/11] scsi: sr: support (un)block events
On Mon, 2012-10-29 at 18:22 -0400, Alan Stern wrote: On Mon, 29 Oct 2012, James Bottomley wrote: On Mon, 2012-10-29 at 17:01 +0800, Aaron Lu wrote: 2 interfaces are added to block/unblock events for the disk sr manages. This is used by SATA ZPODD, when ODD is runtime powered off, the events poll is no longer needed so better be blocked. And once powered on, events poll will be unblocked. These 2 interfaces are needed here because SATA layer does not have access to the gendisk structure sr manages. I'm afraid this is a nasty layering violation. You can't have a low level driver have knowledge of a call back in an upper layer one (in this case sr_block_events). This is all done it looks like because of the problem of getting access to the scsi_cd structure from libata, but it's not the right way to solve it. I also don't really think you need to do any blocking or unblocking. As I said in the previous thread, just fake the events the standard tells you to, so userspace can probe to its heart's content and you can keep the device powered off. James, one of us has misunderstood the event-polling mechanism. As far as I know, the polling calls that Aaron is concerned with are generated by the kernel, not by userspace. I don't think the origin of the poll changes anything in what I said above, does it? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Drivers: scsi
On Sat, 2012-11-03 at 09:40 -0700, K. Y. Srinivasan wrote: Do we currently support dynamic re-sizing of LUNs. Hyper-V can notify capacity change via sense data and I was wondering if this is handled in the generic scsi code. Depends what you mean by dynamic. Experience shows that most users really don't want all this to happen without manual intervention (particularly on shrink). However, we have the mechanics in place. Just send ASC/Q 2A/09 and it can get vectored up to LVM which resizes the volume and can be scripted automatically to resize the filesystem. hypervisor people tend to have problems with scripting, so there are some hacks that do this outside of the normal event system ... see for example virtio_scsi.c James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [SCSI] mvsas: fix shift in mvs_94xx_free_reg_set()
On Mon, 2012-11-05 at 14:53 -0500, Xi Wang wrote: Invoking bit(n) with n = 64 is undefined behavior, since bit(n) does a 64-bit shift. This patch adds a check on the shifting amount. Why is this necessary? As I read the reg set assignment code, it finds a free bit in the 64 bit register and uses that ... which can never be greater than 64 so there's no need for the check. The other two look OK (probably redone as a single patch with a stable tag), but I'd like the input of the mvs people since it seems with the current code, we only use 32 bit regsets and probably hang if we go over that. The bug fix is either to enable the full 64 if it works, or possibly cap at 32 ... what works with all released devices? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] target: Update copyright ownership to 2012
On Fri, 2012-11-09 at 23:00 +, Nicholas A. Bellinger wrote: diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c index 4c8eea2..035c606 100644 --- a/drivers/target/target_core_alua.c +++ b/drivers/target/target_core_alua.c @@ -3,8 +3,9 @@ * * This file contains SPC-3 compliant asymmetric logical unit assigntment (ALUA) * - * Copyright (c) 2009-2010 Rising Tide Systems - * Copyright (c) 2009-2010 Linux-iSCSI.org + * (c) Copyright 2009-2012 RisingTide Systems LLC. + * + * Licensed to the Linux Foundation under the General Public License (GPL) version 2. I don't quite understand this last bit. Firstly, I'm not aware of any agreement with the Linux Foundation over this and secondly one of the cardinal principles of GPL is the copyleft one that all downstreams get equal rights, so you can't licence GPL code to anyone in particular because you have to licence it to everyone. Thirdly, I think it should be the GNU General Public Licence. In the context of the kernel, that's probably taken as read, but if you're going to add it explicitly, might as well be precise. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi target, likely GPL violation
On Wed, 2012-11-07 at 08:50 -0800, Andy Grover wrote: Nick, Your company appears to be shipping kernel features in RTS OS that are not made available under the GPL, specifically support for the EXTENDED_COPY and COMPARE_AND_WRITE SCSI commands, in order to claim full Vmware vSphere 5 VAAI support. http://www.risingtidesystems.com/storage.html http://www.linux-iscsi.org/wiki/VAAI Private emails to you and RTS CEO Marc Fleischmann have not elicited a useful response. You are subsystem maintainer for the in-kernel SCSI target support (drivers/target/*), and your company appears to be violating the GPL. Please explain. Can we please cool it with the inflammatory accusations. Please remember that statements which damage or seek to damage the reputation of a company amount to libel even under US law ... and using phrases like appears to doesn't shield you from this. I also note that whatever their website says RTS OS isn't in VMware's certified compatibility list: http://www.vmware.com/resources/compatibility/pdf/vi_io_guide.pdf Plus it's a grey area what you actually have to support to make that list (especially as XCOPY has now been removed from SBC-3 in favour of token copy), so I'd say that the chain of reasoning you've used to come up with this hearsay allegation of copyright violation is tenuous at best. Anybody who does enforcement will tell you that you begin with first hand proof of a violation. That means obtain the product and make sure it's been modified and that a request for corresponding source fails. In this case, since I presume Red Hat, as a RTS partner, has a bona fide copy of the RTS OS, please verify it does indeed implement or issue the commands which are not in the public git repository and that whoever owns the copy makes a request for the source code. I would really appreciate it if the next email I see from you on this subject is either 1. Yes, I've got first hand proof of a GPL violation (in which case we'll then move to seeing how we can remedy this) or 2. A genuine public apology for the libel, which I'll do my best to prevail on RTS to accept. Because any further discussion of unsubstantiated allegations of this nature exposes us all to jeopardy of legal sanction. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi target, likely GPL violation
On Sun, 2012-11-11 at 10:15 -0500, Bradley M. Kuhn wrote: James wrote: [I'd like to see] a genuine public apology for the libel... Because any further discussion of unsubstantiated allegations of this nature exposes us all to jeopardy of legal sanction. Hey that's a complete misrepresentation. I expressed no such opinion in the email. What I said was: I would really appreciate it if the next email I see from you on this subject is either 1. Yes, I've got first hand proof of a GPL violation (in which case we'll then move to seeing how we can remedy this) or 2. A genuine public apology for the libel, which I'll do my best to prevail on RTS to accept. Because any further discussion of unsubstantiated allegations of this nature exposes us all to jeopardy of legal sanction. That asks for moderation until we have a better investigation of the facts. It definitely doesn't try to prejudge them or express preference for a specific outcome as your misquote makes out. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB enclosures seem to require read(16) with 2TB drives
On Fri, 2012-11-09 at 16:33 +, Elliott, Robert (Server Storage) wrote: I recommend broadening this patch. T10 is discussing making READ (10), WRITE (10), etc. obsolete in SBC-4 in favor of their 16-byte CDB counterparts. The algorithm should be: 1. During discovery, determine if 16-byte CDBs are supported. There are several ways to determine this: a) REPORT SUPPORTED OPERATION CODES command succeeds and reports that READ (16) et al are supported. b) READ (16) command specifying a Transfer Length of zero succeeds. c) READ CAPACITY (16) command succeeds and reports that the capacity is 2 TiB. d) (future) INQUIRY command succeeds fetching the Block Device Characteristics VPD page and notices a new field added by the SBC-4 simplified SCSI feature set proposal. When you consider the problem that we must support all devices (which means the older ones as well), including the annoying USB ones which crash on unexpected commands, you can see that d) is about the only viable option. We can also force RC16 if the capacity is over 2^32 blocks, because the command will be required, so that's probably the place to start. James Since REPORT SUPPORTED OPERATION CODES is optional, it won't always work. READ CAPACITY (16) used to be optional for 2 TiB drives, so it won't always work either. READ (16) will always work, but requires the drive to be spun up beforehand (e.g., with START STOP UNIT). 2. if 16-byte CDBs are supported, then use them; only drop down to 10-byte CDBs if 16-byte CDBs are unavailable. Don't make the decision by comparing the LBA on every IO. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB enclosures seem to require read(16) with 2TB drives
On Fri, 2012-11-09 at 11:08 -0500, Jason J. Herne wrote:
diff --git a/drivers/usb/storage/scsiglue.c
b/drivers/usb/storage/scsiglue.c
index 13b8bcd..6ff785e 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -251,6 +251,11 @@ static int slave_configure(struct scsi_device
*sdev)
US_FL_SCM_MULT_TARG))
us-protocol == USB_PR_BULK)
us-use_last_sector_hacks = 1;
+
+ /* Force read-16 for large capacity drives. */
+ sdev-force_read_16 = 1;
+
+
This turns READ_16 on unconditionally for all USB disks ... is that
safe? As in have you tested this with older USB thumb drives?
Also do we have to worry about TYPE_RBC? ... this looks like a bit of a
global omission in usbstorage.c
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5591ed5..e92b846 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -134,6 +134,7 @@ struct scsi_device {
* because we did a bus reset. */
unsigned use_10_for_rw:1; /* first try 10-byte read / write */
unsigned use_10_for_ms:1; /* first try 10-byte mode
sense/select */
+ unsigned force_read_16:1; /* Use read(16) over read(10) */
This should probably be use_16_for_rw:1 for consistency.
James
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB enclosures seem to require read(16) with 2TB drives
On Mon, 2012-11-12 at 15:31 +0100, Paolo Bonzini wrote:
Il 12/11/2012 12:33, James Bottomley ha scritto:
On Fri, 2012-11-09 at 11:08 -0500, Jason J. Herne wrote:
diff --git a/drivers/usb/storage/scsiglue.c
b/drivers/usb/storage/scsiglue.c
index 13b8bcd..6ff785e 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -251,6 +251,11 @@ static int slave_configure(struct scsi_device
*sdev)
US_FL_SCM_MULT_TARG))
us-protocol == USB_PR_BULK)
us-use_last_sector_hacks = 1;
+
+ /* Force read-16 for large capacity drives. */
+ sdev-force_read_16 = 1;
+
+
This turns READ_16 on unconditionally for all USB disks ... is that
safe? As in have you tested this with older USB thumb drives?
Actually it only turns it on for large capacity drives, as said in the
comment. sdp-force_read_16 only matters for 2TB drives:
If you follow the discussion, we'll need to turn it on for some drives
regardless of size.
+ /* Many large capacity USB drives/controllers require the use of
read(16). */
+ force_read16 = (sdkp-capacity 0xULL sdp-force_read_16);
+
if (host_dif == SD_DIF_TYPE2_PROTECTION) {
SCpnt-cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
@@ -833,7 +836,7 @@ static int sd_prep_fn(struct request_queue *q, struct
request *rq)
SCpnt-cmnd[29] = (unsigned char) (this_count 16) 0xff;
SCpnt-cmnd[30] = (unsigned char) (this_count 8) 0xff;
SCpnt-cmnd[31] = (unsigned char) this_count 0xff;
- } else if (block 0x) {
+ } else if (block 0x || force_read16) {
so the better name would be never_use_10_for_rw_on_large_disks. For
some definitions of better. :)
Hm.
Any reason not to do this always on 2TB drives, which basically means
changing this:
- } else if (block 0x) {
+ } else if (sdkp-capacity 0x) {
and nothing else?
Because of the coming T10 mandate in SBC-4 deprecating everything other
than the 16 byte commands.
James
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB enclosures seem to require read(16) with 2TB drives
On Mon, 2012-11-12 at 10:01 -0500, Jason J. Herne wrote:
Any reason not to do this always on 2TB drives, which basically means
changing this:
- } else if (block 0x) {
+ } else if (sdkp-capacity 0x) {
and nothing else?
This was the intent of my patch, except I wanted to *only* affect USB based
drives as my drive was functioning perfecting when connected to an internal
Sata port. I was adopting the Do not fix what isn't broke mentality.
There's a subtlety here: block is in units of the disk sector size,
sdkp-capacity is in units of 512 bytes (the linux native sector size),
so it would need shifting before doing the determination.
James
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 17/19] be2iscsi: Fix for MBX timeout issue
On Wed, 2012-10-24 at 01:45 -0500, Mike Christie wrote:
On 10/19/2012 06:15 PM, John Soni Jose wrote:
@@ -763,26 +750,14 @@ static int beiscsi_get_port_speed(struct Scsi_Host
*shost)
BS_%d : Getting Port Speed Failed\n);
return -EBUSY;
-} else
- wait_event_interruptible(phba-ctrl.mcc_wait[tag],
- phba-ctrl.mcc_numtag[tag]);
-
- wrb_num = (phba-ctrl.mcc_numtag[tag] 0x00FF) 16;
- extd_status = (phba-ctrl.mcc_numtag[tag] 0xFF00) 8;
- status = phba-ctrl.mcc_numtag[tag] 0x00FF;
-
- if (status || extd_status) {
+}
+ rc = beiscsi_mccq_compl(phba, tag, wrb, NULL);
+ if (rc) {
beiscsi_log(phba, KERN_ERR,
There was some small whitespace issues in this patch. Maybe James will
fix it up when it is merged. I do not know why checkpatch.pl and git did
not warn about them.
I fixed it up. I cc'd the checkpatch.pl maintainer. No idea why it
failed to spot the extra space at line 465 in the patch.
James
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] qla4xxx: Updates for scsi misc branch
On Wed, 2012-10-10 at 11:08 -0500, Michael Christie wrote: On Oct 10, 2012, at 6:24 AM, vikas.chaudh...@qlogic.com wrote: From: Vikas Chaudhary vikas.chaudh...@qlogic.com James, Please apply the following patches to the scsi tree at your earliest convenience. Thanks, Vikas. I think some of these should be common operations instead of vendor ones. Adheer Chandravanshi (1): qla4xxx: Added new BSG command QLISCSI_VND_ABOUT_FIRMWARE What info does this provide? Harish Zunjarrao (3): qla4xxx: Allow reset in link down case qla4xxx: Invoke Set Address Control Block using BSG Does this set the networking info? qla4xxx: Invoke DisableACB using BSG What does this do? Disable the networking? Ping on this, please. Mike's points seem eminently sensible, what is the response? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] aacraid: SCSI dma mapping failure case handling
On Tue, 2012-10-16 at 23:34 +0200, Tomas Henzl wrote:
On 10/16/2012 10:59 PM, Mahesh Rajashekhara wrote:
This patch handles SCSI dma mapping failure case. Reporting error code to
the upper layer instead of BUG_ON().
This patch is created against current upstream kernel.
Signed-off-by: Mahesh Rajashekhara mahesh_rajashekh...@pmc-sierra.com
---
drivers/scsi/aacraid/aachba.c | 63
+++-
drivers/scsi/aacraid/aacraid.h |2 +-
2 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index d79457a..efa2900 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -971,6 +971,7 @@ static int aac_read_raw_io(struct fib * fib, struct
scsi_cmnd * cmd, u64 lba, u3
{
struct aac_dev *dev = fib-dev;
u16 fibsize, command;
+ unsigned long ret;
aac_fib_init(fib);
if (dev-comm_interface == AAC_COMM_MESSAGE_TYPE2 !dev-sync_mode) {
@@ -982,7 +983,10 @@ static int aac_read_raw_io(struct fib * fib, struct
scsi_cmnd * cmd, u64 lba, u3
readcmd2-byteCount = cpu_to_le32(count9);
readcmd2-cid = cpu_to_le16(scmd_id(cmd));
readcmd2-flags = cpu_to_le16(RIO2_IO_TYPE_READ);
- aac_build_sgraw2(cmd, readcmd2,
dev-scsi_host_ptr-sg_tablesize);
+ ret = aac_build_sgraw2(cmd, readcmd2,
+ dev-scsi_host_ptr-sg_tablesize);
+ if (ret 0)
+ return ret;
Hi Mahesh, 'ret' is 'unsigned', the above test will not work.
Tomas
Ping on this please Mahesh? The criticism seems completely valid to me.
James
N�r��yb�X��ǧv�^�){.n�+{{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj��!�i
Re: [PATCH v2 01/19] be2iscsi: Fix the issue with soft reset.
On Sat, 2012-10-20 at 04:41 +0530, John Soni Jose wrote: From: Minh Tran minhduc.t...@emulex.com Fixed soft_reset problem which driver modified all 32bit before a write on second pass. Signed-off-by: Minh Tran minhduc.t...@emulex.com Signed-off-by: Jayamohan Kallickal jayamohan.kallic...@emulex.com I'll put this in, this time, but please remember that if you send me a patch, it needs your signoff as well. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] arcmsr: Re-name the HBA Type
On Fri, 2012-10-12 at 17:05 +0800, NickCheng wrote: -#define ARCMSR_HBCMU_OUTBOUND_DOORBELL_ISR_MASK0x0004 /* When clear, the General Outbound Doorbell interrupt routes to the host.*/ The patch is still linebroken here. It is a massively long line (like about 200 characters). I suspect you just set your exchange server line break to something big, but not bigger than this? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
