diff mbox

Revert "btrfs-progs: disable backtrace and define __always_inline"

Message ID 1421187619-24936-1-git-send-email-jbacik@fb.com (mailing list archive)
State Rejected
Headers show

Commit Message

Josef Bacik Jan. 13, 2015, 10:20 p.m. UTC
This reverts commit c2691f807ddd2c6b261c5707f6838a45d9275390 as it breaks
backtracing on actual glibc environments.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 kerncompat.h | 6 ------
 1 file changed, 6 deletions(-)

Comments

David Sterba Jan. 14, 2015, 12:13 a.m. UTC | #1
On Tue, Jan 13, 2015 at 05:20:19PM -0500, Josef Bacik wrote:
> This reverts commit c2691f807ddd2c6b261c5707f6838a45d9275390 as it breaks
> backtracing on actual glibc environments.

This is to support musl libc. If it does not work then the test is wrong
and should be fixed. CCed the author.

http://openwall.com/lists/musl/2013/03/29/13

Seems that there's no ifdef way to find out the libc used.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Merlijn Wajer Jan. 14, 2015, 12:19 a.m. UTC | #2
Hi,

On 14/01/15 01:13, David Sterba wrote:
> On Tue, Jan 13, 2015 at 05:20:19PM -0500, Josef Bacik wrote:
>> This reverts commit c2691f807ddd2c6b261c5707f6838a45d9275390 as it breaks
>> backtracing on actual glibc environments.
> 
> This is to support musl libc. If it does not work then the test is wrong
> and should be fixed. CCed the author.

Thanks for letting me know. It unfortunately seems like I made a
mistake: it should be __GLIBC__ and not __glibc__.

Sorry about that. Would you like me to create a patch?

Regards,
Merlijn
Josef Bacik Jan. 14, 2015, 12:19 a.m. UTC | #3
It's a regression and I don't have the setup to figure out how to make it work for both.  If somebody else can fix it quickly then fine but if not it needs to be reverted.  Thanks,

Josef

David Sterba <dsterba@suse.cz> wrote:


On Tue, Jan 13, 2015 at 05:20:19PM -0500, Josef Bacik wrote:
> This reverts commit c2691f807ddd2c6b261c5707f6838a45d9275390 as it breaks
> backtracing on actual glibc environments.

This is to support musl libc. If it does not work then the test is wrong
and should be fixed. CCed the author.

https://urldefense.proofpoint.com/v1/url?u=http://openwall.com/lists/musl/2013/03/29/13&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=Dm8rcPdQBMJJL%2B383z6ziMVmnPg9BLbftawEBBcsq6E%3D%0A&s=86bd77631ddae58dca51fe780a120817815205620ff4fe920d578ce1f335ad5e

Seems that there's no ifdef way to find out the libc used.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josef Bacik Jan. 14, 2015, 12:26 a.m. UTC | #4
This only works if you include features.h and I don't know if other libcs provide that.  Thanks,

Josef

Merlijn Wajer <merlijn@wizzup.org> wrote:


Hi,

On 14/01/15 01:13, David Sterba wrote:
> On Tue, Jan 13, 2015 at 05:20:19PM -0500, Josef Bacik wrote:
>> This reverts commit c2691f807ddd2c6b261c5707f6838a45d9275390 as it breaks
>> backtracing on actual glibc environments.
>
> This is to support musl libc. If it does not work then the test is wrong
> and should be fixed. CCed the author.

Thanks for letting me know. It unfortunately seems like I made a
mistake: it should be __GLIBC__ and not __glibc__.

Sorry about that. Would you like me to create a patch?

Regards,
Merlijn

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Merlijn Wajer Jan. 14, 2015, 12:31 a.m. UTC | #5
Hi,

On Wed, 14 Jan 2015, Josef Bacik wrote:

> This only works if you include features.h and I don't know if other libcs provide that.  Thanks,

musl provides /usr/include/features.h and glibc does so as well. (Checked
on musl and glibc systems) Searching the web seems to suggest that uclibc
supports it as well.

Would you be in favour of including features.h and checking for __GLIBC__ ?

Regards,
Merlijn
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josef Bacik Jan. 14, 2015, 12:39 a.m. UTC | #6
Yeah just make sure we still get a backtrace on glibc.  Thanks,

Josef

Merlijn Wajer <merlijn@wizzup.org> wrote:


Hi,

On Wed, 14 Jan 2015, Josef Bacik wrote:

> This only works if you include features.h and I don't know if other libcs provide that.  Thanks,

musl provides /usr/include/features.h and glibc does so as well. (Checked
on musl and glibc systems) Searching the web seems to suggest that uclibc
supports it as well.

Would you be in favour of including features.h and checking for __GLIBC__ ?

Regards,
Merlijn
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Jan. 14, 2015, 12:42 a.m. UTC | #7
On Wed, Jan 14, 2015 at 01:31:34AM +0100, Merlijn Wajer wrote:
> > This only works if you include features.h and I don't know if other libcs provide that.  Thanks,
> 
> musl provides /usr/include/features.h and glibc does so as well. (Checked
> on musl and glibc systems) Searching the web seems to suggest that uclibc
> supports it as well.

I'd say that we care about these two right now.

> Would you be in favour of including features.h and checking for __GLIBC__ ?

Yes please.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Merlijn Wajer Jan. 14, 2015, 12:42 a.m. UTC | #8
Hi,

On 14/01/15 01:42, David Sterba wrote:
> On Wed, Jan 14, 2015 at 01:31:34AM +0100, Merlijn Wajer wrote:
>>> This only works if you include features.h and I don't know if other libcs provide that.  Thanks,
>>
>> musl provides /usr/include/features.h and glibc does so as well. (Checked
>> on musl and glibc systems) Searching the web seems to suggest that uclibc
>> supports it as well.
> 
> I'd say that we care about these two right now.
> 
>> Would you be in favour of including features.h and checking for __GLIBC__ ?
> 
> Yes please.

Alright. I will send a patch tomorrow - it's definitely past bed time
here now. :-)

Sorry for the inconvenience.

Regards,
Merlijn
diff mbox

Patch

diff --git a/kerncompat.h b/kerncompat.h
index 7397274..5c1cca9 100644
--- a/kerncompat.h
+++ b/kerncompat.h
@@ -29,12 +29,6 @@ 
 #include <stddef.h>
 #include <linux/types.h>
 #include <stdint.h>
-
-#ifndef __glibc__
-#define BTRFS_DISABLE_BACKTRACE
-#define __always_inline __inline __attribute__ ((__always_inline__))
-#endif
-
 #ifndef BTRFS_DISABLE_BACKTRACE
 #include <execinfo.h>
 #endif