dahdi: start handling "surprise device removal".

This patch contains interim results while trying to make
device removal work correctly:

 * XPP has protections to prevent dahdi unregistration while
   channels are open -- they are now removed, so we can
   unregister immediately.

 * Handle processes in poll_wait():
   - Wake them during dahdi_chan_unreg() after the channel
     is gone (chan->channo = -1 or chan->file->private_data == NULL)

   - Test in every wait_event_interruptible() that the channel
     was not gone (chan->file->private_data)

   - Return correct values (POLLERR | POLLHUP) instead of
     some errno (would be important in the future if we
     modify asterisk to respond correctly to this condition.

 * Other issues:
   - If unregistered channel is being polled, than call msleep() before
     returning, to give other processes a chance (normally, asterisk
     has RT priority)

   - Call close_channel() from dahdi_chan_unreg() so it releases
     related tonezone

 * There is still a horrible race hidden by msleep(20) in
   dahdi_chan_unreg()

force close channels from dahdi_chan_unreg():

 * Mark them via DAHDI_FLAGBIT_OPEN
 * Call low-level driver close() method if available
 * What about other closing activities?

Signed-off-by: Oron Peled <oron.peled@xorcom.com>
Signed-off-by: Shaun Ruffell <sruffell@digium.com>

git-svn-id: http://svn.asterisk.org/svn/dahdi/linux/trunk@10270 a0bf4364-ded3-4de4-8d8a-66a801d63aff
This commit is contained in:
Oron Peled 2011-10-26 18:56:43 +00:00 committed by Tzafrir Cohen
parent 4b1a2f9127
commit 33c31e9593
2 changed files with 83 additions and 21 deletions

View File

@ -2151,9 +2151,8 @@ static void dahdi_chan_unreg(struct dahdi_chan *chan)
* file handles to this channel are disassociated with the actual * file handles to this channel are disassociated with the actual
* dahdi_chan. */ * dahdi_chan. */
if (chan->file) { if (chan->file) {
module_printk(KERN_NOTICE, "%s: surprise removal\n", __func__);
chan->file->private_data = NULL; chan->file->private_data = NULL;
if (chan->span)
module_put(chan->span->ops->owner);
} }
release_echocan(chan->ec_factory); release_echocan(chan->ec_factory);
@ -2178,6 +2177,40 @@ static void dahdi_chan_unreg(struct dahdi_chan *chan)
spin_unlock_irqrestore(&chan_lock, flags); spin_unlock_irqrestore(&chan_lock, flags);
chan->channo = -1; chan->channo = -1;
/* Let processeses out of their poll_wait() */
wake_up_interruptible(&chan->waitq);
/* release tone_zone */
close_channel(chan);
if (chan->file) {
if (test_bit(DAHDI_FLAGBIT_OPEN, &chan->flags)) {
clear_bit(DAHDI_FLAGBIT_OPEN, &chan->flags);
if (chan->span) {
if (chan->span->ops->close) {
int res;
res = chan->span->ops->close(chan);
if (res)
module_printk(KERN_NOTICE,
"%s: close() failed: %d\n",
__func__, res);
}
}
}
msleep(20);
/*
* FIXME: THE BIG SLEEP above, is hiding a terrible
* race condition:
* - the module_put() ahead, would allow the low-level driver
* to free the channel.
* - We should make sure no-one reference this channel
* from now on.
*/
if (chan->span)
put_span(chan->span);
}
} }
static ssize_t dahdi_chan_read(struct file *file, char __user *usrbuf, static ssize_t dahdi_chan_read(struct file *file, char __user *usrbuf,
@ -2221,10 +2254,15 @@ static ssize_t dahdi_chan_read(struct file *file, char __user *usrbuf,
break; break;
if (file->f_flags & O_NONBLOCK) if (file->f_flags & O_NONBLOCK)
return -EAGAIN; return -EAGAIN;
/* Wake up when data is available or when the board driver
* unregistered the channel. */
rv = wait_event_interruptible(chan->waitq, rv = wait_event_interruptible(chan->waitq,
(chan->outreadbuf > -1)); (!chan->file->private_data || chan->outreadbuf > -1));
if (rv) if (rv)
return rv; return rv;
if (unlikely(!chan->file->private_data))
return -ENODEV;
} }
amnt = count; amnt = count;
if (chan->flags & DAHDI_FLAG_LINEAR) { if (chan->flags & DAHDI_FLAG_LINEAR) {
@ -2348,11 +2386,15 @@ static ssize_t dahdi_chan_write(struct file *file, const char __user *usrbuf,
#endif #endif
return -EAGAIN; return -EAGAIN;
} }
/* Wait for something to be available */
/* Wake up when room in the write queue is available or when
* the board driver unregistered the channel. */
rv = wait_event_interruptible(chan->waitq, rv = wait_event_interruptible(chan->waitq,
(chan->inwritebuf >= 0)); (!chan->file->private_data || chan->inwritebuf > -1));
if (rv) if (rv)
return rv; return rv;
if (unlikely(!chan->file->private_data))
return -ENODEV;
} }
amnt = count; amnt = count;
@ -5411,6 +5453,7 @@ static int dahdi_ioctl_iomux(struct file *file, unsigned long data)
struct dahdi_chan *const chan = chan_from_file(file); struct dahdi_chan *const chan = chan_from_file(file);
unsigned long flags; unsigned long flags;
unsigned int iomask; unsigned int iomask;
int ret = 0;
DEFINE_WAIT(wait); DEFINE_WAIT(wait);
if (get_user(iomask, (int __user *)data)) if (get_user(iomask, (int __user *)data))
@ -5424,6 +5467,15 @@ static int dahdi_ioctl_iomux(struct file *file, unsigned long data)
wait_result = 0; wait_result = 0;
prepare_to_wait(&chan->waitq, &wait, TASK_INTERRUPTIBLE); prepare_to_wait(&chan->waitq, &wait, TASK_INTERRUPTIBLE);
if (unlikely(!chan->file->private_data)) {
static int rate_limit;
if ((rate_limit % 1000) == 0)
module_printk(KERN_NOTICE, "%s: surprise removal\n", __func__);
msleep(5);
ret = -ENODEV;
break;
}
spin_lock_irqsave(&chan->lock, flags); spin_lock_irqsave(&chan->lock, flags);
chan->iomask = iomask; chan->iomask = iomask;
@ -5465,7 +5517,7 @@ static int dahdi_ioctl_iomux(struct file *file, unsigned long data)
spin_lock_irqsave(&chan->lock, flags); spin_lock_irqsave(&chan->lock, flags);
chan->iomask = 0; chan->iomask = 0;
spin_unlock_irqrestore(&chan->lock, flags); spin_unlock_irqrestore(&chan->lock, flags);
return 0; return ret;
} }
#ifdef CONFIG_DAHDI_MIRROR #ifdef CONFIG_DAHDI_MIRROR
@ -5680,9 +5732,11 @@ dahdi_chanandpseudo_ioctl(struct file *file, unsigned int cmd,
if (!i) if (!i)
break; /* skip if none */ break; /* skip if none */
rv = wait_event_interruptible(chan->waitq, rv = wait_event_interruptible(chan->waitq,
(chan->outwritebuf > -1)); (!chan->file->private_data || chan->outwritebuf > -1));
if (rv) if (rv)
return rv; return rv;
if (unlikely(!chan->file->private_data))
return -ENODEV;
} }
break; break;
case DAHDI_IOMUX: /* wait for something to happen */ case DAHDI_IOMUX: /* wait for something to happen */
@ -6355,7 +6409,9 @@ static int dahdi_chan_ioctl(struct file *file, unsigned int cmd, unsigned long d
if (file->f_flags & O_NONBLOCK) if (file->f_flags & O_NONBLOCK)
return -EINPROGRESS; return -EINPROGRESS;
wait_event_interruptible(chan->waitq, wait_event_interruptible(chan->waitq,
is_txstate(chan, DAHDI_TXSIG_ONHOOK)); !chan->file->private_data || is_txstate(chan, DAHDI_TXSIG_ONHOOK));
if (unlikely(!chan->file->private_data))
return -ENODEV;
if (signal_pending(current)) if (signal_pending(current))
return -ERESTARTSYS; return -ERESTARTSYS;
break; break;
@ -6370,7 +6426,9 @@ static int dahdi_chan_ioctl(struct file *file, unsigned int cmd, unsigned long d
if (file->f_flags & O_NONBLOCK) if (file->f_flags & O_NONBLOCK)
return -EINPROGRESS; return -EINPROGRESS;
wait_event_interruptible(chan->waitq, wait_event_interruptible(chan->waitq,
is_txstate(chan, DAHDI_TXSIG_OFFHOOK)); !chan->file->private_data || is_txstate(chan, DAHDI_TXSIG_OFFHOOK));
if (unlikely(!chan->file->private_data))
return -ENODEV;
if (signal_pending(current)) if (signal_pending(current))
return -ERESTARTSYS; return -ERESTARTSYS;
break; break;
@ -8743,8 +8801,14 @@ static unsigned int dahdi_timer_poll(struct file *file, struct poll_table_struct
if (timer->tripped || timer->ping) if (timer->tripped || timer->ping)
ret |= POLLPRI; ret |= POLLPRI;
spin_unlock_irqrestore(&dahdi_timer_lock, flags); spin_unlock_irqrestore(&dahdi_timer_lock, flags);
} else } else {
ret = -EINVAL; static int rate_limit;
if ((rate_limit % 1000) == 0)
module_printk(KERN_NOTICE, "%s: nodev\n", __func__);
msleep(5);
return POLLERR | POLLHUP;
}
return ret; return ret;
} }
@ -8756,8 +8820,14 @@ dahdi_chan_poll(struct file *file, struct poll_table_struct *wait_table)
int ret = 0; int ret = 0;
unsigned long flags; unsigned long flags;
if (unlikely(!c)) if (unlikely(!c)) {
return -EINVAL; static int rate_limit;
if ((rate_limit % 1000) == 0)
module_printk(KERN_NOTICE, "%s: nodev\n", __func__);
msleep(5);
return POLLERR | POLLHUP;
}
poll_wait(file, &c->waitq, wait_table); poll_wait(file, &c->waitq, wait_table);

View File

@ -734,7 +734,6 @@ int xpp_open(struct dahdi_chan *chan)
return -EINVAL; return -EINVAL;
} }
xpd = chan->pvt; xpd = chan->pvt;
xpd = get_xpd(__FUNCTION__, xpd); /* Returned in xpp_close() */
if (!xpd) { if (!xpd) {
NOTICE("open called on a chan with no pvt (xpd)\n"); NOTICE("open called on a chan with no pvt (xpd)\n");
BUG(); BUG();
@ -776,7 +775,6 @@ int xpp_close(struct dahdi_chan *chan)
current->comm, current->pid, current->comm, current->pid,
atomic_read(&PHONEDEV(xpd).open_counter)); atomic_read(&PHONEDEV(xpd).open_counter));
atomic_dec(&PHONEDEV(xpd).open_counter); /* from xpp_open() */ atomic_dec(&PHONEDEV(xpd).open_counter); /* from xpp_open() */
put_xpd(__FUNCTION__, xpd); /* from xpp_open() */
return 0; return 0;
} }
@ -1024,12 +1022,6 @@ int dahdi_unregister_xpd(xpd_t *xpd)
return -EIDRM; return -EIDRM;
} }
update_xpd_status(xpd, DAHDI_ALARM_NOTOPEN); update_xpd_status(xpd, DAHDI_ALARM_NOTOPEN);
/* We should now have only a ref from the xbus (from create_xpd()) */
if(atomic_read(&PHONEDEV(xpd).open_counter)) {
XPD_NOTICE(xpd, "Busy (open_counter=%d). Skipping.\n", atomic_read(&PHONEDEV(xpd).open_counter));
spin_unlock_irqrestore(&xpd->lock, flags);
return -EBUSY;
}
mdelay(2); // FIXME: This is to give chance for transmit/receiveprep to finish. mdelay(2); // FIXME: This is to give chance for transmit/receiveprep to finish.
spin_unlock_irqrestore(&xpd->lock, flags); spin_unlock_irqrestore(&xpd->lock, flags);
if(xpd->card_present) if(xpd->card_present)