Page MenuHomeFreeBSD

[wip] vfs: switch vnode count management away from refcounts to atomics
Needs ReviewPublic

Authored by mjg on Sep 24 2019, 3:28 AM.

Details

Reviewers
kib
Summary

There are missing assertions which can be added later. Main point is to provide a dedicated file with common routines like incrementing only if not zero. I don't have a good name for the file so named it atomic_common2.h temporarily. It would be included at the end of md-specific atomic.h files after all base routines are provided.

Separately I'm going to convert all plain loads to atomic_load_int.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 26665

Event Timeline

mjg created this revision.Sep 24 2019, 3:28 AM
mjg edited the summary of this revision. (Show Details)Sep 24 2019, 3:29 AM
kib added a comment.Sep 24 2019, 9:53 AM

Perhaps name it atomic_ops.h or atomic_algo.h.

sys/amd64/include/atomic.h
682

I would prefer to not include the new header into machine/atomic.h. There is not so much consumers, i think all of them can handle the new header.

sys/sys/atomic_common2.h
1

You need to add copyright boilerplate and $FreeBSD$.

27

Why do you need the rel fence there ?

mjg added a comment.Sep 24 2019, 12:07 PM

How about atomic_util.h then?

sys/amd64/include/atomic.h
682

ok

sys/sys/atomic_common2.h
1

I know, this is just thrown together to get an initial opinion.

27

Actually we are going to need a cst fence here to prevent both stores and reads from leaking after the count is dropped. But it will have to come from fcmpset itself to not necessarily pessimize amd64.

mjg added inline comments.Sep 24 2019, 12:16 PM
sys/sys/atomic_common2.h
27

the current refcount_ code is definitely lacking here, will fix that too

hselasky added inline comments.
sys/sys/atomic_common2.h
40

You probably also want to declare u32 u64 variants aswell.