staging:iio:adc:ad7280a: Use GFP_ATOMIC in interrupt handler

PATCH In review Karim Eshapa 2 participants 3 messages Jun 28, 2018 – Jun 29, 2018

Subsystems (4)

Staging IIO Analog Devices IIO drivers Staging - industrial io

lore

Share & tools

lore

Karim Eshapa

author

2018-06-28 23:49 UTC #0 lore raw

PATCH 4/4 staging:iio:adc:ad7280a: Use GFP_ATOMIC in interrupt handler

Use GFP_ATOMIC rather GFP_KERNEL in interrupt handler, as GFP_KERNEL may sleep according to slab allocator.

Signed-off-by Karim Eshapa <[email protected]>

---
 drivers/staging/iio/adc/ad7280a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/iio/adc/ad7280a.c +1 -1

index b736275c10f5..25c4fbc303f7 100644
@@ -692,7 +692,7 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
 	unsigned int *channels;
 	int i, ret;
 
-	channels = kcalloc(st->scan_cnt, sizeof(*channels), GFP_KERNEL);
+	channels = kcalloc(st->scan_cnt, sizeof(*channels), GFP_ATOMIC);
 	if (!channels)
 		return IRQ_HANDLED;

Signature · 1 line

2.17.1

Dan Carpenter

2018-06-29 07:45 UTC #1 lore raw

Quoted 3 lines · Karim Eshapa

Use GFP_ATOMIC rather GFP_KERNEL in interrupt handler, as GFP_KERNEL may sleep according to slab allocator.

This is a threaded IRQ so it can sleep.

You should always think about the impact of a bug. If this were a bug it would have showed up in testing right away. Most of my patches are in error handling and so the impact is zero unless you deliberately try to inject errors that's why they can go undetected for years.

Also you should always use the Fixes tag to see how the patch was introduced. I often see bugs where the patch is new and it was introduced by someone doing major subsystem cleanups and they don't have the hardware so I know it hasn't been tested. If the patch is a couple years old and the bug is in the success path which every one tests then I wonder if I am misreading the code somehow.

regards,
dan carpenter

karim eshapa

author

2018-06-29 10:12 UTC #2 lore raw

Quoted 7 lines · Dan Carpenter

This is a threaded IRQ so it can sleep.
You should always think about the impact of a bug. If this were a bug it would have showed up in testing right away. Most of my patches are in error handling and so the impact is zero unless you deliberately try to inject errors that's why they can go undetected for years.

Quoted 6 lines

Also you should always use the Fixes tag to see how the patch was introduced. I often see bugs where the patch is new and it was introduced by someone doing major subsystem cleanups and they don't have the hardware so I know it hasn't been tested. If the patch is a couple years old and the bug is in the success path which every one tests then I wonder if I am misreading the code somehow.

Thanks Dan for your precious advises, I'm always trying to find where it may causes a problem or improve performance to make the code looks good and bug free.

Thanks,
Karim

Quoted 23 lines · Dan Carpenter

Quoted 3 lines · Karim Eshapa
Use GFP_ATOMIC rather GFP_KERNEL in interrupt handler, as GFP_KERNEL may sleep according to slab allocator.
This is a threaded IRQ so it can sleep.
You should always think about the impact of a bug. If this were a bug it would have showed up in testing right away. Most of my patches are in error handling and so the impact is zero unless you deliberately try to inject errors that's why they can go undetected for years.
Also you should always use the Fixes tag to see how the patch was introduced. I often see bugs where the patch is new and it was introduced by someone doing major subsystem cleanups and they don't have the hardware so I know it hasn't been tested. If the patch is a couple years old and the bug is in the success path which every one tests then I wonder if I am misreading the code somehow.
regards,
dan carpenter