[PATCH v5 01/25] net: move intel drivers to intel subdirectory

[David Marchand]

On Thu, Jan 23, 2025 at 5:02 PM Thomas Monjalon <thomas at monjalon.net> wrote:
>
> 23/01/2025 15:35, Bruce Richardson:
> > On Thu, Jan 23, 2025 at 03:16:40PM +0100, David Marchand wrote:
> > > On the check-git-log.sh update, we will have many warnings with current update.
> > >
> > > Wrong headline prefix:
> > >     net/intel/common: add pkt reassembly fn for intel drivers
> > >     net/intel/common: provide common Tx entry structures
> > >     net/intel/common: add Tx mbuf ring replenish fn
> > >     net/intel: align Tx queue struct field names
> > >     net/intel: add prefix for driver-specific structs
> > >     net/intel/common: merge ice and i40e Tx queue struct
> > >     net/iavf: use common Tx queue structure
> > >     net/ixgbe: use common Tx queue structure
> > >     net/intel/common: pack Tx queue structure
> > >     net/intel/common: add post-Tx buffer free function
> > >     net/intel/common: add Tx buffer free fn for AVX-512
> > >     net/iavf: use common Tx free fn for AVX-512
> > >     net/ice: move Tx queue mbuf cleanup fn to common
> > >     net/iavf: use common Tx queue mbuf cleanup fn
> > >     net/ice: use vector SW ring for all vector paths
> > >     net/intel/common: remove unneeded code
> > >     net/intel/common: create common mbuf initializer fn
> > >     net/intel/common: extract common Rx vector criteria
> > >
> > > Invalid patch(es) found - checked 25 patches
> > >
> > > I tried to tweak this a bit, with the following heuristic:
> > > * if touching only net/intel/common, accept net/intel/common:
> > > * if touching multiple drivers under net/intel, then accept net/intel:
> > > as prefix,
> > > * if touching some net/intel/$drv (and optionnally net/intel/common),
> > > accept net/$drv,
> > >
> > > diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
> > > index b2da013f6c..41c290f0ca 100755
> > > --- a/devtools/check-git-log.sh
> > > +++ b/devtools/check-git-log.sh
> > > @@ -79,11 +79,18 @@ bad=$(for commit in $commits ; do
> > >         [ -z "$(echo "$files" | grep -v '^\(drivers\|doc\|config\)/')" ] ||
> > >                 continue
> > >         drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f 2,3 | sort -u)
> > > -       # for drivers/net/intel/* use 2nd and 4th fields not 2nd and 3rd
> > >         if [ "$drv" = "net/intel" ] ; then
> > > -               drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f
> > > 2,4 | sort -u)
> > > +               drvgrp="net/intel"
> > > +               drv=$(echo "$files" | grep '^drivers/' | grep -v
> > > '^drivers/net/intel/common' |
> > > +                       cut -d "/" -f 2,4 | sort -u)
> > > +               if [ $(echo "$drv" | wc -l) -eq 0 ] ; then
> > > +                       drv='net/intel/common:'
> > > +               elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then
> > > +                       drv='net/intel:'
> > > +               fi
> > > +       else
> > > +               drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq)
> > >         fi
> > > -       drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq)
> > >         if [ $(echo "$drvgrp" | wc -l) -gt 1 ] ; then
> > >                 echo "$headline" | grep -v '^drivers:'
> > >         elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then
> > >
> > > Which then complains on patches in this series that touch many drivers
> > > (but have net/intel/common: as prefix where I would suggest net/intel:
> > > instead).
> > >
> >
> > I tend to disagree with this suggestion. I think that a prefix can be valid
> > so long as the prefix matches at least one component in the patch.  For example,
> > for the first patch in the set, I think net/intel/common is a better prefix
> > than just "net/intel". I don't massively object to your suggestion, I just
> > prefer patches identify the most relevant component, if possible, rather
> > than generalities.
>
> Saying net/intel means the same thing as net/intel/common to me.
> Why do we care whether the change is in common or multiple drivers?
> At the end it impacts multiple Intel drivers.
> The goal of the prefix is to quickly catch the scope of the change impact.
>
> One more argument: net/intel is shorter :)

True that any change on net/intel/common will affect many drivers.
Whatever is decided, I just want the check to be less prone to false positives.

With current series, output is a bit too vocal to me.


-- 
David Marchand