* [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: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: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: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
* 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: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 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
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).