On Wed, May 23, 2018 at 03:50:32PM +0200, Ulf Hansson wrote: > On 22 May 2018 at 18:54, Martin Hicks <mort@xxxxxxxx> wrote: > > On Mon, May 21, 2018 at 10:22:11AM +0200, Ulf Hansson wrote: > >> On 18 May 2018 at 17:19, Martin Hicks <mort@xxxxxxxx> wrote: > >> > > >> > I now have second thoughts about this change since the same path is used for > >> > discarding smaller blocks, like if you have "discard" enabled on ext4. > >> > These discard requests can complete very quickly and we wouldn't want to add > >> > latency to these commands. It looks like the "quick" discards can complete > >> > in as little as 50 loops (~1ms), but that would be dependent on the speed of > >> > the MMC bus, as well as how much work the eMMC drive has to do. > >> > >> I fully agree. Several things comes into play, so clearly we needs > >> something a bit more clever than a fixed timeout value. > >> > >> A simplistic approach could be to use a small timeout to start with, > >> then increase it while iterating the loops. > >> > >> Perhaps start by sleeping 128us, then 256, 512, 1024, 2048, 4096. Then > >> we probably want to stop increasing at some point. > > > > > > Hi Uffe, > > > > Good suggestion. I've gone ahead an recreated the patch with a linear > > backoff. I had concerns that with some faster eMMC buses (I'm only using DDR52), > > that doubling the timeout each time may not have enough granularity to eliminate > > unnecessary latency, so I made the backoff linear. The patch follows: > > In regards to latency, this boils done to what happens inside the card. > > An actual erase takes lots of time to complete, so no worries in > regards to latency. Although, a trim/discard operations should > normally be rather cheap, even if it depends on the internals of the > device. For a normal trim/discard I don't think any erasure actually takes place. The card just updates it's internal tables to say that this sector no longer contains valid data. This is card-specific, obviously. In the eMMC that I'm dealing with, discard happens quickly. Secure discard actually waits for the erases to happen, and is therefore quite slow. > > > > > > > This drastically reduces the rate at which the MMC_SEND_STATUS cmd polls > > for completion of the MMC Erase operation. The patch does this by adding > > a linear backoff sleep that starts by sleeping for very short periods > > (32-64us), and ramps up to sleeping for 1-2ms. > > So this means, we are going to iterate the loop for 32 times, before > we hit the maximum sleep time. > > Assuming that an usleep_range(n, 2*n), puts us in the middle of that > range, the above 32 iterations would mean we would be sleeping of > ~25ms in total. Then each following iteration means a sleep time of > ~1.5 ms. > > I am worried that this still means too much of polling-commands to be sent. > > First, I am rather sure we don't need to iterate the loop 32 times > before reaching 1024 us sleep time. The steps can be bigger, possibly > also increased over time. > > Second, capping at 1024 is way too early, especially when the total > loop time ends up being in the range of seconds. Ok, I can make those two changes. > > > > > Even on very quickly completing erase operations, the loop iterates 10+ > > times, so not too much extra latency is added to these commands. > > A comparison between before and after the change would be nice, can > you share those numbers? The absolute fastest responses were in 50 loops, with many taking 70-100. 50 loops is something like 1ms on my hardware. So I agree, even if the MMC bus gets much faster in the future, I think waiting a minimum of 128us to start will give us some room to adapt without adding unnecessary latency. It might be nice to have someone test with HS400 hardware (which I don't have) Thanks, mh -- Martin Hicks P.Eng. | mort@xxxxxxxx Bork Consulting Inc. | +1 (613) 266-2296 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
