netdev - Re: [PATCH 00/12] ice: split ice_virtchnl.c git-blame friendly way

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4d9e2ca6-006d-4da4-8e88-3cf6692432a7@intel.com>
Date: Wed, 13 Aug 2025 09:33:35 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: <intel-wired-lan@...ts.osuosl.org>, Tony Nguyen
	<anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>, Kuniyuki Iwashima
	<kuniyu@...gle.com>, <stable@...r.kernel.org>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 00/12] ice: split ice_virtchnl.c git-blame friendly way

On 8/12/25 22:57, Jakub Kicinski wrote:
> On Tue, 12 Aug 2025 15:28:58 +0200 Przemek Kitszel wrote:
>> Summary:
>> Split ice_virtchnl.c into two more files (+headers), in a way
>> that git-blame works better.
>> Then move virtchnl files into a new subdir.
>> No logic changes.
>>
>> I have developed (or discovered ;)) how to split a file in a way that
>> both old and new are nice in terms of git-blame
>> There were no much disscussion on [RFC], so I would like to propose
>> to go forward with this approach.
>>
>> There is more commits needed to have it nice, so it forms a git-log vs
>> git-blame tradeoff, but (after the brief moment that this is on the top)
>> we spend orders of magnitude more time looking at the blame output (and
>> commit messages linked from that) - so I find it much better to see
>> actual logic changes instead of "move xx to yy" stuff (typical for
>> "squashed/single-commit splits").
>>
>> Cherry-picks/rebases work the same with this method as with simple
>> "squashed/single-commit" approach (literally all commits squashed into
>> one (to have better git-log, but shitty git-blame output).
>>
>> Rationale for the split itself is, as usual, "file is big and we want to
>> extend it".
>>
>> This series is available on my github (just rebased from any
>> earlier mentions):
>> https://github.com/pkitszel/linux/tree/virtchnl-split-Aug12
>> (the simple git-email view flattens this series, removing two
>> merges from the view).
>>
>>
>> [RFC]:
>> https://lore.kernel.org/netdev/5b94d14e-a0e7-47bd-82fc-c85171cbf26e@intel.com/T/#u
>>
>> (I would really look at my fork via your preferred git interaction tool
>> instead of looking at the patches below).
> 
> UI tools aside I wish you didn't cut off the diffstat from the cover
> letter :/ It'd make it much easier to understand what you're splitting.

ooof, that was not intentional (turns out that git-send-email itself was
not bothering to generate diffstat when there are merges present :~|)
anyway, here it is:

  drivers/net/ethernet/intel/ice/Makefile      |    8 +-
  drivers/net/ethernet/intel/ice/ice_sriov.h   |    4 +-
  drivers/net/ethernet/intel/ice/ice_vf_lib.h  |    2 +-
  .../intel/ice/{ => virt}/ice_virtchnl.h      |    0
  .../ice/{ => virt}/ice_virtchnl_allowlist.h  |    0
  .../intel/ice/{ => virt}/ice_virtchnl_fdir.h |    0
  .../intel/ice/virt/ice_virtchnl_queues.h     |   16 +
  .../intel/ice/virt/ice_virtchnl_rss.h        |   14 +
  drivers/net/ethernet/intel/ice/ice_sriov.c   |    2 +-
  drivers/net/ethernet/intel/ice/ice_vf_lib.c  |    2 +-
  .../intel/ice/{ => virt}/ice_virtchnl.c      | 2051 ++-------------
  .../ice/{ => virt}/ice_virtchnl_allowlist.c  |    0
  .../intel/ice/{ => virt}/ice_virtchnl_fdir.c |    0
  .../intel/ice/virt/ice_virtchnl_queues.c     |  975 +++++++
  .../intel/ice/virt/ice_virtchnl_rss.c        |  719 +++++
  15 files changed, 1922 insertions(+), 1871 deletions(-)


> 
> Greg, Sasha, I suspect stable will suffer the most from any file split /
> movement. Do you have any recommendation on what should be allowed?