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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

Signed-off-by: Allen Pais <allen.lkml at 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

On Wed, Sep 13, 2017 at 01:02:14PM +0530, Allen Pais
wrote:
> Signed-off-by: Allen Pais <allen.lkml at 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 at lunn.ch>

    Andrew

On Wed, Sep 13, 2017 at 01:02:15PM +0530, Allen Pais
wrote:
> Signed-off-by: Allen Pais <allen.lkml at 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

On Wed, 2017-09-13 at 13:02 +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais <allen.lkml at 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)) {

On Wed, Sep 13, 2017 at 01:02:19PM +0530, Allen Pais
wrote:
> Signed-off-by: Allen Pais <allen.lkml at 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 at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Allen Pais <allen.lkml at gmail.com>
Date: Wed, 13 Sep 2017 13:02:15 +0530
> Signed-off-by: Allen Pais <allen.lkml at 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.

> 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

Allen Pais <allen.lkml at gmail.com> wrote:
> Signed-off-by: Allen Pais <allen.lkml at gmail.com>
Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert at gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

On Wed, Sep 13, 2017 at 01:02:12PM +0530, Allen Pais
wrote:
> Signed-off-by: Allen Pais <allen.lkml at gmail.com>
Applied to drm-misc-next, thanks.
-Daniel
> ---
>  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
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch