Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1370085 > unrolled thread

[bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)

Started byPeter Hurley <peter@hurleysoftware.com>
First post2016-04-03 05:00 +0200
Last post2016-04-06 10:30 +0200
Articles 12 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [bisect] Merge tag 'mmc-v4.6' of  git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6) Peter Hurley <peter@hurleysoftware.com> - 2016-04-03 05:00 +0200
    Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc  (was [GIT PULL] MMC for v.4.6) Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-03 14:00 +0200
      Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc  (was [GIT PULL] MMC for v.4.6) Ulf Hansson <ulf.hansson@linaro.org> - 2016-04-04 13:30 +0200
        Re: [bisect] Merge tag 'mmc-v4.6' of  git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6) Peter Hurley <peter@hurleysoftware.com> - 2016-04-04 19:00 +0200
        Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc  (was [GIT PULL] MMC for v.4.6) Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-04 21:00 +0200
          Re: [bisect] Merge tag 'mmc-v4.6' of  git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6) Peter Hurley <peter@hurleysoftware.com> - 2016-04-04 21:30 +0200
            Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc  (was [GIT PULL] MMC for v.4.6) Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-04 21:50 +0200
              Re: [bisect] Merge tag 'mmc-v4.6' of  git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6) Peter Hurley <peter@hurleysoftware.com> - 2016-04-04 22:10 +0200
          Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc  (was [GIT PULL] MMC for v.4.6) Ulf Hansson <ulf.hansson@linaro.org> - 2016-04-05 11:10 +0200
            Re: [bisect] Merge tag 'mmc-v4.6' of  git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6) Peter Hurley <peter@hurleysoftware.com> - 2016-04-06 02:30 +0200
            Re: [bisect] Merge tag 'mmc-v4.6' of  git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6) Jisheng Zhang <jszhang@marvell.com> - 2016-04-06 10:00 +0200
              Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc  (was [GIT PULL] MMC for v.4.6) Ulf Hansson <ulf.hansson@linaro.org> - 2016-04-06 10:30 +0200

#1370085 — [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)

FromPeter Hurley <peter@hurleysoftware.com>
Date2016-04-03 05:00 +0200
Subject[bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)
Message-ID<rjGqB-531-1@gated-at.bofh.it>
On 03/21/2016 05:59 AM, Ulf Hansson wrote:
> Hi Linus,
> 
> Here's the PR for MMC v4.6.
> 
> Details about the highlights are as usual found in the signed tag.
> 
> Please pull this in!
> 
> Kind regards
> Ulf Hansson
> 
> 
> The following changes since commit fc77dbd34c5c99bce46d40a2491937c3bcbd10af:
> 
>   Linux 4.5-rc6 (2016-02-28 08:41:20 -0800)
> 
> are available in the git repository at:
> 
>   git://git.linaro.org/people/ulf.hansson/mmc.git tags/mmc-v4.6
> 
> for you to fetch changes up to 64e5cd723120643a4c8bef0880a03a60161d3ccb:
> 
>   mmc: sdhci-of-at91: fix wake-up issue when using runtime pm
> (2016-03-18 09:12:32 +0100)

This merge commit e531cdf50a8a0fb7a4d51c06e52097bd01e9bf7c
Merge: 4526b71 64e5cd7
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon Mar 21 14:35:52 2016 -0700

    Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc
    
    Pull MMC updates from Ulf Hansson:
    ...

is fingered by git bisect as the cause for a mashup of mmc block devices
on a Beaglebone Black:

[    2.916209] mmc1: new high speed MMC card at address 0001
[    2.923755] mmcblk0: mmc1:0001 MMC04G 3.60 GiB
[    2.929479] mmcblk0boot0: mmc1:0001 MMC04G partition 1 2.00 MiB
[    2.936821] tps65217 0-0024: TPS65217 ID 0xe version 1.2
[    2.942270] mmcblk0boot1: mmc1:0001 MMC04G partition 2 2.00 MiB
[    2.949388]  mmcblk0: p1 p2
[    2.950031] mmc0: new high speed SDHC card at address e624
[    2.961118] mmcblk1: mmc0:e624 SU08G 7.40 GiB
[    2.967047] at24 0-0050: 32768 byte 24c256 EEPROM, writable, 1 bytes/write
[    2.974190]  mmcblk1: p1 p2

Note how mmc1 => mmcblk0 and mmc0 => mmcblk1.

This produces a failure to boot as the wrong partition is mounted as
root (/dev/mmcblk0p2 is now on the wrong mmc).

The bisect tried all the mmc tree patches which were all good.
I double-checked by cloning the mmc tree and building both mmc-v4.6
and v4.5-rc6, and both tested good.

I interpret that to mean some change in mmc + some new behavior elsewhere
for v4.6 is causing this. Any ideas?

Regards,
Peter Hurley

[toc] | [next] | [standalone]


#1370204 — Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-04-03 14:00 +0200
SubjectRe: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)
Message-ID<rjORc-2Nz-9@gated-at.bofh.it>
In reply to#1370085
On Sat, Apr 2, 2016 at 9:56 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>
> Note how mmc1 => mmcblk0 and mmc0 => mmcblk1.
>
> This produces a failure to boot as the wrong partition is mounted as
> root (/dev/mmcblk0p2 is now on the wrong mmc).

It *looks* very much like somebody is doing asynchronous probing of
the bus, meaning that the devices get probed in random order.

And that "random order" is admittedly probably usually fairly static
on any particular hardware platform, but then something happens to
change timing, and...

This is why you should never probe the actual *bus* asynchronously,
just do the end-point setup async. For example, you'd enumerate ports
(and assign devices to the ports) synchronously, but then after device
assignment the actual device probing can be async.

> The bisect tried all the mmc tree patches which were all good.
> I double-checked by cloning the mmc tree and building both mmc-v4.6
> and v4.5-rc6, and both tested good.
>
> I interpret that to mean some change in mmc + some new behavior elsewhere
> for v4.6 is causing this. Any ideas?

Hmm. If it really is just timing, it could have been around forever,
and just hidden by the fact that normally mmc0 gets probed before
mmc1, but then some other probing thing slowed down or the exact
details of the async workqueue  scheduling changed, and now mmc1 just
*happens* to get probed first..

The thing that changed scheduling order could easily have come from
some non-mmc change.

NOTE! I have nothing to back this up except that (a) we've had
problems like this before and (b) it does look from your dmesg that
mmcX is simply probed in the "wrong" order. I didn't look at exactly
what mmc does or who does the probing.

Maybe Ulf can explain what it is that is _supposed_ to keep the mmc
probe order stable. Ulf?

             Linus

[toc] | [prev] | [next] | [standalone]


#1370559 — Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-04-04 13:30 +0200
SubjectRe: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)
Message-ID<rkaRI-2fN-11@gated-at.bofh.it>
In reply to#1370204
On 3 April 2016 at 13:54, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sat, Apr 2, 2016 at 9:56 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>
>> Note how mmc1 => mmcblk0 and mmc0 => mmcblk1.
>>
>> This produces a failure to boot as the wrong partition is mounted as
>> root (/dev/mmcblk0p2 is now on the wrong mmc).
>
> It *looks* very much like somebody is doing asynchronous probing of
> the bus, meaning that the devices get probed in random order.

Correct.

>
> And that "random order" is admittedly probably usually fairly static
> on any particular hardware platform, but then something happens to
> change timing, and...
>
> This is why you should never probe the actual *bus* asynchronously,
> just do the end-point setup async. For example, you'd enumerate ports
> (and assign devices to the ports) synchronously, but then after device
> assignment the actual device probing can be async.

So to do this, we need to tie the mmc/sd/sdio controller to a
dedicated mmcblk id.

There have been some ideas to fix this by using "aliases" in a DT
based configuration.

>
>> The bisect tried all the mmc tree patches which were all good.
>> I double-checked by cloning the mmc tree and building both mmc-v4.6
>> and v4.5-rc6, and both tested good.
>>
>> I interpret that to mean some change in mmc + some new behavior elsewhere
>> for v4.6 is causing this. Any ideas?
>
> Hmm. If it really is just timing, it could have been around forever,
> and just hidden by the fact that normally mmc0 gets probed before
> mmc1, but then some other probing thing slowed down or the exact
> details of the async workqueue  scheduling changed, and now mmc1 just
> *happens* to get probed first..
>
> The thing that changed scheduling order could easily have come from
> some non-mmc change.
>
> NOTE! I have nothing to back this up except that (a) we've had
> problems like this before and (b) it does look from your dmesg that
> mmcX is simply probed in the "wrong" order. I didn't look at exactly
> what mmc does or who does the probing.
>
> Maybe Ulf can explain what it is that is _supposed_ to keep the mmc
> probe order stable. Ulf?
>
>              Linus

The commit that's likely to cause the regression is:
520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
simultaneously").

This commit further enables asynchronous detection of (e)MMC/SD/SDIO
cards, by converting from an *ordered* work-queue to a *non-ordered*
work-queue for card detection.

Although, one should know that there have *never* been any guarantees
to get a fixed mmcblk id for a card. I expect that's what has been
assumed here.

Let me elaborate a bit on the card detection procedure. When the mmc
controller has been successfully probed, its driver schedules a work
to start enumeration of cards. Only cards that gets detected
successfully becomes registered and those gets an mmcblk id assigned
to it. The picked id, is the first available starting from zero. Now,
as cards can be removable and because drivers for mmc controllers may
sometimes returns -EPROBE_DEFER (for whatever reason), there's never
been support for fixed mmcblk ids.

To deal with this, one should use the so called UUID/PARTUUID. Is
there any reasons to why that can't be done in this case?

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]


#1370770

FromPeter Hurley <peter@hurleysoftware.com>
Date2016-04-04 19:00 +0200
Message-ID<rkg14-61K-9@gated-at.bofh.it>
In reply to#1370559
On 04/04/2016 04:29 AM, Ulf Hansson wrote:
> On 3 April 2016 at 13:54, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>> On Sat, Apr 2, 2016 at 9:56 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>
>>> Note how mmc1 => mmcblk0 and mmc0 => mmcblk1.
>>>
>>> This produces a failure to boot as the wrong partition is mounted as
>>> root (/dev/mmcblk0p2 is now on the wrong mmc).
>>
>> It *looks* very much like somebody is doing asynchronous probing of
>> the bus, meaning that the devices get probed in random order.
> 
> Correct.
> 
>>
>> And that "random order" is admittedly probably usually fairly static
>> on any particular hardware platform, but then something happens to
>> change timing, and...
>>
>> This is why you should never probe the actual *bus* asynchronously,
>> just do the end-point setup async. For example, you'd enumerate ports
>> (and assign devices to the ports) synchronously, but then after device
>> assignment the actual device probing can be async.
> 
> So to do this, we need to tie the mmc/sd/sdio controller to a
> dedicated mmcblk id.
> 
> There have been some ideas to fix this by using "aliases" in a DT
> based configuration.
> 
>>
>>> The bisect tried all the mmc tree patches which were all good.
>>> I double-checked by cloning the mmc tree and building both mmc-v4.6
>>> and v4.5-rc6, and both tested good.
>>>
>>> I interpret that to mean some change in mmc + some new behavior elsewhere
>>> for v4.6 is causing this. Any ideas?
>>
>> Hmm. If it really is just timing, it could have been around forever,
>> and just hidden by the fact that normally mmc0 gets probed before
>> mmc1, but then some other probing thing slowed down or the exact
>> details of the async workqueue  scheduling changed, and now mmc1 just
>> *happens* to get probed first..
>>
>> The thing that changed scheduling order could easily have come from
>> some non-mmc change.
>>
>> NOTE! I have nothing to back this up except that (a) we've had
>> problems like this before and (b) it does look from your dmesg that
>> mmcX is simply probed in the "wrong" order. I didn't look at exactly
>> what mmc does or who does the probing.
>>
>> Maybe Ulf can explain what it is that is _supposed_ to keep the mmc
>> probe order stable. Ulf?
>>
>>              Linus
> 
> The commit that's likely to cause the regression is:
> 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> simultaneously").
> 
> This commit further enables asynchronous detection of (e)MMC/SD/SDIO
> cards, by converting from an *ordered* work-queue to a *non-ordered*
> work-queue for card detection.
> 
> Although, one should know that there have *never* been any guarantees
> to get a fixed mmcblk id for a card. I expect that's what has been
> assumed here.

Tell me about it. I'm in the middle of reverting non-blocking read()
behavior since 3.12 because _one_ userspace app relies on *blocking*
non-blocking read() behavior that existed before 3.12.


> Let me elaborate a bit on the card detection procedure. When the mmc
> controller has been successfully probed, its driver schedules a work
> to start enumeration of cards. Only cards that gets detected
> successfully becomes registered and those gets an mmcblk id assigned
> to it. The picked id, is the first available starting from zero. Now,
> as cards can be removable and because drivers for mmc controllers may
> sometimes returns -EPROBE_DEFER (for whatever reason), there's never
> been support for fixed mmcblk ids.
> 
> To deal with this, one should use the so called UUID/PARTUUID. Is
> there any reasons to why that can't be done in this case?

Well, I can.
But I'm not volunteering to update the other 250,000+ bootloader scripts
that do "root=/dev/mmcblk0p2"

Regards,
Peter Hurley

[toc] | [prev] | [next] | [standalone]


#1370821 — Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-04-04 21:00 +0200
SubjectRe: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)
Message-ID<rkhTb-7zr-1@gated-at.bofh.it>
In reply to#1370559
On Mon, Apr 4, 2016 at 4:29 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> The commit that's likely to cause the regression is:
> 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> simultaneously").

Peter, mind testing if you can revert that and get the old behavior
back? It seems to still revert cleanly, although I didn't check if the
revert actually then builds..

> This commit further enables asynchronous detection of (e)MMC/SD/SDIO
> cards, by converting from an *ordered* work-queue to a *non-ordered*
> work-queue for card detection.
>
> Although, one should know that there have *never* been any guarantees
> to get a fixed mmcblk id for a card. I expect that's what has been
> assumed here.

So quite frankly, for the whole "no regressions" issue, "documented
behavior" simply isn't an issue. It doesn't matter one whit or not if
something has been documented: if it has worked and people have
depended on it, it's what we in the industry call "reality".

And reality trumps documentation. Every time.

So it sounds like either that just needs to be reverted, or some other
way to get reliable device naming needs to happen.

So the *simple* model is to just scan the devices minimally serially,
and allocate the names at that point (so the names are reliable
between boots for the same hardware configuration). And then do the
more expensive device setup asynchronously (ie querying device
information, spinning up disks, whatever - things that can take
anything from milliseonds to several seconds, because they are doing
actual IO). So you'd do some very basic (and _often_ fairly quick)
operations serially, but then try to do the expensive parts
concurrently.

The SCSI layer actually goes a bit further than that: it has a fairly
asynchronous scanning thing, but it does allocate the actual host
device nodes serially, and then it even has an ordered list of
"scanning_hosts" that is used to complete the scanning in-order, so
that the sysfs devices show up in the right order even if things
actually got scanned out-of-order. So scans that finished early will
wait for other scans that are for "earlier" devices, and you end up
with what *looks* ordered to the outside, even if internally it was
all done out-of-order.

So there are multiple approaches to handling this, while still
allowing fairly asynchronous IO.

                 Linus

[toc] | [prev] | [next] | [standalone]


#1370831

FromPeter Hurley <peter@hurleysoftware.com>
Date2016-04-04 21:30 +0200
Message-ID<rkime-81e-9@gated-at.bofh.it>
In reply to#1370821
On 04/04/2016 11:59 AM, Linus Torvalds wrote:
> On Mon, Apr 4, 2016 at 4:29 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> The commit that's likely to cause the regression is:
>> 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
>> simultaneously").
> 
> Peter, mind testing if you can revert that and get the old behavior
> back? It seems to still revert cleanly, although I didn't check if the
> revert actually then builds..

Yeah, a straight revert of 520bd7a8b415 resumes normal service:

[    2.710232] mmc0: host does not support reading read-only switch, assuming write-enable
[    2.718437] mmc0: new high speed SDHC card at address e624
[    2.724801] mmcblk0: mmc0:e624 SU08G 7.40 GiB
[    2.730314]  mmcblk0: p1 p2
...
[    2.808938] mmc1: new high speed MMC card at address 0001
[    2.816352] mmcblk1: mmc1:0001 MMC04G 3.60 GiB
[    2.822075] mmcblk1boot0: mmc1:0001 MMC04G partition 1 2.00 MiB
[    2.829014] mmcblk1boot1: mmc1:0001 MMC04G partition 2 2.00 MiB
[    2.842600]  mmcblk1: p1 p2

Should I send a proper revert?


>> This commit further enables asynchronous detection of (e)MMC/SD/SDIO
>> cards, by converting from an *ordered* work-queue to a *non-ordered*
>> work-queue for card detection.
>>
>> Although, one should know that there have *never* been any guarantees
>> to get a fixed mmcblk id for a card. I expect that's what has been
>> assumed here.
> 
> So quite frankly, for the whole "no regressions" issue, "documented
> behavior" simply isn't an issue. It doesn't matter one whit or not if
> something has been documented: if it has worked and people have
> depended on it, it's what we in the industry call "reality".
> 
> And reality trumps documentation. Every time.
> 
> So it sounds like either that just needs to be reverted, or some other
> way to get reliable device naming needs to happen.
> 
> So the *simple* model is to just scan the devices minimally serially,
> and allocate the names at that point (so the names are reliable
> between boots for the same hardware configuration). And then do the
> more expensive device setup asynchronously (ie querying device
> information, spinning up disks, whatever - things that can take
> anything from milliseonds to several seconds, because they are doing
> actual IO). So you'd do some very basic (and _often_ fairly quick)
> operations serially, but then try to do the expensive parts
> concurrently.
> 
> The SCSI layer actually goes a bit further than that: it has a fairly
> asynchronous scanning thing, but it does allocate the actual host
> device nodes serially, and then it even has an ordered list of
> "scanning_hosts" that is used to complete the scanning in-order, so
> that the sysfs devices show up in the right order even if things
> actually got scanned out-of-order. So scans that finished early will
> wait for other scans that are for "earlier" devices, and you end up
> with what *looks* ordered to the outside, even if internally it was
> all done out-of-order.
> 
> So there are multiple approaches to handling this, while still
> allowing fairly asynchronous IO.
> 
>                  Linus
> 

[toc] | [prev] | [next] | [standalone]


#1370842 — Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-04-04 21:50 +0200
SubjectRe: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)
Message-ID<rkiFB-89A-17@gated-at.bofh.it>
In reply to#1370831
On Mon, Apr 4, 2016 at 12:29 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>
> Yeah, a straight revert of 520bd7a8b415 resumes normal service:

Ok, so we have that as an option.

> Should I send a proper revert?

Let's see if somebody can come up with a better serialization model.
We're only at rc2, and we now have a fallback fix, let's wait a week
or two to see if the mmc people can come up with something.

But maybe you can send me a revert patch around rc4 or so if the
problem still remains, ok?

               Linus

[toc] | [prev] | [next] | [standalone]


#1370850

FromPeter Hurley <peter@hurleysoftware.com>
Date2016-04-04 22:10 +0200
Message-ID<rkiYW-5L-25@gated-at.bofh.it>
In reply to#1370842
On 04/04/2016 12:49 PM, Linus Torvalds wrote:
> On Mon, Apr 4, 2016 at 12:29 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>
>> Yeah, a straight revert of 520bd7a8b415 resumes normal service:
> 
> Ok, so we have that as an option.
> 
>> Should I send a proper revert?
> 
> Let's see if somebody can come up with a better serialization model.
> We're only at rc2, and we now have a fallback fix, let's wait a week
> or two to see if the mmc people can come up with something.
> 
> But maybe you can send me a revert patch around rc4 or so if the
> problem still remains, ok?

Will do.

Regards,
Peter Hurley

[toc] | [prev] | [next] | [standalone]


#1371331 — Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-04-05 11:10 +0200
SubjectRe: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)
Message-ID<rkv9L-N0-3@gated-at.bofh.it>
In reply to#1370821
On 4 April 2016 at 20:59, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Mon, Apr 4, 2016 at 4:29 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> The commit that's likely to cause the regression is:
>> 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
>> simultaneously").
>
> Peter, mind testing if you can revert that and get the old behavior
> back? It seems to still revert cleanly, although I didn't check if the
> revert actually then builds..

I have checked, the revert should be a safe option. There is nothing
added on top that relies on it.

Moreover, I have no problem dealing with the revert, as it me
personally that screwed this up.

>
>> This commit further enables asynchronous detection of (e)MMC/SD/SDIO
>> cards, by converting from an *ordered* work-queue to a *non-ordered*
>> work-queue for card detection.
>>
>> Although, one should know that there have *never* been any guarantees
>> to get a fixed mmcblk id for a card. I expect that's what has been
>> assumed here.
>
> So quite frankly, for the whole "no regressions" issue, "documented
> behavior" simply isn't an issue. It doesn't matter one whit or not if
> something has been documented: if it has worked and people have
> depended on it, it's what we in the industry call "reality".
>
> And reality trumps documentation. Every time.

I totally agree.

Although, what puzzles me around this particular issue, is how an SoC
configuration can rely on this fragile behaviour.
All you have to do to break the assumption of fixed mmcblk ids, is to
boot with an SD card inserted and then without. Perhaps these SoCs
just doesn't support this use case!?

>
> So it sounds like either that just needs to be reverted, or some other
> way to get reliable device naming needs to happen.
>
> So the *simple* model is to just scan the devices minimally serially,
> and allocate the names at that point (so the names are reliable
> between boots for the same hardware configuration). And then do the
> more expensive device setup asynchronously (ie querying device
> information, spinning up disks, whatever - things that can take
> anything from milliseonds to several seconds, because they are doing
> actual IO). So you'd do some very basic (and _often_ fairly quick)
> operations serially, but then try to do the expensive parts
> concurrently.
>
> The SCSI layer actually goes a bit further than that: it has a fairly
> asynchronous scanning thing, but it does allocate the actual host
> device nodes serially, and then it even has an ordered list of
> "scanning_hosts" that is used to complete the scanning in-order, so
> that the sysfs devices show up in the right order even if things
> actually got scanned out-of-order. So scans that finished early will
> wait for other scans that are for "earlier" devices, and you end up
> with what *looks* ordered to the outside, even if internally it was
> all done out-of-order.
>
> So there are multiple approaches to handling this, while still
> allowing fairly asynchronous IO.

Thanks for sharing this information!

I will give it a try and see if I can come up with something that
restores the behaviour, but without having to do the revert. If it
turns out to be too complicated, I can post the revert in a couple of
rcs.

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]


#1372092

FromPeter Hurley <peter@hurleysoftware.com>
Date2016-04-06 02:30 +0200
Message-ID<rkJw5-3LU-5@gated-at.bofh.it>
In reply to#1371331
On 04/05/2016 01:59 AM, Ulf Hansson wrote:
> Although, what puzzles me around this particular issue, is how an SoC
> configuration can rely on this fragile behaviour.
> All you have to do to break the assumption of fixed mmcblk ids, is to
> boot with an SD card inserted and then without. Perhaps these SoCs
> just doesn't support this use case!?

Both configurations boot reliably; without the uSD inserted, the
boot and root partitions on the eMMC are booted instead.

Without a uSD inserted, the only mmc block device is the eMMC which is
/dev/mmcblk0, and the root partition is still /dev/mmcblk0p2.

Note though that this particular bootscript can load add'l bootscripts
from the boot partition; in this particular case, the eMMC root
partition is set as a fixed UUID in the bootscript from the
boot partition.

Regards,
Peter Hurley

[toc] | [prev] | [next] | [standalone]


#1372265

FromJisheng Zhang <jszhang@marvell.com>
Date2016-04-06 10:00 +0200
Message-ID<rkQxA-A1-3@gated-at.bofh.it>
In reply to#1371331
Hi Ulf,

On Tue, 5 Apr 2016 10:59:28 +0200 Ulf Hansson wrote:

> On 4 April 2016 at 20:59, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > On Mon, Apr 4, 2016 at 4:29 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:  
> >>
> >> The commit that's likely to cause the regression is:
> >> 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> >> simultaneously").  
> >
> > Peter, mind testing if you can revert that and get the old behavior
> > back? It seems to still revert cleanly, although I didn't check if the
> > revert actually then builds..  
> 
> I have checked, the revert should be a safe option. There is nothing
> added on top that relies on it.
> 
> Moreover, I have no problem dealing with the revert, as it me
> personally that screwed this up.
> 
> >  
> >> This commit further enables asynchronous detection of (e)MMC/SD/SDIO
> >> cards, by converting from an *ordered* work-queue to a *non-ordered*
> >> work-queue for card detection.
> >>
> >> Although, one should know that there have *never* been any guarantees
> >> to get a fixed mmcblk id for a card. I expect that's what has been
> >> assumed here.  
> >
> > So quite frankly, for the whole "no regressions" issue, "documented
> > behavior" simply isn't an issue. It doesn't matter one whit or not if
> > something has been documented: if it has worked and people have
> > depended on it, it's what we in the industry call "reality".
> >
> > And reality trumps documentation. Every time.  
> 
> I totally agree.
> 
> Although, what puzzles me around this particular issue, is how an SoC
> configuration can rely on this fragile behaviour.
> All you have to do to break the assumption of fixed mmcblk ids, is to
> boot with an SD card inserted and then without. Perhaps these SoCs
> just doesn't support this use case!?

This use case is supported by carefully always letting emmc host be probed
before the sd hosts. For example, this can be done by putting the emmc host
DT node before the SD hosts' ;)

Thanks,
Jisheng

[toc] | [prev] | [next] | [standalone]


#1372283 — Re: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-04-06 10:30 +0200
SubjectRe: [bisect] Merge tag 'mmc-v4.6' of git://git.linaro.org/people/ulf.hansson/mmc (was [GIT PULL] MMC for v.4.6)
Message-ID<rkR0C-12k-11@gated-at.bofh.it>
In reply to#1372265
On 6 April 2016 at 09:47, Jisheng Zhang <jszhang@marvell.com> wrote:
> Hi Ulf,
>
> On Tue, 5 Apr 2016 10:59:28 +0200 Ulf Hansson wrote:
>
>> On 4 April 2016 at 20:59, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>> > On Mon, Apr 4, 2016 at 4:29 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> >>
>> >> The commit that's likely to cause the regression is:
>> >> 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
>> >> simultaneously").
>> >
>> > Peter, mind testing if you can revert that and get the old behavior
>> > back? It seems to still revert cleanly, although I didn't check if the
>> > revert actually then builds..
>>
>> I have checked, the revert should be a safe option. There is nothing
>> added on top that relies on it.
>>
>> Moreover, I have no problem dealing with the revert, as it me
>> personally that screwed this up.
>>
>> >
>> >> This commit further enables asynchronous detection of (e)MMC/SD/SDIO
>> >> cards, by converting from an *ordered* work-queue to a *non-ordered*
>> >> work-queue for card detection.
>> >>
>> >> Although, one should know that there have *never* been any guarantees
>> >> to get a fixed mmcblk id for a card. I expect that's what has been
>> >> assumed here.
>> >
>> > So quite frankly, for the whole "no regressions" issue, "documented
>> > behavior" simply isn't an issue. It doesn't matter one whit or not if
>> > something has been documented: if it has worked and people have
>> > depended on it, it's what we in the industry call "reality".
>> >
>> > And reality trumps documentation. Every time.
>>
>> I totally agree.
>>
>> Although, what puzzles me around this particular issue, is how an SoC
>> configuration can rely on this fragile behaviour.
>> All you have to do to break the assumption of fixed mmcblk ids, is to
>> boot with an SD card inserted and then without. Perhaps these SoCs
>> just doesn't support this use case!?
>
> This use case is supported by carefully always letting emmc host be probed
> before the sd hosts. For example, this can be done by putting the emmc host
> DT node before the SD hosts' ;)

This is just a workaround and it's still *really* fragile.

The workaround you describe, relies on a certain behaviour of the DTS
parsing and the driver core, as you need the the first device in the
DTS to probe first. You are also relying on that the mmc driver
doesn't mess up the probe order by returning -EPROBE_DEFER for the
eMMC slot (because some resources wasn't ready yet).

Anyway, I get the point and thanks for you feedback!

>
> Thanks,
> Jisheng

Kind regards
Uffe

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web