Implement CloudABI's readdir().

[ed]

Summary

CloudABI's readdir() system call could be thought of as a mixture
between FreeBSD's getdents(2) and pread(). Instead of using the file
descriptor offset, userspace provides a 64-bit cloudabi_dircookie_t
continue reading at a given point. CLOUDABI_DIRCOOKIE_START, having
value 0, can be used to return entries at the start of the directory.

The file descriptor offset is not used to store the cookie for the
reason that in a file descriptor centric environment, it would make
sense to allow concurrent use of a single file descriptor.

The remaining space returned by the system call should be filled with a
partially truncated copy of the next entry. The advantage of doing this
is that it gracefully deals with long filenames. If the C library
provides a buffer that is too small to hold a single entry, it can still
extract the directory entry header, meaning that it can retry the read
with a larger buffer or skip it using the cookie.

Test Plan

This implementation passes the cloudlibc unit tests at:

https://github.com/NuxiNL/cloudlibc/tree/master/src/libc/dirent

[kib]

sys/compat/cloudabi/cloudabi_file.c

292 ↗

(On Diff #7431)

This is weird. If ever testing for VDIR, the test should be done after the vnode is locked. Otherwise, the vnode could be reclaimed meantime and become VBAD immediately after the test.

But you do not test for the reclaimed vnode after obtaining the lock, so why testing for the vnode type ?

311 ↗

(On Diff #7431)

Note that in very tough memory loads, malloc() requires a free page, which requires pagedaemon to page out something and the page allocator sleeps waiting for a page. Calling malloc() while owning a vnode lock means that pagedaemon cannot page out that vnode' pages. More, pagedaemon would spend time waiting for the lock timeout.

Move the malloc() call out of the vnode lock.

349 ↗

(On Diff #7431)

There, you call uiomove(9) (i.e. copyout(9)) while holding vnode lock. Since copyout may need to handle the page fault while accessing userspace, you are creating (wrong) lock ordering between vnode lock and VM locks.

[ed]

Don't hold the vnode lock across iterations.

sys/compat/cloudabi/cloudabi_file.c

292 ↗

(On Diff #7431)

That's a good point. To be honest, this code was somewhat inspired by getdents_common() in linux_file.c, which seems to have the same flaws. It looks like even kern_getdirentries() does this.

I've moved the test into the loop body.

311 ↗

(On Diff #7431)

Done!

Quick question: I didn't observe any run-time warnings/errors here as far as I know. If we really shouldn't call malloc(9) while holding a vnode lock, would it make sense to add a KASSERT() in some magical place to prevent this from happening?

349 ↗

(On Diff #7431)

It should be safe to iteratively pick up/drop the lock here. The only downside is that we now have to call mac_vnode_check_readdir() during each iteration, but I can live with that.

[kib]

Ok, do not have objections for the vfs calls, I did not looked at the ABI itself.