Why the title says that the fix is for 'mapped' devices? What specifically is different there?

Also, what if there is more than one knote attached?

I think the proper change would be to patch chn_polltrigger to accept pointer to knote and if it's NULL, do what it does now, if it's not, do what this patch does, but that would require changing chn_pollreset and pulling in struct knote into channel.h which I'm not sure we want or not, so please advise.

I think now the patch is fully up to @christos opinion to take.

chn_polltrigger() has a case for CHN_F_MMAP, isn't it enough? And if not, why?

It's not about mmap, it's about kqueue. Look at the following code:

if (bs->prev_total < c->lw)
    delta = c->lw;
else
    delta = bs->total - bs->prev_total;

For this to report discrete events, prev_total must be reset to total after each firing. The select/poll path does exactly that inside chn_poll():

if (chn_polltrigger(c)) {
    chn_pollreset(c);
    ...
}

Kqueue cannot safely use that same per-channel reset for two reasons:

1. Activation/delivery race. If you reset prev_total inside dsp_kqevent() when KNOTE_LOCKED activates the knote, the event has not yet been delivered to userland. The next interrupt runs kn_scan() again, sees total - prev_total < lw, and drops the event.
2. Multiple knotes on one channel share prev_total. If two kevent registrations are watching the same channel, resetting prev_total for one knote prevents the other from ever seeing its own low-watermark edge.

So chn_polltrigger()'s mmap logic is correct, but it needs a per-caller counter. Select/poll uses the single per-channel prev_total because each poll call is a fresh, one-shot evaluation. Kqueue needs a separate counter per knote, which is kn_sdata.

In D57833#1327561, @meka_tilda.center wrote:

It's not about mmap, it's about kqueue. Look at the following code:

if (bs->prev_total < c->lw)
    delta = c->lw;
else
    delta = bs->total - bs->prev_total;

For this to report discrete events, prev_total must be reset to total after each firing. The select/poll path does exactly that inside chn_poll():

if (chn_polltrigger(c)) {
    chn_pollreset(c);
    ...
}

Kqueue cannot safely use that same per-channel reset for two reasons:

1. Activation/delivery race. If you reset prev_total inside dsp_kqevent() when KNOTE_LOCKED activates the knote, the event has not yet been delivered to userland. The next interrupt runs kn_scan() again, sees total - prev_total < lw, and drops the event.
2. Multiple knotes on one channel share prev_total. If two kevent registrations are watching the same channel, resetting prev_total for one knote prevents the other from ever seeing its own low-watermark edge.

So chn_polltrigger()'s mmap logic is correct, but it needs a per-caller counter. Select/poll uses the single per-channel prev_total because each poll call is a fresh, one-shot evaluation. Kqueue needs a separate counter per knote, which is kn_sdata.

That should definitely be documented in a comment.

Before I do that, let's get the code in perfect shape. I mean, should I change chn_polltrigger and chn_pollreset to handle mmap or should I leave it as it is now?

In D57833#1328671, @meka_tilda.center wrote:

Before I do that, let's get the code in perfect shape. I mean, should I change chn_polltrigger and chn_pollreset to handle mmap or should I leave it as it is now?

I would say leave it as is for now. I'm not sure if it's a good idea to have to pass the knote around.