You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Bojan Smojver <bo...@rexursive.com> on 2009/07/16 03:42:52 UTC
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
On Thu, 2009-07-16 at 01:39 +0000, bojan@apache.org wrote:
> Use more elaborate checks for epoll_create1, dup3 and accept4.
This requires review folks. There is code in the tests that can
potentially hang the configure process. It does work on my Fedora 11 box
(which has the functions) and on RHEL4 (which doesn't have them), but it
would be really great to test this on other platforms, especially other
Linux flavours.
--
Bojan
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2009-07-19 13:36:17 Bojan Smojver napisał(a):
> On Sun, 2009-07-19 at 08:18 +0200, Tollef Fog Heen wrote:
> > Since I'm one of those Debian people who wanted the runtime detection
> > in the past: No, we can't do that.
> >
> > The apr (and all other packages) are built by build daemons on various
> > different architectures. We, as the packagers of apr, don't control
> > those at all. So far, we have specified a minimum version and
> > disabled all features that need a newer kernel version, which
> > obviously isn't great either.
> >
> > So: +1 from me on doing runtime detection.
>
> Sorry, still not convinced. If a particular build system sucks, be that
> whichever distribution, then the build system should be fixed. I do not
> see why everyone should be penalised at run time for someone else's
> bugs.
+1.
AFAIK Gentoo has a good concept of build system where packages are built
directly on users' systems.
--
Arfrever Frehtes Taifersar Arahesis
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Bojan Smojver <bo...@rexursive.com>.
On Mon, 2009-07-20 at 08:13 +0200, Tollef Fog Heen wrote:
> You could force it on using apr_cv_epoll=yes ./configure and such, but
> ugh and eww.
Ah, yes. There is always that too (note to self: edit Rawhide spec file
when APR 1.3.7 gets released).
> Debian does have a minimum kernel version that's supported, and for
> Etch (the previous release), that was «anything 2.6». I don't know if
> there is a tighter supported version for the current and next release,
> and even if it is, it doesn't change the principle we're discussing
> here.
I'll lay off this discussion and leave it up to other folks to decide
what's appropriate.
--
Bojan
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Tollef Fog Heen <tf...@err.no>.
]] Bojan Smojver
| However, there one could have an option to configure that forces new
| functions (i.e. use AC_CHECK_FUNCS instead of the new elaborate tests),
| because Fedora packages are targeted for a particular release. Not sure
| if that is something Debian package do or not.
You could force it on using apr_cv_epoll=yes ./configure and such, but
ugh and eww.
Debian does have a minimum kernel version that's supported, and for Etch
(the previous release), that was «anything 2.6». I don't know if there
is a tighter supported version for the current and next release, and
even if it is, it doesn't change the principle we're discussing here.
--
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Bojan Smojver <bo...@rexursive.com>.
On Sun, 2009-07-19 at 21:52 +0200, Tollef Fog Heen wrote:
> Not doing run-time detection means that everybody using Debian and
> derived distros (including Ubuntu) will get the castrated version
> where any and all features of new kernel versions are not used, so not
> doing runtime detection also has a penalty for users.
Just for the reference, Debian is not the only distro in the same
predicament. Fedora, for instance, will have the same problem [1].
Packages for it are built on RHEL5, which doesn't have the most recent
kernel, so all these functions will not be detected either.
However, there one could have an option to configure that forces new
functions (i.e. use AC_CHECK_FUNCS instead of the new elaborate tests),
because Fedora packages are targeted for a particular release. Not sure
if that is something Debian package do or not.
[1]:
http://www.redhat.com/archives/rhl-devel-list/2009-January/msg01661.html
--
Bojan
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Tollef Fog Heen <tf...@err.no>.
]] Bojan Smojver
(no need to Cc me, I read the list, as my Mail-Followup-To says.)
| Sorry, still not convinced. If a particular build system sucks, be that
| whichever distribution, then the build system should be fixed. I do not
| see why everyone should be penalised at run time for someone else's
| bugs.
It's a trade-off of what you require of the machines running
applications vs what a supported platform is. Maybe a buildd daemon has
to run a newer kernel in order to support its SCSI controller, it
doesn't have to be a wish to get all new and shiny syscalls available.
Not doing run-time detection means that everybody using Debian and
derived distros (including Ubuntu) will get the castrated version where
any and all features of new kernel versions are not used, so not doing
runtime detection also has a penalty for users.
| I don't think the above says anything about building APR on one platform
| and then running it on another. I could be wrong, of course. And, as
| always, if other folks think doing runtime detection is the way to go,
| then we'll do that.
I wouldn't call a vaguely different kernel version a different platform,
but that might just be splitting hairs.
--
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Bojan Smojver <bo...@rexursive.com>.
On Sun, 2009-07-19 at 08:18 +0200, Tollef Fog Heen wrote:
> Since I'm one of those Debian people who wanted the runtime detection
> in the past: No, we can't do that.
>
> The apr (and all other packages) are built by build daemons on various
> different architectures. We, as the packagers of apr, don't control
> those at all. So far, we have specified a minimum version and
> disabled all features that need a newer kernel version, which
> obviously isn't great either.
>
> So: +1 from me on doing runtime detection.
Sorry, still not convinced. If a particular build system sucks, be that
whichever distribution, then the build system should be fixed. I do not
see why everyone should be penalised at run time for someone else's
bugs.
I'll quote from APR project page:
-------------------
The mission of the Apache Portable Runtime (APR) project is to create
and maintain software libraries that provide a predictable and
consistent interface to underlying platform-specific implementations.
The primary goal is to provide an API to which software developers may
code and be assured of predictable if not identical behaviour regardless
of the platform on which their software is built, relieving them of the
need to code special-case conditions to work around or take advantage of
platform-specific deficiencies or features.
-------------------
I don't think the above says anything about building APR on one platform
and then running it on another. I could be wrong, of course. And, as
always, if other folks think doing runtime detection is the way to go,
then we'll do that.
--
Bojan
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Tollef Fog Heen <tf...@err.no>.
]] Bojan Smojver
| On Thu, 2009-07-16 at 09:27 +0100, Joe Orton wrote:
| > In the past I have argued we should never do runtime platform feature
| > detection, i.e., if accept4() works at build-time, we can presume it
| > works at run-time. I think I need to soften that position now; use of
| > a modern userspace on an older kernel seems to be very widespread in
| > Xen hosting sites. I shipped socket(,SOCK_CLOEXEC) support in neon
| > and got lots of complaints, from users across the spectrum of
| > distributions.
|
| Shouldn't these people build their runtime using the older kernel then?
Since I'm one of those Debian people who wanted the runtime detection in
the past: No, we can't do that.
The apr (and all other packages) are built by build daemons on various
different architectures. We, as the packagers of apr, don't control
those at all. So far, we have specified a minimum version and disabled
all features that need a newer kernel version, which obviously isn't
great either.
So: +1 from me on doing runtime detection.
--
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Jul 16, 2009 at 07:05:46PM +1000, Bojan Smojver wrote:
> On Thu, 2009-07-16 at 09:27 +0100, Joe Orton wrote:
> > In the past I have argued we should never do runtime platform feature
> > detection, i.e., if accept4() works at build-time, we can presume it
> > works at run-time. I think I need to soften that position now; use of
> > a modern userspace on an older kernel seems to be very widespread in
> > Xen hosting sites. I shipped socket(,SOCK_CLOEXEC) support in neon
> > and got lots of complaints, from users across the spectrum of
> > distributions.
>
> Shouldn't these people build their runtime using the older kernel then?
The problem is that these Xen hosting sites use a specific guest kernel
which they have tuned/customised/whatever. Then they let people run an
otherwise vanilla Linux distribution on top of that.
> It is bad enough that we need to do "will it really work" gymnastics at
> build time. If we go down the road of runtime checks, we'll soon be
> testing for all kinds of stuff, wasting lots of CPU cycles.
I know, I totally agree. The only mitigation is that those who waste
CPU cycles are those who run with mismatched kernel/userspace. It's the
added complexity which scares me the most.
Maybe let's just do a release with this code as-is, and see how many
people complain, and then revisit the decision if necessary.
> PS. This is obviously some kind of Xen bug (i.e. inability to run the
> latest kernel). Shouldn't they be fixing it?
I think this situation arose because of the historical lack of a stable
hypervisor/guest interface for running paravirt. guests, which did get
fixed more recently ("VMI"?), but, I'm not really an expert here, to say
the least.
Regards, Joe
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2009-07-16 at 09:27 +0100, Joe Orton wrote:
> In the past I have argued we should never do runtime platform feature
> detection, i.e., if accept4() works at build-time, we can presume it
> works at run-time. I think I need to soften that position now; use of
> a modern userspace on an older kernel seems to be very widespread in
> Xen hosting sites. I shipped socket(,SOCK_CLOEXEC) support in neon
> and got lots of complaints, from users across the spectrum of
> distributions.
Shouldn't these people build their runtime using the older kernel then?
It is bad enough that we need to do "will it really work" gymnastics at
build time. If we go down the road of runtime checks, we'll soon be
testing for all kinds of stuff, wasting lots of CPU cycles.
PS. This is obviously some kind of Xen bug (i.e. inability to run the
latest kernel). Shouldn't they be fixing it?
--
Bojan
Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Jul 16, 2009 at 11:42:52AM +1000, Bojan Smojver wrote:
> On Thu, 2009-07-16 at 01:39 +0000, bojan@apache.org wrote:
> > Use more elaborate checks for epoll_create1, dup3 and accept4.
>
> This requires review folks. There is code in the tests that can
> potentially hang the configure process. It does work on my Fedora 11 box
> (which has the functions) and on RHEL4 (which doesn't have them), but it
> would be really great to test this on other platforms, especially other
> Linux flavours.
The accept4() configure test looks a bit overcomplicated; it would be
possible to simplify a bit to avoid the fork etc, by creating a socket
with SOCK_NONBLOCK set, and simply calling accept4() and presuming the
syscall is wired up if it returns EAGAIN.
But, on the issue of runtime/build-time feature detection:
In the past I have argued we should never do runtime platform feature
detection, i.e., if accept4() works at build-time, we can presume it
works at run-time. I think I need to soften that position now; use of a
modern userspace on an older kernel seems to be very widespread in Xen
hosting sites. I shipped socket(,SOCK_CLOEXEC) support in neon and got
lots of complaints, from users across the spectrum of distributions.
So I fear we should probably do the same thing in APR. Any other
opinions? (Debianites, feel free to send me the hate mail now ;)
It would simplify the configure script, e.g. for accept4 support we
could use simply AC_CHECK_FUNCS and change apr_socket_accept something
like this (completely untested):
Thoughts?
Index: network_io/unix/sockets.c
===================================================================
--- network_io/unix/sockets.c (revision 790537)
+++ network_io/unix/sockets.c (working copy)
@@ -230,14 +230,21 @@
apr_status_t apr_socket_accept(apr_socket_t **new, apr_socket_t *sock,
apr_pool_t *connection_context)
{
- int s;
+ int s, flags;
apr_sockaddr_t sa;
sa.salen = sizeof(sa.sa);
#ifdef HAVE_ACCEPT4
+ /* Attempt to use accept4() but fall back on accept(). */
+ flags = FD_CLOEXEC;
s = accept4(sock->socketdes, (struct sockaddr *)&sa.sa, &sa.salen, SOCK_CLOEXEC);
+ if (s < 0 && errno == ENOSYS) {
+ flags = 0;
+ s = accept(sock->socketdes, (struct sockaddr *)&sa.sa, &sa.salen);
+ }
#else
+ flags = 0;
s = accept(sock->socketdes, (struct sockaddr *)&sa.sa, &sa.salen);
#endif
@@ -321,10 +328,10 @@
(*new)->local_interface_unknown = 1;
}
-#ifndef HAVE_ACCEPT4
- {
- int flags;
-
+ /* Toggle the CLOEXEC bit on. Note that this will be done either
+ * if no accept4() support was present, or, if it was present but
+ * failed with ENOSYS. */
+ if (flags == 0) {
if ((flags = fcntl((*new)->socketdes, F_GETFD)) == -1)
return errno;
@@ -332,7 +339,6 @@
if (fcntl((*new)->socketdes, F_SETFD, flags) == -1)
return errno;
}
-#endif
(*new)->inherit = 0;
apr_pool_cleanup_register((*new)->pool, (void *)(*new), socket_cleanup,