Commit Graph

138 Commits

Author SHA1 Message Date
Robert Watson
5e3f7694b1 Replace custom file descriptor array sleep lock constructed using a mutex
and flags with an sxlock.  This leads to a significant and measurable
performance improvement as a result of access to shared locking for
frequent lookup operations, reduced general overhead, and reduced overhead
in the event of contention.  All of these are imported for threaded
applications where simultaneous access to a shared file descriptor array
occurs frequently.  Kris has reported 2x-4x transaction rate improvements
on 8-core MySQL benchmarks; smaller improvements can be expected for many
workloads as a result of reduced overhead.

- Generally eliminate the distinction between "fast" and regular
  acquisisition of the filedesc lock; the plan is that they will now all
  be fast.  Change all locking instances to either shared or exclusive
  locks.

- Correct a bug (pointed out by kib) in fdfree() where previously msleep()
  was called without the mutex held; sx_sleep() is now always called with
  the sxlock held exclusively.

- Universally hold the struct file lock over changes to struct file,
  rather than the filedesc lock or no lock.  Always update the f_ops
  field last. A further memory barrier is required here in the future
  (discussed with jhb).

- Improve locking and reference management in linux_at(), which fails to
  properly acquire vnode references before using vnode pointers.  Annotate
  improper use of vn_fullpath(), which will be replaced at a future date.

In fcntl(), we conservatively acquire an exclusive lock, even though in
some cases a shared lock may be sufficient, which should be revisited.
The dropping of the filedesc lock in fdgrowtable() is no longer required
as the sxlock can be held over the sleep operation; we should consider
removing that (pointed out by attilio).

Tested by:	kris
Discussed with:	jhb, kris, attilio, jeff
2007-04-04 09:11:34 +00:00
Mike Pritchard
45cdcb7aab Change fifo_printinfo to check if the vnode v_fifoinfo pointer
is NULL and print a message to that effect to prevent a panic.
2007-03-02 00:10:11 +00:00
Robert Watson
eca7e73743 Add a_fdidx to comment prototype for fifo_open().
MFC after:	3 days
Submitted by:	Kostik Belousov <kostikbel at gmail dot com>
2006-03-15 10:15:35 +00:00
Robert Watson
945a519a23 If fifo_open() is called with a negative file descriptor, return EINVAL
rather than panicking later.  This can occur if the kernel calls
vn_open() on a fifo, as there will be no associated file descriptor,
and therefore the file descriptor operations cannot be modified to
point to the fifo operation set.

MFC after:	3 days
Reported by:	Martin <nakal at nurfuerspam dot de>
PR:		94278
2006-03-14 19:29:45 +00:00
Robert Watson
2affdbee3e Second attempt at a work-around for fifo-related socket panics during
make -j with high levels of parallelism: acquire Giant in fifo I/O
routines.

Discussed with:	ups
MFC after:	3 days
2005-10-01 20:15:41 +00:00
Robert Watson
a0e81bce69 Back out fifo_vnops.c:1.127, which introduced an sx lock around I/O on
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>
2005-09-27 16:45:22 +00:00
Robert Watson
454c3d13be Assert v_fifoinfo is non-NULL in fifo_close() in order to catch
non-conforming cases sooner.

MFC after:	3 days
Reported by:	Peter Holm <peter at holm dot cc>
2005-09-26 08:17:03 +00:00
Robert Watson
ee47648770 Lock the read socket receive buffer when frobbing the sb_state flag on
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
2005-09-25 19:52:09 +00:00
Robert Watson
5d3df5cc1b For reasons of consistency (and necessity), assert an exclusive vnode
lock on the fifo vnode in fifo_open(): we rely on the vnode lock to
serialize access to v_fifoinfo.

MFC after:	3 days
2005-09-23 12:39:51 +00:00
Robert Watson
7028887eac Add fi_sx, an sx lock to serialize I/O operations on the socket pair
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
2005-09-22 10:51:12 +00:00
Robert Watson
526e258d3a Assert that (vp) is locked in fifo_close(), since we rely on the
exclusive vnode lock to synchronize the reference counts on struct
fifoinfo.

MFC after:	3 days
2005-09-18 10:44:50 +00:00
Robert Watson
ca17bccaa1 The socket pointers in fifoinfo are not permitted to be NULL, so
don't check if they are, it just confuses the fifo code more.

MFC after:	3 days
2005-09-15 15:45:34 +00:00
Robert Watson
447bbaa2cf Trim down now (believed to be) unused fifo_ioctl() and
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
2005-09-13 17:46:48 +00:00
Robert Watson
8a22e151be As a result of kqueue locking work, socket buffer locks will always
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
2005-09-13 10:39:24 +00:00
Robert Watson
db7a6c2f43 Annotate two issues:
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
2005-09-13 09:23:22 +00:00
Robert Watson
88f39e8e95 Introduce no-op nosup fifo kqueue filter and detach routine, which are
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
2005-09-12 19:59:12 +00:00
Robert Watson
48afebb83d When a request is made to register a filter on a fifo that doesn't
apply to the fifo (i.e., not EVFILT_READ or EVFILT_WRITE), reject
it as EINVAL, not by returning 1 (EPERM).

MFC after:	3 days
2005-09-12 18:07:49 +00:00
Robert Watson
114538d85b Remove DFLAG_SEEKABLE from fifo file descriptors: fifos are not seekable
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.
2005-09-12 12:15:12 +00:00
Robert Watson
6dd84b0bdc Only poll the fifo for read events if the fifo is attached to a readable
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
2005-09-12 10:16:18 +00:00
Robert Watson
845e8e827b After going to some trouble to identify only the write-related events
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
2005-09-12 10:13:15 +00:00
Robert Watson
ab5182012a When a writer opens a fifo, wake up the read socket for read, not the
write socket.

MFC after:	3 days
2005-09-12 10:07:21 +00:00
Robert Watson
a1b9943657 Add an assertion that fifo_open() doesn't race against other threads
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
2005-09-12 10:06:38 +00:00
Robert Watson
ba9eeb43fe Rather than reaching into the internals of the UNIX domain socket code
by calling uipc_connect2() to connect two socket endpoints to create a
fifo, call soconnect2().

MFC after:	3 days
2005-09-12 10:05:08 +00:00
Jeff Roberson
8da0046596 - The VI_DOOMED flag now signals the end of a vnode's relationship with
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.
2005-03-13 12:18:25 +00:00
Poul-Henning Kamp
83c6439714 Whitespace in vop_vector{} initializations. 2005-01-13 18:59:48 +00:00
Warner Losh
d167cf6f3a /* -> /*- for copyright notices, minor format tweaks as necessary 2005-01-06 18:10:42 +00:00
Poul-Henning Kamp
5cb471d04d Don't forget to bypass vnodes in corner cases.
Found by:	kkenn and ports/shell/zsh
Thanks to:	jeffr
2004-12-13 10:07:57 +00:00
Poul-Henning Kamp
dce357b112 Explicitly panic vop_read/vop_write on fifos. 2004-12-13 07:07:50 +00:00
Poul-Henning Kamp
aec0fb7b40 Back when VOP_* was introduced, we did not have new-style struct
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)
2004-12-01 23:16:38 +00:00
Poul-Henning Kamp
6fde64c778 Mechanically change prototypes for vnode operations to use the new typedefs. 2004-12-01 12:24:41 +00:00
Poul-Henning Kamp
75ad04b4f6 Add dropped implementation of ioctl for fifos. 2004-11-18 17:18:11 +00:00
Poul-Henning Kamp
003e18aef4 Make vnode bypass for fifos (read, write, poll) mandatory. 2004-11-17 07:30:02 +00:00
Poul-Henning Kamp
d6d64f0f2c Add file ops to fifofs so that we can bypass vnodes (and Giant) for the
heavy-duty operations (read, write, poll/select, kqueue).

Disabled for now, enable with "vfs.fifofs.fops=1" in loader.conf.
2004-11-15 14:51:44 +00:00
Poul-Henning Kamp
1ecf144493 fifos doesn't need a vop_lookup, the default will do fine. 2004-11-13 18:51:13 +00:00
Poul-Henning Kamp
5349c79d75 Properly implement a default version of VOP_GETWRITEMOUNT.
Remove improper access to vop_stdgetwritemount() which should and
will instead rely on the VOP default path.
2004-11-06 11:41:22 +00:00
John-Mark Gurney
ad3b9257c2 Add locking to the kqueue subsystem. This also makes the kqueue subsystem
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)
2004-08-15 06:24:42 +00:00
Robert Watson
7d84f9d293 Remove unlocked read annotation for sbspace(); the read is locked. 2004-06-23 00:35:50 +00:00
Robert Watson
c012260726 Merge some additional leaf node socket buffer locking from
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.
2004-06-18 02:57:55 +00:00
Robert Watson
9535efc00d Merge additional socket buffer locking from rwatson_netperf:
- 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.
2004-06-17 22:48:11 +00:00
Robert Watson
7721f5d760 Grab the socket buffer send or receive mutex when performing a
read-modify-write on the sb_state field.  This commit catches only
the "easy" ones where it doesn't interact with as yet unmerged
locking.
2004-06-15 03:51:44 +00:00
Robert Watson
c0b99ffa02 The socket field so_state is used to hold a variety of socket related
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.
2004-06-14 18:16:22 +00:00
Don Lewis
866046f5a6 Add MSG_NBIO flag option to soreceive() and sosend() that causes
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.
2004-06-01 01:18:51 +00:00
Don Lewis
2526dc2b61 Switch from using the vnode interlock to a private mutex in fifo_open()
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().
2004-05-17 20:16:40 +00:00
Warner Losh
f36cfd49ad Remove advertising clause from University of California Regent's
license, per letter dated July 22, 1999 and email from Peter Wemm,
Alan Cox and Robert Watson.

Approved by: core, peter, alc, rwatson
2004-04-07 20:46:16 +00:00
Robert Watson
db48c0d254 Export uipc_connect2() from uipc_usrreq.c instead of unp_connect2(),
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.
2004-03-31 01:41:30 +00:00
Don Lewis
95c6cd2f4b Use "fip->fi_readers == 0 && fip->fi_writers == 0" as the condition for
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.
2003-11-16 01:11:11 +00:00
Don Lewis
8d0c247562 If fifo_open() is interrupted, fifo_close() may not get called, causing
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
2003-11-10 22:21:00 +00:00
Don Lewis
bbddbed9f3 Partially back out rev 1.87 by nuking fifo_inactive() and moving the
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>
2003-06-16 17:17:09 +00:00
Don Lewis
b156281658 Clean up the fifo_open() implementation:
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.
2003-06-13 06:58:11 +00:00
Don Lewis
3a140162b4 Fix up locking problems in fifo_open() and fifo_close():
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
2003-06-01 06:24:32 +00:00