>[CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>
>On Tue, 29 Nov 2022 06:17:02 +0000 Lin Liu wrote:
>> A NAPI is setup for each network sring to poll data to kernel
>> The sring with source host is destroyed before live migration and
>> new sring with target host is setup after live migration.
>> The NAPI for the old sring is not deleted until setup new sring
>> with target host after migration. With busy_poll/busy_read enabled,
>> the NAPI can be polled before got deleted when resume VM.
>>
>> [50116.602938] BUG: unable to handle kernel NULL pointer dereference at
>> 0000000000000008
>> [50116.603047] IP: xennet_poll+0xae/0xd20
>> [50116.603090] PGD 0 P4D 0
>> [50116.603118] Oops: 0000 [#1] SMP PTI
>> [50116.604624] Call Trace:
>> [50116.604674] ? finish_task_switch+0x71/0x230
>> [50116.604745] ? timerqueue_del+0x1d/0x40
>> [50116.604807] ? hrtimer_try_to_cancel+0xb5/0x110
>> [50116.604882] ? xennet_alloc_rx_buffers+0x2a0/0x2a0
>> [50116.604958] napi_busy_loop+0xdb/0x270
>> [50116.605017] sock_poll+0x87/0x90
>> [50116.605066] do_sys_poll+0x26f/0x580
>> [50116.605125] ? tracing_map_insert+0x1d4/0x2f0
>> [50116.605196] ? event_hist_trigger+0x14a/0x260
>
>You can trim all the ' ? ' entries from the stack trace,
>and the time stamps, FWIW. Makes it easier to read.
Sure, will do in next version
>> [50116.613598] ? finish_task_switch+0x71/0x230
>> [50116.614131] ? __schedule+0x256/0x890
>> [50116.614640] ? recalc_sigpending+0x1b/0x50
>> [50116.615144] ? xen_sched_clock+0x15/0x20
>> [50116.615643] ? __rb_reserve_next+0x12d/0x140
>> [50116.616138] ? ring_buffer_lock_reserve+0x123/0x3d0
>> [50116.616634] ? event_triggers_call+0x87/0xb0
>> [50116.617138] ? trace_event_buffer_commit+0x1c4/0x210
>> [50116.617625] ? xen_clocksource_get_cycles+0x15/0x20
>> [50116.618112] ? ktime_get_ts64+0x51/0xf0
>> [50116.618578] SyS_ppoll+0x160/0x1a0
>> [50116.619029] ? SyS_ppoll+0x160/0x1a0
>> [50116.619475] do_syscall_64+0x73/0x130
>> [50116.619901] entry_SYSCALL_64_after_hwframe+0x41/0xa6
>> ...
>> [50116.806230] RIP: xennet_poll+0xae/0xd20 RSP: ffffb4f041933900
>> [50116.806772] CR2: 0000000000000008
>> [50116.807337] ---[ end trace f8601785b354351c ]---
>>
>> xen frontend should remove the NAPIs for the old srings before live
>> migration as the bond srings are destroyed
>>
>> There is a tiny window between the srings are set to NULL and
>> the NAPIs are disabled, It is safe as the NAPI threads are still
>> frozen at that time
>>
>
>Since this is a fix please add a Fixes tag, and add [PATCH net]
>to the subject.
>
Will do in next version
>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>> index 9af2b027c19c..dc404e05970c 100644
>> --- a/drivers/net/xen-netfront.c
>> +++ b/drivers/net/xen-netfront.c
>> @@ -1862,6 +1862,12 @@ static int netfront_resume(struct xenbus_device *dev)
>> netif_tx_unlock_bh(info->netdev);
>>
>> xennet_disconnect_backend(info);
>> +
>> + rtnl_lock();
>> + if (info->queues)
>> + xennet_destroy_queues(info);
>> + rtnl_unlock();
>Now all callers of xennet_disconnect_backend() destroy queues soon
>after, can we just move the destroy queues into disconnect ?
After the sring is destroyed, the queue and the bond NAPI should also be destroyed,
so Yes, destroy queues can be part of xennet_disconnect_backend,
However, some caller of xennet_disconnect_backend hold rtnl_lock while some others
not, I think it is simpler to keep seperate.
>
>> return 0;
>> }
>>
