Re: [BUG?] GDTH driver not working after upgrade to 2.6.24
On Wed, Jan 30 2008 at 21:47 +0200, Sven Köhler <[EMAIL PROTECTED]> wrote: > Hi, > > so i have upgraded a system to kernel 2.6.24. After that, it failed to > boot with the usual message telling, that the rootfs on device /dev/sda1 > cannot be mounted (a raid1 run by the controller below). > > With 2.6.23.12, everything is working fine. > > # lspci -v: > > 03:01.0 RAID bus controller: Intel Corporation RAID Controller > Subsystem: Intel Corporation Unknown device 01db > Flags: bus master, 66MHz, slow devsel, latency 64, IRQ 17 > Memory at ddffc000 (32-bit, prefetchable) [size=16K] > [virtual] Expansion ROM at deef [disabled] [size=32K] > Capabilities: [80] Power Management version 2 > > # GDT-related dmesg output (2.6.23.12): > > GDT-HA: Storage RAID Controller Driver. Version: 3.05 > ACPI: PCI Interrupt :03:01.0[A] -> GSI 24 (level, low) -> IRQ 17 > GDT-HA: Found 1 PCI Storage RAID Controllers > Configuring GDT-PCI HA at 3/1 IRQ 17 > GDT-HA 0: Name: SRCU42L > scsi0 : SRCU42L > scsi 0:0:0:0: Direct-Access IntelHost Drive #00 PQ: 0 ANSI: 2 > scsi 0:2:6:0: Processor ESG-SHV SCA HSBP M29 1.06 PQ: 0 ANSI: 2 > sd 0:0:0:0: [sda] 143299800 512-byte hardware sectors (73369 MB) > sd 0:0:0:0: [sda] Assuming Write Enabled > sd 0:0:0:0: [sda] Assuming drive cache: write through > sd 0:0:0:0: [sda] 143299800 512-byte hardware sectors (73369 MB) > sd 0:0:0:0: [sda] Assuming Write Enabled > sd 0:0:0:0: [sda] Assuming drive cache: write through > sda: sda1 sda2 < sda5 > > sd 0:0:0:0: [sda] Attached SCSI disk > > # cat /boot/config-2.6.24 |grep GDT > > CONFIG_SCSI_GDTH=y > > > > > Any ideas? > > http://www.kernel.org/diff/diffview.cgi?file=%2Fpub%2Flinux%2Fkernel%2Fv2.6%2Fpatch-2.6.24.bz2 > > show huge drivers/scsi/gdth* related changes. > > Can't test at the moment. System went production. > > > Regards, >Sven > Hi Sven! CCing to the scsi mailing list. Yes the gdth driver passed an open hart surgery in kernel 2.6.24. The bad thing about it is that all three of the Coders that did that did not have any HW to work on. One of them is me. We did cry for tester for a long time but no one came forward. Could you test patches for us? first thing would be to enable debug output patch below. If you absolutely need a 2.6.24 kernel, + gdth in a production system you could checkout the 2.6.23 driver and compile. The old driver will work the same in 2.6.24. It will not however even compile in 2.6.25-rcx. If any one wants to send me a card that uses the gdth driver, I will be very happy to debug this card, and return it once I'm done. Boaz --- git-diff --stat -p drivers/scsi/gdth.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index c825239..eca72c4 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -188,6 +188,7 @@ static int __gdth_queuecommand(gdth_ha_str *ha, struct scsi_cmnd *scp, struct gdth_cmndinfo *cmndinfo); static void gdth_scsi_done(struct scsi_cmnd *scp); +#define DEBUG_GDTH 1 #ifdef DEBUG_GDTH static unchar DebugState = DEBUG_GDTH; - 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: sd incorrectly reports write cache disabled on cache-capable drives/controllers
Hi, On Friday 25 January 2008 22:33:21 James Bottomley wrote: > I don't think it would be correct to assume that there actually is a > problem. Most RAID controllers habitually lie about having a cache in > their INQUIRY strings because they don't want to deal with the kernel > sending SYNCHRONIZE_CACHE commands down. I just tested this with two various Linux-distributions to verify that that the problem is in the inquiry and is not performance related. Unfortunately it turns out there's a rather huge performance difference. When testing this with CentOS, I not only got the "correct" report from the kernel that write cache enabled (and no warnings about things being disabled), and also saw a three times increase in write performance when testing with dd. The tests, which are far from scientific, I used the following command: Read test: dd if=/dev/sda of=/dev/zero bs=64k count=1 Write test: dd if=/dev/zero of/tmp/outfile bs=64k count=1 CentOS results: dd READ test 1st run: 64.9MB/s dd READ test consecutive runs: 2.2 GB/s dd WRITE test 1st run: 304 MB/s dd WRITE test consecute runs: 329 MB/s Other distributions, identical kernel config & initrd setup: dd READ test 1st run: 65,2MB/s dd READ test consecutive runs: 3.1 GB/s dd WRITE test 1st run: 129 MB/s dd WRITE test consecute runs: 98 MB/s So to conclude, what I would like to figure out is what LSI Logic / RedHat is doing to get such a dramatic increase in Write performance and if that is in any way associated with their system reporting that the write cache is actually enabled. Furthermore, I'm highly puzzled to see that the non-RedHat kernels are actually faster than RedHat on cached read access. I've tried this over long periods with various loads and uptimes on the systems, and the test results are quite persistent. Note that on both setups, the mptfusion drivers are loaded as modules from an initrd. No special parameters are passed to these drivers in either setup. I have mirrored the kernel .config between the two, and have also gone through the patches to the 2.6.18 kernel in RedHat that are relevant to mptfusion without finding anything that matches. I have also tried with the very latest drivers from LSI Logic, just recently released, on both systems, and the performance data remain the same. I'm sorry to open this issue again as I hoped and believed that it was resolved as per the description above, but since it turns out to be a very real performance difference associated with it, it might be of interest to others as well to get to the bottom of it? The full dmesg, kernel config and other information is available at http://www.hiawata.com/linux/ (system is still not in production and is therefor available for wild experimentation) Thanks again for your patient and insightful responses - 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: [BUG?] GDTH driver not working after upgrade to 2.6.24
Yes the gdth driver passed an open hart surgery in kernel 2.6.24. The bad thing about it is that all three of the Coders that did that did not have any HW to work on. One of them is me. We did cry for tester for a long time but no one came forward. All i'd like to ask is WHY!? WHY such a big open heart surgery? OK, you have your reasons. Could you test patches for us? first thing would be to enable debug output patch below. The machine is still production. It will be replaced by a 64bit system with an AACRAID card some time in the future. Then, i could maybe test patches on the old machine. But unfortunatly, it's not my machine. It's just administrated by me. And i don't know, what's exactly is the future of it. If you absolutely need a 2.6.24 kernel, + gdth in a production system you could checkout the 2.6.23 driver and compile. The old driver will work the same in 2.6.24. It will not however even compile in 2.6.25-rcx. I see. Thanks for the hint. Would be an alternative. Actually, i don't need 2.6.24 - but if something security related is fixed, then i'm not able to move on to 2.6.24. And i'm using Gentoo. There's not really a kernel maintained by the distribution. If any one wants to send me a card that uses the gdth driver, I will be very happy to debug this card, and return it once I'm done. I don't think, it's a seperate card, that i could send you. It's some 19" rack server with such a card on-board, i think. So it don't see much opportunity for me to test patched and stuff :-( signature.asc Description: OpenPGP digital signature
Re: [BUG?] GDTH driver not working after upgrade to 2.6.24
On Thu, Jan 31 2008 at 13:07 +0200, Sven Köhler <[EMAIL PROTECTED]> wrote: >> Yes the gdth driver passed an open hart surgery in kernel 2.6.24. The bad >> thing >> about it is that all three of the Coders that did that did not have any HW >> to work >> on. One of them is me. We did cry for tester for a long time but no one came >> forward. > > All i'd like to ask is WHY!? > WHY such a big open heart surgery? > OK, you have your reasons. > >> Could you test patches for us? first thing would be to enable debug output >> patch below. > > The machine is still production. It will be replaced by a 64bit system > with an AACRAID card some time in the future. Then, i could maybe test > patches on the old machine. But unfortunatly, it's not my machine. It's > just administrated by me. And i don't know, what's exactly is the future > of it. > >> If you absolutely need a 2.6.24 kernel, + gdth in a production system you >> could >> checkout the 2.6.23 driver and compile. The old driver will work the same in >> 2.6.24. >> It will not however even compile in 2.6.25-rcx. > > I see. Thanks for the hint. Would be an alternative. Actually, i don't > need 2.6.24 - but if something security related is fixed, then i'm not > able to move on to 2.6.24. And i'm using Gentoo. There's not really a > kernel maintained by the distribution. > >> If any one wants to send me a card that uses the gdth driver, I will be very >> happy >> to debug this card, and return it once I'm done. > > I don't think, it's a seperate card, that i could send you. It's some > 19" rack server with such a card on-board, i think. > > > So it don't see much opportunity for me to test patched and stuff :-( > Thanks, Perhaps someone else then. Anyone with gdth HW that can test patches? Your lspci said: "Intel Corporation RAID Controller" Matthew is there a gdth card lying around in an Intel lab near you? James do we need to mark gdth BROKEN for 2.6.24 and higher? Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Tue, 29 Jan 2008, Aegis Lin wrote:
> Quite minor patch, but from the context it should be desired
> that 64KB is available for ATAPI transferrring.
> (Historically) in SCSI/block layer sector size is defined as
> 512 during sector-byte calculation. Originally in ps3rom.c
> CD_FRAMESIZE was used, which makes
> /sys/block/sr0/queue/max_sectors_kb
> to be limited to 16KB.
Can I please have a Signed-off-by?
> diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
> index 17b4a7c..cc7062b 100644
> --- a/drivers/scsi/ps3rom.c
> +++ b/drivers/scsi/ps3rom.c
> @@ -35,7 +35,7 @@
>
> #define BOUNCE_SIZE (64*1024)
>
> -#define PS3ROM_MAX_SECTORS (BOUNCE_SIZE / CD_FRAMESIZE)
> +#define PS3ROM_MAX_SECTORS (BOUNCE_SIZE / 512)
>
>
> struct ps3rom_private {
Indeed, scsi_host_template.max_sectors seems to be just passed to the block
layer, so it concerns 512-byte sectors.
Furthermore, drivers/cdrom/cdrom.c:cdrom_read_cdda_bpc() seems to handle the
conversion from raw CD frame sizes to 512-byte sectors, and limit the requested
number of sectors if needed.
However, I didn't find where it handles conversion from CD data frame sizes to
512-byte sectors.
Am I missing something?
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone:+32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [EMAIL PROTECTED]
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
Re: Integration of SCST in the mainstream Linux kernel
Greetings all, On Wed, 2008-01-30 at 19:56 +0900, FUJITA Tomonori wrote: > On Wed, 30 Jan 2008 09:38:04 +0100 > "Bart Van Assche" <[EMAIL PROTECTED]> wrote: > > > On Jan 30, 2008 12:32 AM, FUJITA Tomonori <[EMAIL PROTECTED]> wrote: > > > > > > iSER has parameters to limit the maximum size of RDMA (it needs to > > > repeat RDMA with a poor configuration)? > > > > Please specify which parameters you are referring to. As you know I > > Sorry, I can't say. I don't know much about iSER. But seems that Pete > and Robin can get the better I/O performance - line speed ratio with > STGT. > > The version of OpenIB might matters too. For example, Pete said that > STGT reads loses about 100 MB/s for some transfer sizes for some > transfer sizes due to the OpenIB version difference or other unclear > reasons. > > http://article.gmane.org/gmane.linux.iscsi.tgt.devel/135 > > It's fair to say that it takes long time and need lots of knowledge to > get the maximum performance of SAN, I think. > > I think that it would be easier to convince James with the detailed > analysis (e.g. where does it take so long, like Pete did), not just > 'dd' performance results. > > Pushing iSCSI target code into mainline failed four times: IET, SCST, > STGT (doing I/Os in kernel in the past), and PyX's one (*1). iSCSI > target code is huge. You said SCST comprises 14,000 lines, but it's > not iSCSI target code. The SCSI engine code comprises 14,000 > lines. You need another 10,000 lines for the iSCSI driver. Note that > SCST's iSCSI driver provides only basic iSCSI features. PyX's iSCSI > target code implemenents more iSCSI features (like MC/S, ERL2, etc) > and comprises about 60,000 lines and it still lacks some features like > iSER, bidi, etc. > The PyX storage engine supports a scatterlist linked list algorithm that maps any sector count + sector size combination down to contiguous struct scatterlist arrays across (potentially) multiple Linux storage subsystems from a single CDB received on a initiator port. This design was a consequence of a requirement for running said engine on Linux v2.2 and v2.4 across non cache coherent systems (MIPS R5900-EE) using a single contiguous memory block mapped into struct buffer_head for PATA access, and struct scsi_cmnd access on USB storage. Note that this was before struct bio and struct scsi_request existed.. The PyX storage engine as it exists at Linux-iSCSI.org today can be thought of as a hybrid OSD processing engine, as it maps storage object memory across a number of tasks from a received command CDB. The ability to pass in pre allocated memory from an RDMA capable adapter, as well as allocated internally (ie: traditional iSCSI without open_iscsi's struct skbuff rx zero-copy) is inherient in the design of the storage engine. The lacking Bidi support can be attributed to lack of greater support (and hence user interest) in Bidi, but I am really glad to see this getting into the SCSI ML and STGT, and is certainly of interest in the long term. Another feature that is missing in the current engine is > 16 Byte CDBs, which I would imagine alot of vendor folks would like to see in Linux as well. This is pretty easy to add in iSCSI with an AHS and in the engine and storage subsystems. > I think that it's reasonable to say that we need more than 'dd' > results before pushing about possible more than 60,000 lines to > mainline. > > (*1) http://linux-iscsi.org/ > - The 60k lines of code also includes functionality (the SE mirroring comes to mind) that I do not plan to push towards mainline, along with other legacy bits so we can build on earlier v2.6 embedded platforms. The existing Target mode LIO-SE that provides linked list scatterlist mapping algorithm that is similar to what Jens and Rusty have been working on, and is under 14k lines including the switch(cdb[0]) + function pointer assignment to per CDB specific structure that is called potentially out-of-order in the RX side context of the CmdSN state machine in RFC-3720. The current SE is also lacking the very SCSI specific task management state machines that not a whole lot of iSCSI implementions implement properly, and seem to be minimal interest to users, and of moderate interest to vendors. Getting this implemented generically in SCSI, as opposed to an transport specific mechanisim would benefit the Linux SCSI target engine. The pSCSI (struct scsi_cmnd), iBlock (struct bio) and FILE (struct file) plugins together are a grand total of 3.5k lines using the v2.9 LIO-SE interface. Assuming we have a single preferred data and control patch for underlying physical and virtual block devices, this could also get smaller. A quick check of the code puts the traditional kernel level iSCSI statemachine at roughly 16k, which is pretty good for the complete state machine. Also, having iSER and traditional iSCSI share MC/S and ERL=2 common code will be of interest, as well as iSCSI login state machines, which are identical minus the
[Fwd: Re: Integration of SCST in the mainstream Linux kernel]
My original email probably got bounced.. --nab --- Begin Message --- Greetings all, On Wed, 2008-01-30 at 19:56 +0900, FUJITA Tomonori wrote: > On Wed, 30 Jan 2008 09:38:04 +0100 > "Bart Van Assche" <[EMAIL PROTECTED]> wrote: > > > On Jan 30, 2008 12:32 AM, FUJITA Tomonori <[EMAIL PROTECTED]> wrote: > > > > > > iSER has parameters to limit the maximum size of RDMA (it needs to > > > repeat RDMA with a poor configuration)? > > > > Please specify which parameters you are referring to. As you know I > > Sorry, I can't say. I don't know much about iSER. But seems that Pete > and Robin can get the better I/O performance - line speed ratio with > STGT. > > The version of OpenIB might matters too. For example, Pete said that > STGT reads loses about 100 MB/s for some transfer sizes for some > transfer sizes due to the OpenIB version difference or other unclear > reasons. > > http://article.gmane.org/gmane.linux.iscsi.tgt.devel/135 > > It's fair to say that it takes long time and need lots of knowledge to > get the maximum performance of SAN, I think. > > I think that it would be easier to convince James with the detailed > analysis (e.g. where does it take so long, like Pete did), not just > 'dd' performance results. > > Pushing iSCSI target code into mainline failed four times: IET, SCST, > STGT (doing I/Os in kernel in the past), and PyX's one (*1). iSCSI > target code is huge. You said SCST comprises 14,000 lines, but it's > not iSCSI target code. The SCSI engine code comprises 14,000 > lines. You need another 10,000 lines for the iSCSI driver. Note that > SCST's iSCSI driver provides only basic iSCSI features. PyX's iSCSI > target code implemenents more iSCSI features (like MC/S, ERL2, etc) > and comprises about 60,000 lines and it still lacks some features like > iSER, bidi, etc. > The PyX storage engine supports a scatterlist linked list algorithm that maps any sector count + sector size combination down to contiguous struct scatterlist arrays across (potentially) multiple Linux storage subsystems from a single CDB received on a initiator port. This design was a consequence of a requirement for running said engine on Linux v2.2 and v2.4 across non cache coherent systems (MIPS R5900-EE) using a single contiguous memory block mapped into struct buffer_head for PATA access, and struct scsi_cmnd access on USB storage. Note that this was before struct bio and struct scsi_request existed.. The PyX storage engine as it exists at Linux-iSCSI.org today can be thought of as a hybrid OSD processing engine, as it maps storage object memory across a number of tasks from a received command CDB. The ability to pass in pre allocated memory from an RDMA capable adapter, as well as allocated internally (ie: traditional iSCSI without open_iscsi's struct skbuff rx zero-copy) is inherient in the design of the storage engine. The lacking Bidi support can be attributed to lack of greater support (and hence user interest) in Bidi, but I am really glad to see this getting into the SCSI ML and STGT, and is certainly of interest in the long term. Another feature that is missing in the current engine is > 16 Byte CDBs, which I would imagine alot of vendor folks would like to see in Linux as well. This is pretty easy to add in iSCSI with an AHS and in the engine and storage subsystems. > I think that it's reasonable to say that we need more than 'dd' > results before pushing about possible more than 60,000 lines to > mainline. > > (*1) http://linux-iscsi.org/ > - The 60k lines of code also includes functionality (the SE mirroring comes to mind) that I do not plan to push towards mainline, along with other legacy bits so we can build on earlier v2.6 embedded platforms. The existing Target mode LIO-SE that provides linked list scatterlist mapping algorithm that is similar to what Jens and Rusty have been working on, and is under 14k lines including the switch(cdb[0]) + function pointer assignment to per CDB specific structure that is called potentially out-of-order in the RX side context of the CmdSN state machine in RFC-3720. The current SE is also lacking the very SCSI specific task management state machines that not a whole lot of iSCSI implementions implement properly, and seem to be minimal interest to users, and of moderate interest to vendors. Getting this implemented generically in SCSI, as opposed to an transport specific mechanisim would benefit the Linux SCSI target engine. The pSCSI (struct scsi_cmnd), iBlock (struct bio) and FILE (struct file) plugins together are a grand total of 3.5k lines using the v2.9 LIO-SE interface. Assuming we have a single preferred data and control patch for underlying physical and virtual block devices, this could also get smaller. A quick check of the code puts the traditional kernel level iSCSI statemachine at roughly 16k, which is pretty good for the complete state machine. Also, having iSER and traditional iSCSI share MC/S and ERL=2 common code will be of interest,
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
Greetings Geert and Co,
I have a related patch that I have been using with ps3rom.c for some
time that fixes a bug in fs/bio.c that assumes 512 byte sectors for
ATAPI operations. This bug actually exists for all non 512 byte sector
devices go through this code path (I found it with
scsi_execute_async()), but I first ran into this issue with ps3rom.c
because max_sectors (32) is small enough to trigger the bug assuming 512
byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. Because
typical max_sector settings for libata and USB are much higher, I have
never ran into this issue outside of ps3rom.c, but the bug exists
nevertheless..
The current patch assumes 512 byte sectors, and adds a sector_size
parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it for
passed struct request. I know that some folks talked about killing
scsi_execute_async() and fixing this problem elsewhere, but until then
please consider this patch. Any input is also appreciated.
:-)
--nab
Signed-off-by: Nicholas A. Bellinger <[EMAIL PROTECTED]>
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a417a6f..caa399c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -297,7 +297,8 @@ static int scsi_bi_endio(struct bio *bio, unsigned
int bytes_done, int error)
* sent to use, as some ULDs use that struct to only organize the
pages.
*/
static int scsi_req_map_sg(struct request *rq, struct scatterlist *sgl,
- int nsegs, unsigned bufflen, gfp_t gfp)
+ int nsegs, unsigned bufflen, gfp_t gfp,
+ unsigned int sector_size)
{
struct request_queue *q = rq->q;
int nr_pages = (bufflen + sgl[0].offset + PAGE_SIZE - 1) >>
PAGE_SHIFT;
@@ -325,6 +326,8 @@ static int scsi_req_map_sg(struct request *rq,
struct scatterlist *sgl,
goto free_bios;
}
bio->bi_end_io = scsi_bi_endio;
+ if (sector_size != bio->bi_sector_size)
+ bio->bi_sector_size =
sector_size;
}
if (bio_add_pc_page(q, bio, page, bytes, off) !=
@@ -399,7 +402,8 @@ int scsi_execute_async(struct scsi_device *sdev,
const unsigned char *cmd,
req->cmd_flags |= REQ_QUIET;
if (use_sg)
- err = scsi_req_map_sg(req, buffer, use_sg, bufflen,
gfp);
+ err = scsi_req_map_sg(req, buffer, use_sg, bufflen, gfp,
+ sdev->sector_size);
else if (bufflen)
err = blk_rq_map_kern(req->q, req, buffer, bufflen,
gfp);
diff --git a/fs/bio.c b/fs/bio.c
index 29a44c1..00e0490 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -188,8 +188,10 @@ struct bio *bio_alloc(gfp_t gfp_mask, int
nr_iovecs)
{
struct bio *bio = bio_alloc_bioset(gfp_mask, nr_iovecs,
fs_bio_set);
- if (bio)
+ if (bio) {
+ bio->bi_sector_size = 512;
bio->bi_destructor = bio_fs_destructor;
+ }
return bio;
}
@@ -328,7 +330,7 @@ static int __bio_add_page(struct request_queue *q,
struct bio *bio, struct page
if (unlikely(bio_flagged(bio, BIO_CLONED)))
return 0;
- if (((bio->bi_size + len) >> 9) > max_sectors)
+ if (((bio->bi_size + len) / bio->bi_sector_size) > max_sectors)
return 0;
/*
@@ -352,8 +354,9 @@ static int __bio_add_page(struct request_queue *q,
struct bio *bio, struct page
}
}
- if (bio->bi_vcnt >= bio->bi_max_vecs)
+ if (bio->bi_vcnt >= bio->bi_max_vecs) {
return 0;
+ }
/*
* we might lose a segment or two here, but rather that than
@@ -1026,7 +1029,7 @@ void bio_endio(struct bio *bio, unsigned int
bytes_done, int error)
}
bio->bi_size -= bytes_done;
- bio->bi_sector += (bytes_done >> 9);
+ bio->bi_sector += (bytes_done / bio->bi_sector_size);
if (bio->bi_end_io)
bio->bi_end_io(bio, bytes_done, error);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 1ddef34..e9ed1d1 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -105,6 +105,7 @@ struct bio {
unsigned intbi_hw_back_size;
unsigned intbi_max_vecs;/* max bvl_vecs we can
hold */
+ unsigned intbi_sector_size; /* sector size */
struct bio_vec *bi_io_vec; /* the actual vec list
*/
On Thu, 2008-01-31 at 14:28 +0100, Geert Uytterhoeven wrote:
> On Tue, 29 Jan 2008, Aegis Lin wrote:
> > Quite minor patch, but from the context it should be desired
> > that 64KB is available for ATAPI transferrring.
> > (Historically) in SCSI/block layer sector size is defined as
> > 512 during sector-byte calculation. Originally in ps3rom.c
> > CD_FRAMESIZE was used, which makes
> > /sys
Re: Integration of SCST in the mainstream Linux kernel
On Jan 31, 2008 2:25 PM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote: > Since this particular code is located in a non-data path critical > section, the kernel vs. user discussion is a wash. If we are talking > about data path, yes, the relevance of DD tests in kernel designs are > suspect :p. For those IB testers who are interested, perhaps having a > look with disktest from the Linux Test Project would give a better > comparision between the two implementations on a RDMA capable fabric > like IB for best case performance. I think everyone is interested in > seeing just how much data path overhead exists between userspace and > kernel space in typical and heavy workloads, if if this overhead can be > minimized to make userspace a better option for some of this very > complex code. I can run disktest on the same setups I ran dd on. This will take some time however. Disktest is new to me -- any hints with regard to suitable combinations of command line parameters are welcome. The most recent version I could find on http://ltp.sourceforge.net/ is ltp-20071231. Bart Van Assche. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Integration of SCST in the mainstream Linux kernel
Hi Bart, On Thu, 2008-01-31 at 15:34 +0100, Bart Van Assche wrote: > On Jan 31, 2008 2:25 PM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote: > > Since this particular code is located in a non-data path critical > > section, the kernel vs. user discussion is a wash. If we are talking > > about data path, yes, the relevance of DD tests in kernel designs are > > suspect :p. For those IB testers who are interested, perhaps having a > > look with disktest from the Linux Test Project would give a better > > comparision between the two implementations on a RDMA capable fabric > > like IB for best case performance. I think everyone is interested in > > seeing just how much data path overhead exists between userspace and > > kernel space in typical and heavy workloads, if if this overhead can be > > minimized to make userspace a better option for some of this very > > complex code. > > I can run disktest on the same setups I ran dd on. This will take some > time however. > > Disktest is new to me -- any hints with regard to suitable > combinations of command line parameters are welcome. The most recent > version I could find on http://ltp.sourceforge.net/ is ltp-20071231. > I posted some numbers with traditional iSCSI on Neterion Xframe I 10 Gb/sec with LRO back in 2005 with disktest on the 1st generation x86_64 hardware available at the time. These tests where designed to show the performance advantages of internexus multiplexing that is available within traditional iSCSI, as well as iSER. The disktest parameters that I used are listed in the following thread: https://www.redhat.com/archives/dm-devel/2005-April/msg00013.html --nab > Bart Van Assche. > - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
On Thu, Jan 31 2008 at 17:08 +0200, Mark Glines <[EMAIL PROTECTED]> wrote: > On Thu, 31 Jan 2008 11:27:39 +0200 > Boaz Harrosh <[EMAIL PROTECTED]> wrote: > >> Please check the below patch. >> >> one thing that I can see is that the isd200 does an INQUARY transfer >> of sizeof(struct inquiry_data) which is 96 bytes, when scsi_scan.c >> sends an INQUARY with 36 bytes buffer. So we have an underflow in >> usb_stor_access_xfer_buf(). >> >> The below patch will only check my theory. I will send a proper fix >> later, please confirm that this fixes it. >> >> What kills me is that this condition has existed before my patch, I'll >> try to see why it is triggered now > > I applied this patch to 2.6.24, and it now works for me. It was > crashing consistently whenever I'd plug this device in, now it goes > through successfully: > Yes Thanks this is grate :) I will send a proper patch to scsi maintainer. Alan is it OK to send this patch threw James's scsi-misc? > > [24775.788039] usb 3-2: new full speed USB device using uhci_hcd and address 3 > [24775.939275] usb 3-2: configuration #1 chosen from 1 choice > [24776.084409] usbcore: registered new interface driver libusual > [24776.103604] Initializing USB Mass Storage driver... > [24776.213916] scsi3 : SCSI emulation for USB Mass Storage devices > [24776.214366] usbcore: registered new interface driver usb-storage > [24776.214377] USB Mass Storage support registered. > [24776.215604] usb-storage: device found at 3 > [24776.215724] usb-storage: waiting for device to settle before scanning > [24778.78] scsi 3:0:0:0: Direct-Access SAMSUNG HM120JC YL10 > PQ: 0 ANSI: 0 > [24778.333715] sd 3:0:0:0: [sdb] 234441648 512-byte hardware sectors (120034 > MB) > [24778.333841] sd 3:0:0:0: [sdb] Write Protect is off > [24778.333848] sd 3:0:0:0: [sdb] Mode Sense: 00 00 00 00 > [24778.333853] sd 3:0:0:0: [sdb] Assuming drive cache: write through > [24778.334196] sd 3:0:0:0: [sdb] 234441648 512-byte hardware sectors (120034 > MB) > [24778.334396] sd 3:0:0:0: [sdb] Write Protect is off > [24778.334403] sd 3:0:0:0: [sdb] Mode Sense: 00 00 00 00 > [24778.334408] sd 3:0:0:0: [sdb] Assuming drive cache: write through > [24778.334414] sdb: sdb1 > [24778.824103] sd 3:0:0:0: [sdb] Attached SCSI disk > [24778.824210] sd 3:0:0:0: Attached scsi generic sg1 type 0 > [24778.825119] usb-storage: device scan complete > > > I'm happy to test further patches. Let me know if you need more > testing. > > Do you still want me to try out the scsi-misc branch? > No, That was my mistake, scsi-misc is now identical to mainline. This here is a new fix that will need to go in. I will send a patch soonish. If you can test it and send a Tested-by: it could be grate > Mark > > >> --- >> drivers/usb/storage/protocol.c |6 ++ >> 1 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/usb/storage/protocol.c >> b/drivers/usb/storage/protocol.c index a41ce21..d0ff1f6 100644 >> --- a/drivers/usb/storage/protocol.c >> +++ b/drivers/usb/storage/protocol.c >> @@ -229,6 +229,12 @@ void usb_stor_set_xfer_buf(unsigned char *buffer, >> unsigned int offset = 0; >> struct scatterlist *sg = NULL; >> >> +BUG_ON(!scsi_sglist(srb)); >> + >> +if(buflen > scsi_bufflen(srb)) >> +buflen = scsi_bufflen(srb); >> +/*FIXME: should we set an underflow condition here*/ >> + >> usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset, >> TO_XFER_BUF); >> if (buflen < scsi_bufflen(srb)) >> Thanks Mark (CCing linux-scsi ml) Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] kill hotplug init/exit section annotations
No-one seems to see much value in these, and they cause about 90% of our
problems with __init/__exit markers, so simply eliminate them. Rather
than run over the whole tree removing them, this patch #defines them to
be nops.
Signed-off-by: James Bottomley <[EMAIL PROTECTED]>
---
I'll probably be going after __exit after this one, but it makes sense
to split them up, since the hotplug annotation removal looks
uncontroversial, whereas __exit and discard section removal might
produce more robust debate. I also think doing the hotplug removal
gives us 90% of the benefits and removes 90% of the section mismatch
problems.
James
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index f784d2f..5099021 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -9,46 +9,11 @@
/* Align . to a 8 byte boundary equals to maximum function alignment. */
#define ALIGN_FUNCTION() . = ALIGN(8)
-/* The actual configuration determine if the init/exit sections
- * are handled as text/data or they can be discarded (which
- * often happens at runtime)
- */
-#ifdef CONFIG_HOTPLUG
-#define DEV_KEEP(sec)*(.dev##sec)
-#define DEV_DISCARD(sec)
-#else
-#define DEV_KEEP(sec)
-#define DEV_DISCARD(sec) *(.dev##sec)
-#endif
-
-#ifdef CONFIG_HOTPLUG_CPU
-#define CPU_KEEP(sec)*(.cpu##sec)
-#define CPU_DISCARD(sec)
-#else
-#define CPU_KEEP(sec)
-#define CPU_DISCARD(sec) *(.cpu##sec)
-#endif
-
-#if defined(CONFIG_MEMORY_HOTPLUG)
-#define MEM_KEEP(sec)*(.mem##sec)
-#define MEM_DISCARD(sec)
-#else
-#define MEM_KEEP(sec)
-#define MEM_DISCARD(sec) *(.mem##sec)
-#endif
-
-
/* .data section */
#define DATA_DATA \
*(.data)\
*(.data.init.refok) \
*(.ref.data)\
- DEV_KEEP(init.data) \
- DEV_KEEP(exit.data) \
- CPU_KEEP(init.data) \
- CPU_KEEP(exit.data) \
- MEM_KEEP(init.data) \
- MEM_KEEP(exit.data) \
. = ALIGN(8); \
VMLINUX_SYMBOL(__start___markers) = .; \
*(__markers)\
@@ -171,12 +136,6 @@
/* __*init sections */ \
__init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \
*(.ref.rodata) \
- DEV_KEEP(init.rodata) \
- DEV_KEEP(exit.rodata) \
- CPU_KEEP(init.rodata) \
- CPU_KEEP(exit.rodata) \
- MEM_KEEP(init.rodata) \
- MEM_KEEP(exit.rodata) \
} \
\
/* Built-in module parameters. */ \
@@ -208,12 +167,6 @@
*(.ref.text)\
*(.text.init.refok) \
*(.exit.text.refok) \
- DEV_KEEP(init.text) \
- DEV_KEEP(exit.text) \
- CPU_KEEP(init.text) \
- CPU_KEEP(exit.text) \
- MEM_KEEP(init.text) \
- MEM_KEEP(exit.text)
/* sched.text is aling to function alignment to secure we have same
@@ -241,33 +194,15 @@
/* init and exit section handling */
#define INIT_DATA \
*(.init.data) \
- DEV_DISCARD(init.data) \
- DEV_DISCARD(init.rodata)\
- CPU_DISCARD(init.data) \
- CPU_DISCARD(init.rodata)\
- MEM_DISCARD(init.data) \
- MEM_DISCARD(init.rodata)
#define INIT_TEXT \
*(.init.text)
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 2008-01-31 at 06:10 -0800, Nicholas A. Bellinger wrote: > Greetings Geert and Co, > > I have a related patch that I have been using with ps3rom.c for some > time that fixes a bug in fs/bio.c that assumes 512 byte sectors for > ATAPI operations. This bug actually exists for all non 512 byte sector > devices go through this code path (I found it with > scsi_execute_async()), but I first ran into this issue with ps3rom.c > because max_sectors (32) is small enough to trigger the bug assuming 512 > byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. Because > typical max_sector settings for libata and USB are much higher, I have > never ran into this issue outside of ps3rom.c, but the bug exists > nevertheless.. > > The current patch assumes 512 byte sectors, and adds a sector_size > parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it for > passed struct request. I know that some folks talked about killing > scsi_execute_async() and fixing this problem elsewhere, but until then > please consider this patch. Any input is also appreciated. My first reaction is really, no; there's no way we should be doing such a nasty layering violation. The block code is set up to work with 512 byte sectors for its internal counts. Something like this will damage all non-512 byte sector devices (and we do have several of those). There are three quantities you need to understand when trying to do something like this 1. The actual internal block sector size for sector counts. This is hard coded to 512 2. The medium sector size. This is stored in hardsect_size in the queue. Block ensures that it always sends down enough 512 byte sectors to be a multiple of this 3. The block size. This represents the unit the user of the device wants to think in terms of (most often for filesystems, the fs block size). It's stored in various places including the block_device bd_block_size. It really sounds like the problem with ps3rom is that hardsect_size isn't being set correctly. As I understand the problem, from the incredibly cursory insight the emails give, so please correct if wrong, you want the hard sector size to be the CD_FRAMESIZE? 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: Integration of SCST in the mainstream Linux kernel
Bart Van Assche wrote: On Jan 31, 2008 2:25 PM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote: Since this particular code is located in a non-data path critical section, the kernel vs. user discussion is a wash. If we are talking about data path, yes, the relevance of DD tests in kernel designs are suspect :p. For those IB testers who are interested, perhaps having a look with disktest from the Linux Test Project would give a better comparision between the two implementations on a RDMA capable fabric like IB for best case performance. I think everyone is interested in seeing just how much data path overhead exists between userspace and kernel space in typical and heavy workloads, if if this overhead can be minimized to make userspace a better option for some of this very complex code. I can run disktest on the same setups I ran dd on. This will take some time however. Disktest was already referenced in the beginning of the performance comparison thread, but its results are not very interesting if we are going to find out, which implementation is more effective, because in the modes, in which usually people run this utility, it produces latency insensitive workload (multiple threads working in parallel). So, such multithreaded disktests results will be different between STGT and SCST only if STGT's implementation will get target CPU bound. If CPU on the target is powerful enough, even extra busy loops in the STGT or SCST hot path code will change nothing. Additionally, multithreaded disktest over RAM disk is a good example of a synthetic benchmark, which has almost no relation with real life workloads. But people like it, because it produces nice looking results. Actually, I don't know what kind of conclusions it is possible to make from disktest's results (maybe only how throughput gets bigger or slower with increasing number of threads?), it's a good stress test tool, but not more. Disktest is new to me -- any hints with regard to suitable combinations of command line parameters are welcome. The most recent version I could find on http://ltp.sourceforge.net/ is ltp-20071231. Bart Van Assche. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - 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: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 2008-01-31 at 09:34 -0600, James Bottomley wrote: > On Thu, 2008-01-31 at 06:10 -0800, Nicholas A. Bellinger wrote: > > Greetings Geert and Co, > > > > I have a related patch that I have been using with ps3rom.c for some > > time that fixes a bug in fs/bio.c that assumes 512 byte sectors for > > ATAPI operations. This bug actually exists for all non 512 byte sector > > devices go through this code path (I found it with > > scsi_execute_async()), but I first ran into this issue with ps3rom.c > > because max_sectors (32) is small enough to trigger the bug assuming 512 > > byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. Because > > typical max_sector settings for libata and USB are much higher, I have > > never ran into this issue outside of ps3rom.c, but the bug exists > > nevertheless.. > > > > The current patch assumes 512 byte sectors, and adds a sector_size > > parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it for > > passed struct request. I know that some folks talked about killing > > scsi_execute_async() and fixing this problem elsewhere, but until then > > please consider this patch. Any input is also appreciated. > > My first reaction is really, no; there's no way we should be doing such > a nasty layering violation. > I don't care for it either, but without this patch (or something similar) all SCSI targets that use scsi_execute_async(), for non 512 byte requests are broken. This causes a problem when a small max_sector is correctly used by the LLD, and trips the check in fs/bio.c:__bio_add_page() if (((bio->bi_size + len) >> 9) > max_sectors) > The block code is set up to work with 512 byte sectors for its internal > counts. Something like this will damage all non-512 byte sector devices > (and we do have several of those). The purpose of the patch was to make none 512 byte sector struct scsi_device (2048 byte sector size ATAPI packets received over IP storage fabric, and queued into ps3rom.c in this particular case) work with scsi_execute_async(). If there is another target mode SCSI interface that does not have this problem existing or planned, I have no problem moving to that one instead. > > There are three quantities you need to understand when trying to do > something like this > > 1. The actual internal block sector size for sector counts. This > is hard coded to 512 > 2. The medium sector size. This is stored in hardsect_size in the > queue. Block ensures that it always sends down enough 512 byte > sectors to be a multiple of this > 3. The block size. This represents the unit the user of the device > wants to think in terms of (most often for filesystems, the fs > block size). It's stored in various places including the > block_device bd_block_size. Yes, the assumption of the two hardcoded locations of 512 byte sectors in __bio_add_page() and bio_endio() is where I was orginally running into problems. I know you have mentioned that no in-tree drivers currently send non 512 byte sector requests into scsi_execute_async(), but the issue exists for all current target mode code when going through said SCSI target interface. So what I am hearing is that the conditionals in __bio_add_page() and bio_endio() that assume 512 byte sector size should be checking when the medium sector size does not match the hardcoded block sector size, byte and use the non 512 sector value for the incoming and completion paths..? > It really sounds like the problem with ps3rom is that hardsect_size > isn't being set correctly. As I understand the problem, from the > incredibly cursory insight the emails give, so please correct if wrong, > you want the hard sector size to be the CD_FRAMESIZE? Sorry, to confuse this. These are two seperate issues. --nab > > 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] kill hotplug init/exit section annotations
On Thu, 31 Jan 2008 09:57:31 -0600 James Bottomley <[EMAIL PROTECTED]> wrote: > No-one seems to see much value in these, and they cause about 90% of > our problems with __init/__exit markers, so simply eliminate them. > Rather than run over the whole tree removing them, this patch > #defines them to be nops. > > Signed-off-by: James Bottomley <[EMAIL PROTECTED]> > > --- > > I'll probably be going after __exit after this one, but it makes sense > to split them up, since the hotplug annotation removal looks > uncontroversial, whereas __exit and discard section removal might > produce more robust debate. I also think doing the hotplug removal > gives us 90% of the benefits and removes 90% of the section mismatch > problems. Since hotplug is so fundamental nowadays the value no longer outweighs the pain/cost to me, so Acked-by: Arjan van de Ven <[EMAIL PROTECTED]> -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kill hotplug init/exit section annotations
On Thu, Jan 31, 2008 at 08:11:20AM -0800, Arjan van de Ven wrote: > On Thu, 31 Jan 2008 09:57:31 -0600 > James Bottomley <[EMAIL PROTECTED]> wrote: > > > No-one seems to see much value in these, and they cause about 90% of > > our problems with __init/__exit markers, so simply eliminate them. > > Rather than run over the whole tree removing them, this patch > > #defines them to be nops. > > > > Signed-off-by: James Bottomley <[EMAIL PROTECTED]> > > > > --- > > > > I'll probably be going after __exit after this one, but it makes sense > > to split them up, since the hotplug annotation removal looks > > uncontroversial, whereas __exit and discard section removal might > > produce more robust debate. I also think doing the hotplug removal > > gives us 90% of the benefits and removes 90% of the section mismatch > > problems. > > > Since hotplug is so fundamental nowadays the value no longer outweighs the > pain/cost > to me, so Granted for normal hotplug. But my computer has neither CPU hotplug not memory hotplug, and I don't see the point for removing these annotations (and they are anyway not what causes problems in normal drivers). > Acked-by: Arjan van de Ven <[EMAIL PROTECTED]> cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
On Thu, 31 Jan 2008, Boaz Harrosh wrote: > >> Please check the below patch. > >> > >> one thing that I can see is that the isd200 does an INQUARY transfer > >> of sizeof(struct inquiry_data) which is 96 bytes, when scsi_scan.c > >> sends an INQUARY with 36 bytes buffer. So we have an underflow in > >> usb_stor_access_xfer_buf(). Maybe the isd200 routine should be changed also, so that it doesn't try to store too much data in the buffer. > I will send a proper patch to scsi maintainer. Alan is it OK to send this > patch threw James's scsi-misc? You should send patches to Matt Dharm, since he is the usb-storage maintainer. Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] GDTH driver not working after upgrade to 2.6.24
Hi, On Jan 31 2008 14:35, Boaz Harrosh wrote: > >Thanks, Perhaps someone else then. >Anyone with gdth HW that can test patches? Is bisecting down the existing chain and finding the bad commit sufficient? (I also take new patches.) >Your lspci said: "Intel Corporation RAID Controller" Matthew >is there a gdth card lying around in an Intel lab near you? > >James do we need to mark gdth BROKEN for 2.6.24 and higher? I say revert it for the time being. - 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: [BUG?] GDTH driver not working after upgrade to 2.6.24
On Thu, Jan 31 2008 at 18:39 +0200, Jan Engelhardt <[EMAIL PROTECTED]> wrote: > Hi, > > On Jan 31 2008 14:35, Boaz Harrosh wrote: >> Thanks, Perhaps someone else then. >> Anyone with gdth HW that can test patches? > > Is bisecting down the existing chain and finding the bad commit > sufficient? (I also take new patches.) Most certainly. If you are willing, please... I'm looking for someone responsive. first - enable debug prints (see the original post) and send me the prints. second - bisection could be grate yes. third - accepting patches and testing could be grate, thanks. > >> Your lspci said: "Intel Corporation RAID Controller" Matthew >> is there a gdth card lying around in an Intel lab near you? >> >> James do we need to mark gdth BROKEN for 2.6.24 and higher? > > I say revert it for the time being. It could be reverted for 2.6.24.x maintenance releases but for 2.6.25-xxx it cannot as it will not compile, and the fix to that is what you see in code. Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 2008-01-31 at 08:10 -0800, Nicholas A. Bellinger wrote: > On Thu, 2008-01-31 at 09:34 -0600, James Bottomley wrote: > > On Thu, 2008-01-31 at 06:10 -0800, Nicholas A. Bellinger wrote: > > > Greetings Geert and Co, > > > > > > I have a related patch that I have been using with ps3rom.c for some > > > time that fixes a bug in fs/bio.c that assumes 512 byte sectors for > > > ATAPI operations. This bug actually exists for all non 512 byte sector > > > devices go through this code path (I found it with > > > scsi_execute_async()), but I first ran into this issue with ps3rom.c > > > because max_sectors (32) is small enough to trigger the bug assuming 512 > > > byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. Because > > > typical max_sector settings for libata and USB are much higher, I have > > > never ran into this issue outside of ps3rom.c, but the bug exists > > > nevertheless.. > > > > > > The current patch assumes 512 byte sectors, and adds a sector_size > > > parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it for > > > passed struct request. I know that some folks talked about killing > > > scsi_execute_async() and fixing this problem elsewhere, but until then > > > please consider this patch. Any input is also appreciated. > > > > My first reaction is really, no; there's no way we should be doing such > > a nasty layering violation. > > > > I don't care for it either, but without this patch (or something > similar) all SCSI targets that use scsi_execute_async(), for non 512 > byte requests are broken. This causes a problem when a small max_sector > is correctly used by the LLD, and trips the check in > fs/bio.c:__bio_add_page() > > if (((bio->bi_size + len) >> 9) > max_sectors) > Could we rewind this discussion back to an actual problem description then, please? Nothing in the standard path for a CD/DVD should be using scsi_execute_async(), what's the actual problem use case? 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: [Scst-devel] Integration of SCST in the mainstream Linux kernel
Vladislav Bolkhovitin wrote: Bart Van Assche wrote: [...] I can run disktest on the same setups I ran dd on. This will take some time however. Disktest was already referenced in the beginning of the performance comparison thread, but its results are not very interesting if we are going to find out, which implementation is more effective, because in the modes, in which usually people run this utility, it produces latency insensitive workload (multiple threads working in parallel). So, such There are other issues with disktest, in that you can easily specify option combinations that generate apparently 5+ GB/s of IO, though actual traffic over the link to storage is very low. Caveat disktest emptor. multithreaded disktests results will be different between STGT and SCST only if STGT's implementation will get target CPU bound. If CPU on the target is powerful enough, even extra busy loops in the STGT or SCST hot path code will change nothing. Additionally, multithreaded disktest over RAM disk is a good example of a synthetic benchmark, which has almost no relation with real life workloads. But people like it, because it produces nice looking results. I agree. The backing store should be a disk for it to have meaning, though please note my caveat above. Actually, I don't know what kind of conclusions it is possible to make from disktest's results (maybe only how throughput gets bigger or slower with increasing number of threads?), it's a good stress test tool, but not more. Unfortunately, I agree. Bonnie++, dd tests, and a few others seem to bear far closer to "real world" tests than disktest and iozone, the latter of which does more to test the speed of RAM cache and system call performance than actual IO. Disktest is new to me -- any hints with regard to suitable combinations of command line parameters are welcome. The most recent version I could find on http://ltp.sourceforge.net/ is ltp-20071231. Bart Van Assche. Here is what I have run: disktest -K 8 -B 256k -I F -N 2000 -P A -w /big/file disktest -K 8 -B 64k -I F -N 2000 -P A -w /big/file disktest -K 8 -B 1k-I B -N 200 -P A /dev/sdb2 and many others. Joe -- Joseph Landman, Ph.D Founder and CEO Scalable Informatics LLC, email: [EMAIL PROTECTED] web : http://www.scalableinformatics.com http://jackrabbit.scalableinformatics.com phone: +1 734 786 8423 fax : +1 866 888 3112 cell : +1 734 612 4615 - 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: [Scst-devel] Integration of SCST in the mainstream Linux kernel
Bart Van Assche wrote: I have ran some tests with Bonnie++, but found out that on a fast network like IB the filesystem used for the test has a really big impact on the test results. This is true of the file systems when physically directly connected to the unit as well. Some file systems are designed with high performance in mind, some are not. If anyone has a suggestion for a better test than dd to compare the performance of SCSI storage protocols, please let it know. Hmmm... if you care about the protocol side, I can't help. Our users are more concerned with the file system side, so this is where we focus our tuning attention. Bart Van Assche. Joe -- Joseph Landman, Ph.D Founder and CEO Scalable Informatics LLC, email: [EMAIL PROTECTED] web : http://www.scalableinformatics.com http://jackrabbit.scalableinformatics.com phone: +1 734 786 8423 fax : +1 866 888 3112 cell : +1 734 612 4615 - 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] kill hotplug init/exit section annotations
On Thu, Jan 31, 2008 at 09:07:49AM -0800, Arjan van de Ven wrote: > On Thu, 31 Jan 2008 18:21:42 +0200 > Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > On Thu, Jan 31, 2008 at 08:11:20AM -0800, Arjan van de Ven wrote: > > > On Thu, 31 Jan 2008 09:57:31 -0600 > > > James Bottomley <[EMAIL PROTECTED]> wrote: > > > > > > > No-one seems to see much value in these, and they cause about 90% > > > > of our problems with __init/__exit markers, so simply eliminate > > > > them. Rather than run over the whole tree removing them, this > > > > patch #defines them to be nops. > > > > > > > > Signed-off-by: James Bottomley > > > > <[EMAIL PROTECTED]> > > > > > > > > --- > > > > > > > > I'll probably be going after __exit after this one, but it makes > > > > sense to split them up, since the hotplug annotation removal looks > > > > uncontroversial, whereas __exit and discard section removal might > > > > produce more robust debate. I also think doing the hotplug > > > > removal gives us 90% of the benefits and removes 90% of the > > > > section mismatch problems. > > > > > > > > > Since hotplug is so fundamental nowadays the value no longer > > > outweighs the pain/cost to me, so > > > > Granted for normal hotplug. > > > > But my computer has neither CPU hotplug > > cpuhotplug is required for suspend/resume. Not on UP computers. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Integration of SCST in the mainstream Linux kernel
On Thu, 2008-01-31 at 18:50 +0300, Vladislav Bolkhovitin wrote: > Bart Van Assche wrote: > > On Jan 31, 2008 2:25 PM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote: > > > >>Since this particular code is located in a non-data path critical > >>section, the kernel vs. user discussion is a wash. If we are talking > >>about data path, yes, the relevance of DD tests in kernel designs are > >>suspect :p. For those IB testers who are interested, perhaps having a > >>look with disktest from the Linux Test Project would give a better > >>comparision between the two implementations on a RDMA capable fabric > >>like IB for best case performance. I think everyone is interested in > >>seeing just how much data path overhead exists between userspace and > >>kernel space in typical and heavy workloads, if if this overhead can be > >>minimized to make userspace a better option for some of this very > >>complex code. > > > > I can run disktest on the same setups I ran dd on. This will take some > > time however. > > Disktest was already referenced in the beginning of the performance > comparison thread, but its results are not very interesting if we are > going to find out, which implementation is more effective, because in > the modes, in which usually people run this utility, it produces latency > insensitive workload (multiple threads working in parallel). So, such > multithreaded disktests results will be different between STGT and SCST > only if STGT's implementation will get target CPU bound. If CPU on the > target is powerful enough, even extra busy loops in the STGT or SCST hot > path code will change nothing. > I think the really interesting numbers are the difference for bulk I/O between kernel and userspace on both traditional iSCSI and the RDMA enabled flavours. I have not been able to determine anything earth shattering from the current run of kernel vs. userspace tests, nor which method of implementation for iSER, SRP, and generic Storage Engine are 'more effective' for that case. Performance and latency to real storage would make alot more sense for the kernel vs. user case. Also workloads against software LVM and Linux MD block devices would be of interest as these would be some of the more typical deployments that would be in the field, and is what Linux-iSCSI.org uses for our production cluster storage today. Having implemented my own iSCSI and SCSI Target mode Storage Engine leads me to believe that putting logic in userspace is probably a good idea in the longterm. If this means putting the entire data IO path into userspace for Linux/iSCSI, then there needs to be a good reason why this will not not scale to multi-port 10 Gb/sec engines in traditional and RDMA mode if we need to take this codepath back into the kernel. The end goal is to have the most polished and complete storage engine and iSCSI stacks designs go upstream, which is something I think we can all agree on. Also, with STGT being a pretty new design which has not undergone alot of optimization, perhaps profiling both pieces of code against similar tests would give us a better idea of where userspace bottlenecks reside. Also, the overhead involved with traditional iSCSI for bulk IO from kernel / userspace would also be a key concern for a much larger set of users, as iSER and SRP on IB is a pretty small userbase and will probably remain small for the near future. > Additionally, multithreaded disktest over RAM disk is a good example of > a synthetic benchmark, which has almost no relation with real life > workloads. But people like it, because it produces nice looking results. > Yes, people like to claim their stacks are the fastest with RAM disk benchmarks. But hooking up their fast network silicon to existing storage hardware and OS storage subsystems and software is where the real game is.. > Actually, I don't know what kind of conclusions it is possible to make > from disktest's results (maybe only how throughput gets bigger or slower > with increasing number of threads?), it's a good stress test tool, > but > not more. > Being able to have a best case baseline with disktest for kernel vs. user would be of interest for both transport protocol and SCSI Target mode Storage Engine profiling. The first run of tests looked pretty bandwith oriented, so disktest works well to determine maximum bandwith. Disktest also is nice for getting reads from cache on hardware RAID controllers because disktest only generates requests with LBAs from 0 -> disktest BLOCKSIZE. --nab > > Disktest is new to me -- any hints with regard to suitable > > combinations of command line parameters are welcome. The most recent > > version I could find on http://ltp.sourceforge.net/ is ltp-20071231. > > > > Bart Van Assche. > > - > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > > the body of a message to [EMAIL PROTECTED] > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > >
[PATCH] bugfix for an underflow condition in usb storage & isd200.c
scsi_scan is issuing a 36-byte INQUIRY request to llds. isd200 would
volunteer 96 bytes of INQUIRY. This caused an underflow condition in
protocol.c usb_stor_access_xfer_buf(). So first fix is to
usb_stor_access_xfer_buf() to properly handle underflow conditions.
Then usb_stor_set_xfer_buf() should report this condition as a negative
resid. Should we also set cmnd->status in the underflow condition?
Then also isd200.c is fixed to only return the type of INQUIRY && SENSE
the upper layer asked for.
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
drivers/usb/storage/isd200.c |7 +--
drivers/usb/storage/protocol.c | 13 +
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 0db4886..4394930 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -1261,6 +1261,7 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
struct us_data *us,
unsigned long lba;
unsigned long blockCount;
unsigned char senseData[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
+ unsigned xfer_len;
memset(ataCdb, 0, sizeof(union ata_cdb));
@@ -1270,8 +1271,9 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
struct us_data *us,
US_DEBUGP(" ATA OUT - INQUIRY\n");
/* copy InquiryData */
+ xfer_len = min(sizeof(info->InquiryData), scsi_bufflen(srb));
usb_stor_set_xfer_buf((unsigned char *) &info->InquiryData,
- sizeof(info->InquiryData), srb);
+ xfer_len, srb);
srb->result = SAM_STAT_GOOD;
sendToTransport = 0;
break;
@@ -1280,7 +1282,8 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
struct us_data *us,
US_DEBUGP(" ATA OUT - SCSIOP_MODE_SENSE\n");
/* Initialize the return buffer */
- usb_stor_set_xfer_buf(senseData, sizeof(senseData), srb);
+ xfer_len = min(sizeof(senseData), scsi_bufflen(srb));
+ usb_stor_set_xfer_buf(senseData, xfer_len, srb);
if (info->DeviceFlags & DF_MEDIA_STATUS_ENABLED)
{
diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c
index a41ce21..6200f62 100644
--- a/drivers/usb/storage/protocol.c
+++ b/drivers/usb/storage/protocol.c
@@ -175,7 +175,7 @@ unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,
* and the starting offset within the page, and update
* the *offset and **sgptr values for the next loop. */
cnt = 0;
- while (cnt < buflen) {
+ while (cnt < buflen && sg) {
struct page *page = sg_page(sg) +
((sg->offset + *offset) >> PAGE_SHIFT);
unsigned int poff =
@@ -228,9 +228,14 @@ void usb_stor_set_xfer_buf(unsigned char *buffer,
{
unsigned int offset = 0;
struct scatterlist *sg = NULL;
+ unsigned int count;
- usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
+ count = usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
TO_XFER_BUF);
- if (buflen < scsi_bufflen(srb))
- scsi_set_resid(srb, scsi_bufflen(srb) - buflen);
+
+ /* Check for underflow */
+ if (buflen > scsi_bufflen(srb))
+ count = buflen;
+
+ scsi_set_resid(srb, scsi_bufflen(srb) - count);
}
--
1.5.3.3
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
On Thu, Jan 31 2008 at 18:45 +0200, Alan Stern <[EMAIL PROTECTED]> wrote: > On Thu, 31 Jan 2008, Boaz Harrosh wrote: > Please check the below patch. one thing that I can see is that the isd200 does an INQUARY transfer of sizeof(struct inquiry_data) which is 96 bytes, when scsi_scan.c sends an INQUARY with 36 bytes buffer. So we have an underflow in usb_stor_access_xfer_buf(). > > Maybe the isd200 routine should be changed also, so that it doesn't try > to store too much data in the buffer. > >> I will send a proper patch to scsi maintainer. Alan is it OK to send this >> patch threw James's scsi-misc? > > You should send patches to Matt Dharm, since he is the usb-storage > maintainer. > > Alan Stern > > - Right, Please see patch posted as reply to the original email. I have also fixed isd200 to return what was asked. The fix to protocol.c is also different and more general now. Will send to Matthew Dharm. Matthew - is it OK to send this threw James, please ACK. Mark - this fix is different we do need testing. Thanks to all Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 2008-01-31 at 10:26 -0600, James Bottomley wrote: > On Thu, 2008-01-31 at 08:10 -0800, Nicholas A. Bellinger wrote: > > On Thu, 2008-01-31 at 09:34 -0600, James Bottomley wrote: > > > On Thu, 2008-01-31 at 06:10 -0800, Nicholas A. Bellinger wrote: > > > > Greetings Geert and Co, > > > > > > > > I have a related patch that I have been using with ps3rom.c for some > > > > time that fixes a bug in fs/bio.c that assumes 512 byte sectors for > > > > ATAPI operations. This bug actually exists for all non 512 byte sector > > > > devices go through this code path (I found it with > > > > scsi_execute_async()), but I first ran into this issue with ps3rom.c > > > > because max_sectors (32) is small enough to trigger the bug assuming 512 > > > > byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. Because > > > > typical max_sector settings for libata and USB are much higher, I have > > > > never ran into this issue outside of ps3rom.c, but the bug exists > > > > nevertheless.. > > > > > > > > The current patch assumes 512 byte sectors, and adds a sector_size > > > > parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it for > > > > passed struct request. I know that some folks talked about killing > > > > scsi_execute_async() and fixing this problem elsewhere, but until then > > > > please consider this patch. Any input is also appreciated. > > > > > > My first reaction is really, no; there's no way we should be doing such > > > a nasty layering violation. > > > > > > > I don't care for it either, but without this patch (or something > > similar) all SCSI targets that use scsi_execute_async(), for non 512 > > byte requests are broken. This causes a problem when a small max_sector > > is correctly used by the LLD, and trips the check in > > fs/bio.c:__bio_add_page() > > > > if (((bio->bi_size + len) >> 9) > max_sectors) > > > > Could we rewind this discussion back to an actual problem description > then, please? Nothing in the standard path for a CD/DVD should be using > scsi_execute_async(), what's the actual problem use case? > The problem case is a SCSI Target Mode engine that receives a 2048 Byte single sector ATAPI READ_10 request from the storage fabric, and uses scsi_execute_async() (the only option >= 2.6.18) to issue said request to the underlying struct scsi_device. Because the underlying bio code assumes 512 byte only sectors, the check in __bio_add_page() incorrectly determines that max_sectors (max_sectors has to be low, as with 32 from ps3rom.c) has been exceeded, and fails the request back up the stack. --nab > 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] kill hotplug init/exit section annotations
On Thu, 31 Jan 2008 18:21:42 +0200 Adrian Bunk <[EMAIL PROTECTED]> wrote: > On Thu, Jan 31, 2008 at 08:11:20AM -0800, Arjan van de Ven wrote: > > On Thu, 31 Jan 2008 09:57:31 -0600 > > James Bottomley <[EMAIL PROTECTED]> wrote: > > > > > No-one seems to see much value in these, and they cause about 90% > > > of our problems with __init/__exit markers, so simply eliminate > > > them. Rather than run over the whole tree removing them, this > > > patch #defines them to be nops. > > > > > > Signed-off-by: James Bottomley > > > <[EMAIL PROTECTED]> > > > > > > --- > > > > > > I'll probably be going after __exit after this one, but it makes > > > sense to split them up, since the hotplug annotation removal looks > > > uncontroversial, whereas __exit and discard section removal might > > > produce more robust debate. I also think doing the hotplug > > > removal gives us 90% of the benefits and removes 90% of the > > > section mismatch problems. > > > > > > Since hotplug is so fundamental nowadays the value no longer > > outweighs the pain/cost to me, so > > Granted for normal hotplug. > > But my computer has neither CPU hotplug cpuhotplug is required for suspend/resume. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Scst-devel] Integration of SCST in the mainstream Linux kernel
On Jan 31, 2008 5:25 PM, Joe Landman <[EMAIL PROTECTED]> wrote: > Vladislav Bolkhovitin wrote: > > Actually, I don't know what kind of conclusions it is possible to make > > from disktest's results (maybe only how throughput gets bigger or slower > > with increasing number of threads?), it's a good stress test tool, but > > not more. > > Unfortunately, I agree. Bonnie++, dd tests, and a few others seem to > bear far closer to "real world" tests than disktest and iozone, the > latter of which does more to test the speed of RAM cache and system call > performance than actual IO. I have ran some tests with Bonnie++, but found out that on a fast network like IB the filesystem used for the test has a really big impact on the test results. If anyone has a suggestion for a better test than dd to compare the performance of SCSI storage protocols, please let it know. Bart Van Assche. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [build bug] drivers/scsi/NCR53C9x.c:913: error: 'Scsi_Cmnd' has no member named 'use_sg'
On Thu, Jan 31 2008 at 19:29 +0200, Ingo Molnar <[EMAIL PROTECTED]> wrote: > FYI, automated testing found the following build breakage: > > drivers/scsi/NCR53C9x.c: In function 'esp_get_dmabufs': > drivers/scsi/NCR53C9x.c:913: error: 'Scsi_Cmnd' has no member named 'use_sg' > drivers/scsi/NCR53C9x.c:914: error: 'Scsi_Cmnd' has no member named > 'request_bufflen' > > config attached. > > Ingo > Cc linux-scsi mailing list. This driver and others are scheduled to be removed in the scsi-pending tree and are awaiting ACKs from - disappeared - maintainers. Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Integration of SCST in the mainstream Linux kernel
On Jan 31, 2008 6:14 PM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote: > Also, with STGT being a pretty new design which has not undergone alot > of optimization, perhaps profiling both pieces of code against similar > tests would give us a better idea of where userspace bottlenecks reside. > Also, the overhead involved with traditional iSCSI for bulk IO from > kernel / userspace would also be a key concern for a much larger set of > users, as iSER and SRP on IB is a pretty small userbase and will > probably remain small for the near future. Two important trends in data center technology are server consolidation and storage consolidation. A.o. every web hosting company can profit from a fast storage solution. I wouldn't call this a small user base. Regarding iSER and SRP on IB: InfiniBand is today the most economic solution for a fast storage network. I do not know which technology will be the most popular for storage consolidation within a few years -- this can be SRP, iSER, IPoIB + SDP, FCoE (Fibre Channel over Ethernet) or maybe yet another technology. No matter which technology becomes the most popular for storage applications, there will be a need for high-performance storage software. References: * Michael Feldman, Battle of the Network Fabrics, HPCwire, December 2006, http://www.hpcwire.com/hpc/1145060.html * NetApp, Reducing Data Center Power Consumption Through Efficient Storage, February 2007, http://www.netapp.com/ftp/wp-reducing-datacenter-power-consumption.pdf Bart Van Assche. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 31 Jan 2008, Nicholas A. Bellinger wrote: > On Thu, 2008-01-31 at 10:26 -0600, James Bottomley wrote: > > On Thu, 2008-01-31 at 08:10 -0800, Nicholas A. Bellinger wrote: > > > On Thu, 2008-01-31 at 09:34 -0600, James Bottomley wrote: > > > > On Thu, 2008-01-31 at 06:10 -0800, Nicholas A. Bellinger wrote: > > > > > Greetings Geert and Co, > > > > > > > > > > I have a related patch that I have been using with ps3rom.c for some > > > > > time that fixes a bug in fs/bio.c that assumes 512 byte sectors for > > > > > ATAPI operations. This bug actually exists for all non 512 byte > > > > > sector > > > > > devices go through this code path (I found it with > > > > > scsi_execute_async()), but I first ran into this issue with ps3rom.c > > > > > because max_sectors (32) is small enough to trigger the bug assuming > > > > > 512 > > > > > byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. Because > > > > > typical max_sector settings for libata and USB are much higher, I have > > > > > never ran into this issue outside of ps3rom.c, but the bug exists > > > > > nevertheless.. > > > > > > > > > > The current patch assumes 512 byte sectors, and adds a sector_size > > > > > parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it > > > > > for > > > > > passed struct request. I know that some folks talked about killing > > > > > scsi_execute_async() and fixing this problem elsewhere, but until then > > > > > please consider this patch. Any input is also appreciated. > > > > > > > > My first reaction is really, no; there's no way we should be doing such > > > > a nasty layering violation. > > > > > > > > > > I don't care for it either, but without this patch (or something > > > similar) all SCSI targets that use scsi_execute_async(), for non 512 > > > byte requests are broken. This causes a problem when a small max_sector > > > is correctly used by the LLD, and trips the check in > > > fs/bio.c:__bio_add_page() > > > > > > if (((bio->bi_size + len) >> 9) > max_sectors) > > > > > > > Could we rewind this discussion back to an actual problem description > > then, please? Nothing in the standard path for a CD/DVD should be using > > scsi_execute_async(), what's the actual problem use case? > > > > The problem case is a SCSI Target Mode engine that receives a 2048 Byte > single sector ATAPI READ_10 request from the storage fabric, and uses > scsi_execute_async() (the only option >= 2.6.18) to issue said request > to the underlying struct scsi_device. Because the underlying bio code > assumes 512 byte only sectors, the check in __bio_add_page() incorrectly > determines that max_sectors (max_sectors has to be low, as with 32 from > ps3rom.c) has been exceeded, and fails the request back up the stack. Thanks for unfocussing James ;-) James, do you have any comment about the first email in this thread, increasing scsi_host_template.max_sectors? With kind regards, Geert Uytterhoeven Software Architect Sony Network and Software Technology Center Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium Phone:+32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: [EMAIL PROTECTED] Internet: http://www.sony-europe.com/ Sony Network and Software Technology Center Europe A division of Sony Service Centre (Europe) N.V. Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium VAT BE 0413.825.160 · RPR Brussels Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
Re: [PATCH] kill hotplug init/exit section annotations
On Thu, Jan 31, 2008 at 07:14:36PM +0200, Adrian Bunk wrote: > > cpuhotplug is required for suspend/resume. > > Not on UP computers. those are less and less common now, most modern laptops are dual core - 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] kill hotplug init/exit section annotations
On Thu, 31 Jan 2008 19:14:36 +0200 Adrian Bunk <[EMAIL PROTECTED]> wrote: > > cpuhotplug is required for suspend/resume. > > Not on UP computers. > great! someone who still has one of those and uses a kernel without it. Can you look at your system.map and see how many kilobytes you've gained? Eg how many kilobytes are in these sections exactly? (but also subtract any cost due to page aligning this stuff ;-) > cu > Adrian > -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Thu, 31 Jan 2008, Boaz Harrosh wrote:
> @@ -228,9 +228,14 @@ void usb_stor_set_xfer_buf(unsigned char *buffer,
> {
> unsigned int offset = 0;
> struct scatterlist *sg = NULL;
> + unsigned int count;
>
> - usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
> + count = usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
> TO_XFER_BUF);
> - if (buflen < scsi_bufflen(srb))
> - scsi_set_resid(srb, scsi_bufflen(srb) - buflen);
> +
> + /* Check for underflow */
> + if (buflen > scsi_bufflen(srb))
> + count = buflen;
> +
> + scsi_set_resid(srb, scsi_bufflen(srb) - count);
> }
This last "if" statement doesn't look right. And since you know that
count will never be larger than scsi_bufflen(srb), you don't have to
check for underflow at all. Just leave out the "if" and call
scsi_set_resid() directly.
Alan Stern
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 2008-01-31 at 09:28 -0800, Nicholas A. Bellinger wrote: > On Thu, 2008-01-31 at 10:26 -0600, James Bottomley wrote: > > On Thu, 2008-01-31 at 08:10 -0800, Nicholas A. Bellinger wrote: > > > On Thu, 2008-01-31 at 09:34 -0600, James Bottomley wrote: > > > > On Thu, 2008-01-31 at 06:10 -0800, Nicholas A. Bellinger wrote: > > > > > Greetings Geert and Co, > > > > > > > > > > I have a related patch that I have been using with ps3rom.c for some > > > > > time that fixes a bug in fs/bio.c that assumes 512 byte sectors for > > > > > ATAPI operations. This bug actually exists for all non 512 byte > > > > > sector > > > > > devices go through this code path (I found it with > > > > > scsi_execute_async()), but I first ran into this issue with ps3rom.c > > > > > because max_sectors (32) is small enough to trigger the bug assuming > > > > > 512 > > > > > byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. Because > > > > > typical max_sector settings for libata and USB are much higher, I have > > > > > never ran into this issue outside of ps3rom.c, but the bug exists > > > > > nevertheless.. > > > > > > > > > > The current patch assumes 512 byte sectors, and adds a sector_size > > > > > parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it > > > > > for > > > > > passed struct request. I know that some folks talked about killing > > > > > scsi_execute_async() and fixing this problem elsewhere, but until then > > > > > please consider this patch. Any input is also appreciated. > > > > > > > > My first reaction is really, no; there's no way we should be doing such > > > > a nasty layering violation. > > > > > > > > > > I don't care for it either, but without this patch (or something > > > similar) all SCSI targets that use scsi_execute_async(), for non 512 > > > byte requests are broken. This causes a problem when a small max_sector > > > is correctly used by the LLD, and trips the check in > > > fs/bio.c:__bio_add_page() > > > > > > if (((bio->bi_size + len) >> 9) > max_sectors) > > > > > > > Could we rewind this discussion back to an actual problem description > > then, please? Nothing in the standard path for a CD/DVD should be using > > scsi_execute_async(), what's the actual problem use case? > > > > The problem case is a SCSI Target Mode engine that receives a 2048 Byte > single sector ATAPI READ_10 request from the storage fabric, and uses > scsi_execute_async() (the only option >= 2.6.18) to issue said request > to the underlying struct scsi_device. Because the underlying bio code > assumes 512 byte only sectors, the check in __bio_add_page() incorrectly > determines that max_sectors (max_sectors has to be low, as with 32 from > ps3rom.c) has been exceeded, and fails the request back up the stack. OK, so this is a totally separate issue from the one you actually posted it as a patch to fix? the queue max_sectors parameter is also counted in the block internal of 512 byte sectors. If you set it to 32 that means you were only expecting 16k of transfers per command maximum. If that's not right, then set the limit correctly. In short, and to repeat: almost every internal size counter to block is in units of 512 byte sectors ... that includes capacity, maximum etc ... 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] kill hotplug init/exit section annotations
On Thu, Jan 31, 2008 at 09:45:26AM -0800, Chris Wedgwood wrote: > On Thu, Jan 31, 2008 at 07:14:36PM +0200, Adrian Bunk wrote: > > > > cpuhotplug is required for suspend/resume. > > > > Not on UP computers. > > those are less and less common now, most modern laptops are dual core Who was talking about laptops? cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Thu, Jan 31, 2008 at 07:19:57PM +0200, Boaz Harrosh wrote: > > scsi_scan is issuing a 36-byte INQUIRY request to llds. isd200 would > volunteer 96 bytes of INQUIRY. This caused an underflow condition in > protocol.c usb_stor_access_xfer_buf(). So first fix is to > usb_stor_access_xfer_buf() to properly handle underflow conditions. > Then usb_stor_set_xfer_buf() should report this condition as a negative > resid. Should we also set cmnd->status in the underflow condition? > > Then also isd200.c is fixed to only return the type of INQUIRY && SENSE > the upper layer asked for. > > Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]> As this is a regression and hits 2.6.24, can you send the final version of this patch to the [EMAIL PROTECTED] address so we can get it into the 2.6.24.y tree? thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Integration of SCST in the mainstream Linux kernel
On Thu, 2008-01-31 at 18:40 +0100, Bart Van Assche wrote: > On Jan 31, 2008 6:14 PM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote: > > Also, with STGT being a pretty new design which has not undergone alot > > of optimization, perhaps profiling both pieces of code against similar > > tests would give us a better idea of where userspace bottlenecks reside. > > Also, the overhead involved with traditional iSCSI for bulk IO from > > kernel / userspace would also be a key concern for a much larger set of > > users, as iSER and SRP on IB is a pretty small userbase and will > > probably remain small for the near future. > > Two important trends in data center technology are server > consolidation and storage consolidation. A.o. every web hosting > company can profit from a fast storage solution. I wouldn't call this > a small user base. > > Regarding iSER and SRP on IB: InfiniBand is today the most economic > solution for a fast storage network. I do not know which technology > will be the most popular for storage consolidation within a few years > -- this can be SRP, iSER, IPoIB + SDP, FCoE (Fibre Channel over > Ethernet) or maybe yet another technology. No matter which technology > becomes the most popular for storage applications, there will be a > need for high-performance storage software. > I meant small referring to storage on IB fabrics which has usually been in the research and national lab settings, with some other vendors offering IB as an alternative storage fabric for those who [w,c]ould not wait for 10 Gb/sec copper Ethernet and Direct Data Placement to come online. These types of numbers compared to say traditional iSCSI, that is getting used all over the place these days in areas I won't bother listing here. As for the future, I am obviously cheering for IP storage fabrics, in particular 10 Gb/sec Ethernet and Direct Data Placement in concert with iSCSI Extentions for RDMA to give the data center a high performance, low latency transport that can do OS independent storage multiplexing and recovery across multiple independently developed implementions. Also avoiding lock-in from un-interoptable storage transports (espically on the high end) that had plauged so many vendors in years past has become an real option in the past few years with a IETF defined block level storage protocol. We are actually going on four years since RFC-3720 was ratified. (April 2004) Making the 'enterprise' ethernet switching equipment go from millisecond to nanosecond latency in a whole different story that goes beyond my area of expertise. I know there is one startup (Fulcrum Micro) who is working on this problem and seems to be making some good progress. --nab > References: > * Michael Feldman, Battle of the Network Fabrics, HPCwire, December > 2006, http://www.hpcwire.com/hpc/1145060.html > * NetApp, Reducing Data Center Power Consumption Through Efficient > Storage, February 2007, > http://www.netapp.com/ftp/wp-reducing-datacenter-power-consumption.pdf > > Bart Van Assche. > - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/24] arm: scsi convert to accessors and !use_sg cleanup
On Wed, 2007-09-12 at 08:42 +0100, Russell King 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.
>
> > 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.
Time is up on this one: this driver now won't build in mainline because
of the promised sg_table updates. Either you ack the changes or provide
your own. If not, I'll mark the driver BROKEN.
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: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 2008-01-31 at 18:41 +0100, Geert Uytterhoeven wrote: > On Thu, 31 Jan 2008, Nicholas A. Bellinger wrote: > > On Thu, 2008-01-31 at 10:26 -0600, James Bottomley wrote: > > > On Thu, 2008-01-31 at 08:10 -0800, Nicholas A. Bellinger wrote: > > > > On Thu, 2008-01-31 at 09:34 -0600, James Bottomley wrote: > > > > > On Thu, 2008-01-31 at 06:10 -0800, Nicholas A. Bellinger wrote: > > > > > > Greetings Geert and Co, > > > > > > > > > > > > I have a related patch that I have been using with ps3rom.c for some > > > > > > time that fixes a bug in fs/bio.c that assumes 512 byte sectors for > > > > > > ATAPI operations. This bug actually exists for all non 512 byte > > > > > > sector > > > > > > devices go through this code path (I found it with > > > > > > scsi_execute_async()), but I first ran into this issue with ps3rom.c > > > > > > because max_sectors (32) is small enough to trigger the bug > > > > > > assuming 512 > > > > > > byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. > > > > > > Because > > > > > > typical max_sector settings for libata and USB are much higher, I > > > > > > have > > > > > > never ran into this issue outside of ps3rom.c, but the bug exists > > > > > > nevertheless.. > > > > > > > > > > > > The current patch assumes 512 byte sectors, and adds a sector_size > > > > > > parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it > > > > > > for > > > > > > passed struct request. I know that some folks talked about killing > > > > > > scsi_execute_async() and fixing this problem elsewhere, but until > > > > > > then > > > > > > please consider this patch. Any input is also appreciated. > > > > > > > > > > My first reaction is really, no; there's no way we should be doing > > > > > such > > > > > a nasty layering violation. > > > > > > > > > > > > > I don't care for it either, but without this patch (or something > > > > similar) all SCSI targets that use scsi_execute_async(), for non 512 > > > > byte requests are broken. This causes a problem when a small max_sector > > > > is correctly used by the LLD, and trips the check in > > > > fs/bio.c:__bio_add_page() > > > > > > > > if (((bio->bi_size + len) >> 9) > max_sectors) > > > > > > > > > > Could we rewind this discussion back to an actual problem description > > > then, please? Nothing in the standard path for a CD/DVD should be using > > > scsi_execute_async(), what's the actual problem use case? > > > > > > > The problem case is a SCSI Target Mode engine that receives a 2048 Byte > > single sector ATAPI READ_10 request from the storage fabric, and uses > > scsi_execute_async() (the only option >= 2.6.18) to issue said request > > to the underlying struct scsi_device. Because the underlying bio code > > assumes 512 byte only sectors, the check in __bio_add_page() incorrectly > > determines that max_sectors (max_sectors has to be low, as with 32 from > > ps3rom.c) has been exceeded, and fails the request back up the stack. > > Thanks for unfocussing James ;-) > > James, do you have any comment about the first email in this thread, > increasing > scsi_host_template.max_sectors? Assuming your BOUNCE_SIZE is in bytes, and represents your largest transfer, then yes, setting .max_sectors to BOUNCE_SIZE>>9 is exactly the right way of phrasing this, since .max_sectors is counted in units of the internal block sector (or 512 bytes). Were you going to submit a full patch to linux-scsi? 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 Mon, 2008-01-07 at 07:07 +0100, Kars de Jong wrote: > On do, 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... > > I'll have a look at this. I can only test it on Blizzard 1260 hardware > though. OK, time's up. These drivers are now unbuildable in mainline because of the promised sg_table updates. They either get removed, fixed or marked as BROKEN. Which is it to be? 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 0/3] iscsi bidi & varlen support
Cheers after 1.3 years these can go in. [PATCH 1/3] iscsi: extended cdb support The varlen support is not yet in mainline for block and scsi-ml. But the API for drivers will not change. All LLD need to do is max_command to the it's maximum and be ready for bigger commands. This is what's done here. Once these commands start coming iscsi will be ready for them. [PATCH 2/3] iscsi: bidi support - libiscsi [PATCH 3/3] iscsi: bidi support - iscsi_tcp bidirectional commands support in iscsi. iSER is not yet ready, but it will not break. There is already a mechanism in libiscsi that will return error if bidi commands are sent iSER way. Pete please send me the iSER bits so we can port them to this latest version. Mike these patches are ontop of iscs branch of the iscsi git tree, they will apply but for compilation you will need to sync with Linus mainline. The patches are for the in-tree iscsi code. I own you the compat patch for the out-off-tree code, but this I will only be Sunday. If we do it fast it might get accepted to 2.6.25 merge window Everybody is invited to a party at Shila ben-yhuda 52 Tel-Aviv 9:45 pm. Drinks and wonderful see-food on us :) Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 31 Jan 2008, James Bottomley wrote: > On Thu, 2008-01-31 at 18:41 +0100, Geert Uytterhoeven wrote: > > James, do you have any comment about the first email in this thread, > > increasing > > scsi_host_template.max_sectors? > > Assuming your BOUNCE_SIZE is in bytes, and represents your largest > transfer, then yes, setting .max_sectors to BOUNCE_SIZE>>9 is exactly > the right way of phrasing this, since .max_sectors is counted in units > of the internal block sector (or 512 bytes). OK, thanks for the confirmation! > Were you going to submit a full patch to linux-scsi? Yes I will. With kind regards, Geert Uytterhoeven Software Architect Sony Network and Software Technology Center Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium Phone:+32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: [EMAIL PROTECTED] Internet: http://www.sony-europe.com/ Sony Network and Software Technology Center Europe A division of Sony Service Centre (Europe) N.V. Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium VAT BE 0413.825.160 · RPR Brussels Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Thu, Jan 31 2008 at 20:00 +0200, Greg KH <[EMAIL PROTECTED]> wrote: > On Thu, Jan 31, 2008 at 07:19:57PM +0200, Boaz Harrosh wrote: >> scsi_scan is issuing a 36-byte INQUIRY request to llds. isd200 would >> volunteer 96 bytes of INQUIRY. This caused an underflow condition in >> protocol.c usb_stor_access_xfer_buf(). So first fix is to >> usb_stor_access_xfer_buf() to properly handle underflow conditions. >> Then usb_stor_set_xfer_buf() should report this condition as a negative >> resid. Should we also set cmnd->status in the underflow condition? >> >> Then also isd200.c is fixed to only return the type of INQUIRY && SENSE >> the upper layer asked for. >> >> Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]> > > As this is a regression and hits 2.6.24, can you send the final version > of this patch to the [EMAIL PROTECTED] address so we can get it into the > 2.6.24.y tree? > > thanks, > > greg k-h > - On Thu, Jan 31 2008 at 20:00 +0200, Mark Glines <[EMAIL PROTECTED]> wrote: >> On Thu, 31 Jan 2008 19:20:26 +0200 >> Hi, >> >> This patch fails to apply to 2.6.24. What should I apply it to, for >> testing? USB Git, or SCSI git or something? >> >> patching file drivers/usb/storage/isd200.c >> Hunk #1 succeeded at 1238 (offset -23 lines). >> Hunk #2 succeeded at 1248 (offset -23 lines). >> Hunk #3 succeeded at 1259 (offset -23 lines). >> patching file drivers/usb/storage/protocol.c >> Hunk #1 FAILED at 175. >> Hunk #2 FAILED at 228. >> 2 out of 2 hunks FAILED -- saving rejects to file >> drivers/usb/storage/protocol.c.rej >> >> Thanks, >> >> Mark > OK, I was confused with what versions this belongs to. This also answers my original question of why it happens with my patch, as it did not it happened before my patch. My patches only went into the 2.6.25 merge window. So I will need to send 2 Patches. One for [EMAIL PROTECTED] on top of 2.6.24. The second is for mainline, which is the one I sent. Should go threw scsi-misc which it is based on. patch for 2.6.24 [EMAIL PROTECTED] on the way Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 2008-01-31 at 11:53 -0600, James Bottomley wrote: > On Thu, 2008-01-31 at 09:28 -0800, Nicholas A. Bellinger wrote: > > On Thu, 2008-01-31 at 10:26 -0600, James Bottomley wrote: > > > On Thu, 2008-01-31 at 08:10 -0800, Nicholas A. Bellinger wrote: > > > > On Thu, 2008-01-31 at 09:34 -0600, James Bottomley wrote: > > > > > On Thu, 2008-01-31 at 06:10 -0800, Nicholas A. Bellinger wrote: > > > > > > Greetings Geert and Co, > > > > > > > > > > > > I have a related patch that I have been using with ps3rom.c for some > > > > > > time that fixes a bug in fs/bio.c that assumes 512 byte sectors for > > > > > > ATAPI operations. This bug actually exists for all non 512 byte > > > > > > sector > > > > > > devices go through this code path (I found it with > > > > > > scsi_execute_async()), but I first ran into this issue with ps3rom.c > > > > > > because max_sectors (32) is small enough to trigger the bug > > > > > > assuming 512 > > > > > > byte sectors during typical ATAPI READ_10 ops with iSCSI/HD. > > > > > > Because > > > > > > typical max_sector settings for libata and USB are much higher, I > > > > > > have > > > > > > never ran into this issue outside of ps3rom.c, but the bug exists > > > > > > nevertheless.. > > > > > > > > > > > > The current patch assumes 512 byte sectors, and adds a sector_size > > > > > > parameter to drivers/scsi/scsi_lib.c:scsi_req_map_sg() to change it > > > > > > for > > > > > > passed struct request. I know that some folks talked about killing > > > > > > scsi_execute_async() and fixing this problem elsewhere, but until > > > > > > then > > > > > > please consider this patch. Any input is also appreciated. > > > > > > > > > > My first reaction is really, no; there's no way we should be doing > > > > > such > > > > > a nasty layering violation. > > > > > > > > > > > > > I don't care for it either, but without this patch (or something > > > > similar) all SCSI targets that use scsi_execute_async(), for non 512 > > > > byte requests are broken. This causes a problem when a small max_sector > > > > is correctly used by the LLD, and trips the check in > > > > fs/bio.c:__bio_add_page() > > > > > > > > if (((bio->bi_size + len) >> 9) > max_sectors) > > > > > > > > > > Could we rewind this discussion back to an actual problem description > > > then, please? Nothing in the standard path for a CD/DVD should be using > > > scsi_execute_async(), what's the actual problem use case? > > > > > > > The problem case is a SCSI Target Mode engine that receives a 2048 Byte > > single sector ATAPI READ_10 request from the storage fabric, and uses > > scsi_execute_async() (the only option >= 2.6.18) to issue said request > > to the underlying struct scsi_device. Because the underlying bio code > > assumes 512 byte only sectors, the check in __bio_add_page() incorrectly > > determines that max_sectors (max_sectors has to be low, as with 32 from > > ps3rom.c) has been exceeded, and fails the request back up the stack. > > OK, so this is a totally separate issue from the one you actually posted > it as a patch to fix? > > the queue max_sectors parameter is also counted in the block internal of > 512 byte sectors. If you set it to 32 that means you were only > expecting 16k of transfers per command maximum. If that's not right, > then set the limit correctly. > > In short, and to repeat: almost every internal size counter to block is > in units of 512 byte sectors ... that includes capacity, maximum etc ... > Ok, after reading your followup with Geert I see that this looks like a bug in ps3rom.c assuming 2048 byte sectors to calculate .max_sectors (which was originally set to 32 as I mentioned). Using the setting BOUNCE_SIZE << 9 where BOUNCE_SIZE is the request size in bytes looks like this will solve the issue. My misunderstanding was that .max_sectors was allowed to be calcuated in non 512 byte sectors, so please disregard my patch. Geert, .max_sectors for ps3rom.c using 512 byte sectors ends up being 128, yes.? James, could we put something in the SCSI docs stating that .max_sectors MUST be calculated against 512 byte sectors..? Thanks again, --nab > 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] kill hotplug init/exit section annotations
On Thu, Jan 31, 2008 at 07:55:43PM +0200, Adrian Bunk wrote: > Who was talking about laptops? If laptops are mostly MP these days, then 'desktops' and 'servers' certainly are --- so pretty much everyone needs CPU hotplug. - 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] kill hotplug init/exit section annotations
On Thu, Jan 31, 2008 at 09:48:01AM -0800, Arjan van de Ven wrote: > On Thu, 31 Jan 2008 19:14:36 +0200 > Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > cpuhotplug is required for suspend/resume. > > > > Not on UP computers. > > > > great! someone who still has one of those and uses a kernel without it. > Can you look at your system.map and see how many kilobytes you've gained? > Eg how many kilobytes are in these sections exactly? I have one. A Atmel AT91 board equipped with an 9263. So lets take a look at the defconfig build for the evaluation board. o-arm/vmlinux.o: file format elf32-littlearm 0 .text 001cdefc 0400 2**10 2 .init.text000165e8 001ce6c0 2**5 26 .init.data32ec 002578cc 2**2 --- 4 .devinit.text 1558 001e5270 2**2 9 .exit.text0bc8 001e8d2c 2**2 10 .cpuinit.text 0924 001e98f4 2**2 11 .meminit.text 04cc 001ea218 2**2 12 .devexit.text 0160 001ea6e4 2**2 38 .cpuinit.data 0040 0025afc0 2**2 39 .meminit.data 000c 0025b000 2**2 __devinit alone gives a net win of 5464 bytes. That is only ~3% of total .text size but this is non-swapable memory where everything is worth it. And the configuration selected is by no means optimal with respect to minimal size. Sam - 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] kill hotplug init/exit section annotations
On Thu, 31 Jan 2008 19:34:25 +0100 Sam Ravnborg <[EMAIL PROTECTED]> wrote: > On Thu, Jan 31, 2008 at 09:48:01AM -0800, Arjan van de Ven wrote: > > On Thu, 31 Jan 2008 19:14:36 +0200 > > Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > > cpuhotplug is required for suspend/resume. > > > > > > Not on UP computers. > > > > > > > great! someone who still has one of those and uses a kernel without > > it. Can you look at your system.map and see how many kilobytes > > you've gained? Eg how many kilobytes are in these sections exactly? > I have one. A Atmel AT91 board equipped with an 9263. > So lets take a look at the defconfig build for the evaluation board. > > > o-arm/vmlinux.o: file format elf32-littlearm > > 0 .text 001cdefc 0400 2**10 > 2 .init.text000165e8 001ce6c0 2**5 > 26 .init.data32ec 002578cc 2**2 > > --- > > 4 .devinit.text 1558 001e5270 2**2 > 9 .exit.text0bc8 001e8d2c 2**2 > 10 .cpuinit.text 0924 001e98f4 2**2 > 11 .meminit.text 04cc 001ea218 2**2 > 12 .devexit.text 0160 001ea6e4 2**2 > 38 .cpuinit.data 0040 0025afc0 2**2 > 39 .meminit.data 000c 0025b000 2**2 > > > __devinit alone gives a net win of 5464 bytes. > That is only ~3% of total .text size but this is non-swapable > memory where everything is worth it. now how much of this is lost again because you have to round the stuff to pagesize? - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Thu, Jan 31 2008 at 19:49 +0200, Alan Stern <[EMAIL PROTECTED]> wrote:
> On Thu, 31 Jan 2008, Boaz Harrosh wrote:
>
>> @@ -228,9 +228,14 @@ void usb_stor_set_xfer_buf(unsigned char *buffer,
>> {
>> unsigned int offset = 0;
>> struct scatterlist *sg = NULL;
>> +unsigned int count;
>>
>> -usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
>> +count = usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
>> TO_XFER_BUF);
>> -if (buflen < scsi_bufflen(srb))
>> -scsi_set_resid(srb, scsi_bufflen(srb) - buflen);
>> +
>> +/* Check for underflow */
>> +if (buflen > scsi_bufflen(srb))
>> +count = buflen;
>> +
>> +scsi_set_resid(srb, scsi_bufflen(srb) - count);
>> }
>
> This last "if" statement doesn't look right. And since you know that
> count will never be larger than scsi_bufflen(srb), you don't have to
> check for underflow at all. Just leave out the "if" and call
> scsi_set_resid() directly.
>
> Alan Stern
>
I was thinking about that hard. And I disagree. It could be that we have
sg-list length (The total of all sg->length) that does not match
scsi_bufflen() - More or less. The code in usb_stor_access_xfer_buf() will
now correctly attempt to transfer according to buflen and what ever is available
at the passed sg's. Now this can be less or it can be more. SCSI standard
defines
this as underflow/overflow. When overflow should be reported as negative values.
and an error status. (BUT not CHECK_CONDITION)
so the code is actualy
if (buflen > scsi_bufflen(srb))
scsi_set_resid(srb, scsi_bufflen(srb) - buflen); /* Negative
overflow */
else
scsi_set_resid(srb, scsi_bufflen(srb) - count);
So this is actually code equivalent to above. Minus the extra call site.
The SCSI standard does allow these conditions and the system should cope
and go on. So a BUG_ON() is not accepted, I think.
But, ok, I mixed up with the comment I'll resend. underflow => overflow.
Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Remove of old NCR53C9x/esp family of drivers
On Thu, Jan 31 2008 at 20:55 +0200, Geert Uytterhoeven <[EMAIL PROTECTED]>
wrote:
> On Thu, 31 Jan 2008, James Bottomley wrote:
>> On Mon, 2008-01-07 at 07:07 +0100, Kars de Jong wrote:
>>> On do, 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...
>>> I'll have a look at this. I can only test it on Blizzard 1260 hardware
>>> though.
>> OK, time's up.
>>
>> These drivers are now unbuildable in mainline because of the promised
>> sg_table updates. They either get removed, fixed or marked as BROKEN.
>> Which is it to be?
>
> Is git smart enough to follow history between files that get removed and
> readded?
> If yes, I think you can remove them.
> If no, please mark them as BROKEN.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
> -
I did submit a fix to all these drivers, but It was said by people that these
drivers should not be fixed because they have a better alternative with the
other
esp family. And any devices not supported by the other family should be not more
then a day of work to support. So doing the better job of supporting them in the
new form is less effort then resurrecting junk code from the graveyard.
I say dump it. Christoph ??!
Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 31 Jan 2008, Nicholas A. Bellinger wrote: > On Thu, 2008-01-31 at 11:53 -0600, James Bottomley wrote: > > On Thu, 2008-01-31 at 09:28 -0800, Nicholas A. Bellinger wrote: > > > The problem case is a SCSI Target Mode engine that receives a 2048 Byte > > > single sector ATAPI READ_10 request from the storage fabric, and uses > > > scsi_execute_async() (the only option >= 2.6.18) to issue said request > > > to the underlying struct scsi_device. Because the underlying bio code > > > assumes 512 byte only sectors, the check in __bio_add_page() incorrectly > > > determines that max_sectors (max_sectors has to be low, as with 32 from > > > ps3rom.c) has been exceeded, and fails the request back up the stack. > > > > OK, so this is a totally separate issue from the one you actually posted > > it as a patch to fix? > > > > the queue max_sectors parameter is also counted in the block internal of > > 512 byte sectors. If you set it to 32 that means you were only > > expecting 16k of transfers per command maximum. If that's not right, > > then set the limit correctly. > > > > In short, and to repeat: almost every internal size counter to block is > > in units of 512 byte sectors ... that includes capacity, maximum etc ... > > > > Ok, after reading your followup with Geert I see that this looks like a > bug in ps3rom.c assuming 2048 byte sectors to calculate .max_sectors > (which was originally set to 32 as I mentioned). Using the setting > BOUNCE_SIZE << 9 where BOUNCE_SIZE is the request size in bytes looks > like this will solve the issue. My misunderstanding was > that .max_sectors was allowed to be calcuated in non 512 byte sectors, > so please disregard my patch. > > Geert, .max_sectors for ps3rom.c using 512 byte sectors ends up being > 128, yes.? Yes. With kind regards, Geert Uytterhoeven Software Architect Sony Network and Software Technology Center Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium Phone:+32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: [EMAIL PROTECTED] Internet: http://www.sony-europe.com/ Sony Network and Software Technology Center Europe A division of Sony Service Centre (Europe) N.V. Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium VAT BE 0413.825.160 · RPR Brussels Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
Re: [Scst-devel] Integration of SCST in the mainstream Linux kernel
On Thu, 2008-01-31 at 18:08 +0100, Bart Van Assche wrote: > If anyone has a suggestion for a better test than dd to compare the > performance of SCSI storage protocols, please let it know. xdd on /dev/sda, sdb, etc. using -dio to do direct IO seems to work decently, though it is hard (ie, impossible) to get a repeatable sequence of IO when using higher queue depths, as it uses threads to generate multiple requests. You may also look at sgpdd_survey from Lustre's iokit, but I've not done much with that -- it uses the sg devices to send lowlevel SCSI commands. I've been playing around with some benchmark code using libaio, but it's not in generally usable shape. xdd: http://www.ioperformance.com/products.htm Lustre IO Kit: http://manual.lustre.org/manual/LustreManual16_HTML/DynamicHTML-20-1.html -- Dave Dillow National Center for Computational Science Oak Ridge National Laboratory (865) 241-6602 office - 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, 31 Jan 2008, James Bottomley wrote:
> On Mon, 2008-01-07 at 07:07 +0100, Kars de Jong wrote:
> > On do, 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...
> >
> > I'll have a look at this. I can only test it on Blizzard 1260 hardware
> > though.
>
> OK, time's up.
>
> These drivers are now unbuildable in mainline because of the promised
> sg_table updates. They either get removed, fixed or marked as BROKEN.
> Which is it to be?
Is git smart enough to follow history between files that get removed and
readded?
If yes, I think you can remove them.
If no, please mark them as BROKEN.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-
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] kill hotplug init/exit section annotations
On Thu, 31 Jan 2008, Chris Wedgwood wrote:
> On Thu, Jan 31, 2008 at 07:55:43PM +0200, Adrian Bunk wrote:
> > Who was talking about laptops?
>
> If laptops are mostly MP these days, then 'desktops' and 'servers'
> certainly are --- so pretty much everyone needs CPU hotplug.
Thank you for giving an exhaustive list of classes of machines Linux runs on!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-
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-31 at 19:55 +0100, Geert Uytterhoeven wrote: > On Thu, 31 Jan 2008, James Bottomley wrote: > > On Mon, 2008-01-07 at 07:07 +0100, Kars de Jong wrote: > > > On do, 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... > > > > > > I'll have a look at this. I can only test it on Blizzard 1260 hardware > > > though. > > > > OK, time's up. > > > > These drivers are now unbuildable in mainline because of the promised > > sg_table updates. They either get removed, fixed or marked as BROKEN. > > Which is it to be? > > Is git smart enough to follow history between files that get removed and > readded? Yes. git revert on the removal changeset will re-add all the files and try to put the entries back into Kconfig and Makefile. > If yes, I think you can remove them. > If no, please mark them as BROKEN. OK ... I'll wave the magic wand. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Thu, 31 Jan 2008, Boaz Harrosh wrote:
> On Thu, Jan 31 2008 at 19:49 +0200, Alan Stern <[EMAIL PROTECTED]> wrote:
> > On Thu, 31 Jan 2008, Boaz Harrosh wrote:
> >
> >> @@ -228,9 +228,14 @@ void usb_stor_set_xfer_buf(unsigned char *buffer,
> >> {
> >>unsigned int offset = 0;
> >>struct scatterlist *sg = NULL;
> >> + unsigned int count;
> >>
> >> - usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
> >> + count = usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
> >>TO_XFER_BUF);
> >> - if (buflen < scsi_bufflen(srb))
> >> - scsi_set_resid(srb, scsi_bufflen(srb) - buflen);
> >> +
> >> + /* Check for underflow */
> >> + if (buflen > scsi_bufflen(srb))
> >> + count = buflen;
> >> +
> >> + scsi_set_resid(srb, scsi_bufflen(srb) - count);
> >> }
> >
> > This last "if" statement doesn't look right. And since you know that
> > count will never be larger than scsi_bufflen(srb), you don't have to
> > check for underflow at all. Just leave out the "if" and call
> > scsi_set_resid() directly.
> >
> > Alan Stern
> >
> I was thinking about that hard. And I disagree. It could be that we have
> sg-list length (The total of all sg->length) that does not match
> scsi_bufflen() - More or less.
If that ever happens we'll be in big trouble. Not just here but in
other places as well.
> The code in usb_stor_access_xfer_buf() will
> now correctly attempt to transfer according to buflen and what ever is
> available
> at the passed sg's. Now this can be less or it can be more. SCSI standard
> defines
> this as underflow/overflow. When overflow should be reported as negative
> values.
> and an error status. (BUT not CHECK_CONDITION)
I don't understand. How could you ever transfer more data than the
requested amount? The host won't expect to receive it and won't be
able to handle it when it arrives.
(Furthmore, the USB mass-storage specification does not permit more
data to be transferred than requested. This fact isn't relevant to our
discussion because we're talking about situations where the data was
transferred via a non-standard protocol. But even so...)
> so the code is actualy
> if (buflen > scsi_bufflen(srb))
> scsi_set_resid(srb, scsi_bufflen(srb) - buflen); /* Negative
> overflow */
Okay, let's assume that both buflen and the sum of the sg-list lengths
are somehow greater than scsi_bufflen(srb). You still don't know that
they are equal to each other. All you know is that the amount actually
transferred is stored in count. Hence the negative overflow value
should be
scsi_bufflen(srb) - count
and not
scsi_bufflen(srb) - bufflen.
Alan Stern
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.24] bugfix for an overflow condition in usb storage & isd200.c
Greg KH <[EMAIL PROTECTED]> rote:
> As this is a regression and hits 2.6.24, can you send the final version
> of this patch to the [EMAIL PROTECTED] address so we can get it into the
> 2.6.24.y tree?
>
> thanks,
>
> greg k-h
Mark - This is for you on top of vanila v2.6.24 kernel from Linus.
---
scsi_scan is issuing a 36-byte INQUIRY request to llds. isd200 would
volunteer 96 bytes of INQUIRY. This caused an overflow condition in
protocol.c usb_stor_access_xfer_buf(). So first fix is to
usb_stor_access_xfer_buf() to properly handle overflow/underflow conditions.
Then usb_stor_set_xfer_buf() should report this condition as a negative
resid. Should we also set cmnd->status in the overflow condition?
Then also isd200.c is fixed to only return the type of INQUIRY && SENSE
the upper layer asked for.
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
drivers/usb/storage/isd200.c |7 +--
drivers/usb/storage/protocol.c | 13 +
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 49ba6c0..8186e93 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -1238,6 +1238,7 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
struct us_data *us,
unsigned long lba;
unsigned long blockCount;
unsigned char senseData[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
+ unsigned xfer_len;
memset(ataCdb, 0, sizeof(union ata_cdb));
@@ -1247,8 +1248,9 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
struct us_data *us,
US_DEBUGP(" ATA OUT - INQUIRY\n");
/* copy InquiryData */
+ xfer_len = min(sizeof(info->InquiryData), scsi_bufflen(srb));
usb_stor_set_xfer_buf((unsigned char *) &info->InquiryData,
- sizeof(info->InquiryData), srb);
+ xfer_len, srb);
srb->result = SAM_STAT_GOOD;
sendToTransport = 0;
break;
@@ -1257,7 +1259,8 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
struct us_data *us,
US_DEBUGP(" ATA OUT - SCSIOP_MODE_SENSE\n");
/* Initialize the return buffer */
- usb_stor_set_xfer_buf(senseData, sizeof(senseData), srb);
+ xfer_len = min(sizeof(senseData), scsi_bufflen(srb));
+ usb_stor_set_xfer_buf(senseData, xfer_len, srb);
if (info->DeviceFlags & DF_MEDIA_STATUS_ENABLED)
{
diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c
index 889622b..038a284 100644
--- a/drivers/usb/storage/protocol.c
+++ b/drivers/usb/storage/protocol.c
@@ -194,7 +194,7 @@ unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,
* and the starting offset within the page, and update
* the *offset and *index values for the next loop. */
cnt = 0;
- while (cnt < buflen) {
+ while (cnt < buflen && sg) {
struct page *page = sg_page(sg) +
((sg->offset + *offset) >> PAGE_SHIFT);
unsigned int poff =
@@ -248,9 +248,14 @@ void usb_stor_set_xfer_buf(unsigned char *buffer,
{
unsigned int offset = 0;
struct scatterlist *sg = NULL;
+ unsigned count;
- usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
+ count = usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
TO_XFER_BUF);
- if (buflen < srb->request_bufflen)
- srb->resid = srb->request_bufflen - buflen;
+
+ /* Check for overflow */
+ if (buflen > scsi_bufflen(srb))
+ count = buflen;
+
+ scsi_set_resid(srb, scsi_bufflen(srb) - count);
}
--
1.5.3.3
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/12] bump iscsi version
From: Mike Christie <[EMAIL PROTECTED]>
Set iscsi version to 2.0-868
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/scsi_transport_iscsi.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index 8e73ff0..fac7534 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -33,7 +33,7 @@
#define ISCSI_SESSION_ATTRS 19
#define ISCSI_CONN_ATTRS 13
#define ISCSI_HOST_ATTRS 4
-#define ISCSI_TRANSPORT_VERSION "2.0-867"
+#define ISCSI_TRANSPORT_VERSION "2.0-868"
struct iscsi_internal {
int daemon_pid;
--
1.5.2.1
-
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 10/12] iscsi: fix up iscsi printk prefix
From: Mike Christie <[EMAIL PROTECTED]>
Some iscsi class messages have the dev_printk prefix and some libiscsi
and iscsi_tcp messages have "iscsi" or the module name as a prefix which
is normally pretty useless when trying to figure out which session
or connection the message is attached to. This patch adds iscsi lib
and class dev_printks so all messages have a common prefix that
can be used to figure out which object printed it.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/iscsi_tcp.c| 57 ++
drivers/scsi/libiscsi.c | 76 ---
drivers/scsi/scsi_transport_iscsi.c | 56 ++
include/scsi/libiscsi.h |7 +++
include/scsi/scsi_transport_iscsi.h |6 +++
5 files changed, 116 insertions(+), 86 deletions(-)
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index b6f99df..8a17867 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -629,8 +629,9 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
int rc;
if (tcp_conn->in.datalen) {
- printk(KERN_ERR "iscsi_tcp: invalid R2t with datalen %d\n",
- tcp_conn->in.datalen);
+ iscsi_conn_printk(KERN_ERR, conn,
+ "invalid R2t with datalen %d\n",
+ tcp_conn->in.datalen);
return ISCSI_ERR_DATALEN;
}
@@ -644,8 +645,9 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
if (!ctask->sc || session->state != ISCSI_STATE_LOGGED_IN) {
- printk(KERN_INFO "iscsi_tcp: dropping R2T itt %d in "
- "recovery...\n", ctask->itt);
+ iscsi_conn_printk(KERN_INFO, conn,
+ "dropping R2T itt %d in recovery.\n",
+ ctask->itt);
return 0;
}
@@ -655,7 +657,8 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
r2t->exp_statsn = rhdr->statsn;
r2t->data_length = be32_to_cpu(rhdr->data_length);
if (r2t->data_length == 0) {
- printk(KERN_ERR "iscsi_tcp: invalid R2T with zero data len\n");
+ iscsi_conn_printk(KERN_ERR, conn,
+ "invalid R2T with zero data len\n");
__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
sizeof(void*));
return ISCSI_ERR_DATALEN;
@@ -668,9 +671,10 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
r2t->data_offset = be32_to_cpu(rhdr->data_offset);
if (r2t->data_offset + r2t->data_length > scsi_bufflen(ctask->sc)) {
- printk(KERN_ERR "iscsi_tcp: invalid R2T with data len %u at "
- "offset %u and total length %d\n", r2t->data_length,
- r2t->data_offset, scsi_bufflen(ctask->sc));
+ iscsi_conn_printk(KERN_ERR, conn,
+ "invalid R2T with data len %u at offset %u "
+ "and total length %d\n", r2t->data_length,
+ r2t->data_offset, scsi_bufflen(ctask->sc));
__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
sizeof(void*));
return ISCSI_ERR_DATALEN;
@@ -736,8 +740,9 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct
iscsi_hdr *hdr)
/* verify PDU length */
tcp_conn->in.datalen = ntoh24(hdr->dlength);
if (tcp_conn->in.datalen > conn->max_recv_dlength) {
- printk(KERN_ERR "iscsi_tcp: datalen %d > %d\n",
- tcp_conn->in.datalen, conn->max_recv_dlength);
+ iscsi_conn_printk(KERN_ERR, conn,
+ "iscsi_tcp: datalen %d > %d\n",
+ tcp_conn->in.datalen, conn->max_recv_dlength);
return ISCSI_ERR_DATALEN;
}
@@ -819,10 +824,12 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct
iscsi_hdr *hdr)
* For now we fail until we find a vendor that needs it
*/
if (ISCSI_DEF_MAX_RECV_SEG_LEN < tcp_conn->in.datalen) {
- printk(KERN_ERR "iscsi_tcp: received buffer of len %u "
- "but conn buffer is only %u (opcode %0x)\n",
- tcp_conn->in.datalen,
- ISCSI_DEF_MAX_RECV_SEG_LEN, opcode);
+ iscsi_conn_printk(KERN_ERR, conn,
+ "iscsi_tcp: received buffer of "
+ "len %u but conn buffer is only %u "
+ "(opcode %0x)\n",
+
[PATCH 11/12] libiscsi: fix session age rollover and remove cid encoding
From: Mike Christie <[EMAIL PROTECTED]>
The session age mask is only 4 bits, but session->age is 32. When
it gets larger then 15 and we try to or the bits some bits get
dropped and the check for session age in iscsi_verify_itt is useless.
The ISCSI_CID_MASK related bits are also useless since cid is always
one.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/libiscsi.c| 14 --
include/scsi/iscsi_proto.h |4 ++--
include/scsi/libiscsi.h|2 --
3 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index c76bd5c..cfffabd 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -160,7 +160,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
hdr->opcode = ISCSI_OP_SCSI_CMD;
hdr->flags = ISCSI_ATTR_SIMPLE;
int_to_scsilun(sc->device->lun, (struct scsi_lun *)hdr->lun);
- hdr->itt = build_itt(ctask->itt, conn->id, session->age);
+ hdr->itt = build_itt(ctask->itt, session->age);
hdr->data_length = cpu_to_be32(scsi_bufflen(sc));
hdr->cmdsn = cpu_to_be32(session->cmdsn);
session->cmdsn++;
@@ -706,14 +706,6 @@ int iscsi_verify_itt(struct iscsi_conn *conn, struct
iscsi_hdr *hdr,
return ISCSI_ERR_BAD_ITT;
}
- if (((__force u32)hdr->itt & ISCSI_CID_MASK) !=
- (conn->id << ISCSI_CID_SHIFT)) {
- iscsi_conn_printk(KERN_ERR, conn,
- "iscsi: received itt %x, expected "
- "CID (%x)\n",
- (__force u32)hdr->itt, conn->id);
- return ISCSI_ERR_BAD_ITT;
- }
itt = get_itt(hdr->itt);
} else
itt = ~0U;
@@ -777,7 +769,7 @@ static void iscsi_prep_mtask(struct iscsi_conn *conn,
*/
nop->cmdsn = cpu_to_be32(session->cmdsn);
if (hdr->itt != RESERVED_ITT) {
- hdr->itt = build_itt(mtask->itt, conn->id, session->age);
+ hdr->itt = build_itt(mtask->itt, session->age);
/*
* TODO: We always use immediate, so we never hit this.
* If we start to send tmfs or nops as non-immediate then
@@ -2037,6 +2029,8 @@ int iscsi_conn_start(struct iscsi_cls_conn *cls_conn)
conn->stop_stage = 0;
conn->tmf_state = TMF_INITIAL;
session->age++;
+ if (session->age == 16)
+ session->age = 0;
break;
case STOP_CONN_TERM:
conn->stop_stage = 0;
diff --git a/include/scsi/iscsi_proto.h b/include/scsi/iscsi_proto.h
index 318a909..5ffec8a 100644
--- a/include/scsi/iscsi_proto.h
+++ b/include/scsi/iscsi_proto.h
@@ -45,8 +45,8 @@
/* initiator tags; opaque for target */
typedef uint32_t __bitwise__ itt_t;
/* below makes sense only for initiator that created this tag */
-#define build_itt(itt, id, age) ((__force itt_t)\
- ((itt) | ((id) << ISCSI_CID_SHIFT) | ((age) << ISCSI_AGE_SHIFT)))
+#define build_itt(itt, age) ((__force itt_t)\
+ ((itt) | ((age) << ISCSI_AGE_SHIFT)))
#define get_itt(itt) ((__force uint32_t)(itt_t)(itt) & ISCSI_ITT_MASK)
#define RESERVED_ITT ((__force itt_t)0x)
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 5daa83c..9b955a9 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -70,8 +70,6 @@ enum {
#define ISCSI_SUSPEND_BIT 1
#define ISCSI_ITT_MASK (0xfff)
-#define ISCSI_CID_SHIFT12
-#define ISCSI_CID_MASK (0x << ISCSI_CID_SHIFT)
#define ISCSI_AGE_SHIFT28
#define ISCSI_AGE_MASK (0xf << ISCSI_AGE_SHIFT)
--
1.5.2.1
-
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 08/12] libiscsi: fix setting of nop timer
From: Mike Christie <[EMAIL PROTECTED]>
If we rollover then we could get a next_timeout of zero, so we need
to set the new timer to that value.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/libiscsi.c |9 +++--
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 8c41ddb..7e781fd 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1385,14 +1385,11 @@ static void iscsi_check_transport_timeouts(unsigned
long data)
iscsi_send_nopout(conn, NULL);
}
next_timeout = last_recv + timeout + (conn->ping_timeout * HZ);
- } else {
+ } else
next_timeout = last_recv + timeout;
- }
- if (next_timeout) {
- debug_scsi("Setting next tmo %lu\n", next_timeout);
- mod_timer(&conn->transport_timer, next_timeout);
- }
+ debug_scsi("Setting next tmo %lu\n", next_timeout);
+ mod_timer(&conn->transport_timer, next_timeout);
done:
spin_unlock(&session->lock);
}
--
1.5.2.1
-
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 07/12] qla4xxx: add qla4xxx async scan support
From: Mike Christie <[EMAIL PROTECTED]>
qla4xxx has the old school startup/probe where it finds presetup sessions
in its flash and then attempts to log into them before returning from the
probe. This however, makes it very simple to add a iscsi class scan finished
helper which the driver can use.
In future patches Dave or I will rip apart the driver to make it more
like qla2xxx, but for now this is a very simple two line patch which
fixes the problem of trying to figure out when the initial sessions
are done being scanned.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/qla4xxx/ql4_os.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index d4dd149..c3c59d7 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -89,6 +89,8 @@ static struct scsi_host_template qla4xxx_driver_template = {
.slave_alloc= qla4xxx_slave_alloc,
.slave_destroy = qla4xxx_slave_destroy,
+ .scan_finished = iscsi_scan_finished,
+
.this_id= -1,
.cmd_per_lun= 3,
.use_clustering = ENABLE_CLUSTERING,
@@ -1306,7 +1308,7 @@ static int __devinit qla4xxx_probe_adapter(struct pci_dev
*pdev,
qla4xxx_version_str, ha->pdev->device, pci_name(ha->pdev),
ha->host_no, ha->firmware_version[0], ha->firmware_version[1],
ha->patch_number, ha->build_number);
-
+ scsi_scan_host(host);
return 0;
remove_host:
--
1.5.2.1
-
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 04/12] iscsi class: add session scanning
From: Mike Christie <[EMAIL PROTECTED]>
This just adds iscsi session scanning which works like fc rport scanning.
The future patches will hook the drivers into Mathew Wilcox's async
scanning infrastructure, so userspace does not have to special case
iscsi and so userspace does not have to make a extra special case for
hardware iscsi root scanning.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/scsi_transport_iscsi.c | 37 --
include/scsi/scsi_transport_iscsi.h |7 +++--
2 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index f876b0a..af88955 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -128,11 +128,11 @@ static int iscsi_setup_host(struct transport_container
*tc, struct device *dev,
INIT_LIST_HEAD(&ihost->sessions);
mutex_init(&ihost->mutex);
- snprintf(ihost->unbind_workq_name, KOBJ_NAME_LEN, "iscsi_unbind_%d",
+ snprintf(ihost->scan_workq_name, KOBJ_NAME_LEN, "iscsi_scan_%d",
shost->host_no);
- ihost->unbind_workq = create_singlethread_workqueue(
- ihost->unbind_workq_name);
- if (!ihost->unbind_workq)
+ ihost->scan_workq = create_singlethread_workqueue(
+ ihost->scan_workq_name);
+ if (!ihost->scan_workq)
return -ENOMEM;
return 0;
}
@@ -143,7 +143,7 @@ static int iscsi_remove_host(struct transport_container
*tc, struct device *dev,
struct Scsi_Host *shost = dev_to_shost(dev);
struct iscsi_host *ihost = shost->shost_data;
- destroy_workqueue(ihost->unbind_workq);
+ destroy_workqueue(ihost->scan_workq);
return 0;
}
@@ -302,6 +302,23 @@ static int iscsi_user_scan(struct Scsi_Host *shost, uint
channel,
return 0;
}
+static void iscsi_scan_session(struct work_struct *work)
+{
+ struct iscsi_cls_session *session =
+ container_of(work, struct iscsi_cls_session, scan_work);
+ unsigned long flags;
+
+ spin_lock_irqsave(&session->lock, flags);
+ if (session->state != ISCSI_SESSION_LOGGED_IN) {
+ spin_unlock_irqrestore(&session->lock, flags);
+ return;
+ }
+ spin_unlock_irqrestore(&session->lock, flags);
+
+ scsi_scan_target(&session->dev, 0, session->target_id,
+SCAN_WILD_CARD, 1);
+}
+
static void session_recovery_timedout(struct work_struct *work)
{
struct iscsi_cls_session *session =
@@ -340,6 +357,8 @@ void __iscsi_unblock_session(struct iscsi_cls_session
*session)
void iscsi_unblock_session(struct iscsi_cls_session *session)
{
+ struct Scsi_Host *shost = iscsi_session_to_shost(session);
+ struct iscsi_host *ihost = shost->shost_data;
unsigned long flags;
spin_lock_irqsave(&session->lock, flags);
@@ -347,6 +366,7 @@ void iscsi_unblock_session(struct iscsi_cls_session
*session)
spin_unlock_irqrestore(&session->lock, flags);
__iscsi_unblock_session(session);
+ queue_work(ihost->scan_workq, &session->scan_work);
}
EXPORT_SYMBOL_GPL(iscsi_unblock_session);
@@ -390,7 +410,7 @@ static int iscsi_unbind_session(struct iscsi_cls_session
*session)
struct Scsi_Host *shost = iscsi_session_to_shost(session);
struct iscsi_host *ihost = shost->shost_data;
- return queue_work(ihost->unbind_workq, &session->unbind_work);
+ return queue_work(ihost->scan_workq, &session->unbind_work);
}
struct iscsi_cls_session *
@@ -411,6 +431,7 @@ iscsi_alloc_session(struct Scsi_Host *shost,
INIT_LIST_HEAD(&session->host_list);
INIT_LIST_HEAD(&session->sess_list);
INIT_WORK(&session->unbind_work, __iscsi_unbind_session);
+ INIT_WORK(&session->scan_work, iscsi_scan_session);
spin_lock_init(&session->lock);
/* this is released in the dev's release function */
@@ -530,13 +551,15 @@ void iscsi_remove_session(struct iscsi_cls_session
*session)
spin_unlock_irqrestore(&session->lock, flags);
__iscsi_unblock_session(session);
iscsi_unbind_session(session);
+
+ /* flush running scans */
+ flush_workqueue(ihost->scan_workq);
/*
* If the session dropped while removing devices then we need to make
* sure it is not blocked
*/
if (!cancel_delayed_work(&session->recovery_work))
flush_workqueue(iscsi_eh_timer_workq);
- flush_workqueue(ihost->unbind_workq);
/* hw iscsi may not have removed all connections from session */
err = device_for_each_child(&session->dev, NULL,
diff --git a/include/scsi/scsi_transport_iscsi.h
b/include/scsi/scsi_transport_iscsi.h
index 0e869d9..1f0ec46 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/sc
[PATCH 05/12] qla4xxx: fix recovery timer and session unblock race
From: Mike Christie <[EMAIL PROTECTED]>
If qla4xxx is resetting up a session and the recovery timer
fires we do not want to just set it to dead, because
the dpc thread could have just set it to online and is in the
middle of resetting it up.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/qla4xxx/ql4_os.c | 19 +++
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 437d169..d4dd149 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -124,16 +124,19 @@ static void qla4xxx_recovery_timedout(struct
iscsi_cls_session *session)
struct ddb_entry *ddb_entry = session->dd_data;
struct scsi_qla_host *ha = ddb_entry->ha;
- DEBUG2(printk("scsi%ld: %s: index [%d] port down retry count of (%d) "
- "secs exhausted, marking device DEAD.\n", ha->host_no,
- __func__, ddb_entry->fw_ddb_index,
- ha->port_down_retry_count));
+ if (atomic_read(&ddb_entry->state) != DDB_STATE_ONLINE) {
+ atomic_set(&ddb_entry->state, DDB_STATE_DEAD);
- atomic_set(&ddb_entry->state, DDB_STATE_DEAD);
+ DEBUG2(printk("scsi%ld: %s: index [%d] port down retry count "
+ "of (%d) secs exhausted, marking device DEAD.\n",
+ ha->host_no, __func__, ddb_entry->fw_ddb_index,
+ ha->port_down_retry_count));
- DEBUG2(printk("scsi%ld: %s: scheduling dpc routine - dpc flags = "
- "0x%lx\n", ha->host_no, __func__, ha->dpc_flags));
- queue_work(ha->dpc_thread, &ha->dpc_work);
+ DEBUG2(printk("scsi%ld: %s: scheduling dpc routine - dpc "
+ "flags = 0x%lx\n",
+ ha->host_no, __func__, ha->dpc_flags));
+ queue_work(ha->dpc_thread, &ha->dpc_work);
+ }
}
static int qla4xxx_host_get_param(struct Scsi_Host *shost,
--
1.5.2.1
-
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 02/12] qla4xxx: have qla4xxx directly call iscsi recovery functions
From: Mike Christie <[EMAIL PROTECTED]>
Qla4xxx can just call the iscsi recovery functions directly.
There is no need for userspace to do this for qla4xxx, because
we do not use the mutex to iterate over devices anymore and iscsi_block
/unblock_session can be called from interrupt context or the dpc thread.
And having userspace do this just creates uneeded headaches for qla4xxx root
situations where the session may experience problems. For example
during the kernel shutdown the scsi layer wants to send sync caches, but at
this time userspace is not up (iscsid is not running), so we cannot
recover from the problem.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/qla4xxx/ql4_init.c |1 +
drivers/scsi/qla4xxx/ql4_os.c | 40 +++---
2 files changed, 5 insertions(+), 36 deletions(-)
diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
index cbe0a17..03e66cb 100644
--- a/drivers/scsi/qla4xxx/ql4_init.c
+++ b/drivers/scsi/qla4xxx/ql4_init.c
@@ -1306,6 +1306,7 @@ int qla4xxx_process_ddb_changed(struct scsi_qla_host *ha,
atomic_set(&ddb_entry->relogin_timer, 0);
clear_bit(DF_RELOGIN, &ddb_entry->flags);
clear_bit(DF_NO_RELOGIN, &ddb_entry->flags);
+ iscsi_unblock_session(ddb_entry->sess);
iscsi_session_event(ddb_entry->sess,
ISCSI_KEVENT_CREATE_SESSION);
/*
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 2e2b9fe..a87fb9f 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -63,8 +63,6 @@ static int qla4xxx_sess_get_param(struct iscsi_cls_session
*sess,
enum iscsi_param param, char *buf);
static int qla4xxx_host_get_param(struct Scsi_Host *shost,
enum iscsi_host_param param, char *buf);
-static void qla4xxx_conn_stop(struct iscsi_cls_conn *conn, int flag);
-static int qla4xxx_conn_start(struct iscsi_cls_conn *conn);
static void qla4xxx_recovery_timedout(struct iscsi_cls_session *session);
/*
@@ -116,8 +114,6 @@ static struct iscsi_transport qla4xxx_iscsi_transport = {
.get_conn_param = qla4xxx_conn_get_param,
.get_session_param = qla4xxx_sess_get_param,
.get_host_param = qla4xxx_host_get_param,
- .start_conn = qla4xxx_conn_start,
- .stop_conn = qla4xxx_conn_stop,
.session_recovery_timedout = qla4xxx_recovery_timedout,
};
@@ -140,38 +136,6 @@ static void qla4xxx_recovery_timedout(struct
iscsi_cls_session *session)
queue_work(ha->dpc_thread, &ha->dpc_work);
}
-static int qla4xxx_conn_start(struct iscsi_cls_conn *conn)
-{
- struct iscsi_cls_session *session;
- struct ddb_entry *ddb_entry;
-
- session = iscsi_dev_to_session(conn->dev.parent);
- ddb_entry = session->dd_data;
-
- DEBUG2(printk("scsi%ld: %s: index [%d] starting conn\n",
- ddb_entry->ha->host_no, __func__,
- ddb_entry->fw_ddb_index));
- iscsi_unblock_session(session);
- return 0;
-}
-
-static void qla4xxx_conn_stop(struct iscsi_cls_conn *conn, int flag)
-{
- struct iscsi_cls_session *session;
- struct ddb_entry *ddb_entry;
-
- session = iscsi_dev_to_session(conn->dev.parent);
- ddb_entry = session->dd_data;
-
- DEBUG2(printk("scsi%ld: %s: index [%d] stopping conn\n",
- ddb_entry->ha->host_no, __func__,
- ddb_entry->fw_ddb_index));
- if (flag == STOP_CONN_RECOVER)
- iscsi_block_session(session);
- else
- printk(KERN_ERR "iscsi: invalid stop flag %d\n", flag);
-}
-
static int qla4xxx_host_get_param(struct Scsi_Host *shost,
enum iscsi_host_param param, char *buf)
{
@@ -308,6 +272,9 @@ int qla4xxx_add_sess(struct ddb_entry *ddb_entry)
DEBUG2(printk(KERN_ERR "Could not add connection.\n"));
return -ENOMEM;
}
+
+ /* finally ready to go */
+ iscsi_unblock_session(ddb_entry->sess);
return 0;
}
@@ -364,6 +331,7 @@ void qla4xxx_mark_device_missing(struct scsi_qla_host *ha,
DEBUG3(printk("scsi%d:%d:%d: index [%d] marked MISSING\n",
ha->host_no, ddb_entry->bus, ddb_entry->target,
ddb_entry->fw_ddb_index));
+ iscsi_block_session(ddb_entry->sess);
iscsi_conn_error(ddb_entry->conn, ISCSI_ERR_CONN_FAILED);
}
--
1.5.2.1
-
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 01/12] iscsi class, libiscsi: add iscsi sysfs session state file
From: Mike Christie <[EMAIL PROTECTED]>
This adds a iscsi session state file which exports the session
state for both software and hardware iscsi. It also hooks libiscsi
in.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/libiscsi.c | 41 +-
drivers/scsi/scsi_transport_iscsi.c | 107 ++-
include/scsi/libiscsi.h | 19 ++
include/scsi/scsi_transport_iscsi.h | 27 -
4 files changed, 161 insertions(+), 33 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 553168a..8c41ddb 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -997,6 +997,7 @@ enum {
FAILURE_SESSION_IN_RECOVERY,
FAILURE_SESSION_RECOVERY_TIMEOUT,
FAILURE_SESSION_LOGGING_OUT,
+ FAILURE_SESSION_NOT_READY,
};
int iscsi_queuecommand(struct scsi_cmnd *sc, void (*done)(struct scsi_cmnd *))
@@ -1017,6 +1018,12 @@ int iscsi_queuecommand(struct scsi_cmnd *sc, void
(*done)(struct scsi_cmnd *))
session = iscsi_hostdata(host->hostdata);
spin_lock(&session->lock);
+ reason = iscsi_session_chkready(session_to_cls(session));
+ if (reason) {
+ sc->result = reason;
+ goto fault;
+ }
+
/*
* ISCSI_STATE_FAILED is a temp. state. The recovery
* code will decide what is best to do with command queued
@@ -1033,18 +1040,23 @@ int iscsi_queuecommand(struct scsi_cmnd *sc, void
(*done)(struct scsi_cmnd *))
switch (session->state) {
case ISCSI_STATE_IN_RECOVERY:
reason = FAILURE_SESSION_IN_RECOVERY;
- goto reject;
+ sc->result = DID_IMM_RETRY << 16;
+ break;
case ISCSI_STATE_LOGGING_OUT:
reason = FAILURE_SESSION_LOGGING_OUT;
- goto reject;
+ sc->result = DID_IMM_RETRY << 16;
+ break;
case ISCSI_STATE_RECOVERY_FAILED:
reason = FAILURE_SESSION_RECOVERY_TIMEOUT;
+ sc->result = DID_NO_CONNECT << 16;
break;
case ISCSI_STATE_TERMINATE:
reason = FAILURE_SESSION_TERMINATE;
+ sc->result = DID_NO_CONNECT << 16;
break;
default:
reason = FAILURE_SESSION_FREED;
+ sc->result = DID_NO_CONNECT << 16;
}
goto fault;
}
@@ -1052,6 +1064,7 @@ int iscsi_queuecommand(struct scsi_cmnd *sc, void
(*done)(struct scsi_cmnd *))
conn = session->leadconn;
if (!conn) {
reason = FAILURE_SESSION_FREED;
+ sc->result = DID_NO_CONNECT << 16;
goto fault;
}
@@ -1091,9 +1104,7 @@ reject:
fault:
spin_unlock(&session->lock);
- printk(KERN_ERR "iscsi: cmd 0x%x is not queued (%d)\n",
- sc->cmnd[0], reason);
- sc->result = (DID_NO_CONNECT << 16);
+ debug_scsi("iscsi: cmd 0x%x is not queued (%d)\n", sc->cmnd[0], reason);
scsi_set_resid(sc, scsi_bufflen(sc));
sc->scsi_done(sc);
spin_lock(host->host_lock);
@@ -1239,7 +1250,8 @@ static int iscsi_exec_task_mgmt_fn(struct iscsi_conn
*conn,
* Fail commands. session lock held and recv side suspended and xmit
* thread flushed
*/
-static void fail_all_commands(struct iscsi_conn *conn, unsigned lun)
+static void fail_all_commands(struct iscsi_conn *conn, unsigned lun,
+ int error)
{
struct iscsi_cmd_task *ctask, *tmp;
@@ -1251,7 +1263,7 @@ static void fail_all_commands(struct iscsi_conn *conn,
unsigned lun)
if (lun == ctask->sc->device->lun || lun == -1) {
debug_scsi("failing pending sc %p itt 0x%x\n",
ctask->sc, ctask->itt);
- fail_command(conn, ctask, DID_BUS_BUSY << 16);
+ fail_command(conn, ctask, error << 16);
}
}
@@ -1259,7 +1271,7 @@ static void fail_all_commands(struct iscsi_conn *conn,
unsigned lun)
if (lun == ctask->sc->device->lun || lun == -1) {
debug_scsi("failing requeued sc %p itt 0x%x\n",
ctask->sc, ctask->itt);
- fail_command(conn, ctask, DID_BUS_BUSY << 16);
+ fail_command(conn, ctask, error << 16);
}
}
@@ -1573,7 +1585,7 @@ int iscsi_eh_device_reset(struct scsi_cmnd *sc)
/* need to grab the recv lock then session lock */
write_lock_bh(conn->recv_lock);
spin_lock(&session->lock);
- fail_all_commands(conn, sc->device->lun);
+ fail_all_commands(conn, sc->device->lun, DID_ERROR);
conn
iscsi update
The following patches were made over scsi-misc. The bugs fixed are: - Have qla4xxx hook into block/unblock code, and use new session state to fail/requeue IO during transport problems. - Hook qla4xxx into async scanning code. Qla4xxx looks more like a normal old scsi card than a iscsi card. There is very little iscsi stuff going on in the driver, so it is much easier to just have this type of iscsi card work like other FC and SPI cards. Currently distros are doing crazy things to special case qla4xxx, so you can boot from it. - Fix libiscsi nop timer setting, bad printks and session age testing. - 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 03/12] qla4xxx: have qla4xxx use iscsi class session state check ready
From: Mike Christie <[EMAIL PROTECTED]>
This has qla4xxx use the iscsi class's check ready function
in the queue command function, so all iscsi drivers return the
same error value for common problems.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/qla4xxx/ql4_os.c | 12
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index a87fb9f..437d169 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -398,9 +398,21 @@ static int qla4xxx_queuecommand(struct scsi_cmnd *cmd,
{
struct scsi_qla_host *ha = to_qla_host(cmd->device->host);
struct ddb_entry *ddb_entry = cmd->device->hostdata;
+ struct iscsi_cls_session *sess = ddb_entry->sess;
struct srb *srb;
int rval;
+ if (!sess) {
+ cmd->result = DID_IMM_RETRY << 16;
+ goto qc_fail_command;
+ }
+
+ rval = iscsi_session_chkready(sess);
+ if (rval) {
+ cmd->result = rval;
+ goto qc_fail_command;
+ }
+
if (atomic_read(&ddb_entry->state) != DDB_STATE_ONLINE) {
if (atomic_read(&ddb_entry->state) == DDB_STATE_DEAD) {
cmd->result = DID_NO_CONNECT << 16;
--
1.5.2.1
-
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 06/12] iscsi class: add async scan helper
From: Mike Christie <[EMAIL PROTECTED]>
In qla4xxx's probe it will call the iscsi session setup functions
for session that got setup on the initial start. This then makes
it easy for the iscsi class to export a helper which indicates
when those scans are done.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/scsi_transport_iscsi.c | 38 --
include/scsi/scsi_transport_iscsi.h |3 +-
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index af88955..af17997 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -127,6 +127,7 @@ static int iscsi_setup_host(struct transport_container *tc,
struct device *dev,
memset(ihost, 0, sizeof(*ihost));
INIT_LIST_HEAD(&ihost->sessions);
mutex_init(&ihost->mutex);
+ atomic_set(&ihost->nr_scans, 0);
snprintf(ihost->scan_workq_name, KOBJ_NAME_LEN, "iscsi_scan_%d",
shost->host_no);
@@ -284,6 +285,25 @@ static int iscsi_is_session_dev(const struct device *dev)
return dev->release == iscsi_session_release;
}
+/**
+ * iscsi_scan_finished - helper to report when running scans are done
+ * @shost: scsi host
+ * @time: scan run time
+ *
+ * This function can be used by drives like qla4xxx to report to the scsi
+ * layer when the scans it kicked off at module load time are done.
+ */
+int iscsi_scan_finished(struct Scsi_Host *shost, unsigned long time)
+{
+ struct iscsi_host *ihost = shost->shost_data;
+ /*
+* qla4xxx will have kicked off some session unblocks before calling
+* scsi_scan_host, so just wait for them to complete.
+*/
+ return !atomic_read(&ihost->nr_scans);
+}
+EXPORT_SYMBOL_GPL(iscsi_scan_finished);
+
static int iscsi_user_scan(struct Scsi_Host *shost, uint channel,
uint id, uint lun)
{
@@ -306,17 +326,21 @@ static void iscsi_scan_session(struct work_struct *work)
{
struct iscsi_cls_session *session =
container_of(work, struct iscsi_cls_session, scan_work);
+ struct Scsi_Host *shost = iscsi_session_to_shost(session);
+ struct iscsi_host *ihost = shost->shost_data;
unsigned long flags;
spin_lock_irqsave(&session->lock, flags);
if (session->state != ISCSI_SESSION_LOGGED_IN) {
spin_unlock_irqrestore(&session->lock, flags);
- return;
+ goto done;
}
spin_unlock_irqrestore(&session->lock, flags);
scsi_scan_target(&session->dev, 0, session->target_id,
SCAN_WILD_CARD, 1);
+done:
+ atomic_dec(&ihost->nr_scans);
}
static void session_recovery_timedout(struct work_struct *work)
@@ -366,7 +390,15 @@ void iscsi_unblock_session(struct iscsi_cls_session
*session)
spin_unlock_irqrestore(&session->lock, flags);
__iscsi_unblock_session(session);
- queue_work(ihost->scan_workq, &session->scan_work);
+ /*
+* Only do kernel scanning if the driver is properly hooked into
+* the async scanning code (drivers like iscsi_tcp do login and
+* scanning from userspace).
+*/
+ if (shost->hostt->scan_finished) {
+ if (queue_work(ihost->scan_workq, &session->scan_work))
+ atomic_inc(&ihost->nr_scans);
+ }
}
EXPORT_SYMBOL_GPL(iscsi_unblock_session);
@@ -550,7 +582,7 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
session->state = ISCSI_SESSION_FREE;
spin_unlock_irqrestore(&session->lock, flags);
__iscsi_unblock_session(session);
- iscsi_unbind_session(session);
+ __iscsi_unbind_session(&session->unbind_work);
/* flush running scans */
flush_workqueue(ihost->scan_workq);
diff --git a/include/scsi/scsi_transport_iscsi.h
b/include/scsi/scsi_transport_iscsi.h
index 1f0ec46..83693ba 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -203,6 +203,7 @@ struct iscsi_cls_session {
struct iscsi_host {
struct list_head sessions;
+ atomic_t nr_scans;
struct mutex mutex;
struct workqueue_struct *scan_workq;
char scan_workq_name[KOBJ_NAME_LEN];
@@ -229,6 +230,6 @@ extern struct iscsi_cls_conn *iscsi_create_conn(struct
iscsi_cls_session *sess,
extern int iscsi_destroy_conn(struct iscsi_cls_conn *conn);
extern void iscsi_unblock_session(struct iscsi_cls_session *session);
extern void iscsi_block_session(struct iscsi_cls_session *session);
-
+extern int iscsi_scan_finished(struct Scsi_Host *shost, unsigned long time);
#endif
--
1.5.2.1
-
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 09/12] iscsi class: fix iscsi conn attr counter
From: Mike Christie <[EMAIL PROTECTED]> There are 13 iscsi conn attrs, but since the IF/OF markers were not being used we did not notice that we forgot to increment the ISCSI_CONN_ATTRS counter. Signed-off-by: Mike Christie <[EMAIL PROTECTED]> --- drivers/scsi/scsi_transport_iscsi.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index af17997..35834bf 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -31,7 +31,7 @@ #include #define ISCSI_SESSION_ATTRS 19 -#define ISCSI_CONN_ATTRS 11 +#define ISCSI_CONN_ATTRS 13 #define ISCSI_HOST_ATTRS 4 #define ISCSI_TRANSPORT_VERSION "2.0-867" -- 1.5.2.1 - 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] kill hotplug init/exit section annotations
On Thu, Jan 31, 2008 at 10:48:11AM -0800, Arjan van de Ven wrote:
> On Thu, 31 Jan 2008 19:34:25 +0100
> Sam Ravnborg <[EMAIL PROTECTED]> wrote:
>
> > On Thu, Jan 31, 2008 at 09:48:01AM -0800, Arjan van de Ven wrote:
> > > On Thu, 31 Jan 2008 19:14:36 +0200
> > > Adrian Bunk <[EMAIL PROTECTED]> wrote:
> > > > > cpuhotplug is required for suspend/resume.
> > > >
> > > > Not on UP computers.
> > > >
> > >
> > > great! someone who still has one of those and uses a kernel without
> > > it. Can you look at your system.map and see how many kilobytes
> > > you've gained? Eg how many kilobytes are in these sections exactly?
> > I have one. A Atmel AT91 board equipped with an 9263.
> > So lets take a look at the defconfig build for the evaluation board.
> >
> >
> > o-arm/vmlinux.o: file format elf32-littlearm
> >
> > 0 .text 001cdefc 0400 2**10
> > 2 .init.text000165e8 001ce6c0 2**5
> > 26 .init.data32ec 002578cc 2**2
> >
> > ---
> >
> > 4 .devinit.text 1558 001e5270 2**2
> > 9 .exit.text0bc8 001e8d2c 2**2
> > 10 .cpuinit.text 0924 001e98f4 2**2
> > 11 .meminit.text 04cc 001ea218 2**2
> > 12 .devexit.text 0160 001ea6e4 2**2
> > 38 .cpuinit.data 0040 0025afc0 2**2
> > 39 .meminit.data 000c 0025b000 2**2
> >
> >
> > __devinit alone gives a net win of 5464 bytes.
> > That is only ~3% of total .text size but this is non-swapable
> > memory where everything is worth it.
>
> now how much of this is lost again because you have to round the stuff to
> pagesize?
Lets take a look in the ARM vmlinux.lds.S file:
.text.head : {
_stext = .;
_sinittext = .;
*(.text.head)
}
.init : { /* Init code and data */
INIT_TEXT
_einittext = .;
__proc_info_begin = .;
*(.proc.info.init)
__proc_info_end = .;
__arch_info_begin = .;
*(.arch.info.init)
__arch_info_end = .;
__tagtable_begin = .;
*(.taglist.init)
__tagtable_end = .;
. = ALIGN(16);
__setup_start = .;
*(.init.setup)
__setup_end = .;
__early_begin = .;
*(.early_param.init)
__early_end = .;
__initcall_start = .;
INITCALLS
__initcall_end = .;
__con_initcall_start = .;
*(.con_initcall.init)
__con_initcall_end = .;
__security_initcall_start = .;
*(.security_initcall.init)
__security_initcall_end = .;
#ifdef CONFIG_BLK_DEV_INITRD
. = ALIGN(32);
__initramfs_start = .;
usr/built-in.o(.init.ramfs)
__initramfs_end = .;
#endif
. = ALIGN(4096);
__per_cpu_start = .;
*(.data.percpu)
*(.data.percpu.shared_aligned)
__per_cpu_end = .;
#ifndef CONFIG_XIP_KERNEL
__init_begin = _stext;
INIT_DATA
. = ALIGN(4096);
__init_end = .;
Everything between _stext and __init_end are freed.
And we have two PAGESIZE alignmnets here - one for the percpu stuff
and the other to align the full area that can be freed.
So there is no special alignment for the __devinit stuff.
And there is no percpu data on ARM so that area is empty.
So it is one or two pages extra that are freed up.
Sam
-
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: [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes
On Thu, 2008-01-31 at 10:44 -0800, Nicholas A. Bellinger wrote: > > In short, and to repeat: almost every internal size counter to block is > > in units of 512 byte sectors ... that includes capacity, maximum etc ... > > > > Ok, after reading your followup with Geert I see that this looks like a > bug in ps3rom.c assuming 2048 byte sectors to calculate .max_sectors > (which was originally set to 32 as I mentioned). Using the setting > BOUNCE_SIZE << 9 where BOUNCE_SIZE is the request size in bytes looks > like this will solve the issue. My misunderstanding was > that .max_sectors was allowed to be calcuated in non 512 byte sectors, > so please disregard my patch. > > Geert, .max_sectors for ps3rom.c using 512 byte sectors ends up being > 128, yes.? > > James, could we put something in the SCSI docs stating that .max_sectors > MUST be calculated against 512 byte sectors..? If that "we" is royal, then certainly you're welcome to submit a patch (just get Randy's ack). However, do take a look at the existing docs first. Certainly the block layer seems to make this very clear: /** * blk_queue_max_sectors - set max sectors for a request for this queue * @q: the request queue for the device * @max_sectors: max sectors in the usual 512b unit ^^ * * Description: *Enables a low level driver to set an upper limit on the size of *received requests. **/ I suspect you just didn't read the docs anyway ... ? 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 2.6.24] bugfix for an overflow condition in usb storage & isd200.c
No. No no no.
The ISD200 code was written by the ISD200 developers. I really don't want
to go mucking about changing what commands actually get send to the ISD200
parts. We have no idea if the will reliably accept a 36-byte INQUIRY.
Just because it happens to work for a couple of people doesn't mean it
works in the general case. Without guidance from In-System, this is a bad
idea.
The way to deal with this is to do this via bounce buffering. The two
commands affected (INQUIRY and MODE_SENSE) are low-performance items.
Discarding data from the end of them is also perfectly legal per spec.
Also, the entire idea of a negative resid makes my head hurt. That sort of
change is in the category of "likely to break something else" which only
expects positive resid values.
Matt
On Thu, Jan 31, 2008 at 09:37:13PM +0200, Boaz Harrosh wrote:
> Greg KH <[EMAIL PROTECTED]> rote:
> > As this is a regression and hits 2.6.24, can you send the final version
> > of this patch to the [EMAIL PROTECTED] address so we can get it into the
> > 2.6.24.y tree?
> >
> > thanks,
> >
> > greg k-h
>
> Mark - This is for you on top of vanila v2.6.24 kernel from Linus.
>
> ---
>
> scsi_scan is issuing a 36-byte INQUIRY request to llds. isd200 would
> volunteer 96 bytes of INQUIRY. This caused an overflow condition in
> protocol.c usb_stor_access_xfer_buf(). So first fix is to
> usb_stor_access_xfer_buf() to properly handle overflow/underflow conditions.
> Then usb_stor_set_xfer_buf() should report this condition as a negative
> resid. Should we also set cmnd->status in the overflow condition?
>
> Then also isd200.c is fixed to only return the type of INQUIRY && SENSE
> the upper layer asked for.
>
> Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
> ---
> drivers/usb/storage/isd200.c |7 +--
> drivers/usb/storage/protocol.c | 13 +
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
> index 49ba6c0..8186e93 100644
> --- a/drivers/usb/storage/isd200.c
> +++ b/drivers/usb/storage/isd200.c
> @@ -1238,6 +1238,7 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
> struct us_data *us,
> unsigned long lba;
> unsigned long blockCount;
> unsigned char senseData[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
> + unsigned xfer_len;
>
> memset(ataCdb, 0, sizeof(union ata_cdb));
>
> @@ -1247,8 +1248,9 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
> struct us_data *us,
> US_DEBUGP(" ATA OUT - INQUIRY\n");
>
> /* copy InquiryData */
> + xfer_len = min(sizeof(info->InquiryData), scsi_bufflen(srb));
> usb_stor_set_xfer_buf((unsigned char *) &info->InquiryData,
> - sizeof(info->InquiryData), srb);
> + xfer_len, srb);
> srb->result = SAM_STAT_GOOD;
> sendToTransport = 0;
> break;
> @@ -1257,7 +1259,8 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb,
> struct us_data *us,
> US_DEBUGP(" ATA OUT - SCSIOP_MODE_SENSE\n");
>
> /* Initialize the return buffer */
> - usb_stor_set_xfer_buf(senseData, sizeof(senseData), srb);
> + xfer_len = min(sizeof(senseData), scsi_bufflen(srb));
> + usb_stor_set_xfer_buf(senseData, xfer_len, srb);
>
> if (info->DeviceFlags & DF_MEDIA_STATUS_ENABLED)
> {
> diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c
> index 889622b..038a284 100644
> --- a/drivers/usb/storage/protocol.c
> +++ b/drivers/usb/storage/protocol.c
> @@ -194,7 +194,7 @@ unsigned int usb_stor_access_xfer_buf(unsigned char
> *buffer,
>* and the starting offset within the page, and update
>* the *offset and *index values for the next loop. */
> cnt = 0;
> - while (cnt < buflen) {
> + while (cnt < buflen && sg) {
> struct page *page = sg_page(sg) +
> ((sg->offset + *offset) >> PAGE_SHIFT);
> unsigned int poff =
> @@ -248,9 +248,14 @@ void usb_stor_set_xfer_buf(unsigned char *buffer,
> {
> unsigned int offset = 0;
> struct scatterlist *sg = NULL;
> + unsigned count;
>
> - usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
> + count = usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
> TO_XFER_BUF);
> - if (buflen < srb->request_bufflen)
> - srb->resid = srb->request_bufflen - buflen;
> +
> + /* Check for overflow */
> + if (buflen > scsi_bufflen(srb))
> + count = buflen;
> +
> + scsi_set_resid(srb, scsi_bufflen(srb) - count);
> }
> --
> 1.5.3.3
>
--
Matthew Dharm Home: [EMAIL PROTECTED]
Maintainer, Linux USB Mass Storage Dr
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Thu, Jan 31 2008 at 21:34 +0200, Alan Stern <[EMAIL PROTECTED]> wrote:
> On Thu, 31 Jan 2008, Boaz Harrosh wrote:
>
>> On Thu, Jan 31 2008 at 19:49 +0200, Alan Stern <[EMAIL PROTECTED]> wrote:
>>> On Thu, 31 Jan 2008, Boaz Harrosh wrote:
>>>
@@ -228,9 +228,14 @@ void usb_stor_set_xfer_buf(unsigned char *buffer,
{
unsigned int offset = 0;
struct scatterlist *sg = NULL;
+ unsigned int count;
- usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
+ count = usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
TO_XFER_BUF);
- if (buflen < scsi_bufflen(srb))
- scsi_set_resid(srb, scsi_bufflen(srb) - buflen);
+
+ /* Check for underflow */
+ if (buflen > scsi_bufflen(srb))
+ count = buflen;
+
+ scsi_set_resid(srb, scsi_bufflen(srb) - count);
}
>>> This last "if" statement doesn't look right. And since you know that
>>> count will never be larger than scsi_bufflen(srb), you don't have to
>>> check for underflow at all. Just leave out the "if" and call
>>> scsi_set_resid() directly.
>>>
>>> Alan Stern
>>>
>> I was thinking about that hard. And I disagree. It could be that we have
>> sg-list length (The total of all sg->length) that does not match
>> scsi_bufflen() - More or less.
>
> If that ever happens we'll be in big trouble. Not just here but in
> other places as well.
Other places is other places, but you cannot put a BUG_ON here for other
places. This place here should be correct.
>
>> The code in usb_stor_access_xfer_buf() will
>> now correctly attempt to transfer according to buflen and what ever is
>> available
>> at the passed sg's. Now this can be less or it can be more. SCSI standard
>> defines
>> this as underflow/overflow. When overflow should be reported as negative
>> values.
>> and an error status. (BUT not CHECK_CONDITION)
>
> I don't understand. How could you ever transfer more data than the
> requested amount? The host won't expect to receive it and won't be
> able to handle it when it arrives.
>
> (Furthmore, the USB mass-storage specification does not permit more
> data to be transferred than requested. This fact isn't relevant to our
> discussion because we're talking about situations where the data was
> transferred via a non-standard protocol. But even so...)
That's fine then the transfer will abort with an error condition and will
be handled correctly.
>
>> so the code is actualy
>> if (buflen > scsi_bufflen(srb))
>> scsi_set_resid(srb, scsi_bufflen(srb) - buflen); /* Negative
>> overflow */
>
> Okay, let's assume that both buflen and the sum of the sg-list lengths
> are somehow greater than scsi_bufflen(srb). You still don't know that
> they are equal to each other. All you know is that the amount actually
> transferred is stored in count. Hence the negative overflow value
> should be
>
> scsi_bufflen(srb) - count
>
> and not
>
> scsi_bufflen(srb) - bufflen.
>
> Alan Stern
>
No, it is possible that sg-length was good and count == bufflen,
But the CDB contains scsi_bufflen(srb) and this is an overflow
condition. with your code resid will be Zero.
Also the standard say that we should return the expected overflow and
not the actual on-the-wire overflow.
We could check and truncate beforehand, and mark a flag for overflow
and set negative resid. But since usb_stor_access_xfer_buf() properly
handles that and returns, then this form saves cycles for the common
case and waists cycles for the very rare BUG case of overflow, but is
still correct and safe.
Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.24] bugfix for an overflow condition in usb storage & isd200.c
On Thu, Jan 31 2008 at 21:49 +0200, Matthew Dharm <[EMAIL PROTECTED]> wrote: > No. No no no. > > The ISD200 code was written by the ISD200 developers. I really don't want > to go mucking about changing what commands actually get send to the ISD200 > parts. We have no idea if the will reliably accept a 36-byte INQUIRY. > > Just because it happens to work for a couple of people doesn't mean it > works in the general case. Without guidance from In-System, this is a bad > idea. > > The way to deal with this is to do this via bounce buffering. The two > commands affected (INQUIRY and MODE_SENSE) are low-performance items. > Discarding data from the end of them is also perfectly legal per spec. > > Also, the entire idea of a negative resid makes my head hurt. That sort of > change is in the category of "likely to break something else" which only > expects positive resid values. > > Matt > Please re-inspect the code again. There is no device involved here. The code completely emulates this commands with a driver made up information. the send_to_device is Zero. (Nothing to bounce) There is one more command like that but the standard does not permit choice in that respect. The negative resid is returned by iscsi for ages so I would say the scsi-ml is fine with it. But if you want I can reset the resid and mark an overflow condition in cmnd->status. Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] make __iscsi_complete_pdu() static
Adrian Bunk wrote: __iscsi_complete_pdu() can now become static. Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> Looks good. Thanks. Acked-by: Mike Christie <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.24] bugfix for an overflow condition in usb storage & isd200.c
On Thu, Jan 31, 2008 at 10:05:19PM +0200, Boaz Harrosh wrote: > On Thu, Jan 31 2008 at 21:49 +0200, Matthew Dharm <[EMAIL PROTECTED]> wrote: > > No. No no no. > > Please re-inspect the code again. > There is no device involved here. The code completely emulates this commands > with a driver made up information. the send_to_device is Zero. > (Nothing to bounce) Okay, I see what you're doing there, and I can live with that. > The negative resid is returned by iscsi for ages so I would say the scsi-ml > is fine with it. But if you want I can reset the resid and mark an overflow > condition in cmnd->status. I'd be fine if one of the SCSI guru's would comment on this as being acceptable. Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver Hey Chief. We've figured out how to save the technical department. We need to be committed. -- The Techs User Friendly, 1/22/1998 pgpCejgQ64dmT.pgp Description: PGP signature
[PATCH 1/3] iscsi: extended cdb support
Support for extended CDBs in iscsi.
All we need is to check if command spills over 16 bytes then allocate
an iscsi-extended-header for the leftovers.
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
drivers/scsi/iscsi_tcp.c |2 +-
drivers/scsi/libiscsi.c| 56
include/scsi/iscsi_proto.h |6 +++-
3 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 8a17867..ee7acfa 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -1975,7 +1975,7 @@ static struct iscsi_transport iscsi_tcp_transport = {
.host_template = &iscsi_sht,
.conndata_size = sizeof(struct iscsi_conn),
.max_conn = 1,
- .max_cmd_len= 16,
+ .max_cmd_len= 260,
/* session management */
.create_session = iscsi_tcp_session_create,
.destroy_session= iscsi_tcp_session_destroy,
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index cfffabd..498cea3 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -137,6 +137,45 @@ static int iscsi_add_hdr(struct iscsi_cmd_task *ctask,
unsigned len)
return 0;
}
+/*
+ * make an extended cdb AHS
+ */
+static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task *ctask)
+{
+ struct scsi_cmnd *cmd = ctask->sc;
+ unsigned rlen, pad_len;
+ unsigned short ahslength;
+ struct iscsi_ecdb_ahdr *ecdb_ahdr;
+ int rc;
+
+ ecdb_ahdr = iscsi_next_hdr(ctask);
+ rlen = cmd->cmd_len - ISCSI_CDB_SIZE;
+
+ BUG_ON(rlen > sizeof(ecdb_ahdr->ecdb));
+ ahslength = rlen + sizeof(ecdb_ahdr->reserved);
+
+ pad_len = iscsi_padding(rlen);
+
+ rc = iscsi_add_hdr(ctask, sizeof(ecdb_ahdr->ahslength) +
+ sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
+ if (rc)
+ return rc;
+
+ if (pad_len)
+ memset(&ecdb_ahdr->ecdb[rlen], 0, pad_len);
+
+ ecdb_ahdr->ahslength = cpu_to_be16(ahslength);
+ ecdb_ahdr->ahstype = ISCSI_AHSTYPE_CDB;
+ ecdb_ahdr->reserved = 0;
+ memcpy(ecdb_ahdr->ecdb, cmd->cmnd + ISCSI_CDB_SIZE, rlen);
+
+ debug_scsi("iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
+ "rlen %d pad_len %d ahs_length %d iscsi_headers_size %u\n",
+ cmd->cmd_len, rlen, pad_len, ahslength, ctask->hdr_len);
+
+ return 0;
+}
+
/**
* iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
* @ctask: iscsi cmd task
@@ -150,7 +189,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
struct iscsi_session *session = conn->session;
struct iscsi_cmd *hdr = ctask->hdr;
struct scsi_cmnd *sc = ctask->sc;
- unsigned hdrlength;
+ unsigned hdrlength, cmd_len;
int rc;
ctask->hdr_len = 0;
@@ -165,10 +204,17 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
hdr->cmdsn = cpu_to_be32(session->cmdsn);
session->cmdsn++;
hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
- memcpy(hdr->cdb, sc->cmnd, sc->cmd_len);
- if (sc->cmd_len < MAX_COMMAND_SIZE)
- memset(&hdr->cdb[sc->cmd_len], 0,
- MAX_COMMAND_SIZE - sc->cmd_len);
+ cmd_len = sc->cmd_len;
+ if (cmd_len < ISCSI_CDB_SIZE)
+ memset(&hdr->cdb[cmd_len], 0,
+ ISCSI_CDB_SIZE - cmd_len);
+ else if (cmd_len > ISCSI_CDB_SIZE) {
+ rc = iscsi_prep_ecdb_ahs(ctask);
+ if (rc)
+ return rc;
+ cmd_len = ISCSI_CDB_SIZE;
+ }
+ memcpy(hdr->cdb, sc->cmnd, cmd_len);
ctask->imm_count = 0;
if (sc->sc_data_direction == DMA_TO_DEVICE) {
diff --git a/include/scsi/iscsi_proto.h b/include/scsi/iscsi_proto.h
index 5ffec8a..e0593bf 100644
--- a/include/scsi/iscsi_proto.h
+++ b/include/scsi/iscsi_proto.h
@@ -112,6 +112,7 @@ struct iscsi_ahs_hdr {
#define ISCSI_AHSTYPE_CDB 1
#define ISCSI_AHSTYPE_RLENGTH 2
+#define ISCSI_CDB_SIZE 16
/* iSCSI PDU Header */
struct iscsi_cmd {
@@ -125,7 +126,7 @@ struct iscsi_cmd {
__be32 data_length;
__be32 cmdsn;
__be32 exp_statsn;
- uint8_t cdb[16];/* SCSI Command Block */
+ uint8_t cdb[ISCSI_CDB_SIZE];/* SCSI Command Block */
/* Additional Data (Command Dependent) */
};
@@ -154,7 +155,8 @@ struct iscsi_ecdb_ahdr {
__be16 ahslength; /* CDB length - 15, including reserved byte */
uint8_t ahstype;
uint8_t reserved;
- uint8_t ecdb[260 - 16]; /* 4-byte aligned extended CDB spillover */
+ /* 4-byte aligned extended CDB spillover */
+ uint8_t ecdb[260 - ISCSI_CDB_SIZE];
};
/* SCSI Response Header */
--
1.5.3.3
-
To unsubscribe from this list: send the line "unsubscribe linux-sc
[PATCH 2/3] iscsi: bidi support - libiscsi
iscsi bidi support at the generic libiscsi level
- prepare the additional bidi_read rlength header.
- access the right scsi_in() and/or scsi_out() side of things.
also for resid.
- Handle BIDI underflow overflow from target
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
drivers/scsi/libiscsi.c | 82 --
1 files changed, 71 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 498cea3..df81f6f 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -176,6 +176,31 @@ static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task
*ctask)
return 0;
}
+static int iscsi_prep_bidi_ahs(struct iscsi_cmd_task *ctask)
+{
+ struct scsi_cmnd *sc = ctask->sc;
+ struct iscsi_rlength_ahdr *rlen_ahdr;
+ int rc;
+
+ rlen_ahdr = iscsi_next_hdr(ctask);
+ rc = iscsi_add_hdr(ctask, sizeof(*rlen_ahdr));
+ if (rc)
+ return rc;
+
+ rlen_ahdr->ahslength =
+ cpu_to_be16(sizeof(rlen_ahdr->read_length) +
+ sizeof(rlen_ahdr->reserved));
+ rlen_ahdr->ahstype = ISCSI_AHSTYPE_RLENGTH;
+ rlen_ahdr->reserved = 0;
+ rlen_ahdr->read_length = cpu_to_be32(scsi_in(sc)->length);
+
+ debug_scsi("bidi-in rlen_ahdr->read_length(%d) "
+ "rlen_ahdr->ahslength(%d)\n",
+ be32_to_cpu(rlen_ahdr->read_length),
+ be16_to_cpu(rlen_ahdr->ahslength));
+ return 0;
+}
+
/**
* iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
* @ctask: iscsi cmd task
@@ -200,7 +225,6 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
hdr->flags = ISCSI_ATTR_SIMPLE;
int_to_scsilun(sc->device->lun, (struct scsi_lun *)hdr->lun);
hdr->itt = build_itt(ctask->itt, session->age);
- hdr->data_length = cpu_to_be32(scsi_bufflen(sc));
hdr->cmdsn = cpu_to_be32(session->cmdsn);
session->cmdsn++;
hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
@@ -217,7 +241,15 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
memcpy(hdr->cdb, sc->cmnd, cmd_len);
ctask->imm_count = 0;
+ if (scsi_bidi_cmnd(sc)) {
+ hdr->flags |= ISCSI_FLAG_CMD_READ;
+ rc = iscsi_prep_bidi_ahs(ctask);
+ if (rc)
+ return rc;
+ }
if (sc->sc_data_direction == DMA_TO_DEVICE) {
+ unsigned out_len = scsi_out(sc)->length;
+ hdr->data_length = cpu_to_be32(out_len);
hdr->flags |= ISCSI_FLAG_CMD_WRITE;
/*
* Write counters:
@@ -238,19 +270,19 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
ctask->unsol_datasn = 0;
if (session->imm_data_en) {
- if (scsi_bufflen(sc) >= session->first_burst)
+ if (out_len >= session->first_burst)
ctask->imm_count = min(session->first_burst,
conn->max_xmit_dlength);
else
- ctask->imm_count = min(scsi_bufflen(sc),
+ ctask->imm_count = min(out_len,
conn->max_xmit_dlength);
hton24(hdr->dlength, ctask->imm_count);
} else
zero_data(hdr->dlength);
if (!session->initial_r2t_en) {
- ctask->unsol_count = min((session->first_burst),
- (scsi_bufflen(sc))) - ctask->imm_count;
+ ctask->unsol_count = min(session->first_burst, out_len)
+- ctask->imm_count;
ctask->unsol_offset = ctask->imm_count;
}
@@ -260,6 +292,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
} else {
hdr->flags |= ISCSI_FLAG_CMD_FINAL;
zero_data(hdr->dlength);
+ hdr->data_length = cpu_to_be32(scsi_in(sc)->length);
if (sc->sc_data_direction == DMA_FROM_DEVICE)
hdr->flags |= ISCSI_FLAG_CMD_READ;
@@ -278,10 +311,12 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
return EIO;
conn->scsicmd_pdus_cnt++;
- debug_scsi("iscsi prep [%s cid %d sc %p cdb 0x%x itt 0x%x len %d "
- "cmdsn %d win %d]\n",
- sc->sc_data_direction == DMA_TO_DEVICE ? "write" : "read",
- conn->id, sc, sc->cmnd[0], ctask->itt, scsi_bufflen(sc),
+ debug_scsi("iscsi prep [%s cid %d sc %p cdb 0x%x itt 0x%x "
+ "len %d bidi_len %d cmdsn %d win %d]\n",
+ scsi_bidi_cmnd(sc) ? "bidirectional" :
+
Re: [PATCH] bugfix for an underflow condition in usb storage & isd200.c
On Thu, 31 Jan 2008, Boaz Harrosh wrote: > >> The code in usb_stor_access_xfer_buf() will > >> now correctly attempt to transfer according to buflen and what ever is > >> available > >> at the passed sg's. Now this can be less or it can be more. SCSI standard > >> defines > >> this as underflow/overflow. When overflow should be reported as negative > >> values. > >> and an error status. (BUT not CHECK_CONDITION) > > > > I don't understand. How could you ever transfer more data than the > > requested amount? The host won't expect to receive it and won't be > > able to handle it when it arrives. > > > > (Furthmore, the USB mass-storage specification does not permit more > > data to be transferred than requested. This fact isn't relevant to our > > discussion because we're talking about situations where the data was > > transferred via a non-standard protocol. But even so...) > That's fine then the transfer will abort with an error condition and will > be handled correctly. I still don't understand. Can you explain exactly what an overflow condition (negative residue) really means? Does it mean that the device had more data available than the host asked for? If it does, then you don't need to change the isd200 code at all. The changes to protocol.c will be enough. However you should realize that usb-storage, as it stands, won't work properly with negative residues. Look at usb_stor_Bulk_transport() in transport.c. The residue variable is declared to be unsigned int. There's also code later on in the routine which assumes the residue is never negative. Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/11] qla2xxx: Update version number to 8.02.00-k8.
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]> --- drivers/scsi/qla2xxx/qla_version.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_version.h b/drivers/scsi/qla2xxx/qla_version.h index 2c2f6b4..c5742cc 100644 --- a/drivers/scsi/qla2xxx/qla_version.h +++ b/drivers/scsi/qla2xxx/qla_version.h @@ -7,7 +7,7 @@ /* * Driver version */ -#define QLA2XXX_VERSION "8.02.00-k7" +#define QLA2XXX_VERSION "8.02.00-k8" #define QLA_DRIVER_MAJOR_VER 8 #define QLA_DRIVER_MINOR_VER 2 -- 1.5.4.rc5.5.gab98 - 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 09/11] qla2xxx: Access the proper 'physical' port in FC-transport callbacks.
From: Seokmann Ju <[EMAIL PROTECTED]>
For following fc_host specific attributes, vports rely on the pport.
So, this patch changed way to access the data for those attributes so that
they can access pport's.
- get_host_speed (speed)
- get_host_port_state (port_state)
- get_host_port_type (port_type)
- get_fc_host_stats
Also, added PORT_SPEED_8GB case in the speed attribute for 8Gb HBAs.
Signed-Off-by: Seokmann Ju <[EMAIL PROTECTED]>
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_attr.c | 11 +++
drivers/scsi/qla2xxx/qla_def.h|2 --
drivers/scsi/qla2xxx/qla_inline.h |7 +++
3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 1dd8591..4894dc8 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -848,7 +848,7 @@ qla2x00_get_host_port_id(struct Scsi_Host *shost)
static void
qla2x00_get_host_speed(struct Scsi_Host *shost)
{
- scsi_qla_host_t *ha = shost_priv(shost);
+ scsi_qla_host_t *ha = to_qla_parent(shost_priv(shost));
uint32_t speed = 0;
switch (ha->link_data_rate) {
@@ -861,6 +861,9 @@ qla2x00_get_host_speed(struct Scsi_Host *shost)
case PORT_SPEED_4GB:
speed = 4;
break;
+ case PORT_SPEED_8GB:
+ speed = 8;
+ break;
}
fc_host_speed(shost) = speed;
}
@@ -868,7 +871,7 @@ qla2x00_get_host_speed(struct Scsi_Host *shost)
static void
qla2x00_get_host_port_type(struct Scsi_Host *shost)
{
- scsi_qla_host_t *ha = shost_priv(shost);
+ scsi_qla_host_t *ha = to_qla_parent(shost_priv(shost));
uint32_t port_type = FC_PORTTYPE_UNKNOWN;
switch (ha->current_topology) {
@@ -978,7 +981,7 @@ qla2x00_issue_lip(struct Scsi_Host *shost)
static struct fc_host_statistics *
qla2x00_get_fc_host_stats(struct Scsi_Host *shost)
{
- scsi_qla_host_t *ha = shost_priv(shost);
+ scsi_qla_host_t *ha = to_qla_parent(shost_priv(shost));
int rval;
struct link_statistics *stats;
dma_addr_t stats_dma;
@@ -1062,7 +1065,7 @@ qla2x00_get_host_fabric_name(struct Scsi_Host *shost)
static void
qla2x00_get_host_port_state(struct Scsi_Host *shost)
{
- scsi_qla_host_t *ha = shost_priv(shost);
+ scsi_qla_host_t *ha = to_qla_parent(shost_priv(shost));
if (!ha->flags.online)
fc_host_port_state(shost) = FC_PORTSTATE_OFFLINE;
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 6f129da..42500bc 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -2041,8 +2041,6 @@ typedef struct vport_params {
#define VP_RET_CODE_NO_MEM 5
#define VP_RET_CODE_NOT_FOUND 6
-#define to_qla_parent(x) (((x)->parent) ? (x)->parent : (x))
-
/*
* ISP operations
*/
diff --git a/drivers/scsi/qla2xxx/qla_inline.h
b/drivers/scsi/qla2xxx/qla_inline.h
index 8e3b044..5d1a3f7 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -119,6 +119,13 @@ static __inline__ void
qla2x00_check_fabric_devices(scsi_qla_host_t *ha)
qla2x00_get_firmware_state(ha, &fw_state);
}
+static __inline__ scsi_qla_host_t * to_qla_parent(scsi_qla_host_t *);
+static __inline__ scsi_qla_host_t *
+to_qla_parent(scsi_qla_host_t *ha)
+{
+ return ha->parent ? ha->parent : ha;
+}
+
/**
* qla2x00_issue_marker() - Issue a Marker IOCB if necessary.
* @ha: HA context
--
1.5.4.rc5.5.gab98
-
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 07/11] qla2xxx: Move RISC-interrupt-register modifications to qla2x00_request_irqs().
There's no functional change involved with this update, instead
it simply migrates the "set cleared interrupt state" codes to a
more approprate method, qla2x00_request_irqs(), and cleans-up the
driver's probe() logic.
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_isr.c | 27 ++-
drivers/scsi/qla2xxx/qla_os.c | 18 --
2 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 642a0c3..14e6f22 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1815,6 +1815,8 @@ int
qla2x00_request_irqs(scsi_qla_host_t *ha)
{
int ret;
+ device_reg_t __iomem *reg = ha->iobase;
+ unsigned long flags;
/* If possible, enable MSI-X. */
if (!IS_QLA2432(ha) && !IS_QLA2532(ha))
@@ -1846,7 +1848,7 @@ qla2x00_request_irqs(scsi_qla_host_t *ha)
DEBUG2(qla_printk(KERN_INFO, ha,
"MSI-X: Enabled (0x%X, 0x%X).\n", ha->chip_revision,
ha->fw_attributes));
- return ret;
+ goto clear_risc_ints;
}
qla_printk(KERN_WARNING, ha,
"MSI-X: Falling back-to INTa mode -- %d.\n", ret);
@@ -1864,15 +1866,30 @@ skip_msi:
ret = request_irq(ha->pdev->irq, ha->isp_ops->intr_handler,
IRQF_DISABLED|IRQF_SHARED, QLA2XXX_DRIVER_NAME, ha);
- if (!ret) {
- ha->flags.inta_enabled = 1;
- ha->host->irq = ha->pdev->irq;
- } else {
+ if (ret) {
qla_printk(KERN_WARNING, ha,
"Failed to reserve interrupt %d already in use.\n",
ha->pdev->irq);
+ goto fail;
+ }
+ ha->flags.inta_enabled = 1;
+ ha->host->irq = ha->pdev->irq;
+clear_risc_ints:
+
+ ha->isp_ops->disable_intrs(ha);
+ spin_lock_irqsave(&ha->hardware_lock, flags);
+ if (IS_FWI2_CAPABLE(ha)) {
+ WRT_REG_DWORD(®->isp24.hccr, HCCRX_CLR_HOST_INT);
+ WRT_REG_DWORD(®->isp24.hccr, HCCRX_CLR_RISC_INT);
+ } else {
+ WRT_REG_WORD(®->isp.semaphore, 0);
+ WRT_REG_WORD(®->isp.hccr, HCCR_CLR_RISC_INT);
+ WRT_REG_WORD(®->isp.hccr, HCCR_CLR_HOST_INT);
}
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
+ ha->isp_ops->enable_intrs(ha);
+fail:
return ret;
}
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index f129c69..4b4c32f 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1576,10 +1576,8 @@ static int __devinit
qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
{
int ret = -ENODEV;
- device_reg_t __iomem *reg;
struct Scsi_Host *host;
scsi_qla_host_t *ha;
- unsigned long flags = 0;
char pci_info[30];
char fw_str[30];
struct scsi_host_template *sht;
@@ -1762,22 +1760,6 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct
pci_device_id *id)
DEBUG2(printk("DEBUG: detect hba %ld at address = %p\n",
ha->host_no, ha));
- ha->isp_ops->disable_intrs(ha);
-
- spin_lock_irqsave(&ha->hardware_lock, flags);
- reg = ha->iobase;
- if (IS_FWI2_CAPABLE(ha)) {
- WRT_REG_DWORD(®->isp24.hccr, HCCRX_CLR_HOST_INT);
- WRT_REG_DWORD(®->isp24.hccr, HCCRX_CLR_RISC_INT);
- } else {
- WRT_REG_WORD(®->isp.semaphore, 0);
- WRT_REG_WORD(®->isp.hccr, HCCR_CLR_RISC_INT);
- WRT_REG_WORD(®->isp.hccr, HCCR_CLR_HOST_INT);
- }
- spin_unlock_irqrestore(&ha->hardware_lock, flags);
-
- ha->isp_ops->enable_intrs(ha);
-
pci_set_drvdata(pdev, ha);
ha->flags.init_done = 1;
--
1.5.4.rc5.5.gab98
-
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 10/11] qla2xxx: Correct issue where incorrect init-fw mailbox command was used on non-NPIV capable ISPs.
BIT_2 of the firmware attributes is only valid on FW-interface-2
type HBAs. Code in commit
c48339decceec8e011498b0fc4c7c7d8b2ea06c1 would cause the
incorrect initialize-firmware mailbox command to be issued for
non-NPIV capable ISPs. Correct this by reverting to previously
used (and correct) pre-condition 'if' check.
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_mbx.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 0c10c0b..99d29ff 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -980,7 +980,7 @@ qla2x00_init_firmware(scsi_qla_host_t *ha, uint16_t size)
DEBUG11(printk("qla2x00_init_firmware(%ld): entered.\n",
ha->host_no));
- if (ha->fw_attributes & BIT_2)
+ if (ha->flags.npiv_supported)
mcp->mb[0] = MBC_MID_INITIALIZE_FIRMWARE;
else
mcp->mb[0] = MBC_INITIALIZE_FIRMWARE;
--
1.5.4.rc5.5.gab98
-
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 04/11] qla2xxx: Clear EFT buffer before firmware reinitialization.
To insure that there is no stale data present during EFT
re-registration.
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_init.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 2e51fa8..97063cb 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -3259,6 +3259,7 @@ qla2x00_abort_isp(scsi_qla_host_t *ha)
clear_bit(ISP_ABORT_RETRY, &ha->dpc_flags);
if (ha->eft) {
+ memset(ha->eft, 0, EFT_SIZE);
rval = qla2x00_enable_eft_trace(ha,
ha->eft_dma, EFT_NUM_BUFFERS);
if (rval) {
--
1.5.4.rc5.5.gab98
-
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 03/11] qla2xxx: Cleanup any outstanding SRB resources during shutdown.
Refactor SRB-failure completion codes in the process. Also,
signal the DPC routine to complete sooner as backend processing
at shutdown-time is superflous.
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_gbl.h |1 +
drivers/scsi/qla2xxx/qla_init.c | 16 +---
drivers/scsi/qla2xxx/qla_os.c | 30 ++
3 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index ba35fc2..193f688 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -66,6 +66,7 @@ extern int ql2xqfullrampup;
extern int num_hosts;
extern int qla2x00_loop_reset(scsi_qla_host_t *);
+extern void qla2x00_abort_all_cmds(scsi_qla_host_t *, int);
/*
* Global Functions in qla_mid.c source file.
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index d0633ca..2e51fa8 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -3213,9 +3213,6 @@ int
qla2x00_abort_isp(scsi_qla_host_t *ha)
{
int rval;
- unsigned long flags = 0;
- uint16_t cnt;
- srb_t *sp;
uint8_tstatus = 0;
if (ha->flags.online) {
@@ -3236,19 +3233,8 @@ qla2x00_abort_isp(scsi_qla_host_t *ha)
LOOP_DOWN_TIME);
}
- spin_lock_irqsave(&ha->hardware_lock, flags);
/* Requeue all commands in outstanding command list. */
- for (cnt = 1; cnt < MAX_OUTSTANDING_COMMANDS; cnt++) {
- sp = ha->outstanding_cmds[cnt];
- if (sp) {
- ha->outstanding_cmds[cnt] = NULL;
- sp->flags = 0;
- sp->cmd->result = DID_RESET << 16;
- sp->cmd->host_scribble = (unsigned char *)NULL;
- qla2x00_sp_compl(ha, sp);
- }
- }
- spin_unlock_irqrestore(&ha->hardware_lock, flags);
+ qla2x00_abort_all_cmds(ha, DID_RESET << 16);
ha->isp_ops->get_flash_version(ha, ha->request_ring);
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index bbdfd81..e2adfce 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1117,6 +1117,27 @@ qla2x00_device_reset(scsi_qla_host_t *ha, fc_port_t
*reset_fcport)
return ha->isp_ops->abort_target(reset_fcport);
}
+void
+qla2x00_abort_all_cmds(scsi_qla_host_t *ha, int res)
+{
+ int cnt;
+ unsigned long flags;
+ srb_t *sp;
+
+ spin_lock_irqsave(&ha->hardware_lock, flags);
+ for (cnt = 1; cnt < MAX_OUTSTANDING_COMMANDS; cnt++) {
+ sp = ha->outstanding_cmds[cnt];
+ if (sp) {
+ ha->outstanding_cmds[cnt] = NULL;
+ sp->flags = 0;
+ sp->cmd->result = res;
+ sp->cmd->host_scribble = (unsigned char *)NULL;
+ qla2x00_sp_compl(ha, sp);
+ }
+ }
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
+}
+
static int
qla2xxx_slave_alloc(struct scsi_device *sdev)
{
@@ -1601,6 +1622,7 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct
pci_device_id *id)
sprintf(ha->host_str, "%s_%ld", QLA2XXX_DRIVER_NAME, ha->host_no);
ha->parent = NULL;
ha->bars = bars;
+ spin_lock_init(&ha->hardware_lock);
/* Set ISP-type information. */
qla2x00_set_isp_flags(ha);
@@ -1614,8 +1636,6 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct
pci_device_id *id)
"Found an ISP%04X, irq %d, iobase 0x%p\n", pdev->device, pdev->irq,
ha->iobase);
- spin_lock_init(&ha->hardware_lock);
-
ha->prev_topology = 0;
ha->init_cb_size = sizeof(init_cb_t);
ha->mgmt_svr_loop_id = MANAGEMENT_SERVER + ha->vp_idx;
@@ -1841,10 +1861,14 @@ qla2x00_remove_one(struct pci_dev *pdev)
static void
qla2x00_free_device(scsi_qla_host_t *ha)
{
+ qla2x00_abort_all_cmds(ha, DID_NO_CONNECT << 16);
+
/* Disable timer */
if (ha->timer_active)
qla2x00_stop_timer(ha);
+ ha->flags.online = 0;
+
/* Kill the kernel thread for this host */
if (ha->dpc_thread) {
struct task_struct *t = ha->dpc_thread;
@@ -1863,8 +1887,6 @@ qla2x00_free_device(scsi_qla_host_t *ha)
if (ha->eft)
qla2x00_disable_eft_trace(ha);
- ha->flags.online = 0;
-
/* Stop currently executing firmware. */
qla2x00_try_to_stop_firmware(ha);
--
1.5.4.rc5.5.gab98
-
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 01/11] qla2xxx: Correct resource_size_t usages.
Hmm, it looks like the conversion to resource_size_t usage
(3776541d8a46347a4924353a192c6ce4a3d04e2e) requires some additional
fixups to cleanup the structure-pointer castings used during IO mapped
accesses to the chip.
There's only a small number of locations, where the driver uses IO
mapped accesses to the hardware, the patch below should take care of
it without introducing to many structural changes to code flow.
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_sup.c | 36 +++-
1 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_sup.c b/drivers/scsi/qla2xxx/qla_sup.c
index b68fb73..26822c8 100644
--- a/drivers/scsi/qla2xxx/qla_sup.c
+++ b/drivers/scsi/qla2xxx/qla_sup.c
@@ -893,6 +893,8 @@ qla2x00_flip_colors(scsi_qla_host_t *ha, uint16_t *pflags)
}
}
+#define PIO_REG(h, r) ((h)->pio_address + offsetof(struct device_reg_2xxx, r))
+
void
qla2x00_beacon_blink(struct scsi_qla_host *ha)
{
@@ -902,15 +904,12 @@ qla2x00_beacon_blink(struct scsi_qla_host *ha)
unsigned long flags;
struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
- if (ha->pio_address)
- reg = (struct device_reg_2xxx __iomem *)ha->pio_address;
-
spin_lock_irqsave(&ha->hardware_lock, flags);
/* Save the Original GPIOE. */
if (ha->pio_address) {
- gpio_enable = RD_REG_WORD_PIO(®->gpioe);
- gpio_data = RD_REG_WORD_PIO(®->gpiod);
+ gpio_enable = RD_REG_WORD_PIO(PIO_REG(ha, gpioe));
+ gpio_data = RD_REG_WORD_PIO(PIO_REG(ha, gpiod));
} else {
gpio_enable = RD_REG_WORD(®->gpioe);
gpio_data = RD_REG_WORD(®->gpiod);
@@ -920,7 +919,7 @@ qla2x00_beacon_blink(struct scsi_qla_host *ha)
gpio_enable |= GPIO_LED_MASK;
if (ha->pio_address) {
- WRT_REG_WORD_PIO(®->gpioe, gpio_enable);
+ WRT_REG_WORD_PIO(PIO_REG(ha, gpioe), gpio_enable);
} else {
WRT_REG_WORD(®->gpioe, gpio_enable);
RD_REG_WORD(®->gpioe);
@@ -936,7 +935,7 @@ qla2x00_beacon_blink(struct scsi_qla_host *ha)
/* Set the modified gpio_data values */
if (ha->pio_address) {
- WRT_REG_WORD_PIO(®->gpiod, gpio_data);
+ WRT_REG_WORD_PIO(PIO_REG(ha, gpiod), gpio_data);
} else {
WRT_REG_WORD(®->gpiod, gpio_data);
RD_REG_WORD(®->gpiod);
@@ -962,14 +961,11 @@ qla2x00_beacon_on(struct scsi_qla_host *ha)
return QLA_FUNCTION_FAILED;
}
- if (ha->pio_address)
- reg = (struct device_reg_2xxx __iomem *)ha->pio_address;
-
/* Turn off LEDs. */
spin_lock_irqsave(&ha->hardware_lock, flags);
if (ha->pio_address) {
- gpio_enable = RD_REG_WORD_PIO(®->gpioe);
- gpio_data = RD_REG_WORD_PIO(®->gpiod);
+ gpio_enable = RD_REG_WORD_PIO(PIO_REG(ha, gpioe));
+ gpio_data = RD_REG_WORD_PIO(PIO_REG(ha, gpiod));
} else {
gpio_enable = RD_REG_WORD(®->gpioe);
gpio_data = RD_REG_WORD(®->gpiod);
@@ -978,7 +974,7 @@ qla2x00_beacon_on(struct scsi_qla_host *ha)
/* Set the modified gpio_enable values. */
if (ha->pio_address) {
- WRT_REG_WORD_PIO(®->gpioe, gpio_enable);
+ WRT_REG_WORD_PIO(PIO_REG(ha, gpioe), gpio_enable);
} else {
WRT_REG_WORD(®->gpioe, gpio_enable);
RD_REG_WORD(®->gpioe);
@@ -987,7 +983,7 @@ qla2x00_beacon_on(struct scsi_qla_host *ha)
/* Clear out previously set LED colour. */
gpio_data &= ~GPIO_LED_MASK;
if (ha->pio_address) {
- WRT_REG_WORD_PIO(®->gpiod, gpio_data);
+ WRT_REG_WORD_PIO(PIO_REG(ha, gpiod), gpio_data);
} else {
WRT_REG_WORD(®->gpiod, gpio_data);
RD_REG_WORD(®->gpiod);
@@ -1244,13 +1240,12 @@ qla2x00_read_flash_byte(scsi_qla_host_t *ha, uint32_t
addr)
if (ha->pio_address) {
uint16_t data2;
- reg = (struct device_reg_2xxx __iomem *)ha->pio_address;
- WRT_REG_WORD_PIO(®->flash_address, (uint16_t)addr);
+ WRT_REG_WORD_PIO(PIO_REG(ha, flash_address), (uint16_t)addr);
do {
- data = RD_REG_WORD_PIO(®->flash_data);
+ data = RD_REG_WORD_PIO(PIO_REG(ha, flash_data));
barrier();
cpu_relax();
- data2 = RD_REG_WORD_PIO(®->flash_data);
+ data2 = RD_REG_WORD_PIO(PIO_REG(ha, flash_data));
} while (data != data2);
} else {
WRT_REG_WORD(®->flash_address, (uint16_t)addr);
@@ -1304,9 +1299,8 @@ qla2x00_write_flash_byte(scsi_qla_host_t *ha, uint32_t
addr, uint8_t data)
/* Always per
[PATCH 3/3] iscsi: bidi support - iscsi_tcp
bidi support for iscsi_tcp
- access the right scsi_in() and/or scsi_out() side of things.
also for resid
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
drivers/scsi/iscsi_tcp.c | 31 +--
1 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index ee7acfa..073dea7 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -528,6 +528,7 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
struct iscsi_session *session = conn->session;
struct scsi_cmnd *sc = ctask->sc;
int datasn = be32_to_cpu(rhdr->datasn);
+ unsigned total_in_length = scsi_in(sc)->length;
iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
if (tcp_conn->in.datalen == 0)
@@ -542,10 +543,10 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
tcp_ctask->exp_datasn++;
tcp_ctask->data_offset = be32_to_cpu(rhdr->offset);
- if (tcp_ctask->data_offset + tcp_conn->in.datalen > scsi_bufflen(sc)) {
+ if (tcp_ctask->data_offset + tcp_conn->in.datalen > total_in_length) {
debug_tcp("%s: data_offset(%d) + data_len(%d) >
total_length_in(%d)\n",
__FUNCTION__, tcp_ctask->data_offset,
- tcp_conn->in.datalen, scsi_bufflen(sc));
+ tcp_conn->in.datalen, total_in_length);
return ISCSI_ERR_DATA_OFFSET;
}
@@ -558,8 +559,8 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
if (res_count > 0 &&
(rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
-res_count <= scsi_bufflen(sc)))
- scsi_set_resid(sc, res_count);
+res_count <= total_in_length))
+ scsi_in(sc)->resid = res_count;
else
sc->result = (DID_BAD_TARGET << 16) |
rhdr->cmd_status;
@@ -670,11 +671,11 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
r2t->data_length, session->max_burst);
r2t->data_offset = be32_to_cpu(rhdr->data_offset);
- if (r2t->data_offset + r2t->data_length > scsi_bufflen(ctask->sc)) {
+ if (r2t->data_offset + r2t->data_length > scsi_out(ctask->sc)->length) {
iscsi_conn_printk(KERN_ERR, conn,
"invalid R2T with data len %u at offset %u "
"and total length %d\n", r2t->data_length,
- r2t->data_offset, scsi_bufflen(ctask->sc));
+ r2t->data_offset,
scsi_out(ctask->sc)->length);
__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
sizeof(void*));
return ISCSI_ERR_DATALEN;
@@ -771,6 +772,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct
iscsi_hdr *hdr)
if (tcp_conn->in.datalen) {
struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
struct hash_desc *rx_hash = NULL;
+ struct scsi_data_buffer *sdb = scsi_in(ctask->sc);
/*
* Setup copy of Data-In into the Scsi_Cmnd
@@ -788,8 +790,8 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct
iscsi_hdr *hdr)
tcp_ctask->data_offset,
tcp_conn->in.datalen);
return iscsi_segment_seek_sg(&tcp_conn->in.segment,
-scsi_sglist(ctask->sc),
-scsi_sg_count(ctask->sc),
+sdb->table.sgl,
+sdb->table.nents,
tcp_ctask->data_offset,
tcp_conn->in.datalen,
iscsi_tcp_process_data_in,
@@ -1332,7 +1334,8 @@ iscsi_tcp_ctask_init(struct iscsi_cmd_task *ctask)
return 0;
/* If we have immediate data, attach a payload */
- err = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc), scsi_sg_count(sc),
+ err = iscsi_tcp_send_data_prep(conn, scsi_out(sc)->table.sgl,
+ scsi_out(sc)->table.nents,
0, ctask->imm_count);
if (err)
return err;
@@ -1386,6 +1389,7 @@ iscsi_tcp_ctask_xmit(struct iscsi_conn *conn, struct
iscsi_cmd_task *ctask)
{
struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
struct scsi_cmnd *sc = ctask->sc;
+ struct
[PATCH 0/11] qla2xxx: updates/fixes for 2.6.24 [8.02.00-k8].
Here's a batch of fixes for 2.6.24. Changes include: - Correct resource_size_t usages. - Add MODULE_FIRMWARE hint for ISP25XX firmware. - Cleanup any outstanding SRB resources during shutdown. - Clear EFT buffer before firmware reinitialization. - Cleanse memory allocation logic during probe. - Consolidate RISC-parity enablement codes. - Move RISC-interrupt-register modifications to qla2x00_request_irqs(). - Correct issue where vport-state was not updated during an ISP_ABORT_NEEDED requst. - Access the proper 'physical' port in FC-transport callbacks. - Correct issue where incorrect init-fw mailbox command was used on non-NPIV capable ISPs. - Update version number to 8.02.00-k8. Diffstat: drivers/scsi/qla2xxx/qla_attr.c| 24 ++- drivers/scsi/qla2xxx/qla_def.h |2 - drivers/scsi/qla2xxx/qla_gbl.h |1 + drivers/scsi/qla2xxx/qla_init.c| 87 +++-- drivers/scsi/qla2xxx/qla_inline.h |7 + drivers/scsi/qla2xxx/qla_isr.c | 27 ++- drivers/scsi/qla2xxx/qla_mbx.c |2 +- drivers/scsi/qla2xxx/qla_os.c | 404 +--- drivers/scsi/qla2xxx/qla_sup.c | 36 ++-- drivers/scsi/qla2xxx/qla_version.h |2 +- 10 files changed, 233 insertions(+), 359 deletions(-) Regards, Andrew Vasquez QLogic Corporation - 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 06/11] qla2xxx: Consolidate RISC-parity enablement codes.
Collapse duplicate codes called during probe() and RISC-reset
into qla2x00_setup_chip().
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_init.c | 70 +-
drivers/scsi/qla2xxx/qla_os.c | 12 ---
2 files changed, 24 insertions(+), 58 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 97063cb..d5c7853 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -925,6 +925,16 @@ qla2x00_setup_chip(scsi_qla_host_t *ha)
{
int rval;
uint32_t srisc_address = 0;
+ struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
+ unsigned long flags;
+
+ if (!IS_FWI2_CAPABLE(ha) && !IS_QLA2100(ha) && !IS_QLA2200(ha)) {
+ /* Disable SRAM, Instruction RAM and GP RAM parity. */
+ spin_lock_irqsave(&ha->hardware_lock, flags);
+ WRT_REG_WORD(®->hccr, (HCCR_ENABLE_PARITY + 0x0));
+ RD_REG_WORD(®->hccr);
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
+ }
/* Load firmware sequences */
rval = ha->isp_ops->load_risc(ha, &srisc_address);
@@ -968,6 +978,19 @@ qla2x00_setup_chip(scsi_qla_host_t *ha)
}
}
+ if (!IS_FWI2_CAPABLE(ha) && !IS_QLA2100(ha) && !IS_QLA2200(ha)) {
+ /* Enable proper parity. */
+ spin_lock_irqsave(&ha->hardware_lock, flags);
+ if (IS_QLA2300(ha))
+ /* SRAM parity */
+ WRT_REG_WORD(®->hccr, HCCR_ENABLE_PARITY + 0x1);
+ else
+ /* SRAM, Instruction RAM and GP RAM parity */
+ WRT_REG_WORD(®->hccr, HCCR_ENABLE_PARITY + 0x7);
+ RD_REG_WORD(®->hccr);
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
+ }
+
if (rval) {
DEBUG2_3(printk("scsi(%ld): Setup chip FAILED .\n",
ha->host_no));
@@ -3344,60 +3367,15 @@ static int
qla2x00_restart_isp(scsi_qla_host_t *ha)
{
uint8_t status = 0;
- struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
- unsigned long flags = 0;
uint32_t wait_time;
/* If firmware needs to be loaded */
if (qla2x00_isp_firmware(ha)) {
ha->flags.online = 0;
- if (!(status = ha->isp_ops->chip_diag(ha))) {
- if (IS_QLA2100(ha) || IS_QLA2200(ha)) {
- status = qla2x00_setup_chip(ha);
- goto done;
- }
-
- spin_lock_irqsave(&ha->hardware_lock, flags);
-
- if (!IS_QLA24XX(ha) && !IS_QLA54XX(ha) &&
- !IS_QLA25XX(ha)) {
- /*
-* Disable SRAM, Instruction RAM and GP RAM
-* parity.
-*/
- WRT_REG_WORD(®->hccr,
- (HCCR_ENABLE_PARITY + 0x0));
- RD_REG_WORD(®->hccr);
- }
-
- spin_unlock_irqrestore(&ha->hardware_lock, flags);
-
+ if (!(status = ha->isp_ops->chip_diag(ha)))
status = qla2x00_setup_chip(ha);
-
- spin_lock_irqsave(&ha->hardware_lock, flags);
-
- if (!IS_QLA24XX(ha) && !IS_QLA54XX(ha) &&
- !IS_QLA25XX(ha)) {
- /* Enable proper parity */
- if (IS_QLA2300(ha))
- /* SRAM parity */
- WRT_REG_WORD(®->hccr,
- (HCCR_ENABLE_PARITY + 0x1));
- else
- /*
-* SRAM, Instruction RAM and GP RAM
-* parity.
-*/
- WRT_REG_WORD(®->hccr,
- (HCCR_ENABLE_PARITY + 0x7));
- RD_REG_WORD(®->hccr);
- }
-
- spin_unlock_irqrestore(&ha->hardware_lock, flags);
- }
}
- done:
if (!status && !(status = qla2x00_init_rings(ha))) {
clear_bit(RESET_MARKER_NEEDED, &ha->dpc_flags);
if (!(status = qla2x00_fw_ready(ha))) {
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 1249545..f129c69 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1773,18 +1773,6 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct
pci_device_id *id)
WRT_REG_WOR
[PATCH 05/11] qla2xxx: Cleanse memory allocation logic during probe.
- Drop loop-till-allocated structure of code within
qla2x00_mem_alloc().
- Properly unwind deallcations of memory during failures.
- Drop qla2x00_allocate_sp_pool() and qla2x00_free_sp_pool()
functions as their implementations can easily be collapsed into
the callers.
- Defer DMA pool allocation of SFP data until requested.
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_attr.c | 13 ++
drivers/scsi/qla2xxx/qla_os.c | 330 ---
2 files changed, 112 insertions(+), 231 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index adf9732..1dd8591 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -428,6 +428,19 @@ qla2x00_sysfs_read_sfp(struct kobject *kobj,
if (!capable(CAP_SYS_ADMIN) || off != 0 || count != SFP_DEV_SIZE * 2)
return 0;
+ if (ha->sfp_data)
+ goto do_read;
+
+ ha->sfp_data = dma_pool_alloc(ha->s_dma_pool, GFP_KERNEL,
+ &ha->sfp_data_dma);
+ if (!ha->sfp_data) {
+ qla_printk(KERN_WARNING, ha,
+ "Unable to allocate memory for SFP read-data.\n");
+ return 0;
+ }
+
+do_read:
+ memset(ha->sfp_data, 0, SFP_BLOCK_SIZE);
addr = 0xa0;
for (iter = 0, offset = 0; iter < (SFP_DEV_SIZE * 2) / SFP_BLOCK_SIZE;
iter++, offset += SFP_BLOCK_SIZE) {
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index e2adfce..1249545 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -204,10 +204,8 @@ static int qla2x00_do_dpc(void *data);
static void qla2x00_rst_aen(scsi_qla_host_t *);
-static uint8_t qla2x00_mem_alloc(scsi_qla_host_t *);
+static int qla2x00_mem_alloc(scsi_qla_host_t *);
static void qla2x00_mem_free(scsi_qla_host_t *ha);
-static int qla2x00_allocate_sp_pool( scsi_qla_host_t *ha);
-static void qla2x00_free_sp_pool(scsi_qla_host_t *ha);
static void qla2x00_sp_free_dma(scsi_qla_host_t *, srb_t *);
/* --
*/
@@ -2025,196 +2023,109 @@ qla2x00_mark_all_devices_lost(scsi_qla_host_t *ha,
int defer)
*
* Returns:
* 0 = success.
-* 1 = failure.
+* !0 = failure.
*/
-static uint8_t
+static int
qla2x00_mem_alloc(scsi_qla_host_t *ha)
{
charname[16];
- uint8_t status = 1;
- int retry= 10;
-
- do {
- /*
-* This will loop only once if everything goes well, else some
-* number of retries will be performed to get around a kernel
-* bug where available mem is not allocated until after a
-* little delay and a retry.
-*/
- ha->request_ring = dma_alloc_coherent(&ha->pdev->dev,
- (ha->request_q_length + 1) * sizeof(request_t),
- &ha->request_dma, GFP_KERNEL);
- if (ha->request_ring == NULL) {
- qla_printk(KERN_WARNING, ha,
- "Memory Allocation failed - request_ring\n");
-
- qla2x00_mem_free(ha);
- msleep(100);
-
- continue;
- }
-
- ha->response_ring = dma_alloc_coherent(&ha->pdev->dev,
- (ha->response_q_length + 1) * sizeof(response_t),
- &ha->response_dma, GFP_KERNEL);
- if (ha->response_ring == NULL) {
- qla_printk(KERN_WARNING, ha,
- "Memory Allocation failed - response_ring\n");
-
- qla2x00_mem_free(ha);
- msleep(100);
-
- continue;
- }
-
- ha->gid_list = dma_alloc_coherent(&ha->pdev->dev, GID_LIST_SIZE,
- &ha->gid_list_dma, GFP_KERNEL);
- if (ha->gid_list == NULL) {
- qla_printk(KERN_WARNING, ha,
- "Memory Allocation failed - gid_list\n");
-
- qla2x00_mem_free(ha);
- msleep(100);
-
- continue;
- }
-
- /* get consistent memory allocated for init control block */
- ha->init_cb = dma_alloc_coherent(&ha->pdev->dev,
- ha->init_cb_size, &ha->init_cb_dma, GFP_KERNEL);
- if (ha->init_cb == NULL) {
- qla_printk(KERN_WARNING, ha,
- "Memory Allocation failed - init_cb\n");
-
- qla2x00_mem_free(ha);
- msleep(100);
-
- continue;
- }
- memset(ha->init_cb, 0, ha->init_cb_size);
-
- snprintf(name, sizeof(name), "%s_%ld", QLA2XXX_DRIVER_NAME,
- ha->host_no);
- ha->s
[PATCH 08/11] qla2xxx: Correct issue where vport-state was not updated during an ISP_ABORT_NEEDED requst.
From: Seokmann Ju <[EMAIL PROTECTED]>
While running IO simultaneously through physical port and virtual
port, if user changes Data Rate (from scli utility), IO through
virtual port fails. It failed because the vport had not received
the ISP_ABORT_NEEDED notification.
Signed-Off-by: Seokmann Ju <[EMAIL PROTECTED]>
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]>
---
drivers/scsi/qla2xxx/qla_os.c | 15 +++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 4b4c32f..0b52ee7 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -2220,6 +2220,9 @@ qla2x00_do_dpc(void *data)
fc_port_t *fcport;
uint8_t status;
uint16_tnext_loopid;
+ struct scsi_qla_host *vha;
+ int i;
+
ha = (scsi_qla_host_t *)data;
@@ -2262,6 +2265,18 @@ qla2x00_do_dpc(void *data)
}
clear_bit(ABORT_ISP_ACTIVE, &ha->dpc_flags);
}
+
+ for_each_mapped_vp_idx(ha, i) {
+ list_for_each_entry(vha, &ha->vp_list,
+ vp_list) {
+ if (i == vha->vp_idx) {
+ set_bit(ISP_ABORT_NEEDED,
+ &vha->dpc_flags);
+ break;
+ }
+ }
+ }
+
DEBUG(printk("scsi(%ld): dpc: qla2x00_abort_isp end\n",
ha->host_no));
}
--
1.5.4.rc5.5.gab98
-
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 02/11] qla2xxx: Add MODULE_FIRMWARE hint for ISP25XX firmware.
Signed-off-by: Andrew Vasquez <[EMAIL PROTECTED]> --- drivers/scsi/qla2xxx/qla_os.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index 3954ed2..bbdfd81 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -3016,3 +3016,4 @@ MODULE_FIRMWARE(FW_FILE_ISP22XX); MODULE_FIRMWARE(FW_FILE_ISP2300); MODULE_FIRMWARE(FW_FILE_ISP2322); MODULE_FIRMWARE(FW_FILE_ISP24XX); +MODULE_FIRMWARE(FW_FILE_ISP25XX); -- 1.5.4.rc5.5.gab98 - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] scsi-ml: Add helper code so transport classes can control queueing
From: Mike Christie <[EMAIL PROTECTED]>
SCSI-ml manages the queueing limits for the device and host, but
does not do so at the target level. Currently this is not needed
and is probably more for the transport code to handle. However,
for bnx2i we will need to be able to limit queueing at this level.
bnx2i will hook into libiscsi, but will allocate a scsi host per
netdevice/hba, so unlike pure software iscsi/iser which is allocating
a host per session, it cannot set the scsi_host->can_queue and return
SCSI_MLQUEUE_HOST_BUSY to reflect queueing limits on the transport.
This patch adds some basic helper code for scsi-ml, that the transport
classes can utilize.
The patch adds code similar to the exisiting SCSI_ML_*BUSY handlers.
You can now return SCSI_MLQUEUE_TARGET_BUSY when we hit
a transport level queueing issue like the hw cannot allocate some
resource at the iscsi session/connection level or the target has temporarily
closed or shrunk the queueing window.
The iscsi class/driver can also set a scsi_target->can_queue value which
reflects the max commands the driver/class can support. For iscsi this
reflects the number of commands we can support for each session due to
session/connection hw limits, driver limits, and to also reflect the
session/targets's queueing window.
This patch was made over the last patchset I sent. It is not for
mergeing into 2.6.25 or scsi-misc yet. I just want to put it out
here, and make sure it is ok and get feedback on what else I might
need to do for this patch before broadcom and I push the bnx2i driver.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/scsi/scsi.c| 11 +--
drivers/scsi/scsi_lib.c| 70 +--
drivers/scsi/scsi_scan.c |1 +
include/scsi/scsi.h|1 +
include/scsi/scsi_device.h | 10 ++
5 files changed, 80 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index b35d194..339760a 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -628,9 +628,12 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
if (rtn) {
if (scsi_delete_timer(cmd)) {
atomic_inc(&cmd->device->iodone_cnt);
- scsi_queue_insert(cmd,
- (rtn == SCSI_MLQUEUE_DEVICE_BUSY) ?
- rtn : SCSI_MLQUEUE_HOST_BUSY);
+
+ if (rtn != SCSI_MLQUEUE_DEVICE_BUSY &&
+ rtn != SCSI_MLQUEUE_TARGET_BUSY)
+ rtn = SCSI_MLQUEUE_HOST_BUSY;
+
+ scsi_queue_insert(cmd, rtn);
}
SCSI_LOG_MLQUEUE(3,
printk("queuecommand : request rejected\n"));
@@ -729,6 +732,7 @@ static struct scsi_driver *scsi_cmd_to_driver(struct
scsi_cmnd *cmd)
void scsi_finish_command(struct scsi_cmnd *cmd)
{
struct scsi_device *sdev = cmd->device;
+ struct scsi_target *starget = scsi_target(sdev);
struct Scsi_Host *shost = sdev->host;
struct scsi_driver *drv;
unsigned int good_bytes;
@@ -744,6 +748,7 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
* XXX(hch): What about locking?
*/
shost->host_blocked = 0;
+ starget->target_blocked = 0;
sdev->device_blocked = 0;
/*
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b12fb31..9c76d10 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -114,6 +114,7 @@ int scsi_queue_insert(struct scsi_cmnd *cmd, int reason)
{
struct Scsi_Host *host = cmd->device->host;
struct scsi_device *device = cmd->device;
+ struct scsi_target *starget = scsi_target(device);
struct request_queue *q = device->request_queue;
unsigned long flags;
@@ -133,10 +134,17 @@ int scsi_queue_insert(struct scsi_cmnd *cmd, int reason)
* if a command is requeued with no other commands outstanding
* either for the device or for the host.
*/
- if (reason == SCSI_MLQUEUE_HOST_BUSY)
- host->host_blocked = host->max_host_blocked;
- else if (reason == SCSI_MLQUEUE_DEVICE_BUSY)
- device->device_blocked = device->max_device_blocked;
+ switch (reason) {
+ case SCSI_MLQUEUE_HOST_BUSY:
+ host->host_blocked = host->max_host_blocked;
+ break;
+ case SCSI_MLQUEUE_DEVICE_BUSY:
+ device->device_blocked = device->max_device_blocked;
+ break;
+ case SCSI_MLQUEUE_TARGET_BUSY:
+ starget->target_blocked = starget->max_target_blocked;
+ break;
+ }
/*
* Decrement the counters, since these commands are no longer
@@ -452,10 +460,12 @@ static void scsi_init_cmd_errh(struct scsi_cmnd *cmd)
void scsi_device_unbusy(struct scsi_device *sdev)
{
struct Scsi_Host *shost = sdev->ho
[PATCH 2/2] iscsi: user per target can_queue
From: Mike Christie <[EMAIL PROTECTED]>
This hooks iscsi_tcp and libiscsi into the target->can_queue
code and it has libiscsi use SCSI_MLQUEUE_TARGET_BUSY.
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
drivers/infiniband/ulp/iser/iscsi_iser.c |1 +
drivers/scsi/iscsi_tcp.c |1 +
drivers/scsi/libiscsi.c | 19 +--
include/scsi/libiscsi.h |1 +
4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index be1b9fb..c818707 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -552,6 +552,7 @@ static struct scsi_host_template iscsi_iser_sht = {
.sg_tablesize = ISCSI_ISER_SG_TABLESIZE,
.max_sectors= 1024,
.cmd_per_lun= ISCSI_MAX_CMD_PER_LUN,
+ .slave_alloc= iscsi_slave_alloc,
.eh_abort_handler = iscsi_eh_abort,
.eh_device_reset_handler= iscsi_eh_device_reset,
.eh_host_reset_handler = iscsi_eh_host_reset,
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 8a17867..c63e0e8 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -1936,6 +1936,7 @@ static struct scsi_host_template iscsi_sht = {
.eh_device_reset_handler= iscsi_eh_device_reset,
.eh_host_reset_handler = iscsi_eh_host_reset,
.use_clustering = DISABLE_CLUSTERING,
+ .slave_alloc= iscsi_slave_alloc,
.slave_configure= iscsi_tcp_slave_configure,
.proc_name = "iscsi_tcp",
.this_id= -1,
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 59f8445..6d6770c 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1097,7 +1097,7 @@ reject:
spin_unlock(&session->lock);
debug_scsi("cmd 0x%x rejected (%d)\n", sc->cmnd[0], reason);
spin_lock(host->host_lock);
- return SCSI_MLQUEUE_HOST_BUSY;
+ return SCSI_MLQUEUE_TARGET_BUSY;
fault:
spin_unlock(&session->lock);
@@ -1118,6 +1118,21 @@ int iscsi_change_queue_depth(struct scsi_device *sdev,
int depth)
}
EXPORT_SYMBOL_GPL(iscsi_change_queue_depth);
+int iscsi_slave_alloc(struct scsi_device *sdev)
+{
+ struct scsi_target *starget = scsi_target(sdev);
+ struct iscsi_cls_session *cls_session = starget_to_session(starget);
+ struct iscsi_session *session;
+
+ if (!cls_session || iscsi_session_chkready(cls_session))
+ return -ENXIO;
+
+ session = class_to_transport_session(cls_session);
+ starget->can_queue = session->cmds_max;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iscsi_slave_alloc);
+
void iscsi_session_recovery_timedout(struct iscsi_cls_session *cls_session)
{
struct iscsi_session *session = class_to_transport_session(cls_session);
@@ -1724,7 +1739,7 @@ iscsi_session_setup(struct iscsi_transport *iscsit,
return NULL;
/* the iscsi layer takes one task for reserve */
- shost->can_queue = cmds_max - 1;
+ shost->can_queue = cmds_max;
shost->cmd_per_lun = qdepth;
shost->max_id = 1;
shost->max_channel = 0;
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 7b90b63..3c2bdc4 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -308,6 +308,7 @@ struct iscsi_session {
/*
* scsi host template
*/
+extern int iscsi_slave_alloc(struct scsi_device *sdev);
extern int iscsi_change_queue_depth(struct scsi_device *sdev, int depth);
extern int iscsi_eh_abort(struct scsi_cmnd *sc);
extern int iscsi_eh_host_reset(struct scsi_cmnd *sc);
--
1.5.2.1
-
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] [RFC] sd: make error handling more robust
I have a RAID that returns a medium error on a read command. The
"information bytes valid" bit is set in the sense data, but the
information bytes are zero:
CDB: 28 00 02 B0 62 00 00 00 02 00
Status: 02 (CHECK CONDITION)
Sense data:
F0 00 03 00 | 00 00 00 0A | 00 00 00 00 | 00 00 00 00
00 00
For HARDWARE_ERROR/MEDIUM_ERROR, sd_done assumes that the sense
information bytes contain the sector in error without sanity-checking
the value, so it ends up returning a completely bogus good_bytes value
greater than xfer_size. This in turn causes the medium error to be
ignored and corrupt data to be returned to the user application. This
problem was introduced by the patch "[SCSI] sd/scsi_lib simplify
sd_rw_intr and scsi_io_completion" in 2.6.18-rc1; with kernels 2.6.17
and before, the application correctly gets an I/O error instead. This
patch fixes this particular problem by checking that bad_lba falls
within a sensible range before using it.
For a read command that returns HARDWARE_ERROR/MEDIUM_ERROR, I also
added the ability to calculate the amount of good data returned using
the data transfer residual if set by the LLDD. If the both the sense
information bytes and the data transfer residual are valid, then they
are used to sanity-check each other.
The following code in sd_done doesn't seem right to me:
if (driver_byte(result) != DRIVER_SENSE &&
(!sense_valid || sense_deferred))
goto out;
It would make more sense to use || rather than && for this test. Even
better, this patch moves the check up higher so that the sense buffer
isn't even accessed unless driver_byte(result) & DRIVER_SENSE.
Finally, for the case of RECOVERED_ERROR/NO_SENSE, this patch adds a
check of the data transfer residual before assuming that the command
completed successfully.
I would like comments on the following:
sd_done doesn't check the data transfer residual for commands that
complete successfully. In the unlikely case that the drive returns good
status without transferring all the data (due to a SCSI bus problem or
disk firmware bug), the error won't be detected. I figured that it was
more likely for a LLDD to set resid incorrectly than for this unlikely
problem to happen, so I didn't add a check for it. Agreed?
Is the new is_sd_cmnd_read_or_write() function necessary? The original
code appears to use blk_fs_request(SCpnt->request) to determine if a
command is read or write. Should I drop is_sd_cmnd_read_or_write() and
use blk_fs_request() instead, or it it OK like this?
Does anyone object to the new data transfer residual checks for
non-read/write commands that return RECOVERED_ERROR/NO_SENSE? Should I
just drop this part of the patch for simplicity?
--- linux-2.6.24-git9/drivers/scsi/sd.c.orig2008-01-31 16:24:04.0
-0500
+++ linux-2.6.24-git9/drivers/scsi/sd.c 2008-01-31 16:24:51.0 -0500
@@ -916,6 +916,29 @@ static struct block_device_operations sd
.revalidate_disk= sd_revalidate_disk,
};
+static int is_sd_cmnd_read_or_write(struct scsi_cmnd *cmd)
+{
+ int is_rw;
+
+ switch (cmd->cmnd[0]) {
+ case READ_6 :
+ case WRITE_6 :
+ case READ_10 :
+ case WRITE_10 :
+ case READ_12 :
+ case WRITE_12 :
+ case READ_16 :
+ case WRITE_16 :
+ is_rw = 1;
+ break;
+
+ default :
+ is_rw = 0;
+ }
+
+ return is_rw;
+}
+
/**
* sd_done - bottom half handler: called when the lower level
* driver has completed (successfully or otherwise) a scsi command.
@@ -928,14 +951,16 @@ static int sd_done(struct scsi_cmnd *SCp
int result = SCpnt->result;
unsigned int xfer_size = scsi_bufflen(SCpnt);
unsigned int good_bytes = result ? 0 : xfer_size;
- u64 start_lba = SCpnt->request->sector;
+ unsigned int sector_size;
+ u64 start_lba;
u64 bad_lba;
struct scsi_sense_hdr sshdr;
int sense_valid = 0;
int sense_deferred = 0;
int info_valid;
+ int resid;
- if (result) {
+ if (driver_byte(result) & DRIVER_SENSE) {
sense_valid = scsi_command_normalize_sense(SCpnt, &sshdr);
if (sense_valid)
sense_deferred = scsi_sense_is_deferred(&sshdr);
@@ -951,23 +976,53 @@ static int sd_done(struct scsi_cmnd *SCp
sshdr.ascq));
}
#endif
- if (driver_byte(result) != DRIVER_SENSE &&
- (!sense_valid || sense_deferred))
+ if (!sense_valid || sense_deferred)
goto out;
+ sector_size = SCpnt->device->sector_size;
+
+ resid = scsi_get_resid(SCpnt);
+ if (resid < 0)
+ resid = 0;
+ else if ((unsigned) resid > xfer_size)
+ resid = xfer_size;
+
switch (sshdr.sense_key) {
case HARDWARE_ERROR:
case MEDIUM_ERROR:
- if (!blk_fs_request(SCpnt-
Add target queueing limit helpers to scsi-ml
These two patches are for RFC, but could go into scsi-misc for 2.6.26 if they are ok off the bat. They add the ability to limit queueing at the target level. This will be needed for bnx2i, but also may be useful to limit transport level commands when using bsg. - 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/2] scsi-ml: Add helper code so transport classes can control queueing
[EMAIL PROTECTED] wrote:
+static inline int scsi_target_ready(struct scsi_device *sdev)
+{
+ struct scsi_target *starget = scsi_target(sdev);
+
+ if (starget->single_lun) {
+ if (starget->starget_sdev_user &&
+ starget->starget_sdev_user != sdev)
+ return 0;
+ starget->starget_sdev_user = sdev;
+ }
+
+ if (starget->can_queue && starget->target_busy >= starget->can_queue)
+ return 0;
+ if (starget->target_busy == 0 && starget->target_blocked) {
+ /*
+* unblock after target_blocked iterates to zero
+*/
+ if (--starget->target_blocked == 0) {
+ SCSI_LOG_MLQUEUE(3,
+ starget_printk(KERN_INFO, starget,
+ "unblocking target at zero depth\n"));
+ } else {
+ blk_plug_device(sdev->request_queue);
+ return 0;
+ }
+ }
+ if (starget->target_blocked)
+ return 0;
Sorry old patch. This one can starve devices here. An updated patch is
attached.
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index b35d194..339760a 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -628,9 +628,12 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
if (rtn) {
if (scsi_delete_timer(cmd)) {
atomic_inc(&cmd->device->iodone_cnt);
- scsi_queue_insert(cmd,
- (rtn == SCSI_MLQUEUE_DEVICE_BUSY) ?
- rtn : SCSI_MLQUEUE_HOST_BUSY);
+
+ if (rtn != SCSI_MLQUEUE_DEVICE_BUSY &&
+ rtn != SCSI_MLQUEUE_TARGET_BUSY)
+rtn = SCSI_MLQUEUE_HOST_BUSY;
+
+ scsi_queue_insert(cmd, rtn);
}
SCSI_LOG_MLQUEUE(3,
printk("queuecommand : request rejected\n"));
@@ -729,6 +732,7 @@ static struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
void scsi_finish_command(struct scsi_cmnd *cmd)
{
struct scsi_device *sdev = cmd->device;
+ struct scsi_target *starget = scsi_target(sdev);
struct Scsi_Host *shost = sdev->host;
struct scsi_driver *drv;
unsigned int good_bytes;
@@ -744,6 +748,7 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
* XXX(hch): What about locking?
*/
shost->host_blocked = 0;
+ starget->target_blocked = 0;
sdev->device_blocked = 0;
/*
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b12fb31..9590630 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -114,6 +114,7 @@ int scsi_queue_insert(struct scsi_cmnd *cmd, int reason)
{
struct Scsi_Host *host = cmd->device->host;
struct scsi_device *device = cmd->device;
+ struct scsi_target *starget = scsi_target(device);
struct request_queue *q = device->request_queue;
unsigned long flags;
@@ -133,10 +134,17 @@ int scsi_queue_insert(struct scsi_cmnd *cmd, int reason)
* if a command is requeued with no other commands outstanding
* either for the device or for the host.
*/
- if (reason == SCSI_MLQUEUE_HOST_BUSY)
- host->host_blocked = host->max_host_blocked;
- else if (reason == SCSI_MLQUEUE_DEVICE_BUSY)
- device->device_blocked = device->max_device_blocked;
+ switch (reason) {
+ case SCSI_MLQUEUE_HOST_BUSY:
+ host->host_blocked = host->max_host_blocked;
+ break;
+ case SCSI_MLQUEUE_DEVICE_BUSY:
+ device->device_blocked = device->max_device_blocked;
+ break;
+ case SCSI_MLQUEUE_TARGET_BUSY:
+ starget->target_blocked = starget->max_target_blocked;
+ break;
+ }
/*
* Decrement the counters, since these commands are no longer
@@ -452,10 +460,12 @@ static void scsi_init_cmd_errh(struct scsi_cmnd *cmd)
void scsi_device_unbusy(struct scsi_device *sdev)
{
struct Scsi_Host *shost = sdev->host;
+ struct scsi_target *starget = scsi_target(sdev);
unsigned long flags;
spin_lock_irqsave(shost->host_lock, flags);
shost->host_busy--;
+ starget->target_busy--;
if (unlikely(scsi_host_in_recovery(shost) &&
(shost->host_failed || shost->host_eh_scheduled)))
scsi_eh_wakeup(shost);
@@ -1296,6 +1306,51 @@ static inline int scsi_dev_queue_ready(struct request_queue *q,
return 1;
}
+
+/*
+ * scsi_target_ready: checks if there we can send commands to target
+ * @sdev: scsi device on starget to check.
+ *
+ * Called with the host lock held.
+ */
+static inline int scsi_target_ready(struct Scsi_Host *shost,
+struct scsi_device *sdev)
+{
+ struct scsi_target *starget = scsi_target(sdev);
+
+ if (starget->single_lun) {
+ if (starget->starget_sdev_user &&
+ starget->starget_sdev_user != sdev)
+ return 0;
+ starget->starget_sdev_user = sdev;
+ }
+
+ if (starget->target_busy == 0 && starget->target_blocked) {
+ /*
+ * unblock after target_blocked iterates to zero
+ */
+ if (--starget->target_blocked == 0) {
+ SCSI_LOG_MLQUEUE(3,
+ starget_printk(KERN_INFO, starget,
+ "unblocking target at zero depth\n"));
+ } else {
+ blk_plug_device(sdev->r
