Attached is a diff that implements device cloning to allow the
/dev/hotplug device to be cloned (to allow multiple concurrent readers).
Rationale:
Currently, iscsid/iscsictl cannot see when the connections it initiates
results in a device (disk) being attached. Recently, Claudio Jeker
committed a diff I wrote to block "iscsictl reload" until all
connections have been finalized (either connecting successfully, or
notified of failure). This fixes an issue I encountered where iscsi
mounts in /etc/fstab would not be ready at boot time, and would result
in the machine going into single user mode when fsck would try to access
them.
The committed diff fixes my original problem. However, I've now
encountered another one. On one of my machines (far slower than the
first one), the devices themselves are much slower to attach - this
leads to the same race condition as before, as "iscsictl reload" is only
aware that the connections have been initiated, not whether the drives
themselves are actually attached.
What I am hoping here is that I can use hotplug to monitor for device
attach events, and match those to the targets we expect to be attaching
from iscsi. Obviously, we can't just use the hotplug device as is
because potentialy we'd be competing with hotplugd.
I am wondering if this is the right solution...some quick thoughts i'd
love to hear feedback on:
1. Kind of feels like I'm conflating what hotplug is for - this isn't
really "hotplugging" per se. This specific use-case is assuming the
volumes are listed in /etc/fstab.
2. Based on 1, would it be more appropriate to create a separate device
with it's own semantics (potentially very different from /dev/hotplug)
that lets us do this...
Inspiration for this diff was drawn from Joshua Stein [1], seeminly he
had a use-case that was somewhat similar to mine at one point but it
doesn't look like this was ever committed. Nor can I find any discussion
on it.
[1]: https://jcs.org/2018/08/31/surface_go
Feedback greatly welcomed.
Thanks,
Ash
diff --git a/sys/dev/hotplug.c b/sys/dev/hotplug.c
index e1e7bad95c9..522939f7f7a 100644
--- a/sys/dev/hotplug.c
+++ b/sys/dev/hotplug.c
@@ -25,15 +25,24 @@
#include <sys/fcntl.h>
#include <sys/hotplug.h>
#include <sys/ioctl.h>
+#include <sys/malloc.h>
#include <sys/poll.h>
#include <sys/vnode.h>
#define HOTPLUG_MAXEVENTS 64
-static int opened;
+struct hotplug_dev {
+ int unit;
+ int evqueue_head;
+ struct selinfo sel_info;
+
+ LIST_ENTRY(hotplug_dev) dev_list;
+};
+
+LIST_HEAD(, hotplug_dev) hotplug_dev_list;
+static int evqueue_head;
static struct hotplug_event evqueue[HOTPLUG_MAXEVENTS];
-static int evqueue_head, evqueue_tail, evqueue_count;
-static struct selinfo hotplug_sel;
+
void filt_hotplugrdetach(struct knote *);
int filt_hotplugread(struct knote *, long);
@@ -45,21 +54,40 @@ const struct filterops hotplugread_filtops = {
.f_event = filt_hotplugread,
};
+#define EVQUEUE_PREV(p) (p == 0 ? HOTPLUG_MAXEVENTS : p - 1)
#define EVQUEUE_NEXT(p) (p == HOTPLUG_MAXEVENTS - 1 ? 0 : p + 1)
+#define EMPTY_EVENT(ev) (ev.he_type == HOTPLUG_EMPTY)
+#define EVENT_IS_HEAD(p) (p == evqueue_head)
+#define HEAD_EVENT evqueue[evqueue_head]
int hotplug_put_event(struct hotplug_event *);
-int hotplug_get_event(struct hotplug_event *);
+int hotplug_get_event(struct hotplug_dev *, struct hotplug_event *);
void hotplugattach(int);
+struct hotplug_dev * hotplug_device_lookup(int);
void
hotplugattach(int count)
{
- opened = 0;
- evqueue_head = 0;
- evqueue_tail = 0;
- evqueue_count = 0;
+ int i;
+
+ for (i = 0; i < HOTPLUG_MAXEVENTS; ++i)
+ evqueue[i].he_type = HOTPLUG_EMPTY;
+
+ LIST_INIT(&hotplug_dev_list);
+}
+
+struct hotplug_dev *
+hotplug_device_lookup(int unit)
+{
+ struct hotplug_dev *d;
+
+ LIST_FOREACH(d, &hotplug_dev_list, dev_list)
+ if (d->unit == unit)
+ return d;
+
+ return (NULL);
}
void
@@ -87,59 +115,79 @@ hotplug_device_detach(enum devclass class, char *name)
int
hotplug_put_event(struct hotplug_event *he)
{
- if (evqueue_count == HOTPLUG_MAXEVENTS && opened) {
- printf("hotplug: event lost, queue full\n");
- return (1);
- }
+ struct hotplug_dev *d;
- evqueue[evqueue_head] = *he;
+ HEAD_EVENT = *he;
evqueue_head = EVQUEUE_NEXT(evqueue_head);
- if (evqueue_count == HOTPLUG_MAXEVENTS)
- evqueue_tail = EVQUEUE_NEXT(evqueue_tail);
- else
- evqueue_count++;
+ HEAD_EVENT.he_type = HOTPLUG_EMPTY;
+
+ LIST_FOREACH(d, &hotplug_dev_list, dev_list) {
+ if (EVENT_IS_HEAD(d->evqueue_head))
+ d->evqueue_head = EVQUEUE_NEXT(evqueue_head);
+ selwakeup(&d->sel_info);
+ }
+
+
wakeup(&evqueue);
- selwakeup(&hotplug_sel);
return (0);
}
int
-hotplug_get_event(struct hotplug_event *he)
+hotplug_get_event(struct hotplug_dev *d, struct hotplug_event *he)
{
- int s;
+ int s = splbio();
- if (evqueue_count == 0)
- return (1);
+ if (!EMPTY_EVENT(evqueue[d->evqueue_head])) {
+ *he = evqueue[d->evqueue_head];
+ d->evqueue_head = EVQUEUE_NEXT(d->evqueue_head);
+ splx(s);
+ return (0);
+ }
- s = splbio();
- *he = evqueue[evqueue_tail];
- evqueue_tail = EVQUEUE_NEXT(evqueue_tail);
- evqueue_count--;
splx(s);
- return (0);
+ return (1);
}
int
hotplugopen(dev_t dev, int flag, int mode, struct proc *p)
{
- if (minor(dev) != 0)
- return (ENXIO);
- if ((flag & FWRITE))
+ struct hotplug_dev *d;
+ int i;
+
+ if (flag & FWRITE)
return (EPERM);
- if (opened)
+
+ if ((d = hotplug_device_lookup(minor(dev))) != NULL)
return (EBUSY);
- opened = 1;
+
+ d = malloc(sizeof(*d), M_DEVBUF, M_WAITOK|M_ZERO);
+ d->unit = minor(dev);
+
+ /*
+ * Set the queue head to the earliest non-empty
+ * event. So the new device can read the whole queue.
+ */
+ i = EVQUEUE_PREV(evqueue_head);
+ while (!EMPTY_EVENT(evqueue[i]) && !EVENT_IS_HEAD(i))
+ i = EVQUEUE_PREV(evqueue_head);
+
+ d->evqueue_head = EVQUEUE_NEXT(i);
+ LIST_INSERT_HEAD(&hotplug_dev_list, d, dev_list);
+
return (0);
}
int
hotplugclose(dev_t dev, int flag, int mode, struct proc *p)
{
- struct hotplug_event he;
+ struct hotplug_dev *d;
- while (hotplug_get_event(&he) == 0)
- continue;
- opened = 0;
+ d = hotplug_device_lookup(minor(dev));
+ if (d == NULL)
+ return (EINVAL);
+
+ LIST_REMOVE(d, dev_list);
+ free(d, M_DEVBUF, sizeof(*d));
return (0);
}
@@ -148,12 +196,16 @@ hotplugread(dev_t dev, struct uio *uio, int flags)
{
struct hotplug_event he;
int error;
+ struct hotplug_dev *d = hotplug_device_lookup(minor(dev));
+
+ if (d == NULL)
+ return (ENXIO);
if (uio->uio_resid != sizeof(he))
return (EINVAL);
again:
- if (hotplug_get_event(&he) == 0)
+ if (hotplug_get_event(d, &he) == 0)
return (uiomove(&he, sizeof(he), uio));
if (flags & IO_NDELAY)
return (EAGAIN);
@@ -167,6 +219,12 @@ again:
int
hotplugioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p)
{
+ struct hotplug_dev *d;
+
+ d = hotplug_device_lookup(minor(dev));
+ if (d == NULL)
+ return (ENXIO);
+
switch (cmd) {
case FIOASYNC:
/* ignore */
@@ -184,12 +242,16 @@ int
hotplugpoll(dev_t dev, int events, struct proc *p)
{
int revents = 0;
+ struct hotplug_dev *d;
+
+ if ((d = hotplug_device_lookup(minor(dev))) == NULL)
+ return (POLLERR);
if (events & (POLLIN | POLLRDNORM)) {
- if (evqueue_count > 0)
+ if (!EMPTY_EVENT(evqueue[EVQUEUE_NEXT(d->evqueue_head)]))
revents |= events & (POLLIN | POLLRDNORM);
else
- selrecord(p, &hotplug_sel);
+ selrecord(p, &d->sel_info);
}
return (revents);
@@ -199,11 +261,15 @@ int
hotplugkqfilter(dev_t dev, struct knote *kn)
{
struct klist *klist;
+ struct hotplug_dev *d;
int s;
+ if ((d = hotplug_device_lookup(minor(dev))) == NULL)
+ return (EINVAL);
+
switch (kn->kn_filter) {
case EVFILT_READ:
- klist = &hotplug_sel.si_note;
+ klist = &d->sel_info.si_note;
kn->kn_fop = &hotplugread_filtops;
break;
default:
@@ -211,6 +277,7 @@ hotplugkqfilter(dev_t dev, struct knote *kn)
}
s = splbio();
+ kn->kn_hook = d;
klist_insert_locked(klist, kn);
splx(s);
return (0);
@@ -219,17 +286,21 @@ hotplugkqfilter(dev_t dev, struct knote *kn)
void
filt_hotplugrdetach(struct knote *kn)
{
+ struct hotplug_dev *d;
int s;
+ d = kn->kn_hook;
s = splbio();
- klist_remove_locked(&hotplug_sel.si_note, kn);
+ klist_remove_locked(&d->sel_info.si_note, kn);
splx(s);
}
int
filt_hotplugread(struct knote *kn, long hint)
{
- kn->kn_data = evqueue_count;
+ struct hotplug_dev *d = kn->kn_hook;
+ if (EMPTY_EVENT(evqueue[d->evqueue_head]))
+ return (0);
- return (evqueue_count > 0);
+ return (1);
}
diff --git a/sys/sys/conf.h b/sys/sys/conf.h
index 5783e5244d2..bccf1dd0437 100644
--- a/sys/sys/conf.h
+++ b/sys/sys/conf.h
@@ -437,7 +437,7 @@ extern struct cdevsw cdevsw[];
dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
(dev_type_write((*))) enodev, dev_init(c,n,ioctl), \
(dev_type_stop((*))) enodev, 0, dev_init(c,n,poll), \
- (dev_type_mmap((*))) enodev, 0, 0, dev_init(c,n,kqfilter) }
+ (dev_type_mmap((*))) enodev, 0, D_CLONE, dev_init(c,n,kqfilter) }
/* open, close, ioctl */
#define cdev_gpio_init(c,n) { \
diff --git a/sys/sys/hotplug.h b/sys/sys/hotplug.h
index f2da3f6cfd8..90695e9d024 100644
--- a/sys/sys/hotplug.h
+++ b/sys/sys/hotplug.h
@@ -23,6 +23,7 @@
* attachment and detachment notifications.
*/
+#define HOTPLUG_EMPTY 0x00 /* empty event */
#define HOTPLUG_DEVAT 0x01 /* device attached */
#define HOTPLUG_DEVDT 0x02 /* device detached */
