Re: "blktrace: fix accounting ..." breaks lttng API in -stable trees

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



----- Original Message -----
> From: "Mathieu Desnoyers" <mathieu.desnoyers@xxxxxxxxxxxx>
> To: "Ben Hutchings" <ben@xxxxxxxxxxxxxxx>
> Cc: "Dirk Behme" <dirk.behme@xxxxxxxxx>, "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 2:38:25 PM
> Subject: Re: "blktrace: fix accounting ..." breaks lttng API in -stable trees
> 
> ----- 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.

Currently, only the 3.2.58 stable kernel is released with this fix. I
will therefore add the following fix into lttng-modules:

-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0))
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0)      \
+       || LTTNG_KERNEL_RANGE(3,2,58, 3,3,0))

and include "../lttng-kernel-version.h" within lttng-probe-block.c in
lttng-modules (to get the LTTNG_KERNEL_RANGE definition).

I'll wait for the release of the other stable branches including this
fix before adding the appropriate ifdef for them into LTTng master and
stable-2.4 branches.

Thanks,

Mathieu

> 
> Thanks,
> 
> Mathieu
> 
> > 
> > Ben.
> > 
> > --
> > Ben Hutchings
> > All the simple programs have been written, and all the good names taken.
> > 
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]