Page MenuHomeFreeBSD

flopen: simplify the code
AbandonedPublic

Authored by oshogbo on Mar 19 2021, 10:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 23, 9:26 AM
Unknown Object (File)
Wed, Oct 23, 9:25 AM
Unknown Object (File)
Thu, Oct 17, 1:29 PM
Unknown Object (File)
Thu, Oct 17, 1:29 PM
Unknown Object (File)
Thu, Oct 17, 12:56 PM
Unknown Object (File)
Oct 1 2024, 4:44 PM
Unknown Object (File)
Sep 29 2024, 7:02 PM
Unknown Object (File)
Sep 28 2024, 12:47 AM
Subscribers

Details

Reviewers
kib
des
Group Reviewers
manpages
Summary

After the fa3bd463cee5 creation of file and lock is an atomic
operation in kernel. We don't have to do heavy verification in the
user land anymore.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 37950
Build 34839: arc lint + arc unit

Event Timeline

This is complicated. There are two kinds of advisory locks, one is POSIX (fcntl), another is flock(2)-like. Main difference is that fcntl-style locks are cleared when the file descriptor is closed (not file), also fcntl locks are bound to the owning process. flock-style locks are associated with files, so e.g. if you pass a file descriptor referencing the locked file over unix domain socket, both sender and receiver have a control over (un)lock.

I believe that O_EXLOCK actually sets the flock-style lock, because it does not promote P_ADVLOCK nor it passed F_POSIX internally. But this requires some confirmation, perhaps in test. Hmm, perhaps test_flopen_lock_child() does this. Did you tried?

Another thing is, there is a recheck that the file was not renamed while we waited for lock. In fact, this is relevant for openat() uses as well. I.e. openat() would still lock the file, but if the check for inode after lock was needed for old flopen(), it equally is needed for openat(O_EXLOCK) version, because the vnode is unlocked while we wait for the lock, allowing e.g. renames.

Thank you @kib for explanation, I will analyze this further.

gbe added a subscriber: gbe.

LGTM

This is a bad idea. The rename check is absolutely essential to flopen(3). If you don't need the rename check, then by all means, just call open(2) or openat(2) directly, but leave this code alone. The comment at the top is there for a reason.

des requested changes to this revision.Apr 18 2021, 3:27 PM
This revision now requires changes to proceed.Apr 18 2021, 3:27 PM