From: Markus Elfring <[email protected]> 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
From: Michael Kelley <[email protected]> Sent: Friday, January 12, 2024 8:19 AM
>
> From: Markus Elfring <[email protected]> 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.
>
Wei Liu -- any input on this? This is just a cleanup/simplification
patch, so it's not urgent.
Michael
On Fri, Feb 09, 2024 at 03:33:24PM +0000, Michael Kelley wrote:
> From: Michael Kelley <[email protected]> Sent: Friday, January 12, 2024 8:19 AM
> >
> > From: Markus Elfring <[email protected]> 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.
> >
>
> Wei Liu -- any input on this? This is just a cleanup/simplification
> patch, so it's not urgent.
These patches are fine. I'll take them via the hyperv-fixes tree.
Thanks,
Wei.