[unix(4) cleanup 4/8] Simplify unp_disconnect() callers.
ClosedPublic
Actions

Authored by markj on Sep 2 2020, 4:56 PM.

Details

Reviewers: None
Commits: rS365762: Simplify unp_disconnect() callers.

Summary

In all cases, PCBs are unlocked after unp_disconnect() returns. Since
unp_disconnect() may release the last PCB reference, callers may have to
bump the refcount before the call just so that they can release them
again.

Change unp_disconnect() to release PCB locks as well as connection
references. Remove code that bumps and then releases the refcount.
Tighten assertions.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

markj requested review of this revision.Sep 2 2020, 4:56 PM

markj created this revision.

Harbormaster completed remote builds in B33329: Diff 76543.Sep 2 2020, 4:56 PM

markj added a parent revision: D26296: [unix(4) cleanup 3/8] Rename unp_pcb_lock2()..Sep 2 2020, 5:08 PM

markj added a child revision: D26298: [unix(4) cleanup 5/8] Avoid an unnecessary malloc() for dgram sockets..

emaste added a subscriber: emaste.Sep 2 2020, 5:20 PM

Use unp_pcb_unlock_pair().

Harbormaster completed remote builds in B33336: Diff 76550.Sep 2 2020, 5:58 PM

kevans mentioned this in D26295: [unix(4) cleanup 2/8] Improve unix socket refcounting..Sep 3 2020, 5:07 PM

markj added inline comments.Sep 3 2020, 5:46 PM

sys/kern/uipc_usrreq.c
2003 ↗	(On Diff #76550)	There is a bug here, fixed later in the series: if unp_conn == NULL we might in fact be dropping the last reference here.

Don't use unp_pcb_rele_notlast() when we may in fact be releasing the last ref.

Harbormaster completed remote builds in B33400: Diff 76668.Sep 4 2020, 8:14 PM

This revision was not accepted when it landed; it landed in state Needs Review.Sep 15 2020, 7:22 PM

Closed by commit rS365762: Simplify unp_disconnect() callers. (authored by markj). · Explain Why

This revision was automatically updated to reflect the committed changes.

markj added a commit: rS365762: Simplify unp_disconnect() callers..

Herald added a subscriber: imp. · View Herald TranscriptSep 15 2020, 7:22 PM

Revision Contents
Changeset List

Path

Size

head/

sys/

kern/

uipc_usrreq.c

82 lines

Diff 77068

View Options

head/sys/kern/uipc_usrreq.c

	Show First 20 Lines • Show All 66 Lines • ▼ Show 20 Lines
	xva_getxoptattr(xvattr_t *xvap)			xva_getxoptattr(xvattr_t *xvap)
	{			{
	xoptattr_t *xoap = NULL;			xoptattr_t *xoap = NULL;
	if (xvap->xva_vattr.va_mask & AT_XVATTR)			if (xvap->xva_vattr.va_mask & AT_XVATTR)
	xoap = &xvap->xva_xoptattrs;			xoap = &xvap->xva_xoptattrs;
	return (xoap);			return (xoap);
	}			}

	static void
	vn_rele_inactive(vnode_t *vp)
	{
	vrele(vp);
	}

	/*			/*
	* Like vn_rele() except if we are going to call VOP_INACTIVE() then do it			* Like vn_rele() except if we are going to call VOP_INACTIVE() then do it
	* asynchronously using a taskq. This can avoid deadlocks caused by re-entering			* asynchronously using a taskq. This can avoid deadlocks caused by re-entering
	* the file system as a result of releasing the vnode. Note, file systems			* the file system as a result of releasing the vnode. Note, file systems
	* already have to handle the race where the vnode is incremented before the			* already have to handle the race where the vnode is incremented before the
	* inactive routine is called and does its locking.			* inactive routine is called and does its locking.
	*			*
	* Warning: Excessive use of this routine can lead to performance problems.			* Warning: Excessive use of this routine can lead to performance problems.
	* This is because taskqs throttle back allocation if too many are created.			* This is because taskqs throttle back allocation if too many are created.
	*/			*/
	void			void
	vn_rele_async(vnode_t vp, taskq_t taskq)			vn_rele_async(vnode_t vp, taskq_t taskq)
	{			{
	VERIFY(vp->v_count > 0);			VERIFY(vp->v_count > 0);
	VI_LOCK(vp);			if (refcount_release_if_not_last(&vp->v_usecount)) {
	if (vp->v_count == 1 && !(vp->v_iflag & VI_DOINGINACT)) {			vdrop(vp);
	VI_UNLOCK(vp);
	VERIFY(taskq_dispatch((taskq_t *)taskq,
	(task_func_t *)vn_rele_inactive, vp, TQ_SLEEP) != 0);
	return;			return;
	}			}
	refcount_release(&vp->v_usecount);			VERIFY(taskq_dispatch((taskq_t *)taskq,
	vdropl(vp);			(task_func_t *)vrele, vp, TQ_SLEEP) != 0);
	}			}

[unix(4) cleanup 4/8] Simplify unp_disconnect() callers.ClosedPublicActions