* [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held
@ 2010-05-30 13:49 Julia Lawall
2010-06-01 8:05 ` Karsten Keil
0 siblings, 1 reply; 4+ 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] 4+ messages in thread* Re: [PATCH 2/2] drivers/isdn/hardware/mISDN: Use GFP_ATOMIC when a lock is held
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
2010-06-01 8:14 ` Julia Lawall
2010-06-01 8:28 ` David Miller
0 siblings, 2 replies; 4+ 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] 4+ 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
@ 2010-06-01 8:14 ` Julia Lawall
2010-06-01 8:28 ` David Miller
1 sibling, 0 replies; 4+ 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] 4+ 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
2010-06-01 8:14 ` Julia Lawall
@ 2010-06-01 8:28 ` David Miller
1 sibling, 0 replies; 4+ 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] 4+ messages in thread
end of thread, other threads:[~2010-06-01 8:28 UTC | newest]
Thread overview: 4+ 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 is held Julia Lawall
2010-06-01 8:05 ` Karsten Keil
2010-06-01 8:14 ` Julia Lawall
2010-06-01 8:28 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).