All of lore.kernel.org
 help / color / mirror / Atom feed
From: dave.long@linaro.org (David Long)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 00/14] uprobes: Add uprobes support for ARM
Date: Mon, 03 Mar 2014 05:08:06 -0500	[thread overview]
Message-ID: <53145486.8060009@linaro.org> (raw)
In-Reply-To: <20140303062324.GB20583@linux.vnet.ibm.com>

On 03/03/14 01:23, Srikar Dronamraju wrote:

> It should be me who should take the blame for this and not Oleg.  This
> was discussed more than couple of times.  I can recollect couple of
> discussions here.
> http://article.gmane.org/gmane.linux.kernel/1017186
> http://article.gmane.org/gmane.linux.kernel/1001605

I wasn't trying to assign blame to anyone, I was just soliciting an 
opinion from the last uprobes maintainer I had a conversation with. 
Thanks for the links.

> I know there were more discussions on this, but I cant dig them out at
> this time.  Finally it was decided that
> 1. Users shouldnt have to select more than one config to select Uprobes.
> 2. There was no point in selecting Uprobes and not having Uprobe_event
> and vice versa.
>
>  From the above, If a user chose UPROBE_EVENT, (which is the interface
> for uprobes), we would automatically assume that he wants to use Uprobes
> framework.
>
>> like "select" is used in part maybe just to avoid the recursive
>> dependency error that would be generated if "depends on" were used
>> in both places.
>
> We did "Select Uprobes" not because of avoiding recursive dependency but
> as told above, to select the framework, given that user has chosen the
> framework. We dont want to give a choice to user to choose uprobe_event
> but not choose Uprobes or vice versa.

I suppose that's more to the point.

>> However I don't think UPROBES should be dependent on
>> UPROBE_EVENT, only the other way around.  As RK noted in previous
>
> Whats the point of having the framework(Uprobes) without an interface?
>

My comment was based only in the fact it built successfully that way on 
both x86 and ARM.  If there's no way to access the functionality without 
both selected then I suppose it does make sense to not allow that 
configuration.  Maybe it's time to remove one of these config symbols. 
I didn't see anything in the email history on this that says that would 
be a bad idea.  I'll try and come up with a patch.

-dl

WARNING: multiple messages have this Message-ID (diff)
From: David Long <dave.long@linaro.org>
To: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Oleg Nesterov <oleg@redhat.com>,
	linux-arm-kernel@lists.infradead.org,
	Rabin Vincent <rabin@rab.in>,
	"Jon Medhurst (Tixy)" <tixy@linaro.org>,
	Ingo Molnar <mingo@redhat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
	davem@davemloft.net, Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Mackerras <paulus@samba.org>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 00/14] uprobes: Add uprobes support for ARM
Date: Mon, 03 Mar 2014 05:08:06 -0500	[thread overview]
Message-ID: <53145486.8060009@linaro.org> (raw)
In-Reply-To: <20140303062324.GB20583@linux.vnet.ibm.com>

On 03/03/14 01:23, Srikar Dronamraju wrote:

> It should be me who should take the blame for this and not Oleg.  This
> was discussed more than couple of times.  I can recollect couple of
> discussions here.
> http://article.gmane.org/gmane.linux.kernel/1017186
> http://article.gmane.org/gmane.linux.kernel/1001605

I wasn't trying to assign blame to anyone, I was just soliciting an 
opinion from the last uprobes maintainer I had a conversation with. 
Thanks for the links.

> I know there were more discussions on this, but I cant dig them out at
> this time.  Finally it was decided that
> 1. Users shouldnt have to select more than one config to select Uprobes.
> 2. There was no point in selecting Uprobes and not having Uprobe_event
> and vice versa.
>
>  From the above, If a user chose UPROBE_EVENT, (which is the interface
> for uprobes), we would automatically assume that he wants to use Uprobes
> framework.
>
>> like "select" is used in part maybe just to avoid the recursive
>> dependency error that would be generated if "depends on" were used
>> in both places.
>
> We did "Select Uprobes" not because of avoiding recursive dependency but
> as told above, to select the framework, given that user has chosen the
> framework. We dont want to give a choice to user to choose uprobe_event
> but not choose Uprobes or vice versa.

I suppose that's more to the point.

>> However I don't think UPROBES should be dependent on
>> UPROBE_EVENT, only the other way around.  As RK noted in previous
>
> Whats the point of having the framework(Uprobes) without an interface?
>

My comment was based only in the fact it built successfully that way on 
both x86 and ARM.  If there's no way to access the functionality without 
both selected then I suppose it does make sense to not allow that 
configuration.  Maybe it's time to remove one of these config symbols. 
I didn't see anything in the email history on this that says that would 
be a bad idea.  I'll try and come up with a patch.

-dl


  reply	other threads:[~2014-03-03 10:08 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-10  7:38 [PATCH v6 00/14] uprobes: Add uprobes support for ARM David Long
2014-02-10  7:38 ` David Long
2014-02-10  7:38 ` [PATCH v6 01/14] uprobes: allow ignoring of probe hits David Long
2014-02-10  7:38   ` David Long
2014-02-10  7:38 ` [PATCH v6 02/14] ARM: move shared uprobe/kprobe definitions into new include file David Long
2014-02-10  7:38   ` David Long
2014-02-10  7:38 ` [PATCH v6 03/14] ARM: Move generic arm instruction parsing code to new files for sharing between features David Long
2014-02-10  7:38   ` David Long
2014-02-10  7:38 ` [PATCH v6 04/14] ARM: move generic thumb instruction parsing code to new files for use by other feature David Long
2014-02-10  7:38   ` David Long
2014-02-10  7:38 ` [PATCH v6 05/14] ARM: use a function table for determining instruction interpreter action David Long
2014-02-10  7:38   ` David Long
2014-02-10  7:38 ` [PATCH v6 06/14] ARM: Disable jprobes test when built into thumb-mode kernel David Long
2014-02-10  7:38   ` David Long
2014-02-10  7:38 ` [PATCH v6 07/14] ARM: Remove use of struct kprobe from generic probes code David Long
2014-02-10  7:38   ` David Long
2014-02-28 10:12   ` Russell King - ARM Linux
2014-02-28 10:12     ` Russell King - ARM Linux
2014-02-28 14:11     ` Jon Medhurst (Tixy)
2014-02-28 14:11       ` Jon Medhurst (Tixy)
2014-02-28 14:45       ` Jon Medhurst (Tixy)
2014-03-02 10:37     ` David Long
2014-03-02 10:37       ` David Long
2014-03-02 12:11       ` Russell King - ARM Linux
2014-03-02 12:11         ` Russell King - ARM Linux
2014-02-10  7:38 ` [PATCH v6 08/14] ARM: Make the kprobes condition_check symbol names more generic David Long
2014-02-10  7:38   ` David Long
2014-02-10  7:39 ` [PATCH v6 09/14] ARM: Change more ARM kprobes symbol names to something more David Long
2014-02-10  7:39   ` David Long
2014-02-10  7:39 ` [PATCH v6 10/14] ARM: Rename the shared kprobes/uprobe return value enum David Long
2014-02-10  7:39   ` David Long
2014-02-10  7:39 ` [PATCH v6 11/14] ARM: Change the remaining shared kprobes/uprobes symbols to something generic David Long
2014-02-10  7:39   ` David Long
2014-02-10  7:39 ` [PATCH v6 12/14] ARM: Add an emulate flag to the kprobes/uprobes instruction decode functions David Long
2014-02-10  7:39   ` David Long
2014-02-10  7:39 ` [PATCH v6 13/14] ARM: Make arch_specific_insn a define for new arch_probes_insn structure David Long
2014-02-10  7:39   ` David Long
2014-02-10  7:39 ` [PATCH v6 14/14] ARM: add uprobes support David Long
2014-02-10  7:39   ` David Long
2014-03-01 12:30 ` [PATCH v6 00/14] uprobes: Add uprobes support for ARM Russell King - ARM Linux
2014-03-01 12:30   ` Russell King - ARM Linux
2014-03-02 12:02   ` David Long
2014-03-02 12:02     ` David Long
2014-03-03  6:23     ` Srikar Dronamraju
2014-03-03  6:23       ` Srikar Dronamraju
2014-03-03 10:08       ` David Long [this message]
2014-03-03 10:08         ` David Long
2014-03-03 10:39       ` Russell King - ARM Linux
2014-03-03 10:39         ` Russell King - ARM Linux
2014-03-03 20:50     ` Oleg Nesterov
2014-03-03 20:50       ` Oleg Nesterov
2014-03-04  0:53       ` Russell King - ARM Linux
2014-03-04  0:53         ` Russell King - ARM Linux
2014-03-04 17:31         ` Oleg Nesterov
2014-03-04 17:31           ` Oleg Nesterov
2014-03-06  8:10           ` David Long
2014-03-06  8:10             ` David Long

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=53145486.8060009@linaro.org \
    --to=dave.long@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.