lkml.org 
[lkml]   [2018]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH V2] xen-swiotlb: use actually allocated size on check physical continuous
From
Date
On 10/18/18 4:52 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 16, 2018 at 03:21:16PM -0700, Joe Jin wrote:
>> xen_swiotlb_{alloc,free}_coherent() allocate/free memory by order,
>> but passed required size to range_straddles_page_boundary(),
>> when first pages are physical continuous,
>> range_straddles_page_boundary() returned true, then did not
>> exchanged memory with Xen, later on free memory, it tried to
>> exchanged non-contiguous memory with Xen, then kernel panic.
>
> I have a hard time understanding the commit message.
>
> I think you mean to say:
>
> xen_swiotlb_{alloc,free}_coherent() allocate/free memory based on the
> order of the pages and not size argument (bytes). This is inconsistent with
> range_straddles_page_boundary and memset which use the 'size' value,
> which may lead to not exchanging memory with Xen (range_straddles_page_boundary()
> returned true). And then the call to xen_swiotlb_free_coherent() would
> actually try to exchange the memory with Xen, leading to the kernel
> hitting an BUG (as the hypercall returned an error).

Sorry for the confusing, yes you right.

BTW: panic triggered by xen_remap_exchanged_ptes()->xen_mc_issue().

Panic calltrace as below:

[ 1129.150648] WARNING: CPU: 1 PID: 21001 at arch/x86/xen/multicalls.c:129
xen_mc_flush+0x1ce/0x1e0()
[ 1129.150653] Modules linked in: ocfs2 mptctl mptbase xen_netback
xen_blkback xen_gntalloc xen_gntdev xen_evtchn xenfs xen_privcmd ocfs2_dlmfs
ocfs2_stack_o2cb ocfs2_dlm ocfs2_nodemanager ocfs2_stackglue configfs bnx2fc
fcoe libfcoe libfc sunrpc 8021q mrp garp bridge stp llc bonding
dm_round_robin dm_multipath iTCO_wdt iTCO_vendor_support pcspkr shpchp
sb_edac edac_core i2c_i801 i2c_core cdc_ether usbnet mii lpc_ich mfd_core
ioatdma sg ipmi_devintf ipmi_si ipmi_msghandler ext4 jbd2 mbcache2 sd_mod
usb_storage ahci libahci megaraid_sas ixgbe dca ptp pps_core vxlan udp_tunnel
ip6_udp_tunnel qla2xxx scsi_transport_fc crc32c_intel be2iscsi bnx2i cnic uio
cxgb4i cxgb4 cxgb3i libcxgbi ipv6 cxgb3 mdio libiscsi_tcp qla4xxx
iscsi_boot_sysfs libiscsi scsi_transport_iscsi wmi dm_mirror dm_region_hash
dm_log dm_mod
[ 1129.150719]
[ 1129.150722] CPU: 1 PID: 21001 Comm: blkid Tainted: G W
4.1.12-124.16.3.el6uek.x86_64 #2
[ 1129.150723] Hardware name: Oracle Corporation ORACLE SERVER
X5-2/ASM,MOTHERBOARD,1U, BIOS 30130400 02/08/2018
[ 1129.150727] 0000000000000000 ffff8803ec7c3c78 ffffffff816e52a3
0000000000000000
[ 1129.150732] ffffffff819bc5c3 ffff8803ec7c3cb8 ffffffff8108481a
ffff8803ec7c3d18
[ 1129.150735] ffff8804a6a4d2c0 0000000000000003 ffff8804994df800
0000000000000001
[ 1129.150740] Call Trace:
[ 1129.150746] [<ffffffff816e52a3>] dump_stack+0x63/0x81
[ 1129.150753] [<ffffffff8108481a>] warn_slowpath_common+0x8a/0xc0
[ 1129.150756] [<ffffffff8108490a>] warn_slowpath_null+0x1a/0x20
[ 1129.150759] [<ffffffff8100559e>] xen_mc_flush+0x1ce/0x1e0
[ 1129.150764] [<ffffffff81007af9>] __xen_pgd_unpin+0x109/0x250
[ 1129.150767] [<ffffffff81007db7>] xen_exit_mmap+0x177/0x2a0
[ 1129.150770] [<ffffffff811c3628>] exit_mmap+0x48/0x160
[ 1129.150776] [<ffffffff8112c68c>] ? __audit_free+0x1cc/0x220
[ 1129.150784] [<ffffffff811e725e>] ? kfree+0x16e/0x180
[ 1129.150787] [<ffffffff81081cf3>] mmput+0x63/0x100
[ 1129.150790] [<ffffffff81087263>] do_exit+0x343/0xa90
[ 1129.150793] [<ffffffff8112c794>] ? __audit_syscall_entry+0xb4/0x110
[ 1129.150796] [<ffffffff81025a2c>] ? do_audit_syscall_entry+0x6c/0x70
[ 1129.150799] [<ffffffff81087a45>] do_group_exit+0x45/0xb0
[ 1129.150805] [<ffffffff81087ac4>] SyS_exit_group+0x14/0x20
[ 1129.150807] [<ffffffff816edc5c>] system_call_fastpath+0x18/0xd6
[ 1129.150810] ---[ end trace 7aec0081e2a07193 ]---

Do you think the patch fine or no? if yes I'll reorg patch description
and resend it.

Thanks,
Joe

>
> This patch fixes it by making the 'size' variable be of the same size
> as the amount of memory allocated.
>
> I checked it as such..
>>
>> Signed-off-by: Joe Jin <joe.jin@oracle.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: Christoph Helwig <hch@lst.de>
>> Cc: Dongli Zhang <dongli.zhang@oracle.com>
>> Cc: John Sobecki <john.sobecki@oracle.com>
>>
>> ---
>> drivers/xen/swiotlb-xen.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
>> index a6f9ba85dc4b..aa081f806728 100644
>> --- a/drivers/xen/swiotlb-xen.c
>> +++ b/drivers/xen/swiotlb-xen.c
>> @@ -303,6 +303,9 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
>> */
>> flags &= ~(__GFP_DMA | __GFP_HIGHMEM);
>>
>> + /* Convert the size to actually allocated. */
>> + size = 1UL << (order + XEN_PAGE_SHIFT);
>> +
>> /* On ARM this function returns an ioremap'ped virtual address for
>> * which virt_to_phys doesn't return the corresponding physical
>> * address. In fact on ARM virt_to_phys only works for kernel direct
>> @@ -351,6 +354,9 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
>> * physical address */
>> phys = xen_bus_to_phys(dev_addr);
>>
>> + /* Convert the size to actually allocated. */
>> + size = 1UL << (order + XEN_PAGE_SHIFT);
>> +
>> if (((dev_addr + size - 1 <= dma_mask)) ||
>> range_straddles_page_boundary(phys, size))
>> xen_destroy_contiguous_region(phys, order);
>> --
>> 2.15.2 (Apple Git-101.1)
>>
>>


--
Oracle <http://www.oracle.com>
Joe Jin | Software Development Director
ORACLE | Linux and Virtualization
500 Oracle Parkway Redwood City, CA US 94065

\
 
 \ /
  Last update: 2018-10-19 04:17    [W:0.106 / U:5.756 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site