Thread (11 messages) flat view 11 messages, 5 authors, 2020-08-05

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.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help