Re: epoll_wait returning EFAULT on Linux 3.2.78

From: Andres Freund <andres(at)anarazel(dot)de>
To: Greg Stark <stark(at)mit(dot)edu>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: epoll_wait returning EFAULT on Linux 3.2.78
Date: 2016-06-02 18:22:44
Message-ID: 20160602182244.7dzhvtxnpq5s4kcb@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2016-06-02 19:15:07 +0100, Greg Stark wrote:
> Ok. I added a comment and also fixed a small typo.

> diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
> index 3fbe0e5..a96fb7b 100644
> --- a/src/backend/storage/ipc/latch.c
> +++ b/src/backend/storage/ipc/latch.c
> @@ -485,35 +485,40 @@ CreateWaitEventSet(MemoryContext context, int nevents)
> char *data;
> Size sz = 0;
>
> - sz += sizeof(WaitEventSet);
> - sz += sizeof(WaitEvent) * nevents;
> + /*
> + * struct epoll_event contains a uint64_t which on some architectures
> + * requires 8-byte alignment and just to be safe be prepared for future
> + * additions of other such elements later in the WaitEventSet structure
> + */

I'd rather phrase this roughly like:

Use MAXALIGN size/alignment to guarantee that later uses of memory are
aligned correctly. E.g. epoll_event might need 8 byte alignment on some
platforms, but earlier allocations like WaitEventSet and WaitEvent might
not sized to guarantee that when purely using sizeof().

> + sz += MAXALIGN(sizeof(WaitEventSet));
> + sz += MAXALIGN(sizeof(WaitEvent) * nevents);
>
> #if defined(WAIT_USE_EPOLL)
> - sz += sizeof(struct epoll_event) * nevents;
> + sz += MAXALIGN(sizeof(struct epoll_event) * nevents);
> #elif defined(WAIT_USE_POLL)
> - sz += sizeof(struct pollfd) * nevents;
> + sz += MAXALIGN(sizeof(struct pollfd) * nevents);
> #elif defined(WAIT_USE_WIN32)
> /* need space for the pgwin32_signal_event */
> - sz += sizeof(HANDLE) * (nevents + 1);
> + sz += MAXALIGN(sizeof(HANDLE) * (nevents + 1));
> #endif
>
> data = (char *) MemoryContextAllocZero(context, sz);
>
> set = (WaitEventSet *) data;
> - data += sizeof(WaitEventSet);
> + data += MAXALIGN(sizeof(WaitEventSet));
>
> set->events = (WaitEvent *) data;
> - data += sizeof(WaitEvent) * nevents;
> + data += MAXALIGN(sizeof(WaitEvent) * nevents);
>
> #if defined(WAIT_USE_EPOLL)
> set->epoll_ret_events = (struct epoll_event *) data;
> - data += sizeof(struct epoll_event) * nevents;
> + data += MAXALIGN(sizeof(struct epoll_event) * nevents);
> #elif defined(WAIT_USE_POLL)
> set->pollfds = (struct pollfd *) data;
> - data += sizeof(struct pollfd) * nevents;
> + data += MAXALIGN(sizeof(struct pollfd) * nevents);
> #elif defined(WAIT_USE_WIN32)
> set->handles = (HANDLE) data;
> - data += sizeof(HANDLE) * nevents;
> + data += MAXALIGN(sizeof(HANDLE) * nevents);
> #endif

You can't easily test WIN32, but I'd suggest running make check with
WAIT_USE_POLL and WAIT_USE_SELECT at the top.

Looks sane otherwise.

Greetings,

Andres Freund

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-06-02 18:28:19 Re: epoll_wait returning EFAULT on Linux 3.2.78
Previous Message Greg Stark 2016-06-02 18:15:07 Re: epoll_wait returning EFAULT on Linux 3.2.78