Re: [PATCH 1/2] bnxt_en: Use refcount_t for refcount
On Wed, Jul 31, 2019 at 9:06 AM Willem de Bruijn wrote: > > On Wed, Jul 31, 2019 at 8:22 AM Chuhong Yuan wrote: > > > > refcount_t is better for reference counters since its > > implementation can prevent overflows. > > So convert atomic_t ref counters to refcount_t. > > > > Signed-off-by: Chuhong Yuan > > --- > > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 8 > > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 2 +- > > 2 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > > b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > > index fc77caf0a076..eb7ed34639e2 100644 > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > > @@ -49,7 +49,7 @@ static int bnxt_register_dev(struct bnxt_en_dev *edev, > > int ulp_id, > > return -ENOMEM; > > } > > > > - atomic_set(>ref_count, 0); > > + refcount_set(>ref_count, 0); > > One feature of refcount_t is that it warns on refcount_inc from 0 to > detect possible use-after_free. It appears that that can trigger here? > I think that's right. We need to change the driver to start counting from 1 instead of 0 if we convert to refcount.
Re: [PATCH 1/2] bnxt_en: Use refcount_t for refcount
On Wed, Jul 31, 2019 at 8:22 AM Chuhong Yuan wrote: > > refcount_t is better for reference counters since its > implementation can prevent overflows. > So convert atomic_t ref counters to refcount_t. > > Signed-off-by: Chuhong Yuan > --- > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 8 > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 2 +- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > index fc77caf0a076..eb7ed34639e2 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > @@ -49,7 +49,7 @@ static int bnxt_register_dev(struct bnxt_en_dev *edev, int > ulp_id, > return -ENOMEM; > } > > - atomic_set(>ref_count, 0); > + refcount_set(>ref_count, 0); One feature of refcount_t is that it warns on refcount_inc from 0 to detect possible use-after_free. It appears that that can trigger here? > ulp->handle = handle; > rcu_assign_pointer(ulp->ulp_ops, ulp_ops); > > @@ -246,12 +246,12 @@ static int bnxt_send_msg(struct bnxt_en_dev *edev, int > ulp_id, > > static void bnxt_ulp_get(struct bnxt_ulp *ulp) > { > - atomic_inc(>ref_count); > + refcount_inc(>ref_count); > }
[PATCH 1/2] bnxt_en: Use refcount_t for refcount
refcount_t is better for reference counters since its implementation can prevent overflows. So convert atomic_t ref counters to refcount_t. Signed-off-by: Chuhong Yuan --- drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 8 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c index fc77caf0a076..eb7ed34639e2 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c @@ -49,7 +49,7 @@ static int bnxt_register_dev(struct bnxt_en_dev *edev, int ulp_id, return -ENOMEM; } - atomic_set(>ref_count, 0); + refcount_set(>ref_count, 0); ulp->handle = handle; rcu_assign_pointer(ulp->ulp_ops, ulp_ops); @@ -87,7 +87,7 @@ static int bnxt_unregister_dev(struct bnxt_en_dev *edev, int ulp_id) synchronize_rcu(); ulp->max_async_event_id = 0; ulp->async_events_bmap = NULL; - while (atomic_read(>ref_count) != 0 && i < 10) { + while (refcount_read(>ref_count) != 0 && i < 10) { msleep(100); i++; } @@ -246,12 +246,12 @@ static int bnxt_send_msg(struct bnxt_en_dev *edev, int ulp_id, static void bnxt_ulp_get(struct bnxt_ulp *ulp) { - atomic_inc(>ref_count); + refcount_inc(>ref_count); } static void bnxt_ulp_put(struct bnxt_ulp *ulp) { - atomic_dec(>ref_count); + refcount_dec(>ref_count); } void bnxt_ulp_stop(struct bnxt *bp) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h index cd78453d0bf0..fc4aa582d190 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h @@ -52,7 +52,7 @@ struct bnxt_ulp { u16 max_async_event_id; u16 msix_requested; u16 msix_base; - atomic_tref_count; + refcount_t ref_count; }; struct bnxt_en_dev { -- 2.20.1