All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock
@ 2010-05-30 13:49 ` Julia Lawall
  0 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2010-05-30 13:49 UTC (permalink / raw)
  To: Karsten Keil, netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

The function inittiger is only called from nj_init_card, where a lock is held.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@gfp exists@
identifier fn;
position p;
@@

fn(...) {
 ... when != spin_unlock_irqrestore
     when any
 GFP_KERNEL@p
 ... when any
}

@locked@
identifier gfp.fn;
@@

spin_lock_irqsave(...)
...  when != spin_unlock_irqrestore
fn(...)

@depends on locked@
position gfp.p;
@@

- GFP_KERNEL@p
+ GFP_ATOMIC
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/isdn/hardware/mISDN/netjet.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/isdn/hardware/mISDN/netjet.c b/drivers/isdn/hardware/mISDN/netjet.c
--- a/drivers/isdn/hardware/mISDN/netjet.c
+++ b/drivers/isdn/hardware/mISDN/netjet.c
@@ -320,12 +320,12 @@ inittiger(struct tiger_hw *card)
 		return -ENOMEM;
 	}
 	for (i = 0; i < 2; i++) {
-		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_KERNEL);
+		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_ATOMIC);
 		if (!card->bc[i].hsbuf) {
 			pr_info("%s: no B%d send buffer\n", card->name, i + 1);
 			return -ENOMEM;
 		}
-		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_KERNEL);
+		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_ATOMIC);
 		if (!card->bc[i].hrbuf) {
 			pr_info("%s: no B%d recv buffer\n", card->name, i + 1);
 			return -ENOMEM;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held
@ 2010-05-30 13:49 ` Julia Lawall
  0 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2010-05-30 13:49 UTC (permalink / raw)
  To: Karsten Keil, netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

The function inittiger is only called from nj_init_card, where a lock is held.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@gfp exists@
identifier fn;
position p;
@@

fn(...) {
 ... when != spin_unlock_irqrestore
     when any
 GFP_KERNEL@p
 ... when any
}

@locked@
identifier gfp.fn;
@@

spin_lock_irqsave(...)
...  when != spin_unlock_irqrestore
fn(...)

@depends on locked@
position gfp.p;
@@

- GFP_KERNEL@p
+ GFP_ATOMIC
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/isdn/hardware/mISDN/netjet.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/drivers/isdn/hardware/mISDN/netjet.c b/drivers/isdn/hardware/mISDN/netjet.c
--- a/drivers/isdn/hardware/mISDN/netjet.c
+++ b/drivers/isdn/hardware/mISDN/netjet.c
@@ -320,12 +320,12 @@ inittiger(struct tiger_hw *card)
 		return -ENOMEM;
 	}
 	for (i = 0; i < 2; i++) {
-		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_KERNEL);
+		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_ATOMIC);
 		if (!card->bc[i].hsbuf) {
 			pr_info("%s: no B%d send buffer\n", card->name, i + 1);
 			return -ENOMEM;
 		}
-		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_KERNEL);
+		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_ATOMIC);
 		if (!card->bc[i].hrbuf) {
 			pr_info("%s: no B%d recv buffer\n", card->name, i + 1);
 			return -ENOMEM;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a
  2010-05-30 13:49 ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held Julia Lawall
@ 2010-06-01  8:05   ` Karsten Keil
  -1 siblings, 0 replies; 8+ messages in thread
From: Karsten Keil @ 2010-06-01  8:05 UTC (permalink / raw)
  To: Julia Lawall; +Cc: netdev, linux-kernel, kernel-janitors

Hello Julia,

thanks for that report, the issue is valid, but I think the fix in that case
should be to move the allocation to an other place, to avoid wasting of
GFP_ATOMIC allocation. I will come up with an other fix.

Karsten


On Sun, May 30, 2010 at 03:49:40PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> The function inittiger is only called from nj_init_card, where a lock is held.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @gfp exists@
> identifier fn;
> position p;
> @@
> 
> fn(...) {
>  ... when != spin_unlock_irqrestore
>      when any
>  GFP_KERNEL@p
>  ... when any
> }
> 
> @locked@
> identifier gfp.fn;
> @@
> 
> spin_lock_irqsave(...)
> ...  when != spin_unlock_irqrestore
> fn(...)
> 
> @depends on locked@
> position gfp.p;
> @@
> 
> - GFP_KERNEL@p
> + GFP_ATOMIC
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/isdn/hardware/mISDN/netjet.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff -u -p a/drivers/isdn/hardware/mISDN/netjet.c b/drivers/isdn/hardware/mISDN/netjet.c
> --- a/drivers/isdn/hardware/mISDN/netjet.c
> +++ b/drivers/isdn/hardware/mISDN/netjet.c
> @@ -320,12 +320,12 @@ inittiger(struct tiger_hw *card)
>  		return -ENOMEM;
>  	}
>  	for (i = 0; i < 2; i++) {
> -		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_KERNEL);
> +		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_ATOMIC);
>  		if (!card->bc[i].hsbuf) {
>  			pr_info("%s: no B%d send buffer\n", card->name, i + 1);
>  			return -ENOMEM;
>  		}
> -		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_KERNEL);
> +		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_ATOMIC);
>  		if (!card->bc[i].hrbuf) {
>  			pr_info("%s: no B%d recv buffer\n", card->name, i + 1);
>  			return -ENOMEM;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held
@ 2010-06-01  8:05   ` Karsten Keil
  0 siblings, 0 replies; 8+ messages in thread
From: Karsten Keil @ 2010-06-01  8:05 UTC (permalink / raw)
  To: Julia Lawall; +Cc: netdev, linux-kernel, kernel-janitors

Hello Julia,

thanks for that report, the issue is valid, but I think the fix in that case
should be to move the allocation to an other place, to avoid wasting of
GFP_ATOMIC allocation. I will come up with an other fix.

Karsten


On Sun, May 30, 2010 at 03:49:40PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> The function inittiger is only called from nj_init_card, where a lock is held.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @gfp exists@
> identifier fn;
> position p;
> @@
> 
> fn(...) {
>  ... when != spin_unlock_irqrestore
>      when any
>  GFP_KERNEL@p
>  ... when any
> }
> 
> @locked@
> identifier gfp.fn;
> @@
> 
> spin_lock_irqsave(...)
> ...  when != spin_unlock_irqrestore
> fn(...)
> 
> @depends on locked@
> position gfp.p;
> @@
> 
> - GFP_KERNEL@p
> + GFP_ATOMIC
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/isdn/hardware/mISDN/netjet.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff -u -p a/drivers/isdn/hardware/mISDN/netjet.c b/drivers/isdn/hardware/mISDN/netjet.c
> --- a/drivers/isdn/hardware/mISDN/netjet.c
> +++ b/drivers/isdn/hardware/mISDN/netjet.c
> @@ -320,12 +320,12 @@ inittiger(struct tiger_hw *card)
>  		return -ENOMEM;
>  	}
>  	for (i = 0; i < 2; i++) {
> -		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_KERNEL);
> +		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_ATOMIC);
>  		if (!card->bc[i].hsbuf) {
>  			pr_info("%s: no B%d send buffer\n", card->name, i + 1);
>  			return -ENOMEM;
>  		}
> -		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_KERNEL);
> +		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_ATOMIC);
>  		if (!card->bc[i].hrbuf) {
>  			pr_info("%s: no B%d recv buffer\n", card->name, i + 1);
>  			return -ENOMEM;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a
  2010-06-01  8:05   ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held Karsten Keil
@ 2010-06-01  8:14     ` Julia Lawall
  -1 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2010-06-01  8:14 UTC (permalink / raw)
  To: Karsten Keil; +Cc: netdev, linux-kernel, kernel-janitors

On Tue, 1 Jun 2010, Karsten Keil wrote:

> Hello Julia,
> 
> thanks for that report, the issue is valid, but I think the fix in that case
> should be to move the allocation to an other place, to avoid wasting of
> GFP_ATOMIC allocation. I will come up with an other fix.

OK, thanks.  It's not so easy to see what to do in the interprocedural 
case.

julia

> Karsten
> 
> 
> On Sun, May 30, 2010 at 03:49:40PM +0200, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > The function inittiger is only called from nj_init_card, where a lock is held.
> > 
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @gfp exists@
> > identifier fn;
> > position p;
> > @@
> > 
> > fn(...) {
> >  ... when != spin_unlock_irqrestore
> >      when any
> >  GFP_KERNEL@p
> >  ... when any
> > }
> > 
> > @locked@
> > identifier gfp.fn;
> > @@
> > 
> > spin_lock_irqsave(...)
> > ...  when != spin_unlock_irqrestore
> > fn(...)
> > 
> > @depends on locked@
> > position gfp.p;
> > @@
> > 
> > - GFP_KERNEL@p
> > + GFP_ATOMIC
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  drivers/isdn/hardware/mISDN/netjet.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff -u -p a/drivers/isdn/hardware/mISDN/netjet.c b/drivers/isdn/hardware/mISDN/netjet.c
> > --- a/drivers/isdn/hardware/mISDN/netjet.c
> > +++ b/drivers/isdn/hardware/mISDN/netjet.c
> > @@ -320,12 +320,12 @@ inittiger(struct tiger_hw *card)
> >  		return -ENOMEM;
> >  	}
> >  	for (i = 0; i < 2; i++) {
> > -		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_KERNEL);
> > +		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_ATOMIC);
> >  		if (!card->bc[i].hsbuf) {
> >  			pr_info("%s: no B%d send buffer\n", card->name, i + 1);
> >  			return -ENOMEM;
> >  		}
> > -		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_KERNEL);
> > +		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_ATOMIC);
> >  		if (!card->bc[i].hrbuf) {
> >  			pr_info("%s: no B%d recv buffer\n", card->name, i + 1);
> >  			return -ENOMEM;
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held
@ 2010-06-01  8:14     ` Julia Lawall
  0 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2010-06-01  8:14 UTC (permalink / raw)
  To: Karsten Keil; +Cc: netdev, linux-kernel, kernel-janitors

On Tue, 1 Jun 2010, Karsten Keil wrote:

> Hello Julia,
> 
> thanks for that report, the issue is valid, but I think the fix in that case
> should be to move the allocation to an other place, to avoid wasting of
> GFP_ATOMIC allocation. I will come up with an other fix.

OK, thanks.  It's not so easy to see what to do in the interprocedural 
case.

julia

> Karsten
> 
> 
> On Sun, May 30, 2010 at 03:49:40PM +0200, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > The function inittiger is only called from nj_init_card, where a lock is held.
> > 
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @gfp exists@
> > identifier fn;
> > position p;
> > @@
> > 
> > fn(...) {
> >  ... when != spin_unlock_irqrestore
> >      when any
> >  GFP_KERNEL@p
> >  ... when any
> > }
> > 
> > @locked@
> > identifier gfp.fn;
> > @@
> > 
> > spin_lock_irqsave(...)
> > ...  when != spin_unlock_irqrestore
> > fn(...)
> > 
> > @depends on locked@
> > position gfp.p;
> > @@
> > 
> > - GFP_KERNEL@p
> > + GFP_ATOMIC
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  drivers/isdn/hardware/mISDN/netjet.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff -u -p a/drivers/isdn/hardware/mISDN/netjet.c b/drivers/isdn/hardware/mISDN/netjet.c
> > --- a/drivers/isdn/hardware/mISDN/netjet.c
> > +++ b/drivers/isdn/hardware/mISDN/netjet.c
> > @@ -320,12 +320,12 @@ inittiger(struct tiger_hw *card)
> >  		return -ENOMEM;
> >  	}
> >  	for (i = 0; i < 2; i++) {
> > -		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_KERNEL);
> > +		card->bc[i].hsbuf = kmalloc(NJ_DMA_TXSIZE, GFP_ATOMIC);
> >  		if (!card->bc[i].hsbuf) {
> >  			pr_info("%s: no B%d send buffer\n", card->name, i + 1);
> >  			return -ENOMEM;
> >  		}
> > -		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_KERNEL);
> > +		card->bc[i].hrbuf = kmalloc(NJ_DMA_RXSIZE, GFP_ATOMIC);
> >  		if (!card->bc[i].hrbuf) {
> >  			pr_info("%s: no B%d recv buffer\n", card->name, i + 1);
> >  			return -ENOMEM;
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a
  2010-06-01  8:05   ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held Karsten Keil
@ 2010-06-01  8:28     ` David Miller
  -1 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2010-06-01  8:28 UTC (permalink / raw)
  To: isdn; +Cc: julia, netdev, linux-kernel, kernel-janitors

From: Karsten Keil <isdn@linux-pingi.de>
Date: Tue, 1 Jun 2010 10:05:35 +0200

> thanks for that report, the issue is valid, but I think the fix in that case
> should be to move the allocation to an other place, to avoid wasting of
> GFP_ATOMIC allocation. I will come up with an other fix.

You'll have to write your patch relative to Julia'a s ince it's already
in my net-2.6 tree, since it's a bug fix.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held
@ 2010-06-01  8:28     ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2010-06-01  8:28 UTC (permalink / raw)
  To: isdn; +Cc: julia, netdev, linux-kernel, kernel-janitors

From: Karsten Keil <isdn@linux-pingi.de>
Date: Tue, 1 Jun 2010 10:05:35 +0200

> thanks for that report, the issue is valid, but I think the fix in that case
> should be to move the allocation to an other place, to avoid wasting of
> GFP_ATOMIC allocation. I will come up with an other fix.

You'll have to write your patch relative to Julia'a s ince it's already
in my net-2.6 tree, since it's a bug fix.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-06-01  8:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-30 13:49 [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock Julia Lawall
2010-05-30 13:49 ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held Julia Lawall
2010-06-01  8:05 ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a Karsten Keil
2010-06-01  8:05   ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held Karsten Keil
2010-06-01  8:14   ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a Julia Lawall
2010-06-01  8:14     ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held Julia Lawall
2010-06-01  8:28   ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a David Miller
2010-06-01  8:28     ` [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held David Miller

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.