14 Feb
2019
14 Feb
'19
4:13 p.m.
On 2/14/19 7:19 AM, Takashi Iwai wrote:
On Wed, 13 Feb 2019 23:07:27 +0100, Pierre-Louis Bossart wrote:
+static size_t sof_wait_trace_avail(struct snd_sof_dev *sdev,
loff_t pos, size_t buffer_size)+{
- wait_queue_entry_t wait;
- /*
* If host offset is less than local pos, it means write pointer of* host DMA buffer has been wrapped. We should output the trace data* at the end of host DMA buffer at first.*/- if (sdev->host_offset < pos)
return buffer_size - pos;- /* If there is available trace data now, it is unnecessary to wait. */
- if (sdev->host_offset > pos)
return sdev->host_offset - pos;- /* wait for available trace data from FW */
- init_waitqueue_entry(&wait, current);
- set_current_state(TASK_INTERRUPTIBLE);
- add_wait_queue(&sdev->trace_sleep, &wait);
- if (signal_pending(current)) {
remove_wait_queue(&sdev->trace_sleep, &wait);- } else {
/* set timeout to max value, no error code */schedule_timeout(MAX_SCHEDULE_TIMEOUT);remove_wait_queue(&sdev->trace_sleep, &wait);- }
Can be slightly optimized here, e.g. no need of two remove_wait_queue() calls.
Yes. This was cleaned-up to remove a goto but we missed the common code. will fix
- /* return bytes available for copy */
- if (sdev->host_offset < pos)
return buffer_size - pos;- return sdev->host_offset - pos;
Are these sdev->host_offset accesses race-free?
If I understand correctly, snd_sof_trace_update_pos() can be called at any time, and the offset might become inconsistent during these multiple references.
It's my understanding that there is a single trace stream and a single consumer (via a debugfs file), but will double check.