Re: [Xen-devel] [PATCH net v2 2/3] xen-netback: don't stop dealloc kthread too early
From: David Vrabel <hidden>
Date: 2014-08-11 15:23:30
On 11/08/14 15:44, Wei Liu wrote:
quoted hunk ↗ jump to hunk
On Mon, Aug 11, 2014 at 03:13:41PM +0100, Zoltan Kiss wrote: [...]quoted
quoted
And cleaning it up a bit (the while() could be a for(;;)).I would recommend this: ---@@ -2066,7 +2066,7 @@ int xenvif_dealloc_kthread(void *data) wait_event_interruptible(queue->dealloc_wq, tx_dealloc_work_todo(queue) || kthread_should_stop()); - if (kthread_should_stop()) + if (kthread_should_stop() && !atomic_read(&queue->inflight_packets)) break; xenvif_tx_dealloc_action(queue);--- If kthread_stop called, this will keep the main loop running until all callbacks are called. Then it proceeds to the exit branch, otherwise doesn't disrupt normal operation.This snippet lacks change to while(). I would generally go for a shorter solution if the code is self-explanatory.@@ -2078,21 +2066,19 @@ int xenvif_dealloc_kthread(void *data) { struct xenvif_queue *queue = data; - while (!kthread_should_stop()) { + for (;;) { wait_event_interruptible(queue->dealloc_wq, tx_dealloc_work_todo(queue) || kthread_should_stop());
This will never sleep if the thread is being stopped when there are packets in flight.
- if (kthread_should_stop())
+ if (kthread_should_stop() &&
+ !atomic_read(&queue->inflight_packets) &&
+ !tx_dealloc_work_todo(queue))
break;Moving the final dealloc into the loop adds a cond_resched() call. This is harmless but not really necessary when the thread is about to stop.
xenvif_tx_dealloc_action(queue);
cond_resched();
}
- /* Unmap anything remaining*/
- if (tx_dealloc_work_todo(queue))
- xenvif_tx_dealloc_action(queue);
-
return 0;
}David