Re: [PATCH] libata: eliminate the home grown dma padding in favour of that provided by the block layer
On Mon, 31 Dec 2007 15:56:08 -0600 James Bottomley [EMAIL PROTECTED] wrote: ATA requires that all DMA transfers begin and end on word boundaries. Because of this, a large amount of machinery grew up in ide to adjust scatterlists on this basis. However, as of 2.5, the block layer has a dma_alignment variable which ensures both the beginning and length of a DMA transfer are aligned on the dma_alignment boundary. Although the block layer does adjust the beginning of the transfer to ensure this happens, it doesn't actually adjust the length, it merely makes sure that space is allocated for transfers beyond the declared length. The upshot of this is that scatterlists may be padded to any size between the actual length and the length adjusted to the dma_alignment safely knowing that memory is allocated in this region. Great! diff --git a/include/linux/libata.h b/include/linux/libata.h index 124033c..2f40d57 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -282,7 +282,7 @@ enum { /* size of buffer to pad xfers ending on unaligned boundaries */ ATA_DMA_PAD_SZ = 4, - ATA_DMA_PAD_BUF_SZ = ATA_DMA_PAD_SZ * ATA_MAX_QUEUE, + ATA_DMA_PAD_MASK= ATA_DMA_PAD_SZ - 1, /* ering size */ ATA_ERING_SIZE = 32, @@ -446,12 +446,9 @@ struct ata_queued_cmd { unsigned long flags; /* ATA_QCFLAG_xxx */ unsigned inttag; unsigned intn_elem; - unsigned intn_iter; - unsigned intorig_n_elem; int dma_dir; - unsigned intpad_len; unsigned intsect_size; unsigned intnbytes; @@ -461,7 +458,6 @@ struct ata_queued_cmd { unsigned intcursg_ofs; struct scatterlist sgent; - struct scatterlist pad_sgent; void*buf_virt; /* DO NOT iterate over __sg manually, use ata_for_each_sg() */ @@ -606,9 +602,6 @@ struct ata_port { struct ata_prd *prd;/* our SG list */ dma_addr_t prd_dma; /* and its DMA mapping */ - void*pad; /* array of DMA pad buffers */ - dma_addr_t pad_dma; - struct ata_ioports ioaddr; /* ATA cmd/ctl/dma register blocks */ u8 ctl;/* cache of ATA control register */ @@ -1080,24 +1073,15 @@ extern void ata_port_pbar_desc(struct ata_port *ap, int bar, ssize_t offset, static inline struct scatterlist * ata_qc_first_sg(struct ata_queued_cmd *qc) { - qc-n_iter = 0; if (qc-n_elem) return qc-__sg; - if (qc-pad_len) - return qc-pad_sgent; return NULL; } static inline struct scatterlist * ata_qc_next_sg(struct scatterlist *sg, struct ata_queued_cmd *qc) { - if (sg == qc-pad_sgent) - return NULL; - if (++qc-n_iter qc-n_elem) - return sg_next(sg); - if (qc-pad_len) - return qc-pad_sgent; - return NULL; + return sg_next(sg); } #define ata_for_each_sg(sg, qc) \ How about removing ata_qc_first_sg and ata_qc_next_sg completely? Now we can just replace ata_qc_next_sg with sg_next. qc-__sg seems to be always initialized to NULL so we can remove ata_qc_first_sg too. diff --git a/include/linux/libata.h b/include/linux/libata.h index 4f6404c..2774882 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1054,25 +1054,8 @@ extern void ata_port_pbar_desc(struct ata_port *ap, int bar, ssize_t offset, const char *name); #endif -/* - * qc helpers - */ -static inline struct scatterlist * -ata_qc_first_sg(struct ata_queued_cmd *qc) -{ - if (qc-n_elem) - return qc-__sg; - return NULL; -} - -static inline struct scatterlist * -ata_qc_next_sg(struct scatterlist *sg, struct ata_queued_cmd *qc) -{ - return sg_next(sg); -} - #define ata_for_each_sg(sg, qc) \ - for (sg = ata_qc_first_sg(qc); sg; sg = ata_qc_next_sg(sg, qc)) + for (sg = qc-__sg; sg; sg = sg_next(sg)) static inline unsigned int ata_tag_valid(unsigned int tag) { - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
advansys broken (at least) on ARM and MIPS
Commit 9d511a4b29de6764931343d03e493f2e04df0271 removed the depends on BROKEN || X86_32 line from advansys' Kconfig entry. I'm not sure that was such a great idea - the module doesn't compile at least on ARM and MIPS: ARM: CC [M] drivers/scsi/advansys.o drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not properly converted to the DMA API drivers/scsi/advansys.c: In function ‘AdvBuildCarrierFreelist’: drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) ... drivers/scsi/advansys.c: In function ‘advansys_get_sense_buffer_dma’: drivers/scsi/advansys.c:9885: error: implicit declaration of function ‘dma_cache_sync’ MIPS: Building modules, stage 2. MODPOST 64 modules ERROR: free_dma [drivers/scsi/advansys.ko] undefined! -- Martin Michlmayr http://www.cyrius.com/ - 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/3] scsi: Convert everyone to scsi_sglist and scsi_sg_count
This patch simply converts direct uses of -use_sg and -request_buffer to use the wrapper macros. This removes the assumption that the sg list is overloaded on request_buffer, and that there's an explicit use_sg field. The -request_buffer assumption is explicit in scsi_debug.c's paranoid checking, so that code had to be shuffled a little. Signed-off-by: Rusty Russell [EMAIL PROTECTED] --- drivers/scsi/NCR5380.c |6 +++--- drivers/scsi/NCR53C9x.c |6 +++--- drivers/scsi/aha1542.c | 14 +++--- drivers/scsi/atari_NCR5380.c |2 +- drivers/scsi/atp870u.c | 22 +++--- drivers/scsi/eata_pio.c |6 +++--- drivers/scsi/fd_mcs.c|6 +++--- drivers/scsi/imm.c |5 ++--- drivers/scsi/in2000.c|6 +++--- drivers/scsi/libsrp.c| 12 ++-- drivers/scsi/pcmcia/nsp_cs.c |8 drivers/scsi/ppa.c |4 ++-- drivers/scsi/qlogicpti.c | 12 ++-- drivers/scsi/scsi_debug.c| 14 +++--- drivers/scsi/seagate.c |4 ++-- drivers/scsi/sr.c|6 +++--- drivers/scsi/sun3_NCR5380.c |6 +++--- drivers/scsi/sun3x_esp.c |4 ++-- drivers/scsi/wd33c93.c |6 +++--- 21 files changed, 77 insertions(+), 76 deletions(-) diff -r 297d045c5da1 drivers/scsi/NCR5380.c --- a/drivers/scsi/NCR5380.cWed Jan 02 17:18:59 2008 +1100 +++ b/drivers/scsi/NCR5380.cWed Jan 02 18:00:10 2008 +1100 @@ -295,9 +295,9 @@ static __inline__ void initialize_SCp(Sc * various queues are valid. */ - if (cmd-use_sg) { - cmd-SCp.buffer = (struct scatterlist *) cmd-request_buffer; - cmd-SCp.buffers_residual = cmd-use_sg - 1; + if (scsi_sg_count(cmd)) { + cmd-SCp.buffer = scsi_sglist(cmd); + cmd-SCp.buffers_residual = scsi_sg_count(cmd) - 1; cmd-SCp.ptr = sg_virt(cmd-SCp.buffer); cmd-SCp.this_residual = cmd-SCp.buffer-length; } else { diff -r 297d045c5da1 drivers/scsi/NCR53C9x.c --- a/drivers/scsi/NCR53C9x.c Wed Jan 02 17:18:59 2008 +1100 +++ b/drivers/scsi/NCR53C9x.c Wed Jan 02 18:00:10 2008 +1100 @@ -910,7 +910,7 @@ EXPORT_SYMBOL(esp_proc_info); static void esp_get_dmabufs(struct NCR_ESP *esp, Scsi_Cmnd *sp) { - if(sp-use_sg == 0) { + if(scsi_sg_count(sp) == 0) { sp-SCp.this_residual = sp-request_bufflen; sp-SCp.buffer = (struct scatterlist *) sp-request_buffer; sp-SCp.buffers_residual = 0; @@ -920,8 +920,8 @@ static void esp_get_dmabufs(struct NCR_E sp-SCp.ptr = (char *) virt_to_phys(sp-request_buffer); } else { - sp-SCp.buffer = (struct scatterlist *) sp-request_buffer; - sp-SCp.buffers_residual = sp-use_sg - 1; + sp-SCp.buffer = scsi_sglist(sp-request_buffer); + sp-SCp.buffers_residual = scsi_sg_count(sp) - 1; sp-SCp.this_residual = sp-SCp.buffer-length; if (esp-dma_mmu_get_scsi_sgl) esp-dma_mmu_get_scsi_sgl(esp, sp); diff -r 297d045c5da1 drivers/scsi/aha1542.c --- a/drivers/scsi/aha1542.cWed Jan 02 17:18:59 2008 +1100 +++ b/drivers/scsi/aha1542.cWed Jan 02 18:00:10 2008 +1100 @@ -689,7 +689,7 @@ static int aha1542_queuecommand(Scsi_Cmn memcpy(ccb[mbo].cdb, cmd, ccb[mbo].cdblen); - if (SCpnt-use_sg) { + if (scsi_sg_count(SCpnt)) { struct scatterlist *sg; struct chain *cptr; #ifdef DEBUG @@ -704,12 +704,12 @@ static int aha1542_queuecommand(Scsi_Cmn HOSTDATA(SCpnt-device-host)-SCint[mbo] = NULL; return SCSI_MLQUEUE_HOST_BUSY; } - scsi_for_each_sg(SCpnt, sg, SCpnt-use_sg, i) { - if (sg-length == 0 || SCpnt-use_sg 16 || + scsi_for_each_sg(SCpnt, sg, scsi_sg_count(SCpnt), i) { + if (sg-length == 0 || scsi_sg_count(SCpnt) 16 || (((int) sg-offset) 1) || (sg-length 1)) { unsigned char *ptr; - printk(KERN_CRIT Bad segment list supplied to aha1542.c (%d, %d)\n, SCpnt-use_sg, i); - scsi_for_each_sg(SCpnt, sg, SCpnt-use_sg, i) { + printk(KERN_CRIT Bad segment list supplied to aha1542.c (%d, %d)\n, scsi_sg_count(SCpnt), i); + scsi_for_each_sg(SCpnt, sg, scsi_sg_count(SCpnt), i) { printk(KERN_CRIT %d: %p %d\n, i, sg_virt(sg), sg-length); }; @@ -721,10 +721,10 @@ static int aha1542_queuecommand(Scsi_Cmn }; any2scsi(cptr[i].dataptr, SCSI_SG_PA(sg));
[PATCH 2/3] usb_storage: usb_stor_bulk_transfer_sg cleanup
usb_stor_bulk_transfer_sg() assumes buf is a scatterlist array if use_sg is non-NULL. Change it to an explicit sg arg, instead, to allow the callers to change to scsi_sglist(). Signed-off-by: Rusty Russell [EMAIL PROTECTED] diff -r 09247461cfda drivers/usb/storage/freecom.c --- a/drivers/usb/storage/freecom.c Thu Jan 03 19:30:25 2008 +1100 +++ b/drivers/usb/storage/freecom.c Thu Jan 03 19:51:09 2008 +1100 @@ -133,7 +133,7 @@ freecom_readdata (struct scsi_cmnd *srb, /* Now transfer all of our blocks. */ US_DEBUGP(Start of read\n); result = usb_stor_bulk_transfer_sg(us, ipipe, srb-request_buffer, - count, srb-use_sg, srb-resid); + count, scsi_sglist(srb), scsi_sg_count(srb), srb-resid); US_DEBUGP(freecom_readdata done!\n); if (result USB_STOR_XFER_SHORT) @@ -167,7 +167,7 @@ freecom_writedata (struct scsi_cmnd *srb /* Now transfer all of our blocks. */ US_DEBUGP(Start of write\n); result = usb_stor_bulk_transfer_sg(us, opipe, srb-request_buffer, - count, srb-use_sg, srb-resid); + count, scsi_sglist(srb), scsi_sg_count(srb), srb-resid); US_DEBUGP(freecom_writedata done!\n); if (result USB_STOR_XFER_SHORT) diff -r 09247461cfda drivers/usb/storage/sddr09.c --- a/drivers/usb/storage/sddr09.c Thu Jan 03 19:30:25 2008 +1100 +++ b/drivers/usb/storage/sddr09.c Thu Jan 03 19:51:09 2008 +1100 @@ -355,7 +355,7 @@ static int static int sddr09_readX(struct us_data *us, int x, unsigned long fromaddress, int nr_of_pages, int bulklen, unsigned char *buf, -int use_sg) { +struct scatterlist *sg, int use_sg) { unsigned char *command = us-iobuf; int result; @@ -382,7 +382,7 @@ sddr09_readX(struct us_data *us, int x, } result = usb_stor_bulk_transfer_sg(us, us-recv_bulk_pipe, - buf, bulklen, use_sg, NULL); + buf, bulklen, sg, use_sg, NULL); if (result != USB_STOR_XFER_GOOD) { US_DEBUGP(Result for bulk_transfer in sddr09_read2%d %d\n, @@ -403,12 +403,13 @@ sddr09_readX(struct us_data *us, int x, */ static int sddr09_read20(struct us_data *us, unsigned long fromaddress, - int nr_of_pages, int pageshift, unsigned char *buf, int use_sg) { + int nr_of_pages, int pageshift, unsigned char *buf, + struct scatterlist *sg, int use_sg) { int bulklen = nr_of_pages pageshift; /* The last 8 bits of fromaddress are ignored. */ return sddr09_readX(us, 0, fromaddress, nr_of_pages, bulklen, - buf, use_sg); + buf, sg, use_sg); } /* @@ -426,11 +427,12 @@ sddr09_read20(struct us_data *us, unsign */ static int sddr09_read21(struct us_data *us, unsigned long fromaddress, - int count, int controlshift, unsigned char *buf, int use_sg) { + int count, int controlshift, unsigned char *buf, + struct scatterlist *sg, int use_sg) { int bulklen = (count controlshift); return sddr09_readX(us, 1, fromaddress, count, bulklen, - buf, use_sg); + buf, sg, use_sg); } /* @@ -444,13 +446,14 @@ sddr09_read21(struct us_data *us, unsign */ static int sddr09_read22(struct us_data *us, unsigned long fromaddress, - int nr_of_pages, int pageshift, unsigned char *buf, int use_sg) { + int nr_of_pages, int pageshift, unsigned char *buf, + struct scatterlist *sg, int use_sg) { int bulklen = (nr_of_pages pageshift) + (nr_of_pages CONTROL_SHIFT); US_DEBUGP(sddr09_read22: reading %d pages, %d bytes\n, nr_of_pages, bulklen); return sddr09_readX(us, 2, fromaddress, nr_of_pages, bulklen, - buf, use_sg); + buf, sg, use_sg); } #if 0 @@ -538,7 +541,8 @@ static int static int sddr09_writeX(struct us_data *us, unsigned long Waddress, unsigned long Eaddress, - int nr_of_pages, int bulklen, unsigned char *buf, int use_sg) { + int nr_of_pages, int bulklen, unsigned char *buf, + struct scatterlist *sg, int use_sg) { unsigned char *command = us-iobuf; int result; @@ -568,7 +572,7 @@ sddr09_writeX(struct us_data *us, } result = usb_stor_bulk_transfer_sg(us, us-send_bulk_pipe, - buf, bulklen, use_sg, NULL); + buf, bulklen, sg, use_sg, NULL); if (result != USB_STOR_XFER_GOOD) { US_DEBUGP(Result for bulk_transfer in sddr09_writeX %d\n, @@ -582,10 +586,10 @@ static int static int sddr09_write_inplace(struct us_data *us, unsigned long address, int
[PATCH 3/3] scsi: convert core to sg_ring
A 'struct sg_ring *sg' element is added to struct scsi_cmd, and the use_sg and __use_sg fields are removed. If this field is null, request_buffer point to the data (as was previously indicated by use_sg == 0). To ease transition, scsi_sglist() is now a backwards-compat routine which chains the sg_ring entries so they can be iterated using sg_next(). Once all the drivers which actually want to use chained sgs are converted to sg_ring, the remaining drivers can revert to using simple sg arrays. The backwards-compat conversion is placed under an (always-on) config option (CONFIG_SCSI_SG_CHAIN). Signed-off-by: Rusty Russell [EMAIL PROTECTED] --- drivers/scsi/Kconfig|6 + drivers/scsi/scsi_error.c | 15 +-- drivers/scsi/scsi_lib.c | 219 +--- drivers/scsi/scsi_lib_dma.c | 27 ++--- drivers/scsi/scsi_tgt_lib.c | 17 +-- include/scsi/scsi_cmnd.h| 38 +-- include/scsi/scsi_eh.h |6 - 7 files changed, 170 insertions(+), 158 deletions(-) diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -29,6 +29,12 @@ config SCSI However, do not compile this as a module if your root file system (the one containing the directory /) is located on a SCSI device. + +config SCSI_SG_CHAIN + bool + default y + ---help--- + Support sg chaining for older SCSI drivers. config SCSI_DMA bool diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -617,20 +617,21 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd ses-cmd_len = scmd-cmd_len; memcpy(ses-cmnd, scmd-cmnd, sizeof(scmd-cmnd)); ses-data_direction = scmd-sc_data_direction; + ses-sg = scmd-sg; ses-bufflen = scmd-request_bufflen; ses-buffer = scmd-request_buffer; - ses-use_sg = scmd-use_sg; ses-resid = scmd-resid; ses-result = scmd-result; if (sense_bytes) { scmd-request_bufflen = min_t(unsigned, sizeof(scmd-sense_buffer), sense_bytes); - sg_init_one(ses-sense_sgl, scmd-sense_buffer, - scmd-request_bufflen); - scmd-request_buffer = ses-sense_sgl; + + sg_ring_single(ses-sense_sg.ring, scmd-sense_buffer, + scmd-request_bufflen); + scmd-sg = ses-sense_sg.ring; + scmd-request_buffer = NULL; scmd-sc_data_direction = DMA_FROM_DEVICE; - scmd-use_sg = 1; memset(scmd-cmnd, 0, sizeof(scmd-cmnd)); scmd-cmnd[0] = REQUEST_SENSE; scmd-cmnd[4] = scmd-request_bufflen; @@ -639,7 +640,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd scmd-request_buffer = NULL; scmd-request_bufflen = 0; scmd-sc_data_direction = DMA_NONE; - scmd-use_sg = 0; + scmd-sg = NULL; if (cmnd) { memset(scmd-cmnd, 0, sizeof(scmd-cmnd)); memcpy(scmd-cmnd, cmnd, cmnd_size); @@ -678,7 +679,7 @@ void scsi_eh_restore_cmnd(struct scsi_cm scmd-sc_data_direction = ses-data_direction; scmd-request_bufflen = ses-bufflen; scmd-request_buffer = ses-buffer; - scmd-use_sg = ses-use_sg; + scmd-sg = ses-sg; scmd-resid = ses-resid; scmd-result = ses-result; } diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -17,7 +17,7 @@ #include linux/pci.h #include linux/delay.h #include linux/hardirq.h -#include linux/scatterlist.h +#include linux/sg_ring.h #include scsi/scsi.h #include scsi/scsi_cmnd.h @@ -34,12 +34,20 @@ #define SG_MEMPOOL_NR ARRAY_SIZE(scsi_sg_pools) #define SG_MEMPOOL_SIZE2 +/* If we support old-style SG chaining, we need an extra sg element to overwrite + * with the chain's next pointer. */ +#ifdef CONFIG_SCSI_SG_CHAIN +#define SCSI_SG_PAD 1 +#else +#define SCSI_SG_PAD 0 +#endif + /* * The maximum number of SG segments that we will put inside a scatterlist * (unless chaining is used). Should ideally fit inside a single page, to * avoid a higher order allocation. */ -#define SCSI_MAX_SG_SEGMENTS 128 +#define SCSI_MAX_SG_SEGMENTS (128 - SCSI_SG_PAD) struct scsi_host_sg_pool { size_t size; @@ -707,7 +715,7 @@ static inline unsigned int scsi_sgtable_ { unsigned int index; - switch (nents) { + switch (nents + SCSI_SG_PAD) { case 1 ... 8: index = 0; break; @@ -737,21 +745,41 @@ static inline unsigned int scsi_sgtable_ return index; } -struct scatterlist *scsi_alloc_sgtable(struct scsi_cmnd *cmd, gfp_t
Re: [PATCH 1/3] scsi: Convert everyone to scsi_sglist and scsi_sg_count
On Thu, Jan 03 2008 at 10:50 +0200, Rusty Russell [EMAIL PROTECTED] wrote: This patch simply converts direct uses of -use_sg and -request_buffer to use the wrapper macros. This removes the assumption that the sg list is overloaded on request_buffer, and that there's an explicit use_sg field. The -request_buffer assumption is explicit in scsi_debug.c's paranoid checking, so that code had to be shuffled a little. Signed-off-by: Rusty Russell [EMAIL PROTECTED] --- drivers/scsi/NCR5380.c |6 +++--- drivers/scsi/NCR53C9x.c |6 +++--- drivers/scsi/aha1542.c | 14 +++--- drivers/scsi/atari_NCR5380.c |2 +- drivers/scsi/atp870u.c | 22 +++--- drivers/scsi/eata_pio.c |6 +++--- drivers/scsi/fd_mcs.c|6 +++--- drivers/scsi/imm.c |5 ++--- drivers/scsi/in2000.c|6 +++--- drivers/scsi/libsrp.c| 12 ++-- drivers/scsi/pcmcia/nsp_cs.c |8 drivers/scsi/ppa.c |4 ++-- drivers/scsi/qlogicpti.c | 12 ++-- drivers/scsi/scsi_debug.c| 14 +++--- drivers/scsi/seagate.c |4 ++-- drivers/scsi/sr.c|6 +++--- drivers/scsi/sun3_NCR5380.c |6 +++--- drivers/scsi/sun3x_esp.c |4 ++-- drivers/scsi/wd33c93.c |6 +++--- 21 files changed, 77 insertions(+), 76 deletions(-) All of these drivers are properly converted in current scsi-misc + scsi-pending. If you are really serious about changing scsi-layer you better work ontop of scsi git trees. Also you can inspect -mm tree it has the scsi_data_buffer patches that does 4/5 what you want. 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 0/2] sg_ring: Gentler scsi merge
On Thu, Jan 03 2008 at 9:00 +0200, Rusty Russell [EMAIL PROTECTED] wrote: OK, after wading through many scsi drivers, I decided to change tack and try to provide a transition path. This is in two stages: 1) These two patches. sg_ring used underneath, but if any driver asks for scsi_sglist() they get a 2.6.24-style chained sg. No other patches are necessary. 2) Once all chained-sg-needing scsi drivers change to use cmd-sg (ie. sg_ring) directly, and the chained sg patches can be reverted. scsi_sglist() and scsi_sg_count() then become: /* You should only use these if you never need chained sgs */ static inline struct scatterlist *scsi_sglist(struct scsi_cmd *cmd) { BUG_ON(!list_empty(cmd-sg-list)); return cmd-sg-sg[0]; } static unsigned int scsi_sg_count(struct scsi_cmd *cmd) { if (!cmd-sg) return 0; BUG_ON(!list_empty(cmd-sg-list)); return cmd-sg-num; } Thanks, Rusty. Look in the mailing list archives for the scsi_sgtable patches. These did exactly what you do here. (OK 95% ;)) (only as a scsi subsystem not as a generic sg) 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: Open-FCoE on linux-scsi
From: Love, Robert W [EMAIL PROTECTED] Subject: RE: Open-FCoE on linux-scsi Date: Mon, 31 Dec 2007 08:34:38 -0800 Hello SCSI mailing list, I'd just like to introduce ourselves a bit before we get started. My name is Robert Love and I'm joined by a team of engineers including Vasu Dev, Chris Leech and Yi Zou. We are committed to maintaining the Open-FCoE project. Aside from Intel engineers we expect engineers from other companies to contribute to Open-FCoE. Our goal is to get the initiator code upstream. We have a lot of working code but recognize that we're early in this project's development. We're looking for direction from you, the experts, on what this project should grow into. I've just added a new fcoe target driver to tgt: http://stgt.berlios.de/ That's great; we'll check it out as soon as everyone is back from the holidays. It's still an experiment. Patches are welcome. The driver runs in user space unlike your target mode driver (I just modified your FCoE code to run it in user space). There seems to be a trend to move non-data-path code userspace, however, Implementing FCoE target drive in user space has no connection with a trend to move non-data-path code user space. It does all the data-path in user space. The examples of the trend to move non-data-path code userspace are open-iscsi, multi-path, etc, I think. I don't like having so much duplicate code. We were going to investigate if we could redesign the target code to have less of a profile and just depend on the initiator modules instead of recompiling openfc as openfc_tgt. What's the general opinion on this? Duplicate code vs. more kernel code? I can see that you're already starting to clean up the code that you ported. Does that mean the duplicate code isn't an issue to you? When we fix bugs in the initiator they're not going to make it into your tree unless you're diligent about watching the list. It's hard to convince the kernel maintainers to merge something into mainline that which can be implemented in user space. I failed twice (with two iSCSI target implementations). Yeah, duplication is not good but the user space code has some great advantages. Both approaches have the pros and cons. The initiator driver succeeded to log in a target, see logical units, and perform some I/Os. It's still very unstable but it would be useful for FCoE developers. I would like to help you push the Open-FCoE initiator to mainline too. What are on your todo list and what you guys working on now? We would really appreciate the help! The best way I could come up with to coordinate this effort was through the BZ- http://open-fcoe.org/bugzilla. I was going to write a BZ wiki entry to help new contributors, but since I haven't yet, here's the bottom line. Sign-up to the BZ, assign bugs to yourself from my name (I'm the default assignee now) and also file bugs as you find them. I don't want to impose much process, but this will allow all of us to know what everyone else is working on. The main things that I think need to be fixed are (in no particular order)- 1) Stability- Just straight up bug fixing. This is ongoing and everyone is looking at bugs. Talking about stability is a bit premature, I think. The first thing to do is finding a design that can be accepted into mainline. 2) Abstractions- We consider libsa a big bug, which we're trying to strip down piece by piece. Vasu took out the LOG_SA code and I'm looking into changing the ASSERTs to BUG_ON/WARN_ONs. That isn't all of it, but that's how we're breaking it down. Agreed, libsa (and libcrc) should be removed. 3) Target- The duplicate code of the target is too much. I want to integrate the target into our -upstream tree. Without doing that, fixes to the -upstream tree won't benefit the target and it will get into worse shape than it already is, unless someone is porting those patches to the target too. I think that ideally we'd want to reduce the target's profile and move it to userspace under tgt. 4) Userspace/Kernel interaction- It's our belief that netlink is the preferred mechanism for kernel/userspace interaction. Yi has converted the FCoE ioctl code to netlink and is looking into openfc next. There are other options and I'm not sure that netlink is the best. I think that there is no general consensus about the best mechanism for kernel/userspace interaction. Even ioctl is still accepted into mainline (e.g. kvm). I expect you get an idea to use netlink from open-iscsi, but unlike open-iscsi, for now the FCoE code does just configuration with kernel/userspace interaction. open-iscsi has non-data path in user space so the kernel need to send variable-length data (PDUs, event, etc) to user space via netlink. So open-iscsi really needs netlink. If you have the FCoE non-data path in user space, netlink would work well for you. I would add one TODO item, better integration with
Re: [PATCH 2/2] use dynamically allocated sense buffer
On Thu, 3 Jan 2008 13:56:37 +0900 FUJITA Tomonori [EMAIL PROTECTED] wrote: This removes static array sense_buffer in scsi_cmnd and uses dynamically allocated sense_buffer (with GFP_DMA). scsi_add_host allocates as many buffers as scsi_host-can_queue. __scsi_get_command attaches sense_buffer to a scsi_cmnd and __scsi_put_command detaches the sense_buffer from it. There is a small possibility that a host need more sense buffers than can_queue. The failure of the buffer allocation works just like the failure of scsi_cmnd allocation. So everything should work as before. Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED] --- drivers/scsi/hosts.c | 10 ++- drivers/scsi/scsi.c | 67 +- drivers/scsi/scsi_priv.h |2 + include/scsi/scsi_cmnd.h |2 +- include/scsi/scsi_host.h |3 ++ 5 files changed, 81 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 9a10b43..35c5f0e 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -205,10 +205,14 @@ int scsi_add_host(struct Scsi_Host *shost, struct device *dev) if (!shost-shost_gendev.parent) shost-shost_gendev.parent = dev ? dev : platform_bus; - error = device_add(shost-shost_gendev); + error = scsi_setup_command_sense_buffer(shost); if (error) goto out; + error = device_add(shost-shost_gendev); + if (error) + goto destroy_pool; + scsi_host_set_state(shost, SHOST_RUNNING); get_device(shost-shost_gendev.parent); @@ -248,6 +252,8 @@ int scsi_add_host(struct Scsi_Host *shost, struct device *dev) class_device_del(shost-shost_classdev); out_del_gendev: device_del(shost-shost_gendev); + destroy_pool: + scsi_destroy_command_sense_buffer(shost); out: return error; } @@ -267,6 +273,8 @@ static void scsi_host_dev_release(struct device *dev) scsi_free_queue(shost-uspace_req_q); } + scsi_destroy_command_sense_buffer(shost); + scsi_destroy_command_freelist(shost); if (shost-bqt) blk_free_tags(shost-bqt); diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index ebc0193..91306a5 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -161,6 +161,9 @@ static struct scsi_host_cmd_pool scsi_cmd_dma_pool = { static DEFINE_MUTEX(host_cmd_pool_mutex); +static struct kmem_cache *sense_buffer_slab; +static int sense_buffer_slab_users; + /** * __scsi_get_command - Allocate a struct scsi_cmnd * @shost: host to transmit command @@ -172,6 +175,11 @@ static DEFINE_MUTEX(host_cmd_pool_mutex); struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask) { struct scsi_cmnd *cmd; + unsigned char *buf; + + buf = mempool_alloc(shost-sense_buffer_pool, __GFP_DMA|gfp_mask); + if (!buf) + return NULL; cmd = kmem_cache_alloc(shost-cmd_pool-slab, gfp_mask | shost-cmd_pool-gfp_mask); @@ -188,6 +196,14 @@ struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask) spin_unlock_irqrestore(shost-free_list_lock, flags); } + if (cmd) { + memset(cmd, 0, sizeof(*cmd)); + cmd-sense_buffer = buf; + /* probably unnecessary */ + memset(buf, 0, SCSI_SENSE_BUFFERSIZE); + } else + mempool_free(buf, shost-sense_buffer_pool); + return cmd; } EXPORT_SYMBOL_GPL(__scsi_get_command); @@ -212,7 +228,6 @@ struct scsi_cmnd *scsi_get_command(struct scsi_device *dev, gfp_t gfp_mask) if (likely(cmd != NULL)) { unsigned long flags; - memset(cmd, 0, sizeof(*cmd)); cmd-device = dev; init_timer(cmd-eh_timeout); INIT_LIST_HEAD(cmd-list); @@ -238,6 +253,8 @@ void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd, { unsigned long flags; + mempool_free(cmd-sense_buffer, shost-sense_buffer_pool); + /* changing locks here, don't need to restore the irq state */ spin_lock_irqsave(shost-free_list_lock, flags); if (unlikely(list_empty(shost-free_list))) { @@ -352,6 +369,54 @@ void scsi_destroy_command_freelist(struct Scsi_Host *shost) mutex_unlock(host_cmd_pool_mutex); } +int scsi_setup_command_sense_buffer(struct Scsi_Host *shost) +{ + int ret; + + mutex_lock(host_cmd_pool_mutex); + if (!sense_buffer_slab) { Sorry, here should be: + if (!sense_buffer_slab_users) { This is an updated patch. diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 9a10b43..35c5f0e 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -205,10 +205,14 @@ int scsi_add_host(struct Scsi_Host *shost, struct device *dev) if (!shost-shost_gendev.parent)
Re: advansys broken (at least) on ARM and MIPS
On Thu, Jan 03, 2008 at 08:43:53AM +0100, Martin Michlmayr wrote: MIPS: Building modules, stage 2. MODPOST 64 modules ERROR: free_dma [drivers/scsi/advansys.ko] undefined! needs to depend on GENERIC_ISA_DMA_API - 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] replace sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE
On Thu, Jan 03 2008 at 6:56 +0200, FUJITA Tomonori [EMAIL PROTECTED] wrote: To remove sense_buffer array in scsi_cmnd structure, this replaces sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE in several LLDs. Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED] --- drivers/ata/libata-scsi.c |4 ++-- drivers/message/fusion/mptscsih.c |2 +- drivers/message/i2o/i2o_scsi.c |2 +- drivers/scsi/53c700.c | 11 ++- drivers/scsi/BusLogic.c |2 +- drivers/scsi/aacraid/aachba.c | 12 ++-- drivers/scsi/advansys.c | 14 +++--- drivers/scsi/aha1542.c |4 ++-- drivers/scsi/aha1740.c |2 +- drivers/scsi/aic7xxx/aic79xx_osm.c |6 +++--- drivers/scsi/aic7xxx/aic7xxx_osm.c |6 +++--- drivers/scsi/aic7xxx_old.c | 12 ++-- drivers/scsi/arcmsr/arcmsr_hba.c|6 +++--- drivers/scsi/arm/fas216.c | 10 +- drivers/scsi/dc395x.c | 16 +++- drivers/scsi/dpt_i2o.c |2 +- drivers/scsi/eata.c |4 ++-- drivers/scsi/eata_pio.c |2 +- drivers/scsi/hptiop.c |2 +- drivers/scsi/ips.c | 10 -- drivers/scsi/ncr53c8xx.c|2 +- drivers/scsi/qla1280.c |4 ++-- drivers/scsi/qla2xxx/qla_isr.c | 12 ++-- drivers/scsi/qlogicpti.c|2 +- drivers/scsi/scsi_error.c |6 +++--- drivers/scsi/scsi_lib.c |2 +- drivers/scsi/sym53c8xx_2/sym_glue.c |5 ++--- drivers/scsi/tmscsim.c |6 +++--- drivers/scsi/u14-34f.c |4 ++-- drivers/scsi/ultrastor.c|2 +- 30 files changed, 85 insertions(+), 89 deletions(-) snip diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c index fb5f202..3bf186e 100644 --- a/drivers/scsi/arm/fas216.c +++ b/drivers/scsi/arm/fas216.c @@ -2009,7 +2009,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt, * have valid data in the sense buffer that could * confuse the higher levels. */ - memset(SCpnt-sense_buffer, 0, sizeof(SCpnt-sense_buffer)); + memset(SCpnt-sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); //printk(scsi%d.%c: sense buffer: , info-host-host_no, '0' + SCpnt-device-id); //{ int i; for (i = 0; i 32; i++) printk(%02x , SCpnt-sense_buffer[i]); printk(\n); } /* @@ -2108,16 +2108,16 @@ request_sense: memset(SCpnt-cmnd, 0, sizeof (SCpnt-cmnd)); SCpnt-cmnd[0] = REQUEST_SENSE; SCpnt-cmnd[1] = SCpnt-device-lun 5; - SCpnt-cmnd[4] = sizeof(SCpnt-sense_buffer); + SCpnt-cmnd[4] = SCSI_SENSE_BUFFERSIZE; SCpnt-cmd_len = COMMAND_SIZE(SCpnt-cmnd[0]); SCpnt-SCp.buffer = NULL; SCpnt-SCp.buffers_residual = 0; SCpnt-SCp.ptr = (char *)SCpnt-sense_buffer; - SCpnt-SCp.this_residual = sizeof(SCpnt-sense_buffer); - SCpnt-SCp.phase = sizeof(SCpnt-sense_buffer); + SCpnt-SCp.this_residual = SCSI_SENSE_BUFFERSIZE; + SCpnt-SCp.phase = SCSI_SENSE_BUFFERSIZE; SCpnt-SCp.Message = 0; SCpnt-SCp.Status = 0; - SCpnt-request_bufflen = sizeof(SCpnt-sense_buffer); + SCpnt-request_bufflen = SCSI_SENSE_BUFFERSIZE; SCpnt-sc_data_direction = DMA_FROM_DEVICE; SCpnt-use_sg = 0; SCpnt-tag = 0; Tomo hi. This driver has a patch in scsi-pending that removes exactly all this code. And converts it to the new scsi_error API. You have caught me in the middle of sweeping the entire tree, converting all these drivers, to code like the patch to fas216.c. This is exactly what is needed to satisfy the condition you stated, is needed for farther cleanup. (Though I admit this was done on a low priority, as I'm busy with other stuff) James It is about time for this patch, it has even been acknowledged by a maintainer. Also the second patch to arm is do. 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] libata: eliminate the home grown dma padding in favour of that provided by the block layer
On Thu, 2008-01-03 at 16:58 +0900, FUJITA Tomonori wrote: On Mon, 31 Dec 2007 15:56:08 -0600 James Bottomley [EMAIL PROTECTED] wrote: @@ -1080,24 +1073,15 @@ extern void ata_port_pbar_desc(struct ata_port *ap, int bar, ssize_t offset, static inline struct scatterlist * ata_qc_first_sg(struct ata_queued_cmd *qc) { - qc-n_iter = 0; if (qc-n_elem) return qc-__sg; - if (qc-pad_len) - return qc-pad_sgent; return NULL; } static inline struct scatterlist * ata_qc_next_sg(struct scatterlist *sg, struct ata_queued_cmd *qc) { - if (sg == qc-pad_sgent) - return NULL; - if (++qc-n_iter qc-n_elem) - return sg_next(sg); - if (qc-pad_len) - return qc-pad_sgent; - return NULL; + return sg_next(sg); } #define ata_for_each_sg(sg, qc) \ How about removing ata_qc_first_sg and ata_qc_next_sg completely? Now we can just replace ata_qc_next_sg with sg_next. qc-__sg seems to be always initialized to NULL so we can remove ata_qc_first_sg too. Sure ... I assumed (without actually looking) that the inlines were there because they were an API used throughout the drivers. Actually, grep tells me they're only used in the ata_for_each_sg macro, so this patch looks good. Thanks, James diff --git a/include/linux/libata.h b/include/linux/libata.h index 4f6404c..2774882 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1054,25 +1054,8 @@ extern void ata_port_pbar_desc(struct ata_port *ap, int bar, ssize_t offset, const char *name); #endif -/* - * qc helpers - */ -static inline struct scatterlist * -ata_qc_first_sg(struct ata_queued_cmd *qc) -{ - if (qc-n_elem) - return qc-__sg; - return NULL; -} - -static inline struct scatterlist * -ata_qc_next_sg(struct scatterlist *sg, struct ata_queued_cmd *qc) -{ - return sg_next(sg); -} - #define ata_for_each_sg(sg, qc) \ - for (sg = ata_qc_first_sg(qc); sg; sg = ata_qc_next_sg(sg, qc)) + for (sg = qc-__sg; sg; sg = sg_next(sg)) static inline unsigned int ata_tag_valid(unsigned int tag) { - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/1] Remove of old NCR53C9x/esp family of drivers
James Could you at least put the following patch in scsi-pending so it can attract some acknowledgment, and retract futile attempts from people at coding these drivers. (And also git is better than me in maintaining it synced) Christoph Hellwig! I need your Recommended-by: on this. I had my regular convert-to-accessors, and because of your recommendation I'm pushing this removal. And I keep doing it again and again because a removal patch is a conflict-to-any by definition. David S. Miller Maciej W. Rozycki Please add a comment in favor of this move. And a few words for the few users left for this hardware. 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: advansys broken (at least) on ARM and MIPS
On Thu, 2008-01-03 at 08:43 +0100, Martin Michlmayr wrote: Commit 9d511a4b29de6764931343d03e493f2e04df0271 removed the depends on BROKEN || X86_32 line from advansys' Kconfig entry. I'm not sure that was such a great idea - the module doesn't compile at least on ARM and MIPS: ARM: CC [M] drivers/scsi/advansys.o drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not properly converted to the DMA API drivers/scsi/advansys.c: In function ‘AdvBuildCarrierFreelist’: drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) ... drivers/scsi/advansys.c: In function ‘advansys_get_sense_buffer_dma’: drivers/scsi/advansys.c:9885: error: implicit declaration of function ‘dma_cache_sync’ That error doesn't look to tbe the fault of the driver. dma_cache_sync() is a piece of the DMA API it looks like ARM doesn't implement? James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] replace sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE
On Thu, 03 Jan 2008 17:01:11 +0200 Boaz Harrosh [EMAIL PROTECTED] wrote: On Thu, Jan 03 2008 at 6:56 +0200, FUJITA Tomonori [EMAIL PROTECTED] wrote: To remove sense_buffer array in scsi_cmnd structure, this replaces sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE in several LLDs. Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED] --- drivers/ata/libata-scsi.c |4 ++-- drivers/message/fusion/mptscsih.c |2 +- drivers/message/i2o/i2o_scsi.c |2 +- drivers/scsi/53c700.c | 11 ++- drivers/scsi/BusLogic.c |2 +- drivers/scsi/aacraid/aachba.c | 12 ++-- drivers/scsi/advansys.c | 14 +++--- drivers/scsi/aha1542.c |4 ++-- drivers/scsi/aha1740.c |2 +- drivers/scsi/aic7xxx/aic79xx_osm.c |6 +++--- drivers/scsi/aic7xxx/aic7xxx_osm.c |6 +++--- drivers/scsi/aic7xxx_old.c | 12 ++-- drivers/scsi/arcmsr/arcmsr_hba.c|6 +++--- drivers/scsi/arm/fas216.c | 10 +- drivers/scsi/dc395x.c | 16 +++- drivers/scsi/dpt_i2o.c |2 +- drivers/scsi/eata.c |4 ++-- drivers/scsi/eata_pio.c |2 +- drivers/scsi/hptiop.c |2 +- drivers/scsi/ips.c | 10 -- drivers/scsi/ncr53c8xx.c|2 +- drivers/scsi/qla1280.c |4 ++-- drivers/scsi/qla2xxx/qla_isr.c | 12 ++-- drivers/scsi/qlogicpti.c|2 +- drivers/scsi/scsi_error.c |6 +++--- drivers/scsi/scsi_lib.c |2 +- drivers/scsi/sym53c8xx_2/sym_glue.c |5 ++--- drivers/scsi/tmscsim.c |6 +++--- drivers/scsi/u14-34f.c |4 ++-- drivers/scsi/ultrastor.c|2 +- 30 files changed, 85 insertions(+), 89 deletions(-) snip diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c index fb5f202..3bf186e 100644 --- a/drivers/scsi/arm/fas216.c +++ b/drivers/scsi/arm/fas216.c @@ -2009,7 +2009,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt, * have valid data in the sense buffer that could * confuse the higher levels. */ - memset(SCpnt-sense_buffer, 0, sizeof(SCpnt-sense_buffer)); + memset(SCpnt-sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); //printk(scsi%d.%c: sense buffer: , info-host-host_no, '0' + SCpnt-device-id); //{ int i; for (i = 0; i 32; i++) printk(%02x , SCpnt-sense_buffer[i]); printk(\n); } /* @@ -2108,16 +2108,16 @@ request_sense: memset(SCpnt-cmnd, 0, sizeof (SCpnt-cmnd)); SCpnt-cmnd[0] = REQUEST_SENSE; SCpnt-cmnd[1] = SCpnt-device-lun 5; - SCpnt-cmnd[4] = sizeof(SCpnt-sense_buffer); + SCpnt-cmnd[4] = SCSI_SENSE_BUFFERSIZE; SCpnt-cmd_len = COMMAND_SIZE(SCpnt-cmnd[0]); SCpnt-SCp.buffer = NULL; SCpnt-SCp.buffers_residual = 0; SCpnt-SCp.ptr = (char *)SCpnt-sense_buffer; - SCpnt-SCp.this_residual = sizeof(SCpnt-sense_buffer); - SCpnt-SCp.phase = sizeof(SCpnt-sense_buffer); + SCpnt-SCp.this_residual = SCSI_SENSE_BUFFERSIZE; + SCpnt-SCp.phase = SCSI_SENSE_BUFFERSIZE; SCpnt-SCp.Message = 0; SCpnt-SCp.Status = 0; - SCpnt-request_bufflen = sizeof(SCpnt-sense_buffer); + SCpnt-request_bufflen = SCSI_SENSE_BUFFERSIZE; SCpnt-sc_data_direction = DMA_FROM_DEVICE; SCpnt-use_sg = 0; SCpnt-tag = 0; Tomo hi. This driver has a patch in scsi-pending that removes exactly all this code. And converts it to the new scsi_error API. Yeah, I know that. You have caught me in the middle of sweeping the entire tree, converting all these drivers, to code like the patch to fas216.c. This is exactly what is needed to satisfy the condition you stated, is needed for farther cleanup. (Though I admit this was done on a low priority, as I'm busy with other stuff) Well, this is not a patch for cleanups. This is a 'grep and replace' style patch, which tries not to do anything clever. I just want to solve this DMA issue with minimum changes. I'll post an updated patch if James merges your the conversion patch first. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: advansys broken (at least) on ARM and MIPS
Adding Russell King to CC who can hopefully answer James' question below. * James Bottomley [EMAIL PROTECTED] [2008-01-03 09:21]: On Thu, 2008-01-03 at 08:43 +0100, Martin Michlmayr wrote: Commit 9d511a4b29de6764931343d03e493f2e04df0271 removed the depends on BROKEN || X86_32 line from advansys' Kconfig entry. I'm not sure that was such a great idea - the module doesn't compile at least on ARM and MIPS: ARM: CC [M] drivers/scsi/advansys.o drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not properly converted to the DMA API drivers/scsi/advansys.c: In function ‘AdvBuildCarrierFreelist’: drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) drivers/scsi/advansys.c:6486: warning: ‘virt_to_bus’ is deprecated (declared at include/asm/memory.h:1 92) ... drivers/scsi/advansys.c: In function ‘advansys_get_sense_buffer_dma’: drivers/scsi/advansys.c:9885: error: implicit declaration of function ‘dma_cache_sync’ That error doesn't look to tbe the fault of the driver. dma_cache_sync() is a piece of the DMA API it looks like ARM doesn't implement? James -- Martin Michlmayr http://www.cyrius.com/ - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Remove of old NCR53C9x/esp family of drivers
On Thu, 2008-01-03 at 17:40 +0200, Boaz Harrosh wrote: As recommended by Christoph Hellwig. There is no use of Fixing these drivers, since there is a much simpler and modern esp infrastructure with David Miller's esp_scsi - Remove all driver files dependent on NCR53C9x.c deleted:drivers/scsi/NCR53C9x.c deleted:drivers/scsi/NCR53C9x.h deleted:drivers/scsi/blz1230.c deleted:drivers/scsi/blz2060.c deleted:drivers/scsi/cyberstorm.c deleted:drivers/scsi/cyberstormII.c deleted:drivers/scsi/dec_esp.c deleted:drivers/scsi/fastlane.c deleted:drivers/scsi/mac_esp.c deleted:drivers/scsi/mca_53c9x.c deleted:drivers/scsi/oktagon_esp.c deleted:drivers/scsi/oktagon_io.S deleted:drivers/scsi/sun3x_esp.c - Remove above list from drivers/scsi/Kconfig drivers/scsi/Makefile OK, I'll split this into four pieces for scsi-pending, since there are three separate interest groups with signoffs to collect (MCA, m68k and alpha) plus the core removal. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Remove of old NCR53C9x/esp family of drivers
On Thu, 3 Jan 2008, James Bottomley wrote: On Thu, 2008-01-03 at 17:40 +0200, Boaz Harrosh wrote: As recommended by Christoph Hellwig. There is no use of Fixing these drivers, since there is a much simpler and modern esp infrastructure with David Miller's esp_scsi - Remove all driver files dependent on NCR53C9x.c deleted:drivers/scsi/NCR53C9x.c deleted:drivers/scsi/NCR53C9x.h deleted:drivers/scsi/blz1230.c deleted:drivers/scsi/blz2060.c deleted:drivers/scsi/cyberstorm.c deleted:drivers/scsi/cyberstormII.c deleted:drivers/scsi/dec_esp.c deleted:drivers/scsi/fastlane.c deleted:drivers/scsi/mac_esp.c deleted:drivers/scsi/mca_53c9x.c deleted:drivers/scsi/oktagon_esp.c deleted:drivers/scsi/oktagon_io.S deleted:drivers/scsi/sun3x_esp.c - Remove above list from drivers/scsi/Kconfig drivers/scsi/Makefile OK, I'll split this into four pieces for scsi-pending, since there are three separate interest groups with signoffs to collect (MCA, m68k and alpha) plus the core removal. Anybody who can look into converting the m68k NCR53C9x drivers and has hardware to test (some of) them? I don't think we can afford losing one third of our SCSI drivers... You can use the following as guidance: commit 5ff263667798946abc15314eae3f341345877d7a Author: Thomas Bogendoerfer [EMAIL PROTECTED] Date: Tue May 22 17:03:44 2007 -0700 [SCSI] jazz_esp: Converted to use esp_core. Use new esp_scsi for JAZZ SCSI host adapter driver 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-03 at 20:05 +0100, Geert Uytterhoeven wrote: On Thu, 3 Jan 2008, James Bottomley wrote: On Thu, 2008-01-03 at 17:40 +0200, Boaz Harrosh wrote: As recommended by Christoph Hellwig. There is no use of Fixing these drivers, since there is a much simpler and modern esp infrastructure with David Miller's esp_scsi - Remove all driver files dependent on NCR53C9x.c deleted:drivers/scsi/NCR53C9x.c deleted:drivers/scsi/NCR53C9x.h deleted:drivers/scsi/blz1230.c deleted:drivers/scsi/blz2060.c deleted:drivers/scsi/cyberstorm.c deleted:drivers/scsi/cyberstormII.c deleted:drivers/scsi/dec_esp.c deleted:drivers/scsi/fastlane.c deleted:drivers/scsi/mac_esp.c deleted:drivers/scsi/mca_53c9x.c deleted:drivers/scsi/oktagon_esp.c deleted:drivers/scsi/oktagon_io.S deleted:drivers/scsi/sun3x_esp.c - Remove above list from drivers/scsi/Kconfig drivers/scsi/Makefile OK, I'll split this into four pieces for scsi-pending, since there are three separate interest groups with signoffs to collect (MCA, m68k and alpha) plus the core removal. Anybody who can look into converting the m68k NCR53C9x drivers and has hardware to test (some of) them? I don't think we can afford losing one third of our SCSI drivers... You can use the following as guidance: commit 5ff263667798946abc15314eae3f341345877d7a Author: Thomas Bogendoerfer [EMAIL PROTECTED] Date: Tue May 22 17:03:44 2007 -0700 [SCSI] jazz_esp: Converted to use esp_core. Use new esp_scsi for JAZZ SCSI host adapter driver I can also offer help to anyone who tries this. It's also a good opportunity to let die drivers that have no committed users. Just to be clear on why we're doing this: the NCR53C9x driver on which these are all based is in a pretty horrendous state of repair. The esp_scsi one is much nicer, actually nicely tested and has a host of features the old driver didn't. However, the principle driving force is the conversion of the SCSI subsystem to the sg_list accessors. esp_scsi is already coverted. NCR53C9x looks like a nasty job. Thus, the moment the conversion patch goes in, all your drivers will break. However, since breakage excites a whole bunch of kernel compile checkers (and lands me with a flood of email), I'm prepared to remove them to prevent this from happening ... unless we can get them converted over to esp_scsi. I'll put the removal in the scsi-pending tree, so it won't be picked up by -mm, but we need to get this situation resolved by 2.6.25 at the latest. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Open-FCoE on linux-scsi
From: Love, Robert W [EMAIL PROTECTED] Subject: RE: Open-FCoE on linux-scsi Date: Mon, 31 Dec 2007 08:34:38 -0800 Hello SCSI mailing list, I'd just like to introduce ourselves a bit before we get started. My name is Robert Love and I'm joined by a team of engineers including Vasu Dev, Chris Leech and Yi Zou. We are committed to maintaining the Open-FCoE project. Aside from Intel engineers we expect engineers from other companies to contribute to Open-FCoE. Our goal is to get the initiator code upstream. We have a lot of working code but recognize that we're early in this project's development. We're looking for direction from you, the experts, on what this project should grow into. I've just added a new fcoe target driver to tgt: http://stgt.berlios.de/ That's great; we'll check it out as soon as everyone is back from the holidays. It's still an experiment. Patches are welcome. The driver runs in user space unlike your target mode driver (I just modified your FCoE code to run it in user space). There seems to be a trend to move non-data-path code userspace, however, Implementing FCoE target drive in user space has no connection with a trend to move non-data-path code user space. It does all the data-path in user space. The examples of the trend to move non-data-path code userspace are open-iscsi, multi-path, etc, I think. I don't like having so much duplicate code. We were going to investigate if we could redesign the target code to have less of a profile and just depend on the initiator modules instead of recompiling openfc as openfc_tgt. What's the general opinion on this? Duplicate code vs. more kernel code? I can see that you're already starting to clean up the code that you ported. Does that mean the duplicate code isn't an issue to you? When we fix bugs in the initiator they're not going to make it into your tree unless you're diligent about watching the list. It's hard to convince the kernel maintainers to merge something into mainline that which can be implemented in user space. I failed twice (with two iSCSI target implementations). Yeah, duplication is not good but the user space code has some great advantages. Both approaches have the pros and cons. The initiator driver succeeded to log in a target, see logical units, and perform some I/Os. It's still very unstable but it would be useful for FCoE developers. I would like to help you push the Open-FCoE initiator to mainline too. What are on your todo list and what you guys working on now? We would really appreciate the help! The best way I could come up with to coordinate this effort was through the BZ- http://open-fcoe.org/bugzilla. I was going to write a BZ wiki entry to help new contributors, but since I haven't yet, here's the bottom line. Sign-up to the BZ, assign bugs to yourself from my name (I'm the default assignee now) and also file bugs as you find them. I don't want to impose much process, but this will allow all of us to know what everyone else is working on. The main things that I think need to be fixed are (in no particular order)- 1) Stability- Just straight up bug fixing. This is ongoing and everyone is looking at bugs. Talking about stability is a bit premature, I think. The first thing to do is finding a design that can be accepted into mainline. How can we get this started? We've provided our current solution, but need feedback to guide us in the right direction. We've received little quips about libsa and libcrc and now it looks like we should look at what we can move to userspace (see below), but that's all the feedback we've got so far. Can you tell us what you think about our current architecture? Then we could discuss your concerns... 2) Abstractions- We consider libsa a big bug, which we're trying to strip down piece by piece. Vasu took out the LOG_SA code and I'm looking into changing the ASSERTs to BUG_ON/WARN_ONs. That isn't all of it, but that's how we're breaking it down. Agreed, libsa (and libcrc) should be removed. 3) Target- The duplicate code of the target is too much. I want to integrate the target into our -upstream tree. Without doing that, fixes to the -upstream tree won't benefit the target and it will get into worse shape than it already is, unless someone is porting those patches to the target too. I think that ideally we'd want to reduce the target's profile and move it to userspace under tgt. 4) Userspace/Kernel interaction- It's our belief that netlink is the preferred mechanism for kernel/userspace interaction. Yi has converted the FCoE ioctl code to netlink and is looking into openfc next. There are other options and I'm not sure that netlink is the best. I think that there is no general consensus about the best mechanism for kernel/userspace interaction. Even ioctl is still accepted into mainline (e.g. kvm). I expect you get an idea to use netlink from open-iscsi, but unlike
Re: [PATCH 1/1] Remove of old NCR53C9x/esp family of drivers
From: Geert Uytterhoeven [EMAIL PROTECTED] Date: Thu, 3 Jan 2008 20:05:27 +0100 (CET) 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... We can't wait forever, and effort spent maintaining the old rotting drivers is effort that should instead be spent on the converted new drivers. Instead of seeing conversions being written, we've been hearing this swan song from the m68k crowd forever, it's getting quite old. I totally object to keeping these things around any longer. And this I support these changes going in. Acked-by: David S. Miller [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 1/1] Remove of old NCR53C9x/esp family of drivers
Hi all, 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 can look into converting some (having worked on the m68k Mac ESP driver in the past - I do recall the Mac driver needs special hacks so it won't be the easiest one to convert). I have no hardware to test these on, however. You can use the following as guidance: commit 5ff263667798946abc15314eae3f341345877d7a Author: Thomas Bogendoerfer [EMAIL PROTECTED] Date: Tue May 22 17:03:44 2007 -0700 [SCSI] jazz_esp: Converted to use esp_core. Use new esp_scsi for JAZZ SCSI host adapter driver Hasn't this come up before on linux-m68k? Someone asked me for information when converting the Mac driver, I think. That could be a good start. I can also offer help to anyone who tries this. It's also a good opportunity to let die drivers that have no committed users. I'll contact you about this, then. Just to be clear on why we're doing this: the NCR53C9x driver on which these are all based is in a pretty horrendous state of repair. The esp_scsi one is much nicer, actually nicely tested and has a host of features the old driver didn't. However, the principle driving force is the conversion of the SCSI subsystem to the sg_list accessors. esp_scsi is already coverted. NCR53C9x looks like a nasty job. Thus, the moment the conversion patch goes in, all your drivers will break. However, Hmm, does that also affect another of the m68k drivers, the 5380 one? More headache for me... Michael - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: INITIO scsi driver fails to work properly
First of all let me wish a happy new year. I come back from the vacations and i compiled the initio driver with #define DEBUG_INTERRUPT 1 #define DEBUG_QUEUE 1 #define DEBUG_STATE 1 #define INT_DISC1 I used the sources from 2.6.24-rc6-git9 kernel. At kernel boot time the initio driver prints the following: scsi: Initio INI-9X00U/UW SCSI device driver Find scb at c0c0 Append pend scb c0c0; After 3 seconds the whole system freezes there and i have to reboot. P.S here is the info from 'lspci -vv' running 2.6.16.13 kernel: 00:08.0 SCSI storage controller: Initio Corporation 360P (rev 02) Subsystem: Unknown device 9292:0202 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- Latency: 32, Cache Line Size 08 Interrupt: pin A routed to IRQ 11 Region 0: I/O ports at d000 [size=256] Region 1: Memory at ef00 (32-bit, non-prefetchable) [size=4K] [virtual] Expansion ROM at 5000 [disabled] [size=128K] - 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
[2.6.24-rc BUGFIX] SRP transport: only remove our own entries
The SCSI SRP transport class currently iterates over all children devices of the host that is being removed in srp_remove_host(). However, not all of those children were created by the SRP transport, and removing them will cause corruption and an oops when their creator tries to remove them. Signed-off-by: David Dillow [EMAIL PROTECTED] Acked-by: FUJITA Tomonori [EMAIL PROTECTED] --- On Fri, 2008-01-04 at 09:47 +0900, FUJITA Tomonori wrote: On Thu, 03 Jan 2008 15:51:25 -0500 I think that this is the root problem and the patch fixes it in the right way. Please send this patch to linux-scsi@vger.kernel.org and a patch to move srp_remove_host before scsi_remove_host in srp_remove_one to Roland. Acked-by: FUJITA Tomonori [EMAIL PROTECTED] diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index 44a340b..65c584d 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -265,7 +265,8 @@ EXPORT_SYMBOL_GPL(srp_rport_del); static int do_srp_rport_del(struct device *dev, void *data) { - srp_rport_del(dev_to_rport(dev)); + if (scsi_is_srp_rport(dev)) + srp_rport_del(dev_to_rport(dev)); return 0; } - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6.24-rc BUGFIX] SRP transport: only remove our own entries
On Thu, 03 Jan 2008 21:34:49 -0500 Dave Dillow [EMAIL PROTECTED] wrote: The SCSI SRP transport class currently iterates over all children devices of the host that is being removed in srp_remove_host(). However, not all of those children were created by the SRP transport, and removing them will cause corruption and an oops when their creator tries to remove them. Signed-off-by: David Dillow [EMAIL PROTECTED] Acked-by: FUJITA Tomonori [EMAIL PROTECTED] --- Thanks! James, please put this patch into scsi-rc-fixes. On Fri, 2008-01-04 at 09:47 +0900, FUJITA Tomonori wrote: On Thu, 03 Jan 2008 15:51:25 -0500 I think that this is the root problem and the patch fixes it in the right way. Please send this patch to linux-scsi@vger.kernel.org and a patch to move srp_remove_host before scsi_remove_host in srp_remove_one to Roland. Acked-by: FUJITA Tomonori [EMAIL PROTECTED] diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index 44a340b..65c584d 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -265,7 +265,8 @@ EXPORT_SYMBOL_GPL(srp_rport_del); static int do_srp_rport_del(struct device *dev, void *data) { - srp_rport_del(dev_to_rport(dev)); + if (scsi_is_srp_rport(dev)) + srp_rport_del(dev_to_rport(dev)); return 0; } - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] scsi: Convert everyone to scsi_sglist and scsi_sg_count
On Thursday 03 January 2008 20:26:13 Boaz Harrosh wrote: On Thu, Jan 03 2008 at 10:50 +0200, Rusty Russell [EMAIL PROTECTED] wrote: This patch simply converts direct uses of -use_sg and -request_buffer to use the wrapper macros. This removes the assumption that the sg list is overloaded on request_buffer, and that there's an explicit use_sg field. All of these drivers are properly converted in current scsi-misc + scsi-pending. If you are really serious about changing scsi-layer you better work ontop of scsi git trees. Hi Boaz, Well, I wouldn't say I'm serious about SCSI 8) but I am delighted to see this being done. Have grabbed scsi-pending from git.kernel.org, thanks for the hint. Also you can inspect -mm tree it has the scsi_data_buffer patches that does 4/5 what you want. Remember, these scsi patches are a side-effect of trying to get my own sg-using code sane. So this is exactly what I *don't* want: another works for scsi solution :( I'll see where we can go from here... Thanks, Rusty. - 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