I have been working on fixing an issue (which was partially fixed by a diff I sent in earlier this year) with iscsid. With iscsi disks in /etc/fstab, sometimes the devices aren't fully up and ready before fsck tries to run - causing the machine to go into single user mode on boot.
The diff that was merged earlier in the year added a poll routine which monitors for connection success before letting iscsictl return. This fixed the issue in some cases, however it's still only half the fix. The principled fix is to, additionally, wait until the scsi_probe calls are complete - at which point we can reasonably assume the disk device are ready for use. This requires some work to the vscsi driver to make this happen, as well as changes to both iscsid and iscsictl. The diffs attached here are a full implementation of this. I was still encountering this issue on one of my machines (much slower than my normal machine), where the connections would be up but the scsi_probe calls would not have completed - even with my earlier diff this would still cause the machine to go to single user mode. However, this indeed fixes that problem completely and I've been running it for a couple of weeks with no problems. The diffs are designed to be applied in the order they appear. In summary, the proposed changes are: (1) taskq.diff adds a taskq_empty function, which lets us check to see if a taskq is, well, empty. This is used in (2). Updates the man pages for taskq/task_add to reflect this. (2) vscsi.diff adds an ioctl to the vscsi driver which lets us monitor for device event queue completion. To aid in this it also separates calls to scsi_probe and scsi_detach to a dedicated taskq, rather than using systq. Updates the man pages for vscsi to reflect this. (3) iscsid.diff does the plumbing to actually call the new ioctl and provide that information back to iscsictl during status poll. (4) iscsictl.diff integrates the device queue information into the polling routine. Updates the man page for iscsictl to describe the new behavior. Based on commmit messages around vscsi it seems there was some plan to do this quite some years ago but it's hard for me to know what the proposed method was (though I assume what was envisaged might be similar to something like this). Feedback sought and greatly welcomed. I am basically certain there are ways this can be improved. Thanks, Ash
Index: sys/sys/task.h =================================================================== RCS file: /cvs/src/sys/sys/task.h,v retrieving revision 1.18 diff -u -p -u -p -r1.18 task.h --- sys/sys/task.h 1 Aug 2020 08:40:20 -0000 1.18 +++ sys/sys/task.h 8 Jun 2021 23:42:00 -0000 @@ -51,6 +51,8 @@ void taskq_barrier(struct taskq *); void taskq_del_barrier(struct taskq *, struct task *); +int taskq_empty(struct taskq *); + void task_set(struct task *, void (*)(void *), void *); int task_add(struct taskq *, struct task *); int task_del(struct taskq *, struct task *); Index: sys/kern/kern_task.c =================================================================== RCS file: /cvs/src/sys/kern/kern_task.c,v retrieving revision 1.31 diff -u -p -u -p -r1.31 kern_task.c --- sys/kern/kern_task.c 1 Aug 2020 08:40:20 -0000 1.31 +++ sys/kern/kern_task.c 8 Jun 2021 23:42:16 -0000 @@ -394,6 +394,18 @@ task_del(struct taskq *tq, struct task * } int +taskq_empty(struct taskq *tq) +{ + int rv; + + mtx_enter(&tq->tq_mtx); + rv = TAILQ_EMPTY(&tq->tq_worklist); + mtx_leave(&tq->tq_mtx); + + return (rv); +} + +int taskq_next_work(struct taskq *tq, struct task *work) { struct task *next; Index: share/man/man9/task_add.9 =================================================================== RCS file: /cvs/src/share/man/man9/task_add.9,v retrieving revision 1.22 diff -u -p -u -p -r1.22 task_add.9 --- share/man/man9/task_add.9 8 Jun 2020 00:29:51 -0000 1.22 +++ share/man/man9/task_add.9 8 Jun 2021 23:42:29 -0000 @@ -20,6 +20,7 @@ .Sh NAME .Nm taskq_create , .Nm taskq_destroy , +.Nm taskq_empty , .Nm taskq_barrier , .Nm taskq_del_barrier , .Nm task_set , @@ -43,6 +44,8 @@ .Fn taskq_barrier "struct taskq *tq" .Ft void .Fn taskq_del_barrier "struct taskq *tq" "struct task *t" +.Ft int +.Fn taskq_empty "struct taskq *tq" .Ft void .Fn task_set "struct task *t" "void (*fn)(void *)" "void *arg" .Ft int @@ -86,6 +89,9 @@ argument: The threads servicing the taskq will be run without the kernel big lock. .El .Pp +.Fn taskq_empty +indicates whether there are any queued tasks. +.Pp .Fn taskq_destroy causes the resources associated with a previously created taskq to be freed. It will wait till all the tasks in the work queue are completed before @@ -220,6 +226,11 @@ or 0 if the task was not already on the .Fn task_pending will return non-zero if the task is queued to run, or 0 if the task is not queued. +.Pp +.Fn taskq_empty +will return 1 if there are no tasks queued, or 0 if there is at least +one task queued. + .Sh SEE ALSO .Xr autoconf 9 , .Xr spl 9
Index: sys/dev/vscsivar.h =================================================================== RCS file: /cvs/src/sys/dev/vscsivar.h,v retrieving revision 1.5 diff -u -p -u -p -r1.5 vscsivar.h --- sys/dev/vscsivar.h 5 Apr 2011 15:28:49 -0000 1.5 +++ sys/dev/vscsivar.h 8 Jun 2021 23:07:53 -0000 @@ -44,7 +44,7 @@ struct vscsi_ioc_data { size_t datalen; }; -#define VSCSI_DATA_READ _IOW('I', 1, struct vscsi_ioc_data) +#define VSCSI_DATA_READ _IOW('I', 1, struct vscsi_ioc_data) #define VSCSI_DATA_WRITE _IOW('I', 2, struct vscsi_ioc_data) struct vscsi_ioc_t2i { @@ -65,7 +65,8 @@ struct vscsi_ioc_devevent { int lun; }; -#define VSCSI_REQPROBE _IOW('I', 4, struct vscsi_ioc_devevent) -#define VSCSI_REQDETACH _IOW('I', 5, struct vscsi_ioc_devevent) +#define VSCSI_REQPROBE _IOW('I', 4, struct vscsi_ioc_devevent) +#define VSCSI_REQDETACH _IOW('I', 5, struct vscsi_ioc_devevent) +#define VSCSI_DEVEVENT_POLL _IOR('I', 6, int) #endif /* _SYS_DEV_VSCSIVAR_H */ Index: sys/dev/vscsi.c =================================================================== RCS file: /cvs/src/sys/dev/vscsi.c,v retrieving revision 1.58 diff -u -p -u -p -r1.58 vscsi.c --- sys/dev/vscsi.c 25 Dec 2020 12:59:52 -0000 1.58 +++ sys/dev/vscsi.c 8 Jun 2021 23:08:04 -0000 @@ -73,6 +73,8 @@ struct vscsi_softc { struct selinfo sc_sel; struct mutex sc_sel_mtx; + + struct taskq *sc_tq; }; #define DEVNAME(_s) ((_s)->sc_dev.dv_xname) @@ -104,6 +106,7 @@ int vscsi_t2i(struct vscsi_softc *, str int vscsi_devevent(struct vscsi_softc *, u_long, struct vscsi_ioc_devevent *); void vscsi_devevent_task(void *); +int vscsi_devevent_poll(struct vscsi_softc *, int *); void vscsi_done(struct vscsi_softc *, struct vscsi_ccb *); void * vscsi_ccb_get(void *); @@ -156,6 +159,10 @@ vscsi_attach(struct device *parent, stru sc->sc_scsibus = (struct scsibus_softc *)config_found(&sc->sc_dev, &saa, scsiprint); + + sc->sc_tq = taskq_create("vscsi", 1, IPL_BIO, 0); + if (sc->sc_tq == NULL) + panic("vscsi: Couldn't allocate task queue."); } void @@ -317,7 +324,9 @@ vscsiioctl(dev_t dev, u_long cmd, caddr_ err = vscsi_devevent(sc, cmd, (struct vscsi_ioc_devevent *)addr); break; - + case VSCSI_DEVEVENT_POLL: + err = vscsi_devevent_poll(sc, (int *) addr); + break; default: err = ENOTTY; break; @@ -487,7 +496,7 @@ vscsi_devevent(struct vscsi_softc *sc, u dt->cmd = cmd; device_ref(&sc->sc_dev); - task_add(systq, &dt->t); + task_add(sc->sc_tq, &dt->t); return (0); } @@ -525,6 +534,17 @@ gone: device_unref(&sc->sc_dev); free(dt, M_TEMP, sizeof(*dt)); +} + +int +vscsi_devevent_poll(struct vscsi_softc *sc, int *tq_status) +{ + if (tq_status == NULL) + return (EINVAL); + + *tq_status = taskq_empty(sc->sc_tq); + + return (0); } int Index: share/man/man4/vscsi.4 =================================================================== RCS file: /cvs/src/share/man/man4/vscsi.4,v retrieving revision 1.14 diff -u -p -u -p -r1.14 vscsi.4 --- share/man/man4/vscsi.4 12 Sep 2017 15:06:20 -0000 1.14 +++ share/man/man4/vscsi.4 8 Jun 2021 23:08:31 -0000 @@ -108,6 +108,11 @@ struct vscsi_ioc_devevent { u_int lun; }; .Ed +.Pp +.It Dv VSCSI_DEVEVENT_POLL Fa "int" +Returns a status flag indicating whether the device event queue (utilized by +VSCSI_REQPROBE and VSCSI_REQDETACH operations) has outstanding events. +.Ed .El .Sh FILES .Bl -tag -width /dev/vscsi0
Index: usr.sbin/iscsid/iscsid.h =================================================================== RCS file: /cvs/src/usr.sbin/iscsid/iscsid.h,v retrieving revision 1.17 diff -u -p -u -p -r1.17 iscsid.h --- usr.sbin/iscsid/iscsid.h 16 Apr 2021 14:37:06 -0000 1.17 +++ usr.sbin/iscsid/iscsid.h 8 Jun 2021 23:10:08 -0000 @@ -259,6 +259,7 @@ struct session_poll { int conn_failed_count; int conn_waiting_count; int sess_conn_status; + int devev_queue_status; }; struct connection { @@ -399,6 +400,7 @@ void vscsi_dispatch(int, short, void *); void vscsi_data(unsigned long, int, void *, size_t); void vscsi_status(int, int, void *, size_t); void vscsi_event(unsigned long, u_int, u_int); +void vscsi_poll_devevent_queue(int *); struct vscsi_stats *vscsi_stats(void); /* Session polling */ Index: usr.sbin/iscsid/poll.c =================================================================== RCS file: /cvs/src/usr.sbin/iscsid/poll.c,v retrieving revision 1.1 diff -u -p -u -p -r1.1 poll.c --- usr.sbin/iscsid/poll.c 16 Apr 2021 14:37:06 -0000 1.1 +++ usr.sbin/iscsid/poll.c 8 Jun 2021 23:10:08 -0000 @@ -82,13 +82,24 @@ poll_session(struct session_poll *p, str } /* - * Set session_poll sess_conn_status based on the number of sessions - * versus number of logged in or failed sessions. If the numbers are - * equal iscsictl can stop polling since all sessions reached a stable state. + * Here, we finalize the polling by checking to see if + * the requested probe/detach ops have been completed + * by the vscsi driver. We only bother to do this once + * we know all the sessions have settled. */ void poll_finalize(struct session_poll *p) { + + /* + * If we aren't awaiting anything else to connect, the combined total of + * logged in and failed connections should equal the number of + * configured sessions. + */ p->sess_conn_status = (p->session_count == p->conn_logged_in_count + p->conn_failed_count); + + /* All connections up, so check the dev event queue. */ + if (p->sess_conn_status) + vscsi_poll_devevent_queue(&p->devev_queue_status); } Index: usr.sbin/iscsid/vscsi.c =================================================================== RCS file: /cvs/src/usr.sbin/iscsid/vscsi.c,v retrieving revision 1.17 diff -u -p -u -p -r1.17 vscsi.c --- usr.sbin/iscsid/vscsi.c 16 Aug 2016 18:41:57 -0000 1.17 +++ usr.sbin/iscsid/vscsi.c 8 Jun 2021 23:10:08 -0000 @@ -338,3 +338,10 @@ vscsi_stats(void) { return &v.stats; } + +void +vscsi_poll_devevent_queue(int *arg) +{ + if (ioctl(v.fd, VSCSI_DEVEVENT_POLL, arg) != 0) + fatal("vscsi_devevent_poll"); +}
Index: usr.sbin/iscsictl/iscsictl.8 =================================================================== RCS file: /cvs/src/usr.sbin/iscsictl/iscsictl.8,v retrieving revision 1.7 diff -u -p -u -p -r1.7 iscsictl.8 --- usr.sbin/iscsictl/iscsictl.8 5 May 2021 12:34:12 -0000 1.7 +++ usr.sbin/iscsictl/iscsictl.8 8 Jun 2021 23:10:23 -0000 @@ -47,8 +47,9 @@ to communicate with The following commands are available: .Bl -tag -width Ds .It Cm reload -Reload the configuration file and wait to return until iscsid reports all -connections have completed (successfully or otherwise), or for up to 10 +Reload the configuration file and wait to return until iscsid reports +all connections have completed (successfully or otherwise) and all +scsi_probe and scsi_detach requests have completed, or for up to 10 seconds. .It Cm show Op Cm summary Show a list of all configured sessions. Index: usr.sbin/iscsictl/iscsictl.c =================================================================== RCS file: /cvs/src/usr.sbin/iscsictl/iscsictl.c,v retrieving revision 1.12 diff -u -p -u -p -r1.12 iscsictl.c --- usr.sbin/iscsictl/iscsictl.c 16 Apr 2021 14:39:33 -0000 1.12 +++ usr.sbin/iscsictl/iscsictl.c 8 Jun 2021 23:10:23 -0000 @@ -431,7 +431,7 @@ poll_and_wait(void) poll_session_status(); /* Poll says we are good to go. */ - if (poll_result.sess_conn_status != 0) { + if (poll_result.sess_conn_status != 0 && poll_result.devev_queue_status != 0) { printf("ok\n"); /* wait a bit longer so all is settled. */ sleep(POLL_DELAY_SEC);