Commit Graph

140 Commits

Author SHA1 Message Date
pjd
fe74e944d1 When we do open, we should lock the vnode exclusively. This fixes few races:
- fifo race, where two threads assign v_fifoinfo,
- v_writecount modifications,
- v_object modifications,
- and probably more...

Discussed with:	kib, ups
Approved by:	re (rwatson)
2007-07-26 16:58:09 +00:00
kib
f13486a222 Revert UF_OPENING workaround for CURRENT.
Change the VOP_OPEN(), vn_open() vnode operation and d_fdopen() cdev operation
argument from being file descriptor index into the pointer to struct file.

Proposed and reviewed by:	jhb
Reviewed by:	daichi (unionfs)
Approved by:	re (kensmith)
2007-05-31 11:51:53 +00:00
rwatson
765a83fd79 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
mpp
61cebfa898 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
rwatson
918de4c556 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
rwatson
40fd390520 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
rwatson
5758dab896 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
rwatson
332a994af0 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
rwatson
87ac2d2498 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
rwatson
c9044f078d 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
rwatson
27cf2ffed6 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
rwatson
9c2c1cb9fb 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
rwatson
2d8b6f2e27 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
rwatson
7584b6b2c3 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
rwatson
0e4f08263a 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
rwatson
afc7b6e916 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
rwatson
2bda369cf8 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
rwatson
bc5e7eb1f3 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
rwatson
491de3e2d2 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
rwatson
6b308e01a1 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
rwatson
1481446aae 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
rwatson
919d519cbb 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
rwatson
5be69d1d56 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
rwatson
c9f007b159 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
rwatson
a079789c36 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
0d9df2e12d - 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
phk
7617afd15d Whitespace in vop_vector{} initializations. 2005-01-13 18:59:48 +00:00
imp
fb0a393e99 /* -> /*- for copyright notices, minor format tweaks as necessary 2005-01-06 18:10:42 +00:00
phk
7bb3e163dd 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
phk
eda2bd825d Explicitly panic vop_read/vop_write on fifos. 2004-12-13 07:07:50 +00:00
phk
59f305606c 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
phk
05b9cb2a46 Mechanically change prototypes for vnode operations to use the new typedefs. 2004-12-01 12:24:41 +00:00
phk
a1fa5f3147 Add dropped implementation of ioctl for fifos. 2004-11-18 17:18:11 +00:00
phk
a94fe69bb7 Make vnode bypass for fifos (read, write, poll) mandatory. 2004-11-17 07:30:02 +00:00
phk
0e1bc6bd7d 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
phk
7204610fc8 fifos doesn't need a vop_lookup, the default will do fine. 2004-11-13 18:51:13 +00:00
phk
723cc1105c 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
jmg
bc1805c6e8 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
rwatson
083bcb28d6 Remove unlocked read annotation for sbspace(); the read is locked. 2004-06-23 00:35:50 +00:00
rwatson
d87fad9f08 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
rwatson
855c4bb01f 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
rwatson
029226f3a8 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
rwatson
f2c0db1521 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
truckman
d503c79cad 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
truckman
6174e9d812 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
imp
b49b7fe799 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
rwatson
8eeaad5c11 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
truckman
24b3a6d135 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
truckman
2305818240 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
truckman
78ee1563af 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