• Add compiler version guards.

Ed pointed out that we may not want absolute paths in general. That's probably true, but I'll note that this change just makes things consistent for now. I think mapping the symlink paths is a no-brainer, at least.

In D19500#419336, @imp wrote:

In D19500#419197, @markj wrote:

In D19500#418483, @imp wrote:

Here's what I had come up with for a glibc compatable header. Feel free to steal any or all of this. Foundation copyright is fine.

I didn't bother to fix the namespace pollution when I did this, but I was never targeting MESA. I did this to save me from hacking a dozen places in nvme-cli. The pollution was fine for the nvme-cli.

For now I'm just trying to create a minimal header that survives an exp-run; we can always add to it.

OK. If we're doing an exp run, then maybe we should include my stuff so we can do just one. It was done by examining a number of different versions of endian.h to figure out what Linux users want to the largest extent possible w/o violating POLA...

I'm writing a few test cases for mlock as part of the user wiring work. I added one which does the following:

In D19615#420052, @emaste wrote:

Assuming it was this report panic: invalid dst page 0xfffff8007f14af98 https://syzkaller.appspot.com/bug?id=de0b1fa4863a64133877eb032d3ef876cb70d00a, eventual commit message should contain Reported-by: syzbot+514d40ce757a3f8b15bc@syzkaller.appspotmail.com

This is broken in a couple of ways:

• bn += blockcnt isn't right if bn is has a partial offset into the indirect block.
• The innermost loop contains a severe bug: it doesn't update ap->in_lbn for the remaining indirect block descriptors, so we may load an indirect block at the wrong logical offset in the vnode. This can cause data corruption if the vnode is subsequently modified.

This was committed in r345244.

In D19598#419790, @mckusick wrote:

I would preface the actual condition "The nbp parameter is non-NULL when the mapping is for a block that contains data, one of an external data block, a direct block, or the final block in a chain of indirect blocks." before the correct clarification "If mapping an extended attribute block, nbp must point to a buffer for that block."

• Update based on Kirk's comment.

• Describe nbp, runb and runp parameters per kib's suggestion.

• Return ENXIO if the offset is negative. Upper layers do not check this for us.
• Don't use ufs_getlbns() to map direct blocks.
• Assert num == 0 when we break from the loop.

In D19599#419628, @markj wrote:

In D19599#419613, @kib wrote:

This looks still inefficient, you do a lot of calculations for each sequential block number. Right now you getblk() indirect blocks for each possible bn. As a hypothetical optimal implementation, you can read indirect blocks and scan them until you find non-zero block pointer. Then you can reconstruct the offset (or keep the offset calculation in parallel with sliding the cursor over the blocks).

Hmm, that's what my implementation does. (My first implementation was indeed inefficient in the way that you described.) The innermost for-loop scans the current indirect block looking for a non-zero entry.

In D19599#419613, @kib wrote:

This looks still inefficient, you do a lot of calculations for each sequential block number. Right now you getblk() indirect blocks for each possible bn. As a hypothetical optimal implementation, you can read indirect blocks and scan them until you find non-zero block pointer. Then you can reconstruct the offset (or keep the offset calculation in parallel with sliding the cursor over the blocks).

• Readd a comment explaining the return value.

• Rename ufs_readmeta() to ufs_readindir().

In D19500#418483, @imp wrote:

Here's what I had come up with for a glibc compatable header. Feel free to steal any or all of this. Foundation copyright is fine.

I didn't bother to fix the namespace pollution when I did this, but I was never targeting MESA. I did this to save me from hacking a dozen places in nvme-cli. The pollution was fine for the nvme-cli.

• Don't define bswap*() in <endian.h>.

In D19500#418232, @zeising wrote:

In D19500#418222, @markj wrote:

So, I don't yet have a good solution for the mesa build. Hiding the bswap* declarations behind an #if __BSD_VISIBLE guard didn't fix the problem there.

I'm inclined to think that because mesa is already including machine/endian.h, it "knows what it's doing" and should perhaps continue to do that instead of including endian.h.

I can probably make it do that, or use the system bswap32. However, it would not surprise me that this crops up elsewhere, since Linux does not define bswap*().

So, I don't yet have a good solution for the mesa build. Hiding the bswap* declarations behind an #if __BSD_VISIBLE guard didn't fix the problem there.

Introduce a wrapper and define __BYTE_ORDER etc.

In D19500#417600, @markj wrote:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236398

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236398

In D19500#417356, @emaste wrote:

Fine with me; should we have an exp-run?

In D19496#417321, @ngie wrote:

Two questions:

• is there a test case for the EBUSY behavior?

In D19496#417290, @jhb wrote:

This looks right to me. The only change I had locally is that I had added a helper function in thr_umtx.h since that seemed to match the style of other code in libthr:

static inline int
_thr_ucond_has_waiters(struct ucond *cv)
{
    return (cv->c_has_waiters != 0);
}

and then used that instead of looking at cvp->kcond directly. I don't mind either way though.

In D19390#416928, @kib wrote:

1. To get accurate user wiring accounting at the pmap layer, you need both the page flag (PG_USER_WIRED ?) and the mapping flag (PG_W), or yet another counter on the page. This seems to be too intrusive for such small feature.

• Change man page wording.
• Call the flag KVME_FLAG_USER_WIRED; we do not set it if the mapping is wired by vslock().

In D19390#416570, @markj wrote:

IMO the concept should be defined at the mapping level, not at the level of the physical page; to effect wiring, the VM can use the reference counter to ensure that the page is not reclaimable, and the PG_USER_WIRED flag caches the information that at least one non-kernel mapping of the page is wired. This is similar in principle to, e.g., PGA_WRITEABLE.

In D19390#416489, @kib wrote:

[I am not suggesting to redo the patch, only discussing the possible approaches].

In D19227#415998, @noah.bergbauer_tum.de wrote:

Feel free to break the change into multiple patches. The change to use a TAILQ could certainly be committed on its own as a "no functional change" diff.

Not sure how to do this? My current workflow is copy-pasting the output of svnlite diff into phabricator. I'm not aware of a convenient way to split up diffs like this.

In D19265#415876, @cem wrote:

In D19265#415826, @markj wrote:

Ah, I assumed this was a port from OneFS. I'll try running the suite this afternoon.

Thanks!

In D19265#415623, @cem wrote:

In D19265#415545, @markj wrote:

In D19265#414685, @cem wrote:

In D19265#413741, @markj wrote:

My notion is that any time a script refers to a constval args[n], libdtrace would emit the "load immediate" DIF instruction which loads the constant value into a register, and any time it refers to a non-constval args[n+1], libdtrace knows to emit DIF with the ABI argument index shifted to take into account the fact that args[n] isn't represented in the ABI.

That sounds pretty reasonable. Any idea how difficult it would be to implement?

In D18320#415796, @mhorne063_gmail.com wrote:

In D18320#415484, @markj wrote:

Am I right that we only write 4KB at a time?

Yes. I tried writing out more than one page at a time and it failed quite spectacularly, so this could be a good spot to revisit and introduce pmap_kenter_temporary() as we discussed on IRC.