From 9cbd090e5dc8fbd80b58f7d8e0e42ee9e52ca526 Mon Sep 17 00:00:00 2001 From: Tzafrir Cohen Date: Wed, 26 Oct 2011 18:56:43 +0000 Subject: 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 Signed-off-by: Shaun Ruffell git-svn-id: http://svn.asterisk.org/svn/dahdi/linux/trunk@10270 a0bf4364-ded3-4de4-8d8a-66a801d63aff --- drivers/dahdi/dahdi-base.c | 96 +++++++++++++++++++++++++++++++++++++------ drivers/dahdi/xpp/xpp_dahdi.c | 8 ---- 2 files changed, 83 insertions(+), 21 deletions(-) diff --git a/drivers/dahdi/dahdi-base.c b/drivers/dahdi/dahdi-base.c index f01074e..5712482 100644 --- a/drivers/dahdi/dahdi-base.c +++ b/drivers/dahdi/dahdi-base.c @@ -2151,9 +2151,8 @@ static void dahdi_chan_unreg(struct dahdi_chan *chan) * file handles to this channel are disassociated with the actual * dahdi_chan. */ if (chan->file) { + module_printk(KERN_NOTICE, "%s: surprise removal\n", __func__); chan->file->private_data = NULL; - if (chan->span) - module_put(chan->span->ops->owner); } release_echocan(chan->ec_factory); @@ -2178,6 +2177,40 @@ static void dahdi_chan_unreg(struct dahdi_chan *chan) spin_unlock_irqrestore(&chan_lock, flags); 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, @@ -2221,10 +2254,15 @@ static ssize_t dahdi_chan_read(struct file *file, char __user *usrbuf, break; if (file->f_flags & O_NONBLOCK) return -EAGAIN; + + /* Wake up when data is available or when the board driver + * unregistered the channel. */ rv = wait_event_interruptible(chan->waitq, - (chan->outreadbuf > -1)); + (!chan->file->private_data || chan->outreadbuf > -1)); if (rv) return rv; + if (unlikely(!chan->file->private_data)) + return -ENODEV; } amnt = count; if (chan->flags & DAHDI_FLAG_LINEAR) { @@ -2348,11 +2386,15 @@ static ssize_t dahdi_chan_write(struct file *file, const char __user *usrbuf, #endif 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, - (chan->inwritebuf >= 0)); + (!chan->file->private_data || chan->inwritebuf > -1)); if (rv) return rv; + if (unlikely(!chan->file->private_data)) + return -ENODEV; } 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); unsigned long flags; unsigned int iomask; + int ret = 0; DEFINE_WAIT(wait); 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; 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); chan->iomask = iomask; @@ -5465,7 +5517,7 @@ static int dahdi_ioctl_iomux(struct file *file, unsigned long data) spin_lock_irqsave(&chan->lock, flags); chan->iomask = 0; spin_unlock_irqrestore(&chan->lock, flags); - return 0; + return ret; } #ifdef CONFIG_DAHDI_MIRROR @@ -5680,9 +5732,11 @@ dahdi_chanandpseudo_ioctl(struct file *file, unsigned int cmd, if (!i) break; /* skip if none */ rv = wait_event_interruptible(chan->waitq, - (chan->outwritebuf > -1)); + (!chan->file->private_data || chan->outwritebuf > -1)); if (rv) return rv; + if (unlikely(!chan->file->private_data)) + return -ENODEV; } break; 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) return -EINPROGRESS; 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)) return -ERESTARTSYS; 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) return -EINPROGRESS; 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)) return -ERESTARTSYS; break; @@ -8743,8 +8801,14 @@ static unsigned int dahdi_timer_poll(struct file *file, struct poll_table_struct if (timer->tripped || timer->ping) ret |= POLLPRI; spin_unlock_irqrestore(&dahdi_timer_lock, flags); - } else - ret = -EINVAL; + } else { + static int rate_limit; + + if ((rate_limit % 1000) == 0) + module_printk(KERN_NOTICE, "%s: nodev\n", __func__); + msleep(5); + return POLLERR | POLLHUP; + } return ret; } @@ -8756,8 +8820,14 @@ dahdi_chan_poll(struct file *file, struct poll_table_struct *wait_table) int ret = 0; unsigned long flags; - if (unlikely(!c)) - return -EINVAL; + if (unlikely(!c)) { + 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); diff --git a/drivers/dahdi/xpp/xpp_dahdi.c b/drivers/dahdi/xpp/xpp_dahdi.c index 2de7845..0f186ef 100644 --- a/drivers/dahdi/xpp/xpp_dahdi.c +++ b/drivers/dahdi/xpp/xpp_dahdi.c @@ -734,7 +734,6 @@ int xpp_open(struct dahdi_chan *chan) return -EINVAL; } xpd = chan->pvt; - xpd = get_xpd(__FUNCTION__, xpd); /* Returned in xpp_close() */ if (!xpd) { NOTICE("open called on a chan with no pvt (xpd)\n"); BUG(); @@ -776,7 +775,6 @@ int xpp_close(struct dahdi_chan *chan) current->comm, current->pid, atomic_read(&PHONEDEV(xpd).open_counter)); atomic_dec(&PHONEDEV(xpd).open_counter); /* from xpp_open() */ - put_xpd(__FUNCTION__, xpd); /* from xpp_open() */ return 0; } @@ -1024,12 +1022,6 @@ int dahdi_unregister_xpd(xpd_t *xpd) return -EIDRM; } 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. spin_unlock_irqrestore(&xpd->lock, flags); if(xpd->card_present) -- cgit v1.2.3