On Tuesday 19 April 2016 16:27:03 David Daney wrote: > The way the MMC driver works is that the driver's .request() method is > called to initiate a request. After .request() is finished, it returns > back to the kernel so other work can be done. > > From the interrupt handler, when the request is complete, the interrupt > handler calls req->done(req); to terminate the whole thing. > > > So we have: > > CPU-A CPU-B CPU-C > > octeon_mmc_request(0) . . > down() . . > queue_request(0); . . > return; . . > other_useful_work . . > . . . > . . . > . . . > octeon_mmc_request(1) . . > down() -> blocks . . > octeon_mmc_interrupt() . > up() -> unblocks . > down() <-unblocks req->done(0) . > queue_request(1); return; . > return; . . > other_useful_work . . > . . octeon_mmc_interrupt() > . . up() > . . req->done(1) > . . return; > . . . > > > We don't want to have the thread on CPU-A wait around in an extra mutex > or completion for the command to finish. The MMC core already has its > own request waiting code, but it doesn't handle the concept of a slot. > These commands can take hundreds or thousands of mS to terminate. The > whole idea of the MMC framework is to queue the request and get back to > doing other work ASAP. Right. This usecase seems to be one of the few that legitimately use semaphores. However, there has been a long effort to eliminate the remaining semaphores from the kernel (mostly much stalled since 2.6.32, but we still try to prevent new ones from creeping in). I had at one point a patch series that removed all the remaining semaphores, but didn't get it merged at the time, and now there are a few dozen new users. > In the case of this octeon_mmc driver we need to serialize the commands > issued to multiple slots, for this we use the semaphore. If you don't > like struct semaphore, we would have to invent a proprietary wait queue > mechanism that has semantics nearly identical to struct semaphore, and > people would complain that we are reinventing the semaphore. > > It doesn't seem clean to cobble up multiple waiting structures > (completion + mutex + logic that surely would contain errors) where a > single (well debugged) struct semaphore does what we want. I think using a wait_event() call could make do this with basically the same amount of code and by naming it right, this could be just as expressive. For the usecase you describe, a single completion structure would actually serve the purpose as well (no need for the mutex), but it would be unusual to wait for the completion before starting the work. If you do this wait_event(&host->wq, !host->current_req); in the request function as well as wake_up(&host->wq); after setting host->current_req to NULL, I think you end up with the same level of readability as the semaphore, and slightly better object code, and you can drop the 'WARN_ON(host->current_req);' which would be known to be impossible. Arnd -- 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
