netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bob Copeland <me@bobcopeland.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: David Miller <davem@davemloft.net>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org, j@w1.fi,
	tgraf@suug.ch, herbert@gondor.apana.org.au
Subject: Re: [PATCH v2] rhashtable: make walk safe from softirq context
Date: Tue, 12 Feb 2019 16:54:37 -0500	[thread overview]
Message-ID: <20190212215437.GA18182@localhost> (raw)
In-Reply-To: <ea6479e089dffc46bb9b09e28d364db4c071998f.camel@sipsolutions.net>

On Tue, Feb 12, 2019 at 08:03:17PM +0100, Johannes Berg wrote:
> commit 60854fd94573f0d3b80b55b40cf0140a0430f3ab
> Author: Bob Copeland <me@bobcopeland.com>
> Date:   Wed Mar 2 10:09:20 2016 -0500
> 
>     mac80211: mesh: convert path table to rhashtable
> 
> which is kinda old. Not sure why this didn't surface before, because the
> spinlock was introduced *before*, otherwise certainly the mutex would've
> caused us to not be able to do this code to start with (commit
> c6ff5268293 - rhashtable: Fix walker list corruption).
> 
> That commit also just converted an existing hashtable walk to
> rhashtable, so not sure that counts as having introduced the problem :-)

Yeah, I didn't change any of the contexts in which the iterations were
called, except making it possible to replace the previous mesh-specific
RCU hashtable with rhashtable by introducing this.  That said, it would
certainly be nice to not have to walk the table to find paths that
use a specific station as nexthop.

> But I guess we should also ask Bob first:
> 1) do you think it'd be easy to maintain a separate list or avoid the
>    iteration in some otherway, and make that a small enough patch to be
>    applicable for stable?

I'm not sure, it's been a while.  I guess most of the difficulties would
be around station removal?

> 2) or do you think maybe the mesh_plink_broken() call could just be
>    lifted into a workqueue instead?

As far as I know, no reason this can't wait until later; we might send
a few frames to the wrong destination but that can happen anyway.  But
there are a couple more walks in there like mesh_path_flush_by_nexthop().

-- 
Bob Copeland %% https://bobcopeland.com/

  reply	other threads:[~2019-02-12 21:54 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
2019-02-12 18:43 ` David Miller
2019-02-12 19:03   ` Johannes Berg
2019-02-12 21:54     ` Bob Copeland [this message]
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=20190212215437.GA18182@localhost \
    --to=me@bobcopeland.com \
    --cc=davem@davemloft.net \
    --cc=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).