In D19273#412540, @cem wrote:

In D19273#412458, @markj wrote:

Solve the race using a per-process generation counter which gets updated
before a tracepoint is removed from the hash table, and after it is
removed from the target process' vmspace.

We do the latter, but I think we're missing the former.

• Remove global.
• Make sure that the parent doesn't go away during the lookup.

I wasn't careful about preserving #ifdefs here: fasttrap has been virtually untouched upstream for the past 5 years, so once this commit is in I'm going to get rid of illumos ifdefs there in an attempt to make the code a bit easier to read.

Can you provide an example DIE where this occurs? Which compiler are you using?

There are some style bugs in the diff, but I'll hold off on commenting on them for the initial review. style(9) is a useful reference if you're not familiar with it.

In D19249#411899, @ken wrote:

I tested this out with a Seagate 8TB host aware drive:

{sm4u-1-mgmt:/usr/home/kenm:!:0} camcontrol inquiry da14 -v
pass14: <ATA ST8000AS0022-1WL ZN03> Fixed Direct Access SPC-4 SCSI device
pass14: Serial Number Z84003SK
pass14: 600.000MB/s transfers, Command Queueing Enabled

It produces the same results as before with zonectl -c rz

In D19249#411871, @asomers wrote:

This change looks good. However, zonectl(8) currently hard-codes the entries_allocated field and doesn't check for a short return. If the kernel truncates the zone list, the user will have no obvious way of knowing. I think zonectl(8) should be modified to either loop until all zones have been reported, or at least tell the user that not all zones were reported.

In D19226#411357, @kib wrote:

In D19226#411354, @markj wrote:

It looks like this is assuming that the old value is not valid, but that's not always true. For example, pmap_promote_pde() simply overwrites the old PDE.

I don't really see how pmap_kextract() is safe wrt torn writes.

It could only work for user pmaps, so I can check this. The pmap lock must be owned there.
For promotions, we can safely write zero then write the update, I believe.

It looks like this is assuming that the old value is not valid, but that's not always true. For example, pmap_promote_pde() simply overwrites the old PDE.

In D19224#411311, @markj wrote:

In D19224#411306, @kib wrote:

Why cannot the same happen for read side ?

pipe_kqfilter() does not handle EVFILT_READ and EVFILT_WRITE symmetrically. In the read case, the knote itself holds a reference on that end of the pipe, so pipeclose() will never be called.

In D19224#411306, @kib wrote:

Why cannot the same happen for read side ?

Sorry for taking a while to get back to this. I have an alternate patch at D19216.

• Return the error from copyin(9).

The code changes look ok to me. Do you plan to upload man page changes here too?

I think you uploaded the diff between the first and second revision rather than the revision as a whole - could you upload the full diff?

Committed in r344106.

Committed in r344106.

Committed in r344106.

Committed in r344106.

Committed in r344106.

In D19148#409749, @pfg wrote:

In D19148#409628, @mav wrote:

In D19148#409623, @pfg wrote:

I thought of that but I saw a -1 when calculating it:

hash->uh_hashmask = hash->uh_hashsize - 1;

.A hashsize of zero makes little sense but it is still a valid value (?).

hashsize of zero makes no sense to me. I haven't checked how exactly it is initialized and whether it is used for some implementation reasons, but smallest possible size of hash is 1 with mask of 0. For size of 0 there is just no meaningful mask. Plus few places where uh_hashmask is used, do not care whether it is negative or overflown unsigned.

A zero value would come from an error or a malicious user.

Sorry it took a while to get to this. Most of my comments are style nits.

Address jhb's comments.

• Assert that we don't batch operations on unmanaged pages.
• Reword a comment.