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-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
