Don't require the socket lock for sorele().

[jhb]

Summary

Previously, sorele() always required the socket lock and dropped the
lock if the released reference was not the last reference. Many
callers locked the socket lock just before calling sorele() resulting
in a wasted lock/unlock when not dropping the last reference.

Move the previous implementation of sorele() into a new
sorele_locked() function and use it instead of sorele() for various
places in uipc_socket.c that called sorele() while already holding the
socket lock.

The sorele() macro now uses refcount_release_if_not_last() try to drop
the socket reference without locking the socket. If that shortcut
fails, it locks the socket and calls sorele_locked().

Sponsored by: Chelsio Communications

[markj]

Did you consider just changing the semantics of sorele()? Every user of this macro outside of uipc_socket.c contains the pattern SOCK_LOCK(so); sorele(so). Then, uipc_socket.c can use a private routine to drop a ref on a locked socket.

[jhb]

In D32741#738872, @markj wrote:

Did you consider just changing the semantics of sorele()? Every user of this macro outside of uipc_socket.c contains the pattern SOCK_LOCK(so); sorele(so). Then, uipc_socket.c can use a private routine to drop a ref on a locked socket.

Well, I worried that would be more of a pain with having the API differ between versions (not sure if there are any out-of-tree modules that might use this, but perhaps not?)

It is true that having sorele_locked() and sorele() is probably more typical of the API names we use.

[markj]

In D32741#738889, @jhb wrote:

In D32741#738872, @markj wrote:

Did you consider just changing the semantics of sorele()? Every user of this macro outside of uipc_socket.c contains the pattern SOCK_LOCK(so); sorele(so). Then, uipc_socket.c can use a private routine to drop a ref on a locked socket.

Well, I worried that would be more of a pain with having the API differ between versions (not sure if there are any out-of-tree modules that might use this, but perhaps not?)

It is true that having sorele_locked() and sorele() is probably more typical of the API names we use.

I'm not aware of anything in the ports tree that makes any real use of the socket API.

[jhb]

In D32741#738900, @markj wrote:

In D32741#738889, @jhb wrote:

In D32741#738872, @markj wrote:

Did you consider just changing the semantics of sorele()? Every user of this macro outside of uipc_socket.c contains the pattern SOCK_LOCK(so); sorele(so). Then, uipc_socket.c can use a private routine to drop a ref on a locked socket.

Well, I worried that would be more of a pain with having the API differ between versions (not sure if there are any out-of-tree modules that might use this, but perhaps not?)

It is true that having sorele_locked() and sorele() is probably more typical of the API names we use.

I'm not aware of anything in the ports tree that makes any real use of the socket API.

If you think it's safe to change the API, then I could make the new one "sorele" and make the old one "sorele_locked"? Note that the new macro still needs to call the old one to drop the last reference, so it can't be a static function in uipc_socket.c (though perhaps sorele_locked() could be a public function in uipc_socket.c?)

[markj]

In D32741#740268, @jhb wrote:

In D32741#738900, @markj wrote:

In D32741#738889, @jhb wrote:

In D32741#738872, @markj wrote:

Did you consider just changing the semantics of sorele()? Every user of this macro outside of uipc_socket.c contains the pattern SOCK_LOCK(so); sorele(so). Then, uipc_socket.c can use a private routine to drop a ref on a locked socket.

Well, I worried that would be more of a pain with having the API differ between versions (not sure if there are any out-of-tree modules that might use this, but perhaps not?)

It is true that having sorele_locked() and sorele() is probably more typical of the API names we use.

I'm not aware of anything in the ports tree that makes any real use of the socket API.

To be clear, I extracted all sources for kmods in the ports tree and searched for sorele(). Nothing turns up.

If you think it's safe to change the API, then I could make the new one "sorele" and make the old one "sorele_locked"? Note that the new macro still needs to call the old one to drop the last reference, so it can't be a static function in uipc_socket.c (though perhaps sorele_locked() could be a public function in uipc_socket.c?)

I think it'd be good to have an out-of-line sorele_locked() in uipc_socket.c.

[jhb]

• Changed sorele semantics to be unlocked (required merging the two patches)

[kib]

sys/kern/uipc_socket.c

1217

Perhaps mention that it also unlocks?