A version with refcounted text references. vm_map_entry keeps such reference on the lowest object in the shadow chain, which must be vnode or tmpfs swap.

On kern_execve() error path, only call VOP_UNSET_TEXT() when it was set by current thread.

In D19923#428953, @mjg wrote:

I gave it a spin with poudriere and it panics:

panic: Assertion error_ == 0 failed at /tank/users/mjg/src/freebsd/sys/kern/kern_exec.c:623
cpuid = 59
time = 1555604110
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe056d49a650
vpanic() at vpanic+0x19d/frame 0xfffffe056d49a6a0
panic() at panic+0x43/frame 0xfffffe056d49a700
kern_execve() at kern_execve+0x12f3/frame 0xfffffe056d49aa50
sys_execve() at sys_execve+0x4c/frame 0xfffffe056d49aad0
amd64_syscall() at amd64_syscall+0x276/frame 0xfffffe056d49abf0
fast_syscall_common() at fast_syscall_common+0x101/frame 0xfffffe056d49abf0

• syscall (59, FreeBSD ELF64, sys_execve), rip = 0x8003d517a, rsp = 0x7fffffffe388, rbp = 0x7fffffffe4d0 ---

KDB: enter: panic
[ thread pid 12060 tid 101862 ]
Stopped at kdb_enter+0x3b: movq $0,kdb_why

This is the unset in interpreted case and is probably a race where 2 different threads are unsetting the flag.
I was working on shared exec myself few years back. I think both writers and execs have to be explicitly counted. Instead of using a dedicated flag to differentiate between them (or separate counts), in Linux they have a hack where negative value signifies execs.

In D19845#428854, @tychon wrote:

In D19845#428768, @greg_unrelenting.technology wrote:

Some more i915 GPU testing (w/o the latest update here): after using Firefox (opengl layers, xwayland) for some time, GPU resets start happening

drmn0: Resetting chip for stuck wait on rcs0
drmn0: Resetting chip for stuck wait on rcs0
drmn0: Resetting chip for stuck wait on rcs0
…
DMAR0: Fault Overflow
DMAR0: vgapci0: pci:0:2:0 sid 10 fault acc 0 adt 0x0 reason 0x5 addr 2e09000
DMAR0: Fault Overflow
DMAR0: vgapci0: pci:0:2:0 sid 10 fault acc 0 adt 0x0 reason 0x5 addr 2e09000

and eventually the whole system freezes if I don't quit the compositor / switch to vt console.

Looks like a symptom of non-translatable physical address. I've encountered drivers which need additional work outside of the scope of this effort. Perhaps this is the case there as I can't any more cases in the Linux KPI where a physical address is substituted for a DMA one.
Also, I assume this is in remap mode. Does it work in identify map mode hw.busdma.default="bounce"? Unless there is an API which escaped, if it works in hw.dmar.enable="0" it's not a regression from before :-/

In D19930#428396, @gallatin wrote:

In D19930#428329, @kib wrote:

I wonder if a KPI of the format

struct ifnet *if_alloc_dev(u_char type, dev_t device);

would be more useful. It would hide all NUMA/non-NUMA/bus_get_domain() failed details that driver authors will copy/paste ad infinitum.
I do not propose to not add if_alloc_domain(), just think that if_alloc_dev() should be the primari KPI. Also it might allow to get more device_t context in if_alloc().

I think I agree with you. I was torn between the 2 different KPIs. Given that you suggested what I already was thinking about, I will implement it.
I'm trying to think now how if_alloc_domain() could be useful for non-device ifnets, and it does not seem to me at all clear that it would be. So I think I will replace if_alloc_domain() with if_alloc_dev()

I wonder if a KPI of the format

struct ifnet *if_alloc_dev(u_char type, dev_t device);

would be more useful. It would hide all NUMA/non-NUMA/bus_get_domain() failed details that driver authors will copy/paste ad infinitum.

s/-d/-g/ in the message.

I do not see that this could end in anything useful.

In D19887#427370, @rmacklem wrote:

I just read the man page (I know, never read the documentation;-) and it seems that
the recursion check is for exclusive locks, so adding SX_DUPOK would allow a process
to acquire a shared lock that it already has.
So, maybe, sticking with the current patch that applies a unique name to each element
is preferred?

I would greatly prefer that kern_proc.c provided the services that nfscl used, instead of open-coding. E.g., it would be the 'lock all pidhash chains', and then pfind_locked(). The later can also assert that the hash chain it accesses is locked.

I still fail to understand why the change is needed.

Did you tested this with DEBUG_VFS_LOCKS ?

In D19874#426663, @trasz wrote:

The relock happens pretty much never, since the function is used for loading the interpreter - which is busy all the time. I had a printf there for testing, and it's called just once, early during boot.

In D19863#426613, @trasz wrote:

But previously we were dropping and picking up the lock just before, to call malloc(9), so this should equal out?

Basically the relock invalidates all benefits of the shared locking.

In D19863#426603, @trasz wrote:

In D19863#426594, @kib wrote:

In D19863#426441, @trasz wrote:

But I don't see how the patch increases the number of locks/unlocks. Could you explain?

I did not count exact number of locks you take, but you now lock per segment, and do this more than once. So the number of locks taken is O(num of segments) instead of O(1).

And that's the part I don't get - where? From what I see, the lock is taken once, around load_sections().

That said, even if we assume that executables all have one segment (in fact two), then there is still more locks after your patch. I did not checked this.

There is one additional lock, at the end - we have to return with an exclusive lock held. I'll think about this some more.

In D19863#426441, @trasz wrote:

But I don't see how the patch increases the number of locks/unlocks. Could you explain?

I did not count exact number of locks you take, but you now lock per segment, and do this more than once. So the number of locks taken is O(num of segments) instead of O(1).

In D19599#426455, @mckusick wrote:

Should this now be closed since it was resolved in D19811?

I do not understand why this patch is needed, and you did not bothered to explain.

Fix two nits according to jhb comments.

Move legacy match to legacy.c as suggested.

In D19833#426071, @jhb wrote:

Well, we should either remove the fallback outright, or we should fix it to actually be used. I think I would be fine with just removing it outright.

Yes, I will update the patch with removal in a minute.

In D19833#425992, @jhb wrote:

Hmm, so scottl@ added this version of MMIO access before we looked at the MCFG table, and I'm not sure that the BIOS for these chipsets don't actually already provide the MCFG table.

At least on the machine of the person who reported the problem, there is MCFG table. Chipset MCH is E7500.

Could you add some comments and update summary to explain the design ?

Update comment.