In D59323#1361227, @kib wrote:

Are you stating that libcapser does not waitpid() on the resulting pid after pdfork()? If yes, the proper solution is to add PD_NOWAITPID to pdfork() call. This should fix it for all consumers.

Yes, but this is a kernel regression no? I hadn't realized that pdfork() children are reaped automatically upon close().

Are you stating that libcapser does not waitpid() on the resulting pid after pdfork()? If yes, the proper solution is to add PD_NOWAITPID to pdfork() call. This should fix it for all consumers.

Invert meaning of -y.
Fix new_proc() prototype.
Remove #if 0 block.
Initialize np var on new process.

Remove one more instance of 'Y'.

Use caph_enter().
Remove -Y option.
Update man page for -y.

In D58094#1361090, @markj wrote:

Why is capability mode optional?
Why not use procdescs unconditionally?

Rebase

In D59269#1360943, @jrtc27 wrote:

In D59269#1360942, @kib wrote:

In D59269#1360939, @jrtc27 wrote:

Regarding leaking, glibc does seem to store them in a hash table tied to link_map (i.e. Obj_Entry) so will free on dlclose, unlike our arm64 implementation. Linked list seems a bit unnecessarily slow, but at the same time extensive dynamic TLSDESC + dlclose is probably not common enough to actually benefit from something like a reallocarray-based storage.

The list is only traversed on dlclose() to free items, so I do not see it as causing any slowdown. In fact, where the hashed lookup would be useful?

I don't understand why they use a hash table to uniquify their dynamic tlsdesc structs. I only note that it's a less cache-hating data structure, but for our implementation if we wanted that we'd just use an array of pointers. But I suppose you still have the dynamic tlsdesc structs scattered throughout memory so pointer chasing the linked list doesn't really make it much worse, beyond a bit of extra data dependency. So I think the linked list is fine.

In D59269#1360939, @jrtc27 wrote:

Regarding leaking, glibc does seem to store them in a hash table tied to link_map (i.e. Obj_Entry) so will free on dlclose, unlike our arm64 implementation. Linked list seems a bit unnecessarily slow, but at the same time extensive dynamic TLSDESC + dlclose is probably not common enough to actually benefit from something like a reallocarray-based storage.

In D59269#1360938, @jrtc27 wrote:

Looking at the glibc source, they do have an equivalent to the dtv_gen field, but the fast path check is whether the DTV's generation is >= that field, not ==. I think this is a time optimisation that avoids the need to access the global generation in order to do the check, instead just using a copy that's more to hand, and also lets you avoid taking the slow path for DTV entries that don't actually need it, even though the generation counter is ahead (e.g. if you dlopen A, use TLS for A, dlopen B, then use TLS for A again, you can still use the fast path, but ye olde __tls_get_addr doesn't know that, so will pessimistically update the DTV for B's load).

Reword the comment about rtld_bind_lock.

amd64: provide the fast path for dynamic TLSDESC resolver

In D59288#1360671, @kfv wrote:

Is it reasonable to provide the inline versions of these methods for complicated cases, like x86? Could it be always a function call?

Sorry, I forgot to answer this one. Well, I think keeping the current split might be preferable; C inlines the CSR ops the same way fesetround() does, and C++ already takes the libm call. IMHO, fegetmode()/fesetmode() sit with that group, not with fegetenv()—please correct me if I'm wrong or missing something. That said, always calling libm would be fine too if you’d rather keep the complicated x86 sequence out of the header. I'm OK either way.

You could memset only device_name then, right before strcpy(). It would be more clear IMHO.

In D59282#1360563, @markj wrote:

Why not use atomics?

In D59288#1360218, @kib wrote:

Is it reasonable to provide the inline versions of these methods for complicated cases, like x86? Could it be always a function call?

Let me formulate what I tried to say, differently. There should be no abi symlink pass, or, if you prefer such wording, there must be at least one swich from the ABI namespace to native namespace. If we get ENOENT and the failed component was provided by an absolute symlink, then we should restart native-rooted lookup from the symlink resolved and the rest of the unprocessed path. If we successfully resolved symlink target in the ABI root and ENOENT came later, it is not the reason to restart with the symlink in native root.

Is it reasonable to provide the inline versions of these methods for complicated cases, like x86? Could it be always a function call?

Please ask @emaste to merge.

This is a special case that should be not handled by the generic VFS lookup code. Ideally, it should be contained to the fdescfs vop_lookup(). I am not sure how, the obvious idea to change the vnode return type when !ISLASTCN has some drawbacks, at very least the vnode must be reclaimed on deactivation, might be it would work then.

In D58855#1359639, @dteske wrote:

In D58855#1359474, @kib wrote:

How this behaves when there is more than one symlink in the path to resolve?

Correctly. Same as one abs symlink.

Only the latest absolute target is kept for a possible native fallback; on ENOENT that target is retried once from the native root, then any further symlinks follow normal rules. Relative symlinks resolve as usual — they just do not affect that fallback.

Remove dtv_gen from tlsdesc_dynarg. There is no point of storing the generation there for our dtv update structure.
Add comment explaining the use of rtld_bind_lock in rtld_get_addr_slow().

I looked at the implementation for aarch64 and I see some issues that I do not understand.

Fix dta/dtv tls_gen check consistency.

In D59034#1357245, @brooks wrote:

In D59034#1354663, @kib wrote:

Why passing struct sendile_args to kern_sendfile()? This is not how all other kern_* wrappers are done, and IMO is worse than flattening the args. You have to explicitly initialize the literal anyway, which makes it equivalent to list the function's argument in the call.

It's the lowest churn change since that's how sendfile() worked, but I could flatten the arguments (or use a different structure that would benefit slightly from default initialization.)

How this behaves when there is more than one symlink in the path to resolve?

Rebase.
Use pdptrace(2) without syscall wrapper, now that the syscall is upstreamed.

pdtrace.2: mention that pdtrace() does not clear errno unlike ptrace()
ptrace.2: cross-ref pdtrace(2)
pdwait tests: adjust for addition of CAP_PTRACE

Move ptrace_check_allowed() call into ptrace_useraction().
Fix sysctl mib name in ptrace.2.

In D59115#1357897, @mmel wrote:

I'm also fine with it, thanks. Is the device_get_softc_class() function OK? That's actually the one I had the most trouble with.

Man page editing.

Renamed to ptrace_useraction().
Fixed tid.
Fixed arguments to ptrace_action() in kern_ptrace().
Man page editing.

In D59115#1356589, @mmel wrote:

kobj_total_data_size looks good — thanks!
kobj_instance_data_offset seems too long to me, and 'instance' is not entirely precise. Can we shorten it to kobj_data_offset?

I believe this is fine for the next batch, my patch requires more work.
[Cannot accept due to the review state]

Fix signed/unsigned comparison.
Report error on too long ABI name.
Man page improvements.

In D59115#1356439, @mmel wrote:

This is one of my dilemmas. KOBJ consistently uses 'baseclass', but 'kobj_offsetof_baseclass' or 'kobj_baseclass_offsetof' looks ugly, especially since these could also be used for the main/leaf class.

What about 'kobj_offsetof_class' and 'kobj_sizeof_class'?

In D59115#1356428, @mmel wrote:

In D59115#1355807, @kib wrote:

kobj should not mention 'softc', it is not limited to the driver support. It should be named more generally.

Completely agree, but despite my best efforts, I can't figure out which word to use instead of “softc”

This is fine, but I think that we can avoid the trouble by locking the proctree_lock around the ioctl handler instead. Please see D59132

In D58337#1355750, @adrian wrote:

So hm, yay or nay on this?

kobj should not mention 'softc', it is not limited to the driver support. It should be named more generally.