On Thu, Sep 28, 2023 at 01:58:45PM +0300, Vitaliy Makkoveev wrote: > filt_vscsiread() checks `sc_ccb_i2t' protected by `sc_state_mtx' > mutex(9), so use it to protect `sc_klist' knotes list too. > > Tested with iscsid(8).
Your diff removes a device_unref(&sc->sc_dev) call in filt_vscsidetach() which seems dubious to me since the reference is still taken in vscsikqfilter(). > Index: sys/dev/vscsi.c > =================================================================== > RCS file: /cvs/src/sys/dev/vscsi.c,v > retrieving revision 1.61 > diff -u -p -r1.61 vscsi.c > --- sys/dev/vscsi.c 2 Jul 2022 08:50:41 -0000 1.61 > +++ sys/dev/vscsi.c 28 Sep 2023 10:47:19 -0000 > @@ -27,13 +27,18 @@ > #include <sys/pool.h> > #include <sys/task.h> > #include <sys/ioctl.h> > -#include <sys/selinfo.h> > +#include <sys/event.h> > > #include <scsi/scsi_all.h> > #include <scsi/scsiconf.h> > > #include <dev/vscsivar.h> > > +/* > + * Locks used to protect struct members and global data > + * s sc_state_mtx > + */ > + > int vscsi_match(struct device *, void *, void *); > void vscsi_attach(struct device *, struct device *, void *); > void vscsi_shutdown(void *); > @@ -64,14 +69,13 @@ struct vscsi_softc { > > struct scsi_iopool sc_iopool; > > - struct vscsi_ccb_list sc_ccb_i2t; > + struct vscsi_ccb_list sc_ccb_i2t; /* [s] */ > struct vscsi_ccb_list sc_ccb_t2i; > int sc_ccb_tag; > struct mutex sc_poll_mtx; > struct rwlock sc_ioc_lock; > > - struct selinfo sc_sel; > - struct mutex sc_sel_mtx; > + struct klist sc_klist; /* [s] */ > }; > > #define DEVNAME(_s) ((_s)->sc_dev.dv_xname) > @@ -110,12 +114,16 @@ void vscsi_ccb_put(void *, void *); > > void filt_vscsidetach(struct knote *); > int filt_vscsiread(struct knote *, long); > +int filt_vscsimodify(struct kevent *, struct knote *); > +int filt_vscsiprocess(struct knote *, struct kevent *); > > const struct filterops vscsi_filtops = { > - .f_flags = FILTEROP_ISFD, > + .f_flags = FILTEROP_ISFD | FILTEROP_MPSAFE, > .f_attach = NULL, > .f_detach = filt_vscsidetach, > .f_event = filt_vscsiread, > + .f_modify = filt_vscsimodify, > + .f_process = filt_vscsiprocess, > }; > > > @@ -133,15 +141,15 @@ vscsi_attach(struct device *parent, stru > > printf("\n"); > > - mtx_init(&sc->sc_state_mtx, IPL_BIO); > + mtx_init(&sc->sc_state_mtx, IPL_MPFLOOR); > sc->sc_state = VSCSI_S_CLOSED; > > TAILQ_INIT(&sc->sc_ccb_i2t); > TAILQ_INIT(&sc->sc_ccb_t2i); > mtx_init(&sc->sc_poll_mtx, IPL_BIO); > - mtx_init(&sc->sc_sel_mtx, IPL_BIO); > rw_init(&sc->sc_ioc_lock, "vscsiioc"); > scsi_iopool_init(&sc->sc_iopool, sc, vscsi_ccb_get, vscsi_ccb_put); > + klist_init_mutex(&sc->sc_klist, &sc->sc_state_mtx); > > saa.saa_adapter = &vscsi_switch; > saa.saa_adapter_softc = sc; > @@ -181,6 +189,7 @@ vscsi_cmd(struct scsi_xfer *xs) > running = 1; > TAILQ_INSERT_TAIL(&sc->sc_ccb_i2t, ccb, ccb_entry); > } > + knote_locked(&sc->sc_klist, 0); > mtx_leave(&sc->sc_state_mtx); > > if (!running) { > @@ -189,8 +198,6 @@ vscsi_cmd(struct scsi_xfer *xs) > return; > } > > - selwakeup(&sc->sc_sel); > - > if (polled) { > mtx_enter(&sc->sc_poll_mtx); > while (ccb->ccb_xs != NULL) > @@ -530,13 +537,10 @@ int > vscsikqfilter(dev_t dev, struct knote *kn) > { > struct vscsi_softc *sc = DEV2SC(dev); > - struct klist *klist; > > if (sc == NULL) > return (ENXIO); > > - klist = &sc->sc_sel.si_note; > - > switch (kn->kn_filter) { > case EVFILT_READ: > kn->kn_fop = &vscsi_filtops; > @@ -547,10 +551,7 @@ vscsikqfilter(dev_t dev, struct knote *k > } > > kn->kn_hook = sc; > - > - mtx_enter(&sc->sc_sel_mtx); > - klist_insert_locked(klist, kn); > - mtx_leave(&sc->sc_sel_mtx); > + klist_insert(&sc->sc_klist, kn); > > /* device ref is given to the knote in the klist */ > > @@ -561,27 +562,42 @@ void > filt_vscsidetach(struct knote *kn) > { > struct vscsi_softc *sc = kn->kn_hook; > - struct klist *klist = &sc->sc_sel.si_note; > - > - mtx_enter(&sc->sc_sel_mtx); > - klist_remove_locked(klist, kn); > - mtx_leave(&sc->sc_sel_mtx); > > - device_unref(&sc->sc_dev); > + klist_remove(&sc->sc_klist, kn); > } > > int > filt_vscsiread(struct knote *kn, long hint) > { > struct vscsi_softc *sc = kn->kn_hook; > - int event = 0; > + > + return (!TAILQ_EMPTY(&sc->sc_ccb_i2t)); > +} > + > +int > +filt_vscsimodify(struct kevent *kev, struct knote *kn) > +{ > + struct vscsi_softc *sc = kn->kn_hook; > + int active; > + > + mtx_enter(&sc->sc_state_mtx); > + active = knote_modify(kev, kn); > + mtx_leave(&sc->sc_state_mtx); > + > + return (active); > +} > + > +int > +filt_vscsiprocess(struct knote *kn, struct kevent *kev) > +{ > + struct vscsi_softc *sc = kn->kn_hook; > + int active; > > mtx_enter(&sc->sc_state_mtx); > - if (!TAILQ_EMPTY(&sc->sc_ccb_i2t)) > - event = 1; > + active = knote_process(kn, kev); > mtx_leave(&sc->sc_state_mtx); > > - return (event); > + return (active); > } > > int > -- :wq Claudio