Page MenuHomeFreeBSD

Provide a wrapper structure around ucred
AbandonedPublic

Authored by mjg on Dec 14 2019, 2:48 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 16, 3:20 PM
Unknown Object (File)
Tue, Apr 16, 3:19 PM
Unknown Object (File)
Fri, Apr 12, 7:23 PM
Unknown Object (File)
Mar 20 2024, 12:40 AM
Unknown Object (File)
Dec 23 2023, 6:42 AM
Unknown Object (File)
Nov 6 2023, 9:14 AM
Unknown Object (File)
Nov 6 2023, 9:04 AM
Unknown Object (File)
Nov 3 2023, 8:27 PM
Subscribers

Details

Reviewers
kib
jeff
jhb
Summary

struct ucred gets refed and unrefed all the time. falloc_install during buildkernel accounts for 1740339 calls, almost all of which use the same creds.

Reduce ping pong by providing a wrapper structure allocated on fork which can be referenced by threads in given process. This opens up reduction of the refing cost in the vm as well.

Note the current code contains an avoidable ref on td_ucred/p_ucred which can be eliminated later.

Consumers were mostly patched by coccinelle.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 28147

Event Timeline

What are the chances you'd be willing to change the swap/vm object creds to use this as well?

That code needs a clean up I'm not up for. In particular it keeps passing cred around as arguments even though it's almost always from curthread. I think someone familiar with the area should sit down with this.

Can you explain how fork works WRT p_credwrap ?

sys/kern/init_main.c
534

And where is the newcredwrap value used ?

In D22811#499523, @kib wrote:

Can you explain how fork works WRT p_credwrap ?

Anything installing credentials (including initial ones on fork) allocates a new credwrap. The point is to allow multiple credential changes while allowing all credwraps to get garbage collected when the time comes with standard refcounting.

sys/kern/init_main.c
534

that's my bad. it should be in proc0 of course. it happens to be harmless since this kernel threads don't use the file descriptor table.

In D22811#499597, @mjg wrote:
In D22811#499523, @kib wrote:

Can you explain how fork works WRT p_credwrap ?

Anything installing credentials (including initial ones on fork) allocates a new credwrap. The point is to allow multiple credential changes while allowing all credwraps to get garbage collected when the time comes with standard refcounting.

But where does this happen in kern_fork.c ? Specifically, which line/call ?

It seems that you patched only one place to use the wrapper for real (files), all other places have to maintain two counters. Do you intend to finish this work so that p_ucred only referenced on fork and dereferenced on exit ? If not, this should be regularized anyway with clear written rules for preference of using p/td_ucred vs. credwrap. I think that 'finished' form should eliminate p_ucred at all.

sys/kern/kern_prot.c
1965

This is somewhat beyond even the 'just a hack' level. Please use sizeof(struct credwrap) there, if needed, extend the structure so tail fills the whole cache line.

sys/sys/file.h
208

Just curious, the additional indirection for f_cred access does not have a visible cost ?

So I have a better idea which expands on this. Key insight is that if needed we can transfer the count to the "real" reference count in cred and there are few well defined places which affect td_ucred and p_ucred. Thus we can manage the counter per-thread and transfer it out to the actual struct only if we are changing creds to something else.

crhold will be:

if (cred == curthread->td_ucred) {
     curthread->td_ucredref++;
} else {
     do the atomic on cred;
}

similarly crfree will check what to do.

This is not completely fleshed out yet but I think will work fine. With my fast path lookup patches I found open1_processes (literally just open/close on differnet files) to be primarily bottlenecked on crhold/crfree/vaccess (which reads from ucred). The wrapper idea would take care of this case, but would immediately run into trouble for the _threaded variant. I'll see about hacking osmething up this week.