| Message ID | 1486731352-8018-10-git-send-email-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 10, 2017 at 1:55 PM, Adrian Hunter <adrian.hunter@intel.com> wrote: > Add a function to control wake-up on new requests. This will enable > Software Command Queuing to choose whether or not to queue new > requests immediately or wait for the current task to complete. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> I think my patch set killed off the context and this whole thing "just works" instead of needing more states and hacks like this. The proliferation of states is reaching a all time high here: + if (cntx->is_waiting_last_req != wake_me) { + spin_lock_irq(q->queue_lock); + cntx->is_waiting_last_req = wake_me; + cntx->is_new_req = wake_me && mq->is_new_req; (...) @@ -43,6 +43,7 @@ struct mmc_queue { (...) + bool is_new_req; Now there is a is_new_req in the context AND an identically named state variable in struct mmc_queue, and when reading the code I suspect a reader would like to know the difference between the two .is_new_req variables, one in the state of the host and one in the state of the queue. This is a cognitive nightmare, sorry. At least this would have to have more specific names. I think all these states come from the polling loop construction and this constant spinning around and checking for errors, doing and undoing the pre/post/bounce stuff, and compulsively flushing the pipeline with NULL and so on. I was trying to get rid of that stuff, but this is expanding it instead. Hm, maybe I get a bit on the complainy side, sorry if so... Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index 479638bfb092..9e7e4eb9250c 100644 --- a/drivers/mmc/core/queue.c +++ b/drivers/mmc/core/queue.c @@ -97,6 +97,7 @@ static int mmc_queue_thread(void *d) cntx->is_waiting_last_req = false; cntx->is_new_req = false; if (!req) { + mq->is_new_req = false; /* * Dispatch queue is empty so set flags for * mmc_request_fn() to wake us up. @@ -147,6 +148,8 @@ static void mmc_request_fn(struct request_queue *q) return; } + mq->is_new_req = true; + cntx = &mq->card->host->context_info; if (cntx->is_waiting_last_req) { @@ -158,6 +161,19 @@ static void mmc_request_fn(struct request_queue *q) wake_up_process(mq->thread); } +void mmc_queue_set_wake(struct mmc_queue *mq, bool wake_me) +{ + struct mmc_context_info *cntx = &mq->card->host->context_info; + struct request_queue *q = mq->queue; + + if (cntx->is_waiting_last_req != wake_me) { + spin_lock_irq(q->queue_lock); + cntx->is_waiting_last_req = wake_me; + cntx->is_new_req = wake_me && mq->is_new_req; + spin_unlock_irq(q->queue_lock); + } +} + static struct scatterlist *mmc_alloc_sg(int sg_len) { struct scatterlist *sg; diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h index 871796c3f406..c9ce0172dd08 100644 --- a/drivers/mmc/core/queue.h +++ b/drivers/mmc/core/queue.h @@ -43,6 +43,7 @@ struct mmc_queue { struct semaphore thread_sem; bool suspended; bool asleep; + bool is_new_req; struct mmc_blk_data *blkdata; struct request_queue *queue; struct mmc_queue_req *mqrq; @@ -69,5 +70,6 @@ extern unsigned int mmc_queue_map_sg(struct mmc_queue *, extern struct mmc_queue_req *mmc_queue_req_find(struct mmc_queue *, struct request *); extern void mmc_queue_req_free(struct mmc_queue *, struct mmc_queue_req *); +extern void mmc_queue_set_wake(struct mmc_queue *, bool); #endif
Add a function to control wake-up on new requests. This will enable Software Command Queuing to choose whether or not to queue new requests immediately or wait for the current task to complete. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/core/queue.c | 16 ++++++++++++++++ drivers/mmc/core/queue.h | 2 ++ 2 files changed, 18 insertions(+)