nilfs-utils: need NPTL threads

PATCH New Kurt Van Dijck 5 participants 20 messages Jan 4, 2018 – Jan 7, 2018 1 message not in the archive

lore

Share & tools

lore

Kurt Van Dijck

author

2018-01-04 10:10 UTC #0 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

nilfs-utils use clock_nanosleep(), which comes with NPTL threads

Signed-off-by Kurt Van Dijck <[email protected]>

---
 package/nilfs-utils/Config.in | 1 +
 1 file changed, 1 insertion(+)

package/nilfs-utils/Config.in +1 -0

index 0c6b7a0..9dda27c 100644
@@ -1,6 +1,7 @@
 config BR2_PACKAGE_NILFS_UTILS
 	bool "nilfs-utils"
 	depends on BR2_TOOLCHAIN_HAS_THREADS # sem_open()
+	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
 	depends on BR2_USE_MMU # util-linux libmount, libblkid
 	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
 	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID

Signature · 1 line

1.8.5.rc3

Kurt Van Dijck

author

2018-01-04 10:10 UTC #1 lore raw

PATCH 2/2 nilfs-utils: fix build with static toolchains

This commit adds a patch to respect the dependency libmount->libblkid->libuuid properly in autoconf. This becomes necessary for static builds.

Signed-off-by Kurt Van Dijck <[email protected]>

---
 ...e-PKG_CHECK_MODULES-for-libmount-and-libb.patch | 139 +++++++++++++++++++++
 1 file changed, 139 insertions(+)
 create mode 100644 package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch new file +139 -0

new file mode 100644
index 0000000..3ec4d43
@@ -0,0 +1,139 @@
+From b98acc19604c9cfd473a2932cc69c1e0a6493ae1 Mon Sep 17 00:00:00 2001
+From: Kurt Van Dijck <[email protected]>
+Date: Thu, 4 Jan 2018 09:47:45 +0100
+Subject: [PATCH] autoconf: use PKG_CHECK_MODULES for libmount and libblkid
+
+The dependencies of libmount to libblkid and libblkid to libuuid
+were not handled correctly, and only work for the shared object scenario.
+This commit switches the autoconfiguration to use PKG_CHECK_MODULES
+which handles the static vs. dynamic case.
+This commit has been compile-tested using buildroot on different compilers,
+including static & dynamic ones
+
+Signed-off-by: Kurt Van Dijck <[email protected]>
+---
+ configure.ac                 | 9 ++-------
+ sbin/mkfs/Makefile.am        | 4 ++--
+ sbin/mkfs/mkfs.c             | 6 +++---
+ sbin/mount/Makefile.am       | 4 ++--
+ sbin/mount/mount_libmount.c  | 4 +---
+ sbin/mount/umount_libmount.c | 2 --
+ 6 files changed, 10 insertions(+), 19 deletions(-)
+
+diff --git a/configure.ac b/configure.ac
+index 6fa8c41..83f52eb 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -104,13 +104,11 @@ AC_CHECK_HEADERS([ctype.h err.h fcntl.h grp.h libintl.h limits.h \
+ 
+ # Check for conditional libraries and headers.
+ if test "${with_libmount}" = "yes"; then
+-   AC_CHECK_LIB(mount, mnt_context_do_mount, [LIB_MOUNT="-lmount"],
++   PKG_CHECK_MODULES([MOUNT], [mount],,
+    	AC_MSG_ERROR([Mount library is enabled but libmount not found]))
+-   AC_CHECK_HEADERS([libmount/libmount.h])
+    with_selinux=no
+ fi
+ AM_CONDITIONAL(CONFIG_LIBMOUNT, [test "$with_libmount" = "yes"])
+-AC_SUBST(LIB_MOUNT)
+ 
+ if test "${with_selinux}" = "yes"; then
+    AC_CHECK_LIB(selinux, getprevcon,
+@@ -125,15 +123,12 @@ fi
+ AC_SUBST([LIB_SELINUX])
+ 
+ if test "${with_blkid}" = "yes"; then
+-   AC_CHECK_LIB(blkid, blkid_new_probe_from_filename,
++   PKG_CHECK_MODULES([BLKID], [blkid],
+    		[AC_DEFINE(HAVE_LIBBLKID, 1,
+ 		    [Define to 1 if you have the 'blkid' library (-lblkid).])
+-		 LIB_BLKID="-lblkid"
+ 		],
+ 		AC_MSG_ERROR([BLKID library not found]))
+-   AC_CHECK_HEADERS([blkid/blkid.h])
+ fi
+-AC_SUBST(LIB_BLKID)
+ 
+ # Checks for typedefs, structures, and compiler characteristics.
+ AC_C_CONST
+diff --git a/sbin/mkfs/Makefile.am b/sbin/mkfs/Makefile.am
+index 28f5128..408c976 100644
+--- a/sbin/mkfs/Makefile.am
++++ b/sbin/mkfs/Makefile.am
+@@ -1,8 +1,8 @@
+ ## Makefile.am
+ 
+-AM_CFLAGS = -Wall
++AM_CFLAGS = -Wall $(BLKID_CFLAGS)
+ AM_CPPFLAGS = -I$(top_srcdir)/include
+-LDADD = -luuid $(LIB_BLKID) $(top_builddir)/lib/libnilfsfeature.la \
++LDADD = -luuid $(BLKID_LIBS) $(top_builddir)/lib/libnilfsfeature.la \
+ 	$(top_builddir)/lib/libmountchk.la \
+ 	$(top_builddir)/lib/libcrc32.la
+ 
+diff --git a/sbin/mkfs/mkfs.c b/sbin/mkfs/mkfs.c
+index d7f161e..0c45d6a 100644
+--- a/sbin/mkfs/mkfs.c
++++ b/sbin/mkfs/mkfs.c
+@@ -66,9 +66,9 @@
+ 
+ #include <errno.h>
+ 
+-#if HAVE_BLKID_BLKID_H
+-#include <blkid/blkid.h>
+-#endif	/* HAVE_BLKID_BLKID_H */
++#if HAVE_LIBBLKID
++#include <blkid.h>
++#endif	/* HAVE_LIBBLKID */
+ 
+ #include "nilfs.h"
+ #include "util.h"
+diff --git a/sbin/mount/Makefile.am b/sbin/mount/Makefile.am
+index f5d3c27..9554aa5 100644
+--- a/sbin/mount/Makefile.am
++++ b/sbin/mount/Makefile.am
+@@ -6,10 +6,10 @@ COMMONHEADERS = mount.nilfs2.h sundries.h xmalloc.h
+ LEGACYSOURCES = fstab.c mount_mntent.c mount_opts.c
+ LEGACYHEADERS = fstab.h mount_constants.h mount_mntent.h mount_opts.h
+ 
+-AM_CFLAGS = -Wall
++AM_CFLAGS = -Wall $(MOUNT_CFLAGS)
+ AM_CPPFLAGS = -I$(top_srcdir)/include
+ LDADD = $(top_builddir)/lib/librealpath.la \
+-	$(top_builddir)/lib/libcleanerexec.la $(LIB_MOUNT) $(LIB_SELINUX) \
++	$(top_builddir)/lib/libcleanerexec.la $(MOUNT_LIBS) $(LIB_SELINUX) \
+ 	$(LIB_POSIX_TIMER)
+ 
+ root_sbin_PROGRAMS = mount.nilfs2 umount.nilfs2
+diff --git a/sbin/mount/mount_libmount.c b/sbin/mount/mount_libmount.c
+index ef40e68..a7fec00 100644
+--- a/sbin/mount/mount_libmount.c
++++ b/sbin/mount/mount_libmount.c
+@@ -67,9 +67,7 @@
+ #include <syslog.h>
+ #endif	/* HAVE_SYSLOG_H */
+ 
+-#if HAVE_LIBMOUNT_LIBMOUNT_H
+-#include <libmount/libmount.h>
+-#endif	/* HAVE_LIBMOUNT_H */
++#include <libmount.h>
+ 
+ #include <stdarg.h>
+ #include <errno.h>
+diff --git a/sbin/mount/umount_libmount.c b/sbin/mount/umount_libmount.c
+index ae5a337..2987f5d 100644
+--- a/sbin/mount/umount_libmount.c
++++ b/sbin/mount/umount_libmount.c
+@@ -59,9 +59,7 @@
+ #include <syslog.h>
+ #endif	/* HAVE_SYSLOG_H */
+ 
+-#if HAVE_LIBMOUNT_LIBMOUNT_H
+ #include <libmount/libmount.h>
+-#endif	/* HAVE_LIBMOUNT_H */
+ 
+ #include <stdarg.h>
+ #include <errno.h>
+-- 
+1.8.5.rc3
+

Signature · 1 line

1.8.5.rc3

Thomas Petazzoni

2018-01-06 14:47 UTC #2 lore raw

PATCH 2/2 nilfs-utils: fix build with static toolchains

Hello,

Quoted 4 lines · Kurt Van Dijck

This commit adds a patch to respect the dependency libmount->libblkid->libuuid properly in autoconf. This becomes necessary for static builds.
Signed-off-by Kurt Van Dijck <[email protected]>

Thanks for working on this! It looks a lot better. I have a few minor/comments, see below.

Also, could you submit this patch upstream?

Quoted 5 lines

+ # Check for conditional libraries and headers.
+ if test "${with_libmount}" = "yes"; then
+-   AC_CHECK_LIB(mount, mnt_context_do_mount, [LIB_MOUNT="-lmount"],
++   PKG_CHECK_MODULES([MOUNT], [mount],,
+    	AC_MSG_ERROR([Mount library is enabled but libmount not found]))

I believe you could drop entirely the AC_MSG_ERROR() message here. PKG_CHECK_MODULES([MOUNT], [mount]) already aborts with an error message if the "mount" pkg-config description cannot be found.

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch line 52

Quoted 13 lines

+ if test "${with_selinux}" = "yes"; then

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch new file +12 -0

+    AC_CHECK_LIB(selinux, getprevcon,
+@@ -125,15 +123,12 @@ fi
+ AC_SUBST([LIB_SELINUX])
+ 
+ if test "${with_blkid}" = "yes"; then
+-   AC_CHECK_LIB(blkid, blkid_new_probe_from_filename,
++   PKG_CHECK_MODULES([BLKID], [blkid],
+    		[AC_DEFINE(HAVE_LIBBLKID, 1,
+ 		    [Define to 1 if you have the 'blkid' library (-lblkid).])
+-		 LIB_BLKID="-lblkid"
+ 		],
+ 		AC_MSG_ERROR([BLKID library not found]))

Ditto here.

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch line 119

Quoted 12 lines

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch new file +12 -0

+diff --git a/sbin/mount/mount_libmount.c b/sbin/mount/mount_libmount.c
+index ef40e68..a7fec00 100644
+--- a/sbin/mount/mount_libmount.c
++++ b/sbin/mount/mount_libmount.c
+@@ -67,9 +67,7 @@
+ #include <syslog.h>
+ #endif	/* HAVE_SYSLOG_H */
+ 
+-#if HAVE_LIBMOUNT_LIBMOUNT_H
+-#include <libmount/libmount.h>
+-#endif	/* HAVE_LIBMOUNT_H */
++#include <libmount.h>

So you're dropping the HAVE_LIBMOUNT_LIBMOUNT_H condition because this file is anyway only built when libmount support is enabled. Correct ?

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch line 133

Quoted 11 lines

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch new file +11 -0

+diff --git a/sbin/mount/umount_libmount.c b/sbin/mount/umount_libmount.c
+index ae5a337..2987f5d 100644
+--- a/sbin/mount/umount_libmount.c
++++ b/sbin/mount/umount_libmount.c
+@@ -59,9 +59,7 @@
+ #include <syslog.h>
+ #endif	/* HAVE_SYSLOG_H */
+ 
+-#if HAVE_LIBMOUNT_LIBMOUNT_H
+ #include <libmount/libmount.h>
+-#endif	/* HAVE_LIBMOUNT_H */

In the previous file, you change <libmount/libmount.h> to <libmount.h>, but not here. Why?

Could you fix those minor issues and send an updated version?

Thanks a lot!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Kurt Van Dijck

author

2018-01-06 20:30 UTC #3 lore raw

PATCH 2/2 nilfs-utils: fix build with static toolchains

Quoted 7 lines

Hello,
Quoted 3 lines · Kurt Van Dijck
This commit adds a patch to respect the dependency libmount->libblkid->libuuid properly in autoconf. This becomes necessary for static builds.

Quoted 6 lines

Quoted 1 line
Signed-off-by Kurt Van Dijck <[email protected]>
Thanks for working on this! It looks a lot better. I have a few minor/comments, see below.
Also, could you submit this patch upstream?

Yep, that was my intent.
But I'd rather wait for the patch to be complete, consistent, stable, so I'd wait for all your comments, and the results of the build server...

Quoted 10 lines

Quoted 6 lines
+ # Check for conditional libraries and headers.
+ if test "${with_libmount}" = "yes"; then
+-   AC_CHECK_LIB(mount, mnt_context_do_mount, [LIB_MOUNT="-lmount"],
++   PKG_CHECK_MODULES([MOUNT], [mount],,
+    	AC_MSG_ERROR([Mount library is enabled but libmount not found]))
I believe you could drop entirely the AC_MSG_ERROR() message here. PKG_CHECK_MODULES([MOUNT], [mount]) already aborts with an error message if the "mount" pkg-config description cannot be found.

I see. I wasn't aware of that.
I'll adapt.

Quoted 17 lines

Quoted 4 lines
+ if test "${with_selinux}" = "yes"; then
package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch new file +2 -0
+    AC_CHECK_LIB(selinux, getprevcon,
+@@ -125,15 +123,12 @@ fi
package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch line 52
Quoted 10 lines
package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch new file +10 -0
+ AC_SUBST([LIB_SELINUX])
+ 
+ if test "${with_blkid}" = "yes"; then
+-   AC_CHECK_LIB(blkid, blkid_new_probe_from_filename,
++   PKG_CHECK_MODULES([BLKID], [blkid],
+    		[AC_DEFINE(HAVE_LIBBLKID, 1,
+ 		    [Define to 1 if you have the 'blkid' library (-lblkid).])
+-		 LIB_BLKID="-lblkid"
+ 		],
+ 		AC_MSG_ERROR([BLKID library not found]))
Ditto here.

Quoted 15 lines

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch line 119
Quoted 12 lines
package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch new file +12 -0
+diff --git a/sbin/mount/mount_libmount.c b/sbin/mount/mount_libmount.c
+index ef40e68..a7fec00 100644
+--- a/sbin/mount/mount_libmount.c
++++ b/sbin/mount/mount_libmount.c
+@@ -67,9 +67,7 @@
+ #include <syslog.h>
+ #endif	/* HAVE_SYSLOG_H */
+ 
+-#if HAVE_LIBMOUNT_LIBMOUNT_H
+-#include <libmount/libmount.h>
+-#endif	/* HAVE_LIBMOUNT_H */
++#include <libmount.h>
So you're dropping the HAVE_LIBMOUNT_LIBMOUNT_H condition because this file is anyway only built when libmount support is enabled. Correct ?

Yep, that's the idea. Makefile.am reveals this.

Quoted 15 lines

package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch line 133
Quoted 12 lines
package/nilfs-utils/0004-autoconf-use-PKG_CHECK_MODULES-for-libmount-and-libb.patch new file +11 -0
+diff --git a/sbin/mount/umount_libmount.c b/sbin/mount/umount_libmount.c
+index ae5a337..2987f5d 100644
+--- a/sbin/mount/umount_libmount.c
++++ b/sbin/mount/umount_libmount.c
+@@ -59,9 +59,7 @@
+ #include <syslog.h>
+ #endif	/* HAVE_SYSLOG_H */
+ 
+-#if HAVE_LIBMOUNT_LIBMOUNT_H
+ #include <libmount/libmount.h>
+-#endif	/* HAVE_LIBMOUNT_H */
In the previous file, you change <libmount/libmount.h> to <libmount.h>, but not here. Why?

I changed to <libmount.h> because pkgconfig add -I/path/to/libmount anyway. I didn't repeat that here because ... I forgot.

Quoted 2 lines

Could you fix those minor issues and send an updated version?

Yep. on my todo list.
Kurt

Baruch Siach

2018-01-04 10:19 UTC #4 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

In reply to Kurt Van Dijck

Hi Kurt,

Quoted 1 line · Kurt Van Dijck

nilfs-utils use clock_nanosleep(), which comes with NPTL threads

What is your indication that this is the case? In the uClibc-ng config UCLIBC_HAS_REALTIME is selected by both linuxthreads and NPTL. When UCLIBC_HAS_REALTIME is enabled, UCLIBC_HAS_ADVANCED_REALTIME is also enabled by default. UCLIBC_HAS_ADVANCED_REALTIME provides clock_nanosleep().

Quoted 10 lines

Signed-off-by Kurt Van Dijck <[email protected]>
---
 package/nilfs-utils/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/nilfs-utils/Config.in +0 -0
index 0c6b7a0..9dda27c 100644

package/nilfs-utils/Config.in line 4

Quoted 4 lines

package/nilfs-utils/Config.in +1 -0

@@ -1,6 +1,7 @@
 config BR2_PACKAGE_NILFS_UTILS
 	bool "nilfs-utils"
 	depends on BR2_TOOLCHAIN_HAS_THREADS # sem_open()
+	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()

NPTL implies threads support, so the NPTL dependency alone is enough.

You also need to update the comment.

package/nilfs-utils/Config.in line 7

Quoted 3 lines

package/nilfs-utils/Config.in +0 -0

 	depends on BR2_USE_MMU # util-linux libmount, libblkid
 	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
 	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID

baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Thomas Petazzoni

2018-01-04 10:33 UTC #5 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Hello,

Quoted 7 lines · Baruch Siach

Quoted 1 line · Kurt Van Dijck
nilfs-utils use clock_nanosleep(), which comes with NPTL threads
What is your indication that this is the case? In the uClibc-ng config UCLIBC_HAS_REALTIME is selected by both linuxthreads and NPTL. When UCLIBC_HAS_REALTIME is enabled, UCLIBC_HAS_ADVANCED_REALTIME is also enabled by default. UCLIBC_HAS_ADVANCED_REALTIME provides clock_nanosleep().

I haven't looked at it in detail, but:

package/blktrace/Config.in:     depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
package/can-utils/Config.in:    depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
package/dvblast/Config.in:      depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Baruch Siach

2018-01-04 10:51 UTC #6 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Hi Thomas,

Quoted 5 lines · Thomas Petazzoni

Quoted 3 lines · Baruch Siach
Quoted 1 line · Kurt Van Dijck
nilfs-utils use clock_nanosleep(), which comes with NPTL threads

Quoted 8 lines

Quoted 4 lines
What is your indication that this is the case? In the uClibc-ng config UCLIBC_HAS_REALTIME is selected by both linuxthreads and NPTL. When UCLIBC_HAS_REALTIME is enabled, UCLIBC_HAS_ADVANCED_REALTIME is also enabled by default. UCLIBC_HAS_ADVANCED_REALTIME provides clock_nanosleep().
I haven't looked at it in detail, but:
package/blktrace/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()

Added in commit 08b9e24eaa4e. No previous threads dependency.

Quoted 1 line

package/can-utils/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()

Added in commit dbfca7963049. No previous threads dependency.

Quoted 1 line

package/dvblast/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()

Added in commit ec875d9bf9b183, changed from a previous plain threads dependency.

Yann, do you remember the reason why "clock_nanosleep() is only availabe with NPTL" as commit ec875d9bf9b183 says?

baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Kurt Van Dijck

author

2018-01-04 11:28 UTC #7 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Quoted 9 lines

Hi Thomas,
Quoted 6 lines · Thomas Petazzoni
Quoted 4 lines · Baruch Siach
Quoted 1 line · Kurt Van Dijck
nilfs-utils use clock_nanosleep(), which comes with NPTL threads
What is your indication that this is the case?

The input of Thomas below indeed was my indication. Adding this effectively solved/avoided build problems using toolchains with threads but without NPTL threads. So I thought the puzzle was complete here ...

Quoted 25 lines

Quoted 8 lines
Quoted 4 lines
In the uClibc-ng config
UCLIBC_HAS_REALTIME is selected by both linuxthreads and NPTL. When UCLIBC_HAS_REALTIME is enabled, UCLIBC_HAS_ADVANCED_REALTIME is also enabled by default. UCLIBC_HAS_ADVANCED_REALTIME provides clock_nanosleep().
I haven't looked at it in detail, but:
package/blktrace/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit 08b9e24eaa4e. No previous threads dependency.
Quoted 1 line
package/can-utils/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit dbfca7963049. No previous threads dependency.
Quoted 1 line
package/dvblast/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit ec875d9bf9b183, changed from a previous plain threads dependency.
Yann, do you remember the reason why "clock_nanosleep() is only availabe with NPTL" as commit ec875d9bf9b183 says?
baruch

Yann E. MORIN

2018-01-04 15:52 UTC #8 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

On 2018-01-04 12:28 +0100, Kurt Van Dijck spake thusly:

Quoted 6 lines

Quoted 6 lines
Hi Thomas,
Quoted 3 lines · Thomas Petazzoni
Quoted 1 line · Baruch Siach
On Thu, Jan 04, 2018 at 11:10:51AM +0100, Kurt Van Dijck wrote:

Quoted 32 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
nilfs-utils use clock_nanosleep(), which comes with NPTL threads
What is your indication that this is the case?
The input of Thomas below indeed was my indication. Adding this effectively solved/avoided build problems using toolchains with threads but without NPTL threads. So I thought the puzzle was complete here ...
Quoted 23 lines
Quoted 8 lines
Quoted 4 lines
In the uClibc-ng config
UCLIBC_HAS_REALTIME is selected by both linuxthreads and NPTL. When UCLIBC_HAS_REALTIME is enabled, UCLIBC_HAS_ADVANCED_REALTIME is also enabled by default. UCLIBC_HAS_ADVANCED_REALTIME provides clock_nanosleep().
I haven't looked at it in detail, but:
package/blktrace/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit 08b9e24eaa4e. No previous threads dependency.
Quoted 1 line
package/can-utils/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit dbfca7963049. No previous threads dependency.
Quoted 1 line
package/dvblast/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit ec875d9bf9b183, changed from a previous plain threads dependency.
Yann, do you remember the reason why "clock_nanosleep() is only availabe with NPTL" as commit ec875d9bf9b183 says?

No, I am not sure, but the current code of uClibc-ng has:

    $ cat librt/Makefile.in
    [...]
    17 ifeq ($(UCLIBC_HAS_THREADS_NATIVE),y)
    18 librt_filter_SRC += mq_notify.c timer_create.c timer_delete.c \
    19 	timer_getoverr.c timer_gettime.c timer_settime.c
    20 else
    21 librt_filter_SRC += clock_nanosleep.c clock_getcpuclockid.c clock_gettime.c
    22 endif
    [...]

So, if I read that correctly, the clock_*.c files are excluded (filtered) if there is no support for native threads.

Now, since glibc and musl always have NPTL and always have clock_nanosleep, and only uClibc has a conditional NPTL, we made clock_nanosleep et al. depend on NPTL.

Q.E.D.

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Baruch Siach

2018-01-04 17:23 UTC #9 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Hi Yann,

Quoted 50 lines · Yann E. MORIN

On 2018-01-04 12:28 +0100, Kurt Van Dijck spake thusly:
Quoted 4 lines
Quoted 4 lines · Thomas Petazzoni
Quoted 3 lines · Baruch Siach
Quoted 2 lines · Kurt Van Dijck
Quoted 32 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
nilfs-utils use clock_nanosleep(), which comes with NPTL threads
What is your indication that this is the case?
The input of Thomas below indeed was my indication. Adding this effectively solved/avoided build problems using toolchains with threads but without NPTL threads. So I thought the puzzle was complete here ...
Quoted 23 lines
Quoted 8 lines
Quoted 4 lines
In the uClibc-ng config
UCLIBC_HAS_REALTIME is selected by both linuxthreads and NPTL. When UCLIBC_HAS_REALTIME is enabled, UCLIBC_HAS_ADVANCED_REALTIME is also enabled by default. UCLIBC_HAS_ADVANCED_REALTIME provides clock_nanosleep().
I haven't looked at it in detail, but:
package/blktrace/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit 08b9e24eaa4e. No previous threads dependency.
Quoted 1 line
package/can-utils/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit dbfca7963049. No previous threads dependency.
Quoted 1 line
package/dvblast/Config.in: depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
Added in commit ec875d9bf9b183, changed from a previous plain threads dependency.
Yann, do you remember the reason why "clock_nanosleep() is only availabe with NPTL" as commit ec875d9bf9b183 says?
No, I am not sure, but the current code of uClibc-ng has:
    $ cat librt/Makefile.in
    [...]
    17 ifeq ($(UCLIBC_HAS_THREADS_NATIVE),y)
    18 librt_filter_SRC += mq_notify.c timer_create.c timer_delete.c \
    19 	timer_getoverr.c timer_gettime.c timer_settime.c
    20 else
    21 librt_filter_SRC += clock_nanosleep.c clock_getcpuclockid.c clock_gettime.c
    22 endif
    [...]

Quoted 8 lines

So, if I read that correctly, the clock_*.c files are excluded (filtered) if there is no support for native threads.
Now, since glibc and musl always have NPTL and always have clock_nanosleep, and only uClibc has a conditional NPTL, we made clock_nanosleep et al. depend on NPTL.
Q.E.D.

This contradicts the UCLIBC_HAS_ADVANCED_REALTIME help text that lists clock_nanosleep() with the API it enables. I guess it is the code the needs fixing here.

Waldemar?

baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

A message that is not in the archive (sent off-list or not yet received): [email protected]

Baruch Siach

2018-01-05 05:23 UTC #11 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Hi Wldemar,

Quoted 2 lines · Waldemar Brodkorb

Baruch Siach wrote,
Quoted 1 line · Yann E. MORIN

[...]

Quoted 30 lines

Quoted 22 lines
Quoted 22 lines
Quoted 2 lines
Yann, do you remember the reason why "clock_nanosleep() is only availabe with NPTL" as commit ec875d9bf9b183 says?
No, I am not sure, but the current code of uClibc-ng has:
    $ cat librt/Makefile.in
    [...]
    17 ifeq ($(UCLIBC_HAS_THREADS_NATIVE),y)
    18 librt_filter_SRC += mq_notify.c timer_create.c timer_delete.c \
    19 	timer_getoverr.c timer_gettime.c timer_settime.c
    20 else
    21 librt_filter_SRC += clock_nanosleep.c clock_getcpuclockid.c clock_gettime.c
    22 endif
    [...]
So, if I read that correctly, the clock_*.c files are excluded (filtered) if there is no support for native threads.
Now, since glibc and musl always have NPTL and always have clock_nanosleep, and only uClibc has a conditional NPTL, we made clock_nanosleep et al. depend on NPTL.
Quoted 7 lines
Quoted 1 line
Q.E.D.
This contradicts the UCLIBC_HAS_ADVANCED_REALTIME help text that lists clock_nanosleep() with the API it enables. I guess it is the code the needs fixing here.
Waldemar?

Quoted 8 lines

Hmm. I do not exactly understand what you mean.
It seems some extra advanced realtime functions where added in commit a202cf6f119f41532b60ad28bc48746b535fd34c, because at this time NPTL was available. A user can disable these extra functions if it is unused in his usecase.
So I believe, when an application is using clock_nanosleep, Buildroot should depend on NPTL for it.

The UCLIBC_HAS_ADVANCED_REALTIME help text says that this options enables clock_nanosleep(). But with current code you must have NPTL enabled for clock_nanosleep(). So the help text is not correct.

This also means that architectures lack NPTL support can't have clock_nanosleep() at all. Is there a reason for that?

Thanks,
baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Kurt Van Dijck

author

2018-01-05 10:17 UTC #12 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Hey,

[...]

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Yann, do you remember the reason why "clock_nanosleep() is only availabe with NPTL" as commit ec875d9bf9b183 says?

[...]

Quoted 6 lines

The UCLIBC_HAS_ADVANCED_REALTIME help text says that this options enables clock_nanosleep(). But with current code you must have NPTL enabled for clock_nanosleep(). So the help text is not correct.
This also means that architectures lack NPTL support can't have clock_nanosleep() at all. Is there a reason for that?

I share the concern, I appears that a shortcut has been taken in the uClibc configuration.

OTOH, the topic of this thread has drifted away.
From the nilfs-utils point of view, depending on NPTL is necessary today, so the patch stands.
I propose to start a different thread, and merge this patch for nilfs-utils. Doing so would fix the current build problems, and when the uclibc problem eventually resolves, nilfs-utils too will be patched to not depend on NPTL.

What do you think about that?

Kurt

Thomas Petazzoni

2018-01-05 10:31 UTC #13 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Hello,

Quoted 18 lines · Kurt Van Dijck

Quoted 6 lines
The UCLIBC_HAS_ADVANCED_REALTIME help text says that this options enables clock_nanosleep(). But with current code you must have NPTL enabled for clock_nanosleep(). So the help text is not correct.
This also means that architectures lack NPTL support can't have clock_nanosleep() at all. Is there a reason for that?
I share the concern, I appears that a shortcut has been taken in the uClibc configuration.
OTOH, the topic of this thread has drifted away.
From the nilfs-utils point of view, depending on NPTL is necessary today, so the patch stands.
I propose to start a different thread, and merge this patch for nilfs-utils. Doing so would fix the current build problems, and when the uclibc problem eventually resolves, nilfs-utils too will be patched to not depend on NPTL.

Quoted 1 line

What do you think about that?

I agree. Especially since the same problem affects other packages, and we have already added a NPTL dependency for those. When/if the uClibc problem is fixed, we can get back to those packages and remove the NPTL dependency if clock_nanosleep() becomes usable without NPTL.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Baruch Siach

2018-01-05 11:24 UTC #14 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

In reply to Kurt Van Dijck

Hi Kurt,

Quoted 22 lines · Kurt Van Dijck

[...]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Yann, do you remember the reason why "clock_nanosleep() is only availabe with NPTL" as commit ec875d9bf9b183 says?
[...]
Quoted 6 lines
The UCLIBC_HAS_ADVANCED_REALTIME help text says that this options enables clock_nanosleep(). But with current code you must have NPTL enabled for clock_nanosleep(). So the help text is not correct.
This also means that architectures lack NPTL support can't have clock_nanosleep() at all. Is there a reason for that?
I share the concern, I appears that a shortcut has been taken in the uClibc configuration.
OTOH, the topic of this thread has drifted away.
From the nilfs-utils point of view, depending on NPTL is necessary today, so the patch stands.
I propose to start a different thread, and merge this patch for nilfs-utils. Doing so would fix the current build problems, and when the uclibc problem eventually resolves, nilfs-utils too will be patched to not depend on NPTL.

Quoted 1 line

What do you think about that?

I agree. There were a few more unrelated comment on this patch. Please fix and repost.

baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Kurt Van Dijck

author

2018-01-05 13:20 UTC #15 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Quoted 3 lines

Hi Kurt,
On Fri, Jan 05, 2018 at 11:17:08AM +0100, Kurt Van Dijck wrote:

[...]

Quoted 2 lines

I agree. There were a few more unrelated comment on this patch. Please fix and repost.

Ok, now I got lost.
I cannot find any comments on this ('[PATCH 1/2] ... NPTL threads') patch other than the uclibc remark you had.

Can you indicate the timestamps of the other comments, so I can re-read them. I can't fix anything without those, isn't it?

Kind regards,
Kurt

Baruch Siach

2018-01-05 14:03 UTC #16 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Hi Kurt,

Quoted 11 lines · Kurt Van Dijck

Quoted 1 line · Kurt Van Dijck
[...]
Quoted 2 lines
I agree. There were a few more unrelated comment on this patch. Please fix and repost.
Ok, now I got lost.
I cannot find any comments on this ('[PATCH 1/2] ... NPTL threads') patch other than the uclibc remark you had.
Can you indicate the timestamps of the other comments, so I can re-read them. I can't fix anything without those, isn't it?

My comment are archived in the link below:

http://patchwork.ozlabs.org/patch/855530/

I refer to the part below the 'depends on' that this patch adds.

baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Waldemar Brodkorb

2018-01-07 20:08 UTC #17 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

In reply to Baruch Siach

Hi Baruch,
Baruch Siach wrote,

Quoted 6 lines

The UCLIBC_HAS_ADVANCED_REALTIME help text says that this options enables clock_nanosleep(). But with current code you must have NPTL enabled for clock_nanosleep(). So the help text is not correct.
This also means that architectures lack NPTL support can't have clock_nanosleep() at all. Is there a reason for that?

You are right. It seems there is no reason for it. clock_nanosleep is internally using a syscall.

I prepared attached patch and running the regression tests.

Do you like to test or review?

I cleaned up old code, which might be only required for Linux kernels older than 2.6.0, which I do not going to support anyway.

best regards
Waldemar
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-rt-cleanup-and-allow-to-build-for-linuxthreads.patch
Type: text/x-diff
Size: 11284 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20180107/62b2ebc0/attachment.patch>

Kurt Van Dijck

author

2018-01-05 19:56 UTC #18 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

In reply to Baruch Siach

Hey,

Quoted 25 lines · Kurt Van Dijck

Quoted 1 line
nilfs-utils use clock_nanosleep(), which comes with NPTL threads
What is your indication that this is the case? In the uClibc-ng config UCLIBC_HAS_REALTIME is selected by both linuxthreads and NPTL. When UCLIBC_HAS_REALTIME is enabled, UCLIBC_HAS_ADVANCED_REALTIME is also enabled by default. UCLIBC_HAS_ADVANCED_REALTIME provides clock_nanosleep().
Quoted 10 lines
Signed-off-by Kurt Van Dijck <[email protected]>
---
 package/nilfs-utils/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/nilfs-utils/Config.in +0 -0
index 0c6b7a0..9dda27c 100644
package/nilfs-utils/Config.in line 4
Quoted 4 lines
package/nilfs-utils/Config.in +1 -0
@@ -1,6 +1,7 @@
 config BR2_PACKAGE_NILFS_UTILS
 	bool "nilfs-utils"
 	depends on BR2_TOOLCHAIN_HAS_THREADS # sem_open()
+	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
NPTL implies threads support, so the NPTL dependency alone is enough.

Quoted 2 lines

You also need to update the comment.

IMHO, appending the comment 'sem_open()' to BR2_..._NPTL would only cause confusion, since sem_open() does not depend on BR2_..._NPTL.

Given the recent discussion about clock_nanosleep not really depending on BR2_..._NPTL, I suspect a change in the near future, so I hesitate to remove the BR2_..._THREADS dependency.

How exactly is the current patch wrong?
KConfig resolves this, better than any of us would, so I see no advantage in modifying the patch.

Quoted 3 lines

package/nilfs-utils/Config.in line 7
Quoted 3 lines
package/nilfs-utils/Config.in +0 -0
 	depends on BR2_USE_MMU # util-linux libmount, libblkid
 	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
 	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID

Kind regards,
Kurt

Thomas Petazzoni

2018-01-06 14:35 UTC #19 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

In reply to Kurt Van Dijck

Hello,

Quoted 12 lines · Kurt Van Dijck

nilfs-utils use clock_nanosleep(), which comes with NPTL threads
Signed-off-by Kurt Van Dijck <[email protected]>
---
 package/nilfs-utils/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/nilfs-utils/Config.in +0 -0
index 0c6b7a0..9dda27c 100644

package/nilfs-utils/Config.in line 7

Quoted 7 lines

package/nilfs-utils/Config.in +1 -0

@@ -1,6 +1,7 @@
 config BR2_PACKAGE_NILFS_UTILS
 	bool "nilfs-utils"
 	depends on BR2_TOOLCHAIN_HAS_THREADS # sem_open()
+	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
 	depends on BR2_USE_MMU # util-linux libmount, libblkid
 	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
 	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID

You forgot to update the Config.in comment accordingly. Also, I've updated the commit log to summarize why we keep both the threads *and* NPTL dependencies, even though it might look redundant.

Applied with those changes. Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Kurt Van Dijck

author

2018-01-06 20:32 UTC #20 lore raw

PATCH 1/2 nilfs-utils: need NPTL threads

Quoted 27 lines

Hello,
Quoted 12 lines · Kurt Van Dijck
nilfs-utils use clock_nanosleep(), which comes with NPTL threads
Signed-off-by Kurt Van Dijck <[email protected]>
---
 package/nilfs-utils/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/nilfs-utils/Config.in +0 -0
index 0c6b7a0..9dda27c 100644
package/nilfs-utils/Config.in line 7
Quoted 7 lines
package/nilfs-utils/Config.in +1 -0
@@ -1,6 +1,7 @@
 config BR2_PACKAGE_NILFS_UTILS
 	bool "nilfs-utils"
 	depends on BR2_TOOLCHAIN_HAS_THREADS # sem_open()
+	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # clock_nanosleep()
 	depends on BR2_USE_MMU # util-linux libmount, libblkid
 	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
 	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
You forgot to update the Config.in comment accordingly. Also, I've updated the commit log to summarize why we keep both the threads *and* NPTL dependencies, even though it might look redundant.

Quoted 1 line

Applied with those changes. Thanks!

Only now, after having seen the commit, I realise you all meant the 'comment' section below, and not the '# clock_nanosleep()' comment :-)

Thanks,
Kurt