* [PATCH] [RFC] xfs: work around unlikely() profiler glitch
@ 2017-01-25 14:08 Arnd Bergmann
2017-01-25 15:09 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2017-01-25 14:08 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Steven Rostedt, Nicolas Pitre, Darrick J. Wong,
linux-xfs, Dave Chinner, Brian Foster, Eric Sandeen
After a patch from Steven Rostedt to rework the profiling of
unlikely(), we get a couple of link errors in XFS on ARM:
fs/xfs/libxfs/xfs_bmap.o: In function `xfs_bmap_btalloc':
xfs_bmap.c:(.text.xfs_bmap_btalloc+0x8cc): undefined reference to `____ilog2_NaN'
xfs_bmap.c:(.text.xfs_bmap_btalloc+0xb34): undefined reference to `__aeabi_uldivmod'
xfs_bmap.c:(.text.xfs_bmap_btalloc+0xb7c): undefined reference to `__aeabi_uldivmod'
As I understand the problem, this is a result of the combination
of __builtin_constant_p() usage in three places: unlikely(),
xfs_bmap_btalloc(), and __div64_32(), where the latter attempts to use
__builtin_constant_p() to decide whether to either call an out-of-line
function for variable divisors or an optimized version for constant
divisors.
As I understand it, here we end up with a case where gcc has determined
that the argument is constant in some cases but variable in other cases,
and tries to split up the handling by implementing both paths and deciding
at runtime.
This seems to be related to the problem fixed in commit b33c8ff4431a
("tracing: Fix freak link error caused by branch tracer").
I don't know why exactly it goes wrong now, but we end up getting object
code emitted for the case that should have been eliminated after the
__builtin_constant_p() check. I don't think we'd ever run into the branches
to ____ilog2_NaN or __aeabi_uldivmod, but we get a link error anyway.
Fixes: d45ae1f7041a ("tracing: Process constants for (un)likely() profiler")i
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Nicolas Pitre <nico@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
fs/xfs/libxfs/xfs_bmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index d22f7930eb75..dca3ddd737d4 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3629,7 +3629,7 @@ xfs_bmap_btalloc(
align = xfs_get_cowextsz_hint(ap->ip);
else if (xfs_alloc_is_userdata(ap->datatype))
align = xfs_get_extsz_hint(ap->ip);
- if (unlikely(align)) {
+ if (unlikely_notrace(align)) {
error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
align, 0, ap->eof, 0, ap->conv,
&ap->offset, &ap->length);
--
2.9.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] [RFC] xfs: work around unlikely() profiler glitch
2017-01-25 14:08 [PATCH] [RFC] xfs: work around unlikely() profiler glitch Arnd Bergmann
@ 2017-01-25 15:09 ` Christoph Hellwig
2017-01-25 15:15 ` Arnd Bergmann
2017-01-25 16:24 ` Darrick J. Wong
0 siblings, 2 replies; 4+ messages in thread
From: Christoph Hellwig @ 2017-01-25 15:09 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Steven Rostedt, Nicolas Pitre, Darrick J. Wong,
linux-xfs, Dave Chinner, Brian Foster, Eric Sandeen
On Wed, Jan 25, 2017 at 03:08:10PM +0100, Arnd Bergmann wrote:
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index d22f7930eb75..dca3ddd737d4 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3629,7 +3629,7 @@ xfs_bmap_btalloc(
> align = xfs_get_cowextsz_hint(ap->ip);
> else if (xfs_alloc_is_userdata(ap->datatype))
> align = xfs_get_extsz_hint(ap->ip);
> - if (unlikely(align)) {
> + if (unlikely_notrace(align)) {
> error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
> align, 0, ap->eof, 0, ap->conv,
> &ap->offset, &ap->length);
The unlikely calls on align in xfs_bmap_btalloc should simply be
removed. They aren't actually unlikely for many workloads. I have
a patch in my queue that I can expedite based on your report.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] [RFC] xfs: work around unlikely() profiler glitch
2017-01-25 15:09 ` Christoph Hellwig
@ 2017-01-25 15:15 ` Arnd Bergmann
2017-01-25 16:24 ` Darrick J. Wong
1 sibling, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2017-01-25 15:15 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Linux Kernel Mailing List, Steven Rostedt, Nicolas Pitre,
Darrick J. Wong, linux-xfs, Dave Chinner, Brian Foster,
Eric Sandeen
On Wed, Jan 25, 2017 at 4:09 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Jan 25, 2017 at 03:08:10PM +0100, Arnd Bergmann wrote:
>> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
>> index d22f7930eb75..dca3ddd737d4 100644
>> --- a/fs/xfs/libxfs/xfs_bmap.c
>> +++ b/fs/xfs/libxfs/xfs_bmap.c
>> @@ -3629,7 +3629,7 @@ xfs_bmap_btalloc(
>> align = xfs_get_cowextsz_hint(ap->ip);
>> else if (xfs_alloc_is_userdata(ap->datatype))
>> align = xfs_get_extsz_hint(ap->ip);
>> - if (unlikely(align)) {
>> + if (unlikely_notrace(align)) {
>> error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
>> align, 0, ap->eof, 0, ap->conv,
>> &ap->offset, &ap->length);
>
> The unlikely calls on align in xfs_bmap_btalloc should simply be
> removed. They aren't actually unlikely for many workloads. I have
> a patch in my queue that I can expedite based on your report.
That would defines help, thanks!
I also noticed that my patch wouldn't work, as unlikely_notrace() is not
defined unless we are actually tracing, so while it fixes some rare
configurations, it breaks all the configurations that matter.
Arnd
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] [RFC] xfs: work around unlikely() profiler glitch
2017-01-25 15:09 ` Christoph Hellwig
2017-01-25 15:15 ` Arnd Bergmann
@ 2017-01-25 16:24 ` Darrick J. Wong
1 sibling, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2017-01-25 16:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Arnd Bergmann, linux-kernel, Steven Rostedt, Nicolas Pitre,
linux-xfs, Dave Chinner, Brian Foster, Eric Sandeen
On Wed, Jan 25, 2017 at 07:09:29AM -0800, Christoph Hellwig wrote:
> On Wed, Jan 25, 2017 at 03:08:10PM +0100, Arnd Bergmann wrote:
> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> > index d22f7930eb75..dca3ddd737d4 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -3629,7 +3629,7 @@ xfs_bmap_btalloc(
> > align = xfs_get_cowextsz_hint(ap->ip);
> > else if (xfs_alloc_is_userdata(ap->datatype))
> > align = xfs_get_extsz_hint(ap->ip);
> > - if (unlikely(align)) {
> > + if (unlikely_notrace(align)) {
> > error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
> > align, 0, ap->eof, 0, ap->conv,
> > &ap->offset, &ap->length);
>
> The unlikely calls on align in xfs_bmap_btalloc should simply be
> removed. They aren't actually unlikely for many workloads. I have
> a patch in my queue that I can expedite based on your report.
I was thinking exactly the same thing. Since it breaks the build
somewhere, can you send a oneliner patch so I can roll it into the rc6
fixes?
--D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-25 16:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-25 14:08 [PATCH] [RFC] xfs: work around unlikely() profiler glitch Arnd Bergmann
2017-01-25 15:09 ` Christoph Hellwig
2017-01-25 15:15 ` Arnd Bergmann
2017-01-25 16:24 ` Darrick J. Wong
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).