From mboxrd@z Thu Jan  1 00:00:00 1970
From: Florian Westphal <fw@strlen.de>
Subject: Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module
Date: Mon, 13 Nov 2017 08:59:49 +0100
Message-ID: <20171113075949.GO5512@breakpoint.cc>
References: <20171106105113.20476-1-fw@strlen.de>
 <20171106105113.20476-3-fw@strlen.de>
 <20171106124454.GI3165@worktop.lehotels.local>
 <20171107061156.GK9424@breakpoint.cc>
 <20171107091004.GI3326@worktop>
 <20171107092458.GF3857@worktop>
 <20171107094751.GM9424@breakpoint.cc>
 <20171107145736.GN3165@worktop.lehotels.local>
 <20171113072159.GN5512@breakpoint.cc>
 <20171113075515.ltphkflhq4uk3rwe@hirez.programming.kicks-ass.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Florian Westphal <fw@strlen.de>, netdev@vger.kernel.org
To: Peter Zijlstra <peterz@infradead.org>
Return-path: <netdev-owner@vger.kernel.org>
Received: from Chamillionaire.breakpoint.cc ([146.0.238.67]:45450 "EHLO
        Chamillionaire.breakpoint.cc" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S1751377AbdKMIAe (ORCPT
        <rfc822;netdev@vger.kernel.org>); Mon, 13 Nov 2017 03:00:34 -0500
Content-Disposition: inline
In-Reply-To: <20171113075515.ltphkflhq4uk3rwe@hirez.programming.kicks-ass.net>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Nov 13, 2017 at 08:21:59AM +0100, Florian Westphal wrote:
> > Reason is that some places do this:
> > 
> > rtnl_register(pf, RTM_FOO, doit, NULL, 0);
> > rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);
> 
> Sure, however,
> 
> > (from different call sites in the stack).
> > > -	if (doit)
> > > -		tab[msgindex].doit = doit;
> > > -	if (dumpit)
> > > -		tab[msgindex].dumpit = dumpit;
> > 
> > Which is the reason for these if () tests.
> 
> then we assign NULL, which is fine, no?

I meant that
1) rtnl_register(pf, RTM_FOO, doit, NULL, 0);
2) rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);

2) overwrites doit() back to NULL.

(it doesn't at the moment due to if() guard quoted above).

We could not do this split, and keep the if () around.

But then we change a member of the link array after it has
been published via rcu_assign_pointer.

AFAIU this is exactly what you want to avoid with this patch.