Remove latch.c workaround for Linux < 2.6.27

Started by Thomas Munroabout 5 years ago5 messageshackers
Jump to latest
#1Thomas Munro
thomas.munro@gmail.com

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+1-21
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thomas Munro (#1)
Re: Remove latch.c workaround for Linux < 2.6.27

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

#3Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Thomas Munro (#1)
Re: Remove latch.c workaround for Linux < 2.6.27

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

#4Thomas Munro
thomas.munro@gmail.com
In reply to: Heikki Linnakangas (#3)
Re: Remove latch.c workaround for Linux < 2.6.27

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).

#5Thomas Munro
thomas.munro@gmail.com
In reply to: Thomas Munro (#4)
Re: Remove latch.c workaround for Linux < 2.6.27

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.