2002-10-15 13:34:11

by Bruno Ducrot

[permalink] [raw]
Subject: [PATCH] [2.4.20-pre10] dpt_i2o fix

The first chunk of attached patch alloc wait_data with
kmalloc(..., GFP_ATOMIC) instead of GPF_KERNEL
in the function adpt_i2o_post_wait() because this function
is called in the function adpt_i2o_passthru() (line 1717 or
so) but with a spin_lock held.

The second chunk correct the expected behaviour in teh
function adpt_i2o_parse_lct().

The test 'if(pDev == NULL)' is always true, and the kmalloc
is done for pDev->next_lun.


--- linux-2.4.20-pre10/drivers/scsi/dpt_i2o.c 2002/10/15 12:07:13 1.1
+++ linux-2.4.20-pre10/drivers/scsi/dpt_i2o.c 2002/10/15 13:05:26
@@ -1122,7 +1122,7 @@
ulong flags = 0;
struct adpt_i2o_post_wait_data *p1, *p2;
struct adpt_i2o_post_wait_data *wait_data =
- kmalloc(sizeof(struct adpt_i2o_post_wait_data),GFP_KERNEL);
+ kmalloc(sizeof(struct adpt_i2o_post_wait_data),GFP_ATOMIC);
adpt_wait_queue_t wait;

if(!wait_data){
@@ -1498,7 +1498,7 @@
pDev->next_lun; pDev = pDev->next_lun){
}
pDev->next_lun = kmalloc(sizeof(struct adpt_device),GFP_KERNEL);
- if(pDev == NULL) {
+ if(pDev->next_lun == NULL) {
return -ENOMEM;
}
memset(pDev->next_lun,0,sizeof(struct adpt_device));


Cheers,

--
Ducrot Bruno
http://www.poupinou.org Page profaissionelle
http://toto.tu-me-saoules.com Haume page


2002-10-21 09:30:42

by Alan

[permalink] [raw]
Subject: Re: [PATCH] [2.4.20-pre10] dpt_i2o fix

On Tue, 2002-10-15 at 14:40, Ducrot Bruno wrote:
> The first chunk of attached patch alloc wait_data with
> kmalloc(..., GFP_ATOMIC) instead of GPF_KERNEL
> in the function adpt_i2o_post_wait() because this function
> is called in the function adpt_i2o_passthru() (line 1717 or
> so) but with a spin_lock held.

Given the nature of the usage I think its probably better to fix it
properly than to just hack it up to be GFP_ATOMIC. If the caller was to
pass in the wait_data buffer then the problem could be fixed cleanly

2002-10-21 10:42:20

by Bruno Ducrot

[permalink] [raw]
Subject: Re: [PATCH] [2.4.20-pre10] dpt_i2o fix

On Mon, Oct 21, 2002 at 10:52:12AM +0100, Alan Cox wrote:
> On Tue, 2002-10-15 at 14:40, Ducrot Bruno wrote:
> > The first chunk of attached patch alloc wait_data with
> > kmalloc(..., GFP_ATOMIC) instead of GPF_KERNEL
> > in the function adpt_i2o_post_wait() because this function
> > is called in the function adpt_i2o_passthru() (line 1717 or
> > so) but with a spin_lock held.
>
> Given the nature of the usage I think its probably better to fix it
> properly than to just hack it up to be GFP_ATOMIC. If the caller was to
> pass in the wait_data buffer then the problem could be fixed cleanly
>

This was needed on some of my production servers after an upgrade to 2.4 series
in order to quickly fix some random crashes, and my customers are not happy.
Of course, a better fix should be wrotten.

Cheers,

--
Ducrot Bruno
http://www.poupinou.org Page profaissionelle
http://toto.tu-me-saoules.com Haume page