Re: [PATCH 2/2] arm: omap2+: PM: change trace_power_domain_target event format.
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: [PATCH 1/2] Trace: PM: promote event 'power_domain_target' to generic power domains.
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: bug on ftrace with v3.17-rc5
On Wed, 17 Sep 2014 12:22:11 -0500
Felipe Balbi 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 : []lr : []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] [] (strnlen) from []
> (string.isra.8+0x34/0xe8)
> [ 2940.323534] [] (string.isra.8) from []
> (vsnprintf+0x198/0x3fc)
> [ 2940.331461] [] (vsnprintf) from []
> (trace_seq_printf+0x68/0x94)
> [ 2940.339494] [] (trace_seq_printf) from []
> (ftrace_raw_output_dwc3_log_request+0x60/0x78 [dwc3])
> [ 2940.350424] [] (ftrace_raw_output_dwc3_log_request [dwc3]) from
> [] (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] [] (print_trace_line) from []
> (s_show+0x3c/0x12c)
> [ 2940.369330] [] (s_show) from [] (seq_read+0x2e8/0x4a0)
> [ 2940.376519] [] (seq_read) from [] (vfs_read+0x98/0x158)
> [ 2940.383796] [] (vfs_read) from [] (SyS_read+0x4c/0xa0)
> [ 2940.390981] [] (SyS_read) from []
> (ret_fast_syscall+0x0/0x48)
> [ 2940.398792] Code: e24cb004
Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout
On Tue, 19 Aug 2014 01:28:09 +0530 Mugunthan V N 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: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout
On Mon, 18 Aug 2014 23:54:26 +0530
Mugunthan V N 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
-- 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
On Wed, 16 Jul 2014 13:41:52 -0500 Felipe Balbi 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 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 > > 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: Function Profiler broken on today's linux-next
On Wed, 16 Jul 2014 11:41:42 -0500 Felipe Balbi 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
On Wed, 16 Jul 2014 11:23:28 -0500 Felipe Balbi 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: [PATCH v3] printk: add option to print cpu id
On Fri, Aug 03, 2012 at 03:48:26PM -0700, Pandita, Vikram wrote: > On Fri, Aug 3, 2012 at 3:36 PM, Greg KH 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
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
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
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 ". > > 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_*
On Fri, 2011-08-19 at 23:04 +0800, tom.leim...@gmail.com wrote: > From: Ming Lei > > 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
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
On Sat, 2010-10-09 at 16:20 -0700, Linus Torvalds wrote: > On Sat, Oct 9, 2010 at 2:15 PM, Steven Rostedt 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
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
On Sat, 2010-10-09 at 11:36 -0700, Linus Torvalds wrote: > On Sat, Oct 9, 2010 at 1:14 AM, Pierre Tardy wrote: > > On Sat, Oct 9, 2010 at 8:28 AM, Ingo Molnar 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
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
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
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
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
Re: [PATCH] tracing, perf: add more power related events
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
