[PATCH] relay: add buffer-only functionality, allowing for early kernel tracing - Kernel

This is a discussion on [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing - Kernel ; relay_open() can now handle NULL base_filename, to register a buffer-only channel. Using a new function, relay_late_setup_files(), one can assign files after creating the channel, thus allowing for doing early tracing in the kernel, before VFS is up. Signed-off-by: Eduard - ...

+ Reply to Thread
Results 1 to 8 of 8

Thread: [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

  1. [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

    relay_open() can now handle NULL base_filename, to register a
    buffer-only channel. Using a new function, relay_late_setup_files(), one
    can assign files after creating the channel, thus allowing for doing
    early tracing in the kernel, before VFS is up.

    Signed-off-by: Eduard - Gabriel Munteanu
    ---
    Documentation/filesystems/relay.txt | 11 ++++
    include/linux/relay.h | 4 ++
    kernel/relay.c | 102 ++++++++++++++++++++++++++---------
    3 files changed, 91 insertions(+), 26 deletions(-)

    diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
    index 094f2d2..b59c228 100644
    --- a/Documentation/filesystems/relay.txt
    +++ b/Documentation/filesystems/relay.txt
    @@ -161,6 +161,7 @@ TBD(curr. line MT:/API/)
    relay_close(chan)
    relay_flush(chan)
    relay_reset(chan)
    + relay_late_setup_files(chan, base_filename, parent)

    channel management typically called on instigation of userspace:

    @@ -294,6 +295,16 @@ user-defined data with a channel, and is immediately available
    (including in create_buf_file()) via chan->private_data or
    buf->chan->private_data.

    +Buffer-only channels
    +--------------------
    +
    +These channels have no files associated and can be created with
    +relay_open(NULL, NULL, ...). Such channels are useful in scenarios such
    +as when doing early tracing in the kernel, before the VFS is up. In these
    +cases, one may open a channel a buffer-only channel and then call
    +relay_late_setup_files() when the kernel is ready to handle files, to expose
    +the buffered data to the userspace.
    +
    Channel 'modes'
    ---------------

    diff --git a/include/linux/relay.h b/include/linux/relay.h
    index 6cd8c44..ca9ee3f 100644
    --- a/include/linux/relay.h
    +++ b/include/linux/relay.h
    @@ -68,6 +68,7 @@ struct rchan
    int is_global; /* One global buffer ? */
    struct list_head list; /* for channel list */
    struct dentry *parent; /* parent dentry passed to open */
    + int has_base_filename; /* has a filename associated? */
    char base_filename[NAME_MAX]; /* saved base filename */
    };

    @@ -169,6 +170,9 @@ struct rchan *relay_open(const char *base_filename,
    size_t n_subbufs,
    struct rchan_callbacks *cb,
    void *private_data);
    +extern int relay_late_setup_files(struct rchan *chan,
    + const char *base_filename,
    + struct dentry *parent);
    extern void relay_close(struct rchan *chan);
    extern void relay_flush(struct rchan *chan);
    extern void relay_subbufs_consumed(struct rchan *chan,
    diff --git a/kernel/relay.c b/kernel/relay.c
    index 4c035a8..4bea94a 100644
    --- a/kernel/relay.c
    +++ b/kernel/relay.c
    @@ -378,6 +378,35 @@ void relay_reset(struct rchan *chan)
    }
    EXPORT_SYMBOL_GPL(relay_reset);

    +static int relay_setup_buf_file(struct rchan *chan,
    + struct rchan_buf *buf,
    + unsigned int cpu)
    +{
    + struct dentry *dentry;
    + char *tmpname;
    +
    + tmpname = kzalloc(NAME_MAX + 1, GFP_KERNEL);
    + if (!tmpname)
    + goto failed;
    + snprintf(tmpname, NAME_MAX, "%s%d", chan->base_filename, cpu);
    +
    + /* Create file in fs */
    + dentry = chan->cb->create_buf_file(tmpname, chan->parent,
    + S_IRUSR, buf,
    + &chan->is_global);
    +
    + if (!dentry)
    + goto failed;
    + buf->dentry = dentry;
    +
    + kfree(tmpname);
    +
    + return 0;
    +
    +failed:
    + return 1;
    +}
    +
    /*
    * relay_open_buf - create a new relay channel buffer
    *
    @@ -386,44 +415,30 @@ EXPORT_SYMBOL_GPL(relay_reset);
    static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
    {
    struct rchan_buf *buf = NULL;
    - struct dentry *dentry;
    - char *tmpname;

    if (chan->is_global)
    return chan->buf[0];

    - tmpname = kzalloc(NAME_MAX + 1, GFP_KERNEL);
    - if (!tmpname)
    - goto end;
    - snprintf(tmpname, NAME_MAX, "%s%d", chan->base_filename, cpu);
    -
    buf = relay_create_buf(chan);
    if (!buf)
    - goto free_name;
    + goto end;
    +
    + if (chan->has_base_filename)
    + if (relay_setup_buf_file(chan, buf, cpu)) goto free_buf;

    buf->cpu = cpu;
    __relay_reset(buf, 1);

    - /* Create file in fs */
    - dentry = chan->cb->create_buf_file(tmpname, chan->parent, S_IRUSR,
    - buf, &chan->is_global);
    - if (!dentry)
    - goto free_buf;
    -
    - buf->dentry = dentry;
    -
    if(chan->is_global) {
    chan->buf[0] = buf;
    buf->cpu = 0;
    }

    - goto free_name;
    + goto end;

    free_buf:
    relay_destroy_buf(buf);
    buf = NULL;
    -free_name:
    - kfree(tmpname);
    end:
    return buf;
    }
    @@ -508,8 +523,8 @@ static int __cpuinit relay_hotcpu_callback(struct notifier_block *nb,

    /**
    * relay_open - create a new relay channel
    - * @base_filename: base name of files to create
    - * @parent: dentry of parent directory, %NULL for root directory
    + * @base_filename: base name of files to create, %NULL for buffering only
    + * @parent: dentry of parent directory, %NULL for root directory or buffer
    * @subbuf_size: size of sub-buffers
    * @n_subbufs: number of sub-buffers
    * @cb: client callback functions
    @@ -531,8 +546,6 @@ struct rchan *relay_open(const char *base_filename,
    {
    unsigned int i;
    struct rchan *chan;
    - if (!base_filename)
    - return NULL;

    if (!(subbuf_size && n_subbufs))
    return NULL;
    @@ -547,12 +560,17 @@ struct rchan *relay_open(const char *base_filename,
    chan->alloc_size = FIX_SIZE(subbuf_size * n_subbufs);
    chan->parent = parent;
    chan->private_data = private_data;
    - strlcpy(chan->base_filename, base_filename, NAME_MAX);
    + if (base_filename) {
    + chan->has_base_filename = 1;
    + strlcpy(chan->base_filename, base_filename, NAME_MAX);
    + } else {
    + chan->has_base_filename = 0;
    + }
    setup_callbacks(chan, cb);
    kref_init(&chan->kref);

    mutex_lock(&relay_channels_mutex);
    - for_each_online_cpu(i) {
    + for_each_present_cpu(i) {
    chan->buf[i] = relay_open_buf(chan, i);
    if (!chan->buf[i])
    goto free_bufs;
    @@ -563,7 +581,7 @@ struct rchan *relay_open(const char *base_filename,
    return chan;

    free_bufs:
    - for_each_online_cpu(i) {
    + for_each_present_cpu(i) {
    if (!chan->buf[i])
    break;
    relay_close_buf(chan->buf[i]);
    @@ -576,6 +594,38 @@ free_bufs:
    EXPORT_SYMBOL_GPL(relay_open);

    /**
    + * relay_late_setup_files - triggers file creation
    + * @chan: channel to operate on
    + * @base_filename: base name of files to create
    + * @parent: dentry of parent directory, %NULL for root directory
    + *
    + * Returns 0 is successful, non-zero otherwise.
    + *
    + * Use to setup files for a previously buffer-only channel.
    + * Useful to do early tracing in kernel, before VFS is up, for example.
    + */
    +int relay_late_setup_files(struct rchan *chan,
    + const char *base_filename,
    + struct dentry *parent)
    +{
    + unsigned int i;
    +
    + if (!chan || !base_filename)
    + return 1;
    +
    + strlcpy(chan->base_filename, base_filename, NAME_MAX);
    + chan->has_base_filename = 1;
    + chan->parent = parent;
    +
    + mutex_lock(&relay_channels_mutex);
    + for_each_present_cpu(i)
    + relay_setup_buf_file(chan, chan->buf[i], i);
    + mutex_unlock(&relay_channels_mutex);
    +
    + return 0;
    +}
    +
    +/**
    * relay_switch_subbuf - switch to a new sub-buffer
    * @buf: channel buffer
    * @length: size of current event
    --
    1.5.2.5
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

    * Eduard - Gabriel Munteanu (eduard.munteanu@linux360.ro) wrote:
    > relay_open() can now handle NULL base_filename, to register a
    > buffer-only channel. Using a new function, relay_late_setup_files(), one
    > can assign files after creating the channel, thus allowing for doing
    > early tracing in the kernel, before VFS is up.
    >


    I like this But could we trace event sooner if we did not depend on
    vmalloc to allocate the buffers ?

    I am thinking of receiving a kernel parameter that would reserve a
    buffer in the linear kernel mapping (kmalloc ?), or to compile-in a
    static buffer, which could be used by relay instead of the vmalloc'd
    buffer. That would permit to trace the memory allocator and some other
    very early stuff. Basically, relay would have it's "memory pool" to
    manage and would do its allocations from this fixed buffer. No free
    would be required.

    Mathieu

    > Signed-off-by: Eduard - Gabriel Munteanu
    > ---
    > Documentation/filesystems/relay.txt | 11 ++++
    > include/linux/relay.h | 4 ++
    > kernel/relay.c | 102 ++++++++++++++++++++++++++---------
    > 3 files changed, 91 insertions(+), 26 deletions(-)
    >
    > diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
    > index 094f2d2..b59c228 100644
    > --- a/Documentation/filesystems/relay.txt
    > +++ b/Documentation/filesystems/relay.txt
    > @@ -161,6 +161,7 @@ TBD(curr. line MT:/API/)
    > relay_close(chan)
    > relay_flush(chan)
    > relay_reset(chan)
    > + relay_late_setup_files(chan, base_filename, parent)
    >
    > channel management typically called on instigation of userspace:
    >
    > @@ -294,6 +295,16 @@ user-defined data with a channel, and is immediately available
    > (including in create_buf_file()) via chan->private_data or
    > buf->chan->private_data.
    >
    > +Buffer-only channels
    > +--------------------
    > +
    > +These channels have no files associated and can be created with
    > +relay_open(NULL, NULL, ...). Such channels are useful in scenarios such
    > +as when doing early tracing in the kernel, before the VFS is up. In these
    > +cases, one may open a channel a buffer-only channel and then call
    > +relay_late_setup_files() when the kernel is ready to handle files, to expose
    > +the buffered data to the userspace.
    > +
    > Channel 'modes'
    > ---------------
    >
    > diff --git a/include/linux/relay.h b/include/linux/relay.h
    > index 6cd8c44..ca9ee3f 100644
    > --- a/include/linux/relay.h
    > +++ b/include/linux/relay.h
    > @@ -68,6 +68,7 @@ struct rchan
    > int is_global; /* One global buffer ? */
    > struct list_head list; /* for channel list */
    > struct dentry *parent; /* parent dentry passed to open */
    > + int has_base_filename; /* has a filename associated? */
    > char base_filename[NAME_MAX]; /* saved base filename */
    > };
    >
    > @@ -169,6 +170,9 @@ struct rchan *relay_open(const char *base_filename,
    > size_t n_subbufs,
    > struct rchan_callbacks *cb,
    > void *private_data);
    > +extern int relay_late_setup_files(struct rchan *chan,
    > + const char *base_filename,
    > + struct dentry *parent);
    > extern void relay_close(struct rchan *chan);
    > extern void relay_flush(struct rchan *chan);
    > extern void relay_subbufs_consumed(struct rchan *chan,
    > diff --git a/kernel/relay.c b/kernel/relay.c
    > index 4c035a8..4bea94a 100644
    > --- a/kernel/relay.c
    > +++ b/kernel/relay.c
    > @@ -378,6 +378,35 @@ void relay_reset(struct rchan *chan)
    > }
    > EXPORT_SYMBOL_GPL(relay_reset);
    >
    > +static int relay_setup_buf_file(struct rchan *chan,
    > + struct rchan_buf *buf,
    > + unsigned int cpu)
    > +{
    > + struct dentry *dentry;
    > + char *tmpname;
    > +
    > + tmpname = kzalloc(NAME_MAX + 1, GFP_KERNEL);
    > + if (!tmpname)
    > + goto failed;
    > + snprintf(tmpname, NAME_MAX, "%s%d", chan->base_filename, cpu);
    > +
    > + /* Create file in fs */
    > + dentry = chan->cb->create_buf_file(tmpname, chan->parent,
    > + S_IRUSR, buf,
    > + &chan->is_global);
    > +
    > + if (!dentry)
    > + goto failed;
    > + buf->dentry = dentry;
    > +
    > + kfree(tmpname);
    > +
    > + return 0;
    > +
    > +failed:
    > + return 1;
    > +}
    > +
    > /*
    > * relay_open_buf - create a new relay channel buffer
    > *
    > @@ -386,44 +415,30 @@ EXPORT_SYMBOL_GPL(relay_reset);
    > static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
    > {
    > struct rchan_buf *buf = NULL;
    > - struct dentry *dentry;
    > - char *tmpname;
    >
    > if (chan->is_global)
    > return chan->buf[0];
    >
    > - tmpname = kzalloc(NAME_MAX + 1, GFP_KERNEL);
    > - if (!tmpname)
    > - goto end;
    > - snprintf(tmpname, NAME_MAX, "%s%d", chan->base_filename, cpu);
    > -
    > buf = relay_create_buf(chan);
    > if (!buf)
    > - goto free_name;
    > + goto end;
    > +
    > + if (chan->has_base_filename)
    > + if (relay_setup_buf_file(chan, buf, cpu)) goto free_buf;
    >
    > buf->cpu = cpu;
    > __relay_reset(buf, 1);
    >
    > - /* Create file in fs */
    > - dentry = chan->cb->create_buf_file(tmpname, chan->parent, S_IRUSR,
    > - buf, &chan->is_global);
    > - if (!dentry)
    > - goto free_buf;
    > -
    > - buf->dentry = dentry;
    > -
    > if(chan->is_global) {
    > chan->buf[0] = buf;
    > buf->cpu = 0;
    > }
    >
    > - goto free_name;
    > + goto end;
    >
    > free_buf:
    > relay_destroy_buf(buf);
    > buf = NULL;
    > -free_name:
    > - kfree(tmpname);
    > end:
    > return buf;
    > }
    > @@ -508,8 +523,8 @@ static int __cpuinit relay_hotcpu_callback(struct notifier_block *nb,
    >
    > /**
    > * relay_open - create a new relay channel
    > - * @base_filename: base name of files to create
    > - * @parent: dentry of parent directory, %NULL for root directory
    > + * @base_filename: base name of files to create, %NULL for buffering only
    > + * @parent: dentry of parent directory, %NULL for root directory or buffer
    > * @subbuf_size: size of sub-buffers
    > * @n_subbufs: number of sub-buffers
    > * @cb: client callback functions
    > @@ -531,8 +546,6 @@ struct rchan *relay_open(const char *base_filename,
    > {
    > unsigned int i;
    > struct rchan *chan;
    > - if (!base_filename)
    > - return NULL;
    >
    > if (!(subbuf_size && n_subbufs))
    > return NULL;
    > @@ -547,12 +560,17 @@ struct rchan *relay_open(const char *base_filename,
    > chan->alloc_size = FIX_SIZE(subbuf_size * n_subbufs);
    > chan->parent = parent;
    > chan->private_data = private_data;
    > - strlcpy(chan->base_filename, base_filename, NAME_MAX);
    > + if (base_filename) {
    > + chan->has_base_filename = 1;
    > + strlcpy(chan->base_filename, base_filename, NAME_MAX);
    > + } else {
    > + chan->has_base_filename = 0;
    > + }
    > setup_callbacks(chan, cb);
    > kref_init(&chan->kref);
    >
    > mutex_lock(&relay_channels_mutex);
    > - for_each_online_cpu(i) {
    > + for_each_present_cpu(i) {
    > chan->buf[i] = relay_open_buf(chan, i);
    > if (!chan->buf[i])
    > goto free_bufs;
    > @@ -563,7 +581,7 @@ struct rchan *relay_open(const char *base_filename,
    > return chan;
    >
    > free_bufs:
    > - for_each_online_cpu(i) {
    > + for_each_present_cpu(i) {
    > if (!chan->buf[i])
    > break;
    > relay_close_buf(chan->buf[i]);
    > @@ -576,6 +594,38 @@ free_bufs:
    > EXPORT_SYMBOL_GPL(relay_open);
    >
    > /**
    > + * relay_late_setup_files - triggers file creation
    > + * @chan: channel to operate on
    > + * @base_filename: base name of files to create
    > + * @parent: dentry of parent directory, %NULL for root directory
    > + *
    > + * Returns 0 is successful, non-zero otherwise.
    > + *
    > + * Use to setup files for a previously buffer-only channel.
    > + * Useful to do early tracing in kernel, before VFS is up, for example.
    > + */
    > +int relay_late_setup_files(struct rchan *chan,
    > + const char *base_filename,
    > + struct dentry *parent)
    > +{
    > + unsigned int i;
    > +
    > + if (!chan || !base_filename)
    > + return 1;
    > +
    > + strlcpy(chan->base_filename, base_filename, NAME_MAX);
    > + chan->has_base_filename = 1;
    > + chan->parent = parent;
    > +
    > + mutex_lock(&relay_channels_mutex);
    > + for_each_present_cpu(i)
    > + relay_setup_buf_file(chan, chan->buf[i], i);
    > + mutex_unlock(&relay_channels_mutex);
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > * relay_switch_subbuf - switch to a new sub-buffer
    > * @buf: channel buffer
    > * @length: size of current event
    > --
    > 1.5.2.5
    >


    --
    Mathieu Desnoyers
    Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

    On Fri, 4 Apr 2008 18:33:03 +0300 Eduard - Gabriel Munteanu wrote:

    > relay_open() can now handle NULL base_filename, to register a
    > buffer-only channel. Using a new function, relay_late_setup_files(), one
    > can assign files after creating the channel, thus allowing for doing
    > early tracing in the kernel, before VFS is up.
    >
    > Signed-off-by: Eduard - Gabriel Munteanu
    > ---
    > Documentation/filesystems/relay.txt | 11 ++++
    > include/linux/relay.h | 4 ++
    > kernel/relay.c | 102 ++++++++++++++++++++++++++---------
    > 3 files changed, 91 insertions(+), 26 deletions(-)
    >
    > diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
    > index 094f2d2..b59c228 100644
    > --- a/Documentation/filesystems/relay.txt
    > +++ b/Documentation/filesystems/relay.txt
    > @@ -161,6 +161,7 @@ TBD(curr. line MT:/API/)
    > relay_close(chan)
    > relay_flush(chan)
    > relay_reset(chan)
    > + relay_late_setup_files(chan, base_filename, parent)
    >
    > channel management typically called on instigation of userspace:
    >
    > @@ -294,6 +295,16 @@ user-defined data with a channel, and is immediately available
    > (including in create_buf_file()) via chan->private_data or
    > buf->chan->private_data.
    >
    > +Buffer-only channels
    > +--------------------
    > +
    > +These channels have no files associated and can be created with
    > +relay_open(NULL, NULL, ...). Such channels are useful in scenarios such
    > +as when doing early tracing in the kernel, before the VFS is up. In these
    > +cases, one may open a channel a buffer-only channel and then call

    ~~~~~~~~~
    d/a channel/

    > +relay_late_setup_files() when the kernel is ready to handle files, to expose
    > +the buffered data to the userspace.
    > +
    > Channel 'modes'
    > ---------------
    >
    > diff --git a/kernel/relay.c b/kernel/relay.c
    > index 4c035a8..4bea94a 100644
    > --- a/kernel/relay.c
    > +++ b/kernel/relay.c
    > @@ -378,6 +378,35 @@ void relay_reset(struct rchan *chan)
    > }
    > EXPORT_SYMBOL_GPL(relay_reset);
    >
    > +static int relay_setup_buf_file(struct rchan *chan,
    > + struct rchan_buf *buf,
    > + unsigned int cpu)
    > +{
    > + struct dentry *dentry;
    > + char *tmpname;
    > +
    > + tmpname = kzalloc(NAME_MAX + 1, GFP_KERNEL);
    > + if (!tmpname)
    > + goto failed;
    > + snprintf(tmpname, NAME_MAX, "%s%d", chan->base_filename, cpu);
    > +
    > + /* Create file in fs */
    > + dentry = chan->cb->create_buf_file(tmpname, chan->parent,
    > + S_IRUSR, buf,
    > + &chan->is_global);
    > +
    > + if (!dentry)
    > + goto failed;


    above leaks tmpname ?

    > + buf->dentry = dentry;
    > +
    > + kfree(tmpname);
    > +
    > + return 0;
    > +
    > +failed:
    > + return 1;
    > +}
    > +
    > /*
    > * relay_open_buf - create a new relay channel buffer
    > *
    > @@ -386,44 +415,30 @@ EXPORT_SYMBOL_GPL(relay_reset);
    > static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
    > {
    > struct rchan_buf *buf = NULL;
    > - struct dentry *dentry;
    > - char *tmpname;
    >
    > if (chan->is_global)
    > return chan->buf[0];
    >
    > - tmpname = kzalloc(NAME_MAX + 1, GFP_KERNEL);
    > - if (!tmpname)
    > - goto end;
    > - snprintf(tmpname, NAME_MAX, "%s%d", chan->base_filename, cpu);
    > -
    > buf = relay_create_buf(chan);
    > if (!buf)
    > - goto free_name;
    > + goto end;
    > +
    > + if (chan->has_base_filename)
    > + if (relay_setup_buf_file(chan, buf, cpu)) goto free_buf;


    Put 'goto free_buf;' on separate line.

    >
    > buf->cpu = cpu;
    > __relay_reset(buf, 1);
    >
    > - /* Create file in fs */
    > - dentry = chan->cb->create_buf_file(tmpname, chan->parent, S_IRUSR,
    > - buf, &chan->is_global);
    > - if (!dentry)
    > - goto free_buf;
    > -
    > - buf->dentry = dentry;
    > -
    > if(chan->is_global) {
    > chan->buf[0] = buf;
    > buf->cpu = 0;
    > }
    >
    > - goto free_name;
    > + goto end;
    >
    > free_buf:
    > relay_destroy_buf(buf);
    > buf = NULL;
    > -free_name:
    > - kfree(tmpname);
    > end:
    > return buf;
    > }
    > @@ -576,6 +594,38 @@ free_bufs:
    > EXPORT_SYMBOL_GPL(relay_open);
    >
    > /**
    > + * relay_late_setup_files - triggers file creation
    > + * @chan: channel to operate on
    > + * @base_filename: base name of files to create
    > + * @parent: dentry of parent directory, %NULL for root directory
    > + *
    > + * Returns 0 is successful, non-zero otherwise.


    s/is/if/

    > + *
    > + * Use to setup files for a previously buffer-only channel.
    > + * Useful to do early tracing in kernel, before VFS is up, for example.
    > + */
    > +int relay_late_setup_files(struct rchan *chan,
    > + const char *base_filename,
    > + struct dentry *parent)
    > +{
    > + unsigned int i;
    > +
    > + if (!chan || !base_filename)
    > + return 1;
    > +
    > + strlcpy(chan->base_filename, base_filename, NAME_MAX);
    > + chan->has_base_filename = 1;
    > + chan->parent = parent;
    > +
    > + mutex_lock(&relay_channels_mutex);
    > + for_each_present_cpu(i)
    > + relay_setup_buf_file(chan, chan->buf[i], i);
    > + mutex_unlock(&relay_channels_mutex);
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > * relay_switch_subbuf - switch to a new sub-buffer
    > * @buf: channel buffer
    > * @length: size of current event


    ---
    ~Randy
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

    On Fri, 4 Apr 2008 12:06:23 -0400
    Mathieu Desnoyers wrote:

    > I like this But could we trace event sooner if we did not depend on
    > vmalloc to allocate the buffers ?
    >
    > I am thinking of receiving a kernel parameter that would reserve a
    > buffer in the linear kernel mapping (kmalloc ?), or to compile-in a
    > static buffer, which could be used by relay instead of the vmalloc'd
    > buffer. That would permit to trace the memory allocator and some other
    > very early stuff. Basically, relay would have it's "memory pool" to
    > manage and would do its allocations from this fixed buffer. No free
    > would be required.
    >
    > Mathieu


    Sure. I'm doing this as part of my GSoC application (well, I haven't
    been accepted yet). I had also proposed this to Pekka Enberg, my
    mentor, but he was reluctant to hack the relay code more invasively. Now
    that you agree with this, I'll work on it.

    kmalloc() doesn't work before kmem_cache_init() and relay code uses
    kmalloc(), not vmalloc(), IIRC.

    I would go another way. Have the relay client call __get_free_pages()
    or similar function, allocate the buffer and pass it on to a variant of
    relay_open(), let's name it relay_early_open(). __get_free_pages() can
    be used before kmem_cache_init() as far as I know. No free is required,
    since early tracing code implies it's built into the kernel and not as
    a module. What do you say?

    Will you merge this or wait until I finish the very early tracing stuff?

    Cheers,
    Eduard
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

    Hi,

    Thanks for spotting these problems, Randy. I'll fix them and resubmit.


    Cheers,
    Eduard
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

    * Eduard - Gabriel Munteanu (eduard.munteanu@linux360.ro) wrote:
    > On Fri, 4 Apr 2008 12:06:23 -0400
    > Mathieu Desnoyers wrote:
    >
    > > I like this But could we trace event sooner if we did not depend on
    > > vmalloc to allocate the buffers ?
    > >
    > > I am thinking of receiving a kernel parameter that would reserve a
    > > buffer in the linear kernel mapping (kmalloc ?), or to compile-in a
    > > static buffer, which could be used by relay instead of the vmalloc'd
    > > buffer. That would permit to trace the memory allocator and some other
    > > very early stuff. Basically, relay would have it's "memory pool" to
    > > manage and would do its allocations from this fixed buffer. No free
    > > would be required.
    > >
    > > Mathieu

    >
    > Sure. I'm doing this as part of my GSoC application (well, I haven't
    > been accepted yet). I had also proposed this to Pekka Enberg, my
    > mentor, but he was reluctant to hack the relay code more invasively. Now
    > that you agree with this, I'll work on it.
    >
    > kmalloc() doesn't work before kmem_cache_init() and relay code uses
    > kmalloc(), not vmalloc(), IIRC.
    >


    It currently uses both :

    It uses alloc_page and vmap to allocate the buffers and it uses
    kzalloc/kmalloc to allocate the relay structures.

    > I would go another way. Have the relay client call __get_free_pages()
    > or similar function, allocate the buffer and pass it on to a variant of
    > relay_open(), let's name it relay_early_open(). __get_free_pages() can
    > be used before kmem_cache_init() as far as I know. No free is required,
    > since early tracing code implies it's built into the kernel and not as
    > a module. What do you say?
    >


    Then I guess we would depend on page_alloc_init(). It's not that bad,
    but I guess the question is : do we prefer to let users specify the
    buffer size to reserve for tracing on the kernel command line (that
    would require __get_free_pages()) or do we compile-in a static buffer
    size, which can be specified in the kernel configuration. I think
    providing both could be an option : flexibility _and_ *very* early
    tracing.

    So making this relay interface flexible enough to receive a buffer
    either reserved by __get_free_pages() or a static buffer seems like a
    good compromise. It would also have to receive the buffer size from the
    caller.

    > Will you merge this or wait until I finish the very early tracing stuff?
    >


    As far as I am concerned, I can only merge this in the LTTng project to
    try it.*I've added Tom Zanussi, the author of relay, who might have some
    ideas to share with us. I would wait until we have a more precise idea
    of the API before I modify LTTng to start using it.

    Regards,

    Mathieu

    > Cheers,
    > Eduard
    >


    --
    Mathieu Desnoyers
    Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

    On Fri, 4 Apr 2008 12:40:22 -0400
    Mathieu Desnoyers wrote:

    > Then I guess we would depend on page_alloc_init(). It's not that bad,
    > but I guess the question is : do we prefer to let users specify the
    > buffer size to reserve for tracing on the kernel command line (that
    > would require __get_free_pages()) or do we compile-in a static buffer
    > size, which can be specified in the kernel configuration. I think
    > providing both could be an option : flexibility _and_ *very* early
    > tracing.


    I wasn't referring to the kernel's users specifying the buffer on the
    command line. For example, LTTng code (I'm not familiar with it) could
    allocate a buffer, by itself or using a helper function, and pass it to
    relay_early_open(). Of course, it could read the command line and call
    that helper function, passing the size required by the user as an
    argument.

    Basically, I'm saying that having relay code manage a large buffer,
    allocating memory from it for each of its callers, could be very
    complex. It would be easier and less complex to let each tracing
    subsystem allocate memory on its own, either through a static buffer or
    with __get_free_pages().

    > So making this relay interface flexible enough to receive a buffer
    > either reserved by __get_free_pages() or a static buffer seems like a
    > good compromise. It would also have to receive the buffer size from
    > the caller.


    Yes, it shouldn't matter to the relay interface whether the buffer was
    statically or dynamically allocated. All it needs to know is where it
    starts and how long it is.

    BTW, I'll resubmit a modified patch soon, as suggested by Randy Dunlap.


    Eduard
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH] relay: add buffer-only functionality, allowing for early kernel tracing

    On Fri, 4 Apr 2008 12:40:22 -0400
    Mathieu Desnoyers wrote:

    > As far as I am concerned, I can only merge this in the LTTng project
    > to try it.*I've added Tom Zanussi, the author of relay, who might
    > have some ideas to share with us. I would wait until we have a more
    > precise idea of the API before I modify LTTng to start using it.
    >
    > Regards,
    >
    > Mathieu


    Hi,

    I've tested my patch a bit more and ran into a problem, and fixed it.
    Forget the old patch, it will panic the kernel if it has to cross
    sub-buffer boundaries. I'll polish the new one a bit (maybe update the
    docs) and resubmit, along with a piece of code from my GSoC project that
    uses the early relay interface (may provide an easy to try demo).

    Unfortunately, as for doing earlier than kmem_cache_init() tracing, I
    don't have the time right now, as I have to carry on with my GSoC
    project. I looked through the source more thoroughly and the needed
    changes are not trivial at all. I'm not saying I won't work on it, but
    it won't be a priority.

    By the way, I got a "delivery failure" message... is something wrong
    with Tom Zanussi's e-mail address? Google showed 2007 LKML entries with
    his e-mail address as . In any case, when
    I'll resubmit the patch, I'll add this other address in the Cc list. Or
    should we reach him another way?

    I'd like to get this merged as it is so far, and mailing it directly to
    akpm (or worse, Linus) doesn't sound like a good idea.


    Cheers,
    Eduard
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread