On 2025-11-27 at 12:21:11 GMT, Janusz Krzysztofik wrote:
On Thursday, 27 November 2025 11:46:05 CET Jani Nikula wrote:
On Thu, 27 Nov 2025, Janusz Krzysztofik janusz.krzysztofik@linux.intel.com wrote:
To my taste, zeroing on allocation would be a more clean solution.
IIUC there are micro optimizations to not clear on allocation when you don't strictly have to...
I'm not advocating one or the other approach, just stating what I believe is the reason.
OK, good to hear there is still someone who is able to recall what the reason could be when no hints can be found in git history nor inline comments.
Both approaches are suboptimal in a way - if we zero on allocation, there's redundant writes to the eb_list2 part of the array, but if we don't, then we need to make an additional call to memset() (which of course is the better way to do compared to a for loop). I have no earthly idea which would be faster though, unless some obvious observation eludes me. I vote for the additional memset() for the clarity of intent.
If that's the case, but we agree on pre-zeroing only the sub-area dedicated to the vma table rather than doing that on failure and limited to one element that follows the one that failed, as Krzysztof initially proposed, then I'd vote for restoring memset() that was dropped with commit 170fa29b14fad ("drm/ i915: Simplify eb_lookup_vmas()"). In any case, a clarification (in commit description or inline comment) on why we chose one solutions and not the another wouldn't hurt.
The way I see it done now given the comments: - keep the kvmalloc() - zero the array like in the mentioned commit (e.g. memset()) - add comments to the new code so no one gets lost in the future (I'd prefer putting explanations in comments instead of the commit log)
Thanks Krzysztof
Thanks, Janusz
BR, Jani.