epoll: edge-triggered descriptors lost their edges on more than one CPU
EPOLLET reported a descriptor again only after a scan had seen it not ready. Xorg watches its clients edge-triggered; when it read part of a client's requests and more arrived before its next scan, the socket never looked not-ready, so it was never reported again -- X waiting on the client, the client waiting on X. One CPU almost never hit it; on a 16-thread laptop the desktop froze within seconds: Window Maker's menu stuck, xterm never mapped, text gone, and Caps Lock dead because X owns the LED. Now a still-ready descriptor is reported again whenever anything has signalled readiness since its last report (poll_generation()), which may be spurious, as EPOLLET permits, and is never missed. Reproduced and verified with QEMU at -smp 8: the root menu works and xterm runs cmatrix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1giNvrHTrPMPw7V6x1kQ4
This commit is contained in:
parent
54b25708c3
commit
4743df8c4a
3 changed files with 25 additions and 2 deletions
|
|
@ -735,6 +735,7 @@ int net_detach(struct device *);
|
||||||
int net_attach_late(struct device *);
|
int net_attach_late(struct device *);
|
||||||
int net_eth_poll(void);
|
int net_eth_poll(void);
|
||||||
void net_eth_link_check(void);
|
void net_eth_link_check(void);
|
||||||
|
uint poll_generation(void);
|
||||||
int net_eth_count(void);
|
int net_eth_count(void);
|
||||||
void net_eth_counts(int i, uint64 *rx, uint64 *tx);
|
void net_eth_counts(int i, uint64 *rx, uint64 *tx);
|
||||||
struct netif *net_eth_netif(int i);
|
struct netif *net_eth_netif(int i);
|
||||||
|
|
|
||||||
|
|
@ -82,6 +82,7 @@ struct epitem {
|
||||||
struct file *f; // checked against the table, never dereferenced alone
|
struct file *f; // checked against the table, never dereferenced alone
|
||||||
uint32 events;
|
uint32 events;
|
||||||
uint64 data;
|
uint64 data;
|
||||||
|
uint seq; // EPOLLET: poll_generation() when last reported
|
||||||
int was_ready; // for EPOLLET: report on becoming ready, not on being
|
int was_ready; // for EPOLLET: report on becoming ready, not on being
|
||||||
int disabled; // EPOLLONESHOT fired, until EPOLL_CTL_MOD
|
int disabled; // EPOLLONESHOT fired, until EPOLL_CTL_MOD
|
||||||
};
|
};
|
||||||
|
|
@ -649,7 +650,8 @@ sys_epoll_ctl(void)
|
||||||
case EPOLL_CTL_ADD:
|
case EPOLL_CTL_ADD:
|
||||||
if (i >= 0) { r = -EEXIST; break; }
|
if (i >= 0) { r = -EEXIST; break; }
|
||||||
if (o->ep.n == EP_MAX) { r = -ENOSPC; break; }
|
if (o->ep.n == EP_MAX) { r = -ENOSPC; break; }
|
||||||
o->ep.items[o->ep.n++] = (struct epitem){ fd, tf, ev.events, ev.data, 0, 0 };
|
o->ep.items[o->ep.n++] = (struct epitem){ .fd = fd, .f = tf, .events = ev.events,
|
||||||
|
.data = ev.data };
|
||||||
break;
|
break;
|
||||||
case EPOLL_CTL_MOD:
|
case EPOLL_CTL_MOD:
|
||||||
if (i < 0) { r = -ENOENT; break; }
|
if (i < 0) { r = -ENOENT; break; }
|
||||||
|
|
@ -676,6 +678,9 @@ static int
|
||||||
ep_ready(struct evobj *o, struct epoll_event *out, int max)
|
ep_ready(struct evobj *o, struct epoll_event *out, int max)
|
||||||
{
|
{
|
||||||
int got = 0;
|
int got = 0;
|
||||||
|
// Taken before any descriptor is looked at, so a kick that lands during the
|
||||||
|
// scan is newer than what is recorded and is reported next time.
|
||||||
|
uint gen = poll_generation();
|
||||||
|
|
||||||
acquiresleep(&o->lock);
|
acquiresleep(&o->lock);
|
||||||
for (int i = 0; i < o->ep.n && got < max; ) {
|
for (int i = 0; i < o->ep.n && got < max; ) {
|
||||||
|
|
@ -699,12 +704,21 @@ ep_ready(struct evobj *o, struct epoll_event *out, int max)
|
||||||
it->was_ready = 0;
|
it->was_ready = 0;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if ((it->events & EPOLLET) && it->was_ready)
|
// Edge-triggered: an edge is new data, not only a not-ready-to-ready
|
||||||
|
// change. A descriptor still ready since it was last reported is
|
||||||
|
// reported again if anything has signalled since -- possibly spuriously,
|
||||||
|
// which EPOLLET allows; never missed, which it does not. Xorg watches its
|
||||||
|
// clients this way. Waiting for a not-ready scan in between lost the
|
||||||
|
// edge whenever X read part of a client's requests and more arrived
|
||||||
|
// before it looked again: on one CPU that almost never happened, on
|
||||||
|
// several it hung the desktop -- X waiting on the client, the client on X.
|
||||||
|
if ((it->events & EPOLLET) && it->was_ready && it->seq == gen)
|
||||||
continue;
|
continue;
|
||||||
if (out) {
|
if (out) {
|
||||||
out[got].events = (uint32)rev;
|
out[got].events = (uint32)rev;
|
||||||
out[got].data = it->data;
|
out[got].data = it->data;
|
||||||
it->was_ready = 1;
|
it->was_ready = 1;
|
||||||
|
it->seq = gen;
|
||||||
if (it->events & EPOLLONESHOT)
|
if (it->events & EPOLLONESHOT)
|
||||||
it->disabled = 1;
|
it->disabled = 1;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -47,6 +47,14 @@ poll_kick(void)
|
||||||
wakeup(&ticks);
|
wakeup(&ticks);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// How many kicks there have been. epoll's edge-triggered mode compares it:
|
||||||
|
// see ep_ready().
|
||||||
|
uint
|
||||||
|
poll_generation(void)
|
||||||
|
{
|
||||||
|
return __atomic_load_n(&poll_seq, __ATOMIC_ACQUIRE);
|
||||||
|
}
|
||||||
|
|
||||||
// Sleep one timer tick, or until a poll_kick, or until killed/signaled.
|
// Sleep one timer tick, or until a poll_kick, or until killed/signaled.
|
||||||
// Returns 1 if a tick went by, 0 if it was a kick -- which must not count
|
// Returns 1 if a tick went by, 0 if it was a kick -- which must not count
|
||||||
// against a timeout, or a timed poll on a busy network would end early --
|
// against a timeout, or a timed poll on a busy network would end early --
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue