From owner-freebsd-current@FreeBSD.ORG Mon Feb 14 19:26:57 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 32AC3106566C; Mon, 14 Feb 2011 19:26:56 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: Matthew Fleming Date: Mon, 14 Feb 2011 14:26:36 -0500 User-Agent: KMail/1.6.2 References: <201102141337.59203.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Message-Id: <201102141426.42450.jkim@FreeBSD.org> Cc: freebsd-current@freebsd.org Subject: Re: acpi_resource bug? X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Feb 2011 19:26:57 -0000 On Monday 14 February 2011 01:45 pm, Matthew Fleming wrote: > On Mon, Feb 14, 2011 at 10:37 AM, John Baldwin wrote: > > On Monday, February 14, 2011 1:30:18 pm Jung-uk Kim wrote: > >> On Monday 14 February 2011 10:29 am, Matthew Fleming wrote: > >> > On Mon, Feb 14, 2011 at 6:24 AM, John Baldwin > >> > > >> > >> wrote: > >> > > On Sunday, February 13, 2011 2:46:07 pm Matthew Fleming wrote: > >> > >> I'm not very familiar with the acpi code, but we have seen > >> > >> an intermittent issue on boot: > >> > >> > >> > >> 1) should the length of the bcopy() be changed to either > >> > >> respect res->Length or the actual length of the > >> > >> ACPI_RESOURCE_DATA for the type? > >> > > > >> > > It should just use res->Length: > >> > > >> > Is there a guarantee that res->Length is <= > >> > sizeof(ACPI_RESOURCE) ? > >> > >> No. �Please try the attached patch (after your r218685). > > > > I think your patch is correct, but are you saying that ACPICA > > will return a resource with a size that doesn't match its type? > > > > ACPI_RESOURCE_DATA is a union of all the various resource types, > > and it does contain both ACPI_RESOURCE_IRQ and > > ACPI_RESOURCE_EXTENDED_IRQ, so it's hard to see how res->Length > > would be greater than the size of ACPI_RESOURCE. > > Jung-uk Kim's patch makes acpi_resource match the other bcopy's in > the acpica directory, and in the case of what I saw it would bcopy > 8+5 bytes instead of res->Length == 16. Actually, it is 8+6 bytes, i.e., sizeof(ACPI_RESOURCE_IRQ) == 6. ;-) Also, res->Length does not include header but it includes padding bytes for alignment. > My concern was that res->Length seemed primarily to be an offset > from the current resource to the next one, and I didn't see why > that may not include a lot of padding (including more than the > target of the bcopy was prepared for). However, my code will also > copy bytes we don't care about any time res->Length is rounded up > from the actual struct size. Correct. > The patch looks fine to me. I don't have direct access to the > machine that was intermittently crashing so I can't really try the > new patch, but my change resolved the issue on it. I'll go ahead and commit my patch if you don't have any more concerns. Thanks for working on this, Jung-uk Kim