netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Herbert Xu <herbert@gondor.apana.org.au>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	Jouni Malinen <j@w1.fi>, Thomas Graf <tgraf@suug.ch>
Subject: Re: [PATCH v2] rhashtable: make walk safe from softirq context
Date: Fri, 8 Feb 2019 06:02:47 +0800	[thread overview]
Message-ID: <20190207220247.vgzgqn627hkwnllp@gondor.apana.org.au> (raw)
In-Reply-To: <20190207214834.awrpkxunnvfdorbe@gondor.apana.org.au>

On Fri, Feb 08, 2019 at 05:48:34AM +0800, Herbert Xu wrote:
>
> > > Could you please show me who is doing this so I can review that
> > > to see whether it's a legitimate use of this API?
> > 
> > I'm sure you'll say it's not legitimate, but it still exists ;-)
> > 
> > mesh_plink_broken() gets called from the TX status path, via
> > ieee80211s_update_metric().
> 
> Let me take a look.

OK I think it is wrong but you already knew that :)

First of all our current walk interface does not guarantee that you
will see all elements because a parallel remove can cause you to miss
elements.  Although Neil Brown was trying to fix that it is still
not in the tree yet.

But more fundamentally, iterating over an unbounded list in a sotirq
context is *broken*!

Either you don't really need a hash table at all because your list
is short enough to start with, or you need to add more hash tables
(or other data structures) to efficiently look things up using these
alternative keys so that you don't end up hogging the softirq.

So this needs to be fixed in mesh.

Once that is gone hopefully we can remove the long obsolete init
API that carries the GFP flag.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

  parent reply	other threads:[~2019-02-07 22:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-06  9:07 [PATCH v2] rhashtable: make walk safe from softirq context Johannes Berg
2019-02-07 13:40 ` Herbert Xu
2019-02-07 13:50   ` Johannes Berg
2019-02-07 21:48     ` Herbert Xu
2019-02-07 21:56       ` Johannes Berg
2019-02-07 22:02       ` Herbert Xu [this message]
2019-02-12 18:43 ` David Miller
2019-02-12 19:03   ` Johannes Berg
2019-02-12 21:54     ` Bob Copeland
2019-02-13  4:35   ` Herbert Xu

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=20190207220247.vgzgqn627hkwnllp@gondor.apana.org.au \
    --to=herbert@gondor.apana.org.au \
    --cc=j@w1.fi \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.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).