search

linux-msm / qdl

BSD 3-Clause "New" or "Revised" License
196 stars 76 forks source link

qdl: Allow user to decide USB OUT buffer multiplier #39

Closed WillDoItMyself closed 3 weeks ago

WillDoItMyself commented 1 year ago

Due to issue related in 760b3df qdl is not sending big chunk of data at once to USB driver. That affects performance of flashing.

This change allows user to set multiplier of buffer size. If may cause issues on some machines but gives huge speed gain.

bmx666 commented 1 year ago

Very useful feature and improvement, it saves me a lot of time, instead of 15 minutes I can flash devices in 2 minutes max! Thank you!

z3ntu commented 2 months ago

Can confirm this works quite well. With this flashing goes from ~14MiB/s without this PR to about ~44MiB/s with --multiplier 2048

multiplier flashing speed
128 38262kB/s
256 40948kB/s
512 42437kB/s
1024 43222kB/s
2048 44038kB/s

When rebasing on master there's some minor conflicts, struct qdl_device has moved to the file qdl.h

andersson commented 2 months ago

Looking at that problem description again, I am puzzled to the exact details and why this wasn't fixed in the kernel instead - it seems unreasonable to allow user space to DOS the kernel like that. And indeed, there is a comment in the kernel documenting that the length can be "almost arbitrarily large". I also don't understand why I accepted changing this to 512 bytes.

With that in mind, I think the proposed multiplier-based mechanism favors user control over user friendliness, and I instead suggest that we clamp the value to e.g 128KB and if that breaks people's systems we 1) debug the kernel 2) consider our options for knobs etc.

andersson commented 3 weeks ago

Replaced with #73, thanks for pushing for this!