[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250714044726.GD1880847@ZenIV>
Date: Mon, 14 Jul 2025 05:47:26 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Allison Henderson <allison.henderson@...cle.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [RFC][PATCH] don't open-code kernel_accept() in
rds_tcp_accept_one()
On Mon, Jul 14, 2025 at 04:36:32AM +0000, Allison Henderson wrote:
> > if (!sock) /* module unload or netns delete in progress */
> > return -ENETUNREACH;
> >
> > - ret = sock_create_lite(sock->sk->sk_family,
> > - sock->sk->sk_type, sock->sk->sk_protocol,
> > - &new_sock);
> > + ret = kernel_accept(sock, &new_sock, O_NONBLOCK);
> > if (ret)
> > - goto out;
> > -
> > - ret = sock->ops->accept(sock, new_sock, &arg);
> > - if (ret < 0)
> > - goto out;
> > -
> > - /* sock_create_lite() does not get a hold on the owner module so we
> > - * need to do it here. Note that sock_release() uses sock->ops to
> > - * determine if it needs to decrement the reference count. So set
> > - * sock->ops after calling accept() in case that fails. And there's
> > - * no need to do try_module_get() as the listener should have a hold
> > - * already.
> > - */
> > - new_sock->ops = sock->ops;
> > - __module_get(new_sock->ops->owner);
> > + return ret;
> I think we need the "goto out" here, or we will miss the mutex unlock. Otherwise kernel_accept looks like a pretty
> synonymous wrapper.
What mutex_unlock()?
if (rs_tcp)
mutex_unlock(&rs_tcp->t_conn_path_lock);
won't be triggered, since rs_tcp remains NULL until
rs_tcp = rds_tcp_accept_one_path(conn);
well after any of the affected code...
No, return is perfectly fine here - failing kernel_accept() has no side
effects and we have
if (!sock) /* module unload or netns delete in progress */
return -ENETUNREACH;
just prior to it. So if we needed to unlock anything on kernel_accept()
failure, the same would apply for the failure exit just before it...
Powered by blists - more mailing lists