Re: [RESEND PATCH v7 2/2] mmc: OCTEON: Add host driver for OCTEON MMC controller

[Arnd Bergmann <arnd@arndb.de>]

On Friday 22 April 2016 10:49:06 David Daney wrote:
> On 04/22/2016 09:42 AM, Arnd Bergmann wrote:
> > On Friday 22 April 2016 15:54:56 Ulf Hansson wrote:
> >>
> >>>
> >>> 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!?
> >
> > Yes, I think that would be good, to avoid curses from whoever tries
> > to eliminate them the next time.
> >
> > I think there is some renewed interest in realtime kernels these
> > days, and semaphores are known to cause problems with priority
> > inversion (precisely because you don't know which thread will
> > release it).
> 
> In this particular case, there can be no priority inversion, as the 
> thing we are waiting for is a hardware event.  The timing of that is not 
> influenced by task scheduling.  The use of wait_event instead of struct 
> semaphore would be purely cosmetic.

My point was just that it's possible someone will try to remove all
the semaphores again soon. You are right that any possible priority
inversion in the existing code would not get changed by using a
different method to wait for the completion of the request.

	Arnd