Re: [PATCH 0/5] sg_ring for scsi
On Thu, Dec 20 2007, Rusty Russell wrote: On Thursday 20 December 2007 18:07:41 FUJITA Tomonori wrote: On Thu, 20 Dec 2007 16:45:18 +1100 Rusty Russell [EMAIL PROTECTED] wrote: OK, some fixes since last time, as I wade through more SCSI drivers. Some drivers use use_sg as a flag to know whether the request_buffer is a scatterlist: I don't need the counter, but I still need the flag, so I fixed that in a more intuitive way (an explicit -sg pointer in the cmd). use_sg and the request_buffer will be removed shortly. http://marc.info/?l=linux-scsim=119754650614813w=2 Thanks! Is there a git tree somewhere with these changes? I think that we tried the similar idea before, scsi_sgtable, but we seem to settle in the current simple approach. Yes, a scsi-specific solution is a bad idea: other people use sg. Manipulating the magic chains is horrible; it looks simple to the places which simply want to iterate through it, but it's awful for code which wants to create them. The current code looks like that to minimize impact on 2.6.24, see this branch: http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=sg for how it folds into lib/sg.c and the magic disappears from SCSI. Rusty, nobody claimed the sg code in 2.6.24 is perfect. I like to get things incrementally there. -- Jens Axboe - 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/6] zfcp fixes for scsi-misc
James, here are some zfcp fixes. They apply on the current version of scsi-misc. Regards, Christof Schmitt - 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 3/6] zfcp: Hold queue lock when checking port/unit handle for abort command
From: Christof Schmitt [EMAIL PROTECTED]
We need to hold the queue-lock when checking whether we still have a valid
unit/port handle for the abort command, i.e whether we can issue this request
for this unit/port. If the error recovery is about to close this unit/port,
then it competes for the queue-lock. If the close request issued by the error
recovery wins, then it is guaranteed that this unit/port has been blocked for
other requests.
Signed-off-by: Christof Schmitt [EMAIL PROTECTED]
Signed-off-by: Martin Peschke [EMAIL PROTECTED]
---
drivers/s390/scsi/zfcp_fsf.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
--- a/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:03.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:23.0 +0100
@@ -1116,6 +1116,10 @@ zfcp_fsf_abort_fcp_command(unsigned long
goto out;
}
+ if (unlikely(!atomic_test_mask(ZFCP_STATUS_COMMON_UNBLOCKED,
+ unit-status)))
+ goto unit_blocked;
+
sbale = zfcp_qdio_sbale_req(fsf_req, fsf_req-sbal_curr, 0);
sbale[0].flags |= SBAL_FLAGS0_TYPE_READ;
sbale[1].flags |= SBAL_FLAGS_LAST_ENTRY;
@@ -1131,22 +1135,13 @@ zfcp_fsf_abort_fcp_command(unsigned long
zfcp_fsf_start_timer(fsf_req, ZFCP_SCSI_ER_TIMEOUT);
retval = zfcp_fsf_req_send(fsf_req);
- if (retval) {
- ZFCP_LOG_INFO(error: Failed to send abort command request
- on adapter %s, port 0x%016Lx, unit 0x%016Lx\n,
- zfcp_get_busid_by_adapter(adapter),
- unit-port-wwpn, unit-fcp_lun);
+ if (!retval)
+ goto out;
+
+ unit_blocked:
zfcp_fsf_req_free(fsf_req);
fsf_req = NULL;
- goto out;
- }
- ZFCP_LOG_DEBUG(Abort FCP Command request initiated
- (adapter%s, port d_id=0x%06x,
- unit x%016Lx, old_req_id=0x%lx)\n,
- zfcp_get_busid_by_adapter(adapter),
- unit-port-d_id,
- unit-fcp_lun, old_req_id);
out:
write_unlock_irqrestore(adapter-request_queue.queue_lock, lock_flags);
return fsf_req;
--
-
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 4/6] zfcp: Hold queue lock when checking port handle for ELS command
From: Christof Schmitt [EMAIL PROTECTED]
We need to hold the queue-lock when checking whether we still have a valid port
handle for the ELS command, i.e whether we can issue this request for this
port. If the error recovery is about to close this port, then it competes for
the queue-lock. If the close request issued by the error recovery wins, then it
is guaranteed that this port has been blocked for other requests.
Signed-off-by: Christof Schmitt [EMAIL PROTECTED]
Signed-off-by: Martin Peschke [EMAIL PROTECTED]
---
drivers/s390/scsi/zfcp_erp.c |2 +-
drivers/s390/scsi/zfcp_fsf.c |7 +++
2 files changed, 8 insertions(+), 1 deletion(-)
--- a/drivers/s390/scsi/zfcp_erp.c 2007-12-20 11:17:46.0 +0100
+++ b/drivers/s390/scsi/zfcp_erp.c 2007-12-20 11:18:42.0 +0100
@@ -456,7 +456,7 @@ zfcp_test_link(struct zfcp_port *port)
zfcp_port_get(port);
retval = zfcp_erp_adisc(port);
- if (retval != 0) {
+ if (retval != 0 retval != -EBUSY) {
zfcp_port_put(port);
ZFCP_LOG_NORMAL(reopen needed for port 0x%016Lx
on adapter %s\n , port-wwpn,
--- a/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:23.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:42.0 +0100
@@ -1668,6 +1668,12 @@ zfcp_fsf_send_els(struct zfcp_send_els *
goto failed_req;
}
+ if (unlikely(!atomic_test_mask(ZFCP_STATUS_COMMON_UNBLOCKED,
+ els-port-status))) {
+ ret = -EBUSY;
+ goto port_blocked;
+ }
+
sbale = zfcp_qdio_sbale_req(fsf_req, fsf_req-sbal_curr, 0);
if (zfcp_use_one_sbal(els-req, els-req_count,
els-resp, els-resp_count)){
@@ -1749,6 +1755,7 @@ zfcp_fsf_send_els(struct zfcp_send_els *
0x%06x)\n, zfcp_get_busid_by_adapter(adapter), d_id);
goto out;
+ port_blocked:
failed_send:
zfcp_fsf_req_free(fsf_req);
--
-
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 6/6] zfcp: Hold queue lock when checking port/unit handle for task management cmd
From: Christof Schmitt [EMAIL PROTECTED]
We need to hold the queue-lock when checking whether we still have a valid
unit/port handle for the task management command, i.e whether we can issue this
request for this unit/port. If the error recovery is about to close this
unit/port, then it competes for the queue-lock. If the close request issued by
the error recovery wins, then it is guaranteed that this unit/port has been
blocked for other requests.
Signed-off-by: Christof Schmitt [EMAIL PROTECTED]
Signed-off-by: Martin Peschke [EMAIL PROTECTED]
---
drivers/s390/scsi/zfcp_fsf.c | 26 +-
1 file changed, 9 insertions(+), 17 deletions(-)
--- a/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:19:01.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:19:19.0 +0100
@@ -3774,6 +3774,10 @@ zfcp_fsf_send_fcp_command_task_managemen
goto out;
}
+ if (unlikely(!atomic_test_mask(ZFCP_STATUS_COMMON_UNBLOCKED,
+ unit-status)))
+ goto unit_blocked;
+
/*
* Used to decide on proper handler in the return path,
* could be either zfcp_fsf_send_fcp_command_task_handler or
@@ -3807,25 +3811,13 @@ zfcp_fsf_send_fcp_command_task_managemen
zfcp_fsf_start_timer(fsf_req, ZFCP_SCSI_ER_TIMEOUT);
retval = zfcp_fsf_req_send(fsf_req);
- if (retval) {
- ZFCP_LOG_INFO(error: Could not send an FCP-command (task
- management) on adapter %s, port 0x%016Lx for
- unit LUN 0x%016Lx\n,
- zfcp_get_busid_by_adapter(adapter),
- unit-port-wwpn,
- unit-fcp_lun);
- zfcp_fsf_req_free(fsf_req);
- fsf_req = NULL;
+ if (!retval)
goto out;
- }
- ZFCP_LOG_TRACE(Send FCP Command (task management function) initiated
- (adapter %s, port 0x%016Lx, unit 0x%016Lx,
- tm_flags=0x%x)\n,
- zfcp_get_busid_by_adapter(adapter),
- unit-port-wwpn,
- unit-fcp_lun,
- tm_flags);
+ unit_blocked:
+ zfcp_fsf_req_free(fsf_req);
+ fsf_req = NULL;
+
out:
write_unlock_irqrestore(adapter-request_queue.queue_lock, lock_flags);
return fsf_req;
--
-
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/5] sg_ring for scsi
On Thu, Dec 20 2007 at 9:58 +0200, Jens Axboe [EMAIL PROTECTED] wrote: On Thu, Dec 20 2007, Rusty Russell wrote: On Thursday 20 December 2007 18:07:41 FUJITA Tomonori wrote: On Thu, 20 Dec 2007 16:45:18 +1100 Rusty Russell [EMAIL PROTECTED] wrote: OK, some fixes since last time, as I wade through more SCSI drivers. Some drivers use use_sg as a flag to know whether the request_buffer is a scatterlist: I don't need the counter, but I still need the flag, so I fixed that in a more intuitive way (an explicit -sg pointer in the cmd). use_sg and the request_buffer will be removed shortly. http://marc.info/?l=linux-scsim=119754650614813w=2 Thanks! Is there a git tree somewhere with these changes? I think that we tried the similar idea before, scsi_sgtable, but we seem to settle in the current simple approach. Yes, a scsi-specific solution is a bad idea: other people use sg. Manipulating the magic chains is horrible; it looks simple to the places which simply want to iterate through it, but it's awful for code which wants to create them. The current code looks like that to minimize impact on 2.6.24, see this branch: http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=sg for how it folds into lib/sg.c and the magic disappears from SCSI. Rusty, nobody claimed the sg code in 2.6.24 is perfect. I like to get things incrementally there. Dear Jens. Is this code scheduled for 2.6.25? I cannot find it in mm tree. Because above code conflicts with the scsi_data_buffer patch, that is in mm tree and I hope will get accepted into 2.6.25. Now the concepts are not at all conflicting, only that they both bang in the same places. (And by the way it does not apply on scsi-misc either. And it did not compile in your tree, a missing bit in ide-scsi.c) I have rebase and fixed your code on top of scsi_data_buffer patchset. Please review. (Patchset posted as reply to this mail) They are totaly untested, based on -mm tree. We should decide the order of these patches and rebase accordingly. AND ... Please send, to-be-included-in-next-kernel patches to Morton. This way we can account for them. Also I do not see Ack-by: of the scsi maintainer in the scsi bits of your patches. Is it not a costume to Ack-on bits that belong to other maintainers, even for 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
[PATCH 2/3] SG: Convert SCSI to use scatterlist helpers for sg chaining
From: Jens Axboe [EMAIL PROTECTED]
Signed-off-by: Jens Axboe [EMAIL PROTECTED]
---
drivers/scsi/libsrp.c|2 +-
drivers/scsi/scsi_error.c|4 +-
drivers/scsi/scsi_lib.c | 150 +-
drivers/usb/storage/isd200.c |4 +-
include/scsi/scsi_cmnd.h |9 +--
5 files changed, 24 insertions(+), 145 deletions(-)
diff --git a/drivers/scsi/libsrp.c b/drivers/scsi/libsrp.c
index 8a8562a..b81350d 100644
--- a/drivers/scsi/libsrp.c
+++ b/drivers/scsi/libsrp.c
@@ -427,7 +427,7 @@ int srp_cmd_queue(struct Scsi_Host *shost, struct srp_cmd
*cmd, void *info,
sc-SCp.ptr = info;
memcpy(sc-cmnd, cmd-cdb, MAX_COMMAND_SIZE);
sc-sdb.length = len;
- sc-sdb.sglist = (void *) (unsigned long) addr;
+ sc-sdb.sgt.sgl = (void *) (unsigned long) addr;
sc-tag = tag;
err = scsi_tgt_queue_command(sc, itn_id, (struct scsi_lun *)cmd-lun,
cmd-tag);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 5c8ba6a..1fd2a8c 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -629,9 +629,9 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct
scsi_eh_save *ses,
sizeof(scmd-sense_buffer), sense_bytes);
sg_init_one(ses-sense_sgl, scmd-sense_buffer,
scmd-sdb.length);
- scmd-sdb.sglist = ses-sense_sgl;
+ scmd-sdb.sgt.sgl = ses-sense_sgl;
scmd-sc_data_direction = DMA_FROM_DEVICE;
- scmd-sdb.sg_count = 1;
+ scmd-sdb.sgt.nents = 1;
memset(scmd-cmnd, 0, sizeof(scmd-cmnd));
scmd-cmnd[0] = REQUEST_SENSE;
scmd-cmnd[4] = scmd-sdb.length;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a6aae56..c7107f1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -750,136 +750,15 @@ static inline unsigned int scsi_sgtable_index(unsigned
short nents)
return index;
}
-static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb,
- unsigned short sg_count, gfp_t gfp_mask)
+static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)
{
- struct scsi_host_sg_pool *sgp;
- struct scatterlist *sgl, *prev, *ret;
- unsigned int index;
- int this, left;
-
- left = sg_count;
- ret = prev = NULL;
- do {
- this = left;
- if (this SCSI_MAX_SG_SEGMENTS) {
- this = SCSI_MAX_SG_SEGMENTS - 1;
- index = SG_MEMPOOL_NR - 1;
- } else
- index = scsi_sgtable_index(this);
-
- left -= this;
-
- sgp = scsi_sg_pools + index;
-
- sgl = mempool_alloc(sgp-pool, gfp_mask);
- if (unlikely(!sgl))
- goto enomem;
-
- sg_init_table(sgl, sgp-size);
-
- /*
-* first loop through, set initial index and return value
-*/
- if (!ret)
- ret = sgl;
-
- /*
-* chain previous sglist, if any. we know the previous
-* sglist must be the biggest one, or we would not have
-* ended up doing another loop.
-*/
- if (prev)
- sg_chain(prev, SCSI_MAX_SG_SEGMENTS, sgl);
-
- /*
-* if we have nothing left, mark the last segment as
-* end-of-list
-*/
- if (!left)
- sg_mark_end(sgl[this - 1]);
-
- /*
-* don't allow subsequent mempool allocs to sleep, it would
-* violate the mempool principle.
-*/
- gfp_mask = ~__GFP_WAIT;
- gfp_mask |= __GFP_HIGH;
- prev = sgl;
- } while (left);
-
- /*
-* -use_sg may get modified after dma mapping has potentially
-* shrunk the number of segments, so keep a copy of it for free.
-*/
- sdb-alloc_sg_count = sdb-sg_count = sg_count;
- sdb-sglist = ret;
- return 0;
-enomem:
- if (ret) {
- /*
-* Free entries chained off ret. Since we were trying to
-* allocate another sglist, we know that all entries are of
-* the max size.
-*/
- sgp = scsi_sg_pools + SG_MEMPOOL_NR - 1;
- prev = ret;
- ret = ret[SCSI_MAX_SG_SEGMENTS - 1];
-
- while ((sgl = sg_chain_ptr(ret)) != NULL) {
- ret = sgl[SCSI_MAX_SG_SEGMENTS - 1];
- mempool_free(sgl, sgp-pool);
- }
-
- mempool_free(prev, sgp-pool);
-
Re: [PATCH 1/3] SG: Move functions to lib/scatterlist.c and add sg chaining allocator helpers
A small comment in body
On Thu, Dec 20 2007 at 16:00 +0200, Boaz Harrosh [EMAIL PROTECTED] wrote:
Manually doing chained sg lists is not trivial, so add some helpers
to make sure that drivers get it right.
Signed-off-by: Jens Axboe [EMAIL PROTECTED]
---
include/linux/scatterlist.h | 125 ---
lib/Makefile|2 +-
lib/scatterlist.c | 281
+++
3 files changed, 307 insertions(+), 101 deletions(-)
create mode 100644 lib/scatterlist.c
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 416e000..c3ca848 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -7,6 +7,12 @@
#include linux/string.h
#include asm/io.h
+struct sg_table {
+ struct scatterlist *sgl;/* the list */
+ unsigned int nents; /* number of mapped entries */
+ unsigned int orig_nents;/* original size of list */
+};
+
/*
* Notes on SG table design.
*
@@ -106,31 +112,6 @@ static inline void sg_set_buf(struct scatterlist *sg,
const void *buf,
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
}
-/**
- * sg_next - return the next scatterlist entry in a list
- * @sg: The current sg entry
- *
- * Description:
- * Usually the next entry will be @sg@ + 1, but if this sg element is part
- * of a chained scatterlist, it could jump to the start of a new
- * scatterlist array.
- *
- **/
-static inline struct scatterlist *sg_next(struct scatterlist *sg)
-{
-#ifdef CONFIG_DEBUG_SG
- BUG_ON(sg-sg_magic != SG_MAGIC);
-#endif
- if (sg_is_last(sg))
- return NULL;
-
- sg++;
- if (unlikely(sg_is_chain(sg)))
- sg = sg_chain_ptr(sg);
-
- return sg;
-}
-
/*
* Loop over each sg element, following the pointer to a new list if
necessary
*/
@@ -138,40 +119,6 @@ static inline struct scatterlist *sg_next(struct
scatterlist *sg)
for (__i = 0, sg = (sglist); __i (nr); __i++, sg = sg_next(sg))
/**
- * sg_last - return the last scatterlist entry in a list
- * @sgl: First entry in the scatterlist
- * @nents: Number of entries in the scatterlist
- *
- * Description:
- * Should only be used casually, it (currently) scan the entire list
- * to get the last entry.
- *
- * Note that the @sgl@ pointer passed in need not be the first one,
- * the important bit is that @nents@ denotes the number of entries that
- * exist from @[EMAIL PROTECTED]
- *
- **/
-static inline struct scatterlist *sg_last(struct scatterlist *sgl,
- unsigned int nents)
-{
-#ifndef ARCH_HAS_SG_CHAIN
- struct scatterlist *ret = sgl[nents - 1];
-#else
- struct scatterlist *sg, *ret = NULL;
- unsigned int i;
-
- for_each_sg(sgl, sg, nents, i)
- ret = sg;
-
-#endif
-#ifdef CONFIG_DEBUG_SG
- BUG_ON(sgl[0].sg_magic != SG_MAGIC);
- BUG_ON(!sg_is_last(ret));
-#endif
- return ret;
-}
-
-/**
* sg_chain - Chain two sglists together
* @prv: First scatterlist
* @prv_nents: Number of entries in prv
@@ -223,47 +170,6 @@ static inline void sg_mark_end(struct scatterlist *sg)
}
/**
- * sg_init_table - Initialize SG table
- * @sgl:The SG table
- * @nents: Number of entries in table
- *
- * Notes:
- * If this is part of a chained sg table, sg_mark_end() should be
- * used only on the last table part.
- *
- **/
-static inline void sg_init_table(struct scatterlist *sgl, unsigned int nents)
-{
- memset(sgl, 0, sizeof(*sgl) * nents);
-#ifdef CONFIG_DEBUG_SG
- {
- unsigned int i;
- for (i = 0; i nents; i++)
- sgl[i].sg_magic = SG_MAGIC;
- }
-#endif
- sg_mark_end(sgl[nents - 1]);
-}
-
-/**
- * sg_init_one - Initialize a single entry sg list
- * @sg: SG entry
- * @buf: Virtual address for IO
- * @buflen: IO length
- *
- * Notes:
- * This should not be used on a single entry that is part of a larger
- * table. Use sg_init_table() for that.
- *
- **/
-static inline void sg_init_one(struct scatterlist *sg, const void *buf,
-unsigned int buflen)
-{
- sg_init_table(sg, 1);
- sg_set_buf(sg, buf, buflen);
-}
-
-/**
* sg_phys - Return physical address of an sg entry
* @sg: SG entry
*
@@ -293,4 +199,23 @@ static inline void *sg_virt(struct scatterlist *sg)
return page_address(sg_page(sg)) + sg-offset;
}
+struct scatterlist *sg_next(struct scatterlist *);
I don't like that this became exported. I would prefer if
it could stay inline. if at all possible?
+struct scatterlist *sg_last(struct scatterlist *s, unsigned int);
+void sg_init_table(struct scatterlist *, unsigned int);
+void sg_init_one(struct scatterlist *, const void *,
Re: [PATCH 2/3] SG: Convert SCSI to use scatterlist helpers for sg chaining
On Thu, Dec 20 2007 at 16:03 +0200, Boaz Harrosh [EMAIL PROTECTED] wrote:
From: Jens Axboe [EMAIL PROTECTED]
Signed-off-by: Jens Axboe [EMAIL PROTECTED]
---
drivers/scsi/libsrp.c|2 +-
drivers/scsi/scsi_error.c|4 +-
drivers/scsi/scsi_lib.c | 150
+-
drivers/usb/storage/isd200.c |4 +-
include/scsi/scsi_cmnd.h |9 +--
5 files changed, 24 insertions(+), 145 deletions(-)
diff --git a/drivers/scsi/libsrp.c b/drivers/scsi/libsrp.c
index 8a8562a..b81350d 100644
--- a/drivers/scsi/libsrp.c
+++ b/drivers/scsi/libsrp.c
@@ -427,7 +427,7 @@ int srp_cmd_queue(struct Scsi_Host *shost, struct srp_cmd
*cmd, void *info,
sc-SCp.ptr = info;
memcpy(sc-cmnd, cmd-cdb, MAX_COMMAND_SIZE);
sc-sdb.length = len;
- sc-sdb.sglist = (void *) (unsigned long) addr;
+ sc-sdb.sgt.sgl = (void *) (unsigned long) addr;
sc-tag = tag;
err = scsi_tgt_queue_command(sc, itn_id, (struct scsi_lun *)cmd-lun,
cmd-tag);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 5c8ba6a..1fd2a8c 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -629,9 +629,9 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct
scsi_eh_save *ses,
sizeof(scmd-sense_buffer), sense_bytes);
sg_init_one(ses-sense_sgl, scmd-sense_buffer,
scmd-sdb.length);
- scmd-sdb.sglist = ses-sense_sgl;
+ scmd-sdb.sgt.sgl = ses-sense_sgl;
scmd-sc_data_direction = DMA_FROM_DEVICE;
- scmd-sdb.sg_count = 1;
+ scmd-sdb.sgt.nents = 1;
memset(scmd-cmnd, 0, sizeof(scmd-cmnd));
scmd-cmnd[0] = REQUEST_SENSE;
scmd-cmnd[4] = scmd-sdb.length;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a6aae56..c7107f1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -750,136 +750,15 @@ static inline unsigned int scsi_sgtable_index(unsigned
short nents)
return index;
}
-static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb,
- unsigned short sg_count, gfp_t gfp_mask)
+static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)
{
- struct scsi_host_sg_pool *sgp;
- struct scatterlist *sgl, *prev, *ret;
- unsigned int index;
- int this, left;
-
- left = sg_count;
- ret = prev = NULL;
- do {
- this = left;
- if (this SCSI_MAX_SG_SEGMENTS) {
- this = SCSI_MAX_SG_SEGMENTS - 1;
- index = SG_MEMPOOL_NR - 1;
- } else
- index = scsi_sgtable_index(this);
-
- left -= this;
-
- sgp = scsi_sg_pools + index;
-
- sgl = mempool_alloc(sgp-pool, gfp_mask);
- if (unlikely(!sgl))
- goto enomem;
-
- sg_init_table(sgl, sgp-size);
-
- /*
- * first loop through, set initial index and return value
- */
- if (!ret)
- ret = sgl;
-
- /*
- * chain previous sglist, if any. we know the previous
- * sglist must be the biggest one, or we would not have
- * ended up doing another loop.
- */
- if (prev)
- sg_chain(prev, SCSI_MAX_SG_SEGMENTS, sgl);
-
- /*
- * if we have nothing left, mark the last segment as
- * end-of-list
- */
- if (!left)
- sg_mark_end(sgl[this - 1]);
-
- /*
- * don't allow subsequent mempool allocs to sleep, it would
- * violate the mempool principle.
- */
- gfp_mask = ~__GFP_WAIT;
- gfp_mask |= __GFP_HIGH;
- prev = sgl;
- } while (left);
-
- /*
- * -use_sg may get modified after dma mapping has potentially
- * shrunk the number of segments, so keep a copy of it for free.
- */
- sdb-alloc_sg_count = sdb-sg_count = sg_count;
- sdb-sglist = ret;
- return 0;
-enomem:
- if (ret) {
- /*
- * Free entries chained off ret. Since we were trying to
- * allocate another sglist, we know that all entries are of
- * the max size.
- */
- sgp = scsi_sg_pools + SG_MEMPOOL_NR - 1;
- prev = ret;
- ret = ret[SCSI_MAX_SG_SEGMENTS - 1];
-
- while ((sgl = sg_chain_ptr(ret)) != NULL) {
- ret = sgl[SCSI_MAX_SG_SEGMENTS - 1];
- mempool_free(sgl, sgp-pool);
- }
-
-
Re: INITIO scsi driver fails to work properly
On Thu, Dec 20, 2007 at 01:32:02AM -0800, Natalie Protasevich wrote: The problem is that it appears to the casual observer as if they can then add information to the bug through the web interface. But that information will never be forwarded to the mailing list. Unless there's a way of marking bugs as 'unchangable through the web interface' or 'all messages appended to this bug need to be forwarded', Bugzilla just doesn't fit our needs. The Debian BTS fits our way of working much better. Perhaps somebody should investigate a migration. This is excellent observation by Matthew and James. There is no magic in bugzilla not being loved, it is just not the right set of features for effective work on a problem. It doesn't support multiple developer' collaboration well. Actually, Bugzilla *could* be configured so that, say, linux-scsi was copied for all SCSI bugs (linux-scsi could just be added to the cc list). The problem though is that Bugzilla will then proceed to cc linux-scsi for all the Bugzilla state change details which might annoy the denizens of the linux-scsi list. But if new entries on the Bugzilla entry could be set to forward to the appropriate mailing list with the messaging *looking* a lot more like a mail message, I suspect it could be acceptable. One of the advantages of the Debian BTS is that it's much more integrated into the e-mail workflow. (Although it lacks the roll up and reporting capabilities that are beloved by managers...) But hey, it could be worse. We could have chosen the Sourceforge bug tracker. :-) - Ted - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: INITIO scsi driver fails to work properly
On Thu, 2007-12-20 at 01:32 -0800, Natalie Protasevich wrote: On Dec 19, 2007 9:05 AM, Matthew Wilcox [EMAIL PROTECTED] wrote: On Wed, Dec 19, 2007 at 10:50:40AM -0600, James Bottomley wrote: So, to get the best of both worlds, file a bugzilla and note the bugid. Then email a complete report to the relevant list, but add [BUG bugid] to the subject line and cc [EMAIL PROTECTED] If you do this, bugzilla will keep track of the entire discussion as it progresses and allow those who track bugs through bugzilla to get a pretty accurate idea of the status. You should never need to touch bugzilla again once the initial bug report is filed: all future information flow is via the mailing lists. The problem is that it appears to the casual observer as if they can then add information to the bug through the web interface. But that information will never be forwarded to the mailing list. Unless there's a way of marking bugs as 'unchangable through the web interface' or 'all messages appended to this bug need to be forwarded', Bugzilla just doesn't fit our needs. The Debian BTS fits our way of working much better. Perhaps somebody should investigate a migration. This is excellent observation by Matthew and James. There is no magic in bugzilla not being loved, it is just not the right set of features for effective work on a problem. It doesn't support multiple developer' collaboration well. This distaste is not universal, since some people don't have a problem with bugzilla as is, maybe those who tend to work on problems alone... But making it to be a workable tool for everyone is definitely worth it. Any other favorite bugzillas that are nice to work with and that have the advantage above? We have actually been trying for over two years to get bugzilla fixed so that it suits our email and list publishing workflow for fixing bugs. I surmise that 90% of our problems with bugzilla could be solved if it simply tipped a SCSI bug report onto the SCSI list when it was created in such a way that all replies were gathered back into bugzilla. Unfortunately, no-one who maintains our bugzilla has actually been able to make this happen. The other 10% of the problem is that bugzilla doesn't seem to have a way properly to integrate people who insist on using its web interface to reply into the email flow. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHSET 0/3] varlen extended and vendor-specific cdbs
On Tue, Dec 18 2007 at 15:33 +0200, Boaz Harrosh [EMAIL PROTECTED] wrote: Submitted is a patchset for adding support for variable-length, extended, and vendor specific CDBs. It should now cover the entire range of the SCSI standard. snip ping James. I understand that this is the last thing on your mind. But could we at least agree at the scsi_cmnd API, which is, No API. That is: scsi_cmnd-cmnd changes to a pointer that can point to a user buffer. Andrew Hi! It would be grate if you could put these patches in next -mm for testing. the first patch in this series has a potential of been dangerous. Since I changed an embedded array to a pointer, places that did cmd-cmnd will no longer work. I have scanned all occurrences of \bcmnd\b but I might have missed a place. Thanks Boaz - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix page_alloc for larger I/O segments (improved)
On Fri, Dec 14, 2007 at 11:13:40AM -0700, Matthew Wilcox wrote: I'll send it to our DB team to see if this improves our numbers at all. It does, by approximately 0.67%. This is about double the margin of error, and a significant improvement. Thanks! -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. - 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/5] dma_map_sg_ring() helper
On Thursday 20 December 2007 18:42:44 David Miller wrote:
From: FUJITA Tomonori [EMAIL PROTECTED]
Date: Thu, 20 Dec 2007 16:06:31 +0900
On Thu, 20 Dec 2007 16:49:30 +1100
Rusty Russell [EMAIL PROTECTED] wrote:
+/**
+ * dma_map_sg_ring - Map an entire sg ring
+ * @dev: Device to free noncoherent memory for
+ * @sg: The sg_ring
+ * @direction: DMA_TO_DEVICE, DMA_FROM_DEVICE or DMA_BIDIRECTIONAL.
+ *
+ * This returns -ENOMEM if mapping fails. It's not clear that telling
you + * it failed is useful though.
+ */
+int dma_map_sg_ring(struct device *dev, struct sg_ring *sg,
+ enum dma_data_direction direction)
+{
+ struct sg_ring *i;
+ unsigned int num;
+
+ for (i = sg; i; i = sg_ring_next(i, sg)) {
+ BUG_ON(i-num i-max);
+ num = dma_map_sg(dev, i-sg, i-num, direction);
+ if (num == 0 i-num != 0)
+ goto unmap;
+ }
+ return 0;
I don't think that this works for IOMMUs that could merge sg entries.
Right, it won't work at all.
The caller has to be told how many DMA entries it really did use to
compose the mapping, and there has to be a way to properly iterate
over them. The assumption that the IOMMU will map the SG entries
1-to-1 is invalid.
Good catch. Indeed, what's missing is one line:
i-num = num;
Of course, an arch-specific version of this could merge between sg_rings,
too, but that's an optimization.
Thanks,
Rusty.
dma_map_sg_ring() helper
Obvious counterpart to dma_map_sg. Note that this is arch-independent
code; sg_rings are backwards compatible with simple sg arrays.
Signed-off-by: Rusty Russell [EMAIL PROTECTED]
---
drivers/base/dma-mapping.c | 13 +
include/linux/dma-mapping.h |4
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -8,6 +8,7 @@
*/
#include linux/dma-mapping.h
+#include linux/sg_ring.h
/*
* Managed DMA API
@@ -162,6 +163,60 @@ void dmam_free_noncoherent(struct device
}
EXPORT_SYMBOL(dmam_free_noncoherent);
+/**
+ * dma_map_sg_ring - Map an entire sg ring
+ * @dev: Device to free noncoherent memory for
+ * @sg: The sg_ring
+ * @direction: DMA_TO_DEVICE, DMA_FROM_DEVICE or DMA_BIDIRECTIONAL.
+ *
+ * This returns -ENOMEM if mapping fails. It's not clear that telling you
+ * it failed is useful though.
+ */
+int dma_map_sg_ring(struct device *dev, struct sg_ring *sg,
+enum dma_data_direction direction)
+{
+ struct sg_ring *i;
+ unsigned int num;
+
+ for (i = sg; i; i = sg_ring_next(i, sg)) {
+ BUG_ON(i-num i-max);
+ num = dma_map_sg(dev, i-sg, i-num, direction);
+ if (num == 0 i-num != 0)
+ goto unmap;
+ i-num = num;
+ }
+ return 0;
+
+unmap:
+ while (sg) {
+ dma_unmap_sg(dev, sg-sg, sg-num, direction);
+ sg = sg_ring_next(sg, i);
+ }
+ return -ENOMEM;
+
+}
+EXPORT_SYMBOL(dma_map_sg_ring);
+
+/**
+ * dma_unmap_sg_ring - Unmap an entire sg ring
+ * @dev: Device to free noncoherent memory for
+ * @sg: The sg_ring
+ * @direction: DMA_TO_DEVICE, DMA_FROM_DEVICE or DMA_BIDIRECTIONAL.
+ *
+ * Call after dma_map_sg_ring() succeeds.
+ */
+void dma_unmap_sg_ring(struct device *dev, struct sg_ring *sg,
+ enum dma_data_direction direction)
+{
+ struct sg_ring *i;
+
+ for (i = sg; i; i = sg_ring_next(i, sg)) {
+ BUG_ON(i-num i-max);
+ dma_unmap_sg(dev, i-sg, i-num, direction);
+ }
+}
+EXPORT_SYMBOL(dma_unmap_sg_ring);
+
#ifdef ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY
static void dmam_coherent_decl_release(struct device *dev, void *res)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -87,6 +87,12 @@ dma_mark_declared_memory_occupied(struct
}
#endif
+struct sg_ring;
+extern int dma_map_sg_ring(struct device *dev, struct sg_ring *sg,
+ enum dma_data_direction direction);
+extern void dma_unmap_sg_ring(struct device *dev, struct sg_ring *sg,
+ enum dma_data_direction direction);
+
/*
* Managed 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 0/5] sg_ring for scsi
On Thursday 20 December 2007 18:58:07 David Miller wrote: From: Rusty Russell [EMAIL PROTECTED] Date: Thu, 20 Dec 2007 18:53:48 +1100 Manipulating the magic chains is horrible; it looks simple to the places which simply want to iterate through it, but it's awful for code which wants to create them. I'm not saying complexity is inherent in this stuff, but assuming that it is the complexity should live as far away from the minions (the iterators in this case). Therefore, the creators is the right spot for the hard stuff. In this case, the main benefit of the sg chaining was that the conversion of most scsi drivers was easy (basically sg++ - sg = sg_next(sg)). The conversion to sg_ring is more complex, but the end result is not significantly more complex. However, the cost to code which manipulates sg chains was significant: I tried using them in virtio and it was too ugly to live (so that doesn't support sg chaining). If this was the best we could do, that'd be fine. But, as demonstrated, there are real benefits of having an explicit header: 1) It removes the chain-end/explicit count ambiguity (see http://lkml.org/lkml/2007/10/25/209 thread) 2) It allows others to manipulate sg chains, which couldn't be done before (eg. the ATA code which wants to append a padding element). 3) I can now hand you an sg ring for you to fill: sg chains can't do that. In short, sg_ring is generally useful primitive. sg chains are a clever hack for scsi_lib to create, and everyone else to read. Hope that clarifies, 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
Re: [PATCH 2/5] dma_map_sg_ring() helper
On Fri, 21 Dec 2007 09:58:56 +1100
Rusty Russell [EMAIL PROTECTED] wrote:
On Thursday 20 December 2007 18:42:44 David Miller wrote:
From: FUJITA Tomonori [EMAIL PROTECTED]
Date: Thu, 20 Dec 2007 16:06:31 +0900
On Thu, 20 Dec 2007 16:49:30 +1100
Rusty Russell [EMAIL PROTECTED] wrote:
+/**
+ * dma_map_sg_ring - Map an entire sg ring
+ * @dev: Device to free noncoherent memory for
+ * @sg: The sg_ring
+ * @direction: DMA_TO_DEVICE, DMA_FROM_DEVICE or DMA_BIDIRECTIONAL.
+ *
+ * This returns -ENOMEM if mapping fails. It's not clear that telling
you + * it failed is useful though.
+ */
+int dma_map_sg_ring(struct device *dev, struct sg_ring *sg,
+enum dma_data_direction direction)
+{
+ struct sg_ring *i;
+ unsigned int num;
+
+ for (i = sg; i; i = sg_ring_next(i, sg)) {
+ BUG_ON(i-num i-max);
+ num = dma_map_sg(dev, i-sg, i-num, direction);
+ if (num == 0 i-num != 0)
+ goto unmap;
+ }
+ return 0;
I don't think that this works for IOMMUs that could merge sg entries.
Right, it won't work at all.
The caller has to be told how many DMA entries it really did use to
compose the mapping, and there has to be a way to properly iterate
over them. The assumption that the IOMMU will map the SG entries
1-to-1 is invalid.
Good catch. Indeed, what's missing is one line:
i-num = num;
Of course, an arch-specific version of this could merge between sg_rings,
too, but that's an optimization.
We need to pass the whole sg entries to the IOMMUs at a time.
-
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/5] dma_map_sg_ring() helper
On Friday 21 December 2007 11:00:27 FUJITA Tomonori wrote: We need to pass the whole sg entries to the IOMMUs at a time. Hi Fujita, OK, it's certainly possible to have an arch override. For which architecture is this BTW? 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
Re: [PATCH 2/5] dma_map_sg_ring() helper
From: Rusty Russell [EMAIL PROTECTED] Date: Fri, 21 Dec 2007 11:35:12 +1100 On Friday 21 December 2007 11:00:27 FUJITA Tomonori wrote: We need to pass the whole sg entries to the IOMMUs at a time. Hi Fujita, OK, it's certainly possible to have an arch override. For which architecture is this BTW? SPARC64, POWERPC, maybe IA-64 etc. Basically any platform that potentially does virtual remamping and thus linearization. I think it should always be provided, the new APIs give less information to the implementation and that's a step backwards. - 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/2] scsi: Use new __dma_buffer to align sense buffer in scsi_cmnd
The sense buffer ins scsi_cmnd can nowadays be DMA'ed into directly
by some low level drivers (that typically happens with USB mass
storage).
This is a problem on non cache coherent architectures such as
embedded PowerPCs where the sense buffer can share cache lines with
other structure members, which leads to various forms of corruption.
This uses the newly defined __dma_buffer annotation to enforce that
on such platforms, the sense_buffer is contained within its own
cache line. This has no effect on cache coherent architectures.
Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED]
---
include/scsi/scsi_cmnd.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-merge.orig/include/scsi/scsi_cmnd.h 2007-12-21 13:07:14.0
+1100
+++ linux-merge/include/scsi/scsi_cmnd.h2007-12-21 13:07:29.0
+1100
@@ -88,7 +88,7 @@ struct scsi_cmnd {
working on */
#define SCSI_SENSE_BUFFERSIZE 96
- unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE];
+ unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE] __dma_buffer;
/* obtained by REQUEST SENSE when
* CHECK CONDITION is received on original
* command (auto-sense) */
-
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] DMA buffer alignment annotations
This patch based on some earlier work by Roland Dreier introduces
a pair of annotations that can be used to enforce alignment of
objects that can be DMA'ed into, and to enforce that an DMA'able
object within a structure isn't sharing a cache line with some
other object.
Such sharing of a data structure between DMA and non-DMA objects
isn't a recommended practice, but it does happen and in some case
might even make sense, so we now have a way to make it work
propertly.
The current patch only enables such alignment for some PowerPC
platforms that do not have coherent caches. Other platforms such
as ARM, MIPS, etc... can define ARCH_MIN_DMA_ALIGNMENT if they
want to benefit from this, I don't know them well enough to do
it myself.
The initial issue I'm fixing (in a second patch) by using these
is the SCSI sense buffer which is currently part of the scsi
command structure and can be DMA'ed to. On non-coherent platforms,
this causes various corruptions as this cache line is shared with
various other fields of the scsi_cmnd data structure.
Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED]
---
Documentation/DMA-mapping.txt | 32
include/asm-generic/page.h| 10 ++
include/asm-powerpc/page.h|8
3 files changed, 50 insertions(+)
--- linux-merge.orig/include/asm-generic/page.h 2007-07-27 13:44:45.0
+1000
+++ linux-merge/include/asm-generic/page.h 2007-12-21 13:07:28.0
+1100
@@ -20,6 +20,16 @@ static __inline__ __attribute_const__ in
return order;
}
+#ifndef ARCH_MIN_DMA_ALIGNMENT
+#define __dma_aligned
+#define __dma_buffer
+#else
+#define __dma_aligned __attribute__((aligned(ARCH_MIN_DMA_ALIGNMENT)))
+#define __dma_buffer __dma_buffer_line(__LINE__)
+#define __dma_buffer_line(line)__dma_aligned;\
+ char __dma_pad_##line[0] __dma_aligned
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* __KERNEL__ */
Index: linux-merge/include/asm-powerpc/page.h
===
--- linux-merge.orig/include/asm-powerpc/page.h 2007-09-28 11:42:10.0
+1000
+++ linux-merge/include/asm-powerpc/page.h 2007-12-21 13:15:02.0
+1100
@@ -77,6 +77,14 @@
#define VM_DATA_DEFAULT_FLAGS64(VM_READ | VM_WRITE | \
VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+/*
+ * On non cache coherent platforms, we enforce cache aligned DMA
+ * buffers inside of structures
+ */
+#ifdef CONFIG_NOT_COHERENT_CACHE
+#define ARCH_MIN_DMA_ALIGNMENT L1_CACHE_BYTES
+#endif
+
#ifdef __powerpc64__
#include asm/page_64.h
#else
Index: linux-merge/Documentation/DMA-mapping.txt
===
--- linux-merge.orig/Documentation/DMA-mapping.txt 2007-12-21
13:17:14.0 +1100
+++ linux-merge/Documentation/DMA-mapping.txt 2007-12-21 13:20:00.0
+1100
@@ -75,6 +75,38 @@ What about block I/O and networking buff
networking subsystems make sure that the buffers they use are valid
for you to DMA from/to.
+Note that on non-cache-coherent architectures, having a DMA buffer
+that shares a cache line with other data can lead to memory
+corruption.
+
+The __dma_buffer macro exists to allow safe DMA buffers to be declared
+easily and portably as part of larger structures without causing bloat
+on cache-coherent architectures. To get this macro, architectures have
+to define ARCH_MIN_DMA_ALIGNMENT to the requested alignment value in
+their asm/page.h before including asm-generic/page.h
+
+Of course these structures must be contained in memory that can be
+used for DMA as described above.
+
+To use __dma_buffer, just declare a struct like:
+
+ struct mydevice {
+ int field1;
+ char buffer[BUFFER_SIZE] __dma_buffer;
+ int field2;
+ };
+
+If this is used in code like:
+
+ struct mydevice *dev;
+ dev = kmalloc(sizeof *dev, GFP_KERNEL);
+
+then dev-buffer will be safe for DMA on all architectures. On a
+cache-coherent architecture the members of dev will be aligned exactly
+as they would have been without __dma_buffer; on a non-cache-coherent
+architecture buffer and field2 will be aligned so that buffer does not
+share a cache line with any other data.
+
DMA addressing limitations
Does your device have any DMA addressing limitations? For example, is
-
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/5] dma_map_sg_ring() helper
On Friday 21 December 2007 11:40:00 David Miller wrote:
From: Rusty Russell [EMAIL PROTECTED]
Date: Fri, 21 Dec 2007 11:35:12 +1100
On Friday 21 December 2007 11:00:27 FUJITA Tomonori wrote:
We need to pass the whole sg entries to the IOMMUs at a time.
Hi Fujita,
OK, it's certainly possible to have an arch override. For which
architecture is this BTW?
SPARC64, POWERPC, maybe IA-64 etc.
Basically any platform that potentially does virtual
remamping and thus linearization.
Fujita said need which confused me. I already said it should be handed
down as an optimization; I was curious what I had broken :)
I think it should always be provided, the new APIs give
less information to the implementation and that's a step
backwards.
Absolutely. In fact, I think the sg_ring header would be made safer if it
had the dma_num in it as well: it's more explicit and less surprising to
the caller than mangling sg-num.
How are these two patches then?
===
Introduce sg_ring: a ring of scatterlist arrays.
This patch introduces 'struct sg_ring', a layer on top of scatterlist
arrays. It meshes nicely with routines which expect a simple array of
'struct scatterlist' because it is easy to break down the ring into
its constituent arrays.
The sg_ring header also encodes the maximum number of entries, useful
for routines which populate an sg. We need never hand around a number
of elements any more.
Signed-off-by: Rusty Russell [EMAIL PROTECTED]
---
include/linux/sg_ring.h | 74
1 files changed, 74 insertions(+), 0 deletions(-)
create mode 100644 include/linux/sgring.h
diff --git a/include/linux/sg_ring.h b/include/linux/sg_ring.h
new file mode 100644
--- /dev/null
+++ b/include/linux/sg_ring.h
@@ -0,0 +1,128 @@
+#ifndef _LINUX_SG_RING_H
+#define _LINUX_SG_RING_H
+#include linux/scatterlist.h
+
+/**
+ * struct sg_ring - a ring of scatterlists
+ * @list: the list_head chaining them together
+ * @num: the number of valid sg entries
+ * @dma_num: the number of valid sg entries after dma mapping
+ * @max: the maximum number of sg entries (size of the sg array).
+ * @sg: the array of scatterlist entries.
+ *
+ * This provides a convenient encapsulation of one or more scatter gather
+ * arrays. dma_map_sg_ring() (and friends) set @dma_num: some architectures
+ * coalesce sg entries, to this will be num.
+ */
+struct sg_ring
+{
+ struct list_head list;
+ unsigned int num, dma_num, max;
+ struct scatterlist sg[0];
+};
+
+/* This helper declares an sg ring on the stack or in a struct. */
+#define DECLARE_SG_RING(name, max) \
+ struct {\
+ struct sg_ring ring;\
+ struct scatterlist sg[max]; \
+ } name
+
+/**
+ * sg_ring_init - initialize a scatterlist ring.
+ * @sg: the sg_ring.
+ * @max: the size of the trailing sg array.
+ *
+ * After initialization sg is alone in the ring.
+ */
+static inline void sg_ring_init(struct sg_ring *sg, unsigned int max)
+{
+#ifdef CONFIG_DEBUG_SG
+ unsigned int i;
+ for (i = 0; i max; i++)
+ sg-sg[i].sg_magic = SG_MAGIC;
+ sg-num = 0x;
+ sg-dma_num = 0x;
+#endif
+ INIT_LIST_HEAD(sg-list);
+ sg-max = max;
+ /* FIXME: This is to clear the page bits. */
+ sg_init_table(sg-sg, sg-max);
+}
+
+/**
+ * sg_ring_single - initialize a one-element scatterlist ring.
+ * @sg: the sg_ring.
+ * @buf: the pointer to the buffer.
+ * @buflen: the length of the buffer.
+ *
+ * Does sg_ring_init and also sets up first (and only) sg element.
+ */
+static inline void sg_ring_single(struct sg_ring *sg,
+ const void *buf,
+ unsigned int buflen)
+{
+ sg_ring_init(sg, 1);
+ sg-num = 1;
+ sg_init_one(sg-sg[0], buf, buflen);
+}
+
+/**
+ * sg_ring_next - next array in a scatterlist ring.
+ * @sg: the sg_ring.
+ * @head: the sg_ring head.
+ *
+ * This will return NULL once @sg has looped back around to @head.
+ */
+static inline struct sg_ring *sg_ring_next(const struct sg_ring *sg,
+ const struct sg_ring *head)
+{
+ sg = list_first_entry(sg-list, struct sg_ring, list);
+ if (sg == head)
+ sg = NULL;
+ return (struct sg_ring *)sg;
+}
+
+/* Helper for writing for loops. */
+static inline struct sg_ring *sg_ring_iter(const struct sg_ring *head,
+ const struct sg_ring *sg,
+ unsigned int *i)
+{
+ (*i)++;
+ /* While loop lets us skip any zero-entry sg_ring arrays */
+ while (*i == sg-num) {
+ *i = 0;
+ sg = sg_ring_next(sg, head);
+ if (!sg)
+ break;
+ }
+ return (struct sg_ring *)sg;
+}
+
+/**
+ * sg_ring_for_each - iterate through an
Re: [PATCH 0/5] sg_ring for scsi
On Friday 21 December 2007 13:28:34 FUJITA Tomonori wrote: I'm not sure about chaining the headers (as your sg_ring and scsi_sgtable do) would simplify LLDs. Have you looked at ips or qla1280? Not yet, am working my way through the drivers, but I don't expect it will be a simplification to the normal SCSI LLDs. Most of them are mere consumers of sgs... I'm not a SCSI person: I'm patching SCSI because I have to to get my own sg-using code clean :) Hope that clarifies, 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
Re: [PATCH 0/5] sg_ring for scsi
On Fri, 21 Dec 2007 14:26:47 +1100 Rusty Russell [EMAIL PROTECTED] wrote: On Friday 21 December 2007 13:28:34 FUJITA Tomonori wrote: I'm not sure about chaining the headers (as your sg_ring and scsi_sgtable do) would simplify LLDs. Have you looked at ips or qla1280? Not yet, am working my way through the drivers, but I don't expect it will be a simplification to the normal SCSI LLDs. Most of them are mere consumers of sgs... Some scsi drivers like ips access to sglist in a tricky way. I feel that they don't work with the sg_ring interface well. So if you convert scsi_lib.c to use sg_ring, please see how it works with the tricky drivers before that. I'm not a SCSI person: I'm patching SCSI because I have to to get my own sg-using code clean :) I'm SCSI-biased. If you don't convert scsi to use sg_ring, I don't complain. :) Though it would be better to have only one mechanism to handle large sglist in kernel. - 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
