Re: [PATCH 5/6] mmc: debugfs: Move card status retrieveal into the block layer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[...]

>> 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.

Done!

>
>>> +#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.
>
> In that case CONFIG_MMC_BLOCK contains 'm' instead of 'y'
> which confusingly does not evaluate to true in the preprocessor
> (it assumes it is a misspelled 'n' I guess).
>
> And then the autobuilders wreak havoc.
>
> And that is why the IS_ENABLED() defines exist in the first
> place IIUC.

Thanks for explanation! It makes more sense to me now!

>
> 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.
>
>>>  void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req);
>>
>> I don't get what mmc_blk_issue_rq() has to do with this change? Could
>> you please explain?
>
> If we start doing stubs we should stub everything IMO, but if you
> prefer I can make that a separate patch. Seems a bit overdone
> though.
>
>> Hmm, this thing seems a bit upside-down.
>>
>> Currently it's possible to build the mmc block device driver as a
>> module. In cases like this, accessing the card debugs node to request
>> the card's status, would trigger a call to mmc_blk_card_status_get().
>> How would that work when the mmc block device driver isn't loaded and
>> probed?
>
> Module symbol resolution and driver loading is always necessary,
> it is no different for e.g. a wifi driver using the 802.11 library.
> It is definately possible to shoot oneself in the foot, but I think
> udev & friends usually load things in the right order?

My my point is that *if* the card debugfs nodes shows up to the user -
they should work (no matter if the mmc block device has been probed or
not).

Can we guarantee that, is this approach?

>
>> 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.

Yes, that's a nice option.

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

Again, this is for debug purpose. I don't see an issue moving also
this one within CONFIG_MMC_BLOCK.

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

Right.

According to Avri, apparently some userspace tools measuring
performance. I would be interested to know a bit more about those
tools.

Kind regards
Uffe



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux