| Message ID | 1425304483-7987-5-git-send-email-mhocko@suse.cz |
|---|---|
| State | RFC |
| Delegated to: | David Miller |
| Headers | show |
Michal Hocko wrote: > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > index ccf3436024bc..f351920fc293 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > @@ -1220,6 +1220,10 @@ static int set_filter_wr(struct adapter *adapter, int fidx) > struct fw_filter_wr *fwr; > unsigned int ftid; > > + skb = alloc_skb(sizeof(*fwr), GFP_KERNEL); > + if (!skb) > + return -ENOMEM; > + > /* If the new filter requires loopback Destination MAC and/or VLAN > * rewriting then we need to allocate a Layer 2 Table (L2T) entry for > * the filter. > @@ -1227,19 +1231,21 @@ static int set_filter_wr(struct adapter *adapter, int fidx) > if (f->fs.newdmac || f->fs.newvlan) { > /* allocate L2T entry for new filter */ > f->l2t = t4_l2t_alloc_switching(adapter->l2t); > - if (f->l2t == NULL) > + if (f->l2t == NULL) { > + kfree(skb); I think we need to use kfree_skb() than kfree() for memory allocated by alloc_skb(). > return -EAGAIN; > + } > if (t4_l2t_set_switching(adapter, f->l2t, f->fs.vlan, > f->fs.eport, f->fs.dmac)) { > cxgb4_l2t_release(f->l2t); > f->l2t = NULL; > + kfree(skb); Ditto. > return -ENOMEM; > } > } > > ftid = adapter->tids.ftid_base + fidx; > > - skb = alloc_skb(sizeof(*fwr), GFP_KERNEL | __GFP_NOFAIL); > fwr = (struct fw_filter_wr *)__skb_put(skb, sizeof(*fwr)); > memset(fwr, 0, sizeof(*fwr)); > -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/hw/cxgb4/mem.c b/drivers/infiniband/hw/cxgb4/mem.c index cb43c2299ac0..81b028eaf1f5 100644 --- a/drivers/infiniband/hw/cxgb4/mem.c +++ b/drivers/infiniband/hw/cxgb4/mem.c @@ -73,7 +73,7 @@ static int _c4iw_write_mem_dma_aligned(struct c4iw_rdev *rdev, u32 addr, c4iw_init_wr_wait(&wr_wait); wr_len = roundup(sizeof(*req) + sizeof(*sgl), 16); - skb = alloc_skb(wr_len, GFP_KERNEL | __GFP_NOFAIL); + skb = alloc_skb(wr_len, GFP_KERNEL); if (!skb) return -ENOMEM; set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0); diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c index ccf3436024bc..f351920fc293 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c @@ -1220,6 +1220,10 @@ static int set_filter_wr(struct adapter *adapter, int fidx) struct fw_filter_wr *fwr; unsigned int ftid; + skb = alloc_skb(sizeof(*fwr), GFP_KERNEL); + if (!skb) + return -ENOMEM; + /* If the new filter requires loopback Destination MAC and/or VLAN * rewriting then we need to allocate a Layer 2 Table (L2T) entry for * the filter. @@ -1227,19 +1231,21 @@ static int set_filter_wr(struct adapter *adapter, int fidx) if (f->fs.newdmac || f->fs.newvlan) { /* allocate L2T entry for new filter */ f->l2t = t4_l2t_alloc_switching(adapter->l2t); - if (f->l2t == NULL) + if (f->l2t == NULL) { + kfree(skb); return -EAGAIN; + } if (t4_l2t_set_switching(adapter, f->l2t, f->fs.vlan, f->fs.eport, f->fs.dmac)) { cxgb4_l2t_release(f->l2t); f->l2t = NULL; + kfree(skb); return -ENOMEM; } } ftid = adapter->tids.ftid_base + fidx; - skb = alloc_skb(sizeof(*fwr), GFP_KERNEL | __GFP_NOFAIL); fwr = (struct fw_filter_wr *)__skb_put(skb, sizeof(*fwr)); memset(fwr, 0, sizeof(*fwr)); @@ -1337,7 +1343,10 @@ static int del_filter_wr(struct adapter *adapter, int fidx) len = sizeof(*fwr); ftid = adapter->tids.ftid_base + fidx; - skb = alloc_skb(len, GFP_KERNEL | __GFP_NOFAIL); + skb = alloc_skb(len, GFP_KERNEL); + if (!skb) + return -ENOMEM; + fwr = (struct fw_filter_wr *)__skb_put(skb, len); t4_mk_filtdelwr(ftid, fwr, adapter->sge.fw_evtq.abs_id);
set_filter_wr is requesting __GFP_NOFAIL allocation although it can return ENOMEM without any problems obviously (t4_l2t_set_switching does that already). So the non-failing requirement is too strong without any obvious reason. Drop __GFP_NOFAIL and reorganize the code to have the failure paths easier. The same applies to _c4iw_write_mem_dma_aligned which uses __GFP_NOFAIL and then checks the return value and returns -ENOMEM on failure. This doesn't make any sense what so ever. Either the allocation cannot fail or it can. del_filter_wr seems to be safe as well because the filter entry is not marked as pending and the return value is propagated up the stack up to c4iw_destroy_listen. Signed-off-by: Michal Hocko <mhocko@suse.cz> --- drivers/infiniband/hw/cxgb4/mem.c | 2 +- drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-)