Hi, On Fri, Sep 7, 2018 at 3:00 AM, <dkota@xxxxxxxxxxxxxx> wrote: >> In v2, I said: >> >>> I'm not sure where to comment about this, so adding it to the end: >>> >>> Between v1 and v2 you totally removed all the locking. Presumably >>> this is because you didn't want to hold the lock in >>> handle_fifo_timeout() while waiting for the completion. IMO taking >>> the lock out was the wrong thing to do. You should keep it, but just >>> drop the lock before wait_for_completion_timeout() and add it back >>> afterwards. Specifically you _don't_ want the IRQ and timeout code >>> stomping on each other. >> >> >> ...but still no spinlock? > > I see there is no need of taking the spinlock as timeout will be handled > after the calculated time as per data size and speed. > There is 99.9% less chances of interrupt during the timeout handler. >> >> >> >> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1201081 The thing is, we want it to be 100% reliable, not 99.9% reliable. Is it somehow wrong to add the spinlock? ...or are you noticing performance problems with the spinlock there? It's just nice not to have to think about it. -Doug
