Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Linus Torvalds]

On Fri, Nov 27, 2020 at 12:51 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
>  - Some DT patches for the Rockchip RK3399 platform,
>    in particular fixing the MMC device ordering that
>    recently became nondeterministic with async probe.

Uhhuh.

I didn't realize this MMC breakage happened.

That's just an MMC bug. Other subsystems have been able to do async
probing without making device ordering be random.

So this really smells wrong, and I just never realized.

Added Douglas Anderson to the cc - the whole PROBE_PREFER_ASYNCHRONOUS
behavior appears broken.

You basically should do the device numbering synchronously (or better
yet - asynchronously, but single-threaded for the subsystem), and then
asynchronously probe the actual device details after you've numbered
them reliably.

This is not something new - we do this for pretty much all the other
block devices, and MMC is just doing things wrong.

See SCSI probing for the traditional horrible cases (where just
spinning up a disk could take tens of seconds).  "Slow probing" is not
an excuse of "random ordering".

Behaving randomly is simply not acceptable.

             Linus

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Doug Anderson]

Hi,

On Fri, Nov 27, 2020 at 2:56 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Nov 27, 2020 at 12:51 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> >  - Some DT patches for the Rockchip RK3399 platform,
> >    in particular fixing the MMC device ordering that
> >    recently became nondeterministic with async probe.
>
> Uhhuh.
>
> I didn't realize this MMC breakage happened.
>
> That's just an MMC bug. Other subsystems have been able to do async
> probing without making device ordering be random.
>
> So this really smells wrong, and I just never realized.
>
> Added Douglas Anderson to the cc - the whole PROBE_PREFER_ASYNCHRONOUS
> behavior appears broken.
>
> You basically should do the device numbering synchronously (or better
> yet - asynchronously, but single-threaded for the subsystem), and then
> asynchronously probe the actual device details after you've numbered
> them reliably.

+Ulf too

IMO if a sane/consistent ordering is needed for MMC devices that the
correct solution is that they should be assigned statically and not
rely on the happenstance of which drivers happened to probe in the
system in which order.  Assigning numbers statically is exactly what
we can do now in the device tree and I believe that the patches you
were merging from Arnd were doing exactly that.  To me it feels like
this is the correct long term solution here.

Without static assignment, maybe we could do numbering of MMC devices
in some type of a pre-probe routine?  Is that what you're suggesting?
If so, it seems like it might be a pretty big change to figure out how
that ought to work (if it can be made to work).

If numbering is done by a probe routine then I think we can't
guarantee order.  Among other things, I think EPROBE_DEFER could cause
us problems, like in this example:

1. A regulator driver starts probing, but runs async to allow other
drivers to load.

2. The MMC driver's probe routine runs.

3. In the MMC driver's probe routine, we try to get the regulator.
Depending on how far the regulator driver has run (which may depend on
external factors), we might get it or we might be told EPROBE_DEFER.
Let's assume EPROBE_DEFER.

4. A second MMC driver's probe routine runs.

5. The regulator finishes loading.

6. The first MMC driver's probe routine runs again and works this time.

In this case the first MMC driver's probe routine ran _after_ the
second MMC driver's probe routine even without the MMC drivers being
async.

I believe there are other examples.

Now, of course, it should be noted that on a given board with a given
kernel configuration it might be impossible to ever hit a case where
the order was inconsistent.  However, I believe that there are
legitimate cases where the order would have been inconsistent.  Also
note that code changes to other parts of the system (like adding async
to the regulator subsystem) could suddenly introduce different
ordering even if it was previously impossible.


> This is not something new - we do this for pretty much all the other
> block devices, and MMC is just doing things wrong.

Just to confirm, I assume you are specifically excluding all block
devices attached via USB, right?  I believe that, by its very nature,
it's nearly impossible to guarantee the ordering of USB device
enumeration especially with complex USB hierarchies, but I certainly
could be wrong.  Until devices are enumerated, we can't necessarily
know how many block devices will be provided.


> See SCSI probing for the traditional horrible cases (where just
> spinning up a disk could take tens of seconds).  "Slow probing" is not
> an excuse of "random ordering".

It's odd that you say that.  I actually fought for static numbering
years ago and one of the arguments _against_ allowing static numbering
for MMC devices [1] was that, to quote:

> Exactly the same issue arises if you have more than one ATA or SCSI
> adapter in your PC - things can be probed out of order and end up
> in different /dev/sd* slots.

That was over 4 years ago now, so perhaps things have changed in the
meantime, or perhaps the person who said that was wrong.

Ah, I just re-read more of the old thread and you're probably talking
about what Peter refers to [2].  I guess, though, that must rely on
all parent devices (like PCIe) doing their probes in a fast enough and
synchronous way, right?  ...and I guess it relies on parent devices
not running into problems like the EPROBE_DEFER problem that I talked
about above?  Specifically, someone could have a MMC or SCSI adapter
in a PCIe slot, right?  Until we probe the PCIe slot we don't know if
it provides any block devices.


> Behaving randomly is simply not acceptable.

Agreed, which is why I'm a fan of the static assignment solution.


In the end, I believe that we could revert my patches and go back to
numbering being consistent for most (but not all) use cases.  Even
before my patches, MMC already did most of its slow stuff async and my
patches only save ~40ms of boot time per MMC device, so the solution
you talked about for SCSI was already there, but I wanted to save the
extra 40 ms.  If we revert, though, I'm not sure how we will ever get
to the goal of "async probe by default" then, but maybe that was a
pipe dream?  ...presumably we'd also agree that we'll be limiting how
much async work can happen at boot in general since anything in the
path that might lead to a block device needs to be synchronous /
ordered, right?


In any cases, I'm interested to hear thoughts from you and others on the above.


[1] https://lore.kernel.org/r/20160429181248.GW19428@n2100.arm.linux.org.uk/
[2] https://lore.kernel.org/r/5723FCE3.9070308@hurleysoftware.com/


-Doug

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Linus Torvalds]

On Mon, Nov 30, 2020 at 9:04 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Without static assignment, maybe we could do numbering of MMC devices
> in some type of a pre-probe routine?  Is that what you're suggesting?

Yes.

So basically, the way the async probing works for say SCSI is that we
have multiple "layers of asynchroniety". We have the usual "init calls
done asynchronously", but then within the init calls themselves you
can start sub-scans asynchronously.

In order to get reliable ordering between multiple controllers, the
PCI bus is probed in order in pci_init() (or whatever), so each SCSI
controller gets called in a fixed order.

That then gets to scsi_scan_host() does that async_schedule() thing to
actually scan the SCSI buses on that host.

> If so, it seems like it might be a pretty big change to figure out how
> that ought to work (if it can be made to work).

Try to start out doing discovery synchronously, but then any extra
work (actual IO like partition scanning etc) with an "async_work()".

> Just to confirm, I assume you are specifically excluding all block
> devices attached via USB, right?

I have this memory that we tried to scan the root bus in order, just
because people will have internal USB devices that way, and it makes
it *much* easier if they show up in predetermined places.

Note that it really is only the internal devices that matter. Once you
start plugging in an external USB hug and random devices, ordering
clearly won't be reliable.

So this is not a "everything must be ordered". But people who think
that that then means "everythinbg can be random" are wrong. It's
really really inconvenient, because things like disk labels and
various device IDs are not reliable either, and can be very confusing
when two otherwise identical machines look different before you've
installed a full system on them, for example.

So you should strive very hard to make everything that is directly
connected to one SoC always come up in the same order, for example.

And storage devices are special. As Dmitry points out, it doesn't much
matter in what order something like a mouse is scanned. Who cares? If
you have multiple pointer devices, you'll either use them
interchangably, or you'll have some other way to identify them. But
storage is a _lot_ more basic than basically stateless input devices
are.

And no, disk labels really don't always exist. Never ever say "but but
but disk labels".

               Linus

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Russell King - ARM Linux admin]

On Mon, Nov 30, 2020 at 09:44:03AM -0800, Linus Torvalds wrote:
> On Mon, Nov 30, 2020 at 9:04 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Without static assignment, maybe we could do numbering of MMC devices
> > in some type of a pre-probe routine?  Is that what you're suggesting?
> 
> Yes.
> 
> So basically, the way the async probing works for say SCSI is that we
> have multiple "layers of asynchroniety". We have the usual "init calls
> done asynchronously", but then within the init calls themselves you
> can start sub-scans asynchronously.
> 
> In order to get reliable ordering between multiple controllers, the
> PCI bus is probed in order in pci_init() (or whatever), so each SCSI
> controller gets called in a fixed order.
> 
> That then gets to scsi_scan_host() does that async_schedule() thing to
> actually scan the SCSI buses on that host.

I'm afraid that you don't get stable device numbering on x86. You get
something that _looks_ like stable device numbering, but it really
isn't.

If you think that /dev/sda for example is always the machine's internal
HDD, that is wrong.

I have a HP Pavilion laptop with its internal HDD with a Windows
installation. Because I didn't want to destroy that in any way, I
bought an external USB3 SATA enclosure and SSD, and installed Debian
Stable on there.

When I installed Debian stable, the HDD was /dev/sda and the SSD was
/dev/sdb. When I boot Debian, the SSD is /dev/sda and the internal
HDD is /dev/sdb.

Maybe /dev/sda through /dev/sdd should be reserved for internal
motherboard drives?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Linus Torvalds]

On Mon, Nov 30, 2020 at 10:05 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> If you think that /dev/sda for example is always the machine's internal
> HDD, that is wrong.

Yes. See the whole part about

 "Note that it really is only the internal devices that matter. Once you
  start plugging in an external USB hug and random devices, ordering
  clearly won't be reliable.

  So this is not a "everything must be ordered". But people who think
  that that then means "everythinbg can be random" are wrong"

in my email.

And the key word here is:

> I have a HP Pavilion laptop with its internal HDD with a Windows
> installation. Because I didn't want to destroy that in any way, I
> bought an external USB3 SATA enclosure and SSD, and installed Debian
> Stable on there.

.. but it will still generally be stable with the same hardware
configuration, and not fluctuate randomly from boot to boot when the
hardware is the same.

Is it a guarantee? No. External plugged in hardware can change things.
In fact, when you have things like Thunderbolt involved, since it just
looks like PCI, even the PCI probing order won't be the same with or
without the plugged-in device.

But again: avoid randomness. The difference between non-random and
random is that one is predictable and one is not.

Predictability is good. It's not necessariyl always achievable, but
it's very much something we should strive for VERY HARD.

Predictability and reproducibility help debugging enormously. It means
that things like "git bisect" work a lot better. It makes user reports
much more understandable when two users with identical hardware see
identical issues.

Seriously. Anybody who argues against reproducibility is so far out to
lunch that it's not even funny.

You seem to argue on a complete technicality.

            Linus

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Russell King - ARM Linux admin]

On Mon, Nov 30, 2020 at 10:15:29AM -0800, Linus Torvalds wrote:
> On Mon, Nov 30, 2020 at 10:05 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > If you think that /dev/sda for example is always the machine's internal
> > HDD, that is wrong.
> 
> Yes. See the whole part about
> 
>  "Note that it really is only the internal devices that matter. Once you
>   start plugging in an external USB hug and random devices, ordering
>   clearly won't be reliable.
> 
>   So this is not a "everything must be ordered". But people who think
>   that that then means "everythinbg can be random" are wrong"
> 
> in my email.
> 
> And the key word here is:
> 
> > I have a HP Pavilion laptop with its internal HDD with a Windows
> > installation. Because I didn't want to destroy that in any way, I
> > bought an external USB3 SATA enclosure and SSD, and installed Debian
> > Stable on there.
> 
> .. but it will still generally be stable with the same hardware
> configuration, and not fluctuate randomly from boot to boot when the
> hardware is the same.
> 
> Is it a guarantee? No. External plugged in hardware can change things.
> In fact, when you have things like Thunderbolt involved, since it just
> looks like PCI, even the PCI probing order won't be the same with or
> without the plugged-in device.
> 
> But again: avoid randomness. The difference between non-random and
> random is that one is predictable and one is not.
> 
> Predictability is good. It's not necessariyl always achievable, but
> it's very much something we should strive for VERY HARD.
> 
> Predictability and reproducibility help debugging enormously. It means
> that things like "git bisect" work a lot better. It makes user reports
> much more understandable when two users with identical hardware see
> identical issues.
> 
> Seriously. Anybody who argues against reproducibility is so far out to
> lunch that it's not even funny.
> 
> You seem to argue on a complete technicality.

Yes, I fully agree - but the problem is that how people read your
"avoid randomness".

Randomness as in "doesn't change if I change kernel configuration"
or "doesn't change order on reboot to a similar kernel".

People who have complained about mmc in the past have done so from
the point of view of "I changed something, now my mmc block devices
are different".

It is my understanding that mmc behaves in the same way as exactly
what I see on my HP laptop. The device numbering is stable provided
the hardware (and indeed software) configuration is stable.

If I decide to rebuild the Debian kernel for my HP laptop, and build
in the drivers necessary for all its hardware, the internal HDD will
switch to being /dev/sda instead of /dev/sdb as it is now... which
doesn't seem to be any different from complaints like "I've changed
some code in the kernel now my MMC devices have changed order."

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Doug Anderson]

Hi,

On Mon, Nov 30, 2020 at 9:44 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So basically, the way the async probing works for say SCSI is that we
> have multiple "layers of asynchroniety". We have the usual "init calls
> done asynchronously", but then within the init calls themselves you
> can start sub-scans asynchronously.
>
> In order to get reliable ordering between multiple controllers, the
> PCI bus is probed in order in pci_init() (or whatever), so each SCSI
> controller gets called in a fixed order.
>
> That then gets to scsi_scan_host() does that async_schedule() thing to
> actually scan the SCSI buses on that host.
>
> > If so, it seems like it might be a pretty big change to figure out how
> > that ought to work (if it can be made to work).
>
> Try to start out doing discovery synchronously, but then any extra
> work (actual IO like partition scanning etc) with an "async_work()".

Right, so basically this is like how MMC did it before my patches.  I
was just trying to save the extra 40 ms.


> > Just to confirm, I assume you are specifically excluding all block
> > devices attached via USB, right?
>
> I have this memory that we tried to scan the root bus in order, just
> because people will have internal USB devices that way, and it makes
> it *much* easier if they show up in predetermined places.
>
> Note that it really is only the internal devices that matter. Once you
> start plugging in an external USB hug and random devices, ordering
> clearly won't be reliable.

I guess that's more of a heuristic though, right?  There may be
computers with an internal USB hub before the storage.  There may also
be computers (small embedded raspberry-pi-type-things) where the
primary storage for the system is expected to be an external USB
device, possibly connected through a hub.


> So this is not a "everything must be ordered". But people who think
> that that then means "everythinbg can be random" are wrong. It's
> really really inconvenient, because things like disk labels and
> various device IDs are not reliable either, and can be very confusing
> when two otherwise identical machines look different before you've
> installed a full system on them, for example.
>
> So you should strive very hard to make everything that is directly
> connected to one SoC always come up in the same order, for example.

So I guess the answer here is: strive very hard but you don't have to
guarantee that every corner case is covered?


> And storage devices are special. As Dmitry points out, it doesn't much
> matter in what order something like a mouse is scanned. Who cares? If
> you have multiple pointer devices, you'll either use them
> interchangably, or you'll have some other way to identify them. But
> storage is a _lot_ more basic than basically stateless input devices
> are.

Yes, storage devices are special, but the point I was trying to make
is that with SoCs there may be a whole lot of devices (with complex
dependencies) in the path of storage devices.  They can need clocks,
regulators, GPIOs, etc and any of those things could be slow to probe
and we might want to speed up boot by making them async.  They also
could be parented on busses that could be slow to probe or might
themselves have dependencies on clocks, regulators, GPIOs, etc.  It
won't always be obvious which of these things are in the path of a
block device.

In a traditional PC I think there are fewer dependencies?


I guess the question is: why is static assignment of numbers not an
acceptable solution to the problem?  It gives us the desired fixed
numbers and automatically avoids all weird probe ordering / dependency
problems.

-Doug

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Linus Torvalds]

On Mon, Nov 30, 2020 at 10:11 AM Doug Anderson <dianders@chromium.org> wrote:
>
> So I guess the answer here is: strive very hard but you don't have to
> guarantee that every corner case is covered?

Yes. Covering every possible theoretical case is basically impossible.
I mean, it can be something like "the firmware glitched for whatever
reason, and didn't set up a device, and it didn't show up at boot at
all until you did something explicit".

(Example: airplane mode wireless switches, but also possibly things
like just slightly unreliable USB hubs etc - I bet we've all seen
those).

So the rule should be that you strive very hard to make boots have
reproducible behavior as far as practically necessary, and avoid
obvious timing-induced ordering issues.

> In a traditional PC I think there are fewer dependencies?

I'd say that they were "different", not necessarily "fewer".

> I guess the question is: why is static assignment of numbers not an
> acceptable solution to the problem?  It gives us the desired fixed
> numbers and automatically avoids all weird probe ordering / dependency
> problems.

I think that if this had been done originally, it would probably be fine.

But I still have this - possibly unreasonable - expectation that the
promise of DT was that it wouldn't be 1:1 tied to the kernel all the
time. That was always the promise, after all.

So the whole "add DT markers because the subsystem now screws up
ordering" smells really bad to me.

             Linus

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Russell King - ARM Linux admin]

On Mon, Nov 30, 2020 at 10:22:58AM -0800, Linus Torvalds wrote:
> > I guess the question is: why is static assignment of numbers not an
> > acceptable solution to the problem?  It gives us the desired fixed
> > numbers and automatically avoids all weird probe ordering / dependency
> > problems.
> 
> I think that if this had been done originally, it would probably be fine.

It was not done originally, because the original structure of the MMC
bus was:

host controller ---+--- card 1
                   +--- card 2
		   +--- card 3
		   ...

So one host controller could be connected to multiple different cards,
and the bus has a way to detect each card individually. This means we
had no idea how many cards would be connected to any one controller,
and it was entirely sensible to allocate MMC block devices in the order
we discovered the cards.

The SD specification, this became limited to just one card per
controller to allow for faster speeds.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Ulf Hansson]

On Mon, 30 Nov 2020 at 19:23, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Nov 30, 2020 at 10:11 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > So I guess the answer here is: strive very hard but you don't have to
> > guarantee that every corner case is covered?
>
> Yes. Covering every possible theoretical case is basically impossible.
> I mean, it can be something like "the firmware glitched for whatever
> reason, and didn't set up a device, and it didn't show up at boot at
> all until you did something explicit".
>
> (Example: airplane mode wireless switches, but also possibly things
> like just slightly unreliable USB hubs etc - I bet we've all seen
> those).
>
> So the rule should be that you strive very hard to make boots have
> reproducible behavior as far as practically necessary, and avoid
> obvious timing-induced ordering issues.

I agree, but let me also try to clarify a few things.

Indeed, we have been striving towards getting a more consistent
behavior when assigning block numbers for MMC/SD cards. Some time ago
the number depended on:

- The time it took to initialize the MMC/SD cards.
- The probing of the mmc block device.
- The probing of the mmc host drivers.

It's been highly random, unfortunately.

Therefore, we have and are still pointing to things like "disk
labels", but those have limitations.

A while ago, we eliminated the impact of the two first parts, which
left us with solely the probe order of mmc host drivers. Furthermore,
we recently introduced support for the mmc aliases in DT, in a way to
support cases when a "disk label" is not feasible.

>
> > In a traditional PC I think there are fewer dependencies?
>
> I'd say that they were "different", not necessarily "fewer".
>
> > I guess the question is: why is static assignment of numbers not an
> > acceptable solution to the problem?  It gives us the desired fixed
> > numbers and automatically avoids all weird probe ordering / dependency
> > problems.
>
> I think that if this had been done originally, it would probably be fine.
>
> But I still have this - possibly unreasonable - expectation that the
> promise of DT was that it wouldn't be 1:1 tied to the kernel all the
> time. That was always the promise, after all.
>
> So the whole "add DT markers because the subsystem now screws up
> ordering" smells really bad to me.

As stated above, the randomness has been there all the time. We have
had only "disk labels" to help and that's what we have been telling
people consistently. To me, the new mmc aliases in DT, is another step
in the right direction.

That said, perhaps we went too far to optimize boot times while
enabling async probe for some of the mmc host drivers. Clearly, not
all users have been paying attention, but was lucky to receive
"stable" mmc block numbers.

So, I think we have two options. If people are willing to move to
"disk labels" or to patch their DTBs with mmc aliases, things can stay
as is. Otherwise, we can revert the async probe parts of the mmc host
drivers, but that would still leave us in a fragile situation.

Kind regards
Uffe

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Geert Uytterhoeven]

CC devicetree

On Tue, Dec 1, 2020 at 3:06 PM Arnd Bergmann <arnd@kernel.org> wrote:
> On Tue, Dec 1, 2020 at 12:39 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > So, I think we have two options. If people are willing to move to
> > "disk labels" or to patch their DTBs with mmc aliases, things can stay
> > as is. Otherwise, we can revert the async probe parts of the mmc host
> > drivers, but that would still leave us in a fragile situation.
>
> Can you reliably detect whether the mmc aliases in the dt exist?
> If that's possible, maybe the async flag could be masked out to only have
> an effect when the device number is known.

IMHO DT aliases are not a proper solution for this.

Yes, you can detect reliably if an alias exists in the DT.
The problems start when having multiple devices, some with aliases,
some without.  And when devices can appear dynamically (without
aliases, as there is no support for dynamically updating the aliases
list).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Doug Anderson]

Hi,

On Mon, Dec 7, 2020 at 1:55 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Mon, Dec 7, 2020 at 9:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Dec 1, 2020 at 3:06 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > > On Tue, Dec 1, 2020 at 12:39 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > So, I think we have two options. If people are willing to move to
> > > > "disk labels" or to patch their DTBs with mmc aliases, things can stay
> > > > as is. Otherwise, we can revert the async probe parts of the mmc host
> > > > drivers, but that would still leave us in a fragile situation.
> > >
> > > Can you reliably detect whether the mmc aliases in the dt exist?
> > > If that's possible, maybe the async flag could be masked out to only have
> > > an effect when the device number is known.
> >
> > IMHO DT aliases are not a proper solution for this.
> >
> > Yes, you can detect reliably if an alias exists in the DT.
> > The problems start when having multiple devices, some with aliases,
> > some without.  And when devices can appear dynamically (without
> > aliases, as there is no support for dynamically updating the aliases
> > list).
>
> Actually you hit a problem earlier than that: the async probe is a
> property of the host controller driver, which may be a pci_driver,
> platform_driver, usb_driver, or anything else really. To figure out
> whether to probe it asynchronously, it would have to be the driver
> core, or each bus type that can host these to understand which
> device driver is responsible for probing an eMMC device attached
> to the host.

From what I've seen so far, my current thought on this issue is that
it's up to Ulf as the MMC maintainer what the next steps are.  For me,
at least, his argument that MMC block numbers have already shuffled
around several times in the last several years is at least some
evidence that they weren't exactly stable to begin with.  While we
could go back to the numbers that happened to be chosen as of kernel
v5.9, if someone was updating from a much older kernel then they may
have different expectations of what numbers are good / bad I think.

I will also offer one possible suggestion: what about a KConfig option
here?  In theory we could add a KConfig option like
"CONFIG_MMC_LEGACY_PROBE" or something that.  One can argue about what
the default ought to be, but maybe that would satisfy folks?  If you
were happy giving up a little bit of boot speed to get the v5.9 block
numbers then you could set this.


-Doug

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Geert Uytterhoeven]

Hi Doug,

On Mon, Dec 7, 2020 at 11:15 PM Doug Anderson <dianders@chromium.org> wrote:
> On Mon, Dec 7, 2020 at 1:55 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > On Mon, Dec 7, 2020 at 9:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, Dec 1, 2020 at 3:06 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > > > On Tue, Dec 1, 2020 at 12:39 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > > So, I think we have two options. If people are willing to move to
> > > > > "disk labels" or to patch their DTBs with mmc aliases, things can stay
> > > > > as is. Otherwise, we can revert the async probe parts of the mmc host
> > > > > drivers, but that would still leave us in a fragile situation.
> > > >
> > > > Can you reliably detect whether the mmc aliases in the dt exist?
> > > > If that's possible, maybe the async flag could be masked out to only have
> > > > an effect when the device number is known.
> > >
> > > IMHO DT aliases are not a proper solution for this.
> > >
> > > Yes, you can detect reliably if an alias exists in the DT.
> > > The problems start when having multiple devices, some with aliases,
> > > some without.  And when devices can appear dynamically (without
> > > aliases, as there is no support for dynamically updating the aliases
> > > list).
> >
> > Actually you hit a problem earlier than that: the async probe is a
> > property of the host controller driver, which may be a pci_driver,
> > platform_driver, usb_driver, or anything else really. To figure out
> > whether to probe it asynchronously, it would have to be the driver
> > core, or each bus type that can host these to understand which
> > device driver is responsible for probing an eMMC device attached
> > to the host.
>
> From what I've seen so far, my current thought on this issue is that
> it's up to Ulf as the MMC maintainer what the next steps are.  For me,
> at least, his argument that MMC block numbers have already shuffled
> around several times in the last several years is at least some
> evidence that they weren't exactly stable to begin with.  While we
> could go back to the numbers that happened to be chosen as of kernel
> v5.9, if someone was updating from a much older kernel then they may
> have different expectations of what numbers are good / bad I think.
>
> I will also offer one possible suggestion: what about a KConfig option
> here?  In theory we could add a KConfig option like
> "CONFIG_MMC_LEGACY_PROBE" or something that.  One can argue about what
> the default ought to be, but maybe that would satisfy folks?  If you
> were happy giving up a little bit of boot speed to get the v5.9 block
> numbers then you could set this.

This is not limited to MMC.
The same is true for sdX, ethX (e* these days), ttyS*, i2cX, spiX, ...
The rule has always been to handle it by udev, disklabels, ...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Ulf Hansson]

On Tue, 8 Dec 2020 at 08:32, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Doug,
>
> On Mon, Dec 7, 2020 at 11:15 PM Doug Anderson <dianders@chromium.org> wrote:
> > On Mon, Dec 7, 2020 at 1:55 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > > On Mon, Dec 7, 2020 at 9:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Tue, Dec 1, 2020 at 3:06 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > > > > On Tue, Dec 1, 2020 at 12:39 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > > > So, I think we have two options. If people are willing to move to
> > > > > > "disk labels" or to patch their DTBs with mmc aliases, things can stay
> > > > > > as is. Otherwise, we can revert the async probe parts of the mmc host
> > > > > > drivers, but that would still leave us in a fragile situation.
> > > > >
> > > > > Can you reliably detect whether the mmc aliases in the dt exist?
> > > > > If that's possible, maybe the async flag could be masked out to only have
> > > > > an effect when the device number is known.
> > > >
> > > > IMHO DT aliases are not a proper solution for this.
> > > >
> > > > Yes, you can detect reliably if an alias exists in the DT.
> > > > The problems start when having multiple devices, some with aliases,
> > > > some without.  And when devices can appear dynamically (without
> > > > aliases, as there is no support for dynamically updating the aliases
> > > > list).
> > >
> > > Actually you hit a problem earlier than that: the async probe is a
> > > property of the host controller driver, which may be a pci_driver,
> > > platform_driver, usb_driver, or anything else really. To figure out
> > > whether to probe it asynchronously, it would have to be the driver
> > > core, or each bus type that can host these to understand which
> > > device driver is responsible for probing an eMMC device attached
> > > to the host.
> >
> > From what I've seen so far, my current thought on this issue is that
> > it's up to Ulf as the MMC maintainer what the next steps are.  For me,
> > at least, his argument that MMC block numbers have already shuffled
> > around several times in the last several years is at least some
> > evidence that they weren't exactly stable to begin with.  While we
> > could go back to the numbers that happened to be chosen as of kernel
> > v5.9, if someone was updating from a much older kernel then they may
> > have different expectations of what numbers are good / bad I think.
> >
> > I will also offer one possible suggestion: what about a KConfig option
> > here?  In theory we could add a KConfig option like
> > "CONFIG_MMC_LEGACY_PROBE" or something that.  One can argue about what
> > the default ought to be, but maybe that would satisfy folks?  If you
> > were happy giving up a little bit of boot speed to get the v5.9 block
> > numbers then you could set this.
>
> This is not limited to MMC.
> The same is true for sdX, ethX (e* these days), ttyS*, i2cX, spiX, ...
> The rule has always been to handle it by udev, disklabels, ...

That's right.

Although, those rules haven't been sufficient for some cases, which
has been discussed many many times by now. I can dig out some of the
old threads from the mail archive, just tell me and will help to find
them.

For mmc we have decided to improve the situation by adding support for
aliases in DT. The support seems robust enough to me, but if you think
there are problems with it, please tell me and I am happy to help to
improve it.

In regards to adding a new Kconfig option for "legacy probe", I am
open to this if that's what people think is needed. Although, as
pointed out earlier in this thread, it won't move us into a stable
situation. The only solution to get to that point, is either to use
udev/disklabel rules or the mmc aliases in DT.

Kind regards
Uffe

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Geert Uytterhoeven]

Hi Ulf,

On Mon, Dec 14, 2020 at 9:23 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On Tue, 8 Dec 2020 at 08:32, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, Dec 7, 2020 at 11:15 PM Doug Anderson <dianders@chromium.org> wrote:
> > > On Mon, Dec 7, 2020 at 1:55 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > > > On Mon, Dec 7, 2020 at 9:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Tue, Dec 1, 2020 at 3:06 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > > > > > On Tue, Dec 1, 2020 at 12:39 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > > > > So, I think we have two options. If people are willing to move to
> > > > > > > "disk labels" or to patch their DTBs with mmc aliases, things can stay
> > > > > > > as is. Otherwise, we can revert the async probe parts of the mmc host
> > > > > > > drivers, but that would still leave us in a fragile situation.
> > > > > >
> > > > > > Can you reliably detect whether the mmc aliases in the dt exist?
> > > > > > If that's possible, maybe the async flag could be masked out to only have
> > > > > > an effect when the device number is known.
> > > > >
> > > > > IMHO DT aliases are not a proper solution for this.
> > > > >
> > > > > Yes, you can detect reliably if an alias exists in the DT.
> > > > > The problems start when having multiple devices, some with aliases,
> > > > > some without.  And when devices can appear dynamically (without
> > > > > aliases, as there is no support for dynamically updating the aliases
> > > > > list).
> > > >
> > > > Actually you hit a problem earlier than that: the async probe is a
> > > > property of the host controller driver, which may be a pci_driver,
> > > > platform_driver, usb_driver, or anything else really. To figure out
> > > > whether to probe it asynchronously, it would have to be the driver
> > > > core, or each bus type that can host these to understand which
> > > > device driver is responsible for probing an eMMC device attached
> > > > to the host.
> > >
> > > From what I've seen so far, my current thought on this issue is that
> > > it's up to Ulf as the MMC maintainer what the next steps are.  For me,
> > > at least, his argument that MMC block numbers have already shuffled
> > > around several times in the last several years is at least some
> > > evidence that they weren't exactly stable to begin with.  While we
> > > could go back to the numbers that happened to be chosen as of kernel
> > > v5.9, if someone was updating from a much older kernel then they may
> > > have different expectations of what numbers are good / bad I think.
> > >
> > > I will also offer one possible suggestion: what about a KConfig option
> > > here?  In theory we could add a KConfig option like
> > > "CONFIG_MMC_LEGACY_PROBE" or something that.  One can argue about what
> > > the default ought to be, but maybe that would satisfy folks?  If you
> > > were happy giving up a little bit of boot speed to get the v5.9 block
> > > numbers then you could set this.
> >
> > This is not limited to MMC.
> > The same is true for sdX, ethX (e* these days), ttyS*, i2cX, spiX, ...
> > The rule has always been to handle it by udev, disklabels, ...
>
> That's right.
>
> Although, those rules haven't been sufficient for some cases, which
> has been discussed many many times by now. I can dig out some of the
> old threads from the mail archive, just tell me and will help to find
> them.
>
> For mmc we have decided to improve the situation by adding support for
> aliases in DT. The support seems robust enough to me, but if you think
> there are problems with it, please tell me and I am happy to help to
> improve it.

DT rule #1: DT is hardware description (device naming is software
policy).

The only generally accepted aliases are "serial0" (the first serial
port, as such labeled on the board, to be used for the console;  there
may be more labeled ports, and thus more "serialX" aliases), and
"ethernet0" (the first Ethernet port, so U-Boot knows to which port to
add an appropriate "local-mac-address" property, when booting over the
network).  So yeah, you can claim the first SD card slot is labeled as
such, and thus deserves an alias.  Then the issue is what you do with
the remaining slots, which can be added either statically or
dynamically.  And what if for some reason the labeled MMC slot is not
probed first...

The description of commit 7678f4c20fa7670f ("serial: sh-sci: Add support
for dynamic instances") mentions some background and remaining
issues w.r.t. aliases.

> In regards to adding a new Kconfig option for "legacy probe", I am
> open to this if that's what people think is needed. Although, as
> pointed out earlier in this thread, it won't move us into a stable
> situation. The only solution to get to that point, is either to use
> udev/disklabel rules or the mmc aliases in DT.

IMHO that will be a fragile solution, too: over time, it may become
harder and harder to maintain the original probe order.

And if it will be an option, it means there will be two code paths to
maintain, increasing the burden.



Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Ulf Hansson]

On Tue, 15 Dec 2020 at 09:19, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ulf,
>
> On Mon, Dec 14, 2020 at 9:23 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > On Tue, 8 Dec 2020 at 08:32, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Mon, Dec 7, 2020 at 11:15 PM Doug Anderson <dianders@chromium.org> wrote:
> > > > On Mon, Dec 7, 2020 at 1:55 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > > > > On Mon, Dec 7, 2020 at 9:23 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > On Tue, Dec 1, 2020 at 3:06 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > > > > > > On Tue, Dec 1, 2020 at 12:39 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > > > > > So, I think we have two options. If people are willing to move to
> > > > > > > > "disk labels" or to patch their DTBs with mmc aliases, things can stay
> > > > > > > > as is. Otherwise, we can revert the async probe parts of the mmc host
> > > > > > > > drivers, but that would still leave us in a fragile situation.
> > > > > > >
> > > > > > > Can you reliably detect whether the mmc aliases in the dt exist?
> > > > > > > If that's possible, maybe the async flag could be masked out to only have
> > > > > > > an effect when the device number is known.
> > > > > >
> > > > > > IMHO DT aliases are not a proper solution for this.
> > > > > >
> > > > > > Yes, you can detect reliably if an alias exists in the DT.
> > > > > > The problems start when having multiple devices, some with aliases,
> > > > > > some without.  And when devices can appear dynamically (without
> > > > > > aliases, as there is no support for dynamically updating the aliases
> > > > > > list).
> > > > >
> > > > > Actually you hit a problem earlier than that: the async probe is a
> > > > > property of the host controller driver, which may be a pci_driver,
> > > > > platform_driver, usb_driver, or anything else really. To figure out
> > > > > whether to probe it asynchronously, it would have to be the driver
> > > > > core, or each bus type that can host these to understand which
> > > > > device driver is responsible for probing an eMMC device attached
> > > > > to the host.
> > > >
> > > > From what I've seen so far, my current thought on this issue is that
> > > > it's up to Ulf as the MMC maintainer what the next steps are.  For me,
> > > > at least, his argument that MMC block numbers have already shuffled
> > > > around several times in the last several years is at least some
> > > > evidence that they weren't exactly stable to begin with.  While we
> > > > could go back to the numbers that happened to be chosen as of kernel
> > > > v5.9, if someone was updating from a much older kernel then they may
> > > > have different expectations of what numbers are good / bad I think.
> > > >
> > > > I will also offer one possible suggestion: what about a KConfig option
> > > > here?  In theory we could add a KConfig option like
> > > > "CONFIG_MMC_LEGACY_PROBE" or something that.  One can argue about what
> > > > the default ought to be, but maybe that would satisfy folks?  If you
> > > > were happy giving up a little bit of boot speed to get the v5.9 block
> > > > numbers then you could set this.
> > >
> > > This is not limited to MMC.
> > > The same is true for sdX, ethX (e* these days), ttyS*, i2cX, spiX, ...
> > > The rule has always been to handle it by udev, disklabels, ...
> >
> > That's right.
> >
> > Although, those rules haven't been sufficient for some cases, which
> > has been discussed many many times by now. I can dig out some of the
> > old threads from the mail archive, just tell me and will help to find
> > them.
> >
> > For mmc we have decided to improve the situation by adding support for
> > aliases in DT. The support seems robust enough to me, but if you think
> > there are problems with it, please tell me and I am happy to help to
> > improve it.
>
> DT rule #1: DT is hardware description (device naming is software
> policy).

I do admit, the use of aliases in DT, for *all* cases including mmc is
a bit of a stretch. Anyway, it's there.

>
> The only generally accepted aliases are "serial0" (the first serial
> port, as such labeled on the board, to be used for the console;  there
> may be more labeled ports, and thus more "serialX" aliases), and
> "ethernet0" (the first Ethernet port, so U-Boot knows to which port to
> add an appropriate "local-mac-address" property, when booting over the
> network).  So yeah, you can claim the first SD card slot is labeled as
> such, and thus deserves an alias.  Then the issue is what you do with
> the remaining slots, which can be added either statically or
> dynamically.  And what if for some reason the labeled MMC slot is not
> probed first...

The probe order shouldn't matter. By pre-parsing all mmc aliases (for
all slots) existing in the DTB we can keep track of reserved ids. In
this way, for the non-alias case, we can dynamically select a
non-reserved id.

Although, perhaps more improvements can be made.

>
> The description of commit 7678f4c20fa7670f ("serial: sh-sci: Add support
> for dynamic instances") mentions some background and remaining
> issues w.r.t. aliases.
>
> > In regards to adding a new Kconfig option for "legacy probe", I am
> > open to this if that's what people think is needed. Although, as
> > pointed out earlier in this thread, it won't move us into a stable
> > situation. The only solution to get to that point, is either to use
> > udev/disklabel rules or the mmc aliases in DT.
>
> IMHO that will be a fragile solution, too: over time, it may become
> harder and harder to maintain the original probe order.
>
> And if it will be an option, it means there will be two code paths to
> maintain, increasing the burden.

Alright, sounds like we can conclude that the Kconfig option doesn't
seem like a good way forward.

Kind regards
Uffe

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Dmitry Torokhov]

Hi Linus,

On Fri, Nov 27, 2020 at 3:02 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Nov 27, 2020 at 12:51 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> >  - Some DT patches for the Rockchip RK3399 platform,
> >    in particular fixing the MMC device ordering that
> >    recently became nondeterministic with async probe.
>
> Uhhuh.
>
> I didn't realize this MMC breakage happened.
>
> That's just an MMC bug. Other subsystems have been able to do async
> probing without making device ordering be random.
>
> So this really smells wrong, and I just never realized.
>
> Added Douglas Anderson to the cc - the whole PROBE_PREFER_ASYNCHRONOUS
> behavior appears broken.
>
> You basically should do the device numbering synchronously (or better
> yet - asynchronously, but single-threaded for the subsystem), and then
> asynchronously probe the actual device details after you've numbered
> them reliably.

Generally speaking it is either unnecessary for a lot of devices where
we do not care what number they get, harmful (why do I need to probe
i2c touchscreen and touchpad sequentially if I do not care which one
comes first but my boot will take 0.5 seconds longer if I serialize),
or impossible (if device is hit-pluggable). For many years userspace
has been moving away from static device assignments and we should not
be going back.

There are some devices that are special because it is hard to deal
with devices shifting during early boot, and for them maintaining
static ordering might be beneficial for now, but they are exceptions.

>
> This is not something new - we do this for pretty much all the other
> block devices, and MMC is just doing things wrong.
>
> See SCSI probing for the traditional horrible cases (where just
> spinning up a disk could take tens of seconds).  "Slow probing" is not
> an excuse of "random ordering".

And still my external USB disk will get a "random" /dev/sdX depending
on when I plug it in relative to all other USB sticks.

Thanks.

-- 
Dmitry

Re: [GIT PULL] ARM: SoC fixes for v5.10, part 3

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

Hi!

> > On Fri, Nov 27, 2020 at 12:51 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > >
> > >  - Some DT patches for the Rockchip RK3399 platform,
> > >    in particular fixing the MMC device ordering that
> > >    recently became nondeterministic with async probe.
> >
> > Uhhuh.
> >
> > I didn't realize this MMC breakage happened.
> >
> > That's just an MMC bug. Other subsystems have been able to do async
> > probing without making device ordering be random.
> >
> > So this really smells wrong, and I just never realized.
> >
> > Added Douglas Anderson to the cc - the whole PROBE_PREFER_ASYNCHRONOUS
> > behavior appears broken.
> >
> > You basically should do the device numbering synchronously (or better
> > yet - asynchronously, but single-threaded for the subsystem), and then
> > asynchronously probe the actual device details after you've numbered
> > them reliably.
> 
> Generally speaking it is either unnecessary for a lot of devices where
> we do not care what number they get, harmful (why do I need to probe
> i2c touchscreen and touchpad sequentially if I do not care which one
> comes first but my boot will take 0.5 seconds longer if I serialize),
> or impossible (if device is hit-pluggable). For many years userspace
> has been moving away from static device assignments and we should not
> be going back.

Lets not make userspace's life harder than it should be.

You can't break existing configurations, and people still use
root=/dev/mmcblkX. Recently kernel on Droid 4 switched from MMC on
mmcblk0 and mmcblk1 to MMC on mmcblk0 and mmcblk2. Of course that
broke my config.

And I'm fairly sure I'm not alone.

(And it may not be this bug, and I do have workaround now.)

Best regards,
								Pavel

-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]