From: Jesse Brandeburg <jesse.brandeburg@intel.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
David Miller <davem@davemloft.net>,
Networking <netdev@vger.kernel.org>, <nhorman@redhat.com>,
<sassmann@redhat.com>, <jogreene@redhat.com>,
jacob.e.keller@intel.com
Subject: Re: [net-next 05/14] i40e: fix up 32 bit timespec references
Date: Mon, 25 Feb 2019 08:47:59 -0800 [thread overview]
Message-ID: <20190225084759.00004795@intel.com> (raw)
In-Reply-To: <CAK8P3a3fSofgDdLZ3S+MaEVz8DHnX6HO6xGiRWfCjQ614Kf4Cg@mail.gmail.com>
On Mon, 25 Feb 2019 15:46:18 +0100
Arnd Bergmann <arnd@arndb.de> wrote:
> I just stumbled over code added by this older patch, and can't make sense
> of the commit description here. Was this an attempt to fix a bug, or
> just a cleanup?
Thanks Arnd for mentioning/posting this! This was an attempt at a clean
up, unfortunately I made a mistake which introduced a bug.
> > - then = ns_to_timespec64(delta);
> > mutex_lock(&pf->tmreg_lock);
> >
> > i40e_ptp_read(pf, &now);
> > - now = timespec64_add(now, then);
> > + timespec64_add_ns(&now, delta);
> > i40e_ptp_write(pf, (const struct timespec64 *)&now);
>
> The problem I noticed here is that 'delta' is a user provided 64-bit
> number from clock_adjtime(), and timespec64_add_ns() performs uses
> a repeated addition instead of a div/mod pair. When the number
> is large, we may end up adding a single second 8 billion times,
> which may take a while even on a fast CPU.
>
> Should the commit 0ac30ce43323 ("i40e: fix up 32 bit timespec
> references") just be reverted?
Interestingly, we just were discussing this fix this past Friday, and
have a patch internally to fix the issue correctly. Either way it
is a commit (revert or fix) so we might as well just send the fix ASAP.
Our team should have something posted to intel-wired-lan/netdev shortly,
and we'll get the patch officially to netdev after a brief test.
next prev parent reply other threads:[~2019-02-25 16:48 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-26 10:31 [net-next 00/14][pull request] 40GbE Intel Wired LAN Driver Updates 2017-07-25 Jeff Kirsher
2017-07-26 10:31 ` [net-next 01/14] i40e: fix incorrect variable assignment Jeff Kirsher
2017-07-26 10:31 ` [net-next 02/14] i40e: Fix for trace found with S4 state Jeff Kirsher
2017-07-26 10:31 ` [net-next 03/14] i40e: remove WQ_UNBOUND and the task limit of our workqueue Jeff Kirsher
2017-07-26 10:31 ` [net-next 04/14] i40e: Handle admin Q timeout when releasing NVM Jeff Kirsher
2017-07-26 10:31 ` [net-next 05/14] i40e: fix up 32 bit timespec references Jeff Kirsher
2019-02-25 14:46 ` Arnd Bergmann
2019-02-25 16:47 ` Jesse Brandeburg [this message]
2019-02-25 19:20 ` Keller, Jacob E
2017-07-26 10:31 ` [net-next 06/14] i40e: fix odd formatting and indent Jeff Kirsher
2017-07-26 10:31 ` [net-next 07/14] i40e/i40evf: make IPv6 ATR code clearer Jeff Kirsher
2017-07-26 10:31 ` [net-next 08/14] i40e/i40evf: remove mismatched type warnings Jeff Kirsher
2017-07-26 10:31 ` [net-next 09/14] i40e: display correct UDP tunnel type name Jeff Kirsher
2017-07-26 10:31 ` [net-next 10/14] i40evf: add some missing includes Jeff Kirsher
2017-07-26 10:31 ` [net-next 11/14] i40e: report BPF prog id during XDP_QUERY_PROG Jeff Kirsher
2017-07-26 10:31 ` [net-next 12/14] i40evf: Use le32_to_cpu before evaluating HW desc fields Jeff Kirsher
2017-07-26 10:31 ` [net-next 13/14] i40evf: remove unnecessary __packed Jeff Kirsher
2017-07-26 10:31 ` [net-next 14/14] i40e: handle setting administratively set MAC address back to zero Jeff Kirsher
2017-07-26 23:59 ` [net-next 00/14][pull request] 40GbE Intel Wired LAN Driver Updates 2017-07-25 David Miller
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=20190225084759.00004795@intel.com \
--to=jesse.brandeburg@intel.com \
--cc=arnd@arndb.de \
--cc=davem@davemloft.net \
--cc=jacob.e.keller@intel.com \
--cc=jeffrey.t.kirsher@intel.com \
--cc=jogreene@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=nhorman@redhat.com \
--cc=sassmann@redhat.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 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).