[PATCH] Only lock send buffer in sopoll() if needed

John Baldwin jhb at freebsd.org
Mon Oct 6 16:38:09 UTC 2014


On Sunday, October 05, 2014 08:08:17 AM Robert N. M. Watson wrote:
> I'm not convinced that the race with SBS_CANTSENDMORE is OK, even though I
> really want to write that it is. The risk here is that we miss an
> asynchronous disconnect event, and that the thread then blocks even though
> an event is pending, which is a nasty turn of events. We might want to dig
> about a bit and see whether this case 'matters' -- e.g., that there are
> important cases where a test for writability might need to catch
> SBS_CANTRCVMORE but SBS_CANTSENDMORE is not simultaneously set.

I thought about this a bit more and this would be ok so long as the code that 
does a selwakeup() on so_rcv does so after setting SBS_CANTSENDMORE.  However, 
checking both soisdisconnecting() and soisdisconnected() shows that they call 
sorwakeup() and unlock the sb_rcv lock before setting the flag, so the race is 
not ok.

> Could you say a bit more about the performance benefits of this change -- is
> it substantial? If so, we might need to add a new socket-buffer flag along
> the lines of SB[RS]_DISCONNECTED that is 'broadcast' to both socket buffers
> on certain events. Doing so might require rejiggering elsewhere by causing
> additional locks to need to be held, possibly out of order.

I had posted this to an older thread on current@ where Luigi asked about the 
overhead of locking both for sopoll().  I never got a reply from Luigi (and no 
one else responded on that thread).  I found this again recently in an old 
tree and was curious what other folks thought of the idea.  I do not have any 
workloads I am working with where this is a factor.

-- 
John Baldwin


More information about the freebsd-net mailing list