* [patch] cciss: Fix race between disk-adding code and interrupt handler
@ 2008-04-14 14:20 scameron
2008-04-14 17:05 ` Jens Axboe
0 siblings, 1 reply; 12+ messages in thread
From: scameron @ 2008-04-14 14:20 UTC (permalink / raw
To: linux-kernel, axboe, mike.miller, mikem
Fix race condition between cciss_init_one(), cciss_update_drive_info(),
and cciss_check_queues(). cciss_softirq_done would try to start
queues which were not quite ready to be started, as its checks for
readiness were not sufficiently synchronized with the queue initializing
code in cciss_init_one and cciss_update_drive_info. Slow cpu and
large numbers of logical drives seem to make the race more likely
to cause a problem.
Signed-off-by: Stephen M. Cameron <scameron@beardog.cca.cpqcorp.net>
---
linux-2.6.25-rc9/drivers/block/cciss.c | 25 ++++++++++++++++++++++++-
linux-2.6.25-rc9/drivers/block/cciss.h | 3 +++
2 files changed, 27 insertions(+), 1 deletion(-)
diff -puN linux-2.6.25-rc9/drivers/block/cciss.c~cciss_init_one_race linux-2.6.25-rc9/drivers/block/cciss.c
--- linux-2.6.25-rc9/linux-2.6.25-rc9/drivers/block/cciss.c~cciss_init_one_race 2008-04-14 08:21:03.000000000 -0500
+++ linux-2.6.25-rc9-scameron/linux-2.6.25-rc9/drivers/block/cciss.c 2008-04-14 08:22:04.000000000 -0500
@@ -1270,7 +1270,9 @@ static void cciss_check_queues(ctlr_info
/* make sure the disk has been added and the drive is real
* because this can be called from the middle of init_one.
*/
- if (!(h->drv[curr_queue].queue) || !(h->drv[curr_queue].heads))
+ if (!(h->drv[curr_queue].queue) ||
+ !(h->drv[curr_queue].heads) ||
+ !h->drv[curr_queue].queue_ready)
continue;
blk_start_queue(h->gendisk[curr_queue]->queue);
@@ -1394,6 +1396,11 @@ geo_inq:
/* if it's the controller it's already added */
if (drv_index) {
+
+ /* Prevent race with interrupt handler's queue starting code. */
+ h->drv[drv_index].queue_ready = 0;
+ wmb();
+
disk->queue = blk_init_queue(do_cciss_request, &h->lock);
sprintf(disk->disk_name, "cciss/c%dd%d", ctlr, drv_index);
disk->major = h->major;
@@ -1420,6 +1427,11 @@ geo_inq:
hba[ctlr]->drv[drv_index].block_size);
h->drv[drv_index].queue = disk->queue;
+
+ /* Prevent race with interrupt handler's queue starting code. */
+ wmb();
+ h->drv[drv_index].queue_ready = 1;
+
add_disk(disk);
}
@@ -3473,6 +3485,11 @@ static int __devinit cciss_init_one(stru
struct gendisk *disk = hba[i]->gendisk[j];
struct request_queue *q;
+ /* prevent race with interrupt handler's */
+ /* queue starting code. */
+ drv->queue_ready = 0;
+ wmb();
+
/* Check if the disk was allocated already */
if (!disk){
hba[i]->gendisk[j] = alloc_disk(1 << NWD_SHIFT);
@@ -3520,6 +3537,12 @@ static int __devinit cciss_init_one(stru
continue;
blk_queue_hardsect_size(q, drv->block_size);
set_capacity(disk, drv->nr_blocks);
+
+ /* prevent race with interrupt handler's */
+ /* queue starting code. */
+ wmb();
+ drv->queue_ready = 1;
+
add_disk(disk);
j++;
} while (j <= hba[i]->highest_lun);
diff -puN linux-2.6.25-rc9/drivers/block/cciss.h~cciss_init_one_race linux-2.6.25-rc9/drivers/block/cciss.h
--- linux-2.6.25-rc9/linux-2.6.25-rc9/drivers/block/cciss.h~cciss_init_one_race 2008-04-14 08:21:06.000000000 -0500
+++ linux-2.6.25-rc9-scameron/linux-2.6.25-rc9/drivers/block/cciss.h 2008-04-14 08:21:58.000000000 -0500
@@ -39,6 +39,9 @@ typedef struct _drive_info_struct
*to prevent it from being opened or it's queue
*from being started.
*/
+ int queue_ready; /* This is used to prevent the interrupt handler */
+ /* from racing (while starting up queues) with */
+ /* cciss_init_one() (while setting up new queues) */
} drive_info_struct;
#ifdef CONFIG_CISS_SCSI_TAPE
_
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] cciss: Fix race between disk-adding code and interrupt handler
2008-04-14 14:20 [patch] cciss: Fix race between disk-adding code and interrupt handler scameron
@ 2008-04-14 17:05 ` Jens Axboe
2008-04-14 17:27 ` scameron
0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2008-04-14 17:05 UTC (permalink / raw
To: scameron; +Cc: linux-kernel, mike.miller, mikem
On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
>
>
> Fix race condition between cciss_init_one(), cciss_update_drive_info(),
> and cciss_check_queues(). cciss_softirq_done would try to start
> queues which were not quite ready to be started, as its checks for
> readiness were not sufficiently synchronized with the queue initializing
> code in cciss_init_one and cciss_update_drive_info. Slow cpu and
> large numbers of logical drives seem to make the race more likely
> to cause a problem.
Hmm, this seems backwards to me. cciss_softirq_done() isn't going to
start the queues, until an irq has triggered for instance. Why isn't the
init properly ordered instead of band-aiding around this with a
'queue_ready' variable?
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] cciss: Fix race between disk-adding code and interrupt handler
2008-04-14 17:05 ` Jens Axboe
@ 2008-04-14 17:27 ` scameron
2008-04-14 17:37 ` Jens Axboe
0 siblings, 1 reply; 12+ messages in thread
From: scameron @ 2008-04-14 17:27 UTC (permalink / raw
To: Jens Axboe; +Cc: linux-kernel, mike.miller, mikem, scameron
> On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
> >
> >
> > Fix race condition between cciss_init_one(), cciss_update_drive_info(),
> > and cciss_check_queues(). cciss_softirq_done would try to start
> > queues which were not quite ready to be started, as its checks for
> > readiness were not sufficiently synchronized with the queue initializing
> > code in cciss_init_one and cciss_update_drive_info. Slow cpu and
> > large numbers of logical drives seem to make the race more likely
> > to cause a problem.
>
> Hmm, this seems backwards to me. cciss_softirq_done() isn't going to
> start the queues, until an irq has triggered for instance. Why isn't the
> init properly ordered instead of band-aiding around this with a
> 'queue_ready' variable?
>
Each call to add_disk() will trigger some interrupts,
and earlier added disks may cause the queues of later,
not-yet-completely added disks to be started.
I suppose the init routine might be reorganized to initialize all
the queues, then have second loop call add_disk() for all
of them. Is that what you had in mind by "properly ordered?"
Disks may be added at run time though, and I think this tears
down all but the first disk, and re-adds them all, if I remember
right, so there is some complication there to think about.
-- steve
On Mon, Apr 14, 2008 at 07:05:05PM +0200, Jens Axboe wrote:
> --
> Jens Axboe
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] cciss: Fix race between disk-adding code and interrupt handler
2008-04-14 17:27 ` scameron
@ 2008-04-14 17:37 ` Jens Axboe
2008-04-14 17:45 ` scameron
0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2008-04-14 17:37 UTC (permalink / raw
To: scameron; +Cc: linux-kernel, mike.miller, mikem
On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
>
>
> > On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
> > >
> > >
> > > Fix race condition between cciss_init_one(), cciss_update_drive_info(),
> > > and cciss_check_queues(). cciss_softirq_done would try to start
> > > queues which were not quite ready to be started, as its checks for
> > > readiness were not sufficiently synchronized with the queue initializing
> > > code in cciss_init_one and cciss_update_drive_info. Slow cpu and
> > > large numbers of logical drives seem to make the race more likely
> > > to cause a problem.
> >
> > Hmm, this seems backwards to me. cciss_softirq_done() isn't going to
> > start the queues, until an irq has triggered for instance. Why isn't the
> > init properly ordered instead of band-aiding around this with a
> > 'queue_ready' variable?
> >
>
> Each call to add_disk() will trigger some interrupts,
> and earlier added disks may cause the queues of later,
> not-yet-completely added disks to be started.
>
> I suppose the init routine might be reorganized to initialize all
> the queues, then have second loop call add_disk() for all
> of them. Is that what you had in mind by "properly ordered?"
Yep precisely, don't call add_disk() until everything is set up.
> Disks may be added at run time though, and I think this tears
> down all but the first disk, and re-adds them all, if I remember
> right, so there is some complication there to think about.
Well, other drivers manage quite fine without resorting to work-arounds
:-)
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] cciss: Fix race between disk-adding code and interrupt handler
2008-04-14 17:37 ` Jens Axboe
@ 2008-04-14 17:45 ` scameron
2008-04-14 17:52 ` Jens Axboe
0 siblings, 1 reply; 12+ messages in thread
From: scameron @ 2008-04-14 17:45 UTC (permalink / raw
To: Jens Axboe; +Cc: linux-kernel, mike.miller, mikem
On Mon, Apr 14, 2008 at 07:37:20PM +0200, Jens Axboe wrote:
> On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
> >
> >
> > > On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
> > > >
> > > >
> > > > Fix race condition between cciss_init_one(), cciss_update_drive_info(),
> > > > and cciss_check_queues(). cciss_softirq_done would try to start
> > > > queues which were not quite ready to be started, as its checks for
> > > > readiness were not sufficiently synchronized with the queue initializing
> > > > code in cciss_init_one and cciss_update_drive_info. Slow cpu and
> > > > large numbers of logical drives seem to make the race more likely
> > > > to cause a problem.
> > >
> > > Hmm, this seems backwards to me. cciss_softirq_done() isn't going to
> > > start the queues, until an irq has triggered for instance. Why isn't the
> > > init properly ordered instead of band-aiding around this with a
> > > 'queue_ready' variable?
> > >
> >
> > Each call to add_disk() will trigger some interrupts,
> > and earlier added disks may cause the queues of later,
> > not-yet-completely added disks to be started.
> >
> > I suppose the init routine might be reorganized to initialize all
> > the queues, then have second loop call add_disk() for all
> > of them. Is that what you had in mind by "properly ordered?"
>
> Yep precisely, don't call add_disk() until everything is set up.
>
> > Disks may be added at run time though, and I think this tears
> > down all but the first disk, and re-adds them all, if I remember
> > right, so there is some complication there to think about.
>
> Well, other drivers manage quite fine without resorting to work-arounds
> :-)
Ok. Thanks for the constructive criticism. I'll rethink it.
Fortunately, (or unfortunately) the race is apparently pretty hard
to trigger, it's been in there for ages, and we've only just seen it
manifest as a problem recently and only in one particular configuration.
-- steve
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] cciss: Fix race between disk-adding code and interrupt handler
2008-04-14 17:45 ` scameron
@ 2008-04-14 17:52 ` Jens Axboe
2008-04-16 18:59 ` scameron
2008-04-16 19:00 ` [patch] cciss: fix warning oops on rmmod of driver scameron
0 siblings, 2 replies; 12+ messages in thread
From: Jens Axboe @ 2008-04-14 17:52 UTC (permalink / raw
To: scameron; +Cc: linux-kernel, mike.miller, mikem
On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
> On Mon, Apr 14, 2008 at 07:37:20PM +0200, Jens Axboe wrote:
> > On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
> > >
> > >
> > > > On Mon, Apr 14 2008, scameron@beardog.cca.cpqcorp.net wrote:
> > > > >
> > > > >
> > > > > Fix race condition between cciss_init_one(), cciss_update_drive_info(),
> > > > > and cciss_check_queues(). cciss_softirq_done would try to start
> > > > > queues which were not quite ready to be started, as its checks for
> > > > > readiness were not sufficiently synchronized with the queue initializing
> > > > > code in cciss_init_one and cciss_update_drive_info. Slow cpu and
> > > > > large numbers of logical drives seem to make the race more likely
> > > > > to cause a problem.
> > > >
> > > > Hmm, this seems backwards to me. cciss_softirq_done() isn't going to
> > > > start the queues, until an irq has triggered for instance. Why isn't the
> > > > init properly ordered instead of band-aiding around this with a
> > > > 'queue_ready' variable?
> > > >
> > >
> > > Each call to add_disk() will trigger some interrupts,
> > > and earlier added disks may cause the queues of later,
> > > not-yet-completely added disks to be started.
> > >
> > > I suppose the init routine might be reorganized to initialize all
> > > the queues, then have second loop call add_disk() for all
> > > of them. Is that what you had in mind by "properly ordered?"
> >
> > Yep precisely, don't call add_disk() until everything is set up.
> >
> > > Disks may be added at run time though, and I think this tears
> > > down all but the first disk, and re-adds them all, if I remember
> > > right, so there is some complication there to think about.
> >
> > Well, other drivers manage quite fine without resorting to work-arounds
> > :-)
>
> Ok. Thanks for the constructive criticism. I'll rethink it.
>
> Fortunately, (or unfortunately) the race is apparently pretty hard
> to trigger, it's been in there for ages, and we've only just seen it
> manifest as a problem recently and only in one particular configuration.
Hopefully that will not matter. If you rework the init code so that
everything is up and running before you allow any IO going on, then
it'll be easier to 'prove' that you can't hit such races. If you can
have disk added at runtime, make them go through the same init
process/function.
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] cciss: Fix race between disk-adding code and interrupt handler
2008-04-14 17:52 ` Jens Axboe
@ 2008-04-16 18:59 ` scameron
2008-04-16 19:03 ` Miller, Mike (OS Dev)
2008-04-17 11:16 ` Jens Axboe
2008-04-16 19:00 ` [patch] cciss: fix warning oops on rmmod of driver scameron
1 sibling, 2 replies; 12+ messages in thread
From: scameron @ 2008-04-16 18:59 UTC (permalink / raw
To: Jens Axboe; +Cc: linux-kernel, mike.miller, mikem
Fix race condition between cciss_init_one(), cciss_update_drive_info(),
and cciss_check_queues().
Signed-off-by: Stephen M. Cameron <scameron@beardog.cca.cpqcorp.net>
---
linux-2.6.25-rc9/drivers/block/cciss.c | 17 ++++++++++++++++-
drivers/block/cciss.h | 0
2 files changed, 16 insertions(+), 1 deletion(-)
diff -puN linux-2.6.25-rc9/drivers/block/cciss.c~cciss_init_one_race linux-2.6.25-rc9/drivers/block/cciss.c
--- linux-2.6.25-rc9/linux-2.6.25-rc9/drivers/block/cciss.c~cciss_init_one_race 2008-04-14 08:21:03.000000000 -0500
+++ linux-2.6.25-rc9-scameron/linux-2.6.25-rc9/drivers/block/cciss.c 2008-04-16 08:15:38.000000000 -0500
@@ -1349,6 +1349,10 @@ static void cciss_update_drive_info(int
spin_lock_irqsave(CCISS_LOCK(h->ctlr), flags);
h->drv[drv_index].busy_configuring = 1;
spin_unlock_irqrestore(CCISS_LOCK(h->ctlr), flags);
+
+ /* deregister_disk sets h->drv[drv_index].queue = NULL */
+ /* which keeps the interrupt handler from starting */
+ /* the queue. */
ret = deregister_disk(h->gendisk[drv_index],
&h->drv[drv_index], 0);
h->drv[drv_index].busy_configuring = 0;
@@ -1419,6 +1423,10 @@ geo_inq:
blk_queue_hardsect_size(disk->queue,
hba[ctlr]->drv[drv_index].block_size);
+ /* Make sure all queue data is written out before */
+ /* setting h->drv[drv_index].queue, as setting this */
+ /* allows the interrupt handler to start the queue */
+ wmb();
h->drv[drv_index].queue = disk->queue;
add_disk(disk);
}
@@ -3520,10 +3528,17 @@ static int __devinit cciss_init_one(stru
continue;
blk_queue_hardsect_size(q, drv->block_size);
set_capacity(disk, drv->nr_blocks);
- add_disk(disk);
j++;
} while (j <= hba[i]->highest_lun);
+ /* Make sure all queue data is written out before */
+ /* interrupt handler, triggered by add_disk, */
+ /* is allowed to start them. */
+ wmb();
+
+ for (j = 0; j <= hba[i]->highest_lun; j++)
+ add_disk(hba[i]->gendisk[j]);
+
return 1;
clean4:
diff -puN linux-2.6.25-rc9/drivers/block/cciss.h~cciss_init_one_race linux-2.6.25-rc9/drivers/block/cciss.h
_
^ permalink raw reply [flat|nested] 12+ messages in thread
* [patch] cciss: fix warning oops on rmmod of driver
2008-04-14 17:52 ` Jens Axboe
2008-04-16 18:59 ` scameron
@ 2008-04-16 19:00 ` scameron
2008-04-16 19:18 ` Miller, Mike (OS Dev)
2008-04-17 11:15 ` Jens Axboe
1 sibling, 2 replies; 12+ messages in thread
From: scameron @ 2008-04-16 19:00 UTC (permalink / raw
To: Jens Axboe; +Cc: linux-kernel, mike.miller, mikem
* Fix oops on cciss rmmod due to calling pci_free_consistent with
irqs disabled.
Signed-off-by: Stephen M. Cameron <scameron@beardog.cca.cpqcorp.net>
---
linux-2.6.25-rc9/drivers/block/cciss_scsi.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -puN linux-2.6.25-rc9/drivers/block/cciss_scsi.c~pci_free_consistent_oops linux-2.6.25-rc9/drivers/block/cciss_scsi.c
--- root/linux-2.6.25-rc9/drivers/block/cciss_scsi.c~pci_free_consistent_oops 2008-04-16 12:58:53.000000000 -0500
+++ root-root/linux-2.6.25-rc9/drivers/block/cciss_scsi.c 2008-04-16 12:59:22.000000000 -0500
@@ -1349,9 +1349,9 @@ cciss_unregister_scsi(int ctlr)
/* set scsi_host to NULL so our detect routine will
find us on register */
sa->scsi_host = NULL;
+ spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
scsi_cmd_stack_free(ctlr);
kfree(sa);
- spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
}
static int
_
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [patch] cciss: Fix race between disk-adding code and interrupt handler
2008-04-16 18:59 ` scameron
@ 2008-04-16 19:03 ` Miller, Mike (OS Dev)
2008-04-17 11:16 ` Jens Axboe
1 sibling, 0 replies; 12+ messages in thread
From: Miller, Mike (OS Dev) @ 2008-04-16 19:03 UTC (permalink / raw
To: scameron@beardog.cca.cpqcorp.net, Jens Axboe
Cc: linux-kernel@vger.kernel.org, mikem@beardog.cca.cpqcorp.net
> -----Original Message-----
> From: scameron@beardog.cca.cpqcorp.net
> [mailto:scameron@beardog.cca.cpqcorp.net]
> Sent: Wednesday, April 16, 2008 1:59 PM
> To: Jens Axboe
> Cc: linux-kernel@vger.kernel.org; Miller, Mike (OS Dev);
> mikem@beardog.cca.cpqcorp.net
> Subject: Re: [patch] cciss: Fix race between disk-adding code
> and interrupt handler
>
>
> Fix race condition between cciss_init_one(),
> cciss_update_drive_info(), and cciss_check_queues().
>
> Signed-off-by: Stephen M. Cameron <scameron@beardog.cca.cpqcorp.net>
Acked-by: Mike Miller <mike.miller@hp.com>
>
> ---
>
> linux-2.6.25-rc9/drivers/block/cciss.c | 17 ++++++++++++++++-
> drivers/block/cciss.h | 0
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff -puN
> linux-2.6.25-rc9/drivers/block/cciss.c~cciss_init_one_race
> linux-2.6.25-rc9/drivers/block/cciss.c
> ---
> linux-2.6.25-rc9/linux-2.6.25-rc9/drivers/block/cciss.c~cciss_
> init_one_race 2008-04-14 08:21:03.000000000 -0500
> +++
> linux-2.6.25-rc9-scameron/linux-2.6.25-rc9/drivers/block/cciss
> .c 2008-04-16 08:15:38.000000000 -0500
> @@ -1349,6 +1349,10 @@ static void cciss_update_drive_info(int
> spin_lock_irqsave(CCISS_LOCK(h->ctlr), flags);
> h->drv[drv_index].busy_configuring = 1;
> spin_unlock_irqrestore(CCISS_LOCK(h->ctlr), flags);
> +
> + /* deregister_disk sets
> h->drv[drv_index].queue = NULL */
> + /* which keeps the interrupt handler from starting */
> + /* the queue. */
> ret = deregister_disk(h->gendisk[drv_index],
> &h->drv[drv_index], 0);
> h->drv[drv_index].busy_configuring = 0; @@
> -1419,6 +1423,10 @@ geo_inq:
> blk_queue_hardsect_size(disk->queue,
>
> hba[ctlr]->drv[drv_index].block_size);
>
> + /* Make sure all queue data is written out before */
> + /* setting h->drv[drv_index].queue, as setting this */
> + /* allows the interrupt handler to start the queue */
> + wmb();
> h->drv[drv_index].queue = disk->queue;
> add_disk(disk);
> }
> @@ -3520,10 +3528,17 @@ static int __devinit cciss_init_one(stru
> continue;
> blk_queue_hardsect_size(q, drv->block_size);
> set_capacity(disk, drv->nr_blocks);
> - add_disk(disk);
> j++;
> } while (j <= hba[i]->highest_lun);
>
> + /* Make sure all queue data is written out before */
> + /* interrupt handler, triggered by add_disk, */
> + /* is allowed to start them. */
> + wmb();
> +
> + for (j = 0; j <= hba[i]->highest_lun; j++)
> + add_disk(hba[i]->gendisk[j]);
> +
> return 1;
>
> clean4:
> diff -puN
> linux-2.6.25-rc9/drivers/block/cciss.h~cciss_init_one_race
> linux-2.6.25-rc9/drivers/block/cciss.h
> _
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [patch] cciss: fix warning oops on rmmod of driver
2008-04-16 19:00 ` [patch] cciss: fix warning oops on rmmod of driver scameron
@ 2008-04-16 19:18 ` Miller, Mike (OS Dev)
2008-04-17 11:15 ` Jens Axboe
1 sibling, 0 replies; 12+ messages in thread
From: Miller, Mike (OS Dev) @ 2008-04-16 19:18 UTC (permalink / raw
To: scameron@beardog.cca.cpqcorp.net, Jens Axboe
Cc: linux-kernel@vger.kernel.org, mikem@beardog.cca.cpqcorp.net
> -----Original Message-----
> From: scameron@beardog.cca.cpqcorp.net
> [mailto:scameron@beardog.cca.cpqcorp.net]
> Sent: Wednesday, April 16, 2008 2:01 PM
> To: Jens Axboe
> Cc: linux-kernel@vger.kernel.org; Miller, Mike (OS Dev);
> mikem@beardog.cca.cpqcorp.net
> Subject: [patch] cciss: fix warning oops on rmmod of driver
>
>
> * Fix oops on cciss rmmod due to calling pci_free_consistent with
> irqs disabled.
>
> Signed-off-by: Stephen M. Cameron <scameron@beardog.cca.cpqcorp.net>
Acked-by: Mike Miller <mike.miller@hp.com>
> ---
>
> linux-2.6.25-rc9/drivers/block/cciss_scsi.c | 2 +-
> 1 files changed, 1 insertion(+), 1 deletion(-)
>
> diff -puN
> linux-2.6.25-rc9/drivers/block/cciss_scsi.c~pci_free_consisten
> t_oops linux-2.6.25-rc9/drivers/block/cciss_scsi.c
> ---
> root/linux-2.6.25-rc9/drivers/block/cciss_scsi.c~pci_free_cons
> istent_oops 2008-04-16 12:58:53.000000000 -0500
> +++ root-root/linux-2.6.25-rc9/drivers/block/cciss_scsi.c
> 2008-04-16 12:59:22.000000000 -0500
> @@ -1349,9 +1349,9 @@ cciss_unregister_scsi(int ctlr)
> /* set scsi_host to NULL so our detect routine will
> find us on register */
> sa->scsi_host = NULL;
> + spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
> scsi_cmd_stack_free(ctlr);
> kfree(sa);
> - spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
> }
>
> static int
> _
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] cciss: fix warning oops on rmmod of driver
2008-04-16 19:00 ` [patch] cciss: fix warning oops on rmmod of driver scameron
2008-04-16 19:18 ` Miller, Mike (OS Dev)
@ 2008-04-17 11:15 ` Jens Axboe
1 sibling, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2008-04-17 11:15 UTC (permalink / raw
To: scameron; +Cc: linux-kernel, mike.miller, mikem
On Wed, Apr 16 2008, scameron@beardog.cca.cpqcorp.net wrote:
>
> * Fix oops on cciss rmmod due to calling pci_free_consistent with
> irqs disabled.
>
> Signed-off-by: Stephen M. Cameron <scameron@beardog.cca.cpqcorp.net>
Applied
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch] cciss: Fix race between disk-adding code and interrupt handler
2008-04-16 18:59 ` scameron
2008-04-16 19:03 ` Miller, Mike (OS Dev)
@ 2008-04-17 11:16 ` Jens Axboe
1 sibling, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2008-04-17 11:16 UTC (permalink / raw
To: scameron; +Cc: linux-kernel, mike.miller, mikem
On Wed, Apr 16 2008, scameron@beardog.cca.cpqcorp.net wrote:
>
> Fix race condition between cciss_init_one(), cciss_update_drive_info(),
> and cciss_check_queues().
That's certainly belt and suspenders, but it's looking much better than
the previous version. Applied, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-04-17 11:16 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-14 14:20 [patch] cciss: Fix race between disk-adding code and interrupt handler scameron
2008-04-14 17:05 ` Jens Axboe
2008-04-14 17:27 ` scameron
2008-04-14 17:37 ` Jens Axboe
2008-04-14 17:45 ` scameron
2008-04-14 17:52 ` Jens Axboe
2008-04-16 18:59 ` scameron
2008-04-16 19:03 ` Miller, Mike (OS Dev)
2008-04-17 11:16 ` Jens Axboe
2008-04-16 19:00 ` [patch] cciss: fix warning oops on rmmod of driver scameron
2008-04-16 19:18 ` Miller, Mike (OS Dev)
2008-04-17 11:15 ` Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.