[PATCH v2] ax25: use GFP_KERNEL in ax25_dev_device_up()

[PATCH v2] ax25: use GFP_KERNEL in ax25_dev_device_up()

[Peter Lafreniere]

ax25_dev_device_up() is only called during device setup, which is
done in user context. In addition, ax25_dev_device_up()
unconditionally calls ax25_register_dev_sysctl(), which already
allocates with GFP_KERNEL.

Since it is allowed to sleep in this function, here we change
ax25_dev_device_up() to use GFP_KERNEL to reduce unnecessary
out-of-memory errors.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Peter Lafreniere <pjlafren@mtu.edu>
---
v1 -> v2:
 - Renamed patch from "ax25: use GFP_KERNEL over GFP_ATOMIC where possible"
   (Is that okay?)
 - Removed invalid changes to ax25_rt_add()

This isn't related to the patch itself, but thanks are owed to everyone
who had to handle my mistakes in submitting this patch the first time.
First the posting during the merge window, which I know better to do,
then me making the stupidest error of missing a lock directly next to the
relevant code.

I'm sorry for taking up your time, especially with these small and
insignificant patches. That said, I hope to be able to work with everyone
in this community in the future.

Thank you all for your time,
Peter L.

 net/ax25/ax25_dev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ax25/ax25_dev.c b/net/ax25/ax25_dev.c
index 95a76d571c44..af5892ee0066 100644
--- a/net/ax25/ax25_dev.c
+++ b/net/ax25/ax25_dev.c
@@ -52,7 +52,7 @@ void ax25_dev_device_up(struct net_device *dev)
 {
 	ax25_dev *ax25_dev;
 
-	if ((ax25_dev = kzalloc(sizeof(*ax25_dev), GFP_ATOMIC)) == NULL) {
+	if ((ax25_dev = kzalloc(sizeof(*ax25_dev), GFP_KERNEL)) == NULL) {
 		printk(KERN_ERR "AX.25: ax25_dev_device_up - out of memory\n");
 		return;
 	}
@@ -60,7 +60,7 @@ void ax25_dev_device_up(struct net_device *dev)
 	refcount_set(&ax25_dev->refcount, 1);
 	dev->ax25_ptr     = ax25_dev;
 	ax25_dev->dev     = dev;
-	dev_hold_track(dev, &ax25_dev->dev_tracker, GFP_ATOMIC);
+	dev_hold_track(dev, &ax25_dev->dev_tracker, GFP_KERNEL);
 	ax25_dev->forward = NULL;
 	ax25_dev->device_up = true;
 
-- 
2.36.1

Re: [PATCH v2] ax25: use GFP_KERNEL in ax25_dev_device_up()

[Dan Carpenter]

On Mon, Jun 06, 2022 at 01:40:12PM -0400, Peter Lafreniere wrote:
> ax25_dev_device_up() is only called during device setup, which is
> done in user context. In addition, ax25_dev_device_up()
> unconditionally calls ax25_register_dev_sysctl(), which already
> allocates with GFP_KERNEL.
> 
> Since it is allowed to sleep in this function, here we change
> ax25_dev_device_up() to use GFP_KERNEL to reduce unnecessary
> out-of-memory errors.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Peter Lafreniere <pjlafren@mtu.edu>
> ---

I don't know if you really want to add Reported-by tags.  That stuff is
just in the kbuild-bot template and it's an automated thing.  Some people
add it and some don't.

It doesn't really matter either way.

> v1 -> v2:
>  - Renamed patch from "ax25: use GFP_KERNEL over GFP_ATOMIC where possible"
>    (Is that okay?)

No problem.

>  - Removed invalid changes to ax25_rt_add()
> 
> This isn't related to the patch itself, but thanks are owed to everyone
> who had to handle my mistakes in submitting this patch the first time.
> First the posting during the merge window, which I know better to do,
> then me making the stupidest error of missing a lock directly next to the
> relevant code.

It's all part of learning.  No stress.  There was no danger because we
caught it in review and automatically with static analysis.

regards,
dan carpenter