On Thu, 2010-08-26 at 12:47 +0200, Jan Kiszka wrote:
> Philippe Gerum wrote:
> > On Thu, 2010-08-26 at 11:38 +0200, Jan Kiszka wrote:
> >> Philippe Gerum wrote:
> >>> On Wed, 2010-08-25 at 12:25 +0200, Jan Kiszka wrote:
> >>>> Philippe Gerum wrote:
> >>>>> On Wed, 2010-08-25 at 11:19 +0200, Jan Kiszka wrote:
> >>>>>> Philippe Gerum wrote:
> >>>>>>> On Wed, 2010-08-25 at 10:58 +0200, Jan Kiszka wrote:
> >>>>>>>> Philippe Gerum wrote:
> >>>>>>>>> On Wed, 2010-08-25 at 10:50 +0200, Jan Kiszka wrote:
> >>>>>>>>>> Philippe Gerum wrote:
> >>>>>>>>>>> On Fri, 2010-07-02 at 13:50 +0200, Wolfgang Mauerer wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> <snip>
> >>>>>>>>>>>
> >>>>>>>>>>>> diff --git a/include/linux/ipipe_tickdev.h 
> >>>>>>>>>>>> b/include/linux/ipipe_tickdev.h
> >>>>>>>>>>>> index 4a1cb1b..86f13e0 100644
> >>>>>>>>>>>> --- a/include/linux/ipipe_tickdev.h
> >>>>>>>>>>>> +++ b/include/linux/ipipe_tickdev.h
> >>>>>>>>>>>> @@ -25,6 +25,7 @@
> >>>>>>>>>>>>  #if defined(CONFIG_IPIPE) && defined(CONFIG_GENERIC_CLOCKEVENTS)
> >>>>>>>>>>> Since we should have CONFIG_HAVE_IPIPE_HOSTRT by now, let's use 
> >>>>>>>>>>> it.
> >>>>>>>>>> Don't get yet how this fits here.
> >>>>>>>>> arch-dep would define CONFIG_HAVE_IPIPE_HOSTRT [if IPIPE]
> >>>>>>>>>
> >>>>>>>> Still don't see the relation to the line you cited above.
> >>>>>>>>
> >>>>>>> That is because you chose to have CONFIG_IPIPE_HOSTRT and
> >>>>>>> CONFIG_HAVE_IPIPE_HOSTRT. I would have only defined the latter, the 
> >>>>>>> way
> >>>>>>> you define the former. I'm looking for the hostrt support to be 
> >>>>>>> compiled
> >>>>>>> in if CONFIG_HAVE_IPIPE_HOSTRT is available from the arch-dep section,
> >>>>>>> so we don't need CONFIG_IPIPE_HOSTRT. Generic bits may depend on 
> >>>>>>> HAVE_*
> >>>>>>> as well.
> >>>>>> First of all, the code you cited _above_ is not changed by our patches,
> >>>>>> so the context still puzzles me (but maybe you are referring to some
> >>>>>> other place in fact).
> >>>>> Patch v2 says:
> >>>>>
> >>>>> diff --git a/kernel/ipipe/Kconfig b/kernel/ipipe/Kconfig
> >>>>> index de5e6a3..bc7a00c 100644
> >>>>> --- a/kernel/ipipe/Kconfig
> >>>>> +++ b/kernel/ipipe/Kconfig
> >>>>> @@ -33,3 +33,10 @@ config IPIPE_UNMASKED_CONTEXT_SWITCH
> >>>>>         bool
> >>>>>         depends on IPIPE
> >>>>>         default n
> >>>>> +
> >>>>> +config HAVE_IPIPE_HOSTRT
> >>>>> +       bool
> >>>>> +
> >>>>> +config IPIPE_HOSTRT
> >>>>> +       def_bool y
> >>>>> +       depends on HAVE_IPIPE_HOSTRT && IPIPE
> >>>>>
> >>>>> So what's your point?
> >>>>>
> >>>>>> Second, CONFIG_HAVE_IPIPE_HOSTRT is designed to be set independently of
> >>>>>> CONFIG_IPIPE - it's a static arch feature like all the other
> >>>>>> CONFIG_HAVE_* in arch/*/Kconfig. So it takes a second, generically
> >>>>>> defined CONFIG switch if the generic support also depends on
> >>>>>> CONFIG_IPIPE like in this case.
> >>>>> Which does not make any sense. We don't want to make this selectable at
> >>>>> all. Mainline has CONFIG_HAVE_SYSCALL_WRAPPERS for instance, and you
> >>>>> won't find any CONFIG_SYSCALL_WRAPPERS, because it makes no sense not to
> >>>>> use them when the architecture _have_ them. It goes exactly the same way
> >>>>> with hostrt.
> >>>> Don't find your example.
> >>> I just did:
> >>>  find . -name 'Kconfig*' -print |xargs grep SYSCALL_WRAPPERS
> >> Ah, now I see.
> >>
> >>>>  But maybe you should have a look at
> >>>> [HAVE_]USER_RETURN_NOTIFIER (and maybe I should push [HAVE_]IPIPE_HOSTRT
> >>>> into arch/Kconfig).
> >>> And select it conditionally on IPIPE in arch/x86/Kconfig? why not.
> >> I can change this if you want us to, but I think it would be better to
> >> have the generic dependency on IPIPE in a generic Kconfig - not every
> >> arch version.
> > 
> > What I mean is:
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index acda512..1b87a53 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -151,4 +151,7 @@ config HAVE_MIXED_BREAKPOINTS_REGS
> >  config HAVE_USER_RETURN_NOTIFIER
> >     bool
> >  
> > +config HAVE_IPIPE_HOSTRT
> > +       bool
> > +
> >  source "kernel/gcov/Kconfig"
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index dcb0593..96ab0f4 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -58,6 +58,7 @@ config X86
> >     select ANON_INODES
> >     select HAVE_ARCH_KMEMCHECK
> >     select HAVE_USER_RETURN_NOTIFIER
> > +   select HAVE_IPIPE_HOSTRT if IPIPE
> >  
> >  config INSTRUCTION_DECODER
> >     def_bool (KPROBES || PERF_EVENTS)
> > 
> 
> I fully understood what you meant, and to end this discussion I already
> implemented it. We will just have to make sure that every arch properly
> adds the required "if IPIPE" suffix.

Given that "the other way" (i.e. moving everything to the generic
section) woul not work, and that adding an I-pipe symbol conditionally
to have the pipeline enabled is not generally silly, I think it's ok as
well.

> 
> Jan
> 
> 

-- 
Philippe.



_______________________________________________
Adeos-main mailing list
[email protected]
https://mail.gna.org/listinfo/adeos-main

Reply via email to