Mailing List Archive

Re: cciss: bug fix for crash when running hpacucli
Andrew Morton wrote:

> "Mike Miller (OS Dev)" <m...@beardog.cca.cpqcorp.net> wrote:
>
>> This patch fixes a crash when running hpacucli with multiple logical volumes
>> on a cciss controller. We were not properly initializing the disk->queue
>> and causing a fault.
>
>Please confirm that this is safe&appropriate for backporting into 2.6.16.x?

I think it should be ok, so long as those kernels contain
Jens's softirq changes, and I think they do, iirc.

The problem was an ioctl that the hpacucli program uses to
tell the driver to re-query the controller about what logical drives are
configured (after it say, adds or deletes logical drives) didn't bother to set up
the queue in this ioctl with the softirq function as it does in
cciss_init_one (the latter being called at init time).

The business end of the patch is this hunk,
which mimics code in cciss.c:cciss_init_one():

@@ -1249,6 +1296,8 @@ static void cciss_update_drive_info(int

blk_queue_max_sectors(disk->queue, 512);

+ blk_queue_softirq_done(disk->queue, cciss_softirq_done);
+
disk->queue->queuedata = hba[ctlr];

blk_queue_hardsect_size(disk->queue,

The rest of it is just moving functions around to satisfy the compiler
about function prototypes.

Well, maybe that doesn't answer your question if you were wanting
stronger verification, such as actual testing with those kernels.
If the function cciss_init_one contains a call to blk_queue_softirq_done,
but cciss_update_drive_info does not contain such a call, then
it's a safe bet this patch or something very like it is needed.
cciss_update_drive_info needs to do the same thing per new drive as
cciss_init_one does.

-- steve


__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: cciss: bug fix for crash when running hpacucli [ In reply to ]
> -----Original Message-----
> From: Stephen Cameron [mailto:smcameron@yahoo.com]
> Sent: Monday, April 10, 2006 11:49 PM
> To: linux-kernel@vger.kernel.org; Miller, Mike (OS Dev); akpm@osdl.org
> Subject: Re: cciss: bug fix for crash when running hpacucli
>
> Andrew Morton wrote:
>
> > "Mike Miller (OS Dev)" <m...@beardog.cca.cpqcorp.net> wrote:
> >
> >> This patch fixes a crash when running hpacucli with
> multiple logical
> >> volumes on a cciss controller. We were not properly
> initializing the
> >> disk->queue and causing a fault.
> >
> >Please confirm that this is safe&appropriate for backporting
> into 2.6.16.x?

I created and tested the patch on 2.6.16.2.

mikem


>
> I think it should be ok, so long as those kernels contain
> Jens's softirq changes, and I think they do, iirc.
>
> The problem was an ioctl that the hpacucli program uses to
> tell the driver to re-query the controller about what logical
> drives are configured (after it say, adds or deletes logical
> drives) didn't bother to set up the queue in this ioctl with
> the softirq function as it does in cciss_init_one (the latter
> being called at init time).
>
> The business end of the patch is this hunk, which mimics code
> in cciss.c:cciss_init_one():
>
> @@ -1249,6 +1296,8 @@ static void cciss_update_drive_info(int
>
> blk_queue_max_sectors(disk->queue, 512);
>
> + blk_queue_softirq_done(disk->queue,
> cciss_softirq_done);
> +
> disk->queue->queuedata = hba[ctlr];
>
> blk_queue_hardsect_size(disk->queue,
>
> The rest of it is just moving functions around to satisfy the
> compiler about function prototypes.
>
> Well, maybe that doesn't answer your question if you were
> wanting stronger verification, such as actual testing with
> those kernels.
> If the function cciss_init_one contains a call to
> blk_queue_softirq_done, but cciss_update_drive_info does not
> contain such a call, then it's a safe bet this patch or
> something very like it is needed.
> cciss_update_drive_info needs to do the same thing per new
> drive as cciss_init_one does.
>
> -- steve
>
>
> __________________________________________________
> Do You Yahoo!?
> Tired of spam? Yahoo! Mail has the best spam protection
> around http://mail.yahoo.com
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/