On Tue, Jan 31, 2023 at 06:00:45PM +0000, Visa Hankala wrote:
> On Tue, Jan 31, 2023 at 12:44:47PM +0300, Vitaliy Makkoveev wrote:
> > Since we have soalloc() to do common socket initialization, move the
> > rest within. I mostly need to do this because standalone socket's buffer
> > locking require to introduce another klistops data for buffers and there
> > is no reason to add more copypaste to sonewconn().
> >
> > Also this makes `socket_klistops' private to kern/uipc_socket.c
> >
> > @@ -226,9 +225,6 @@ sonewconn(struct socket *head, int conns
> > so->so_rcv.sb_lowat = head->so_rcv.sb_lowat;
> > so->so_rcv.sb_timeo_nsecs = head->so_rcv.sb_timeo_nsecs;
> >
> > - klist_init(&so->so_rcv.sb_klist, &socket_klistops, so);
> > - klist_init(&so->so_snd.sb_klist, &socket_klistops, so);
> > - sigio_init(&so->so_sigio);
> > sigio_copy(&so->so_sigio, &head->so_sigio);
>
> With this change, something should call klist_free() and sigio_free()
> for 'so' if soreserve() fails in sonewconn().
>
klist_init() and sigio_init() alloc nothing, but for consistency reason
they shold.
I like to do this in the combined error path for soneconn() and
pru_attach().
Index: sys/kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.299
diff -u -p -r1.299 uipc_socket.c
--- sys/kern/uipc_socket.c 27 Jan 2023 21:01:59 -0000 1.299
+++ sys/kern/uipc_socket.c 31 Jan 2023 18:44:57 -0000
@@ -112,6 +112,16 @@ const struct filterops soexcept_filtops
.f_process = filt_soprocess,
};
+void klist_soassertlk(void *);
+int klist_solock(void *);
+void klist_sounlock(void *, int);
+
+const struct klistops socket_klistops = {
+ .klo_assertlk = klist_soassertlk,
+ .klo_lock = klist_solock,
+ .klo_unlock = klist_sounlock,
+};
+
#ifndef SOMINCONN
#define SOMINCONN 80
#endif /* SOMINCONN */
@@ -148,6 +158,11 @@ soalloc(int wait)
return (NULL);
rw_init_flags(&so->so_lock, "solock", RWL_DUPOK);
refcnt_init(&so->so_refcnt);
+ klist_init(&so->so_rcv.sb_klist, &socket_klistops, so);
+ klist_init(&so->so_snd.sb_klist, &socket_klistops, so);
+ sigio_init(&so->so_sigio);
+ TAILQ_INIT(&so->so_q0);
+ TAILQ_INIT(&so->so_q);
return (so);
}
@@ -176,11 +191,6 @@ socreate(int dom, struct socket **aso, i
if (prp->pr_type != type)
return (EPROTOTYPE);
so = soalloc(M_WAIT);
- klist_init(&so->so_rcv.sb_klist, &socket_klistops, so);
- klist_init(&so->so_snd.sb_klist, &socket_klistops, so);
- sigio_init(&so->so_sigio);
- TAILQ_INIT(&so->so_q0);
- TAILQ_INIT(&so->so_q);
so->so_type = type;
if (suser(p) == 0)
so->so_state = SS_PRIV;
@@ -2333,12 +2343,6 @@ klist_sounlock(void *arg, int ls)
sounlock(so);
}
-
-const struct klistops socket_klistops = {
- .klo_assertlk = klist_soassertlk,
- .klo_lock = klist_solock,
- .klo_unlock = klist_sounlock,
-};
#ifdef DDB
void
Index: sys/kern/uipc_socket2.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.134
diff -u -p -r1.134 uipc_socket2.c
--- sys/kern/uipc_socket2.c 27 Jan 2023 18:46:34 -0000 1.134
+++ sys/kern/uipc_socket2.c 31 Jan 2023 18:44:57 -0000
@@ -41,7 +41,6 @@
#include <sys/socket.h>
#include <sys/socketvar.h>
#include <sys/signalvar.h>
-#include <sys/event.h>
#include <sys/pool.h>
/*
@@ -213,12 +212,8 @@ sonewconn(struct socket *head, int conns
/*
* Inherit watermarks but those may get clamped in low mem situations.
*/
- if (soreserve(so, head->so_snd.sb_hiwat, head->so_rcv.sb_hiwat)) {
- if (persocket)
- sounlock(so);
- pool_put(&socket_pool, so);
- return (NULL);
- }
+ if (soreserve(so, head->so_snd.sb_hiwat, head->so_rcv.sb_hiwat))
+ goto error;
so->so_snd.sb_wat = head->so_snd.sb_wat;
so->so_snd.sb_lowat = head->so_snd.sb_lowat;
so->so_snd.sb_timeo_nsecs = head->so_snd.sb_timeo_nsecs;
@@ -226,9 +221,6 @@ sonewconn(struct socket *head, int conns
so->so_rcv.sb_lowat = head->so_rcv.sb_lowat;
so->so_rcv.sb_timeo_nsecs = head->so_rcv.sb_timeo_nsecs;
- klist_init(&so->so_rcv.sb_klist, &socket_klistops, so);
- klist_init(&so->so_snd.sb_klist, &socket_klistops, so);
- sigio_init(&so->so_sigio);
sigio_copy(&so->so_sigio, &head->so_sigio);
soqinsque(head, so, 0);
@@ -259,13 +251,7 @@ sonewconn(struct socket *head, int conns
if (error) {
soqremque(so, 0);
- if (persocket)
- sounlock(so);
- sigio_free(&so->so_sigio);
- klist_free(&so->so_rcv.sb_klist);
- klist_free(&so->so_snd.sb_klist);
- pool_put(&socket_pool, so);
- return (NULL);
+ goto error;
}
if (connstatus) {
@@ -280,6 +266,16 @@ sonewconn(struct socket *head, int conns
sounlock(so);
return (so);
+
+error:
+ if (persocket)
+ sounlock(so);
+ sigio_free(&so->so_sigio);
+ klist_free(&so->so_rcv.sb_klist);
+ klist_free(&so->so_snd.sb_klist);
+ pool_put(&socket_pool, so);
+
+ return (NULL);
}
void
Index: sys/sys/event.h
===================================================================
RCS file: /cvs/src/sys/sys/event.h,v
retrieving revision 1.67
diff -u -p -r1.67 event.h
--- sys/sys/event.h 31 Mar 2022 01:41:22 -0000 1.67
+++ sys/sys/event.h 31 Jan 2023 18:44:57 -0000
@@ -286,7 +286,6 @@ struct timespec;
extern const struct filterops sig_filtops;
extern const struct filterops dead_filtops;
-extern const struct klistops socket_klistops;
extern void kqpoll_init(unsigned int);
extern void kqpoll_done(unsigned int);
