netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Cong Wang <xiyou.wangcong@gmail.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>,
	Thomas Graf <tgraf@suug.ch>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>
Subject: Re: Crash due to mutex genl_lock called from RCU context
Date: Sat, 26 Nov 2016 22:28:21 -0800	[thread overview]
Message-ID: <CAM_iQpUnz2kkjOFuk3fKKEYDh54b9WHk1dvH098mGtrGTPjZFQ@mail.gmail.com> (raw)
In-Reply-To: <1480213570.18162.31.camel@edumazet-glaptop3.roam.corp.google.com>

On Sat, Nov 26, 2016 at 6:26 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Are you telling me inet_release() is called when we close() the first
> file descriptor ?
>
> fd1 = socket()
> fd2 = dup(fd1);
> close(fd2) -> release() ???

Sorry, I didn't express myself clearly, I meant your change,
if exclude the SOCK_RCU_FREE part, basically reverts this commit:

commit 3f660d66dfbc13ea4b61d3865851b348444c24b4
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Thu May 3 03:17:14 2007 -0700

    [NETLINK]: Kill CB only when socket is unused

IOW, ->release() is called when the last sock fd ref is gone, but ->destructor()
is called with the last sock ref is gone. They are very different.


>> I don't see why we need to get genl_lock in ->done() here, because we are
>> already the last sock using it and module ref protects the ops from being
>> removed via module, seems we are pretty safe without any lock.
>
> Well, at least this exposes a real bug in Thomas patch.
>
> Removing the lock might be done for net-next, not stable branches.

I am confused, what Subash reported is a kernel warning which can
surely be fixed by removing genl lock (if it is correct, I need to double
check), so why for net-next?

  reply	other threads:[~2016-11-27  6:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-26  2:15 Crash due to mutex genl_lock called from RCU context subashab
2016-11-26  4:11 ` Eric Dumazet
2016-11-26  4:54   ` Eric Dumazet
2016-11-26  5:59     ` subashab
2016-11-27  2:08     ` Cong Wang
2016-11-27  2:26       ` Eric Dumazet
2016-11-27  6:28         ` Cong Wang [this message]
2016-11-27 16:23           ` Eric Dumazet
2016-11-28  6:53             ` Cong Wang
2016-11-28 11:22               ` Herbert Xu
2016-11-29  4:33                 ` Cong Wang
2016-11-30  0:49                 ` David Miller

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=CAM_iQpUnz2kkjOFuk3fKKEYDh54b9WHk1dvH098mGtrGTPjZFQ@mail.gmail.com \
    --to=xiyou.wangcong@gmail.com \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=subashab@codeaurora.org \
    --cc=tgraf@suug.ch \
    /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).