linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sergei Shtylyov <sergei.shtylyov@gmail.com>
To: "ashiduka@fujitsu.com" <ashiduka@fujitsu.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v2] ravb: Fixed the problem that rmmod can not be done
Date: Fri, 31 Jul 2020 19:28:38 +0300	[thread overview]
Message-ID: <32d3b998-322b-7c0a-b14a-41ca66dc601a@gmail.com> (raw)
In-Reply-To: <OSAPR01MB3844C77766155CAB10BE296CDF4E0@OSAPR01MB3844.jpnprd01.prod.outlook.com>

Hello!

On 7/31/20 1:18 PM, ashiduka@fujitsu.com wrote:

> I understand that the commit log needs to be corrected.

   The subject also could be more concise...

> (Shimoda-san's point is also correct)
> 
> If there is anything else that needs to be corrected, please point it out.

   OK, I'll try to post a proper patch review...

>>    That seems a common pattern, inlluding the Renesas sh_eth
>> driver...
> 
> Yes.

    Not at all so common as I thought! Only 4 drivers use mdio-bitbang, 2 of them are
for the Renesas SoCs...

> If I can get an R-Car Gen2 board, I will also fix sh_eth driver.

    Do yuo have R-Car V3H at hand, by chance? It does have a GEther controler used for
booting up... 

>>    No, the driver's remove() method calls ravb_mdio_release() and
>> that one calls
>> free_mdio_bitbang() that calls module_put(); the actual reason lies
>> somewehre deeper than this...
> 
> No.
> Running rmmod calls delete_module() in kernel/module.c before ravb_mdio_release() is called.
> delete_module()
>    -> try_stop_module()
>      -> try_release_module_ref()
> In try_release_module_ref(), check refcnt and if it is counted up, ravb_mdio_release() is not
> called and rmmod is terminated.

   Yes, after some rummaging in the module support code, I have to agree here. I was
just surprised with you finding such a critical bug so late in the drivers' life cycle.
Well, due to usually using NFS the EtherAVB (and Ether too) driver is probably alwaysbuilt in-kernel...

> Thanks & Best Regards,
> Yuusuke Ashizuka <ashiduka@fujitsu.com>

   Trim your messages after your goodbye. That original message stuff typically isn't
tolerated in the Linux mailing lists, nearly the same as top-posting...

[...]

MBR, Sergei

  reply	other threads:[~2020-07-31 16:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-30  3:56 [PATCH] ravb: Fixed the problem that rmmod can not be done Yuusuke Ashizuka
2020-07-30  7:55 ` kernel test robot
2020-07-30 10:01 ` [PATCH v2] " Yuusuke Ashizuka
2020-07-30 11:37   ` Yoshihiro Shimoda
2020-07-30 16:24     ` Sergei Shtylyov
2020-07-31  6:43       ` Yoshihiro Shimoda
2020-07-31 17:45         ` Sergei Shtylyov
2020-07-30 16:04   ` Sergei Shtylyov
2020-07-31 10:18     ` ashiduka
2020-07-31 16:28       ` Sergei Shtylyov [this message]
2020-08-06  2:26         ` ashiduka
2020-07-31 18:32   ` Sergei Shtylyov
2020-08-06  2:28     ` ashiduka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32d3b998-322b-7c0a-b14a-41ca66dc601a@gmail.com \
    --to=sergei.shtylyov@gmail.com \
    --cc=ashiduka@fujitsu.com \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).