From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH net-next 5/6] r8169: change type of driver_data
Date: Sun, 25 Mar 2018 23:29:11 +0200
Message-ID: <0ca42355-d226-3136-c147-99ef46dc824a@gmail.com>
References: <6160c32d-3905-683a-7117-d2b11d310a34@gmail.com>
 <70541d5c-7a5e-3988-585d-2804ab41e4db@gmail.com>
 <20180325.164310.604360450302392080.davem@davemloft.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: nic_swsd@realtek.com, netdev@vger.kernel.org
To: David Miller <davem@davemloft.net>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-wr0-f177.google.com ([209.85.128.177]:41997 "EHLO
        mail-wr0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752282AbeCYV3S (ORCPT
        <rfc822;netdev@vger.kernel.org>); Sun, 25 Mar 2018 17:29:18 -0400
Received: by mail-wr0-f177.google.com with SMTP id s18so16943168wrg.9
        for <netdev@vger.kernel.org>; Sun, 25 Mar 2018 14:29:17 -0700 (PDT)
In-Reply-To: <20180325.164310.604360450302392080.davem@davemloft.net>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Am 25.03.2018 um 22:43 schrieb David Miller:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Date: Sat, 24 Mar 2018 23:18:25 +0100
> 
>> Several functions accessing the device driver_data field don't need the
>> net_device. All needed parameters can be accessed via struct
>> rtl8169_private, therefore change type of driver_data accordingly.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> This doesn't work.
> 
> The pci_set_drvdata() call happens after register_netdevice().
> 
> At the exact moment register_netdevice() is called, any part of the
> driver can be invoked before it returns.
> 
> Therefore this change will result in crashes because the drvdata won't
> be initialized early enough.
> 
Thanks for catching this. The patch however just changes the parameter
of pci_set_drvdata, not the position of this call.
This means the potential crash scenario we have already w/o the patch
and it's a long-standing bug.

Having said that I'd submit a fix for this bug first and then a
rebased version of the cleanup patch set (if fine with you).

> I really think you're taking things too far with the cleanups of this
> driver.  It is a reasonably old driver used by a lot of people, and I
> think avoiding regressions is 1000 times more important than
> "streamlining" control plane functions that have not effect whatsoever
> on real driver performance.
> 
> Thank you.
>