a fifo. While this did indeed close the race, confirming suspicions
about the nature of the problem, it causes difficulties with blocking
I/O on fifos.
Discussed with: ups
Also spotted by: Peter Holm <peter at holm dot cc>
that socket during open, not the write socket receive buffer. This
might explain clearing of the sb_state SB_LOCK flag seen occasionally
in soreceive() on fifos.
MFC after: 3 days
Spotted by: ups
underlying the POSIX fifo implementation. In 6.x/7.x, fifo access is
moved from the VFS layer, where it was serialized using the vnode
lock, to the file descriptor layer, where access is protected by a
reference count but not serialized. This exposed socket buffer
locking to high levels of parallelism in specific fifo workloads, such
as make -j 32, which expose as yet unresolved socket buffer bugs.
fi_sx re-adds serialization about the read and write routines,
although not paths that simply test socket buffer mbuf queue state,
such as the poll and kqueue methods. This restores the extra locking
cost previously present in some cases, but is an effective workaround
for the instability that has been experienced. This workaround should
be removed once the bug in socket buffer handling has been fixed.
Reported by: kris, jhb, Julien Gabel <jpeg at thilelli dot net>,
Peter Holm <peter at holm dot cc>, others
MFC after: 3 days
fifo_kqfilter() VOP implementations, since they in theory are used
only on open file descriptors, in which case the ioctls are via
fifo_ioctl_f() and kqueue requests are via fifo_kqfilter_f().
Generate warnings if they are entered for now. These printf()
calls should become panic() calls.
Annotate and re-implement fifo_ioctl_f(): don't arbitrarily
forward ioctls to the socket layer, only forward the ones we
explicitly support for fifos. In the case of FIONREAD, don't
forward the request to the write socket on a read-write fifo, or
the read result is overwritten. Annotate a nasty case for the
undefined POSIX O_RDWR on fifos, in which failure of the second
ioctl will result in the socket pair being in an inconsistent
state.
Assert copyright as I find myself rewriting non-trivial parts of
fifofs.
MFC after: 3 days
be held when entering a kqueue filter for fifos via a socket buffer
event: as such, assert the lock unconditionally rather than acquiring
it conditionall.
MFC after: 3 days
1) fifo_kqfilter() is not actually ever used, it likely should be GC'd.
2) fifo_kqfilter_f() doesn't implement EVFILT_VNODE, so detecting events
on the underlying vnode for a fifo no longer works (it did in 4.x).
Likely, fifo_kqfilter_f() should forward the request to the VFS using
fp->f_vnode, which would work once fifo_kqfilter() was detached from
the vnode operation vector (removing the fifo override).
Discussed with: phk
used when a read filter is requested on a write-only fifo descriptor, or
a write filter is requested on a read-only fifo descriptor. This
permits the filters to be registered, but never raises the event, which
causes kqueue behavior for fifos to more closely match similar semantics
for poll and select, which permit testing for the condition even though
the condition will never be raised, and is consistent with POSIX's notion
that a fifo has identical semantics to a one-way IPC channel created
using pipe() on most operating systems.
The fifo regression test suite can now run to completion on HEAD without
errors.
MFC after: 3 days
according to POSIX, not to mention the fact that it doesn't make sense
(and hence isn't really implemented). This causes the fifo_misc
regression test to succeed.
file descriptor. Otherwise, the read end of a fifo might return that it
is writable (which it isn't).
Only poll the fifo for write events if the fifo attached to a writable
file descriptor. Otherwise, the write end of a fifo might return that
it is readable (which it isn't).
In the event that a file is FREAD|FWRITE (which is allowed by POSIX, but
has undefined behavior), we poll for both.
MFC after: 3 days
to poll the write socket for, the fifo polling code proceeded to poll
for the complete set of events. Use 'levents' instead of 'events' as
the argument to poll, and only poll the write socket if there is
interest in write events.
MFC after: 3 days
while sleeping to allocate fifo state: due to using the vnode lock to
serialize access to a fifo during open, it shouldn't happen (tm).
MFC after: 3 days
the filesystem. Check that rather than VI_XLOCK.
- VOP_INACTIVE should no longer drop the vnode lock.
- The vnode lock is required around calls to vrecycle() and vgone().
Sponsored by: Isilon Systems, Inc.
initializations but we did have lofty goals and big ideals.
Adjust to more contemporary circumstances and gain type checking.
Replace the entire vop_t frobbing thing with properly typed
structures. The only casualty is that we can not add a new
VOP_ method with a loadable module. History has not given
us reason to belive this would ever be feasible in the the
first place.
Eliminate in toto VOCALL(), vop_t, VNODEOP_SET() etc.
Give coda correct prototypes and function definitions for
all vop_()s.
Generate a bit more data from the vnode_if.src file: a
struct vop_vector and protype typedefs for all vop methods.
Add a new vop_bypass() and make vop_default be a pointer
to another struct vop_vector.
Remove a lot of vfs_init since vop_vector is ready to use
from the compiler.
Cast various vop_mumble() to void * with uppercase name,
for instance VOP_PANIC, VOP_NULL etc.
Implement VCALL() by making vdesc_offset the offsetof() the
relevant function pointer in vop_vector. This is disgusting
but since the code is generated by a script comparatively
safe. The alternative for nullfs etc. would be much worse.
Fix up all vnode method vectors to remove casts so they
become typesafe. (The bulk of this is generated by scripts)
a more complete subsystem, and removes the knowlege of how things are
implemented from the drivers. Include locking around filter ops, so a
module like aio will know when not to be unloaded if there are outstanding
knotes using it's filter ops.
Currently, it uses the MTX_DUPOK even though it is not always safe to
aquire duplicate locks. Witness currently doesn't support the ability
to discover if a dup lock is ok (in some cases).
Reviewed by: green, rwatson (both earlier versions)
rwatson_netperf:
Introduce conditional locking of the socket buffer in fifofs kqueue
filters; KNOTE() will be called holding the socket buffer locks in
fifofs, but sometimes the kqueue() system call will poll using the
same entry point without holding the socket buffer lock.
Introduce conditional locking of the socket buffer in the socket
kqueue filters; KNOTE() will be called holding the socket buffer
locks in the socket code, but sometimes the kqueue() system call
will poll using the same entry points without holding the socket
buffer lock.
Simplify the logic in sodisconnect() since we no longer need spls.
NOTE: To remove conditional locking in the kqueue filters, it would
make sense to use a separate kqueue API entry into the socket/fifo
code when calling from the kqueue() system call.
- Lock down low hanging fruit use of sb_flags with socket buffer
lock.
- Lock down low hanging fruit use of so_state with socket lock.
- Lock down low hanging fruit use of so_options.
- Lock down low-hanging fruit use of sb_lowwat and sb_hiwat with
socket buffer lock.
- Annotate situations in which we unlock the socket lock and then
grab the receive socket buffer lock, which are currently actually
the same lock. Depending on how we want to play our cards, we
may want to coallesce these lock uses to reduce overhead.
- Convert a if()->panic() into a KASSERT relating to so_state in
soaccept().
- Remove a number of splnet()/splx() references.
More complex merging of socket and socket buffer locking to
follow.
flags relating to several aspects of socket functionality. This change
breaks out several bits relating to send and receive operation into a
new per-socket buffer field, sb_state, in order to facilitate locking.
This is required because, in order to provide more granular locking of
sockets, different state fields have different locking properties. The
following fields are moved to sb_state:
SS_CANTRCVMORE (so_state)
SS_CANTSENDMORE (so_state)
SS_RCVATMARK (so_state)
Rename respectively to:
SBS_CANTRCVMORE (so_rcv.sb_state)
SBS_CANTSENDMORE (so_snd.sb_state)
SBS_RCVATMARK (so_rcv.sb_state)
This facilitates locking by isolating fields to be located with other
identically locked fields, and permits greater granularity in socket
locking by avoiding storing fields with different locking semantics in
the same short (avoiding locking conflicts). In the future, we may
wish to coallesce sb_state and sb_flags; for the time being I leave
them separate and there is no additional memory overhead due to the
packing/alignment of shorts in the socket buffer structure.
them to behave the same as if the SS_NBIO socket flag had been set
for this call. The SS_NBIO flag for ordinary sockets is set by
fcntl(fd, F_SETFL, O_NONBLOCK).
Pass the MSG_NBIO flag to the soreceive() and sosend() calls in
fifo_read() and fifo_write() instead of frobbing the SS_NBIO flag
on the underlying socket for each I/O operation. The O_NONBLOCK
flag is a property of the descriptor, and unlike ordinary sockets,
fifos may be referenced by multiple descriptors.
to avoid lock order problems when manipulating the sockets associated
with the fifo.
Minor optimization of a couple of calls to fifo_cleanup() from
fifo_open().
and consume that interface in portalfs and fifofs instead. In the
new world order, unp_connect2() assumes that the unpcb mutex is
held, whereas uipc_connect2() validates that the passed sockets are
UNIX domain sockets, then grabs the mutex.
NB: the portalfs and fifofs code gets down and dirty with UNIX domain
sockets. Maybe this is a bad thing.
disposing fifo resources in fifo_cleanup() instead using of
"vp->v_usecount == 1". There may be other references to the vnode, for
instance by nullfs, at the time fifo_open() or fifo_close() is called,
which could cause a resource leak.
Don't bother grabbing the vnode interlock in fifo_cleanup() since it no
longer accesses v_usecount.
a resource leak. Move the resource deallocation code from fifo_close()
to a new function, fifo_cleanup(), and call fifo_cleanup() from
fifo_close() and the appropriate places in fifo_open().
Tested by: Lukas Ertl
Pointy hat to: truckman
resource deallocation back to fifo_close(). This eliminates any
stale data that might be stuck in the socket buffers after all the
readers and writers have closed the fifo.
Tested by: Thorsten Schroeder <ths@katjusha.de>
Restructure the error handling portion of the resource allocation
code to eliminate duplicated code.
Test for the O_NONBLOCK && fi_readers == 0 case before incrementing
fi_writers and modifying the the socket flag to avoid having to
undo these operations in this error case.
Restructure and simplify the code that handles blocking opens.
There should be no change to functionality.
Sleep on the vnode interlock while waiting for another
caller to increment fi_readers or fi_writers. Hold the
vnode interlock while incrementing fi_readers or fi_writers
to prevent a wakeup from being missed.
Only access fi_readers and fi_writers while holding the vnode
lock. Previously fifo_close() decremented their values without
holding a lock.
Move resource deallocation from fifo_close() to fifo_inactive(),
which allows the VOP_CLOSE() call in the error return path in
fifo_open() to be removed. Fifo_open() was calling VOP_CLOSE()
with the vnode lock held, in violation the current vnode locking
API. Also the way fifo_close() used vrefcnt() to decide whether
to deallocate resources was bogus according to comments in the
vrefcnt() implementation.
Reviewed by: bde
fifo_open() waiting for another reader or writer if one arrived and
departed while we were waiting (or a little earlier).
Rev.1.79 broke blocking opens of fifos by making them time out after 1
second. This was bad for at least apsfilter.
Tested by: "Simon 'corecode' Schubert" <corecode@corecode.ath.cx>,
Alexander Leidinger <Alexander@leidinger.net>,
phk
MFC after: 4 weeks
