On Fri, Aug 12, 2016 at 2:03 PM, Takashi Iwai tiwai@suse.de wrote:
On Fri, 12 Aug 2016 13:10:37 +0200, Andrej Kruták wrote:
+static void line6_hwdep_push_message(struct usb_line6 *line6) +{
unsigned long head = line6->buffer_circular.head;/* The spin_unlock() and next spin_lock() provide needed ordering. */unsigned long tail = ACCESS_ONCE(line6->buffer_circular.tail);if (!line6->buffer_circular.active)return;if (CIRC_SPACE(head, tail, LINE6_MESSAGE_MAXCOUNT) >= 1) {unsigned char *item = &line6->buffer_circular.data[head * LINE6_MESSAGE_MAXLEN];memcpy(item, line6->buffer_message, line6->message_length);line6->buffer_circular.data_len[head] = line6->message_length;smp_store_release(&line6->buffer_circular.head,(head + 1) & (LINE6_MESSAGE_MAXCOUNT - 1));up(&line6->buffer_circular.sem);}Hmm... this kind of a simple FIFO can be seen in anywhere in the kernel code, and I'm sure that you can find an easier way to implement it. The whole code looks a bit scary as it being home-brewed.
This code is based on Documentation/circular-buffers.txt, except for the semaphore magic.
The example there is basically a semi lock-free implementation. For your purpose it's an overkill. This is no severely hot path, thus a simpler version would make life easier.
Fair enough, I'll spinlock-ize it then.
Also, the blocking read/write control isn't usually done by a semaphore. Then you can handle the interrupt there.
I actually wonder why, semaphores seemed perfect for this... Do you have some hints?
Assume you want to interrupt the user-space app while it's being blocked by the semaphore. With your code, you can't.
You can - down_interruptible() is there for this exact reason. As said, semaphore is basically wait-queue + counter, so there's no real reason not to do this IMO.
But in the end - if you have some nice example of FIFO buffer in some simple driver, I have no problem using a more already-proven/standard way :-)