Re: [PATCH] n_tty: Fix unordered accesses to lockless read buffer

[Peter Hurley]

On 01/01/2015 06:00 AM, Christian Riesch wrote:
> Peter,
> 
> Thank you for this patch! Unfortunately I had not much time for this
> since my last patch submission, so I am happy that someone continued
> this work.
> 
> I have a few comments/questions, please see below.
> 
> On Tue, Dec 30, 2014 at 1:05 PM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote:
>> Add commit_head buffer index, which the producer-side publishes
>> after input processing. This ensures the consumer-side observes
>> correctly-ordered writes in raw mode
> 
> I understand that the commit_head reduces the number of memory
> barriers and makes some things easier. But what is so special about
> raw mode that requires the introduction of commit_head?

commit_head is simply the read_head after each received buffer is
processed by the input worker. In this context, I meant 'raw mode' as
any non-canonical mode, ie., any mode in which copy_from_read_buf()
is used by the reader. I'll replace 'raw' with 'non-canonical' instead.

>> (ie., the buffer data is
>> written before the buffer index is advanced).
>>
>> Further, remove read_cnt() and expand inline, using ACCESS_ONCE()
> 
> "ACCESS_ONCE() and memory barriers"?

This portion of the changelog refers only to read_cnt(), for which
memory barriers are not required.

But, while writing the explanatory fragment [1], I realized that
input_available_p() must load the most recent relevant head index
(either canon_head or commit_head based on the mode) because
it may conclude there is no more input _and_ then observe an end-of-file
condition. So I need to fix this up to do the smp_load_acquire() in
input_available_p() and ACCESS_ONCE() in the *_copy_from_read_buf().

Regards,
Peter Hurley

[1]
Strictly speaking, the ACCESS_ONCE()'s are optimizations. Neither the
producer nor consumer require the most recent 'opposed' index; if the
compiler elided the opposed index load, instead reusing an existing load




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/