On Sat, 2014-05-03 at 19:21 +0000, Mathieu Desnoyers wrote:
> ----- Original Message -----
> > From: "Dirk Behme" <dirk.behme@xxxxxxxxx>
> > To: "Mathieu Desnoyers" <mathieu.desnoyers@xxxxxxxxxxxx>
> > Cc: "Ben Hutchings" <ben@xxxxxxxxxxxxxxx>, "Kamal Mostafa" <kamal@xxxxxxxxxxxxx>, "gregkh"
> > <gregkh@xxxxxxxxxxxxxxxxxxx>, "stable" <stable@xxxxxxxxxxxxxxx>, "Luis Henriques" <luis.henriques@xxxxxxxxxxxxx>,
> > "Jiri Slaby" <jslaby@xxxxxxx>, "Roman Pen" <r.peniaev@xxxxxxxxx>, "Steven Rostedt" <rostedt@xxxxxxxxxxx>, "Frederic
> > Weisbecker" <fweisbec@xxxxxxxxx>, "Ingo Molnar" <mingo@xxxxxxxxxx>, "Jens Axboe" <axboe@xxxxxx>
> > Sent: Saturday, May 3, 2014 3:08:07 PM
> > Subject: Re: "blktrace: fix accounting ..." breaks lttng API in -stable trees
> >
> > Am 03.05.2014 20:38, schrieb Mathieu Desnoyers:
> > > ----- Original Message -----
> > >> From: "Ben Hutchings" <ben@xxxxxxxxxxxxxxx>
> > >> To: "Dirk Behme" <dirk.behme@xxxxxxxxx>
> > >> Cc: "Kamal Mostafa" <kamal@xxxxxxxxxxxxx>, "gregkh"
> > >> <gregkh@xxxxxxxxxxxxxxxxxxx>, "Mathieu Desnoyers"
> > >> <mathieu.desnoyers@xxxxxxxxxxxx>, "stable" <stable@xxxxxxxxxxxxxxx>, "Luis
> > >> Henriques"
> > >> <luis.henriques@xxxxxxxxxxxxx>, "Jiri Slaby" <jslaby@xxxxxxx>, "Roman Pen"
> > >> <r.peniaev@xxxxxxxxx>, "Steven Rostedt"
> > >> <rostedt@xxxxxxxxxxx>, "Frederic Weisbecker" <fweisbec@xxxxxxxxx>, "Ingo
> > >> Molnar" <mingo@xxxxxxxxxx>, "Jens Axboe"
> > >> <axboe@xxxxxx>
> > >> Sent: Saturday, May 3, 2014 2:33:56 PM
> > >> Subject: Re: "blktrace: fix accounting ..." breaks lttng API in -stable
> > >> trees
> > >>
> > >> On Sat, 2014-05-03 at 20:08 +0200, Dirk Behme wrote:
> > >>> Am 03.05.2014 18:43, schrieb Ben Hutchings:
> > >>>> On Sat, 2014-05-03 at 18:35 +0200, Dirk Behme wrote:
> > >>>>> Am 02.05.2014 20:09, schrieb Kamal Mostafa:
> > >>>>>> On Fri, 2014-05-02 at 13:30 -0400, gregkh wrote:
> > >>>>>>> On Fri, May 02, 2014 at 10:07:33AM -0700, Kamal Mostafa wrote:
> > >>>>>>>> Dirk Behme points out that this "Cc: stable" commit breaks the
> > >>>>>>>> lttng-modules userspace API when applied to stable kernels. Stable
> > >>>>>>>> versions 3.2, 3.8, 3.11, and 3.13 (at least) have all queued it:
> > >>>>>>>>
> > >>>>>>>> af5040da01ef980670b3741b3e10733ee3e33566
> > >>>>>>>> blktrace: fix accounting of partially completed requests
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> On Thu, 2014-05-01 at 10:28 +0200, Dirk Behme wrote:
> > >>>>>>>>> [...] might break the build of the user space lttng-modules
> > >>>>>>>>> (lttng-probe-block.c) due the the API change of
> > >>>>>>>>> trace_block_rq_complete().
> > >>>>>>>>
> > >>>>>>>>> [...] On the other hand, looking into the lttng-modules git
> > >>>>>>>>> http://git.lttng.org/?p=lttng-modules.git;a=commitdiff;h=1c53e689434a6bdd7dc3f54c07bfb72750d1d24c
> > >>>>>>>>> looks like this is the necessary user space adaption to the kernel
> > >>>>>>>>> change? So this looks like that lttng-modules expects a
> > >>>>>>>>> KERNEL_VERSION
> > >>>>>>>>>> = (3,15,0) to have this commit?
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> My inclination is that we probably need to revert/drop "af5040d
> > >>>>>>>> blktrace: fix accounting..." from the stable kernels to unbreak the
> > >>>>>>>> userspace API.
> > >>>>>>>
> > >>>>>>> Then you will run into this issue with 3.15, when it is released.
> > >>>>>>
> > >>>>>> No, I think "#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0))" in
> > >>>>>> the
> > >>>>>> lttng-modules commit referenced above guards against that.
> > >>>>>> Apparently,
> > >>>>>> lttng-modules expects to see the new API in >= 3.15 and the old API in
> > >>>>>> the stable kernels.
> > >>>>>
> > >>>>> I'm not in the position to judge if it's a lttng or a kernel community
> > >>>>> issue ;)
> > >>>>>
> > >>>>> So just for my understanding:
> > >>>>>
> > >>>>> Having
> > >>>>>
> > >>>>> #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0))
> > >>>>>
> > >>>>> in lttng-modules commit
> > >>>>>
> > >>>>> http://git.lttng.org/?p=lttng-modules.git;a=commitdiff;h=1c53e689434a6bdd7dc3f54c07bfb72750d1d24c
> > >>>>>
> > >>>>> does mean that we don't have a lttng-modules version which will build
> > >>>>> against the -stable kernels (3.2.58, 3,8.13.22 etc) with the back
> > >>>>> ported commit [1], atm?
> > >>>> [...]
> > >>>>
> > >>>> I don't understand why lttng-modules has its own definitions of
> > >>>> tracepoints. However, I suspect that it doesn't use anything beyond the
> > >>>> event structure definition, which has *not* changed, making that commit
> > >>>> a no-op.
> > >>>>
> > >>>> Someone who cares should actually experiment with using old
> > >>>> lttng-modules with blktrace eventsf rom a patched kernel.
> > >>>
> > >>> Not sure if this helps, but using -stable kernel 3.2.58 and recent
> > >>> lttng-modules.git:
> > >> [...]
> > >>> CC [M] lttng-modules.git/probes/lttng-probe-block.o
> > >> [...]
> > >>> Error: Conflicting types for »trace_block_rq_complete«
> > >>> In file included from lttng-modules.git/probes/lttng-probe-block.c:31:0:
> > >>> include/trace/events/block.h:123:1: Note: Previous definition of
> > >>> »trace_block_rq_complete« was here
> > >>> make[3]: *** [lttng-modules.git/probes/lttng-probe-block.o] Error 1
> > >> [...]
> > >>
> > >> So it's rebuilding the OOT module that fails, not trying to run already
> > >> existing userland. That's just tough; module API changes on a stable
> > >> branch are allowed and have happened many times. (Distributions tend to
> > >> care a bit more about this.)
> > >
> > > I'm currently going back starting from 3.2.xx stable kernels onward to
> > > add the missing ifdefs in LTTng to follow the module API changes within
> > > those stable branches. Indeed, LTTng being an OOT module will adapt to
> > > those upstream changes. I'll push the fixes into LTTng master and
> > > stable-2.4 branches.
> >
> > Many thanks!
> >
> > It would be nice if you could cover the stable branches done by the
> > Ubuntu team
> >
> > http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=summary
> >
> > too. Esp. I'm talking about linux-3.8.y.
>
> There are two aspects to this issue. First, I'm unsure how I can
> use the ".21" EXTRAVERSION to perform any kind of comparison on the
> version number within C macros.
The attached patch (which I'll submit to mainline pending review here)
adds a new KERNEL_EXTRAVERSION(a,b,c,d) macro to allow that. Comments?
Mathieu, if this new macro were available would you make use of it in
lttng-modules?
> So technically, handling the ubuntu
> stable branch versioning seems less than straightforward.
>
> The second aspect to this problem is that policy-wise, LTTng follows
> Linux upstream and stable branches. If there are any tweaks that need to
> be done for distro-specific tweaks, [...]
Calling this "the ubuntu stable branch" is a bit of a misnomer...
These extended stable 3.x.y.z kernels (3.5.7.z, 3.8.13.z, 3.11.10.z, and
3.13.11.z) contain only cc: stable commits from mainline, contain no
Ubuntu-specific changes, and follow the same patch acceptance rules and
release cycle methods as the 3.x.y kernel.org stable kernels.
-Kamal
> it's usually up to the distribution
> to integrate the changes within their own distributed LTTng packages.
> If we start adding distro-specific ifdefs within lttng-modules, then
> there will be an unmanageable list of distributions for which we'll need
> to clutter our code base.
>
> Thoughts ?
>
> Thanks,
>
> Mathieu
>
> >
> > Thanks
> >
> > Dirk
> >
> >
>
>From a3f7266605bdaa4c3279d8ea01b73b0ace2791e1 Mon Sep 17 00:00:00 2001
From: Kamal Mostafa <kamal@xxxxxxxxxxxxx>
Date: Tue, 13 May 2014 10:43:59 -0700
Subject: Makefile: version.h macros KERNEL_EXTRAVERSION,
LINUX_EXTRAVERSION_CODE
Adds new macro KERNEL_EXTRAVERSION(a,b,c,d) and value
LINUX_EXTRAVERSION_CODE to version.h, allowing ranged version checks of
extended-stable versions that use a numeric EXTRAVERSION value.
The new KERNEL_EXTRAVERSION(a,b,c,d) works like KERNEL_VERSION(a,b,c)...
If EXTRAVERSION is set to ".{integer}" (the common extended-stable kernel
version number scheme) then {integer} corresponds to KERNEL_EXTRAVERSION's
fourth arg. If EXTRAVERSION is blank or a non-".{integer}" string like
"-rc5", that corresponds to a fourth parameter of 0.
Example usage:
#if ( LINUX_EXTRAVERSION_CODE >= KERNEL_EXTRAVERSION(3,13,11,5) )
// compiles for (3.13.11 with "EXTRAVERSION = .5") or greater value
#endif
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Kamal Mostafa <kamal@xxxxxxxxxxxxx>
---
Makefile | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 8a8440a..a735495 100644
--- a/Makefile
+++ b/Makefile
@@ -938,10 +938,15 @@ define filechk_utsrelease.h
(echo \#define UTS_RELEASE \"$(KERNELRELEASE)\";)
endef
+extraversionnum := $(shell \
+ echo "$(EXTRAVERSION)" | sed -n '/\.\([0-9]*\).*/s//\1/p')
define filechk_version.h
(echo \#define LINUX_VERSION_CODE $(shell \
expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \
- echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))';)
+ echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))'; \
+ echo \#define LINUX_EXTRAVERSION_CODE $(shell \
+ expr $(VERSION) \* 268435456 + 0$(PATCHLEVEL) \* 1048576 + 0$(SUBLEVEL) \* 4096 + 0$(extraversionnum)); \
+ echo '#define KERNEL_EXTRAVERSION(a,b,c,d) (((a)<<28) + ((b)<<20) + ((c)<<12) + (d))';)
endef
$(version_h): $(srctree)/Makefile FORCE
--
1.9.1