Re: [PATCH 1/2] Trace: PM: promote event 'power_domain_target' to generic power domains.

2015-09-25 Thread Steven Rostedt
On Fri, 25 Sep 2015 15:22:24 +0200
Marc Titinger  wrote:

> From: Marc Titinger 
> 
> - change arg3 to a state name string: we got the current CPU rom the trace
> backend already. This also prepares for multiple/named states in the power
> domain, consistent with idle-states. states in the domain may match given
> CPU states in this case, we will trace them by their name, and potentially
> use arg2 as a link to their index for the cpuidle driver.
> 
> - tested with Juno DP
> 
> -0 [000]42.395510: power_domain_target:  a53_pd index=0 
> 'cluster-sleep-0'
> -0 [000]42.395528: cpu_idle: state=4294967295 
> cpu_id=0
> -0 [000]42.395668: cpu_idle: state=2 cpu_id=0
> 
> - compiled for Omap2+
> 
> Signed-off-by: Marc Titinger 
> ---
>  drivers/base/power/domain.c  |  9 +
>  include/trace/events/power.h | 29 +++--
>  2 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 59ccd92..b9e2a37 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -21,6 +21,10 @@
>  #include 
>  #include 
>  
> +#ifdef CONFIG_PM_ADVANCED_DEBUG
> +#include 
> +#endif

Are the events in events/power.h only available when PM_ADVANCED_DEBUG
is enabled? If so, this should probably be encapsulated in that header
file, with an "#else" that makes all the trace_power_*() calls into
static inlined nops. Then you can get rid of the ugly #ifdef in this
file.


> +
>  #define GENPD_RETRY_MAX_MS   250 /* Approximate */
>  
>  #define GENPD_DEV_CALLBACK(genpd, type, callback, dev)   \
> @@ -328,6 +332,11 @@ static int __pm_genpd_poweron(struct generic_pm_domain 
> *genpd)
>  
>   out:
>   genpd->status = GPD_STATE_ACTIVE;
> +
> +#ifdef CONFIG_PM_ADVANCED_DEBUG
> + trace_power_domain_target(genpd->name, genpd->state_idx,
> + genpd->states[genpd->state_idx].name);
> +#endif
>   return 0;
>  
>   err:
> diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> index 284244e..8f93be6 100644
> --- a/include/trace/events/power.h
> +++ b/include/trace/events/power.h
> @@ -237,9 +237,9 @@ DECLARE_EVENT_CLASS(clock,
>   TP_ARGS(name, state, cpu_id),
>  
>   TP_STRUCT__entry(
> - __string(   name,   name)
> - __field(u64,state   )
> - __field(u64,cpu_id  )
> + __string(name, name)
> + __field(u64, state)
> + __field(u64, cpu_id)

Note, the standard formatting for the tracepoints is with the spaces.
Igonre checkpatch for that, tracepoints don't follow it.

-- Steve

>   ),
>  
>   TP_fast_assign(
> @@ -278,31 +278,32 @@ DEFINE_EVENT(clock, clock_set_rate,
>   */
>  DECLARE_EVENT_CLASS(power_domain,

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] arm: omap2+: PM: change trace_power_domain_target event format.

2015-09-25 Thread Steven Rostedt
On Fri, 25 Sep 2015 15:22:25 +0200
Marc Titinger  wrote:

> From: Marc Titinger 
> 
> power_domain_target arg3 is now a string (event name) with generic power
> domains. In the case of Omap, it is a hint to the prev/next switch op.
> Incidentally this trace is now conditioned by CONFIG_PM_ADVANCED_DEBUG.

I'm curious to why the addition of this config option?

> 
> Compiled for Omap2+ but not tested.
> 
> Signed-off-by: Marc Titinger 
> ---
>  arch/arm/mach-omap2/powerdomain.c | 32 
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/powerdomain.c 
> b/arch/arm/mach-omap2/powerdomain.c
> index 78af6d8..cd77696 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -160,7 +160,7 @@ static void _update_logic_membank_counters(struct 
> powerdomain *pwrdm)
>  static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
>  {
>  
> - int prev, next, state, trace_state = 0;
> + int prev, state;
>  
>   if (pwrdm == NULL)
>   return -EINVAL;
> @@ -177,18 +177,25 @@ static int _pwrdm_state_switch(struct powerdomain 
> *pwrdm, int flag)
>   pwrdm->state_counter[prev]++;
>   if (prev == PWRDM_POWER_RET)
>   _update_logic_membank_counters(pwrdm);
> - /*
> -  * If the power domain did not hit the desired state,
> -  * generate a trace event with both the desired and hit states
> -  */
> - next = pwrdm_read_next_pwrst(pwrdm);
> - if (next != prev) {
> - trace_state = (PWRDM_TRACE_STATES_FLAG |
> +
> +#ifdef CONFIG_PM_ADVANCED_DEBUG

You do realize that you can add this to the block:


if (trace_power_domain_target_enabled()) {

as it seems this code is only run to pass data to the tracepoint. The
above if statement will keep this block in the 'out-of-line' path when
tracing is not enabled via a static_key (jump-label).

-- Steve

> + {
> + /*
> +  * If the power domain did not hit the desired state,
> +  * generate a trace event with both the desired and hit
> +  * states */
> + int next = pwrdm_read_next_pwrst(pwrdm);
> +
> + if (next != prev) {
> + int trace_state = (PWRDM_TRACE_STATES_FLAG |
>  ((next & OMAP_POWERSTATE_MASK) << 8) |
>  ((prev & OMAP_POWERSTATE_MASK) << 0));
> - trace_power_domain_target(pwrdm->name, trace_state,
> -   smp_processor_id());
> + trace_power_domain_target(pwrdm->name,
> + trace_state, "PWRDM_STATE_PREV");
> + }
>   }
> +#endif
> +
>   break;
>   default:
>   return -EINVAL;
> @@ -522,9 +529,10 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 
> pwrst)
>pwrdm->name, pwrst);
>  
>   if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
> +#ifdef CONFIG_PM_ADVANCED_DEBUG
>   /* Trace the pwrdm desired target state */
> - trace_power_domain_target(pwrdm->name, pwrst,
> -   smp_processor_id());
> + trace_power_domain_target(pwrdm->name, pwrst, "set_next_pwrst");
> +#endif
>   /* Program the pwrdm desired target state */
>   ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
>   }

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: bug on ftrace with v3.17-rc5

2014-09-17 Thread Steven Rostedt
On Wed, 17 Sep 2014 12:22:11 -0500
Felipe Balbi ba...@ti.com wrote:

 Hi,
 
 I just triggered a bug with trace by using tail on the trace file:
 
 # tail trace
 [ 2940.039229] Unable to handle kernel paging request at virtual address 
 814efa9e
 [ 2940.046904] pgd = ec3dc000
 [ 2940.049737] [814efa9e] *pgd=
 [ 2940.053552] Internal error: Oops: 5 [#1] SMP ARM
 [ 2940.058379] Modules linked in: usb_f_acm u_serial g_serial usb_f_uac2 
 libcomposite configfs xhci_hcd dwc3 udc_core matrix_keypad dwc3_omap 
 lis3lv02d_i2c lis3lv02d input_polldev [last unloaded: g_audio]
 [ 2940.077238] CPU: 0 PID: 3020 Comm: tail Tainted: GW
 3.17.0-rc5-dirty #1097
 [ 2940.085596] task: ed1b1040 ti: ed07c000 task.ti: ed07c000
 [ 2940.091258] PC is at strnlen+0x18/0x68
 [ 2940.095177] LR is at 0xfffe
 [ 2940.098454] pc : [c0356df8]lr : [fffe]psr: a013
 [ 2940.098454] sp : ed07ddb0  ip : ed07ddc0  fp : ed07ddbc
 [ 2940.110445] r10: c070ff70  r9 : ed07de70  r8 : 
 [ 2940.115906] r7 : 814efa9e  r6 :   r5 : ed4b6087  r4 : ed4b50c7
 [ 2940.122726] r3 :   r2 : 814efa9e  r1 :   r0 : 814efa9e
 [ 2940.129546] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM Segment 
 user
 [ 2940.137000] Control: 10c5387d  Table: ac3dc059  DAC: 0015
 [ 2940.143006] Process tail (pid: 3020, stack limit = 0xed07c248)
 [ 2940.149098] Stack: (0xed07ddb0 to 0xed07e000)
 [ 2940.153660] dda0: ed07dde4 ed07ddc0 
 c0359628 c0356dec
 [ 2940.162203] ddc0:  ed4b50c7 bf03ae9c ed4b6087 bf03ae9e 0002 
 ed07de3c ed07dde8
 [ 2940.170740] dde0: c035ab50 c0359600   ff0a  
 ed07de30 ed4b5088
 [ 2940.179275] de00: ed4b50c7 0fc0 ff0a0004  ed4b5088 ed4b5088 
  1000
 [ 2940.187810] de20: 1008 0fc0 ed4b5088  ed07de68 ed07de40 
 c00f1e64 c035a9c4
 [ 2940.196341] de40: bf03dae0 ed07de70 ed4b4000 ec25b280 ed4b4000 ec25b280 
 bf03dae0 ed07de9c
 [ 2940.204886] de60: ed07de78 bf033324 c00f1e0c bf03ae9c 814efa9e ed428bc0 
 814eca3e 
 [ 2940.213428] de80: 814eba3e ed4b4000 03bd1201 c0c34790 ed07ded4 ed07dea0 
 c00edc0c bf0332d0
 [ 2940.221994] dea0: 02c7 ed07df10 ed07decc ed07deb8 ed4b4000 209c 
 ec278ac0 
 [ 2940.230536] dec0: 2000 ec0db340 ed07def4 ed07ded8 c00ee7ec c00eda90 
 c00ee7b0 ec278ac0
 [ 2940.239075] dee0: ed4b4000 02d5 ed07df44 ed07def8 c018b8d0 c00ee7bc 
 c0166d3c ec278af0
 [ 2940.247621] df00: 0001f090 ed07df78 02c7  02c8  
  ec0db340
 [ 2940.256173] df20: 0001f090 ed07df78 ec0db340 2000 0001f090  
 ed07df74 ed07df48
 [ 2940.264729] df40: c0166e98 c018b5f4 0001 c018535c 000168c1  
 ec0db340 ec0db340
 [ 2940.273284] df60: 2000 0001f090 ed07dfa4 ed07df78 c01675c4 c0166e0c 
 000168c1 
 [ 2940.281829] df80: 2000 000a 0001f090 0003 c000f064 ed07c000 
  ed07dfa8
 [ 2940.290365] dfa0: c000ede0 c0167584 2000 000a 0003 0001f090 
 2000 
 [ 2940.298909] dfc0: 2000 000a 0001f090 0003 7fffe000 0001e1e0 
 2004 002f
 [ 2940.307445] dfe0:  beed38ec 000104c8 b6e6397c 4010 0003 
  
 [ 2940.315992] [c0356df8] (strnlen) from [c0359628] 
 (string.isra.8+0x34/0xe8)
 [ 2940.323534] [c0359628] (string.isra.8) from [c035ab50] 
 (vsnprintf+0x198/0x3fc)
 [ 2940.331461] [c035ab50] (vsnprintf) from [c00f1e64] 
 (trace_seq_printf+0x68/0x94)
 [ 2940.339494] [c00f1e64] (trace_seq_printf) from [bf033324] 
 (ftrace_raw_output_dwc3_log_request+0x60/0x78 [dwc3])
 [ 2940.350424] [bf033324] (ftrace_raw_output_dwc3_log_request [dwc3]) from 
 [c00edc0c] (print_trace_line+0x188/0x418)

This shows it bugging with your tracepoint dwc3_log_request.

+DECLARE_EVENT_CLASS(dwc3_log_request,
+ TP_PROTO(struct dwc3_request *req),
+ TP_ARGS(req),
+ TP_STRUCT__entry(
+ __field(struct dwc3_request *, req)
+ ),
+ TP_fast_assign(
+ __entry-req = req;
+ ),
+ TP_printk(%s: req %p length %u/%u == %d,
+ __entry-req-dep-name, __entry-req,
+ __entry-req-request.actual, __entry-req-request.length,
+ __entry-req-request.status
+ )
+);

(Bah, cut and paste from the git web page doesn't format nicely)

Anyway, I take issues with that: __entry-req-request.length,
and all the other __entry-req-*

Basically, you saved a pointer in the buffer called req and than you
dereference it when you are printing. Does that pointer still exist?

If whatever you assigned req to gets freed, you can not dereference
it from the buffer! If you need to save the length and other fields, you
need to do it in the fast assign.

-- Steve



 [ 2940.361507] [c00edc0c] (print_trace_line) from [c00ee7ec] 
 (s_show+0x3c/0x12c)
 [ 2940.369330] [c00ee7ec] (s_show) from [c018b8d0] (seq_read+0x2e8/0x4a0)
 [ 2940.376519] [c018b8d0] (seq_read) from [c0166e98] (vfs_read+0x98/0x158)
 [ 2940.383796] [c0166e98] (vfs_read) from [c01675c4] (SyS_read+0x4c/0xa0)
 [ 2940.390981] [c01675c4] 

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

2014-08-18 Thread Steven Rostedt
On Mon, 18 Aug 2014 23:54:26 +0530
Mugunthan V N mugunthan...@ti.com wrote:

 On Saturday 16 August 2014 08:48 PM, Markus Pargmann wrote:
  +   mac_addr[5] = (macid_lo  8)  0xff;
  +   mac_addr[4] = macid_lo  0xff;
  +   mac_addr[3] = (macid_hi  24)  0xff;
  +   mac_addr[2] = (macid_hi  16)  0xff;
  +   mac_addr[1] = (macid_hi  8)  0xff;
  +   mac_addr[0] = macid_hi  0xff;
  +
 This will fail incase of DRA74x and DRA72x platforms, please check for
 u-boot src for parsing logic as TRM is not out yet. Below is the actual
 code for DRA7 platforms for MAC address parsing
 
 mac_addr[0] = (mac_hi  0xFF)  16;
 mac_addr[1] = (mac_hi  0xFF00)  8;
 mac_addr[2] = mac_hi  0xFF;
 mac_addr[3] = (mac_lo  0xFF)  16;
 mac_addr[4] = (mac_lo  0xFF00)  8;
 mac_addr[5] = mac_lo  0xFF;
 

But this fails with my beaglebone white.

I tested Markus's patches and it came up with the same ethaddr that
U-Boot had.

From U-Boot:

ethaddr=d4:94:a1:8b:ec:78

With Markus's changes:

 eth0  Link encap:Ethernet  HWaddr D4:94:A1:8B:EC:78

But when I changed the code to match what you wrote, I got this:

 eth0  Link encap:Ethernet  HWaddr CE:5A:8B:0E:44:45

but it also gave me:

cpsw 4a10.ethernet: Random MACID = ce:5a:8b:0e:44:45

which means it failed the valid mac test.

Here's how I implemented your change:

#if 1
mac_addr[0] = (macid_hi  0xFF)  16;
mac_addr[1] = (macid_hi  0xFF00)  8;
mac_addr[2] = macid_hi  0xFF;
mac_addr[3] = (macid_lo  0xFF)  16;
mac_addr[4] = (macid_lo  0xFF00)  8;
mac_addr[5] = macid_lo  0xFF;

#else   
mac_addr[5] = (macid_lo  8)  0xff;
mac_addr[4] = macid_lo  0xff;
mac_addr[3] = (macid_hi  24)  0xff;
mac_addr[2] = (macid_hi  16)  0xff;
mac_addr[1] = (macid_hi  8)  0xff;
mac_addr[0] = macid_hi  0xff;
#endif  

Just to be consistent, I updated the code as this too:

mac_addr[0] = (macid_hi  16)  0xFF;
mac_addr[1] = (macid_hi  8)  0xFF;
mac_addr[2] = macid_hi  0xFF;
mac_addr[3] = (macid_lo  16)  0xFF;
mac_addr[4] = (macid_lo  8)  0xFF;
mac_addr[5] = macid_lo  0xFF;

With the same affect.

Thus, for this patchset, as is:

Tested-by: Steven Rostedt rost...@goodmis.org

-- Steve
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

2014-08-18 Thread Steven Rostedt
On Tue, 19 Aug 2014 01:28:09 +0530
Mugunthan V N mugunthan...@ti.com wrote:


 This will fail for DRA7xx not in AM33xx

OK, is there a way to test the difference?

-- Steve

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Function Profiler broken on today's linux-next

2014-07-16 Thread Steven Rostedt
On Wed, 16 Jul 2014 11:23:28 -0500
Felipe Balbi ba...@ti.com wrote:

 Hi folks,
 
 I was trying to use Kernel Function Profiler to figure out why my
 driver's IRQ handler is taking so much CPU time but to my surprise,
 whenever I try to trace anything, I get a Unable to handle kernel
 paging request at virtual address  error.
 
 Is anybody else seen that or did I screw up my Kernel config ?

No, it's probably my fault. The ftrace infrastructure is going through
an overhaul and some dramatic changes are happening. Although it has
passed all my tests, I can't cover everyone's configs and use cases.

Please send me your config and the steps you did to enable function
profiling. This is x86 correct? Other archs are already known to be
broken and I'm working on fixing them now.

-- Steve



 
 cheers
 
 ps: I have tried tracing a few different drivers and they all trigger
 the same problem.
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Function Profiler broken on today's linux-next

2014-07-16 Thread Steven Rostedt
On Wed, 16 Jul 2014 11:41:42 -0500
Felipe Balbi ba...@ti.com wrote:


 .config attached. It's actually an ARM platform, I can help out with
 testing anything you need.

In that case, can you see if it works under my repo?

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
branch: ftrace/core

Thanks,

-- Steve

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Function Profiler broken on today's linux-next

2014-07-16 Thread Steven Rostedt
On Wed, 16 Jul 2014 13:41:52 -0500
Felipe Balbi ba...@ti.com wrote:

 Hi,
 
 On Wed, Jul 16, 2014 at 01:29:44PM -0500, Felipe Balbi wrote:
  On Wed, Jul 16, 2014 at 01:24:13PM -0400, Steven Rostedt wrote:
   On Wed, 16 Jul 2014 11:41:42 -0500
   Felipe Balbi ba...@ti.com wrote:
   
   
.config attached. It's actually an ARM platform, I can help out with
testing anything you need.
   
   In that case, can you see if it works under my repo?
   
   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
   branch: ftrace/core
  
  works fine, thanks. FWIW
  
  Tested-by: Felipe Balbi ba...@ti.com
 
 Actually it seems like it's not counting time properly if I run my test
 multiple times in a row:
 
 # /root/testusb -t 1 -c 10 -s 2048 -a
 # head -3 trace_stat/function0
   Function HitTimeAvg   
 s^2
    ------   
 ---
   thread_interrupt   174876166120839 us 949.935 us  
 3048498 us
 
 # /root/testusb -t 1 -c 10 -s 2048 -a
 # head -3 trace_stat/function0
   Function   Hit   TimeAvg
   s^2
      ---   ---
   ---
   dwc3_thread_interrupt286796967267300 us 4190545199 
 us 3241109 us 
 
 # /root/testusb -t 1 -c 10 -s 2048 -a
 # head -3 trace_stat/function0
   Function  Hit  Time Avg 
   s^2
 ---    ---
   ---
   dwc3_thread_interrupt   401346642632072 us   1601.192 
 us 1194316 us 
 
 and so on...
 

Did it use to work? In other words, is this a regression, or just some
another bug?

-- Steve

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] printk: add option to print cpu id

2012-08-06 Thread Steven Rostedt
On Fri, Aug 03, 2012 at 03:48:26PM -0700, Pandita, Vikram wrote:
 On Fri, Aug 3, 2012 at 3:36 PM, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Aug 03, 2012 at 03:25:17PM -0700, Pandita, Vikram wrote:
   This was something that got used internally and helped at times.
  
   Could you have used the trace point instead?
 
  As i understood the trace_prink(), one would need to modify existing
  printk - trace_printk. Is my understanding correct?
 
  No, you should just be able to watch the tracepoint, right?
 
 yes.
 Assumption being you know _EXACTLY_ what code piece to watch for.
 Which may not be the case all times.

But it traces all printks.

 # echo 1  /sys/kernel/debug/tracing/events/printk/console/enable
 # mount /home/rostedt
 # cat /sys/kernel/debug/tracing/trace
# tracer: nop
#
# entries-in-buffer/entries-written: 2/2   #P:4
#
#  _-= irqs-off
# / _= need-resched
#| / _---= hardirq/softirq
#|| / _--= preempt-depth
#||| / delay
#   TASK-PID   CPU#  TIMESTAMP  FUNCTION
#  | |   |      | |
modprobe-2707  [002] d..197.079458: console: [   95.816945] NFS: 
Registering the id_resolver key type

modprobe-2707  [002] d..197.084534: console: [   95.822038] Key 
type id_resolver registered

If you wanted this from boot up, you can just add to the kernel command line:

  trace_event=console

-- Steve

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4 0/4] Add ability to set defaultless network device MAC addresses to deterministic computed locally administered values

2012-07-10 Thread Steven Rostedt
On Tue, 2012-07-10 at 20:58 +0800, Andy Green (林安廸) wrote:
 On 10/07/12 20:37, the mail apparently from Florian Fainelli included:
 

 Why should Ubuntu, Fedora etc stink up their OSes with Panda-specific 
 workarounds?  And Panda is not the only device with this issue.

Actually I think you just answered your own question ;-)

Anyway, I don't think an initrd solution is the best. Yeah, it's fine
for a work around, but then I need to go and screw with the initrd if it
doesn't have support for the board. If the network card already has a
MAC address, why should the kernel give it another *random* one?

This isn't a complex patch set, where the complexity should be put into
userspace. And it makes it very convenient for people that just want the
board to boot so they can test it. I'm not developing any SoC or BSP,
I'm just using it to make sure my kernel changes can also be implemented
for ARM.

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2 2/4] net ethernet introduce mac_la_ap helper

2012-07-02 Thread Steven Rostedt
On Mon, 2012-07-02 at 16:12 +, Arnd Bergmann wrote:

   include/net/mac-la-ap.h  |   28 
   net/Kconfig  |   14 
   net/ethernet/Makefile|2 +
   net/ethernet/mac-la-ap.c |  165 
  ++
   4 files changed, 209 insertions(+)
   create mode 100644 include/net/mac-la-ap.h
   create mode 100644 net/ethernet/mac-la-ap.c
  diff --git a/include/net/mac-la-ap.h b/include/net/mac-la-ap.h
  new file mode 100644
  index 000..d5189b5
  --- /dev/null
  +++ b/include/net/mac-la-ap.h
  @@ -0,0 +1,28 @@
  +/*
  + * mac-la-ap.h:  Locally Administered MAC address for Async probed devices
  + */
 
 I find the name a bit non-obvious, not sure if we can come up with the
 perfect one.
 
 How about mac-platform?

I really find it unfortunate that 'mac' for ethernet has the same name
as the Apple box as well. 'mac' in other contexts can usually
differentiate these two, but saying 'mac-platform' just screams of
forbidden fruit.

mac-probe.h ?

-- Steve



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 0/2] OMAP2+: PANDA: Provide unique-ish MAC addresses for Ethernet and WLAN interfaces

2012-06-28 Thread Steven Rostedt
On Thu, 2012-06-28 at 14:18 +, Arnd Bergmann wrote:

 It's been a while since we discussed these patches, but Steven Rostedt
 just tripped over the same problem and the patches work for
 him, so he says Tested-by: Steven Rostedt rost...@goodmis.org.
 
 Can we get the patches into linux-3.6 please?

Yes please.

Right now I have my ktest.pl setup adding these patches before running
tests. I really dislike having to modify a kernel that I'm testing,
because I'm not really testing that said kernel, but a modified version
of it.

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] trace points: power: remove 'cpu_id' from trace_clock_*

2011-08-19 Thread Steven Rostedt
On Fri, 2011-08-19 at 23:04 +0800, tom.leim...@gmail.com wrote:
 From: Ming Lei tom.leim...@gmail.com
 
 This patch removes the 'cpu_id' parameter of the clock_*
 trace point, based on the ideas below:
 
 - the cpu_id which is passed to trace point is always the current
   cpu
 - the current cpu info has been included into the trace result already
 - smp_processor_id() can't be used safely in preemptible context.
 

Do you know if these changes wont break powertop?

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: PATCH [0/4] perf: clean-up of power events API

2010-10-10 Thread Steven Rostedt
On Sun, 2010-10-10 at 08:41 +0200, Peter Zijlstra wrote:
 On Sat, 2010-10-09 at 21:39 -0400, Steven Rostedt wrote:
  I've been hesitant in the pass from doing the TRACE_EVENT_ABI()
  before, because Peter Zijlstra (who is currently MIA) has been strongly
  against it. 
 
 I see no point in the TRACE_EVENT_ABI() because if I need to change such
 a tracepoint to reflect changes in the kernel then I will freely do so.
 
 Even seemingly stable points like sched_switch(), which we all agree
 will stay around forever (gotta have context switches on a multi-tasking
 OS) will not stay stable when we add/change scheduling policies.
 
 Sure, the prev and next task thing will stay the same, but the meaning
 and interpretation of things like the prio field will not, esp when we
 go add something like a deadline scheduler that isn't priority based.
 
 So one possibility is to simply remove all that information from the
 tracepoints, remove the prio and state fields, but how useful is that?
 
 I guess what I'm saying is that even if we were to provide _ABI I see us
 getting into this very same argument over and over again, making me want
 to remove all this trace event muck right now before it gets worse.

Then how's this as a compromise. We do not add a TRACE_EVENT_ABI(), but
instead manually add the ABI interface to existing tracepoints. Let's
use the sched example you shown above.

We can connect to the sched_switch() tracepoint manually in something
perhaps called kernel/abi_trace.c or trace_abi.c (whatever).

Here we create the directories manually there:

/sys/kernel/event/sched/sched_switch/

But this sched_switch will only include the prev and next pids, comms,
and perhaps even run state. But not the prio (since we see that
changing).

It would then need the code to enable the trace point with:

register_trace_sched_switch(sched_switch_abi_probe, NULL);

Where we have

static void
sched_switch_abi_probe(void *ignore,
struct task_switch *prev,
struct task_struct *next)
{
/* code to grab just the ABI stuff */
}

And this code can then record to what ever hooked to it.

Making this a manual effort will make it easier to control what becomes
an ABI. We can have long discussions and flames over what goes here. But
that's good since debates before an ABI is created is much better than
debates after one is created.

I'm afraid that a easy macro called TRACE_EVENT_ABI() would have the
same issue. ABIs may be created too quickly before they are thought
through.

Thoughts?

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: PATCH [0/4] perf: clean-up of power events API

2010-10-09 Thread Steven Rostedt
On Sat, 2010-10-09 at 11:36 -0700, Linus Torvalds wrote:
 On Sat, Oct 9, 2010 at 1:14 AM, Pierre Tardy tar...@gmail.com wrote:
  On Sat, Oct 9, 2010 at 8:28 AM, Ingo Molnar mi...@elte.hu wrote:
 
 
  The thing is, Arjan is 100% right that a library for this is not a
  'solution', it's an unnecessary complication.
  Yes. sounds like overengineering.
 
 I also want to remind people that backwards compatibility should
 always absolutely be the #1 priority. Using libraries to hide
 differences is a totally moronic thing to do, because if you can do a
 compatibility library with good interfaces, then damn it, the kernel
 interface should already _be_ that good interface.
 
 And no, even if you interact purely with open source programs, the
 backwards compatibility requirement doesn't go away. It's a damn pain
 in the ass to have to recompile, and it means that you have a much
 harder time mixing and matching, and just updating the kernel on top
 of a standard distribution.
 
 So changing kernel interfaces that get exported to user space is
 always a disaster. Anybody who _designs_ for that kind of disaster
 shouldn't be participating in kernel development, because they've
 shown themselves to be unable to understand the pain and suffering.
 
 Yes, we do it. Sometimes we change interfaces because not changing
 them is too damn painful. But it should absolutely not be the design
 model.

The difference here compared to all other user interfaces, is that this
interface has the sole purpose of showing what is happening inside the
kernel. By saying that we expose this to userspace, it must too be
stable is saying that all kernel internals that use trace events must
never change.

The big push against tracepoints/trace-markers/trace-events in the
beginning was the fear that they will hinder kernel development because
they become interfaces for users to see what is happening inside the
kernel. When I wrote the interface, I put it in the debugfs system so
people will know that this is a debug interface and can change without
notice.

Trace-events, unlike syscalls, may change depending on how you compiled
the kernel. There's no guarantee that they will even exist on a system.

If all trace-events are now stable ABI, then I suggest we stop adding
any more events, and only add new ones to places that we do not expect
to develop the kernel on anymore.

Not sure what other solution there is. Trace points have been added way
too freely, because any maintainer could add them to their system any
way they felt like it. Now if they are frozen in stone, then the code
that they expose must also be frozen.

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: PATCH [0/4] perf: clean-up of power events API

2010-10-09 Thread Steven Rostedt
On Sat, 2010-10-09 at 09:19 -0700, Arjan van de Ven wrote:

  I.e. it's not an ABI in the classic sense - we do not (because we
  cannot) guarantee the infinite availability of these events. But we can
  guarantee that the fields do not change in some stupid, avoidable way.
 
 also I have to say that some events are more likely to change than others
 
 function foo in the kernel called is more likely to change than the 
 processor went to THIS frequency.
 The concept of CPU frequencies has been with us fora long time and is 
 going to be there for a long time as well ..

Perhaps for basic concepts, we need a standard trace-event. Are people
willing to have a TRACE_EVENT_ABI() (it's trivial to write), and we can
mark those events with that macro that we know tools depend on.

These events can be exposed in a /sys/kernel/events/... directory, to
let tools know what what events they can rely on.

We've talked about doing this before, I've just been waiting to hear a
consensus on if we should. I know Peter Zijlstra was against the idea,
and too bad he's off gallivanting to share his input now.

-- Steve
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: PATCH [0/4] perf: clean-up of power events API

2010-10-09 Thread Steven Rostedt
On Sat, 2010-10-09 at 16:20 -0700, Linus Torvalds wrote:
 On Sat, Oct 9, 2010 at 2:15 PM, Steven Rostedt rost...@goodmis.org wrote:
 
  The difference here compared to all other user interfaces, is that this
  interface has the sole purpose of showing what is happening inside the
  kernel.
 
 Bogus and dishonest argument.
 
 Listen to yourself, and read this thread again.
 
 The thread was about doing some kind of open-source library to allow
 non-open-source access to these events, and keeping backwards
 compatibility in user space. In fact, that is what you yourself said.
 
 So you claimed it could be backwards-compatible. If that's the case,
 then there is no excuse for not being so in the kernel.
 
 You can't have it both ways. Stop the f*cking waffling.

Let me rephrase it then, and lets forget about the library. I was just
brain storming ideas.

I'm all for labeling specific trace points as ABI, such that, these
trace points have had sufficient thought and are not expected to change
in the near future. But I'm against the idea that any tracepoint that
has been shown to userspace can be considered stable.

With or without libraries, I'm for two kinds of interfaces: One that is
stable and has been thoroughly thought through, and one that is free for
the maintainers to have an interface to let them see what is happening
in the kernel, even on a production system, but be able to change them
whenever they feel the need.

That's the basis of my idea. A stable backward-compatible interface, and
an interface that is unstable for developers. Whether we put the stable
interface into a library (to keep the ugliness from developers, which
you obviously do not like), or two, distinctly label the tracepoint as
ABI, to let the developers and everyone else know what tracepoints an
application can count on and what ones they should not.

Thus my waffling is really wanting both, a stable ABI and an unstable
one. I've been hesitant in the pass from doing the TRACE_EVENT_ABI()
before, because Peter Zijlstra (who is currently MIA) has been strongly
against it.

-- Steve
 



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: PATCH [0/4] perf: clean-up of power events API

2010-10-08 Thread Steven Rostedt
On Fri, 2010-10-08 at 09:22 -0700, Arjan van de Ven wrote:
 On 10/8/2010 6:41 AM, Mathieu Desnoyers wrote:

 because that is not workable... at least nobody has shown to be able to 
 make this work.
 libraries (after compilation) live in /lib or /usr/lib (or lib64 I 
 suppose).
 what mechanism ensures that a user who compiles his kernel gets a 
 library compatible with that kernel in /usr/lib?
 and can said library deal with older kernels too? And distro kernels?

Perhaps we should have make install of a kernel also install this
library?

Have two libraries? One that is linked to the app, the other that can
search for another library to link on load too (like a kernel.ld.so)

Then we could see the kernel version, and search for a library that is
compatible, and load that one.

The app only needs to worry about loading the generic library. The
generic library can test for compatible libraries for the kernel.

Could just be..

  libkernel.ld.so  which then loads..

  /lib/modules/2.6.36/libkernel.so


Just a little brain storming.

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: PATCH [0/4] perf: clean-up of power events API

2010-10-08 Thread Steven Rostedt
On Fri, 2010-10-08 at 13:49 -0400, Frank Ch. Eigler wrote:
 Hi -
 
 On Fri, Oct 08, 2010 at 01:21:35PM -0400, Steven Rostedt wrote:
  [...]
  Perhaps we should have make install of a kernel also install this
  library?
  [...]
  The app only needs to worry about loading the generic library. The
  generic library can test for compatible libraries for the kernel.
  [...]
 
 If this library were to be distributed with the kernel, what would
 make the generic side of the interface any less permanent than a
 kernel ABI?  That is, if there is a libkernel-internals.so built from
 kernel sources, wouldn't its ABI become necessarily as fixed as any
 old syscall or procfs file?

One thing, the backwards compatibility would reside in user space. The
big advantage to that than for this to be in kernel space is that it is
only there when used. When we have backward compatibility in the kernel,
it's there in memory for everyone, whether you want it or not.

 
 One can have some backward compatibility with symbol versioning et
 al., but would that be sufficiently powerful to avoid handcuffing
 kernel developers' inclinations to make random future changes?
 

Sure, also note, that this is a two lib design. We still have
a /usr/lib/libkernel.so that the apps will interface with. This will
need to load in the other kernel versions. When we change interfaces, we
can make the /usr/lib/libkernel.so.1 .2 etc.

Also doing this dynamically from a library, we can check if the kernel
versions work. It can test if the used function is compatible or not,
use an older version, or just tell the user sorry, please update your
libkernel.so for this kernel. Doing this in userspace will allow a lot
more flexibility. We just need to think hard how these transactions will
work, and make it flexible for future enhancements.

But the kernel is free to do whatever it wants. The libraries will need
to worry about keeping the applications happy ;-)

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: PATCH [0/4] perf: clean-up of power events API

2010-10-07 Thread Steven Rostedt
On Thu, 2010-10-07 at 17:23 +0200, Pierre Tardy wrote:

 
 actually, over all the events pytimechart supports, only power traces
 are stable...

Let me rephrase that for you...

actually, over all the events pytimechart supports, only power traces
are inflexible...


-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] tracing, perf: add more power related events

2010-09-22 Thread Steven Rostedt
On Wed, 2010-09-22 at 19:30 +0200, Peter Zijlstra wrote:
 On Wed, 2010-09-22 at 10:06 -0700, Arjan van de Ven wrote:
 

 That said, I really didn't read this discussion much, but your stance
 seems to be that any tracepoint you use must stay valid, and I object to
 that.

We could add a TRACE_EVENT_ABI() as Ingo has been suggesting. If
anything, it could mean that the given tracepoint will always have the
same name. And perhaps the data it holds will always be there, but may
also be extended.

 
 What will do you do when we include a new scheduling policy and all the
 scheduler tracepoints need to change? (yes that's really going to
 happen)

The tracepoint sched_switch should stay the same. We may add more data,
but the comm, pid, prio = comm, pid, prio, I don't see going away.

 
 I'm not going to carry double tracepoints, and I'm not going to not
 merge that policy. 

Not sure what you mean by double tracepoints

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] tracing, perf: add more power related events

2010-09-22 Thread Steven Rostedt
On Wed, 2010-09-22 at 20:26 +0200, Peter Zijlstra wrote:
 On Wed, 2010-09-22 at 14:15 -0400, Steven Rostedt wrote:
  On Wed, 2010-09-22 at 19:30 +0200, Peter Zijlstra wrote:
   On Wed, 2010-09-22 at 10:06 -0700, Arjan van de Ven wrote:
   
  
   That said, I really didn't read this discussion much, but your stance
   seems to be that any tracepoint you use must stay valid, and I object to
   that.
  
  We could add a TRACE_EVENT_ABI() as Ingo has been suggesting. If
  anything, it could mean that the given tracepoint will always have the
  same name. And perhaps the data it holds will always be there, but may
  also be extended.
 
 I still don't see why you need TRACE_EVENT_ABI for that, if its the same
 name and the format can be extended you get the same results with what
 we've got. Apps need to read/parse the format thing anyway.

Just a marker that these trace points are being used by apps.

 
   
   What will do you do when we include a new scheduling policy and all the
   scheduler tracepoints need to change? (yes that's really going to
   happen)
  
  The tracepoint sched_switch should stay the same. We may add more data,
  but the comm, pid, prio = comm, pid, prio, I don't see going away.
 
 Right, it would need additional fields. Preferably not only at the end.

Why not at the end? The tools should easily be able to represent them in
anyway they want. The print-fmt field helps in this regard too.

But then again, we present the fields in the data. The tools should use
a parse library (which a generic one will soon be out too). This way, we
don't need them at the end but the parsing tools could find the fields
no matter where they are in the record.

 
   I'm not going to carry double tracepoints, and I'm not going to not

not going to not eeek! double negative!

   merge that policy. 
  
  Not sure what you mean by double tracepoints
 
 Two different tracepoints in the same location.

Agreed, that is wrong to have.

-- Steve


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html