Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1731407 > unrolled thread

[PATCH 01/10] arch:powerpc: return -ENOMEM on failed allocation

Started byAllen Pais <allen.lkml@gmail.com>
First post2017-09-13 09:40 +0200
Last post2017-09-14 06:50 +0200
Articles 20 — 6 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 01/10] arch:powerpc: return -ENOMEM on failed allocation Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
    [PATCH 02/10] drivers:crypto: return -ENOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
    [PATCH 08/10] driver:cxgbit: return -NOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
    [PATCH 03/10] driver:gpu: return -ENOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
    [PATCH 04/10] drivers:mpt: return -ENOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
    [PATCH 10/10] fs:btrfs: return -ENOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
      Re: [PATCH 10/10] fs:btrfs: return -ENOMEM on allocation failure. David Sterba <dsterba@suse.cz> - 2017-09-13 17:20 +0200
    [PATCH 09/10] driver:video: return -ENOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
    [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
      Re: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure. Andrew Lunn <andrew@lunn.ch> - 2017-09-13 14:10 +0200
        Re: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure. Allen <allen.lkml@gmail.com> - 2017-09-13 15:10 +0200
        Re: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure. Allen <allen.lkml@gmail.com> - 2017-09-20 09:20 +0200
    [PATCH 07/10] driver:megaraid: return -ENOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
    [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure. Allen Pais <allen.lkml@gmail.com> - 2017-09-13 09:40 +0200
      Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation  failure. Andrew Lunn <andrew@lunn.ch> - 2017-09-13 14:20 +0200
        Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure. Allen <allen.lkml@gmail.com> - 2017-09-13 15:10 +0200
        Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure. Allen <allen.lkml@gmail.com> - 2017-09-20 09:00 +0200
      Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation  failure. David Miller <davem@davemloft.net> - 2017-09-13 18:30 +0200
    Re: [PATCH 01/10] arch:powerpc: return -ENOMEM on failed allocation Joe Perches <joe@perches.com> - 2017-09-13 17:00 +0200
      Re: [PATCH 01/10] arch:powerpc: return -ENOMEM on failed allocation Allen <allen.lkml@gmail.com> - 2017-09-14 06:50 +0200

#1731407 — [PATCH 01/10] arch:powerpc: return -ENOMEM on failed allocation

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 01/10] arch:powerpc: return -ENOMEM on failed allocation
Message-ID<upar7-4lj-1@gated-at.bofh.it>
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 arch/powerpc/platforms/cell/spider-pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spider-pci.c b/arch/powerpc/platforms/cell/spider-pci.c
index d1e61e2..82aa3f7 100644
--- a/arch/powerpc/platforms/cell/spider-pci.c
+++ b/arch/powerpc/platforms/cell/spider-pci.c
@@ -106,7 +106,7 @@ static int __init spiderpci_pci_setup_chip(struct pci_controller *phb,
 	dummy_page_va = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!dummy_page_va) {
 		pr_err("SPIDERPCI-IOWA:Alloc dummy_page_va failed.\n");
-		return -1;
+		return -ENOMEM;
 	}
 
 	dummy_page_da = dma_map_single(phb->parent, dummy_page_va,
@@ -137,7 +137,7 @@ int __init spiderpci_iowa_init(struct iowa_bus *bus, void *data)
 	if (!priv) {
 		pr_err("SPIDERPCI-IOWA:"
 		       "Can't allocate struct spiderpci_iowa_private");
-		return -1;
+		return -ENOMEM;
 	}
 
 	if (of_address_to_resource(np, 0, &r)) {
-- 
2.7.4

[toc] | [next] | [standalone]


#1731408 — [PATCH 02/10] drivers:crypto: return -ENOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 02/10] drivers:crypto: return -ENOMEM on allocation failure.
Message-ID<upar7-4lj-3@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/crypto/omap-aes-gcm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/omap-aes-gcm.c b/drivers/crypto/omap-aes-gcm.c
index 7d4f8a4..2542224 100644
--- a/drivers/crypto/omap-aes-gcm.c
+++ b/drivers/crypto/omap-aes-gcm.c
@@ -186,7 +186,7 @@ static int do_encrypt_iv(struct aead_request *req, u32 *tag, u32 *iv)
 	sk_req = skcipher_request_alloc(ctx->ctr, GFP_KERNEL);
 	if (!sk_req) {
 		pr_err("skcipher: Failed to allocate request\n");
-		return -1;
+		return -ENOMEM;
 	}
 
 	init_completion(&result.completion);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731409 — [PATCH 08/10] driver:cxgbit: return -NOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 08/10] driver:cxgbit: return -NOMEM on allocation failure.
Message-ID<upar8-4lj-9@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/target/iscsi/cxgbit/cxgbit_target.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/cxgbit/cxgbit_target.c b/drivers/target/iscsi/cxgbit/cxgbit_target.c
index 514986b..47127d6 100644
--- a/drivers/target/iscsi/cxgbit/cxgbit_target.c
+++ b/drivers/target/iscsi/cxgbit/cxgbit_target.c
@@ -399,7 +399,7 @@ cxgbit_map_skb(struct iscsi_cmd *cmd, struct sk_buff *skb, u32 data_offset,
 	if (padding) {
 		page = alloc_page(GFP_KERNEL | __GFP_ZERO);
 		if (!page)
-			return -1;
+			return -ENOMEM;
 		skb_fill_page_desc(skb, i, page, 0, padding);
 		skb->data_len += padding;
 		skb->len += padding;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731411 — [PATCH 03/10] driver:gpu: return -ENOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 03/10] driver:gpu: return -ENOMEM on allocation failure.
Message-ID<upar8-4lj-15@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/gpu/drm/gma500/mid_bios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
index d75ecb3..1fa1633 100644
--- a/drivers/gpu/drm/gma500/mid_bios.c
+++ b/drivers/gpu/drm/gma500/mid_bios.c
@@ -237,7 +237,7 @@ static int mid_get_vbt_data_r10(struct drm_psb_private *dev_priv, u32 addr)
 
 	gct = kmalloc(sizeof(*gct) * vbt.panel_count, GFP_KERNEL);
 	if (!gct)
-		return -1;
+		return -ENOMEM;
 
 	gct_virtual = ioremap(addr + sizeof(vbt),
 			sizeof(*gct) * vbt.panel_count);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731413 — [PATCH 04/10] drivers:mpt: return -ENOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 04/10] drivers:mpt: return -ENOMEM on allocation failure.
Message-ID<upar8-4lj-19@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/message/fusion/mptbase.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 84eab28..7920b2b 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -4328,15 +4328,15 @@ initChainBuffers(MPT_ADAPTER *ioc)
 	if (ioc->ReqToChain == NULL) {
 		sz = ioc->req_depth * sizeof(int);
 		mem = kmalloc(sz, GFP_ATOMIC);
-		if (mem == NULL)
-			return -1;
+		if (!mem)
+			return -ENOMEM;
 
 		ioc->ReqToChain = (int *) mem;
 		dinitprintk(ioc, printk(MYIOC_s_DEBUG_FMT "ReqToChain alloc  @ %p, sz=%d bytes\n",
 			 	ioc->name, mem, sz));
 		mem = kmalloc(sz, GFP_ATOMIC);
-		if (mem == NULL)
-			return -1;
+		if (!mem)
+			return -ENOMEM;
 
 		ioc->RequestNB = (int *) mem;
 		dinitprintk(ioc, printk(MYIOC_s_DEBUG_FMT "RequestNB alloc  @ %p, sz=%d bytes\n",
@@ -4402,8 +4402,8 @@ initChainBuffers(MPT_ADAPTER *ioc)
 	sz = num_chain * sizeof(int);
 	if (ioc->ChainToChain == NULL) {
 		mem = kmalloc(sz, GFP_ATOMIC);
-		if (mem == NULL)
-			return -1;
+		if (!mem)
+			return -ENOMEM;
 
 		ioc->ChainToChain = (int *) mem;
 		dinitprintk(ioc, printk(MYIOC_s_DEBUG_FMT "ChainToChain alloc @ %p, sz=%d bytes\n",
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731414 — [PATCH 10/10] fs:btrfs: return -ENOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 10/10] fs:btrfs: return -ENOMEM on allocation failure.
Message-ID<upar8-4lj-21@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 fs/btrfs/check-integrity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 7d5a9b5..efa4c23 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -2913,7 +2913,7 @@ int btrfsic_mount(struct btrfs_fs_info *fs_info,
 	state = kvzalloc(sizeof(*state), GFP_KERNEL);
 	if (!state) {
 		pr_info("btrfs check-integrity: allocation failed!\n");
-		return -1;
+		return -ENOMEM;
 	}
 
 	if (!btrfsic_is_initialized) {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731672 — Re: [PATCH 10/10] fs:btrfs: return -ENOMEM on allocation failure.

FromDavid Sterba <dsterba@suse.cz>
Date2017-09-13 17:20 +0200
SubjectRe: [PATCH 10/10] fs:btrfs: return -ENOMEM on allocation failure.
Message-ID<uphCj-Ah-33@gated-at.bofh.it>
In reply to#1731414
On Wed, Sep 13, 2017 at 01:02:19PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
> ---
>  fs/btrfs/check-integrity.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
> index 7d5a9b5..efa4c23 100644
> --- a/fs/btrfs/check-integrity.c
> +++ b/fs/btrfs/check-integrity.c
> @@ -2913,7 +2913,7 @@ int btrfsic_mount(struct btrfs_fs_info *fs_info,
>  	state = kvzalloc(sizeof(*state), GFP_KERNEL);
>  	if (!state) {
>  		pr_info("btrfs check-integrity: allocation failed!\n");
> -		return -1;
> +		return -ENOMEM;

Makes sense, also please fix the -1 a few lines below that also result
from failed memory allocation, indirectly from btrfsic_dev_state_alloc().

>  	}
>  
>  	if (!btrfsic_is_initialized) {
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]


#1731415 — [PATCH 09/10] driver:video: return -ENOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 09/10] driver:video: return -ENOMEM on allocation failure.
Message-ID<upar9-4lj-29@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/matrox/matroxfb_base.c b/drivers/video/fbdev/matrox/matroxfb_base.c
index f6a0b9a..5cd238d 100644
--- a/drivers/video/fbdev/matrox/matroxfb_base.c
+++ b/drivers/video/fbdev/matrox/matroxfb_base.c
@@ -2058,7 +2058,7 @@ static int matroxfb_probe(struct pci_dev* pdev, const struct pci_device_id* dumm
 
 	minfo = kzalloc(sizeof(*minfo), GFP_KERNEL);
 	if (!minfo)
-		return -1;
+		return -ENOMEM;
 
 	minfo->pcidev = pdev;
 	minfo->dead = 0;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731417 — [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 05/10] drivers:net: return -ENOMEM on allocation failure.
Message-ID<upar9-4lj-31@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/net/bonding/bond_alb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index c02cc81..89df377 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -864,7 +864,7 @@ static int rlb_initialize(struct bonding *bond)
 
 	new_hashtbl = kmalloc(size, GFP_KERNEL);
 	if (!new_hashtbl)
-		return -1;
+		return -ENOMEM;
 
 	spin_lock_bh(&bond->mode_lock);
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731575 — Re: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure.

FromAndrew Lunn <andrew@lunn.ch>
Date2017-09-13 14:10 +0200
SubjectRe: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure.
Message-ID<upeEq-7cs-21@gated-at.bofh.it>
In reply to#1731417
On Wed, Sep 13, 2017 at 01:02:14PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>

Hi Allen

Although correct, if you look higher up the call chain, this appears
to be not so useful.

rlb_initialize() is only called by bond_alb_initialize(), and it
propagates the -1. That is only called by bond_open() with:

	if (bond_alb_initialize(bond, (BOND_MODE(bond) == BOND_MODE_ALB)))
		return -ENOMEM;

So you might want to also modify this code, to return the return
value, rather than use the hard coded ENOMEM.

Since you code is OK as far as it goes:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[toc] | [prev] | [next] | [standalone]


#1731595 — Re: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure.

FromAllen <allen.lkml@gmail.com>
Date2017-09-13 15:10 +0200
SubjectRe: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure.
Message-ID<upfAt-7M6-1@gated-at.bofh.it>
In reply to#1731575
> propagates the -1. That is only called by bond_open() with:
>
>         if (bond_alb_initialize(bond, (BOND_MODE(bond) == BOND_MODE_ALB)))
>                 return -ENOMEM;
>
> So you might want to also modify this code, to return the return
> value, rather than use the hard coded ENOMEM.
>

 I'll modify the above and send it out a separate patch.

Thank you.

[toc] | [prev] | [next] | [standalone]


#1735593 — Re: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure.

FromAllen <allen.lkml@gmail.com>
Date2017-09-20 09:20 +0200
SubjectRe: [PATCH 05/10] drivers:net: return -ENOMEM on allocation failure.
Message-ID<urHsC-8f3-39@gated-at.bofh.it>
In reply to#1731575
>
> rlb_initialize() is only called by bond_alb_initialize(), and it
> propagates the -1. That is only called by bond_open() with:
>
>         if (bond_alb_initialize(bond, (BOND_MODE(bond) == BOND_MODE_ALB)))
>                 return -ENOMEM;
>

Would this work?

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index c02cc81..89df377 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -864,7 +864,7 @@ static int rlb_initialize(struct bonding *bond)

        new_hashtbl = kmalloc(size, GFP_KERNEL);
        if (!new_hashtbl)
-               return -1;
+               return -ENOMEM;

        spin_lock_bh(&bond->mode_lock);

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c99dc59..edef242 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3284,6 +3284,7 @@ static int bond_open(struct net_device *bond_dev)
        struct bonding *bond = netdev_priv(bond_dev);
        struct list_head *iter;
        struct slave *slave;
+       int ret;

        /* reset slave->backup and slave->inactive */
        if (bond_has_slaves(bond)) {
@@ -3303,8 +3304,9 @@ static int bond_open(struct net_device *bond_dev)
                /* bond_alb_initialize must be called before the timer
                 * is started.
                 */
-               if (bond_alb_initialize(bond, (BOND_MODE(bond) ==
BOND_MODE_ALB)))
-                       return -ENOMEM;
+               ret = bond_alb_initialize(bond, (BOND_MODE(bond) ==
BOND_MODE_ALB));
+               if (ret)
+                       return ret;
                if (bond->params.tlb_dynamic_lb)
                        queue_delayed_work(bond->wq, &bond->alb_work, 0);
        }

[toc] | [prev] | [next] | [standalone]


#1731418 — [PATCH 07/10] driver:megaraid: return -ENOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 07/10] driver:megaraid: return -ENOMEM on allocation failure.
Message-ID<upar9-4lj-43@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/scsi/megaraid/megaraid_mbox.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_mbox.c b/drivers/scsi/megaraid/megaraid_mbox.c
index ec3c438..b09a0a6 100644
--- a/drivers/scsi/megaraid/megaraid_mbox.c
+++ b/drivers/scsi/megaraid/megaraid_mbox.c
@@ -724,8 +724,8 @@ megaraid_init_mbox(adapter_t *adapter)
 	 * controllers
 	 */
 	raid_dev = kzalloc(sizeof(mraid_device_t), GFP_KERNEL);
-	if (raid_dev == NULL) return -1;
-
+	if (!raid_dev)
+		return -ENOMEM;
 
 	/*
 	 * Attach the adapter soft state to raid device soft state
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731419 — [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.

FromAllen Pais <allen.lkml@gmail.com>
Date2017-09-13 09:40 +0200
Subject[PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.
Message-ID<upar9-4lj-45@gated-at.bofh.it>
In reply to#1731407
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/net/ethernet/sun/cassini.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 382993c..fc0ea3a 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -3984,7 +3984,7 @@ static inline int cas_alloc_rx_desc(struct cas *cp, int ring)
 	size = RX_DESC_RINGN_SIZE(ring);
 	for (i = 0; i < size; i++) {
 		if ((page[i] = cas_page_alloc(cp, GFP_KERNEL)) == NULL)
-			return -1;
+			return -ENOMEM;
 	}
 	return 0;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1731582 — Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.

FromAndrew Lunn <andrew@lunn.ch>
Date2017-09-13 14:20 +0200
SubjectRe: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.
Message-ID<upeO6-7fC-17@gated-at.bofh.it>
In reply to#1731419
On Wed, Sep 13, 2017 at 01:02:15PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
> ---
>  drivers/net/ethernet/sun/cassini.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
> index 382993c..fc0ea3a 100644
> --- a/drivers/net/ethernet/sun/cassini.c
> +++ b/drivers/net/ethernet/sun/cassini.c
> @@ -3984,7 +3984,7 @@ static inline int cas_alloc_rx_desc(struct cas *cp, int ring)
>  	size = RX_DESC_RINGN_SIZE(ring);
>  	for (i = 0; i < size; i++) {
>  		if ((page[i] = cas_page_alloc(cp, GFP_KERNEL)) == NULL)
> -			return -1;
> +			return -ENOMEM;
>  	}
>  	return 0;
>  }

static int cas_alloc_rxds(struct cas *cp)
{
	int i;

	for (i = 0; i < N_RX_DESC_RINGS; i++) {
		if (cas_alloc_rx_desc(cp, i) < 0) {
	       		cas_free_rxds(cp);
			return -1;
		}
	}
	return 0;
}

Again, your change is correct, but in the end the value is not used.
And if you fix it at the cas_alloc_rxds level, you also need a fix at
the next level up:

	err = -ENOMEM;
	if (cas_tx_tiny_alloc(cp) < 0)
		goto err_unlock;

	/* alloc rx descriptors */
	if (cas_alloc_rxds(cp) < 0)
		goto err_tx_tiny;

again, the return value is discarded.

       Andrew

[toc] | [prev] | [next] | [standalone]


#1731597 — Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.

FromAllen <allen.lkml@gmail.com>
Date2017-09-13 15:10 +0200
SubjectRe: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.
Message-ID<upfAt-7M6-5@gated-at.bofh.it>
In reply to#1731582
>
> static int cas_alloc_rxds(struct cas *cp)
> {
>         int i;
>
>         for (i = 0; i < N_RX_DESC_RINGS; i++) {
>                 if (cas_alloc_rx_desc(cp, i) < 0) {
>                         cas_free_rxds(cp);
>                         return -1;
>                 }
>         }
>         return 0;
> }
>
> Again, your change is correct, but in the end the value is not used.
> And if you fix it at the cas_alloc_rxds level, you also need a fix at
> the next level up:
>
>         err = -ENOMEM;
>         if (cas_tx_tiny_alloc(cp) < 0)
>                 goto err_unlock;
>
>         /* alloc rx descriptors */
>         if (cas_alloc_rxds(cp) < 0)
>                 goto err_tx_tiny;
>
> again, the return value is discarded.

 I agree. I could send out v2 with fixes at both level.

- Allen

[toc] | [prev] | [next] | [standalone]


#1735569 — Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.

FromAllen <allen.lkml@gmail.com>
Date2017-09-20 09:00 +0200
SubjectRe: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.
Message-ID<urH9g-7S7-19@gated-at.bofh.it>
In reply to#1731582
> static int cas_alloc_rxds(struct cas *cp)
> {
>         int i;
>
>         for (i = 0; i < N_RX_DESC_RINGS; i++) {
>                 if (cas_alloc_rx_desc(cp, i) < 0) {
>                         cas_free_rxds(cp);
>                         return -1;
>                 }
>         }
>         return 0;
> }
>
> Again, your change is correct, but in the end the value is not used.
> And if you fix it at the cas_alloc_rxds level, you also need a fix at
> the next level up:
>
>         err = -ENOMEM;
>         if (cas_tx_tiny_alloc(cp) < 0)
>                 goto err_unlock;
>
>         /* alloc rx descriptors */
>         if (cas_alloc_rxds(cp) < 0)
>                 goto err_tx_tiny;
>
> again, the return value is discarded.

Andrew,

How about the below patch as a fix?

diff --git a/drivers/net/ethernet/sun/cassini.c
b/drivers/net/ethernet/sun/cassini.c
index 382993c..a88552c 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -3984,19 +3984,20 @@ static inline int cas_alloc_rx_desc(struct cas
*cp, int ring)
        size = RX_DESC_RINGN_SIZE(ring);
        for (i = 0; i < size; i++) {
                if ((page[i] = cas_page_alloc(cp, GFP_KERNEL)) == NULL)
-                       return -1;
+                       return -ENOMEM;
        }
        return 0;
 }

 static int cas_alloc_rxds(struct cas *cp)
 {
-       int i;
+       int i, ret;

        for (i = 0; i < N_RX_DESC_RINGS; i++) {
-               if (cas_alloc_rx_desc(cp, i) < 0) {
+               ret = cas_alloc_rx_desc(cp, i);
+               if (ret < 0) {
                        cas_free_rxds(cp);
-                       return -1;
+                       return ret;
                }
        }
        return 0;
@@ -4241,7 +4242,7 @@ static int cas_tx_tiny_alloc(struct cas *cp)
 static int cas_open(struct net_device *dev)
 {
        struct cas *cp = netdev_priv(dev);
-       int hw_was_up, err;
+       int hw_was_up;
        unsigned long flags;

        mutex_lock(&cp->pm_mutex);
@@ -4264,7 +4265,6 @@ static int cas_open(struct net_device *dev)
                cas_unlock_all_restore(cp, flags);
        }

-       err = -ENOMEM;
        if (cas_tx_tiny_alloc(cp) < 0)
                goto err_unlock;

@@ -4309,7 +4309,7 @@ static int cas_open(struct net_device *dev)
        cas_tx_tiny_free(cp);
 err_unlock:
        mutex_unlock(&cp->pm_mutex);
-       return err;
+       return -ENOMEM;
 }

 static int cas_close(struct net_device *dev)

[toc] | [prev] | [next] | [standalone]


#1731704 — Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.

FromDavid Miller <davem@davemloft.net>
Date2017-09-13 18:30 +0200
SubjectRe: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.
Message-ID<upiI1-1gS-7@gated-at.bofh.it>
In reply to#1731419
From: Allen Pais <allen.lkml@gmail.com>
Date: Wed, 13 Sep 2017 13:02:15 +0530

> Signed-off-by: Allen Pais <allen.lkml@gmail.com>

This is quite pointless as the caller doesn't do anything with
the value, it just tests whether a negative value is returned
or not.

[toc] | [prev] | [next] | [standalone]


#1731652

FromJoe Perches <joe@perches.com>
Date2017-09-13 17:00 +0200
Message-ID<uphiW-e2-15@gated-at.bofh.it>
In reply to#1731407
On Wed, 2017-09-13 at 13:02 +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>

I think the changelog for this series of conversions
should show that you've validated the change by
inspecting the return call chain at each modified line.

Also, it seems you've cc'd the same mailing lists for
all of the patches modified by this series.

It would be better to individually address each patch
in the series only cc'ing the appropriate maintainers
and mailing lists.

A cover letter would be good too.

> ---
>  arch/powerpc/platforms/cell/spider-pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/cell/spider-pci.c b/arch/powerpc/platforms/cell/spider-pci.c
> index d1e61e2..82aa3f7 100644
> --- a/arch/powerpc/platforms/cell/spider-pci.c
> +++ b/arch/powerpc/platforms/cell/spider-pci.c
> @@ -106,7 +106,7 @@ static int __init spiderpci_pci_setup_chip(struct pci_controller *phb,
>  	dummy_page_va = kmalloc(PAGE_SIZE, GFP_KERNEL);
>  	if (!dummy_page_va) {
>  		pr_err("SPIDERPCI-IOWA:Alloc dummy_page_va failed.\n");
> -		return -1;
> +		return -ENOMEM;
>  	}
>  
>  	dummy_page_da = dma_map_single(phb->parent, dummy_page_va,
> @@ -137,7 +137,7 @@ int __init spiderpci_iowa_init(struct iowa_bus *bus, void *data)
>  	if (!priv) {
>  		pr_err("SPIDERPCI-IOWA:"
>  		       "Can't allocate struct spiderpci_iowa_private");
> -		return -1;
> +		return -ENOMEM;
>  	}
>  
>  	if (of_address_to_resource(np, 0, &r)) {

[toc] | [prev] | [next] | [standalone]


#1732036

FromAllen <allen.lkml@gmail.com>
Date2017-09-14 06:50 +0200
Message-ID<upug9-e7-3@gated-at.bofh.it>
In reply to#1731652
> I think the changelog for this series of conversions
> should show that you've validated the change by
> inspecting the return call chain at each modified line.
>
> Also, it seems you've cc'd the same mailing lists for
> all of the patches modified by this series.
>
> It would be better to individually address each patch
> in the series only cc'ing the appropriate maintainers
> and mailing lists.
>
> A cover letter would be good too.

 Point noted. Thanks.

- Allen

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web