* [PATCH 2/2] ixgbe: Use refcount_t for refcount
@ 2019-08-02 10:55 Chuhong Yuan
2019-08-02 13:46 ` Willem de Bruijn
2019-08-05 21:43 ` Bowers, AndrewX
0 siblings, 2 replies; 5+ messages in thread
From: Chuhong Yuan @ 2019-08-02 10:55 UTC (permalink / raw)
Cc: Jeff Kirsher, David S . Miller, intel-wired-lan, netdev,
linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Also convert refcount from 0-based to 1-based.
This patch depends on PATCH 1/2.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++---
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 00710f43cfd2..d313d00065cd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -773,7 +773,7 @@ int ixgbe_setup_fcoe_ddp_resources(struct ixgbe_adapter *adapter)
fcoe->extra_ddp_buffer = buffer;
fcoe->extra_ddp_buffer_dma = dma;
- atomic_set(&fcoe->refcnt, 0);
+ refcount_set(&fcoe->refcnt, 1);
/* allocate pci pool for each cpu */
for_each_possible_cpu(cpu) {
@@ -837,7 +837,7 @@ int ixgbe_fcoe_enable(struct net_device *netdev)
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_fcoe *fcoe = &adapter->fcoe;
- atomic_inc(&fcoe->refcnt);
+ refcount_inc(&fcoe->refcnt);
if (!(adapter->flags & IXGBE_FLAG_FCOE_CAPABLE))
return -EINVAL;
@@ -883,7 +883,7 @@ int ixgbe_fcoe_disable(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
- if (!atomic_dec_and_test(&adapter->fcoe.refcnt))
+ if (!refcount_dec_and_test(&adapter->fcoe.refcnt))
return -EINVAL;
if (!(adapter->flags & IXGBE_FLAG_FCOE_ENABLED))
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h
index 724f5382329f..7ace5fee6ede 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h
@@ -51,7 +51,7 @@ struct ixgbe_fcoe_ddp_pool {
struct ixgbe_fcoe {
struct ixgbe_fcoe_ddp_pool __percpu *ddp_pool;
- atomic_t refcnt;
+ refcount_t refcnt;
spinlock_t lock;
struct ixgbe_fcoe_ddp ddp[IXGBE_FCOE_DDP_MAX_X550];
void *extra_ddp_buffer;
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ixgbe: Use refcount_t for refcount
2019-08-02 10:55 [PATCH 2/2] ixgbe: Use refcount_t for refcount Chuhong Yuan
@ 2019-08-02 13:46 ` Willem de Bruijn
2019-08-05 22:02 ` [Intel-wired-lan] " Alexander Duyck
2019-08-05 21:43 ` Bowers, AndrewX
1 sibling, 1 reply; 5+ messages in thread
From: Willem de Bruijn @ 2019-08-02 13:46 UTC (permalink / raw)
To: Chuhong Yuan
Cc: Jeff Kirsher, David S . Miller, intel-wired-lan,
Network Development, linux-kernel
On Fri, Aug 2, 2019 at 6:55 AM Chuhong Yuan <hslester96@gmail.com> wrote:
>
> refcount_t is better for reference counters since its
> implementation can prevent overflows.
> So convert atomic_t ref counters to refcount_t.
>
> Also convert refcount from 0-based to 1-based.
>
> This patch depends on PATCH 1/2.
>
> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++---
> drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> index 00710f43cfd2..d313d00065cd 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> @@ -773,7 +773,7 @@ int ixgbe_setup_fcoe_ddp_resources(struct ixgbe_adapter *adapter)
>
> fcoe->extra_ddp_buffer = buffer;
> fcoe->extra_ddp_buffer_dma = dma;
> - atomic_set(&fcoe->refcnt, 0);
> + refcount_set(&fcoe->refcnt, 1);
Same point as in the cxgb4 driver patch: how can you just change the
initial value without modifying the condition for release?
This is not a suggestion to resubmit all these changes again with a
change to the release condition.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [Intel-wired-lan] [PATCH 2/2] ixgbe: Use refcount_t for refcount
2019-08-02 10:55 [PATCH 2/2] ixgbe: Use refcount_t for refcount Chuhong Yuan
2019-08-02 13:46 ` Willem de Bruijn
@ 2019-08-05 21:43 ` Bowers, AndrewX
2019-08-05 21:59 ` Willem de Bruijn
1 sibling, 1 reply; 5+ messages in thread
From: Bowers, AndrewX @ 2019-08-05 21:43 UTC (permalink / raw)
To: netdev, linux-kernel, intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Chuhong Yuan
> Sent: Friday, August 2, 2019 3:55 AM
> Cc: netdev@vger.kernel.org; Chuhong Yuan <hslester96@gmail.com>; linux-
> kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org; David S . Miller
> <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: 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.
>
> Also convert refcount from 0-based to 1-based.
>
> This patch depends on PATCH 1/2.
>
> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++---
> drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: Use refcount_t for refcount
2019-08-05 21:43 ` Bowers, AndrewX
@ 2019-08-05 21:59 ` Willem de Bruijn
0 siblings, 0 replies; 5+ messages in thread
From: Willem de Bruijn @ 2019-08-05 21:59 UTC (permalink / raw)
To: Bowers, AndrewX; +Cc: netdev, linux-kernel, intel-wired-lan
On Mon, Aug 5, 2019 at 5:43 PM Bowers, AndrewX <andrewx.bowers@intel.com> wrote:
>
> > -----Original Message-----
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> > Behalf Of Chuhong Yuan
> > Sent: Friday, August 2, 2019 3:55 AM
> > Cc: netdev@vger.kernel.org; Chuhong Yuan <hslester96@gmail.com>; linux-
> > kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org; David S . Miller
> > <davem@davemloft.net>
> > Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: 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.
> >
> > Also convert refcount from 0-based to 1-based.
> >
> > This patch depends on PATCH 1/2.
> >
> > Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> > ---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
To reiterate, this patchset should not be applied as is. It is not
correct to simply change the initial refcount.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: Use refcount_t for refcount
2019-08-02 13:46 ` Willem de Bruijn
@ 2019-08-05 22:02 ` Alexander Duyck
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Duyck @ 2019-08-05 22:02 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Chuhong Yuan, Network Development, intel-wired-lan, linux-kernel,
David S . Miller
On Fri, Aug 2, 2019 at 6:47 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Fri, Aug 2, 2019 at 6:55 AM Chuhong Yuan <hslester96@gmail.com> wrote:
> >
> > refcount_t is better for reference counters since its
> > implementation can prevent overflows.
> > So convert atomic_t ref counters to refcount_t.
> >
> > Also convert refcount from 0-based to 1-based.
> >
> > This patch depends on PATCH 1/2.
> >
> > Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> > ---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> > index 00710f43cfd2..d313d00065cd 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> > @@ -773,7 +773,7 @@ int ixgbe_setup_fcoe_ddp_resources(struct ixgbe_adapter *adapter)
> >
> > fcoe->extra_ddp_buffer = buffer;
> > fcoe->extra_ddp_buffer_dma = dma;
> > - atomic_set(&fcoe->refcnt, 0);
> > + refcount_set(&fcoe->refcnt, 1);
>
> Same point as in the cxgb4 driver patch: how can you just change the
> initial value without modifying the condition for release?
>
> This is not a suggestion to resubmit all these changes again with a
> change to the release condition.
So I am pretty sure this patch is badly broken. It doesn't make any
sense to be resetting with the refcnt in
ixgbe_setup_fcoe_ddp_resources. The value is initialized to zero when
the adapter structure was allocated.
Consider this a NAK from me.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-05 22:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-02 10:55 [PATCH 2/2] ixgbe: Use refcount_t for refcount Chuhong Yuan
2019-08-02 13:46 ` Willem de Bruijn
2019-08-05 22:02 ` [Intel-wired-lan] " Alexander Duyck
2019-08-05 21:43 ` Bowers, AndrewX
2019-08-05 21:59 ` Willem de Bruijn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).