unix: provide an option to return locked from unp_connectat()
ClosedPublic
Actions

Authored by glebius on May 23 2022, 9:01 PM.

Details

Reviewers

Group Reviewers

network
transport

Commits

rG315167c0de1a: unix: provide an option to return locked from unp_connectat()

Summary

Use this new version in unix/dgram socket when sending to a target
address. This removes extra lock release/acquisition and possible
counter-intuitive ENOTCONN.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Warnings

Severity	Location	Code	Message
Warning	sys/kern/uipc_usrreq.c:724	SPELL1	Possible Spelling Mistake
Warning	sys/kern/uipc_usrreq.c:1823	SPELL1	Possible Spelling Mistake

Unit

No Test Coverage

Build Status

Buildable 45714
Build 42602: arc lint + arc unit

Event Timeline

glebius created this revision.May 23 2022, 9:01 PM

Herald added a subscriber: imp. · View Herald TranscriptMay 23 2022, 9:01 PM

glebius requested review of this revision.May 23 2022, 9:01 PM

Harbormaster completed remote builds in B45679: Diff 106297.May 23 2022, 9:01 PM

glebius added a parent revision: D35297: unix/dgram: inline sbappendaddr_locked() into uipc_sosend_dgram().May 23 2022, 9:01 PM

glebius added a child revision: D35299: sockets: enable protocol specific socket buffers.

glebius added a child revision: D35305: libc/syslog: deprecate use of "/var/run/logpriv".May 23 2022, 9:07 PM

glebius added reviewers: network, markj.May 23 2022, 9:12 PM

Note: this patch is a step towards D35303.

markj added inline comments.May 24 2022, 3:02 PM

sys/kern/uipc_usrreq.c
1828	I don't have a concrete suggestion right now, but I don't like these kinds of "naked" boolean flags. IMO it'd be better to have a separate function.
1978–1979
1989	vput() can sleep when releasing the final reference, so we can't hold PCB locks here.

markj added inline comments.May 25 2022, 6:48 PM

sys/kern/uipc_usrreq.c
1989	Thinking a bit more, unp2 should already hold a use-ref on the vnode, and this ref is protected by the PCB lock. So after the VOP_UNP_CONNECT() call I believe the use count on `vp` will be >= 2. The problem is that unp2's PCB lock is released after calling sonewconn(), and after that point the use count can drop to 1. But in the `!connreq` case (i.e., the SOCK_DGRAM case), I think the vput() call is effectively guaranteed not to sleep. Not sure if that's sufficient for you.

glebius marked an inline comment as done.May 25 2022, 7:09 PM

glebius added inline comments.

sys/kern/uipc_usrreq.c
1989	Thanks for input! I will put that into a comment.

Add comment. Rebase.

Harbormaster completed remote builds in B45714: Diff 106379.May 26 2022, 1:40 AM

glebius marked 2 inline comments as done.May 26 2022, 1:40 AM

markj added inline comments.May 27 2022, 1:48 PM

sys/kern/uipc_usrreq.c
1987	So let's assert `!(return_locked && connreq)`?

Add assertion befor vput().

Harbormaster completed remote builds in B45744: Diff 106423.May 27 2022, 3:28 PM

glebius added a reviewer: transport.May 28 2022, 3:50 AM

markj added inline comments.May 30 2022, 1:17 PM

sys/kern/uipc_usrreq.c
1216	Instead use `unp2 = unp_pcb_lock_peer(unp)` and check for NULL. I believe it is possible for the socket to be disconnected again while PCB locks are dropped.