14 Feb
                
                    2019
                
            
            
                14 Feb
                
                '19
                
            
            
            
        
    
                2:19 p.m.
            
        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.
- /* 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.
thanks,
Takashi