From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Lalancette Subject: Re: [PATCH]: Better checking in range_straddles_page_boundary Date: Wed, 22 Oct 2008 12:57:37 +0200 Message-ID: <48FF0721.9090207@redhat.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org Keir Fraser wrote: > On 22/10/08 10:05, "Chris Lalancette" wrote: > >> Attached is a simple patch to slightly rework the logic in >> range_straddles_page_boundary(). The reason for this is to avoid a crash we >> are >> seeing on 32-bit dom0. Basically, the contiguous_bitmap is allocated based on >> max_low_pfn. However, the swiotlb code can be passed a request (in >> swiotlb_map_sg) that is > 1 page (I've seen 2 pages), and is also a page >> (sg->page) > max_low_pfn. If this combination happens, then you get a fatal >> page fault when doing the test_bit(pfn, contiguous_bitmap). For that reason, >> rework the logic in range_straddles_page_boundary so that if it gets a request >> 1 page, and it's above max_low_pfn, then we force the splitting of the >> request. >> In our testing, this seems to fix the issue. > > How about we just get rid of the contiguous_bitmap? We might as well if you > are going to push that check after calling > check_pages_physically_contiguous(), since no extent which is acceptable to > contiguous_bitmap should fail on check_pages_physically_contiguous(). I > think I kept contiguous_bitmap only as a fast check before calling that > slower function. Oh, hm, good point. If we think that the fast check is still good to have, the other option is to leave range_straddles_page_boundary as-is, and just allocate contiguous_bitmap with max_pfn (or end_pfn; I can never remember which is which i386 vs. x86_64). Either way works for me, though; I don't think the check_pages_physically_contiguous is a huge performance bottleneck, especially since *most* requests are for 1 page; it's just the odd request that comes in with 2 pages (or more, but I've never seen more than 2 pages). -- Chris Lalancette