@@ -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;}return0;}
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-09-13 12:09:16
On Wed, Sep 13, 2017 at 01:02:14PM +0530, Allen Pais wrote:
Signed-off-by: Allen Pais <redacted>
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
@@ -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;}return0;}
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
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
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.
From: Joe Perches <joe@perches.com> Date: 2017-09-13 14:53:29
On Wed, 2017-09-13 at 13:02 +0530, Allen Pais wrote:
Signed-off-by: Allen Pais <redacted>
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.
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
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.
--
2.7.4
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau