Commit 9ae2ee65 authored by Federico Vaga's avatar Federico Vaga

kernel: fix deadlock on fmc->irq_request(), improve comments

With some kernel configuration the previous locking policy can
generate a deadlock on the spec->irq_lock spinlock.

For example, this will happen sistematically if the option
CONFIG_DEBUG_SHIRQ_FIXME is enabled. In this case, the handler
is called immediatly an it is not a separeted thread.

This patch move out from the locking section what doesn't need
protection and is 'cause' of the deadlock.

The deadlock was introduced with patch d4bbcc73 by using a large
locking section. In realty the PCI request_irq is protected
anyway thanks to the proctection of the variable 'first_time' and
the 'spec_vic_irq_request' function.

[more explaination about patch d4bbcc73]
The purpose of that patch was to protect the initialization of
the first VIC handler and the release of the last one. It was
happening that while we are releasing the last VIC handler,
some one was registering a new one but the spec->vic is still there.
The result was that sometimes the PCI handler was removed
(last VIC handler) but not re-requested for the incoming request.
Signen-off-by: Federico Vaga's avatarFederico Vaga <federico.vaga@cern.ch>
parent 094793fc
......@@ -127,6 +127,10 @@ static irqreturn_t spec_vic_irq_handler(int id, void *data)
struct spec_dev *spec = (struct spec_dev *)fmc->carrier_data;
irqreturn_t rv;
/*
* Lock to avoid to remove the VIC or the handler itself while
* it's running
*/
spin_lock(&spec->irq_lock);
rv = spec_vic_irq_dispatch(spec);
spin_unlock(&spec->irq_lock);
......@@ -157,29 +161,29 @@ static int spec_irq_request(struct fmc_device *fmc, irq_handler_t handler,
/* VIC mode interrupt */
if (!(flags & IRQF_SHARED)) {
/*
* Lock to serialize request/free and have a consistent status
* during these operations
*/
spin_lock(&spec->irq_lock);
first_time = !spec->vic;
/* configure the VIC */
rv = spec_vic_irq_request(spec, fmc, fmc->irq, handler);
if (rv) {
spin_unlock(&spec->irq_lock);
if (rv)
return rv;
}
/* on first IRQ, configure VIC "master" handler and GPIO too */
if (first_time) {
rv = spec_shared_irq_request(fmc, spec_vic_irq_handler,
"spec-vic", IRQF_SHARED);
if (rv) {
spin_unlock(&spec->irq_lock);
if (rv)
return rv;
}
fmc->op->gpio_config(fmc, spec_vic_gpio_cfg,
ARRAY_SIZE(spec_vic_gpio_cfg));
}
spin_unlock(&spec->irq_lock);
} else {
rv = spec_shared_irq_request(fmc, handler, name, flags);
pr_debug("Requesting irq '%s' in shared mode (rv %d)\n", name,
......@@ -226,6 +230,11 @@ static int spec_irq_free(struct fmc_device *fmc)
{
struct spec_dev *spec = fmc->carrier_data;
/*
* Lock to serialize request/free and have a consistent status
* during these operations. And, to avoid to run VIC handler while
* it is going to desappear
*/
spin_lock(&spec->irq_lock);
if (spec->vic)
spec_vic_irq_free(spec, fmc->irq);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment