From: Benny Halevy <bhalevy@panasas.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
Boaz Harrosh <bharrosh@panasas.com>,
Jens Axboe <jens.axboe@oracle.com>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Linux Kernel Development <linux-kernel@vger.kernel.org>,
mingo@elte.hu
Subject: Re: [PATCH 09/10] Change table chaining layout
Date: Thu, 25 Oct 2007 18:03:25 +0200 [thread overview]
Message-ID: <4720BE4D.1010100@panasas.com> (raw)
In-Reply-To: <alpine.LFD.0.999.0710250834390.30120@woody.linux-foundation.org>
On Oct. 25, 2007, 17:40 +0200, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> On Thu, 25 Oct 2007, Rusty Russell wrote:
>> On Wednesday 24 October 2007 01:22:55 Linus Torvalds wrote:
>>> Well, I'd personally actually prefer to *not* have the count be passed
>>> down explicitly, because it's just too error prone.
>> Well, the duplication is bad, but walking lists to find the length is
>> inefficient so you pass around the length as well.
>
> Nobody should *ever* walk the list to find the length. Does anybody really
> do that? Yes, we pass the thing down, but do people *need* it?
>
> [ Side note: some of the users of that length currently would seem to be
> buggy in the presense of continuation entries, and seem to assume that
> the "list" is just a contiguous array. In fatc, that's almost the only
> valid use for the "count" thing, since any other use _has_ to walk it
> entry by entry anyway, no? ]
>
> The thing is, nobody should care. You walk the list to fill things in, or
> to write it out to some HW-specific DMA table, you should never care about
> the length. However, you *do* care about the "where does it end" part: to
> be able to detect overflows (which should never happen, but from a
> debugging standpoint it needs to be detectable rather than just silently
> use or corrupt memory).
>
> But if people really want/need the length, then we damn well should have a
> "header" thing, not two independent "list + length" parameters.
There are a number of indicators that need to be kept in sync, depending
on the usage. The number of entries is set when it is allocated and is
currently needed to free it up (note that in the sgtable "sketch" James
proposed we saved the sg pool index in the sgtable header and used it to
free each chunk to the right pool). The number of entries is then used in
many places to scan the list, however, after the sg list is dma mapped, the
dma mapping may be shorter than the original sg list when multiple pages are
coalesced and there we need to defer to use the dma_length (plus the number
of entries) to determine the end of the list.
IMO I think that the byte count can be used authoritatively to scan
the contents of the sg list either before or after dma mapping
while the number of entries is relevant to walking the list in a context free
manner (i.e. to go over all the entries that were allocated)
>
> Linus
> -
next prev parent reply other threads:[~2007-10-25 16:03 UTC|newest]
Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-22 18:10 [PATCH 00/10] SG updates Jens Axboe
2007-10-22 18:10 ` [PATCH 01/10] [SG] Add helpers for manipulating SG entries Jens Axboe
2007-10-22 18:10 ` [PATCH 02/10] [SG] Update block layer to use sg helpers Jens Axboe
2007-10-23 5:13 ` Heiko Carstens
2007-10-23 5:16 ` Jens Axboe
2007-10-23 5:42 ` [PATCH] fix ll_rw_blk.c build on s390 Heiko Carstens
2007-10-23 5:44 ` [PATCH] net: fix xfrm build - missing scatterlist.h include Heiko Carstens
2007-10-23 7:28 ` Jens Axboe
2007-10-23 14:32 ` [PATCH 02/10] [SG] Update block layer to use sg helpers John Stoffel
2007-10-22 18:10 ` [PATCH 03/10] [SG] Update crypto/ to " Jens Axboe
2007-10-22 18:10 ` [PATCH 04/10] [SG] Update drivers to use " Jens Axboe
2007-10-23 6:28 ` Heiko Carstens
2007-10-23 7:14 ` Jens Axboe
2007-10-23 7:16 ` Heiko Carstens
2007-10-22 18:10 ` [PATCH 05/10] [SG] Update fs/ " Jens Axboe
2007-10-22 18:11 ` [PATCH 06/10] [SG] Update net/ " Jens Axboe
2007-10-23 10:44 ` Christian Borntraeger
2007-10-23 10:45 ` Jens Axboe
2007-10-22 18:11 ` [PATCH 07/10] [SG] Update swiotlb " Jens Axboe
2007-10-22 18:11 ` [PATCH 08/10] [SG] Update arch/ " Jens Axboe
2007-10-22 21:10 ` Benny Halevy
2007-10-23 7:26 ` Jens Axboe
2007-10-22 18:11 ` [PATCH 09/10] Change table chaining layout Jens Axboe
2007-10-22 19:39 ` Geert Uytterhoeven
2007-10-22 19:49 ` Linus Torvalds
2007-10-22 19:52 ` Jens Axboe
2007-10-22 20:16 ` Alan Cox
2007-10-22 20:38 ` Matt Mackall
2007-10-22 20:44 ` Linus Torvalds
2007-10-22 21:43 ` Alan Cox
2007-10-22 21:47 ` Linus Torvalds
2007-10-23 0:07 ` David Miller
2007-10-23 7:18 ` Geert Uytterhoeven
2007-10-23 9:29 ` Boaz Harrosh
2007-10-23 9:41 ` Jens Axboe
2007-10-23 9:50 ` Boaz Harrosh
2007-10-23 9:55 ` Jens Axboe
2007-10-23 10:23 ` Boaz Harrosh
2007-10-23 10:29 ` Jens Axboe
2007-10-23 15:22 ` Linus Torvalds
2007-10-24 8:05 ` Jens Axboe
2007-10-24 9:03 ` Geert Uytterhoeven
2007-10-24 9:12 ` Jens Axboe
2007-10-24 13:35 ` Olivier Galibert
2007-10-24 13:38 ` Jens Axboe
2007-10-24 13:45 ` Olivier Galibert
2007-10-24 15:16 ` Linus Torvalds
2007-10-25 8:40 ` Rusty Russell
2007-10-25 9:11 ` Jens Axboe
2007-10-25 11:54 ` Rusty Russell
2007-10-26 0:03 ` Rusty Russell
2007-10-25 15:40 ` Linus Torvalds
2007-10-25 16:03 ` Benny Halevy [this message]
2007-10-26 5:01 ` Paul Mackerras
2007-10-26 14:52 ` Linus Torvalds
2007-10-26 17:28 ` Jens Axboe
2007-11-05 6:11 ` [RFC PATCH 1/2] sg_ring instead of scatterlist chaining Rusty Russell
2007-11-05 6:15 ` [RFC PATCH 2/2] sg_ring instead of scatterlist chaining in virtio Rusty Russell
2007-11-05 16:40 ` [RFC PATCH 1/2] sg_ring instead of scatterlist chaining Randy Dunlap
2007-10-23 10:33 ` [PATCH 09/10] Change table chaining layout Ingo Molnar
2007-10-23 10:56 ` Jens Axboe
2007-10-23 11:27 ` Ingo Molnar
2007-10-23 19:23 ` Geert Uytterhoeven
2007-10-23 21:46 ` Jens Axboe
2007-10-24 6:56 ` Jens Axboe
2007-10-22 21:16 ` Benny Halevy
2007-10-22 21:21 ` Jeff Garzik
2007-10-22 21:47 ` Matt Mackall
2007-10-22 22:52 ` Alan Cox
2007-10-22 23:46 ` Matt Mackall
2007-10-23 0:11 ` Jeff Garzik
2007-10-23 4:09 ` powerpc: Fix fallout from sg_page() changes Olof Johansson
2007-10-23 4:31 ` IB/ehca: Fix sg_page() fallout Olof Johansson
2007-10-23 5:05 ` Jens Axboe
2007-10-23 5:54 ` Olof Johansson
2007-10-23 7:12 ` Jens Axboe
2007-10-23 7:13 ` powerpc: Fix fallout from sg_page() changes Jens Axboe
2007-10-23 17:08 ` [PATCH 09/10] Change table chaining layout Boaz Harrosh
2007-10-23 18:33 ` Jens Axboe
2007-10-23 19:56 ` Andi Kleen
2007-10-23 20:20 ` Jens Axboe
2007-10-23 20:57 ` Andi Kleen
2007-10-23 21:44 ` Jens Axboe
2007-10-22 18:11 ` [PATCH 10/10] Add CONFIG_DEBUG_SG sg validation Jens Axboe
2007-10-23 14:48 ` [PATCH][SG] fix typo in ps3rom.c Arnd Bergmann
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=4720BE4D.1010100@panasas.com \
--to=bhalevy@panasas.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=bharrosh@panasas.com \
--cc=geert@linux-m68k.org \
--cc=jens.axboe@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=rusty@rustcorp.com.au \
--cc=torvalds@linux-foundation.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