In D37102#845792, @kib wrote:

Perhaps you do not get responses before proposed semantic is quite strange.

Taking the liberty of subscribing a couple of people who've been active in kern_event.c, to see if they have any flames/feedback/ideas for me or might be able to suggest who could be interested in this area...

You mean conceptually, or the performance I get on my drive? I have to admit that I only got as far as finding 5.27.1.4 in the NVMe base spec and realising that I want to send that in a Set Features command, and seeing that there is a patch D32700 that might make that easier, but not figuring out how to do it with unpatched nvmecontrol (I guess maybe admin-passthru and some raw bytes?). Got an example command?

More context.

In fact, if aio_error and aio_return are backwards compatible in that way, could we also create new versions of aio_write, etc that set AIO_KEVENT_FLAG_REAP automatically? That way existing programs could reap the benefits of your work with no code changes, just a recompile.

Split into two, moved the rusage stuff to D33271.

LGTM. I tried it with my aio_fsync/aio_waitcomplete-heavy workload and saw no problem (though that isn't saying much, as I've only ever seen ERELOOKUP/panic twice).

Right. Thanks!

Reuploaded patch with -U99999, no change.

-U999999

In D29757#668704, @kib wrote:

This all looks fine to me, two notes:

1. Please split the test into two

Added a test to show that we don't report POLLRDHUP unless you asked for it.

Ok, now ignoring POLLINIGNEOF for this.

This time with -U999999, and mentioning illumos as requested.

Looks OK to me. I suppose we could also use bitmask style for LIO_SYNC and LIO_DATA_ONLY?

Updated to address review feedback and fix oversights in man page.

Rebased.

Rebased. Previous confusion turned out to be explained by problems fixed by D27353 and problems in my test methodology.

Rebased (mostly: ZFS moved).

Great feature. +1 for allowing it in lio_listio() for batched submissions in later work (I'd also like to see LIO_FSYNC allowed in there too). I wonder if it is OK to use C11 anon union syntax in a system header; I see that sys/ucred.h does that.

In D27518#615127, @asomers wrote:

Thanks for doing this. It looks like it could be pretty helpful. Helpful enough that I may need to learn to start using truss.

Dropped the SIGEV_THREAD function pointer. Switched to a hardcoded limit on how many lio_listio array elements to display, following a coding pattern from nearby similar things.

In D27518#615067, @asomers wrote:

This looks like a good change, but I'm not a truss user. I use ktrace instead. With this patch, how does the output of truss compare to ktrace?

Added aio_mlock(2).

Tested, LGTM.

Hmm. My filesystem has 32kb blocks, and this generates reads, according to dwatch -X io:

In D25024#551329, @kib wrote:

I think that currently f_nextoff is often redundant, and this can be used to avoid further extending the size of the struct. Most of the time, I believe, f_nextoff == f_offset. You can use this to consider both f_offset/f_nextoff as the pointers to two streams of accesses. More, it could serve a situation where you have e.g. two read streams, instead of current patch insistence of one stream being read and one write.

In D25071#552096, @kib wrote:

I believe that my main objection against defining O_DSYNC as O_SYNC was that it de-facto provides some intentionally wrong ABI. Defining O_DSYNC as you did, and converting it to IO_DSYNC or better IO_DATASYNC is fine even if no filesystem handle the flag differently from IO_SYNC ATM.

Thanks for the feedback!

Handled feedback from asomers@: added a test, added a missing check in aio_queue_file(), used -U999999 for more context. I also created a separate review ticket D25090 to add O_DSYNC for open(2) and fcntl(2). The present patch is rebased on top of that one.

Thanks! Fixed.

This seems correct at a high level. It's required to read the data back. (For what it's worth, failure to distinguish between data integrity meta-data relating to file extension and frippery like mtime seems to have been a problem on other popular OSs' fdatasync(2) implementations in the past, which I think is what leads Postgres to trust fdatasync(2) only when it knows there has been a full fsync(2) since the last file size change. It would be cool to be able to save on some I/Os be being less paranoid.)

Discussed with Mateusz, he would prefer to write a Coccinelle script to automate this change, so I am abandoning this patch.

Thanks for the reminder. I will commit this later today.