On 21 April 2016 at 15:19, Arnd Bergmann <arnd@xxxxxxxx> wrote: > On Thursday 21 April 2016 14:44:08 Ulf Hansson wrote: >> [...] >> >> >> So the question I have is *why* do you have to be in IRQ context when >> >> using the semaphore... >> >> >> >> I would rather see that you use a threaded IRQ handler, perhaps in >> >> conjunction with a hard IRQ handler if that is needed. >> > >> > That does not solve the problem though: it is not allowed for a mutex >> > to be taken in the request function but released in the interrupt, >> > both have to be in the same thread. >> > >> > Using a threaded IRQ handler would help by avoiding the spinlock >> > inside of it (it could be replaced with a mutex there), but it >> > doesn't solve the problem of serializing between the slots. >> >> My point is, the may not need to be released in the IRQ handler, but >> instead in the threaded IRQ handler. > > That doesn't change anything. > >> >> Although to clarify a bit more, the mmc core invokes *all* mmc host >> >> ops callbacks from non-atomic context. >> > >> > Oh, so you mean the .request() function must not sleep and cannot >> > call mutex_lock() or down() or wait_event()? >> >> No. >> >> *non-atomic* context. :-) >> >> I should probably said thread/process context instead. >> > > My mistake, you were being clear enough here, I just misread. > >> >> In the ->request() callback: >> >> .... >> >> mutex_lock() > >> >> In the threaded IRQ handler: >> >> ... >> >> mutex_unlock() >> >> >> >> Do you think something along these lines should work for your case? >> > >> > This is the case I described above, it is against the rules for mutexes() >> > and you will get a lockdep warning if you attempt this. >> >> No. >> >> Considering my comments in this reply, can you please re-think and see >> if my solution could make sense? > > The problem is not that mutex_unlock() has to be called from non-atomic > context (it doesn't, you are allowed to call mutex_unlock while holding > a spinlock), but instead the problem is that it has to be done from the > same thread that called mutex_lock(), and the threaded IRQ handler > is never the same thread that is currently holding the mutex. Of course, you are right! > > My suggestion with using > > wait_event(host->wq, !cmpxchg(host->current_req, NULL, mrq)); > > should sufficiently solve the problem, but the suggestion of using > a kthread (even though not needed for taking a mutex) would still > have some advantages and one disadvantage: > > + We never need to spin in the irq context (also achievable using > a threaded handler) > + The request callback always returns immediately after queuing up > the request to the kthread, rather than blocking for a potentially > long time while waiting for an operation in another slot to complete > + it very easily avoids the problem of serialization between > the slots, and ensures that each slot gets an equal chance to > send the next request. > - you get a slightly higher latency for waking up the kthread in > oder to do a simple request (comparable amount of latency that > is introduced by an irq thread). > Currently I can't think of anything better, so I guess something along these lines is worth a try. No matter what, I guess we want to avoid to use a semaphore as long as possible, right!? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
