Re: [PATCH] relayfs redux, part 2
Roman Zippel writes:
> Hi,
>
> On Fri, 28 Jan 2005, Tom Zanussi wrote:
>
> > +static inline int rchan_create_file(const char *chanpath,
> > + struct dentry **dentry,
> > + struct rchan_buf *data)
> > +{
> > + int err;
> > + const char * fname;
> > + struct dentry *topdir;
> > +
> > + err = rchan_create_dir(chanpath, , );
> > + if (err && (err != -EEXIST))
> > + return err;
> > +
> > + err = relayfs_create_file(fname, topdir, dentry, data, S_IRUSR);
> > +
> > + return err;
> > +}
>
> What protects topdir from being removed inbetween?
> Why is necessary to let the user create/remove files/dirs at all?
Good point - file/dir creation/removal should only be done by
relay_open/close. I'll make sure to fix both of these things.
[...]
>
> For the first version I would suggest to use just local_irq_save/_restore.
> Getting it right with local_add_return is not trivial and I'm pretty sure
> your relay_switch_buffer() gets it wrong, e.g. the caller for whom (offset
> < bufsize) must close the subbuffer. Also buffer->data in relay_reserve
> may have become invalid (e.g. by an interrupt just before it).
>
Yes, there are a couple of bugs here, but I think they're easily fixed
- thanks for spotting them.
For the buffer->data problem, the value of buffer->data at the time
offset is reserved would have to be saved along with it for it to make
sense.
For relay_switch_buffer(), only the offset of the first event that
didn't fit would be less than bufsize so if there was a check for that
i.e. if offset < bufsize, calculate padding, otherwise don't and just
continue, the interrupt would continue through and actually accomplish
the buffer switch, while the interrupted event would calculate the
padding and then exit immediately because of the (offset + length <
bufsize) recheck. Both would then loop around again to retry and
succeed. Well, I'm not sure that explanation is clear, but it seems
like it should work with that small change.
Please let me know if you disagree or see any other problems.
> You can also move all the rchan_buf members which are not written to in
> the event path and which are common to all channels back to rchan.
Yes, I was actually planning on doing that, but hadn't gotten to it
yet.
Thanks,
Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Roman Zippel writes:
Hi,
On Fri, 28 Jan 2005, Tom Zanussi wrote:
+static inline int rchan_create_file(const char *chanpath,
+ struct dentry **dentry,
+ struct rchan_buf *data)
+{
+ int err;
+ const char * fname;
+ struct dentry *topdir;
+
+ err = rchan_create_dir(chanpath, fname, topdir);
+ if (err (err != -EEXIST))
+ return err;
+
+ err = relayfs_create_file(fname, topdir, dentry, data, S_IRUSR);
+
+ return err;
+}
What protects topdir from being removed inbetween?
Why is necessary to let the user create/remove files/dirs at all?
Good point - file/dir creation/removal should only be done by
relay_open/close. I'll make sure to fix both of these things.
[...]
For the first version I would suggest to use just local_irq_save/_restore.
Getting it right with local_add_return is not trivial and I'm pretty sure
your relay_switch_buffer() gets it wrong, e.g. the caller for whom (offset
bufsize) must close the subbuffer. Also buffer-data in relay_reserve
may have become invalid (e.g. by an interrupt just before it).
Yes, there are a couple of bugs here, but I think they're easily fixed
- thanks for spotting them.
For the buffer-data problem, the value of buffer-data at the time
offset is reserved would have to be saved along with it for it to make
sense.
For relay_switch_buffer(), only the offset of the first event that
didn't fit would be less than bufsize so if there was a check for that
i.e. if offset bufsize, calculate padding, otherwise don't and just
continue, the interrupt would continue through and actually accomplish
the buffer switch, while the interrupted event would calculate the
padding and then exit immediately because of the (offset + length
bufsize) recheck. Both would then loop around again to retry and
succeed. Well, I'm not sure that explanation is clear, but it seems
like it should work with that small change.
Please let me know if you disagree or see any other problems.
You can also move all the rchan_buf members which are not written to in
the event path and which are common to all channels back to rchan.
Yes, I was actually planning on doing that, but hadn't gotten to it
yet.
Thanks,
Tom
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Hi,
On Fri, 28 Jan 2005, Tom Zanussi wrote:
> +static inline int rchan_create_file(const char *chanpath,
> + struct dentry **dentry,
> + struct rchan_buf *data)
> +{
> + int err;
> + const char * fname;
> + struct dentry *topdir;
> +
> + err = rchan_create_dir(chanpath, , );
> + if (err && (err != -EEXIST))
> + return err;
> +
> + err = relayfs_create_file(fname, topdir, dentry, data, S_IRUSR);
> +
> + return err;
> +}
What protects topdir from being removed inbetween?
Why is necessary to let the user create/remove files/dirs at all?
> +void *relay_reserve(struct rchan *chan,
> + unsigned length,
> + int cpu)
> +{
> + unsigned offset;
> + struct rchan_buf *buffer;
> +
> + buffer = relay_get_buffer(chan, cpu);
> +
> + while(1) {
> + offset = local_add_return(>offset, length);
> + if (likely(offset + length <= buffer->bufsize))
> + break;
> + buffer = relay_switch_buffer(buffer, offset, length);
> + if (buffer == NULL)
> + return NULL;
> + }
> +
> + return buffer->data + offset;
> +}
> +
> [..]
> +
> +unsigned relay_write(struct rchan *chan,
> + const void *data,
> + unsigned length)
> +{
> + int cpu;
> + char *reserved;
> + unsigned count = 0;
> +
> + cpu = get_cpu();
> +
> + reserved = relay_reserve(chan, length, cpu);
> + if(reserved) {
> + memcpy(reserved, data, length);
> + count = length;
> + }
> +
> + put_cpu();
> +
> + return count;
> +}
For the first version I would suggest to use just local_irq_save/_restore.
Getting it right with local_add_return is not trivial and I'm pretty sure
your relay_switch_buffer() gets it wrong, e.g. the caller for whom (offset
< bufsize) must close the subbuffer. Also buffer->data in relay_reserve
may have become invalid (e.g. by an interrupt just before it).
You can also move all the rchan_buf members which are not written to in
the event path and which are common to all channels back to rchan.
relay_write should so look more like this:
unsigned int relay_write(struct rchan *chan, const void *data,
unsigned int length)
{
struct rchan_buf *buffer;
unsigned long flags;
local_irq_save(flags);
buffer = chan->buff[smp_processor_id()];
if (unlikely(buffer->offset + length > chan->size)) {
if (relay_switch_buffer(chan, buffer)) {
length = 0;
goto out;
}
}
memcpy(buffer->data + offset, data, length);
buffer->offset += length;
out:
local_irq_restore(flags);
return length;
}
relay_reserve() should be more or less obvious from this.
bye, Roman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Greg KH wrote: > When relayfs is built into the kernel, those symbols are then global to > the whole static kernel. > > Please be nice and rename them. My pleasure :) Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
On Mon, Jan 31, 2005 at 05:10:27PM -0500, Karim Yaghmour wrote: > > Greg KH wrote: > > On Fri, Jan 28, 2005 at 01:38:22PM -0600, Tom Zanussi wrote: > > > >>+extern void * alloc_rchan_buf(unsigned long size, > >>+ struct page ***page_array, > >>+ int *page_count); > >>+extern void free_rchan_buf(void *buf, > >>+ struct page **page_array, > >>+ int page_count); > > > > > > As these will be "polluting" the global namespace of the kernel, could > > you add "relayfs_" to the front of them? > > BTW, these functions are in buffers.h which is an internal header to > fs/relayfs/*.c files. buffers.h is not included in anything outside. > Correct me if I'm wrong, but there is no namespace pollution in that > case, right? All that does contribute to namespace pollution is in > include/linux/relayfs_fs.h. When relayfs is built into the kernel, those symbols are then global to the whole static kernel. Please be nice and rename them. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Greg KH wrote: > On Fri, Jan 28, 2005 at 01:38:22PM -0600, Tom Zanussi wrote: > >>+extern void * alloc_rchan_buf(unsigned long size, >>+ struct page ***page_array, >>+ int *page_count); >>+extern void free_rchan_buf(void *buf, >>+struct page **page_array, >>+int page_count); > > > As these will be "polluting" the global namespace of the kernel, could > you add "relayfs_" to the front of them? BTW, these functions are in buffers.h which is an internal header to fs/relayfs/*.c files. buffers.h is not included in anything outside. Correct me if I'm wrong, but there is no namespace pollution in that case, right? All that does contribute to namespace pollution is in include/linux/relayfs_fs.h. Thanks, Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi wrote: > I don't think they need to be mutually exclusive - we could keep > relay_reserve(), but the relay_write() that's currently built on top > of relay_reserve() would use the putc code instead. It's complicating > the API a bit, but if it makes everyone happy... Actually I think that this would be a much better use of relay_write(), which is unlikely to be used by any client that requires relay_reserve() to start with. Also, I don't think it complicates the API at all. Compared to the original API, what we've got now is very simple. So it basically boils down to: - use relay_write() if you want putc-like functionality. - use relay_reserve() if you want to reserve space and write separately. This is even better than having a separate ad-hoc mode. Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Karim Yaghmour writes: > > Tom Zanussi wrote: > > OK, makes sense to me - I'll get rid of relay_reserve and replace it > > with the simple putc write and variant. > > Please don't do that. Instead, bring back the ad-hoc mode code, that's > what is was for anyway. > I don't think they need to be mutually exclusive - we could keep relay_reserve(), but the relay_write() that's currently built on top of relay_reserve() would use the putc code instead. It's complicating the API a bit, but if it makes everyone happy... Tom - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi wrote: > OK, makes sense to me - I'll get rid of relay_reserve and replace it > with the simple putc write and variant. Please don't do that. Instead, bring back the ad-hoc mode code, that's what is was for anyway. > You could just create and log into a separate relayfs channel, if you > wanted to. Not sure we need to add anything special to support that. Postprocessing doesn't solve world famine ;) As far as LTT goes, splitting events like this makes it impossible to read large traces. Other clients are free to do as they wish. Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Andi Kleen wrote: > It's doing a complicated function call which does who knows what in > the logging fast path (I stopped reading after some point) > It definitely is not putc ! I was anticipating some people would have this requirement, and this is why I introduced the ad-hoc mode. Roman asked me to get rid of it because nobody had yet asked for it, so this is why it was dropped. As it is, the implementation you are suggesting is insufficient for LTT, which is relayfs' first formal client. I think that it would be better to provide two underlying mechanisms for relayfs at this point, we had already stripped it as thin as it would go for things like LTT. >>separate grabbing a slot in the buffer from the memcpy because some >>applications such as ltt want to be able to directly write into the >>slot without having to copy it into another buffer first. How about > > > If the inline function to log was fast enough it wouldn't need > any such hacks. Actually that's not true. There are two problems with this statement: a- It requires prepackaged data units. b- It's only useful for fixed-size data units. Any efficient client that has complex data units will want to write directly into the buffer instead of creating an intermediate package which is then memcopied. With the modified code, we are now forced to create an intermediate package, which is wrong. Also, if the client wants to log variable-size events, he would have to re-implement lots of the writing code. Note that I really think relay_write() should be dropped altogether. Clients should call on relay_reserve() and do whatever is necessary after that. > Note that gcc is quite good at optimizing memcpy, so essentially > when you e.g. do log(singleint) it should be roughly equivalent > to a int store into the buffer + the check if there is enough > buffer space. I understand the point you are trying to make, but I really think that this is best implemented as two separate buffering schemes instead of breaking the existing one (which had already been trimmed down quite thin following Roman's input.) > You could avoid the local_irq_save() if you use separate interrupt > buffers that are only accessed in non nesting interrupt context > (like softirqs) That would require a sorting step at output though. Not > sure if it's worth it. The problem is that hardirqs can nest anyways, > so it wouldn't work for them. However a lot of important code runs > in softirq (like the network stack) where this is true. For the kind of data sizes we are looking at for LTT (100GBs) splitting buffers is not viable anyway. Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Andi Kleen writes:
> On Sat, Jan 29, 2005 at 10:58:22PM -0600, Tom Zanussi wrote:
> > > The logging fast path seems still a bit slow to me. I would like
> > > to have a logging macro that is not much worse than a stdio putc,
> > > basically something like
> > >
> > > get_cpu();
> > > if (buffer space > N) {
> > > memcpy(buffer, input, N);
> > > buffer pointer += N;
> > > } else {
> > > FreeBuffer(input, N);
> > > }
> > > put_cpu();
> > >
> >
> > I think what we have now is somewhat similar, except that we wanted to
>
> It's doing a complicated function call which does who knows what in
> the logging fast path (I stopped reading after some point)
> It definitely is not putc !
>
I agree with your suggestions below, but I just wanted to point out
that the only function called in the fast path is local_add_return(),
which is just a few lines - the more complicated stuff in
switch_buffer() is only called when crossing buffer boundaries.
> > separate grabbing a slot in the buffer from the memcpy because some
> > applications such as ltt want to be able to directly write into the
> > slot without having to copy it into another buffer first. How about
>
> If the inline function to log was fast enough it wouldn't need
> any such hacks.
>
> Note that gcc is quite good at optimizing memcpy, so essentially
> when you e.g. do log(singleint) it should be roughly equivalent
> to a int store into the buffer + the check if there is enough
> buffer space.
>
> > something like this for relay reserve, with the local_add_return()
> > gone since we're assuming the client protects the buffer properly for
> > whatever it's doing:
>
> I think relay_reserve shouldn't be in the fast path at all.
>
> The simple write should simple be the traditional stdio putc pattern
>
> if (buffer + datalen < bufferend) {
> memcpy(buffer, data, datalen);
> buffer += datalen;
> } else {
> flush(buffer, datalen); /* flush takes care of the slow path */
> }
>
> This is quite fast for the fast path and expands to reasonably compact
> inline code too.
>
> The only interesting part is how to protect this against interrupts.
> For that you need an local_irq_save(); local_irq_restore() which
> can be unfortunately quite costly (P4 is really slow at PUSHF)
>
> That is why I would provide a __ variant of the simple
> where the caller guarantees no disruption by interrupts.
>
> On preemptive kernel the local_irq_save takes care of CPU
> preemption, the __ variant should probably disable preemption
> to avoid mistakes. For non __ it is not needed.
OK, makes sense to me - I'll get rid of relay_reserve and replace it
with the simple putc write and variant.
>
> > > This would need interrupt protection only if interrupts can access
> > > it, best you use separate buffers for that too.
> >
> > Not sure what you mean by separate buffers for that too. Can you
> > expand on that a little?
>
> You could avoid the local_irq_save() if you use separate interrupt
> buffers that are only accessed in non nesting interrupt context
> (like softirqs) That would require a sorting step at output though. Not
> sure if it's worth it. The problem is that hardirqs can nest anyways,
> so it wouldn't work for them. However a lot of important code runs
> in softirq (like the network stack) where this is true.
You could just create and log into a separate relayfs channel, if you
wanted to. Not sure we need to add anything special to support that.
Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
On Sat, Jan 29, 2005 at 10:58:22PM -0600, Tom Zanussi wrote:
> > The logging fast path seems still a bit slow to me. I would like
> > to have a logging macro that is not much worse than a stdio putc,
> > basically something like
> >
> > get_cpu();
> > if (buffer space > N) {
> > memcpy(buffer, input, N);
> > buffer pointer += N;
> > } else {
> > FreeBuffer(input, N);
> > }
> > put_cpu();
> >
>
> I think what we have now is somewhat similar, except that we wanted to
It's doing a complicated function call which does who knows what in
the logging fast path (I stopped reading after some point)
It definitely is not putc !
> separate grabbing a slot in the buffer from the memcpy because some
> applications such as ltt want to be able to directly write into the
> slot without having to copy it into another buffer first. How about
If the inline function to log was fast enough it wouldn't need
any such hacks.
Note that gcc is quite good at optimizing memcpy, so essentially
when you e.g. do log(singleint) it should be roughly equivalent
to a int store into the buffer + the check if there is enough
buffer space.
> something like this for relay reserve, with the local_add_return()
> gone since we're assuming the client protects the buffer properly for
> whatever it's doing:
I think relay_reserve shouldn't be in the fast path at all.
The simple write should simple be the traditional stdio putc pattern
if (buffer + datalen < bufferend) {
memcpy(buffer, data, datalen);
buffer += datalen;
} else {
flush(buffer, datalen); /* flush takes care of the slow path */
}
This is quite fast for the fast path and expands to reasonably compact
inline code too.
The only interesting part is how to protect this against interrupts.
For that you need an local_irq_save(); local_irq_restore() which
can be unfortunately quite costly (P4 is really slow at PUSHF)
That is why I would provide a __ variant of the simple
where the caller guarantees no disruption by interrupts.
On preemptive kernel the local_irq_save takes care of CPU
preemption, the __ variant should probably disable preemption
to avoid mistakes. For non __ it is not needed.
> > This would need interrupt protection only if interrupts can access
> > it, best you use separate buffers for that too.
>
> Not sure what you mean by separate buffers for that too. Can you
> expand on that a little?
You could avoid the local_irq_save() if you use separate interrupt
buffers that are only accessed in non nesting interrupt context
(like softirqs) That would require a sorting step at output though. Not
sure if it's worth it. The problem is that hardirqs can nest anyways,
so it wouldn't work for them. However a lot of important code runs
in softirq (like the network stack) where this is true.
-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Andi Kleen wrote: It's doing a complicated function call which does who knows what in the logging fast path (I stopped reading after some point) It definitely is not putc ! I was anticipating some people would have this requirement, and this is why I introduced the ad-hoc mode. Roman asked me to get rid of it because nobody had yet asked for it, so this is why it was dropped. As it is, the implementation you are suggesting is insufficient for LTT, which is relayfs' first formal client. I think that it would be better to provide two underlying mechanisms for relayfs at this point, we had already stripped it as thin as it would go for things like LTT. separate grabbing a slot in the buffer from the memcpy because some applications such as ltt want to be able to directly write into the slot without having to copy it into another buffer first. How about If the inline function to log was fast enough it wouldn't need any such hacks. Actually that's not true. There are two problems with this statement: a- It requires prepackaged data units. b- It's only useful for fixed-size data units. Any efficient client that has complex data units will want to write directly into the buffer instead of creating an intermediate package which is then memcopied. With the modified code, we are now forced to create an intermediate package, which is wrong. Also, if the client wants to log variable-size events, he would have to re-implement lots of the writing code. Note that I really think relay_write() should be dropped altogether. Clients should call on relay_reserve() and do whatever is necessary after that. Note that gcc is quite good at optimizing memcpy, so essentially when you e.g. do log(singleint) it should be roughly equivalent to a int store into the buffer + the check if there is enough buffer space. I understand the point you are trying to make, but I really think that this is best implemented as two separate buffering schemes instead of breaking the existing one (which had already been trimmed down quite thin following Roman's input.) You could avoid the local_irq_save() if you use separate interrupt buffers that are only accessed in non nesting interrupt context (like softirqs) That would require a sorting step at output though. Not sure if it's worth it. The problem is that hardirqs can nest anyways, so it wouldn't work for them. However a lot of important code runs in softirq (like the network stack) where this is true. For the kind of data sizes we are looking at for LTT (100GBs) splitting buffers is not viable anyway. Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi wrote: OK, makes sense to me - I'll get rid of relay_reserve and replace it with the simple putc write and variant. Please don't do that. Instead, bring back the ad-hoc mode code, that's what is was for anyway. You could just create and log into a separate relayfs channel, if you wanted to. Not sure we need to add anything special to support that. Postprocessing doesn't solve world famine ;) As far as LTT goes, splitting events like this makes it impossible to read large traces. Other clients are free to do as they wish. Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Karim Yaghmour writes: Tom Zanussi wrote: OK, makes sense to me - I'll get rid of relay_reserve and replace it with the simple putc write and variant. Please don't do that. Instead, bring back the ad-hoc mode code, that's what is was for anyway. I don't think they need to be mutually exclusive - we could keep relay_reserve(), but the relay_write() that's currently built on top of relay_reserve() would use the putc code instead. It's complicating the API a bit, but if it makes everyone happy... Tom - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi wrote: I don't think they need to be mutually exclusive - we could keep relay_reserve(), but the relay_write() that's currently built on top of relay_reserve() would use the putc code instead. It's complicating the API a bit, but if it makes everyone happy... Actually I think that this would be a much better use of relay_write(), which is unlikely to be used by any client that requires relay_reserve() to start with. Also, I don't think it complicates the API at all. Compared to the original API, what we've got now is very simple. So it basically boils down to: - use relay_write() if you want putc-like functionality. - use relay_reserve() if you want to reserve space and write separately. This is even better than having a separate ad-hoc mode. Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Greg KH wrote: On Fri, Jan 28, 2005 at 01:38:22PM -0600, Tom Zanussi wrote: +extern void * alloc_rchan_buf(unsigned long size, + struct page ***page_array, + int *page_count); +extern void free_rchan_buf(void *buf, +struct page **page_array, +int page_count); As these will be polluting the global namespace of the kernel, could you add relayfs_ to the front of them? BTW, these functions are in buffers.h which is an internal header to fs/relayfs/*.c files. buffers.h is not included in anything outside. Correct me if I'm wrong, but there is no namespace pollution in that case, right? All that does contribute to namespace pollution is in include/linux/relayfs_fs.h. Thanks, Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
On Mon, Jan 31, 2005 at 05:10:27PM -0500, Karim Yaghmour wrote: Greg KH wrote: On Fri, Jan 28, 2005 at 01:38:22PM -0600, Tom Zanussi wrote: +extern void * alloc_rchan_buf(unsigned long size, + struct page ***page_array, + int *page_count); +extern void free_rchan_buf(void *buf, + struct page **page_array, + int page_count); As these will be polluting the global namespace of the kernel, could you add relayfs_ to the front of them? BTW, these functions are in buffers.h which is an internal header to fs/relayfs/*.c files. buffers.h is not included in anything outside. Correct me if I'm wrong, but there is no namespace pollution in that case, right? All that does contribute to namespace pollution is in include/linux/relayfs_fs.h. When relayfs is built into the kernel, those symbols are then global to the whole static kernel. Please be nice and rename them. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Greg KH wrote: When relayfs is built into the kernel, those symbols are then global to the whole static kernel. Please be nice and rename them. My pleasure :) Karim -- Author, Speaker, Developer, Consultant Pushing Embedded and Real-Time Linux Systems Beyond the Limits http://www.opersys.com || [EMAIL PROTECTED] || 1-866-677-4546 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Hi,
On Fri, 28 Jan 2005, Tom Zanussi wrote:
+static inline int rchan_create_file(const char *chanpath,
+ struct dentry **dentry,
+ struct rchan_buf *data)
+{
+ int err;
+ const char * fname;
+ struct dentry *topdir;
+
+ err = rchan_create_dir(chanpath, fname, topdir);
+ if (err (err != -EEXIST))
+ return err;
+
+ err = relayfs_create_file(fname, topdir, dentry, data, S_IRUSR);
+
+ return err;
+}
What protects topdir from being removed inbetween?
Why is necessary to let the user create/remove files/dirs at all?
+void *relay_reserve(struct rchan *chan,
+ unsigned length,
+ int cpu)
+{
+ unsigned offset;
+ struct rchan_buf *buffer;
+
+ buffer = relay_get_buffer(chan, cpu);
+
+ while(1) {
+ offset = local_add_return(buffer-offset, length);
+ if (likely(offset + length = buffer-bufsize))
+ break;
+ buffer = relay_switch_buffer(buffer, offset, length);
+ if (buffer == NULL)
+ return NULL;
+ }
+
+ return buffer-data + offset;
+}
+
[..]
+
+unsigned relay_write(struct rchan *chan,
+ const void *data,
+ unsigned length)
+{
+ int cpu;
+ char *reserved;
+ unsigned count = 0;
+
+ cpu = get_cpu();
+
+ reserved = relay_reserve(chan, length, cpu);
+ if(reserved) {
+ memcpy(reserved, data, length);
+ count = length;
+ }
+
+ put_cpu();
+
+ return count;
+}
For the first version I would suggest to use just local_irq_save/_restore.
Getting it right with local_add_return is not trivial and I'm pretty sure
your relay_switch_buffer() gets it wrong, e.g. the caller for whom (offset
bufsize) must close the subbuffer. Also buffer-data in relay_reserve
may have become invalid (e.g. by an interrupt just before it).
You can also move all the rchan_buf members which are not written to in
the event path and which are common to all channels back to rchan.
relay_write should so look more like this:
unsigned int relay_write(struct rchan *chan, const void *data,
unsigned int length)
{
struct rchan_buf *buffer;
unsigned long flags;
local_irq_save(flags);
buffer = chan-buff[smp_processor_id()];
if (unlikely(buffer-offset + length chan-size)) {
if (relay_switch_buffer(chan, buffer)) {
length = 0;
goto out;
}
}
memcpy(buffer-data + offset, data, length);
buffer-offset += length;
out:
local_irq_restore(flags);
return length;
}
relay_reserve() should be more or less obvious from this.
bye, Roman
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Greg KH writes: > On Fri, Jan 28, 2005 at 01:38:22PM -0600, Tom Zanussi wrote: > > +extern void * alloc_rchan_buf(unsigned long size, > > +struct page ***page_array, > > +int *page_count); > > +extern void free_rchan_buf(void *buf, > > + struct page **page_array, > > + int page_count); > > As these will be "polluting" the global namespace of the kernel, could > you add "relayfs_" to the front of them? > > Also, any reason why you haven't exported the fops of relayfs so that > others can use this in their filesystems (like proc and debugfs)? > No reason - I just forgot. I'll make sure that gets fixed along with all the other things you pointed out here in the next patch. Thanks, Tom - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Andi Kleen writes:
> Tom Zanussi <[EMAIL PROTECTED]> writes:
>
> > Hi,
> >
> > This patch is the result of the latest round of liposuction on relayfs
> > - the patch size is now 44K, down from 110K and the 200K before that.
> > I'm posting it as a patch against 2.6.10 rather than -mm in order to
> > make it easier to review, but will create one for -mm once the changes
> > have settled down.
>
> The logging fast path seems still a bit slow to me. I would like
> to have a logging macro that is not much worse than a stdio putc,
> basically something like
>
> get_cpu();
> if (buffer space > N) {
> memcpy(buffer, input, N);
> buffer pointer += N;
> } else {
> FreeBuffer(input, N);
> }
> put_cpu();
>
I think what we have now is somewhat similar, except that we wanted to
separate grabbing a slot in the buffer from the memcpy because some
applications such as ltt want to be able to directly write into the
slot without having to copy it into another buffer first. How about
something like this for relay reserve, with the local_add_return()
gone since we're assuming the client protects the buffer properly for
whatever it's doing:
relay_reserve(length)
{
if (buffer->offset + length <= bufsize) {
reserved = buffer->data + buffer->offset;
buffer->offset += length;
} else { /* slow path */
reserved = switch_buffer(length);
if (reserved)
buffer->offset += length;
}
return reserved;
}
If you set up the channel to use MODE_CONTINUOUS, meaning continuously
cycle around the buffer, your logging function or macro would look
something like this:
simplest_write(data, length)
{
get_cpu();
reserved = relay_reserve();
memcpy(reserved, data, length);
put_cpu();
}
Clients who want to stop logging if the buffers are full would have an
extra test for when the reserve fails, which can never happen in
continuous mode:
/* uses MODE_NO_OVERWRITE */
ltt_write(data, length)
{
local_irq_save();
reserved = relay_reserve();
if (reserved) { /* if NULL, buffer full */
memcpy(reserved, item1, len1);
.
.
.
memcpy(reserved, itemN, lenN);
}
local_irq_restore();
}
> This would need interrupt protection only if interrupts can access
> it, best you use separate buffers for that too.
Not sure what you mean by separate buffers for that too. Can you
expand on that a little?
Thanks,
Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
On Fri, Jan 28, 2005 at 01:38:22PM -0600, Tom Zanussi wrote:
> +extern void * alloc_rchan_buf(unsigned long size,
> + struct page ***page_array,
> + int *page_count);
> +extern void free_rchan_buf(void *buf,
> +struct page **page_array,
> +int page_count);
As these will be "polluting" the global namespace of the kernel, could
you add "relayfs_" to the front of them?
Also, any reason why you haven't exported the fops of relayfs so that
others can use this in their filesystems (like proc and debugfs)?
> +/**
> + * rchan_create_file - create relay file, including parent directories
> + * @chanpath: path to file, including filename
> + * @dentry: result dentry
> + * @data: data to associate with the file
> + *
> + * Returns 0 if successful, negative otherwise.
> + */
> +static inline int rchan_create_file(const char *chanpath,
> + struct dentry **dentry,
> + struct rchan_buf *data)
> +{
> + int err;
> + const char * fname;
> + struct dentry *topdir;
> +
> + err = rchan_create_dir(chanpath, , );
> + if (err && (err != -EEXIST))
> + return err;
> +
> + err = relayfs_create_file(fname, topdir, dentry, data, S_IRUSR);
> +
> + return err;
> +}
Why not just return the * to the dentry? Gets rid of one paramater...
Also, if a path is passed to this function, when the "channel" is
finally torn down, that path is not removed, right? Or did I miss that
in the code somewhere?
> +/**
> + * check_attribute_flags - check sanity of channel attributes
> + * @flags: channel attributes
> + *
> + * Returns 0 if successful, negative otherwise.
> + */
> +static inline int check_attribute_flags(u32 *attribute_flags)
> +{
> + u32 flags = *attribute_flags;
> +
> + if ((flags & RELAY_MODE_CONTINUOUS) &&
> + (flags & RELAY_MODE_NO_OVERWRITE))
> + return -EINVAL; /* Can't have it both ways */
> +
> + if (!(flags & RELAY_MODE_CONTINUOUS) &&
> + !(flags & RELAY_MODE_NO_OVERWRITE))
> + *attribute_flags |= RELAY_MODE_CONTINUOUS; /* Default */
> +
> + return 0;
> +}
Why be nice to your caller? Just force them to get the flags right, and
if not, error out, you don't have to provide a "default" as this is a
required parameter to start up a channel, right?
> +/*
> + * Per-cpu relay channel buffer
> + */
> +struct rchan_buf
> +{
> + void *start;/* start of channel buffer */
> + void *data; /* start of current sub-buffer */
> + local_t offset; /* current offset into sub-buffer */
> + unsigned bufsize; /* sub-buffer size */
> + unsigned alloc_size;/* total buffer size allocated */
> + unsigned nbufs; /* number of sub-buffers */
> + unsigned bufs_produced; /* count of sub-buffers produced */
> + unsigned bufs_consumed; /* count of sub-buffers consumed */
> + wait_queue_head_t read_wait;/* reader wait queue */
> + struct work_struct wake_readers; /* reader wake-up work struct */
> + struct rchan_callbacks *cb; /* client callbacks */
> + struct work_struct work;/* remove file work struct */
> + u32 flags; /* relay channel attributes */
> + struct dentry *dentry; /* channel file dentry */
> + atomic_t refcount; /* channel refcount */
Please use struct kref for this.
> + struct page **page_array; /* array of current buffer pages */
> + int page_count; /* number of current buffer pages */
> + unsigned *padding; /* padding counts per sub-buffer */
> + unsigned *commit; /* commit counts per sub-buffer */
> + int finalized; /* buffer has been finalized */
> +} cacheline_aligned;
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi <[EMAIL PROTECTED]> writes:
> Hi,
>
> This patch is the result of the latest round of liposuction on relayfs
> - the patch size is now 44K, down from 110K and the 200K before that.
> I'm posting it as a patch against 2.6.10 rather than -mm in order to
> make it easier to review, but will create one for -mm once the changes
> have settled down.
The logging fast path seems still a bit slow to me. I would like
to have a logging macro that is not much worse than a stdio putc,
basically something like
get_cpu();
if (buffer space > N) {
memcpy(buffer, input, N);
buffer pointer += N;
} else {
FreeBuffer(input, N);
}
put_cpu();
This would need interrupt protection only if interrupts can access
it, best you use separate buffers for that too.
-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi [EMAIL PROTECTED] writes:
Hi,
This patch is the result of the latest round of liposuction on relayfs
- the patch size is now 44K, down from 110K and the 200K before that.
I'm posting it as a patch against 2.6.10 rather than -mm in order to
make it easier to review, but will create one for -mm once the changes
have settled down.
The logging fast path seems still a bit slow to me. I would like
to have a logging macro that is not much worse than a stdio putc,
basically something like
get_cpu();
if (buffer space N) {
memcpy(buffer, input, N);
buffer pointer += N;
} else {
FreeBuffer(input, N);
}
put_cpu();
This would need interrupt protection only if interrupts can access
it, best you use separate buffers for that too.
-Andi
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
On Fri, Jan 28, 2005 at 01:38:22PM -0600, Tom Zanussi wrote:
+extern void * alloc_rchan_buf(unsigned long size,
+ struct page ***page_array,
+ int *page_count);
+extern void free_rchan_buf(void *buf,
+struct page **page_array,
+int page_count);
As these will be polluting the global namespace of the kernel, could
you add relayfs_ to the front of them?
Also, any reason why you haven't exported the fops of relayfs so that
others can use this in their filesystems (like proc and debugfs)?
+/**
+ * rchan_create_file - create relay file, including parent directories
+ * @chanpath: path to file, including filename
+ * @dentry: result dentry
+ * @data: data to associate with the file
+ *
+ * Returns 0 if successful, negative otherwise.
+ */
+static inline int rchan_create_file(const char *chanpath,
+ struct dentry **dentry,
+ struct rchan_buf *data)
+{
+ int err;
+ const char * fname;
+ struct dentry *topdir;
+
+ err = rchan_create_dir(chanpath, fname, topdir);
+ if (err (err != -EEXIST))
+ return err;
+
+ err = relayfs_create_file(fname, topdir, dentry, data, S_IRUSR);
+
+ return err;
+}
Why not just return the * to the dentry? Gets rid of one paramater...
Also, if a path is passed to this function, when the channel is
finally torn down, that path is not removed, right? Or did I miss that
in the code somewhere?
+/**
+ * check_attribute_flags - check sanity of channel attributes
+ * @flags: channel attributes
+ *
+ * Returns 0 if successful, negative otherwise.
+ */
+static inline int check_attribute_flags(u32 *attribute_flags)
+{
+ u32 flags = *attribute_flags;
+
+ if ((flags RELAY_MODE_CONTINUOUS)
+ (flags RELAY_MODE_NO_OVERWRITE))
+ return -EINVAL; /* Can't have it both ways */
+
+ if (!(flags RELAY_MODE_CONTINUOUS)
+ !(flags RELAY_MODE_NO_OVERWRITE))
+ *attribute_flags |= RELAY_MODE_CONTINUOUS; /* Default */
+
+ return 0;
+}
Why be nice to your caller? Just force them to get the flags right, and
if not, error out, you don't have to provide a default as this is a
required parameter to start up a channel, right?
+/*
+ * Per-cpu relay channel buffer
+ */
+struct rchan_buf
+{
+ void *start;/* start of channel buffer */
+ void *data; /* start of current sub-buffer */
+ local_t offset; /* current offset into sub-buffer */
+ unsigned bufsize; /* sub-buffer size */
+ unsigned alloc_size;/* total buffer size allocated */
+ unsigned nbufs; /* number of sub-buffers */
+ unsigned bufs_produced; /* count of sub-buffers produced */
+ unsigned bufs_consumed; /* count of sub-buffers consumed */
+ wait_queue_head_t read_wait;/* reader wait queue */
+ struct work_struct wake_readers; /* reader wake-up work struct */
+ struct rchan_callbacks *cb; /* client callbacks */
+ struct work_struct work;/* remove file work struct */
+ u32 flags; /* relay channel attributes */
+ struct dentry *dentry; /* channel file dentry */
+ atomic_t refcount; /* channel refcount */
Please use struct kref for this.
+ struct page **page_array; /* array of current buffer pages */
+ int page_count; /* number of current buffer pages */
+ unsigned *padding; /* padding counts per sub-buffer */
+ unsigned *commit; /* commit counts per sub-buffer */
+ int finalized; /* buffer has been finalized */
+} cacheline_aligned;
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Andi Kleen writes:
Tom Zanussi [EMAIL PROTECTED] writes:
Hi,
This patch is the result of the latest round of liposuction on relayfs
- the patch size is now 44K, down from 110K and the 200K before that.
I'm posting it as a patch against 2.6.10 rather than -mm in order to
make it easier to review, but will create one for -mm once the changes
have settled down.
The logging fast path seems still a bit slow to me. I would like
to have a logging macro that is not much worse than a stdio putc,
basically something like
get_cpu();
if (buffer space N) {
memcpy(buffer, input, N);
buffer pointer += N;
} else {
FreeBuffer(input, N);
}
put_cpu();
I think what we have now is somewhat similar, except that we wanted to
separate grabbing a slot in the buffer from the memcpy because some
applications such as ltt want to be able to directly write into the
slot without having to copy it into another buffer first. How about
something like this for relay reserve, with the local_add_return()
gone since we're assuming the client protects the buffer properly for
whatever it's doing:
relay_reserve(length)
{
if (buffer-offset + length = bufsize) {
reserved = buffer-data + buffer-offset;
buffer-offset += length;
} else { /* slow path */
reserved = switch_buffer(length);
if (reserved)
buffer-offset += length;
}
return reserved;
}
If you set up the channel to use MODE_CONTINUOUS, meaning continuously
cycle around the buffer, your logging function or macro would look
something like this:
simplest_write(data, length)
{
get_cpu();
reserved = relay_reserve();
memcpy(reserved, data, length);
put_cpu();
}
Clients who want to stop logging if the buffers are full would have an
extra test for when the reserve fails, which can never happen in
continuous mode:
/* uses MODE_NO_OVERWRITE */
ltt_write(data, length)
{
local_irq_save();
reserved = relay_reserve();
if (reserved) { /* if NULL, buffer full */
memcpy(reserved, item1, len1);
.
.
.
memcpy(reserved, itemN, lenN);
}
local_irq_restore();
}
This would need interrupt protection only if interrupts can access
it, best you use separate buffers for that too.
Not sure what you mean by separate buffers for that too. Can you
expand on that a little?
Thanks,
Tom
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Greg KH writes: On Fri, Jan 28, 2005 at 01:38:22PM -0600, Tom Zanussi wrote: +extern void * alloc_rchan_buf(unsigned long size, +struct page ***page_array, +int *page_count); +extern void free_rchan_buf(void *buf, + struct page **page_array, + int page_count); As these will be polluting the global namespace of the kernel, could you add relayfs_ to the front of them? Also, any reason why you haven't exported the fops of relayfs so that others can use this in their filesystems (like proc and debugfs)? No reason - I just forgot. I'll make sure that gets fixed along with all the other things you pointed out here in the next patch. Thanks, Tom - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi <[EMAIL PROTECTED]> wrote: > > This patch is the result of the latest round of liposuction on relayfs > - the patch size is now 44K, down from 110K and the 200K before that. > I'm posting it as a patch against 2.6.10 rather than -mm in order to > make it easier to review, but will create one for -mm once the changes > have settled down. Actually, I'll drop all the relayfs and ltt patches from -mm. They seem to have done their job ;) When things settle down and the code is ready for a new run, you know where I sit. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi wrote: > diff -urpN -X dontdiff linux-2.6.10/fs/Kconfig linux-2.6.10-cur/fs/Kconfig ... > + This file system is also available as a module ( = code which can be > + inserted in and removed from the running kernel whenever you want). > + The module is called relayfs. If you want to compile it as a > + module, say M here and read . ... This is a real nit, but personally I'd remove the stuff in parens above. It's not relayfs' job to educate users about what a module is. I'll try to give some more substantive feedback next week. Tim - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi wrote: diff -urpN -X dontdiff linux-2.6.10/fs/Kconfig linux-2.6.10-cur/fs/Kconfig ... + This file system is also available as a module ( = code which can be + inserted in and removed from the running kernel whenever you want). + The module is called relayfs. If you want to compile it as a + module, say M here and read file:Documentation/modules.txt. ... This is a real nit, but personally I'd remove the stuff in parens above. It's not relayfs' job to educate users about what a module is. I'll try to give some more substantive feedback next week. Tim - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] relayfs redux, part 2
Tom Zanussi [EMAIL PROTECTED] wrote: This patch is the result of the latest round of liposuction on relayfs - the patch size is now 44K, down from 110K and the 200K before that. I'm posting it as a patch against 2.6.10 rather than -mm in order to make it easier to review, but will create one for -mm once the changes have settled down. Actually, I'll drop all the relayfs and ltt patches from -mm. They seem to have done their job ;) When things settle down and the code is ready for a new run, you know where I sit. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
