* [PATCH] MAINTAINERS: update phylink/sfp keyword matching
@ 2020-08-05 14:34 Russell King
2020-08-05 18:11 ` Linus Torvalds
0 siblings, 1 reply; 11+ messages in thread
From: Russell King @ 2020-08-05 14:34 UTC (permalink / raw)
To: linux-kernel, Linus Torvalds; +Cc: netdev
syzbot has revealed that the "phylink" keyword exists in non-phylink
related contexts in the bluetooth stack. To avoid receiving
inappropriate notifications, change the keyword matching regexp to
something which avoids this, while still allowing changes to networking
drivers that make use of phylink to be detected.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
Linus,
Is this something you're willing to merge directly please?
Thanks.
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4e2698cc7e23..3b11a8b84129 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15431,7 +15431,7 @@ F: drivers/net/phy/phylink.c
F: drivers/net/phy/sfp*
F: include/linux/phylink.h
F: include/linux/sfp.h
-K: phylink
+K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
SGI GRU DRIVER
M: Dimitri Sivanich <sivanich@sgi.com>
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 14:34 [PATCH] MAINTAINERS: update phylink/sfp keyword matching Russell King
@ 2020-08-05 18:11 ` Linus Torvalds
2020-08-05 18:22 ` Russell King - ARM Linux admin
0 siblings, 1 reply; 11+ messages in thread
From: Linus Torvalds @ 2020-08-05 18:11 UTC (permalink / raw)
To: Russell King; +Cc: Linux Kernel Mailing List, Netdev
On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> Is this something you're willing to merge directly please?
Done.
That said:
> -K: phylink
> +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
That's a very awkward pattern. I wonder if there could be better ways
to express this (ie "only apply this pattern to these files" kind of
thing)
Isn't the 'F' pattern already complete enough that maybe the K pattern
isn't even worth it?
Just a thought, no biggie.
Linus
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 18:11 ` Linus Torvalds
@ 2020-08-05 18:22 ` Russell King - ARM Linux admin
2020-08-05 18:47 ` Joe Perches
2020-08-05 18:54 ` Joe Perches
0 siblings, 2 replies; 11+ messages in thread
From: Russell King - ARM Linux admin @ 2020-08-05 18:22 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux Kernel Mailing List, Netdev
On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > Is this something you're willing to merge directly please?
>
> Done.
>
> That said:
>
> > -K: phylink
> > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
>
> That's a very awkward pattern. I wonder if there could be better ways
> to express this (ie "only apply this pattern to these files" kind of
> thing)
Yes, it's extremely awkward - I spent much of the morning with perl
testing it out on the drivers/ subtree.
> Isn't the 'F' pattern already complete enough that maybe the K pattern
> isn't even worth it?
Unfortunately not; I used not to have a K: line, which presented the
problem that we had users of phylink added to the kernel that were not
being reviewed. So, the suggestion was to add a K: line.
However, I'm now being spammed by syzbot (I've received multiple emails
about the same problem) because, rather than MAINTAINERS being applied
to just patches, it is now being applied to entire source files. This
means that the previous "K: phylink" entry matches not just on patches
(which can be easily ignored) but entire files, such as
net/bluetooth/hci_event.c which happens to contain "phylink" in a
function name.
So, when syzbot identifies there is a problem in
net/bluetooth/hci_event.c, it sends me a report, despite it having
no relevance for me.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 18:22 ` Russell King - ARM Linux admin
@ 2020-08-05 18:47 ` Joe Perches
2020-08-05 21:24 ` Andrew Lunn
2020-08-05 22:09 ` Russell King - ARM Linux admin
2020-08-05 18:54 ` Joe Perches
1 sibling, 2 replies; 11+ messages in thread
From: Joe Perches @ 2020-08-05 18:47 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Linus Torvalds, Dmitry Vyukov
Cc: Linux Kernel Mailing List, Netdev
On Wed, 2020-08-05 at 19:22 +0100, Russell King - ARM Linux admin wrote:
> On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> > On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > Is this something you're willing to merge directly please?
> >
> > Done.
> >
> > That said:
> >
> > > -K: phylink
> > > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> >
> > That's a very awkward pattern. I wonder if there could be better ways
> > to express this (ie "only apply this pattern to these files" kind of
> > thing)
>
> Yes, it's extremely awkward - I spent much of the morning with perl
> testing it out on the drivers/ subtree.
There are a lot of phylink_<foo> in the kernel.
Are those really the only uses you want to watch?
$ git grep -P -oh 'phylink_\w+'| sort | uniq -c
4 phylink_add
7 phylink_an_mode_str
4 phylink_apply_manual_flow
3 phylink_attach_phy
26 phylink_autoneg_inband
4 phylink_bringup_phy
3 phylink_change_inband_advert
6 phylink_clear
4 phylink_complete
2 phylink_complete_evt
145 phylink_config
3 phylink_connect
8 phylink_connect_phy
39 phylink_create
10 phylink_dbg
3 phylink_decode_c37_word
2 phylink_decode_sgmii_word
22 phylink_destroy
11 phylink_disable_state
1 phylink_disconnect
23 phylink_disconnect_phy
4 phylink_do_bit
2 phylink_downs
12 phylink_err
7 phylink_ethtool_get_eee
1 phylink_ethtool_get_eee_err
10 phylink_ethtool_get_pauseparam
11 phylink_ethtool_get_wol
13 phylink_ethtool_ksettings_get
13 phylink_ethtool_ksettings_set
10 phylink_ethtool_nway_reset
7 phylink_ethtool_set_eee
10 phylink_ethtool_set_pauseparam
9 phylink_ethtool_set_wol
2 phylink_fixed_poll
6 phylink_fixed_state
4 phylink_gen_key
5 phylink_get_eee_err
6 phylink_get_fixed_state
3 phylink_get_ksettings
2 phylink_handler
10 phylink_helper_basex_speed
6 phylink_info
4 phylink_init_eee
3 phylink_is_empty_linkmode
4 phylink_link_down
2 phylink_link_handler
168 phylink_link_state
2 phylink_link_up
11 phylink_mac_an_restart
19 phylink_mac_change
33 phylink_mac_config
3 phylink_mac_initial_config
33 phylink_mac_link_down
18 phylink_mac_link_state
28 phylink_mac_link_up
36 phylink_mac_ops
5 phylink_mac_pcs_an_restart
10 phylink_mac_pcs_get_state
4 phylink_major_config
2 phylink_merge_link_mode
4 phylink_mii_c22_pcs_an_restart
4 phylink_mii_c22_pcs_config
4 phylink_mii_c22_pcs_get_state
5 phylink_mii_c22_pcs_set_advertisement
3 phylink_mii_c45_pcs_get_state
3 phylink_mii_emul_read
13 phylink_mii_ioctl
2 phylink_mii_read
2 phylink_mii_write
4 phylink_node
19 phylink_of_phy_connect
16 phylink_ops
2 phylink_op_type
2 phylink_parse_fixedlink
2 phylink_parse_mode
2 phylink_pause_to_str
18 phylink_pcs
6 phylink_pcs_ops
5 phylink_phy_change
2 phylink_phy_no_inband
2 phylink_phy_read
2 phylink_phy_write
6 phylink_printk
2 phylink_register
2 phylink_register_sfp
2 phylink_resolve
2 phylink_resolve_flow
8 phylink_run_resolve
3 phylink_run_resolve_and_disable
406 phylink_set
5 phylink_set_pcs
23 phylink_set_port_modes
2 phylink_setup
2 phylink_sfp_attach
4 phylink_sfp_config
2 phylink_sfp_connect_phy
2 phylink_sfp_detach
2 phylink_sfp_disconnect_phy
2 phylink_sfp_link_down
2 phylink_sfp_link_up
2 phylink_sfp_module_insert
2 phylink_sfp_module_start
2 phylink_sfp_module_stop
11 phylink_speed_down
7 phylink_speed_up
21 phylink_start
23 phylink_stop
31 phylink_test
5 phylink_to_dpaa2_mac
7 phylink_to_port
2 phylink_ups
125 phylink_validate
4 phylink_warn
7 phylink_zero
> > Isn't the 'F' pattern already complete enough that maybe the K pattern
> > isn't even worth it?
>
> Unfortunately not; I used not to have a K: line, which presented the
> problem that we had users of phylink added to the kernel that were not
> being reviewed. So, the suggestion was to add a K: line.
>
> However, I'm now being spammed by syzbot (I've received multiple emails
> about the same problem) because, rather than MAINTAINERS being applied
> to just patches, it is now being applied to entire source files. This
> means that the previous "K: phylink" entry matches not just on patches
> (which can be easily ignored) but entire files, such as
> net/bluetooth/hci_event.c which happens to contain "phylink" in a
> function name.
>
> So, when syzbot identifies there is a problem in
> net/bluetooth/hci_event.c, it sends me a report, despite it having
> no relevance for me.
Maybe instead syzbot could ignore K: lines
by adding --nokeywords to its command line
for get_maintainer.pl.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 18:22 ` Russell King - ARM Linux admin
2020-08-05 18:47 ` Joe Perches
@ 2020-08-05 18:54 ` Joe Perches
2020-08-05 22:02 ` Russell King - ARM Linux admin
1 sibling, 1 reply; 11+ messages in thread
From: Joe Perches @ 2020-08-05 18:54 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Linus Torvalds
Cc: Linux Kernel Mailing List, Netdev
On Wed, 2020-08-05 at 19:22 +0100, Russell King - ARM Linux admin wrote:
> On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> > On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > Is this something you're willing to merge directly please?
> >
> > Done.
> >
> > That said:
> >
> > > -K: phylink
> > > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> >
> > That's a very awkward pattern. I wonder if there could be better ways
> > to express this (ie "only apply this pattern to these files" kind of
> > thing)
>
> Yes, it's extremely awkward - I spent much of the morning with perl
> testing it out on the drivers/ subtree.
And perhaps easier to read would be to use multiple K: lines.
(?: used to avoid unnecessary capture groups)
K: phylink\.h|struct\s+phylink
K: (?:\.|\-\>)phylink_
K: phylink_(?:autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 18:47 ` Joe Perches
@ 2020-08-05 21:24 ` Andrew Lunn
2020-08-05 22:09 ` Russell King - ARM Linux admin
1 sibling, 0 replies; 11+ messages in thread
From: Andrew Lunn @ 2020-08-05 21:24 UTC (permalink / raw)
To: Joe Perches
Cc: Russell King - ARM Linux admin, Linus Torvalds, Dmitry Vyukov,
Linux Kernel Mailing List, Netdev
On Wed, Aug 05, 2020 at 11:47:38AM -0700, Joe Perches wrote:
> On Wed, 2020-08-05 at 19:22 +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> > > On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > Is this something you're willing to merge directly please?
> > >
> > > Done.
> > >
> > > That said:
> > >
> > > > -K: phylink
> > > > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> > >
> > > That's a very awkward pattern. I wonder if there could be better ways
> > > to express this (ie "only apply this pattern to these files" kind of
> > > thing)
> >
> > Yes, it's extremely awkward - I spent much of the morning with perl
> > testing it out on the drivers/ subtree.
>
> There are a lot of phylink_<foo> in the kernel.
> Are those really the only uses you want to watch?
Hi Joe
I think Rusells intention here is to match on MAC drivers which make
use of the phylink API exported to them.
SFF/SFP/SFP+ MODULE SUPPORT
M: Russell King <linux@armlinux.org.uk>
L: netdev@vger.kernel.org
S: Maintained
F: drivers/net/phy/phylink.c
F: drivers/net/phy/sfp*
F: include/linux/phylink.h
F: include/linux/sfp.h
K: phylink
> $ git grep -P -oh 'phylink_\w+'| sort | uniq -c
Try that again, but skip files matched by the F: clauses.
I suspect the matches you get then more closely approximates the K:
Russell is suggesting.
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 18:54 ` Joe Perches
@ 2020-08-05 22:02 ` Russell King - ARM Linux admin
2020-08-05 22:09 ` Joe Perches
0 siblings, 1 reply; 11+ messages in thread
From: Russell King - ARM Linux admin @ 2020-08-05 22:02 UTC (permalink / raw)
To: Joe Perches; +Cc: Linus Torvalds, Linux Kernel Mailing List, Netdev
On Wed, Aug 05, 2020 at 11:54:25AM -0700, Joe Perches wrote:
> On Wed, 2020-08-05 at 19:22 +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> > > On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > Is this something you're willing to merge directly please?
> > >
> > > Done.
> > >
> > > That said:
> > >
> > > > -K: phylink
> > > > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> > >
> > > That's a very awkward pattern. I wonder if there could be better ways
> > > to express this (ie "only apply this pattern to these files" kind of
> > > thing)
> >
> > Yes, it's extremely awkward - I spent much of the morning with perl
> > testing it out on the drivers/ subtree.
>
> And perhaps easier to read would be to use multiple K: lines.
> (?: used to avoid unnecessary capture groups)
>
> K: phylink\.h|struct\s+phylink
> K: (?:\.|\-\>)phylink_
That one is definitely incorrect. It is not .phylink_ or ->phylink_,
it was .phylink (without _) or >phylink_
> K: phylink_(?:autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
>
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 18:47 ` Joe Perches
2020-08-05 21:24 ` Andrew Lunn
@ 2020-08-05 22:09 ` Russell King - ARM Linux admin
2020-08-05 23:07 ` Joe Perches
1 sibling, 1 reply; 11+ messages in thread
From: Russell King - ARM Linux admin @ 2020-08-05 22:09 UTC (permalink / raw)
To: Joe Perches
Cc: Linus Torvalds, Dmitry Vyukov, Linux Kernel Mailing List, Netdev
On Wed, Aug 05, 2020 at 11:47:38AM -0700, Joe Perches wrote:
> On Wed, 2020-08-05 at 19:22 +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> > > On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > Is this something you're willing to merge directly please?
> > >
> > > Done.
> > >
> > > That said:
> > >
> > > > -K: phylink
> > > > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> > >
> > > That's a very awkward pattern. I wonder if there could be better ways
> > > to express this (ie "only apply this pattern to these files" kind of
> > > thing)
> >
> > Yes, it's extremely awkward - I spent much of the morning with perl
> > testing it out on the drivers/ subtree.
>
> There are a lot of phylink_<foo> in the kernel.
> Are those really the only uses you want to watch?
It is sufficient; as I said, I've spent a morning running this:
#!/usr/bin/perl
$re = 'phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)';
foreach $f (@ARGV) {
open F, $f;
$l = 1;
while (<F>) {
chomp;
print "$f:$l: $_\n" if /$re/;
$l++;
}
close F;
}
through:
$ find drivers -type f -print0 | xargs -0 ./check.pl | diff -u pl-ref.out - |less
where pl-ref.out is the original K: matching of just "phylink" and
looking at the differences to ensure I'm excluding just stuff that
doesn't concern me, while getting a high hit rate on the stuff
that I do want.
Now, I'm not saying that there isn't a better way, but this is not
something I want to spend days on. So I got something that works
for me, and that's what I've sent Linus.
Going through your list...
> 4 phylink_add
Not sure what this is. Doesn't seem to be anything to do with what
I maintain.
> 7 phylink_an_mode_str
static function.
> 4 phylink_apply_manual_flow
static function.
> 3 phylink_attach_phy
static function.
> 26 phylink_autoneg_inband
This one public and included.
> 4 phylink_bringup_phy
static function.
> 3 phylink_change_inband_advert
static function.
> 6 phylink_clear
This one public and included.
> 4 phylink_complete
> 2 phylink_complete_evt
Nothing to do with phylink.
> 145 phylink_config
Included.
> 3 phylink_connect
> 8 phylink_connect_phy
Both included under one.
> 39 phylink_create
Included.
> 10 phylink_dbg
static function.
... shall I go on?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 22:02 ` Russell King - ARM Linux admin
@ 2020-08-05 22:09 ` Joe Perches
2020-08-05 22:12 ` Russell King - ARM Linux admin
0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2020-08-05 22:09 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Linus Torvalds, Linux Kernel Mailing List, Netdev
On Wed, 2020-08-05 at 23:02 +0100, Russell King - ARM Linux admin wrote:
> On Wed, Aug 05, 2020 at 11:54:25AM -0700, Joe Perches wrote:
> > On Wed, 2020-08-05 at 19:22 +0100, Russell King - ARM Linux admin wrote:
> > > On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> > > > On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > Is this something you're willing to merge directly please?
> > > >
> > > > Done.
> > > >
> > > > That said:
> > > >
> > > > > -K: phylink
> > > > > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> > > >
> > > > That's a very awkward pattern. I wonder if there could be better ways
> > > > to express this (ie "only apply this pattern to these files" kind of
> > > > thing)
> > >
> > > Yes, it's extremely awkward - I spent much of the morning with perl
> > > testing it out on the drivers/ subtree.
> >
> > And perhaps easier to read would be to use multiple K: lines.
> > (?: used to avoid unnecessary capture groups)
> >
> > K: phylink\.h|struct\s+phylink
> > K: (?:\.|\-\>)phylink_
>
> That one is definitely incorrect. It is not .phylink_ or ->phylink_,
> it was .phylink (without _) or >phylink_
Hi Russell.
I don't see the difference.
All uses of .phylink are followed with _
as far as I can tell.
$ git grep -Poh "\.phylink\S*"|sort|uniq -c
1 .phylink_fixed_state
2 .phylink_mac_an_restart
9 .phylink_mac_config
1 .phylink_mac_config.
11 .phylink_mac_link_down
6 .phylink_mac_link_state
9 .phylink_mac_link_up
38 .phylink_validate
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 22:09 ` Joe Perches
@ 2020-08-05 22:12 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 11+ messages in thread
From: Russell King - ARM Linux admin @ 2020-08-05 22:12 UTC (permalink / raw)
To: Joe Perches; +Cc: Linus Torvalds, Linux Kernel Mailing List, Netdev
On Wed, Aug 05, 2020 at 03:09:34PM -0700, Joe Perches wrote:
> On Wed, 2020-08-05 at 23:02 +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Aug 05, 2020 at 11:54:25AM -0700, Joe Perches wrote:
> > > On Wed, 2020-08-05 at 19:22 +0100, Russell King - ARM Linux admin wrote:
> > > > On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> > > > > On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > > Is this something you're willing to merge directly please?
> > > > >
> > > > > Done.
> > > > >
> > > > > That said:
> > > > >
> > > > > > -K: phylink
> > > > > > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> > > > >
> > > > > That's a very awkward pattern. I wonder if there could be better ways
> > > > > to express this (ie "only apply this pattern to these files" kind of
> > > > > thing)
> > > >
> > > > Yes, it's extremely awkward - I spent much of the morning with perl
> > > > testing it out on the drivers/ subtree.
> > >
> > > And perhaps easier to read would be to use multiple K: lines.
> > > (?: used to avoid unnecessary capture groups)
> > >
> > > K: phylink\.h|struct\s+phylink
> > > K: (?:\.|\-\>)phylink_
> >
> > That one is definitely incorrect. It is not .phylink_ or ->phylink_,
> > it was .phylink (without _) or >phylink_
>
> Hi Russell.
>
> I don't see the difference.
>
> All uses of .phylink are followed with _
> as far as I can tell.
>
> $ git grep -Poh "\.phylink\S*"|sort|uniq -c
> 1 .phylink_fixed_state
> 2 .phylink_mac_an_restart
> 9 .phylink_mac_config
> 1 .phylink_mac_config.
> 11 .phylink_mac_link_down
> 6 .phylink_mac_link_state
> 9 .phylink_mac_link_up
> 38 .phylink_validate
Yes, you're right, but as I explained, I got something that works for
me, and I wasn't going to put more effort in.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
2020-08-05 22:09 ` Russell King - ARM Linux admin
@ 2020-08-05 23:07 ` Joe Perches
0 siblings, 0 replies; 11+ messages in thread
From: Joe Perches @ 2020-08-05 23:07 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Linus Torvalds, Dmitry Vyukov, Linux Kernel Mailing List, Netdev
On Wed, 2020-08-05 at 23:09 +0100, Russell King - ARM Linux admin wrote:
> On Wed, Aug 05, 2020 at 11:47:38AM -0700, Joe Perches wrote:
> > On Wed, 2020-08-05 at 19:22 +0100, Russell King - ARM Linux admin wrote:
> > > On Wed, Aug 05, 2020 at 11:11:28AM -0700, Linus Torvalds wrote:
> > > > On Wed, Aug 5, 2020 at 7:34 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > Is this something you're willing to merge directly please?
> > > >
> > > > Done.
> > > >
> > > > That said:
> > > >
> > > > > -K: phylink
> > > > > +K: phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> > > >
> > > > That's a very awkward pattern. I wonder if there could be better ways
> > > > to express this (ie "only apply this pattern to these files" kind of
> > > > thing)
> > >
> > > Yes, it's extremely awkward - I spent much of the morning with perl
> > > testing it out on the drivers/ subtree.
> >
> > There are a lot of phylink_<foo> in the kernel.
> > Are those really the only uses you want to watch?
>
> It is sufficient; as I said, I've spent a morning running this:
Cool for you, I was just asking.
How you determine what functions you're interested in
is up to you.
Another mechanism might have been something like:
$ git grep -Poh '(?:static\s+inline|EXPORT_SYMBOL).*phylink_[a-z]+' | \
grep -Poh 'phylink_[a-z]+' | sort | uniq
Maybe something that could be made generic in get_maintainer
for people maintaining specific EXPORT_SYMBOL(foo) blocks.
Maybe another letter/look could be added to MAINTAINERS like:
E: symbol_regex
as another similar mechanism for keywords but just for data
or functions marked as exported symbols.
For example, that E: might help minimize xdp matches as xdp
is used by many other bits of code that aren't express data
path uses.
MAINTAINERS-XDP (eXpress Data Path)
MAINTAINERS-M: Alexei Starovoitov <ast@kernel.org>
MAINTAINERS-M: Daniel Borkmann <daniel@iogearbox.net>
MAINTAINERS-M: David S. Miller <davem@davemloft.net>
MAINTAINERS-M: Jakub Kicinski <kuba@kernel.org>
MAINTAINERS-M: Jesper Dangaard Brouer <hawk@kernel.org>
MAINTAINERS-M: John Fastabend <john.fastabend@gmail.com>
MAINTAINERS-L: netdev@vger.kernel.org
MAINTAINERS-L: bpf@vger.kernel.org
MAINTAINERS-S: Supported
MAINTAINERS-F: include/net/xdp.h
MAINTAINERS-F: include/trace/events/xdp.h
MAINTAINERS-F: kernel/bpf/cpumap.c
MAINTAINERS-F: kernel/bpf/devmap.c
MAINTAINERS-F: net/core/xdp.c
MAINTAINERS-N: xdp
MAINTAINERS:K: xdp
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-08-05 23:07 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-05 14:34 [PATCH] MAINTAINERS: update phylink/sfp keyword matching Russell King
2020-08-05 18:11 ` Linus Torvalds
2020-08-05 18:22 ` Russell King - ARM Linux admin
2020-08-05 18:47 ` Joe Perches
2020-08-05 21:24 ` Andrew Lunn
2020-08-05 22:09 ` Russell King - ARM Linux admin
2020-08-05 23:07 ` Joe Perches
2020-08-05 18:54 ` Joe Perches
2020-08-05 22:02 ` Russell King - ARM Linux admin
2020-08-05 22:09 ` Joe Perches
2020-08-05 22:12 ` Russell King - ARM Linux admin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).