Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching

From: Joe Perches
Date: Wed Aug 05 2020 - 15:07:21 EST


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@xxxxxxxxxxxxxxx> 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.