Xen devel - Oct 2008 - [PATCH]: Better checking in range_straddles_page_boundary

All,
    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.

Note that the patch is against the RHEL-5 2.6.18 code, but the patch should
apply with a little fuzz to the linux-2.6.18-xen.hg tree.

Signed-off-by: Chris Lalancette <clalance@redhat.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 22/10/08 10:05, "Chris Lalancette" <clalance@redhat.com>
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.

 -- Keir




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser wrote:
> On 22/10/08 10:05, "Chris Lalancette" <clalance@redhat.com>
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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel