LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Pranith Kumar <bobby.prani@gmail.com>
To: Scott Wood <scottwood@freescale.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	"supporter:S390" <linux390@de.ibm.com>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Josh Triplett <josh@joshtriplett.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Anton Blanchard <anton@samba.org>,
	Andy Fleming <afleming@freescale.com>,
	Tiejun Chen <tiejun.chen@windriver.com>,
	Daniel Walter <dwalter@google.com>,
	Jens Freimann <jfrei@linux.vnet.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	"open list:LINUX FOR POWERPC..." <linuxppc-dev@lists.ozlabs.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:S390" <linux-s390@vger.kernel.org>
Subject: Re: [PATCH] srcu: Isolate srcu sections using CONFIG_SRCU
Date: Tue, 30 Dec 2014 00:06:22 -0500	[thread overview]
Message-ID: <CAJhHMCBTYOsfZOH92KhTAbqH+UqsjuBNUGzxRwUT+Ah7P0eJ2g@mail.gmail.com> (raw)
In-Reply-To: <1419894305.4961.2.camel@freescale.com>

On Mon, Dec 29, 2014 at 6:05 PM, Scott Wood <scottwood@freescale.com> wrote:
> On Sat, 2014-12-27 at 12:17 -0500, Pranith Kumar wrote:
>> Isolate the SRCU functions and data structures within CONFIG_SRCU so that there
>> is a compile time failure if srcu is used when not enabled. This was decided to
>> be better than waiting until link time for a failure to occur.
>
> Yes, false positives and extra ifdefs are so much better. :-P
>
> Why not just ifdef the functions/macros, and leave the types alone?  If
> you're worried about direct access to struct members, you could even
> ifdef the members away while leaving the struct itself.  It is not
> normal practice in Linux to need ifdefs around #includes.

Yup, totally agree that this is not ideal. The idea here is to not
even compile the structure for tinification purposes. ifdefs for
headers are ugly, but given the current code structure, I was not able
to figure out any other way around it without major overhaul.

>
>> There are places which include kvm headers and utilize kvm data structures
>> without checking if KVM is enabled. In two such archs(s390, ppc64), the current
>> patch makes the uses of KVM conditional on KVM being enabled. The other option,
>> which is to enable KVM unconditionally seemed a bit too much as we could easily
>> figure out KVM only parts and enclose them in ifdefs.
>
> Maybe not so easy (mpc85xx_smp_defconfig with NOTIFY stuff turned off so
> that SRCU gets deselected):
>
> In file included from /home/scott/fsl/git/linux/upstream/arch/powerpc/include/asm/kvm_ppc.h:30:0,
>                  from /home/scott/fsl/git/linux/upstream/arch/powerpc/kernel/smp.c:39:
> /home/scott/fsl/git/linux/upstream/include/linux/kvm_host.h:366:21: error: field 'srcu' has incomplete type
> /home/scott/fsl/git/linux/upstream/include/linux/kvm_host.h:367:21: error: field 'irq_srcu' has incomplete type
> /home/scott/fsl/git/linux/upstream/scripts/Makefile.build:257: recipe for target 'arch/powerpc/kernel/smp.o' failed
> make[2]: *** [arch/powerpc/kernel/smp.o] Error 1
> /home/scott/fsl/git/linux/upstream/Makefile:955: recipe for target 'arch/powerpc/kernel' failed
> make[1]: *** [arch/powerpc/kernel] Error 2
> make[1]: *** Waiting for unfinished jobs....
>
> Are you sure KVM is the only SRCU user so impacted?  It's also likely
> that new such problems get introduced, because most people are going to
> have SRCU enabled and thus not notice the breakage they're adding.

Well, it is the major one which I encountered until now. There might
be other problems lurking which I will gladly try to fix if and when
they are reported.

>
> There's also at least one place that needs to be fixed, that currently
> expects to get other headers indirectly via srcu.h:
>
> /home/scott/fsl/git/linux/upstream/lib/assoc_array.c: In function 'assoc_array_apply_edit':
> /home/scott/fsl/git/linux/upstream/lib/assoc_array.c:1425:2: error: implicit declaration of function 'call_rcu' [-Werror=implicit-function-declaration]
> cc1: some warnings being treated as errors
> /home/scott/fsl/git/linux/upstream/scripts/Makefile.build:257: recipe for target 'lib/assoc_array.o' failed

I will send a patch fixing this(need to use rcupdate.h here
explicitly). Thanks for reporting these!

>
> -Scott
>
>



-- 
Pranith

  reply	other threads:[~2014-12-30  5:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-27 17:17 Pranith Kumar
2014-12-27 17:17 ` [PATCH] powerpc/powernv: Select CONFIG_PRINTK to fix build failure Pranith Kumar
2014-12-29  9:01   ` Michael Ellerman
2014-12-30  5:46     ` Pranith Kumar
2014-12-29 10:03 ` [PATCH] srcu: Isolate srcu sections using CONFIG_SRCU Martin Schwidefsky
2014-12-30  4:32   ` Pranith Kumar
2014-12-30 19:44     ` Scott Wood
2014-12-29 23:05 ` Scott Wood
2014-12-30  5:06   ` Pranith Kumar [this message]
  -- strict thread matches above, loose matches on Subject: below --
2014-12-09 18:48 Pranith Kumar
2014-12-09 20:12 ` Mathieu Desnoyers
2014-12-10  0:41 ` Paul E. McKenney
2014-12-09 14:16 Pranith Kumar
2014-12-09 17:02 ` Paul E. McKenney
2014-12-08 20:23 Pranith Kumar
2014-12-08 15:55 Pranith Kumar
2014-12-08 18:10 ` Paul E. McKenney

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=CAJhHMCBTYOsfZOH92KhTAbqH+UqsjuBNUGzxRwUT+Ah7P0eJ2g@mail.gmail.com \
    --to=bobby.prani@gmail.com \
    --cc=afleming@freescale.com \
    --cc=anton@samba.org \
    --cc=benh@kernel.crashing.org \
    --cc=borntraeger@de.ibm.com \
    --cc=dwalter@google.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jfrei@linux.vnet.ibm.com \
    --cc=josh@joshtriplett.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux390@de.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=rostedt@goodmis.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=scottwood@freescale.com \
    --cc=tiejun.chen@windriver.com \
    --subject='Re: [PATCH] srcu: Isolate srcu sections using CONFIG_SRCU' \
    /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

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).