loopdev icon indicating copy to clipboard operation
loopdev copied to clipboard

Wait for ueventd to create loop device on Android

Open tiann opened this issue 2 years ago • 11 comments

Reference: https://cs.android.com/android/_/android/platform/external/toybox/+/51a43ad5225153582b40d3fd289701efc63c8f62

tiann avatar Feb 12 '23 09:02 tiann

I will try it with inotify :)

tiann avatar Feb 13 '23 10:02 tiann

We need this PR : https://github.com/notify-rs/notify/pull/470 to be merged first

tiann avatar Feb 13 '23 11:02 tiann

The loopdev crate has a neat small list of dependencies. Not sure if adding inotify (for target_os = "android") is the way to go. Using the libc interface directly would also be possible and the use case here is simple enough.

Sidenote: I use inotify-rs on Android in production.

flxo avatar Feb 13 '23 15:02 flxo

If you only need inotify [the linux event API], and no other OS support, then notify [the crate] is probably overkill and inotify-rs could be enough.

0xpr03 avatar Feb 13 '23 16:02 0xpr03

And i did a benchmark:

Without inotify:

cost: 20.716512ms
cost: 35.034µs
cost: 20.799764ms
cost: 20.795166ms
cost: 20.71757ms
cost: 20.412802ms
cost: 20.288493ms
cost: 20.745402ms
cost: 20.602376ms
cost: 20.689046ms

Average: 18ms

With inotify:

cost: 11.391805ms
cost: 15.421µs
cost: 6.936767ms
cost: 2.65031ms
cost: 12.248576ms
cost: 8.989787ms
cost: 7.116781ms
cost: 9.271932ms
cost: 15.299µs
cost: 1.726196ms

Average: 6ms

Both are release build for arm64-v8a, It is about 12ms faster.

tiann avatar Feb 14 '23 02:02 tiann

Maybe we change the interval wait time to 1ms without inotify, and it should also work

tiann avatar Feb 14 '23 02:02 tiann

What should we do now? I changed it to loop wait 1ms

tiann avatar Feb 20 '23 02:02 tiann

hi @mdaffin. What's your opinion on that?

flxo avatar Feb 20 '23 08:02 flxo

I've been using this fix for quite some time now: https://github.com/tiann/KernelSU/blob/main/userspace/ksud/Cargo.toml#L38, but I had to maintain my own branch. Would you consider merging it in? I'm open to any suggestions you may have.

tiann avatar Apr 17 '23 03:04 tiann

@mdaffin Could you please check this?

tiann avatar Apr 18 '23 00:04 tiann

I am not a huge fan of spinning on a 1ms delay for up to 2 seconds. It is all well and good if your tests are coming back in ~6ms, but then that raises the question as to what situation it could take to to 2 seconds to update. If we want a delay loop I think it might be better for an exponential back off instead - ie double the delay until a reasonable limit or the timeout is hit? Or does that sound like too much complexity?

Sorry for the delay in looking at this.

mdaffin avatar Apr 18 '23 23:04 mdaffin