[Bug uprobes/4738] New: UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[Bug uprobes/4738] New: UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[ananth at in dot ibm dot com]

Since UPROBES_SSOL and URETPROBES config options cannot exist without
UPROBES=y/m, they need to depend on UPROBES in the Kconfig menu.

-- 
           Summary: UPROBES_SSOL and URETPROBES must depend on UPROBES in
                    Kconfig
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: uprobes
        AssignedTo: systemtap at sources dot redhat dot com
        ReportedBy: ananth at in dot ibm dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[jkenisto at us dot ibm dot com]

------- Additional Comments From jkenisto at us dot ibm dot com  2007-07-07 00:16 -------
The need for the dependency is debatable.  UPROBES should be toggleable by the
user, but UPROBES_SSOL and URETPROBES should not be: they are defined (or not)
by the uprobes implementer.  If UPROBES is left unconfigured, the uprobes source
won't even be compiled, so the effect (if any) of such a change would be limited
to the uprobes.h headers.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |minor
             Status|NEW                         |ASSIGNED
           Priority|P2                          |P3


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[ananth at in dot ibm dot com]

------- Additional Comments From ananth at in dot ibm dot com  2007-07-09 04:16 -------
In which case, we have one of two choices:

a> Make the dependency implicit as we do with kprobes (no KPROBES=y implies no
kretprobes either); this will require that we don't touch the Kconfig files,
except to add CONFIG_UPROBES.
b> If you want to retain the Kconfig entries for SSOL and URETPROBES, put in an
explicit dependency for the two on UPROBES. Per Kconfig language semantics, you
shouldn't be able to select a dependent feature without an explicit dependency
specified in the Kconfig file.

Need to choose one; You can't have both :-)

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[jkenisto at us dot ibm dot com]

------- Additional Comments From jkenisto at us dot ibm dot com  2007-07-09 20:52 -------
(In reply to comment #2)
> In which case, we have one of two choices:
> 
> a> Make the dependency implicit as we do with kprobes (no KPROBES=y implies no
> kretprobes either);

That's pretty much what we're doing now.

> this will require that we don't touch the Kconfig files,
> except to add CONFIG_UPROBES.

No, it doesn't require that.  See advice from Dave Hansen, appended to this comment.


> b> If you want to retain the Kconfig entries for SSOL and URETPROBES, put in an
> explicit dependency for the two on UPROBES. Per Kconfig language semantics, you
> shouldn't be able to select a dependent feature without an explicit dependency
> specified in the Kconfig file.

Again, support for SSOL or uretprobes is not something we need/want the user to
select.

> 
> Need to choose one; You can't have both :-)

I disagree.  But I would be willing to consider the following modifications, if
you'll test them and verify that (1) they do what you want and (2) SSOL and
uretprobes do NOT show up as choices in make *config.  (I ask you cuz you've
already been experimenting with Kconfig changes.)

config UPROBES_SSOL
        bool
        default y
+       depends on UPROBES

config URETPROBES
        bool
        default y
+       depends on UPROBES

Note that there should be no reason for arch-specific code to test
CONFIG_UPROBES_SSOL or CONFIG_URETPROBES.

Jim

Advice from Dave Hansen, 4/24/07:
[even number of >s is Dave; odd number of >s is Jim]
-----
> > > > > +#define SS_OUT_OF_LINE
> > > >
> > > > This should be done in Kconfig instead of here.
> > >
> > > Defining this says that this architecture's implementation of uprobes
> > > supports single-stepping out of line (SSOL).  What we expect of a
> > > port to a new architecture is:
> > >
> > > 1) Support only single-stepping inline, which is pretty easy.  Debug
> > > that.  Leave SS_OUT_OF_LINE undefined.
> > >
> > > 2) Then, add support for SSOL -- e.g., add the fixups in
> > > uprobe_resume_execution().  Add the #define of SS_OUT_OF_LINE.
> > >
> > > We don't put this in Kconfig because it's not something the user can or
> > > should enable/disable.
> >
> > Kconfig is not just about what users can enable or disable.  It is about
> > expressing dependencies for the kernel, and does not always involve
> > prompting.  The architecture Kconfigs have all kinds of stuff that you
> > never see in the actual 'make menuconfig' menus.
>
> There are 1-2 dozen such macros that AREN'T defined in Kconfig files --
> stuff like ARCH_HAS_NMI_WATCHDOG, ARCH_HAS_SETUP_ADDITIONAL_PAGES,
> ARCH_SUPPORTS_KRETPROBES.  Are these mistakes, or is there documentation
> somewhere as to what goes in Kconfig and what doesn't?

They're bad form.  Everything goes in Kconfig unless you have a really,
really good excuse.
-----


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[ananth at in dot ibm dot com]

------- Additional Comments From ananth at in dot ibm dot com  2007-07-10 04:53 -------
Subject: Re:  UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

On Mon, Jul 09, 2007 at 08:52:47PM -0000, jkenisto at us dot ibm dot com wrote:
> 
> ------- Additional Comments From jkenisto at us dot ibm dot com  2007-07-09 20:52 -------
> (In reply to comment #2)
> > In which case, we have one of two choices:
> > 
> > a> Make the dependency implicit as we do with kprobes (no KPROBES=y implies no
> > kretprobes either);
> 
> That's pretty much what we're doing now.
> 
> > this will require that we don't touch the Kconfig files,
> > except to add CONFIG_UPROBES.
> 
> No, it doesn't require that.  See advice from Dave Hansen, appended to this comment.

I disagree. If you follow that caveat, then why aren't KRETPROBES and
JPROBES in Kconfig?

> > b> If you want to retain the Kconfig entries for SSOL and URETPROBES, put in an
> > explicit dependency for the two on UPROBES. Per Kconfig language semantics, you
> > shouldn't be able to select a dependent feature without an explicit dependency
> > specified in the Kconfig file.
> 
> Again, support for SSOL or uretprobes is not something we need/want the user to
> select.
> 
> > 
> > Need to choose one; You can't have both :-)
> 
> I disagree.  But I would be willing to consider the following modifications, if
> you'll test them and verify that (1) they do what you want and (2) SSOL and
> uretprobes do NOT show up as choices in make *config.  (I ask you cuz you've
> already been experimenting with Kconfig changes.)
> 
> config UPROBES_SSOL
>         bool
>         default y
> +       depends on UPROBES
> 
> config URETPROBES
>         bool
>         default y
> +       depends on UPROBES

This is precisely what I've been asking for (dependency) and have suggested
in <b> above. I'd be happy with this change.
 
> Note that there should be no reason for arch-specific code to test
> CONFIG_UPROBES_SSOL or CONFIG_URETPROBES.

But that is orthogonal to this issue. My main point being, if I run
menuconfig on an arch that doesn't implement SSOL/URETPROBES, I
shouldn't be able to set them independently of UPROBES *and* more
importantly, they shouldn't show up if UPROBES=n.

Ananth


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[jkenisto at us dot ibm dot com]

------- Additional Comments From jkenisto at us dot ibm dot com  2007-07-10 21:01 -------
(In reply to comment #4)
...
> > 
> > > this will require that we don't touch the Kconfig files,
> > > except to add CONFIG_UPROBES.
> > 
> > No, it doesn't require that.  See advice from Dave Hansen, appended to this
comment.
> 
> I disagree. If you follow that caveat, then why aren't KRETPROBES and
> JPROBES in Kconfig?

According to Dave H, they should be.

...

> > ...  But I would be willing to consider the following modifications, if
> > you'll test them and verify that (1) they do what you want and (2) SSOL and
> > uretprobes do NOT show up as choices in make *config.  (I ask you cuz you've
> > already been experimenting with Kconfig changes.)
> > 
> > config UPROBES_SSOL
> >         bool
> >         default y
> > +       depends on UPROBES
> > 
> > config URETPROBES
> >         bool
> >         default y
> > +       depends on UPROBES
> 
> This is precisely what I've been asking for (dependency) and have suggested
> in <b> above. I'd be happy with this change.

I'm glad you're happy with that solution.  But your original email on the
subject (6/21/07 6:21 am PDT) suggested something rather different -- including
a prompt string on the "bool" line, which would cause the config options to show
up in make *config.

>  
> > Note that there should be no reason for arch-specific code to test
> > CONFIG_UPROBES_SSOL or CONFIG_URETPROBES.
> 
> But that is orthogonal to this issue. My main point being, if I run
> menuconfig on an arch that doesn't implement SSOL/URETPROBES, I
> shouldn't be able to set them independently of UPROBES *and* more
> importantly, they shouldn't show up if UPROBES=n.

Again (and again and again), they shouldn't show up as user-configurable options
in make *config under any circumstances.

> 
> Ananth
> 

I will add the indicated "depends on UPROBES" lines in the next patch set.  I've
verified that they add the dependency without making the options user-configurable.

Jim

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2007-07-10 21:11 -------
Could someone spell out why SSOL and URETPROBES should be configuration
options at all?


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[jkenisto at us dot ibm dot com]

------- Additional Comments From jkenisto at us dot ibm dot com  2007-07-10 22:21 -------
(In reply to comment #6)
> Could someone spell out why SSOL and URETPROBES should be configuration
> options at all?
> 

See "Advice from Dave Hansen" (from a code review) in Comment #3.  Look at
arch/*/Kconfig, and you'll see lots of "config" options with no prompts -- which
means they don't show up in make *config and aren't intended to be tweaked by
the user.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2007-07-11 00:41 -------
(In reply to comment #7)
> See "Advice from Dave Hansen" (from a code review) in Comment #3.  Look at
> arch/*/Kconfig, and you'll see lots of "config" options with no prompts [...]

What I'm missing is why these should show up as CONFIG_ANYTHING at all.
Why not just include uretprobes always, and SSOL automagically as available
on the platform?


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[ananth at in dot ibm dot com]

------- Additional Comments From ananth at in dot ibm dot com  2007-07-11 04:19 -------
Subject: Re:  UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

On Tue, Jul 10, 2007 at 09:01:38PM -0000, jkenisto at us dot ibm dot com wrote:
> 
> > I disagree. If you follow that caveat, then why aren't KRETPROBES and
> > JPROBES in Kconfig?
> 
> According to Dave H, they should be.

I am sorry, I don't agree. If you go by that convention with the kernel,
you'd have a terribly unmanagable Kconfig.

...
 
> Again (and again and again), they shouldn't show up as user-configurable options
> in make *config under any circumstances.

Then simply remove it from Kconfig. I am not OK with having it in
Kconfig if it is not user configurable.


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[jkenisto at us dot ibm dot com]

------- Additional Comments From jkenisto at us dot ibm dot com  2007-07-11 17:02 -------
(In reply to comment #8)
> (In reply to comment #7)
> > See "Advice from Dave Hansen" (from a code review) in Comment #3.  Look at
> > arch/*/Kconfig, and you'll see lots of "config" options with no prompts [...]
> 
> What I'm missing is why these should show up as CONFIG_ANYTHING at all.
> Why not just include uretprobes always,

Some people seem to think uretprobes is harder to grok and/or port than one
might suppose.  So we provide for the possibility of a port that implements
uprobes but not uretprobes.

> and SSOL automagically as available
> on the platform?

We do.  However, as part of the magic, the platform-dependent code must indicate
whether it supports SSOL.  The interminable argument is about how to express that.



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug uprobes/4738] UPROBES_SSOL and URETPROBES must depend on UPROBES in Kconfig

[jkenisto at us dot ibm dot com]

------- Additional Comments From jkenisto at us dot ibm dot com  2007-07-11 17:25 -------
(In reply to comment #9)
> > > I disagree. If you follow that caveat, then why aren't KRETPROBES and
> > > JPROBES in Kconfig?
> > 
> > According to Dave H, they should be.
> 
> I am sorry, I don't agree. If you go by that convention with the kernel,

... as the kernel community seems to be doing...

> you'd have a terribly unmanagable Kconfig.

... and yet Linux survives.
> 
> ...
>  
> > Again (and again and again), they shouldn't show up as user-configurable options
> > in make *config under any circumstances.
> 
> Then simply remove it from Kconfig.

No.

> I am not OK with having it in
> Kconfig if it is not user configurable.

Got it.  When we post uprobes to LKML, if there's an outcry about this, and it
turns out Dave H was wrong, and patches are submitted to remove those hundreds
of promptless entries in arch/*/Kconfig, I'll fix uprobes.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=4738

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.