Thread (1 message) 1 message, 1 author, 2016-01-14

Re: [PATCH] af_unix: Use kfree for addresses in unix_bind

From: Rainer Weikusat <hidden>
Date: 2016-01-14 19:05:31
Also in: lkml

Eric Dumazet [off-list ref] writes:
On Thu, 2016-01-14 at 16:22 +0000, Rainer Weikusat wrote:
quoted
Use kfree instead of unix_release_addr when freeing newly-allocated
unix_address structures after binding the socket failed. The second
function does an atomic_dec_and_test in order to free the address once
its reference count falls to zero which isn't necessary for the
unix_bind error path as the new structure wasn't published yet. 'Using
kfree' is also how unix_autobind handles this case.

Signed-off-by: Rainer Weikusat <redacted>
---
This looks net-next material ?
The patch was against net-next. But it's not exactly a new feature.
net-next tree is closed during merge window.
Sorry, I didn't know that.
Not sure what you gain by optimizing error paths ...
What does the error path gain by being differently implemented in
two functionally closely related functions (unix_bind and unix_autobind)?
I already lost the (small amount of) time it took to determine that
there's no reason why unix_release_addr should be called in one case but
not in the other. But this may well help someone else avoid the effort
in future.

With a little more perspective: Something I'd like to do is to shrink
the u->readlock protected critical sections in _bind and _autobind
somewhat. At least the memory allocations certainly don't need to be
protected by the lock. This would mean moving the allocation code and
consequently, the freeing code, too. And changing the code of one
function but keeping the unix_release_lock, perhaps with a comment a la

/* This is useless. But it always grew here. */

while doing the same with the kfree in the other just felt too bizarre.

NB: That's "my" answer: It adds entropy to the code for no gain. One
could also argue that the error path shouldn't execute atomic
instructions for no purpose.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help