Messages in this thread Patch in this message | | | Subject | Re: [v2 PATCH 1/1] tg3: fix meaningless hw_stats reading after tg3_halt memset 0 hw_stats | From | Zumeng Chen <> | Date | Sat, 5 May 2018 10:40:55 +0800 |
| |
On 05/03/2018 01:04 PM, Michael Chan wrote: > On Wed, May 2, 2018 at 5:30 PM, Zumeng Chen <zumeng.chen@gmail.com> wrote: >> On 2018年05月03日 01:32, Michael Chan wrote: >>> On Wed, May 2, 2018 at 3:27 AM, Zumeng Chen <zumeng.chen@gmail.com> wrote: >>>> On 2018年05月02日 13:12, Michael Chan wrote: >>>>> On Tue, May 1, 2018 at 5:42 PM, Zumeng Chen <zumeng.chen@gmail.com> >>>>> wrote: >>>>> >>>>>> diff --git a/drivers/net/ethernet/broadcom/tg3.h >>>>>> b/drivers/net/ethernet/broadcom/tg3.h >>>>>> index 3b5e98e..c61d83c 100644 >>>>>> --- a/drivers/net/ethernet/broadcom/tg3.h >>>>>> +++ b/drivers/net/ethernet/broadcom/tg3.h >>>>>> @@ -3102,6 +3102,7 @@ enum TG3_FLAGS { >>>>>> TG3_FLAG_ROBOSWITCH, >>>>>> TG3_FLAG_ONE_DMA_AT_ONCE, >>>>>> TG3_FLAG_RGMII_MODE, >>>>>> + TG3_FLAG_HALT, >>>>> I think you should be able to use the existing INIT_COMPLETE flag >>>> >>>> No, it will bring the uncertain factors into the existed complicate >>>> logic >>>> of INIT_COMPLETE. >>>> And I think it's very simple logic here to fix the meaningless hw_stats >>>> reading and the problem >>>> of commit f5992b72. I even suspect if you have read INIT_COMPLETE related >>>> codes carefully. >>>> >>> We should use an existing flag whenever appropriate >> >> I disagree. This is sort of blahblah... > I don't want to see another flag added that is practically the same as > !INIT_COMPLETE. The driver already has close to one hundred flags. > Adding a new flag that is similar to an existing flag will just make > the code more difficult to understand and maintain. > > If you don't want to fix it the cleaner way, Siva or I will fix it.
Feel free to go, I just take a double look, INIT_COMPLETE can directly be used as follows:
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index 08bbb63..0e04fd7 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -8733,14 +8733,11 @@ static void tg3_free_consistent(struct tg3 *tp) tg3_mem_rx_release(tp); tg3_mem_tx_release(tp);
- /* Protect tg3_get_stats64() from reading freed tp->hw_stats. */ - tg3_full_lock(tp, 0); if (tp->hw_stats) { dma_free_coherent(&tp->pdev->dev, sizeof(struct tg3_hw_stats), tp->hw_stats, tp->stats_mapping); tp->hw_stats = NULL; } - tg3_full_unlock(tp); }
/* @@ -14178,7 +14175,7 @@ static void tg3_get_stats64(struct net_device *dev, struct tg3 *tp = netdev_priv(dev);
spin_lock_bh(&tp->lock); - if (!tp->hw_stats) { + if (!tp->hw_stats || tg3_flag(tp, INIT_COMPLETE)) { *stats = tp->net_stats_prev; spin_unlock_bh(&tp->lock); return; Cheers, Zumeng
| |