Page MenuHomeFreeBSD
Differential D57553

netexport: Move struct netexport into a .h file and refcnt it
ClosedPublic
Actions

Authored by rmacklem on Fri, Jun 12, 9:10 PM.
Tags
None
Referenced Files
F161940166: D57553.diff
Wed, Jul 8, 4:52 AM
Unknown Object (File)
Sat, Jul 4, 2:52 AM
Unknown Object (File)
Wed, Jun 24, 11:22 AM
Unknown Object (File)
Wed, Jun 24, 11:17 AM
Unknown Object (File)
Tue, Jun 23, 5:31 PM
Unknown Object (File)
Tue, Jun 23, 5:24 PM
Unknown Object (File)
Tue, Jun 23, 6:26 AM
Unknown Object (File)
Tue, Jun 23, 6:18 AM
View All 41 Files
Subscribers
glebius
imp
melifaro
olce

Details

Reviewers
kib
markj
Commits
rG2e6615b26bb5: netexport: Move struct netexport in a .h file and refcnt it
Summary

This patch moves "struct netexport" into a separate
netexport.h file and refcounts the structure, plus adds
a few fields that will be used in a future NFS server
commit.

malloc()/free() is replaced by vfs_netexport_alloc()
and vfs_netexport_free().

Test Plan

Tested with a NFS patch that uses the new fields
and does acquire a refcount on the structure so
that it will not be free'd prematurely.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

rmacklem created this revision.Fri, Jun 12, 9:10 PM
Herald added subscribers: olce, glebius, melifaro, imp. · View Herald TranscriptFri, Jun 12, 9:10 PM
rmacklem requested review of this revision.Fri, Jun 12, 9:10 PM
rmacklem mentioned this in D57554: pNFSd: Add a directory of newly created files for the pNFSd.Fri, Jun 12, 9:25 PM
kib added inline comments.Sat, Jun 13, 1:31 AM
sys/kern/vfs_export.c
686

Should be refcount_init()

rmacklem updated this revision to Diff 179692.Sat, Jun 13, 8:42 PM

Use refcount_init() as suggested by kib@.

rmacklem marked an inline comment as done.Sat, Jun 13, 8:42 PM
markj added inline comments.Sat, Jun 13, 9:07 PM
sys/kern/vfs_export.c
61

Why not group it with the other sys/* includes?

352

Is this line resetting the netexport structure? It will clobber the refcount and mutex.

rmacklem added inline comments.Sat, Jun 13, 9:27 PM
sys/kern/vfs_export.c
61

Because it won't compile.. netexport needs
the radix_node structure defined.

352

Ouch!! Good catch. I'll fix this for the next version of
the patch.

rmacklem updated this revision to Diff 179696.Sat, Jun 13, 11:44 PM

Fix the memset() so that it only zeros out
part of the structure, as spotted by markj@.

rmacklem marked an inline comment as done.Sat, Jun 13, 11:45 PM
kib accepted this revision.Sun, Jun 14, 2:54 AM
kib added inline comments.
sys/kern/vfs_export.c
61

I would add a short comment on this line to explain the ordering requirement.

This revision is now accepted and ready to land.Sun, Jun 14, 2:54 AM
rmacklem updated this revision to Diff 179763.Sun, Jun 14, 10:30 PM

Added vfs_netexport_acquire() as suggested by markj@
and renamed vfs_netexport_free() to vfs_netexport_release(),
since that seemed more consistent.

I put a comment in netexport.h indicating it needs to be
included after net/radix.h.

This revision now requires review to proceed.Sun, Jun 14, 10:30 PM
kib accepted this revision.Mon, Jun 15, 11:15 AM
This revision is now accepted and ready to land.Mon, Jun 15, 11:15 AM
markj added inline comments.Mon, Jun 15, 1:25 PM
sys/kern/vfs_export.c
352

I'd add an explicit vfs_netexport_reset() helper for this. That would also let you assert that the pnfs fields are already set correctly, since they're not zeroed here.

Should we verify that the refcount is 1 here?

rmacklem updated this revision to Diff 179838.Tue, Jun 16, 12:32 AM

Add vfs_netexport_reset() as suggested by markj@.
Also, add KASSERT()s to check ne_ref > 0.

This revision now requires review to proceed.Tue, Jun 16, 12:32 AM
rmacklem added inline comments.Tue, Jun 16, 12:34 AM
sys/kern/vfs_export.c
352

ne_ref could be > 1 once the replenisher is
running, which is a separate commit.

markj accepted this revision.Tue, Jun 16, 1:26 PM
markj added inline comments.
sys/kern/vfs_export.c
352

Then, isn't it dangerous to reset fields of the structure while some concurrent access is possible? I guess the replenisher won't be looking at ne4 or ne6, ok.

sys/sys/netexport.h
70

This define should go before the field, so that it's clearer at a glance which fields are zeroed. With this version, at first glance it looks like ne_mtx will be zeroed too.

This revision is now accepted and ready to land.Tue, Jun 16, 1:26 PM
Closed by commit rG2e6615b26bb5: netexport: Move struct netexport in a .h file and refcnt it (authored by rmacklem). · Explain WhyTue, Jun 16, 3:37 PM
This revision was automatically updated to reflect the committed changes.
rmacklem added a commit: rG2e6615b26bb5: netexport: Move struct netexport in a .h file and refcnt it.

Revision Contents
Changeset List

PathSize
sys/
kern/
vfs_export.c
vfs_export.c.precreate
79 lines
vfs_mount.c
vfs_mount.c.precreate
2 lines
sys/
mount.h
mount.h.precreate
2 lines
netexport.h
netexport.h.precreate
87 lines

Diff 179838

View Options

sys/kern/vfs_export.c

Loading...
View Options

sys/kern/vfs_mount.c

Loading...
View Options

sys/sys/mount.h

Loading...
View Options

sys/sys/netexport.h

Loading...