Re: [PATCH] MAINTAINERS: update phylink/sfp keyword matching
From: Joe Perches <joe@perches.com>
Date: 2020-08-05 19:07:21
Also in:
lkml
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:quoted
On Wed, Aug 5, 2020 at 7:34 AM Russell King [off-list ref] wrote:quoted
Is this something you're willing to merge directly please?Done. That said:quoted
-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
quoted
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.