Hi,
this is a cleanup of printer.c. It removes sleep_on and relatives, kills
module removal races and fixes return values.
It compiles, but I lack the hardware to test.
Regards
Oliver
--- printer.c.alt Thu Nov 29 16:56:25 2001
+++ printer.c Sat Dec 1 13:14:29 2001
@@ -20,6 +20,7 @@
* v0.7 - fixed bulk-IN read and poll (David Paschal, [EMAIL PROTECTED])
* v0.8 - add devfs support
* v0.9 - fix unplug-while-open paths
+ * v0.10 - remove sleep_on and add module reference counting
([EMAIL PROTECTED])
*/
/*
@@ -98,11 +99,14 @@
unsigned int quirks; /* quirks flags */
unsigned char used; /* True if open */
unsigned char bidir; /* interface is bidirectional
*/
+ unsigned char readable; /* True if data was
read */
+ unsigned char writeable; /* True if data was
written */
unsigned char *device_id_string; /* IEEE 1284 DEVICE ID string
(ptr) */
/* first 2 bytes are
(big-endian) length */
};
extern devfs_handle_t usb_devfs_handle; /* /dev/usb dir. */
+static DECLARE_WAIT_QUEUE_HEAD(usblp_waiters);
static struct usblp *usblp_table[USBLP_MINORS];
@@ -148,10 +152,10 @@
usblp_ctrl_msg(usblp, USBLP_REQ_RESET, USB_DIR_OUT, USB_RECIP_OTHER, 0,
NULL, 0)
/*
- * URB callback.
+ * URB callbacks.
*/
-static void usblp_bulk(struct urb *urb)
+static void usblp_read_callback(struct urb *urb)
{
struct usblp *usblp = urb->context;
@@ -159,12 +163,28 @@
return;
if (urb->status)
- warn("usblp%d: nonzero read/write bulk status received: %d",
+ warn("usblp%d: nonzero read bulk status received: %d",
usblp->minor, urb->status);
+ usblp->readable = 1;
wake_up_interruptible(&usblp->wait);
}
+static void usblp_write_callback(struct urb *urb)
+{
+ struct usblp *usblp = urb->context;
+
+ if (!usblp || !usblp->dev || !usblp->used)
+ return;
+
+ if (urb->status)
+ warn("usblp%d: nonzero write bulk status received: %d",
+ usblp->minor, urb->status);
+
+ usblp->writeable = 1;
+ wake_up_interruptible(&usblp->wait);
+}
+
/*
* Get and print printer errors.
*/
@@ -208,6 +228,7 @@
if (minor < 0 || minor >= USBLP_MINORS)
return -ENODEV;
+ MOD_INC_USE_COUNT;
lock_kernel();
usblp = usblp_table[minor];
@@ -238,6 +259,8 @@
usblp->writeurb.transfer_buffer_length = 0;
usblp->writeurb.status = 0;
+ usblp->readable = 0;
+ usblp->writeable = 1; /* in the beginning we can write */
if (usblp->bidir) {
usblp->readcount = 0;
@@ -250,6 +273,8 @@
}
out:
unlock_kernel();
+ if (retval < 0)
+ MOD_DEC_USE_COUNT;
return retval;
}
@@ -279,6 +304,7 @@
} else /* finish cleanup from disconnect */
usblp_cleanup (usblp);
unlock_kernel();
+ MOD_DEC_USE_COUNT;
return 0;
}
@@ -371,31 +397,46 @@
{
struct usblp *usblp = file->private_data;
int timeout, err = 0, writecount = 0;
+ DECLARE_WAITQUEUE(wait, current);
while (writecount < count) {
- // FIXME: only use urb->status inside completion
- // callbacks; this way is racey...
- if (usblp->writeurb.status == -EINPROGRESS) {
+ if(!usblp->dev)
+ return -ENODEV;
+
+ if (!usblp->writeable) {
if (file->f_flags & O_NONBLOCK)
- return -EAGAIN;
+ return writecount ? writecount : -EAGAIN;
timeout = USBLP_WRITE_TIMEOUT;
- while (timeout && usblp->writeurb.status == -EINPROGRESS) {
-
- if (signal_pending(current))
- return writecount ? writecount : -EINTR;
- timeout = interruptible_sleep_on_timeout(&usblp->wait,
timeout);
+ add_wait_queue(&usblp_waiters, &wait);
+ for (;;) {
+ if (!timeout) {
+ remove_wait_queue(&usblp_waiters,
&wait);
+ return writecount ? writecount :
-EIO;
+ }
+
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ if (!usblp->writeable) {
+ if (signal_pending(current)) {
+
set_current_state(TASK_RUNNING);
+
remove_wait_queue(&usblp_waiters, &wait);
+ return writecount ? writecount :
+-EINTR;
+ }
+
+ timeout = schedule_timeout(timeout);
+ } else {
+ break; /* We can now write */
+ }
}
+ remove_wait_queue(&usblp_waiters, &wait);
+ set_current_state(TASK_RUNNING);
}
down (&usblp->sem);
- if (!usblp->dev) {
- up (&usblp->sem);
- return -ENODEV;
- }
if (usblp->writeurb.status != 0) {
if (usblp->quirks & USBLP_QUIRK_BIDIR) {
@@ -428,8 +469,10 @@
if (copy_from_user(usblp->writeurb.transfer_buffer, buffer +
writecount,
usblp->writeurb.transfer_buffer_length)) return
-EFAULT;
+ usblp->writeable = 0;
usblp->writeurb.dev = usblp->dev;
- usb_submit_urb(&usblp->writeurb);
+ if (0 > usb_submit_urb(&usblp->writeurb))
+ count = -EIO;
up (&usblp->sem);
}
@@ -439,6 +482,7 @@
static ssize_t usblp_read(struct file *file, char *buffer, size_t count,
loff_t *ppos)
{
struct usblp *usblp = file->private_data;
+ DECLARE_WAITQUEUE(wait, current);
if (!usblp->bidir)
return -EINVAL;
@@ -449,30 +493,38 @@
goto done;
}
- if (usblp->readurb.status == -EINPROGRESS) {
+ if (!usblp->readable) {
if (file->f_flags & O_NONBLOCK) {
count = -EAGAIN;
goto done;
}
- // FIXME: only use urb->status inside completion
- // callbacks; this way is racey...
- while (usblp->readurb.status == -EINPROGRESS) {
+ add_wait_queue(&usblp_waiters, &wait);
+ for (;;) {
if (signal_pending(current)) {
count = -EINTR;
+ remove_wait_queue(&usblp_waiters, &wait);
goto done;
}
- up (&usblp->sem);
- interruptible_sleep_on(&usblp->wait);
- down (&usblp->sem);
+ if (!usblp->dev) { /* We drop the lock below, thus
we need to recheck */
+ count = -ENODEV;
+ remove_wait_queue(&usblp_waiters, &wait);
+ goto done;
+ }
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (!usblp->readable) {
+ up (&usblp->sem);
+ schedule();
+ down (&usblp->sem);
+ } else {
+ break;
+ }
}
+ remove_wait_queue(&usblp_waiters, &wait);
+ set_current_state(TASK_RUNNING);
}
- if (!usblp->dev) {
- count = -ENODEV;
- goto done;
- }
if (usblp->readurb.status) {
err("usblp%d: error %d reading from printer",
@@ -495,6 +547,7 @@
if ((usblp->readcount += count) == usblp->readurb.actual_length) {
usblp->readcount = 0;
usblp->readurb.dev = usblp->dev;
+ usblp->readable = 0;
usb_submit_urb(&usblp->readurb);
}
@@ -554,6 +607,8 @@
char *buf;
char name[6];
+ MOD_INC_USE_COUNT;
+
/* If a bidirectional interface exists, use it. */
for (i = 0; i < dev->actconfig->interface[ifnum].num_altsetting; i++) {
@@ -580,26 +635,26 @@
if ((epwrite->bEndpointAddress & 0x80) == 0x80) {
if (interface->bNumEndpoints == 1)
- return NULL;
+ goto err_out;
epwrite = interface->endpoint + 1;
epread = bidir ? interface->endpoint + 0 : NULL;
}
if ((epwrite->bEndpointAddress & 0x80) == 0x80)
- return NULL;
+ goto err_out;
if (bidir && (epread->bEndpointAddress & 0x80) != 0x80)
- return NULL;
+ goto err_out;
for (minor = 0; minor < USBLP_MINORS && usblp_table[minor]; minor++);
if (usblp_table[minor]) {
err("no more free usblp devices");
- return NULL;
+ goto err_out;
}
if (!(usblp = kmalloc(sizeof(struct usblp), GFP_KERNEL))) {
err("out of memory");
- return NULL;
+ goto err_out;
}
memset(usblp, 0, sizeof(struct usblp));
init_MUTEX (&usblp->sem);
@@ -625,22 +680,22 @@
if (!(buf = kmalloc(USBLP_BUF_SIZE * (bidir ? 2 : 1), GFP_KERNEL))) {
err("out of memory");
kfree(usblp);
- return NULL;
+ goto err_out;
}
if (!(usblp->device_id_string = kmalloc(DEVICE_ID_SIZE, GFP_KERNEL))) {
err("out of memory");
kfree(usblp);
kfree(buf);
- return NULL;
+ goto err_out;
}
FILL_BULK_URB(&usblp->writeurb, dev, usb_sndbulkpipe(dev,
epwrite->bEndpointAddress),
- buf, 0, usblp_bulk, usblp);
+ buf, 0, usblp_write_callback, usblp);
if (bidir)
FILL_BULK_URB(&usblp->readurb, dev, usb_rcvbulkpipe(dev,
epread->bEndpointAddress),
- buf + USBLP_BUF_SIZE, USBLP_BUF_SIZE, usblp_bulk, usblp);
+ buf + USBLP_BUF_SIZE, USBLP_BUF_SIZE, usblp_read_callback,
+usblp);
/* Get the device_id string if possible. FIXME: Could make this
kmalloc(length). */
err = usblp_get_id(usblp, 0, usblp->device_id_string, DEVICE_ID_SIZE - 1);
@@ -675,7 +730,12 @@
info("usblp%d: USB %sdirectional printer dev %d if %d alt %d",
minor, bidir ? "Bi" : "Uni", dev->devnum, ifnum, alts);
+ MOD_DEC_USE_COUNT;
return usblp_table[minor] = usblp;
+
+err_out:
+ MOD_DEC_USE_COUNT;
+ return NULL;
}
static void usblp_disconnect(struct usb_device *dev, void *ptr)
@@ -687,6 +747,7 @@
BUG ();
}
+ MOD_INC_USE_COUNT;
down (&usblp->sem);
lock_kernel();
usblp->dev = NULL;
@@ -700,6 +761,7 @@
else /* cleanup later, on close */
up (&usblp->sem);
unlock_kernel();
+ MOD_DEC_USE_COUNT;
}
static struct usb_device_id usblp_ids [] = {
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
