Remove latch.c workaround for Linux < 2.6.27
Hello,
Commit 82ebbeb0 added a workaround for (already in 2017) ancient Linux
kernels with no EPOLL_CLOEXEC. I don't see any such systems in the
build farm today (and if there is one hiding in there somewhere, it's
well past time to upgrade). I'd like to rip that code out, because
I'm about to commit some new code that uses another 2.6.17+
XXX_CLOEXEC flag, and it'd be silly to have to write new workaround
code for that too, and a contradiction to have fallback code in one
place but not another. Any objections?
Attachments:
0001-Remove-latch.c-workaround-for-Linux-2.6.27.patchtext/x-patch; charset=US-ASCII; name=0001-Remove-latch.c-workaround-for-Linux-2.6.27.patchDownload
From 9c6ac992f60a904f8073e62b1b8085ff9df7ae0b Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sat, 27 Feb 2021 11:22:16 +1300
Subject: [PATCH] Remove latch.c workaround for Linux < 2.6.27.
Ancient Linux had no EPOLL_CLOEXEC flag, so commit 82ebbeb0 added a
separate code path with an fcntl() call. Kernels of that vintage are
long gone. Now seems like a good time to drop this extra code, because
otherwise we'd have to add similar workaround code to new patches using
XXX_CLOEXEC flags to avoid contradicting ourselves.
---
src/backend/storage/ipc/latch.c | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
index f2d005eea0..79b9627831 100644
--- a/src/backend/storage/ipc/latch.c
+++ b/src/backend/storage/ipc/latch.c
@@ -666,31 +666,12 @@ CreateWaitEventSet(MemoryContext context, int nevents)
/* treat this as though epoll_create1 itself returned EMFILE */
elog(ERROR, "epoll_create1 failed: %m");
}
-#ifdef EPOLL_CLOEXEC
set->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
if (set->epoll_fd < 0)
{
ReleaseExternalFD();
elog(ERROR, "epoll_create1 failed: %m");
}
-#else
- /* cope with ancient glibc lacking epoll_create1 (e.g., RHEL5) */
- set->epoll_fd = epoll_create(nevents);
- if (set->epoll_fd < 0)
- {
- ReleaseExternalFD();
- elog(ERROR, "epoll_create failed: %m");
- }
- if (fcntl(set->epoll_fd, F_SETFD, FD_CLOEXEC) == -1)
- {
- int save_errno = errno;
-
- close(set->epoll_fd);
- ReleaseExternalFD();
- errno = save_errno;
- elog(ERROR, "fcntl(F_SETFD) failed on epoll descriptor: %m");
- }
-#endif /* EPOLL_CLOEXEC */
#elif defined(WAIT_USE_KQUEUE)
if (!AcquireExternalFD())
{
@@ -736,7 +717,7 @@ CreateWaitEventSet(MemoryContext context, int nevents)
*
* Note: preferably, this shouldn't have to free any resources that could be
* inherited across an exec(). If it did, we'd likely leak those resources in
- * many scenarios. For the epoll case, we ensure that by setting FD_CLOEXEC
+ * many scenarios. For the epoll case, we ensure that by setting EPOLL_CLOEXEC
* when the FD is created. For the Windows case, we assume that the handles
* involved are non-inheritable.
*/
--
2.30.0
Thomas Munro <thomas.munro@gmail.com> writes:
Commit 82ebbeb0 added a workaround for (already in 2017) ancient Linux
kernels with no EPOLL_CLOEXEC. I don't see any such systems in the
build farm today (and if there is one hiding in there somewhere, it's
well past time to upgrade). I'd like to rip that code out, because
I'm about to commit some new code that uses another 2.6.17+
XXX_CLOEXEC flag, and it'd be silly to have to write new workaround
code for that too, and a contradiction to have fallback code in one
place but not another. Any objections?
I believe we've dropped support for RHEL5, so no objection here.
regards, tom lane
On 27 February 2021 01:10:23 EET, Thomas Munro <thomas.munro@gmail.com> wrote:
Hello,
Commit 82ebbeb0 added a workaround for (already in 2017) ancient Linux
kernels with no EPOLL_CLOEXEC. I don't see any such systems in the
build farm today (and if there is one hiding in there somewhere, it's
well past time to upgrade). I'd like to rip that code out, because
I'm about to commit some new code that uses another 2.6.17+
XXX_CLOEXEC flag, and it'd be silly to have to write new workaround
code for that too, and a contradiction to have fallback code in one
place but not another. Any objections?
What happens if you try to try to compile or run on such an ancient kernel? Does it fall back to something else? Can you still make it work with different configure options?
I'm just curious, not objecting.
- Heikki
On Sat, Feb 27, 2021 at 9:01 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 27 February 2021 01:10:23 EET, Thomas Munro <thomas.munro@gmail.com> wrote:
Commit 82ebbeb0 added a workaround for (already in 2017) ancient Linux
kernels with no EPOLL_CLOEXEC. I don't see any such systems in the
build farm today (and if there is one hiding in there somewhere, it's
well past time to upgrade). I'd like to rip that code out, because
I'm about to commit some new code that uses another 2.6.17+
XXX_CLOEXEC flag, and it'd be silly to have to write new workaround
code for that too, and a contradiction to have fallback code in one
place but not another. Any objections?What happens if you try to try to compile or run on such an ancient kernel? Does it fall back to something else? Can you still make it work with different configure options?
I'm just curious, not objecting.
With this patch, I guess you'd have to define WAIT_USE_POLL. I
suppose we could fall back to that automatically if EPOLL_CLOEXEC
isn't defined, if anyone thinks that's worth bothering with.
Even though Linux < 2.6.17 is not relevant, one thing I have wondered
about is what other current OSes might have copied Linux's epoll API
and get me into trouble by being incomplete. So far I have found only
illumos, based on googling about OSes that are in our build farm (my
idea of what OSes we support in some sense), and BF animal damselfly's
configure output seems to confirm that it does have it. Googling
tells me that it does seem to have the full modern version of the API,
so fingers crossed (looks like it also has signalfd() too, which is
interesting for my latch optimisation patch which assumes that if you
have epoll you also have signalfd).
On Sat, Feb 27, 2021 at 9:30 PM Thomas Munro <thomas.munro@gmail.com> wrote:
On Sat, Feb 27, 2021 at 9:01 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
What happens if you try to try to compile or run on such an ancient kernel? Does it fall back to something else? Can you still make it work with different configure options?
I'm just curious, not objecting.
With this patch, I guess you'd have to define WAIT_USE_POLL. I
suppose we could fall back to that automatically if EPOLL_CLOEXEC
isn't defined, if anyone thinks that's worth bothering with.
I thought about doing:
/* don't overwrite manual choice */
-#elif defined(HAVE_SYS_EPOLL_H)
+#elif defined(HAVE_SYS_EPOLL_H) && defined(EPOLL_CLOEXEC)
#define WAIT_USE_EPOLL... but on reflection, I don't think we should expend energy on a
desupported OS vintage that isn't present in our build farm, at least
not without a reasonable field complaint; I wouldn't even know if it
worked.
So, pushed without that.