Re: [PATCH] mmc: core: remove issue_fn indirect function call

[Grant Grundler]

On Fri, Sep 27, 2013 at 2:04 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
...
>> Two ways to handle this: I can
>> 1) add a local function prototype of mmc_blk_issue_rq() to queue.c
>> 2) move mmc_init_queue() and mmc_queue_thread() from queue.c to block.c
>>
>> (2) actually makes sense since both functions are block IO specific.
>>
>> Thoughts? Preference? Other ideas?
>
> Hi Grant,
>
> Generally I am in favour of cleaning up messy code. But in this case
> it now seems a bit overworked.
>
> How about actually leaving it as is?

Hi Ulf,
Sure - I can leave it.

I'll point out that mmcqd uses about 1/2 a CPU core when busy driving
eMMC devices behind dw_mmc interface. That's pretty bad. I haven't
worked out exact "service demand" (avg CPU cycles to service one IO)
but that is the right metric to be looking at when evaluating design
changes.

My feeling is there too many layers in this subsystem. I count 6
layers of data structures when starting with struct mmc_blk_data. I've
attached my drawing (scanned pdf). "boxes" are host memory footprint.
Arrows are pointers to other host memory.

I thought the block layer provides sufficient IO request queueing and
can't justify why this code needs to manage it's own array (of two)
struct mmc_queue_req. Given how much faster CPUs have always been than
block storage, the difference in latency (nano or microseconds at
best) doesn't seem worth managing our own local queues - especially
since I'm convinced this code has SMP bugs that don't exist in the
block layer.

cheers,
grant

Attachment: 20130927141112165.pdf
Description: Adobe PDF document