On 10/07/21(Sat) 21:53, Vitaliy Makkoveev wrote: > Hi, > > In filt_solisten_common() you touches `so_qlen’ only. It’s not > related to buffer and not protected by introduced `sb_mtx’ so > the solock() replacement in filt_solisten*() is wrong. > > However, in filt_solisten_common() you only checks is > `so_qlen’ != 0 condition and such check could be performed lockless. > I propose you to commit this by separate diff.
The dance is there because knote needs a lock which could be the mutex. > > @@ -208,8 +208,10 @@ uipc_usrreq(struct socket *so, int req, struct mbuf > > *m, struct mbuf *nam, > > * Adjust backpressure on sender > > * and wakeup any waiting to write. > > */ > > + mtx_enter(&so2->so_snd.sb_mtx); > > so2->so_snd.sb_mbcnt = so->so_rcv.sb_mbcnt; > > so2->so_snd.sb_cc = so->so_rcv.sb_cc; > > + mtx_leave(&so2->so_snd.sb_mtx); > > sowwakeup(so2); > > break; > > > > This is 'PRU_RCVD’ case, so you hold solock() on `so’ and it’s receive > buffer is locked by sblock(). Is it assumed `sb_mbcnt’ and `sb_cc’ > modification of `so_rcv’ protected by solock()? Should the both buffers > be locked here? I’m asking because you only remove solock() from kqueue(9) > path and the solock() still serialises the rest of sockets, but you are > going to reduce solock(). > > The same question for 'PRU_SEND’ case. In this case the fields of "so2" are modified. Modifications are always serialized by the solock. The mutex is there to prevent another thread running the kqueue filters to read between the update of `sb_mbcnt' and `sb_cc'. > > @@ -284,8 +286,10 @@ uipc_usrreq(struct socket *so, int req, struct mbuf > > *m, struct mbuf *nam, > > sbappendrecord(so2, &so2->so_rcv, m); > > else > > sbappend(so2, &so2->so_rcv, m); > > + mtx_enter(&so2->so_snd.sb_mtx); > > so->so_snd.sb_mbcnt = so2->so_rcv.sb_mbcnt; > > so->so_snd.sb_cc = so2->so_rcv.sb_cc; > > + mtx_leave(&so2->so_snd.sb_mtx); > > if (so2->so_rcv.sb_cc > 0) > > sorwakeup(so2); > > > Since you touch 'so2->so_rcv’ content here, you want to lock it instead > of 'so2->so_snd’, right? Indeed, that's a mistake, thanks!
