From: Jakub Kicinski <kuba@kernel.org>
To: Richard Cochran <richardcochran@gmail.com>
Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>,
netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Marc Zyngier <maz@kernel.org>,
linux-kernel@vger.kernel.org,
Krzysztof Kozlowski <krzk@kernel.org>,
Vivien Didelot <vivien.didelot@gmail.com>,
Dinh Nguyen <dinguyen@kernel.org>,
Brandon Streiff <brandon.streiff@ni.com>,
Olof Johansson <olof@lixom.net>,
Vladimir Oltean <olteanv@gmail.com>,
David Miller <davem@davemloft.net>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH net 2/4] net: mvpp2: Remove unneeded Kconfig dependency.
Date: Sat, 23 Jan 2021 12:12:27 -0800 [thread overview]
Message-ID: <20210123121227.16384ff5@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20210123132626.GA22662@hoboy.vegasvil.org>
On Sat, 23 Jan 2021 05:26:26 -0800 Richard Cochran wrote:
> On Fri, Jan 22, 2021 at 06:14:44PM -0800, Jakub Kicinski wrote:
>
> > (I would put it in net-next tho, given the above this at most a space
> > optimization.)
>
> It isn't just about space but also time. The reason why I targeted
> net and not net-next was that NETWORK_PHY_TIMESTAMPING activates a
> function call to skb_clone_tx_timestamp() for every transmitted frame.
>
> static inline void skb_tx_timestamp(struct sk_buff *skb)
> {
> skb_clone_tx_timestamp(skb);
> if (skb_shinfo(skb)->tx_flags & SKBTX_SW_TSTAMP)
> skb_tstamp_tx(skb, NULL);
> }
>
> In the abscence of a PHY time stamping device, the check for its
> presence inside of skb_clone_tx_timestamp() will of course fail, but
> this still incurs the cost of the call on every transmitted skb.
>
> Similarly netif_receive_skb() futilely calls skb_defer_rx_timestamp()
> on every received skb.
>
> I would argue that most users don't want this option activated by
> accident.
I see. The only thing I'm worried about then is the churn in patch 3.
This would land in Linus's tree shortly before rc6, kinda late to be
taking chances in the name of minor optimizations :S
> (And yes, we could avoid the functions call by moving the check
> outside of the global functions and inline to the call sites. I'll be
> sure to have that in the shiny new improved scheme under discussion.)
WARNING: multiple messages have this Message-ID (diff)
From: Jakub Kicinski <kuba@kernel.org>
To: Richard Cochran <richardcochran@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
David Miller <davem@davemloft.net>,
netdev@vger.kernel.org,
Russell King - ARM Linux admin <linux@armlinux.org.uk>,
Krzysztof Kozlowski <krzk@kernel.org>,
linux-kernel@vger.kernel.org, Dinh Nguyen <dinguyen@kernel.org>,
Brandon Streiff <brandon.streiff@ni.com>,
Marc Zyngier <maz@kernel.org>, Olof Johansson <olof@lixom.net>,
Vladimir Oltean <olteanv@gmail.com>,
Vivien Didelot <vivien.didelot@gmail.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH net 2/4] net: mvpp2: Remove unneeded Kconfig dependency.
Date: Sat, 23 Jan 2021 12:12:27 -0800 [thread overview]
Message-ID: <20210123121227.16384ff5@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20210123132626.GA22662@hoboy.vegasvil.org>
On Sat, 23 Jan 2021 05:26:26 -0800 Richard Cochran wrote:
> On Fri, Jan 22, 2021 at 06:14:44PM -0800, Jakub Kicinski wrote:
>
> > (I would put it in net-next tho, given the above this at most a space
> > optimization.)
>
> It isn't just about space but also time. The reason why I targeted
> net and not net-next was that NETWORK_PHY_TIMESTAMPING activates a
> function call to skb_clone_tx_timestamp() for every transmitted frame.
>
> static inline void skb_tx_timestamp(struct sk_buff *skb)
> {
> skb_clone_tx_timestamp(skb);
> if (skb_shinfo(skb)->tx_flags & SKBTX_SW_TSTAMP)
> skb_tstamp_tx(skb, NULL);
> }
>
> In the abscence of a PHY time stamping device, the check for its
> presence inside of skb_clone_tx_timestamp() will of course fail, but
> this still incurs the cost of the call on every transmitted skb.
>
> Similarly netif_receive_skb() futilely calls skb_defer_rx_timestamp()
> on every received skb.
>
> I would argue that most users don't want this option activated by
> accident.
I see. The only thing I'm worried about then is the churn in patch 3.
This would land in Linus's tree shortly before rc6, kinda late to be
taking chances in the name of minor optimizations :S
> (And yes, we could avoid the functions call by moving the check
> outside of the global functions and inline to the call sites. I'll be
> sure to have that in the shiny new improved scheme under discussion.)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-01-23 20:13 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 4:05 [PATCH net 0/4] Remove unneeded PHY time stamping option Richard Cochran
2021-01-21 4:05 ` Richard Cochran
2021-01-21 4:06 ` [PATCH net 1/4] net: dsa: mv88e6xxx: Remove bogus Kconfig dependency Richard Cochran
2021-01-21 4:06 ` Richard Cochran
2021-01-21 19:59 ` Brandon Streiff
2021-01-21 19:59 ` Brandon Streiff
2021-01-21 4:06 ` [PATCH net 2/4] net: mvpp2: Remove unneeded " Richard Cochran
2021-01-21 4:06 ` Richard Cochran
2021-01-21 10:27 ` Russell King - ARM Linux admin
2021-01-21 10:27 ` Russell King - ARM Linux admin
2021-01-21 15:08 ` Richard Cochran
2021-01-21 15:08 ` Richard Cochran
2021-01-23 2:14 ` Jakub Kicinski
2021-01-23 2:14 ` Jakub Kicinski
2021-01-23 9:39 ` Russell King - ARM Linux admin
2021-01-23 9:39 ` Russell King - ARM Linux admin
2021-01-23 13:26 ` Richard Cochran
2021-01-23 13:26 ` Richard Cochran
2021-01-23 20:12 ` Jakub Kicinski [this message]
2021-01-23 20:12 ` Jakub Kicinski
2021-01-23 21:14 ` Richard Cochran
2021-01-23 21:14 ` Richard Cochran
2021-01-23 21:38 ` Jakub Kicinski
2021-01-23 21:38 ` Jakub Kicinski
2021-01-21 4:06 ` [PATCH net 3/4] ARM: socfpga_defconfig: Disable PHY time stamping by default Richard Cochran
2021-01-21 4:06 ` Richard Cochran
2021-01-21 4:06 ` [PATCH net 4/4] ARM: axm55xx_defconfig: " Richard Cochran
2021-01-21 4:06 ` Richard Cochran
2021-01-21 15:10 ` [PATCH net 0/4] Remove unneeded PHY time stamping option Richard Cochran
2021-01-21 15:10 ` Richard Cochran
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210123121227.16384ff5@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
--to=kuba@kernel.org \
--cc=andrew@lunn.ch \
--cc=brandon.streiff@ni.com \
--cc=davem@davemloft.net \
--cc=dinguyen@kernel.org \
--cc=f.fainelli@gmail.com \
--cc=krzk@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=maz@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olof@lixom.net \
--cc=olteanv@gmail.com \
--cc=richardcochran@gmail.com \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.