From owner-freebsd-hackers@FreeBSD.ORG Tue Sep 28 20:03:45 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2B91C106566B; Tue, 28 Sep 2010 20:03:45 +0000 (UTC) (envelope-from neelnatu@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 4393A8FC13; Tue, 28 Sep 2010 20:03:26 +0000 (UTC) Received: by wya21 with SMTP id 21so10455wya.13 for ; Tue, 28 Sep 2010 13:03:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=jQV7DSJ0011K8um/7nOzkkQsKX3hOuNCMosto38uu9M=; b=MfwsnEC+PP1cHE4R1Dvjb3JtOFf7XcNnPqdcbcjQitY0LQoOFdXMXMl9laSArIVKj5 Vh95ttDvC1Qmpi/Yk+Q5UAmwmbRe384pmh7HNX0b4AYrpF38umkhIAqO/s5sdTmar5wd dTDR+0ja1bd7tJDAo/qMLTL8DJadAgbopvhew= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=TjJsBPE30eeacL+ZLaueLiPI+1UBxlAQtWkLXlyYv/LZ+6uD9IgEFgpBgyDUdxeaJp q3tTtji3QYJhz159gZR3TbW1TC/12PC9UAi4eZbsniujuFrjnQEIC/TP/DPxK+oXG1BR 2I86tvg7X2XGExJ2SWGeTO0gW0gyTOLW9/qdo= MIME-Version: 1.0 Received: by 10.216.235.106 with SMTP id t84mr448515weq.46.1285704129092; Tue, 28 Sep 2010 13:02:09 -0700 (PDT) Received: by 10.216.133.5 with HTTP; Tue, 28 Sep 2010 13:02:08 -0700 (PDT) In-Reply-To: <201009280936.40203.jhb@freebsd.org> References: <201009271104.17478.jhb@freebsd.org> <201009280936.40203.jhb@freebsd.org> Date: Tue, 28 Sep 2010 13:02:08 -0700 Message-ID: From: Neel Natu To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-hackers@freebsd.org Subject: Re: PATCH: fix bogus error message "bus_dmamem_alloc failed to align memory properly" X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Sep 2010 20:03:45 -0000 Hi John, On Tue, Sep 28, 2010 at 6:36 AM, John Baldwin wrote: > On Monday, September 27, 2010 5:13:03 pm Neel Natu wrote: >> Hi John, >> >> Thanks for reviewing this. >> >> On Mon, Sep 27, 2010 at 8:04 AM, John Baldwin wrote: >> > On Friday, September 24, 2010 9:00:44 pm Neel Natu wrote: >> >> Hi, >> >> >> >> This patch fixes the bogus error message from bus_dmamem_alloc() abou= t >> >> the buffer not being aligned properly. >> >> >> >> The problem is that the check is against a virtual address as opposed >> >> to the physical address. contigmalloc() makes guarantees about >> >> the alignment of physical addresses but not the virtual address >> >> mapping it. >> >> >> >> Any objections if I commit this patch? >> > >> > Hmmm, I guess you are doing super-page alignment rather than sub-page >> > alignment? =A0In general I thought the busdma code only handled sub-pa= ge >> > alignment and doesn't fully handle requests for super-page alignment. >> > >> >> Yes, this is for allocations with sizes greater than PAGE_SIZE and >> alignment requirements also greater than a PAGE_SIZE. >> >> > For example, since it insists on walking individual pages at a time, i= f you >> > had an alignment setting of 4 pages and passed in a single, aligned 4-= page >> > buffer, bus_dma would actually bounce the last 3 pages so that each in= dividual >> > page is 4-page aligned. =A0At least, I think that is what would happen= . >> > >> >> I think you are referring to bus_dmamap_load() operation that would >> follow the bus_dmamem_alloc(), right? The memory allocated by >> bus_dmamem_alloc() does not need to be bounced. In fact, the dmamap >> pointer returned by bus_dmamem_alloc() is NULL. >> >> At least for the amd64 implementation there is code in >> _bus_dmamap_load_buffer() which will coalesce individual dma segments >> if they satisfy 'boundary' and 'segsize' constraints. > > So the problem is earlier in the routine where it does this: > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Get the physical address for this segme= nt. > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pmap) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0curaddr =3D pmap_extract(p= map, vaddr); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0curaddr =3D pmap_kextract(= vaddr); > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Compute the segment size, and adjust co= unts. > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0max_sgsize =3D MIN(buflen, dmat->maxsegsz)= ; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sgsize =3D PAGE_SIZE - ((vm_offset_t)curad= dr & PAGE_MASK); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (map->pagesneeded !=3D 0 && run_filter(= dmat, curaddr)) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sgsize =3D roundup2(sgsize= , dmat->alignment); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sgsize =3D MIN(sgsize, max= _sgsize); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0curaddr =3D add_bounce_pag= e(dmat, map, vaddr, sgsize); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sgsize =3D MIN(sgsize, max= _sgsize); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} > > If you have a map that does need bouncing, then it will split up the page= s. > It happens to work for bus_dmamem_alloc() because that returns a NULL map > which doesn't bounce. =A0But if you had a PCI device which supported only > 32-bit addresses on a 64-bit machine with an aligned, 4 page buffer above > 4GB and did a bus_dma_map_load() on that buffer, it would get split up in= to > 4 separate 4 page-aligned pages. > You are right. I assume that you are ok with the patch and the discussion above was an FYI, right? best Neel > -- > John Baldwin >