Date: Thu, 21 Sep 2006 19:17:33 -0700 (PDT) From: Doug White <dwhite@gumbysoft.com> To: Jan Mikkelsen <janm@transactionware.com> Cc: freebsd-stable@freebsd.org Subject: Re: Patch: sym(4) "VTOBUS FAILED" panics on amd64, amd64/89550 Message-ID: <20060921190148.S87814@carver.gumbysoft.com> In-Reply-To: <002d01c6ddc7$76d10cc0$0202a8c0@transactzbkv04> References: <002d01c6ddc7$76d10cc0$0202a8c0@transactzbkv04>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 22 Sep 2006, Jan Mikkelsen wrote:
> Quick summary: sym(4) assumes on amd64 that virtual addresses provided by
> bus_dmamem_alloc() have the same alignment as the physical addresses (in
> this case, 2*PAGE_SIZE). They don't, and stuff breaks. This patch works
> around that.
Why is this? busdma supports alignment constraints; why not just set the
alignment to what you need it set at? I realize sym has its own hand
rolled DMA management craziness but alignment is something busdma can take
care of easily.
Also the changes to MEMO_CLUSTER_SIZE seems to be already compensated for
by the code blocks that calculate it:
#define MEMO_SHIFT 4 /* 16 bytes minimum memory chunk */
#ifndef __amd64__
#define MEMO_PAGE_ORDER 0 /* 1 PAGE maximum */
#else
#define MEMO_PAGE_ORDER 1 /* 2 PAGEs maximum on amd64 */
#endif
#if 0
#define MEMO_FREE_UNUSED /* Free unused pages immediately */
#endif
#define MEMO_WARN 1
#define MEMO_CLUSTER_SHIFT (PAGE_SHIFT+MEMO_PAGE_ORDER)
#define MEMO_CLUSTER_SIZE (1UL << MEMO_CLUSTER_SHIFT)
#define MEMO_CLUSTER_MASK (MEMO_CLUSTER_SIZE-1)
This results in 2*PAGE_SIZE clusters on amd64 and PAGE_SIZE clusters on
other platforms. Since you seem to like doing MEMO_CLUSTER_SIZE * 2, why
not just increase MEMO_PAGE_ORDER to 2 (and get 4*PAGE_SIZE clusters) ?
Oh dear, I didn't notice that the call to bus_dma_tag_create() has bad
arguments. From the (RELENG_6) source:
if (!bus_dma_tag_create(dev_dmat, 1, MEMO_CLUSTER_SIZE,
BUS_SPACE_MAXADDR_32BIT,
BUS_SPACE_MAXADDR_32BIT,
NULL, NULL, MEMO_CLUSTER_SIZE, 1,
MEMO_CLUSTER_SIZE, 0,
busdma_lock_mutex, &Giant, &mp->dmat)) {
As you fixed, that second BUS_SPACE_MAXADDR_32BIT should be
BUS_SPACE_MAXADDR since its an exclusion zone. I'm suprised that doesn't
fix it right there.
If increasing MEMO_PAGE_ORDER fixes the issue, then the #ifs are
extraneous.
Also we generally prefer diffs in unidiff (-u) format, its a little easier
to figure out exactly what changed just by looking at the diff. Thanks!
--
Doug White | FreeBSD: The Power to Serve
dwhite@gumbysoft.com | www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060921190148.S87814>