On 11/16/21 18:29, Ben Gardon wrote:
TL;DR: this type of optional refactoring doesn't belong in a patch Cc'd for stable,
and my personal preference is to always declare variables at function scope (it's
not a hard rule though, Paolo has overruled me at least once:-) ).
That makes sense. I don't have a preference either way. Paolo, if you
want the version without the refactor, the version I sent in the RFC
should be good. If the refactor is desired, I can separate it out into
another patch and send a v2 of this patch as a mini series, tagging
only the fix for stable.
It's really a damned-if-you-do/damned-if-you-don't situation. And also
keeping the patch as similar as possible in stable has the advantage
that future backports have a slightly lower chance of breaking due to
shadowed variables.
In the end I agree with both of you :) and in this case I tend to accept
the patch as written. So I queued it, though it probably will not be in
the immediately next pull request.
My plan for the next couple days is to send a pull request and finally
move the development tree to 5.16-rc1, so that I can push to kvm/next
all the SVM, memslot and xarray stuff that's pending. Then I'll go back
to this one.
Paolo
I've generally preferred declaring variables at function scope too
since that seems like the overwhelming convention, but it's always
struck me as a bit of a waste to not make use of scoping rules more.
It does make it nice and clear how things should be laid out when
debugging the kernel with GDB or something though.
In any case, please let me know how you'd like the changes organized
and I can send up follow ups as needed, or we can just move forward
with the RFC version.
