From: Nathan Chancellor <nathan@kernel.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: gregory.greenman@intel.com, kvalo@kernel.org,
ndesaulniers@google.com, trix@redhat.com,
avraham.stern@intel.com, krishnanand.prabhu@intel.com,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
patches@lists.linux.dev, Arnd Bergmann <arnd@arndb.de>,
"kernelci.org bot" <bot@kernelci.org>
Subject: Re: [PATCH wireless-next] wifi: iwlwifi: mvm: Avoid 64-bit division in iwl_mvm_get_crosstimestamp_fw()
Date: Wed, 29 Mar 2023 10:24:58 -0700 [thread overview]
Message-ID: <20230329172458.GA209886@dev-arch.thelio-3990X> (raw)
In-Reply-To: <9058a032c177e9b04adbf944ad34c5ed8090d9d6.camel@sipsolutions.net>
On Wed, Mar 29, 2023 at 07:20:43PM +0200, Johannes Berg wrote:
> On Wed, 2023-03-29 at 10:05 -0700, Nathan Chancellor wrote:
> >
> > GCC has optimizations for division by a constant that clang does not
> > implement, so this issue is not visible when building with GCC.
>
> Huh yeah, we did 32-bit builds with gcc ...
>
> > Using div_u64() would resolve this issue, but Arnd points out that this
> > can be quite expensive and the timestamp is being read at nanosecond
> > granularity.
>
> Doesn't matter though, all the calculations are based on just the
> command response from the firmware, which (tries to) take it in a
> synchronised fashion.
Okay, that is good information, thanks for providing it!
> So taking more time here would be fine, as far as I can tell.
>
> > Nick pointed out that the result of this division is being
> > stored to a 32-bit type anyways, so truncate gp2_10ns first then do the
> > division, which elides the need for libcalls.
>
> That loses ~7 top bits though, no? I'd be more worried about that, than
> the time div_u64() takes.
Right, I sent this version of the fix to spur discussion around whether
or not this was an acceptable approach, rather than having the question
sit unanswered in our issue tracker :) I have no problems sending a v2
to use div_u64() and be done with it, which I will do shortly.
Thanks for the quick input, cheers!
Nathan
next prev parent reply other threads:[~2023-03-29 17:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-29 17:05 [PATCH wireless-next] wifi: iwlwifi: mvm: Avoid 64-bit division in iwl_mvm_get_crosstimestamp_fw() Nathan Chancellor
2023-03-29 17:20 ` Johannes Berg
2023-03-29 17:24 ` Nathan Chancellor [this message]
2023-03-29 17:30 ` Nick Desaulniers
2023-03-29 19:00 ` Johannes Berg
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=20230329172458.GA209886@dev-arch.thelio-3990X \
--to=nathan@kernel.org \
--cc=arnd@arndb.de \
--cc=avraham.stern@intel.com \
--cc=bot@kernelci.org \
--cc=gregory.greenman@intel.com \
--cc=johannes@sipsolutions.net \
--cc=krishnanand.prabhu@intel.com \
--cc=kvalo@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=ndesaulniers@google.com \
--cc=netdev@vger.kernel.org \
--cc=patches@lists.linux.dev \
--cc=trix@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 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.