From: Brian King <brking@us.ibm.com>
To: "David S. Miller" <davem@davemloft.net>
Cc: hugh@veritas.com, James.Bottomley@SteelEye.com, akpm@osdl.org,
linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: [PATCH] ipr: don't doublefree pages from scatterlist
Date: Mon, 06 Feb 2006 08:46:19 -0600 [thread overview]
Message-ID: <43E7613B.5060706@us.ibm.com> (raw)
In-Reply-To: <20060206.014608.22328385.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]
David S. Miller wrote:
> From: Hugh Dickins <hugh@veritas.com>
> Date: Mon, 6 Feb 2006 09:32:54 +0000 (GMT)
>
>> From looking at the source, the architectures I found to be doing
>> scatterlist coalescing in some cases were alpha, ia64, parisc (some
>> code under drivers), powerpc, sparc64 and x86_64.
>>
>> I agree with you that it would be possible for them to do the coalescing
>> by just adjusting dma_address and dma_length (though it's architecture-
>> dependent whether there are such fields at all), not interfering with
>> the input page and length; and maybe some of them do proceed that way.
>> I didn't find the coalescing code in any of them very easy to follow.
>>
>> So please examine arch/x86_64/kernel/pci_gart.c gart_map_sg (and
>> dma_map_cont which it calls): x86_64 was the architecture on which
>> the problem was really found with drivers/scsi/st.c, and avoided
>> by that boot option iommu=nomerge.
>>
>> Lines like "*sout = *s;" and "*sout = sg[start];" are structure-
>> copying whole scallerlist entries from one position in the list
>> to another, without explicit mention of the page and length fields.
>
> That's a bug, frankly. Sparc64 doesn't need to do anything like
> that. Spamming the page pointers is really really bogus and I'm
> surprised this doesn't make more stuff explode.
>
> It was never the intention to allow the DMA mapping support code
> to modify the page, offset, and length members of the scatterlist.
> Only the DMA components.
>
> I'd really prefer that those assignments get fixed and an explicit
> note added to Documentation/DMA-mapping.txt about this.
How about this for a documentation fix?
Brian
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dma_mapping_clarification.patch --]
[-- Type: text/x-patch; name="dma_mapping_clarification.patch", Size: 1452 bytes --]
The current pci_map_sg API is a bit unclear what it is allowed
to modify in the passed scatterlist when coalescing entries.
Clarify the pci_map_sg API to prevent it from modifying
the page, offset, and length fields in the scatterlist.
Signed-off-by: Brian King <brking@us.ibm.com>
---
linux-2.6-bjking1/Documentation/DMA-mapping.txt | 5 ++++-
1 files changed, 4 insertions(+), 1 deletion(-)
diff -puN Documentation/DMA-mapping.txt~dma_mapping_clarification Documentation/DMA-mapping.txt
--- linux-2.6/Documentation/DMA-mapping.txt~dma_mapping_clarification 2006-02-06 08:23:10.000000000 -0600
+++ linux-2.6-bjking1/Documentation/DMA-mapping.txt 2006-02-06 08:38:43.000000000 -0600
@@ -513,7 +513,10 @@ consecutive sglist entries can be merged
ends and the second one starts on a page boundary - in fact this is a huge
advantage for cards which either cannot do scatter-gather or have very
limited number of scatter-gather entries) and returns the actual number
-of sg entries it mapped them to. On failure 0 is returned.
+of sg entries it mapped them to. The implementation is free to do this
+by modifying the scatterlist fields specified for DMA. The scatterlist
+fields used as an input to this function (i.e. page, offset, and length)
+will NOT be modified. On failure 0 is returned.
Then you should loop count times (note: this can be less than nents times)
and use sg_dma_address() and sg_dma_len() macros where you previously
_
next prev parent reply other threads:[~2006-02-06 14:46 UTC|newest]
Thread overview: 99+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20051129092432.0f5742f0.akpm@osdl.org>
2005-11-29 18:34 ` Fw: crash on x86_64 - mm related? Ryan Richter
[not found] ` <Pine.LNX.4.63.0511292147120.5739@kai.makisara.local>
2005-11-29 20:31 ` Ryan Richter
2005-11-29 20:48 ` Kai Makisara
2005-11-29 20:58 ` Ryan Richter
2005-11-29 21:36 ` Kai Makisara
2005-11-30 5:12 ` Kai Makisara
2005-12-01 19:18 ` Kai Makisara
2005-12-01 19:38 ` Linus Torvalds
2005-12-01 19:56 ` Ryan Richter
2005-12-01 20:21 ` Hugh Dickins
2005-12-01 21:44 ` Kai Makisara
2005-12-02 18:03 ` Ryan Richter
2005-12-02 18:43 ` Jesper Juhl
2005-12-02 19:12 ` Hugh Dickins
2005-12-02 19:44 ` Ryan Richter
2005-12-02 20:40 ` Hugh Dickins
2005-12-03 17:29 ` Ryan Richter
2005-12-06 16:08 ` Ryan Richter
2005-12-06 20:31 ` Hugh Dickins
2005-12-06 20:43 ` Ryan Richter
2005-12-07 18:37 ` Hugh Dickins
2005-12-08 2:26 ` Ryan Richter
2005-12-12 16:54 ` Ryan Richter
2005-12-12 17:40 ` Linus Torvalds
2005-12-12 17:45 ` James Bottomley
2005-12-12 18:04 ` Ryan Richter
2005-12-12 18:09 ` Linus Torvalds
2005-12-12 18:24 ` James Bottomley
2005-12-15 19:09 ` Ryan Richter
2005-12-16 4:01 ` James Bottomley
2005-12-17 3:31 ` Ryan Richter
2005-12-26 23:42 ` Ryan Richter
2005-12-27 16:21 ` Kai Makisara
2006-01-03 19:03 ` Ryan Richter
2006-01-04 17:27 ` Ryan Richter
2006-01-04 21:48 ` Kai Makisara
2006-01-05 5:40 ` Ryan Richter
2006-01-05 20:12 ` Ryan Richter
2006-01-05 21:18 ` Linus Torvalds
2006-01-08 22:36 ` Ryan Richter
2006-01-09 3:31 ` Ryan Richter
2006-01-09 4:07 ` Linus Torvalds
2006-01-09 5:13 ` Andrew Morton
2006-01-09 5:45 ` Ryan Richter
2006-01-09 5:57 ` Andrew Morton
2006-01-09 9:44 ` Hugh Dickins
2006-01-09 18:53 ` Ryan Richter
2006-01-09 19:31 ` Hugh Dickins
2006-01-09 20:05 ` Ryan Richter
2006-01-18 0:12 ` Ryan Richter
2006-01-18 16:00 ` Hugh Dickins
2006-02-03 19:46 ` Hugh Dickins
2006-02-03 19:51 ` [PATCH] ib: don't doublefree pages from scatterlist Hugh Dickins
2006-02-03 23:13 ` Roland Dreier
2006-02-03 19:53 ` [PATCH] st: " Hugh Dickins
2006-02-03 20:38 ` Mike Christie
2006-02-03 21:16 ` Hugh Dickins
2006-02-04 12:10 ` Kai Makisara
2006-02-04 15:01 ` Hugh Dickins
2006-02-03 19:55 ` [PATCH] ipr: " Hugh Dickins
2006-02-03 22:06 ` Brian King
2006-02-04 0:26 ` Hugh Dickins
2006-02-05 21:35 ` Brian King
2006-02-06 9:32 ` Hugh Dickins
2006-02-06 9:46 ` David S. Miller
2006-02-06 14:46 ` Brian King [this message]
2006-02-06 16:45 ` Hugh Dickins
2006-02-06 17:38 ` James Bottomley
2006-02-06 19:15 ` Brian King
2006-02-06 21:11 ` Andi Kleen
2006-02-06 21:49 ` David S. Miller
2006-02-06 22:11 ` Hugh Dickins
2006-02-06 22:13 ` Andi Kleen
2006-02-07 3:09 ` Ryan Richter
2006-02-11 22:38 ` Ryan Richter
2006-02-12 18:57 ` Hugh Dickins
2006-02-12 21:29 ` Andi Kleen
2006-02-13 17:21 ` Hugh Dickins
2006-02-06 15:02 ` James Bottomley
2006-02-06 17:01 ` Hugh Dickins
2006-02-03 19:56 ` [PATCH] osst: " Hugh Dickins
2006-02-03 21:10 ` Fw: crash on x86_64 - mm related? Ryan Richter
2006-02-04 11:58 ` Kai Makisara
2006-02-04 14:46 ` Hugh Dickins
2006-01-05 22:09 ` Kai Makisara
2006-01-04 18:26 ` Ryan Richter
2005-12-07 18:30 ` Ryan Richter
2005-12-07 18:56 ` Hugh Dickins
2005-12-07 19:06 ` Ryan Richter
2005-12-06 17:57 ` Ryan Richter
2005-12-01 20:28 ` James Bottomley
2005-12-01 21:17 ` Kai Makisara
2005-12-02 13:45 ` Hugh Dickins
2005-12-02 17:59 ` Kai Makisara
2005-12-02 18:55 ` Hugh Dickins
2005-12-02 19:46 ` Kai Makisara
2005-12-02 20:47 ` Hugh Dickins
2005-12-04 9:29 ` Kai Makisara
2005-12-01 19:53 ` Ryan Richter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=43E7613B.5060706@us.ibm.com \
--to=brking@us.ibm.com \
--cc=James.Bottomley@SteelEye.com \
--cc=akpm@osdl.org \
--cc=davem@davemloft.net \
--cc=hugh@veritas.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).