RE: [PATCH 1/2] Drivers: hv: vmbus: Remove duplication and cleanup code in create_gpadl_header()

From: Michael Kelley
Date: Fri Jan 12 2024 - 11:19:22 EST


From: Markus Elfring <Markus.Elfring@xxxxxx> Sent: Friday, January 12, 2024 12:06 AM
>
>
> > Eliminate the duplication by making minor tweaks to the logic and
> > associated comments. While here, simplify the handling of memory
> > allocation errors, and use umin() instead of open coding it.
>
>
> I got the impression that the adjustment for the mentioned macro
> should be performed in a separate update step of the presented patch series.
> https://elixir.bootlin.com/linux/v6.7/source/include/linux/minmax.h#L95
>
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu
> mentation/process/submitting-patches.rst?h=v6.7#n81
>

To me, this is a judgment call. Breaking out the umin() change into
a separate patch is OK, but for consistency then I should probably
break out the change to memory allocation errors in the same
way. Then we would have three patches, plus the patch to
separately handle the indentation so the changes are reviewable.
To me, that's overkill for updates to a single function that have
no functionality change. The intent of the patch is to cleanup
and simplify a single 13-year old function, and it's OK to do
that in a single patch (plus the indentation patch).

Wei Liu is the maintainer for the Hyper-V code. Wei -- any
objections to keeping a single patch (plus the indentation patch)?
But I'll break it out if that's your preference.

Michael