On Tue, May 23, 2017 at 11:49 AM, Linus Walleij
[off-list ref] wrote:

On Mon, May 22, 2017 at 9:42 AM, Ulf Hansson [off-list ref] wrote:

quoted

On 19 May 2017 at 15:37, Linus Walleij [off-list ref] wrote:

quoted

quoted

The debugfs file "status" (in e.g. /debug/mmc3/mmc3:0001) is
only available if and only if the card used is an (e)MMC or
SD card, not for SDIO, as can be seen from this guard in
mmc_add_card_debugfs();

if (mmc_card_mmc(card) || mmc_card_sd(card))
  (...create debugfs "status" entry...)

Further this debugfs entry suffers from all the same starvation
issues as the other userspace things, under e.g. a heavy
dd operation.

It is therefore logical to move this over to the block layer
when it is enabled, using the new custom requests and issue
it using the block request queue.

This makes this debugfs card access land under the request
queue host lock instead of orthogonally taking the lock.

Tested during heavy dd load by cat:in the status file. We
add IS_ENABLED() guards and keep the code snippet just
issueing the card status as a static inline in the header
so we can still have card status working when the block
layer is compiled out.

Seems like a bit of re-factoring/cleanup could help here as
preparation step. I just posted a patch [1] cleaning up how the mmc
block layer fetches the card's status.

Perhaps that could at least simplify a bit for $subject patch,
especially since it also makes mmc_send_status() being exported.

OK if you merge your stuff I can iterate my patches on top,
no problem.

quoted

quoted

+#if IS_ENABLED(CONFIG_MMC_BLOCK)

What wrong with a regular ifdefs stubs? Why do you need IS_ENABLED()?

This is because the entire block layer can be compiled as a
module.

The block layer cannot be a module, but the MMC layer can.

I'm all for making CONFIG_MMC_BLOCK into a bool... but
don't know how people (Intel laptops) feel about that extra
code in their kernel at all times.

I think CONFIG_MMC should remain tristate. However, we could
consider making CONFIG_MMC_BLOCK a 'bool' symbol and
linking it into the mmc-core module when enabled.

quoted

It seems like the life cycle of the card debugfs node is now being
controlled as when the mmc block device driver has been successfully
probed. We need to deal with that somehow.

Only for these two files but yes.

ext_csd is a bit dubious as it is only available on storage devices
(eMMC) that can not be SD, SDIO or combo cards, and we could make
it only appear if using the block layer.

The card status however we need to keep if people want it.

We *COULD* consider just thrashing these debugfs files. It is not
technically ABI and I wonder who is actually using them.

I think the ext_csd is extremely useful on storage devices to have
exposed to user space in some form. There is a comment saying
that we don't store it in RAM at the moment as it is rarely used,
but we could change that, as it's only 512 bytes per eMMC device
and there is rarely more than one of them. IIRC, the ext_csd is
entirely readonly and doesn't change during runtime, so we
gain nothing by issuing the commands every time someone reads
the debugfs file.

       Arnd