Hi, The current relayfs read implementation works fine, but was designed to be used mainly for 'draining' the buffer after a tracing run. It turns out that people really want to be able to read from the buffer during a live trace, for example the blktrace application submitted recently:
http://marc.theaimsgroup.com/?l=linux-kernel&m=112480046405961&w=2 Here's an improved read implementation for relayfs which allows for that. This version has been tested pretty thoroughly, using both the blktrace application and a new example I added to the relay-apps tarball called 'readtest' which is basically a unit test for the read functionality. All the tests I've come up with have passed and it looks pretty solid at this point. Here's a link to the test code: http://prdownloads.sourceforge.net/relayfs/relay-apps-0.8.tar.gz?download Andrew, please apply. Thanks, Tom Signed-off-by: Tom Zanussi <[EMAIL PROTECTED]> diff -urpN -X dontdiff linux-2.6.13-rc6-mm2/Documentation/filesystems/relayfs.txt linux-2.6.13-rc6-mm2-cur/Documentation/filesystems/relayfs.txt --- linux-2.6.13-rc6-mm2/Documentation/filesystems/relayfs.txt 2005-08-25 19:28:59.000000000 -0500 +++ linux-2.6.13-rc6-mm2-cur/Documentation/filesystems/relayfs.txt 2005-08-25 17:07:48.000000000 -0500 @@ -82,10 +82,15 @@ mmap() results in channel buffer being memory space. Note that you can't do a partial mmap - you must map the entire file, which is NRBUF * SUBBUFSIZE. -read() read the contents of a channel buffer. If there are active - channel writers, results may be unpredictable - users should - make sure that all logging to the channel has ended before - using read(). +read() read the contents of a channel buffer. The bytes read are + 'consumed' by the reader i.e. they won't be available again + to subsequent reads. If the channel is being used in + no-overwrite mode (the default), it can be read at any time + even if there's an active kernel writer. If the channel is + being used in overwrite mode and there are active channel + writers, results may be unpredictable - users should make + sure that all logging to the channel has ended before using + read() with overwrite mode. poll() POLLIN/POLLRDNORM/POLLERR supported. User applications are notified when sub-buffer boundaries are crossed. @@ -256,8 +261,8 @@ consulted. The default subbuf_start() implementation, used if the client doesn't define any callbacks, or doesn't define the subbuf_start() callback, -implements the simplest possible 'overwrite' mode i.e. it does nothing -but return 1. +implements the simplest possible 'no-overwrite' mode i.e. it does +nothing but return 0. Header information can be reserved at the beginning of each sub-buffer by calling the subbuf_start_reserve() helper function from within the diff -urpN -X dontdiff linux-2.6.13-rc6-mm2/fs/relayfs/inode.c linux-2.6.13-rc6-mm2-cur/fs/relayfs/inode.c --- linux-2.6.13-rc6-mm2/fs/relayfs/inode.c 2005-08-25 19:29:02.000000000 -0500 +++ linux-2.6.13-rc6-mm2-cur/fs/relayfs/inode.c 2005-08-25 18:21:31.000000000 -0500 @@ -295,101 +295,143 @@ static int relayfs_release(struct inode } /** - * relayfs_read_start - find the first available byte to read - * - * If the read_pos is in the middle of padding, return the - * position of the first actually available byte, otherwise - * return the original value. + * relayfs_read_consume - update the consumed count for the buffer */ -static inline size_t relayfs_read_start(size_t read_pos, - size_t avail, - size_t start_subbuf, - struct rchan_buf *buf) +static void relayfs_read_consume(struct rchan_buf *buf, + size_t read_pos, + size_t bytes_consumed) { - size_t read_subbuf, adj_read_subbuf; - size_t padding, padding_start, padding_end; size_t subbuf_size = buf->chan->subbuf_size; size_t n_subbufs = buf->chan->n_subbufs; + size_t read_subbuf; - read_subbuf = read_pos / subbuf_size; - adj_read_subbuf = (read_subbuf + start_subbuf) % n_subbufs; + if (buf->bytes_consumed + bytes_consumed > subbuf_size) { + relay_subbufs_consumed(buf->chan, buf->cpu, 1); + buf->bytes_consumed = 0; + } - if ((read_subbuf + 1) * subbuf_size <= avail) { - padding = buf->padding[adj_read_subbuf]; - padding_start = (read_subbuf + 1) * subbuf_size - padding; - padding_end = (read_subbuf + 1) * subbuf_size; - if (read_pos >= padding_start && read_pos < padding_end) { - read_subbuf = (read_subbuf + 1) % n_subbufs; - read_pos = read_subbuf * subbuf_size; + buf->bytes_consumed += bytes_consumed; + read_subbuf = read_pos / buf->chan->subbuf_size; + if (buf->bytes_consumed + buf->padding[read_subbuf] == subbuf_size) { + if ((read_subbuf == buf->subbufs_produced % n_subbufs) && + (buf->offset == subbuf_size)) + return; + relay_subbufs_consumed(buf->chan, buf->cpu, 1); + buf->bytes_consumed = 0; + } +} + +/** + * relayfs_read_avail - boolean, are there unconsumed bytes available? + */ +static int relayfs_read_avail(struct rchan_buf *buf, size_t read_pos) +{ + size_t bytes_produced, bytes_consumed, write_offset; + size_t subbuf_size = buf->chan->subbuf_size; + size_t n_subbufs = buf->chan->n_subbufs; + size_t produced = buf->subbufs_produced % n_subbufs; + size_t consumed = buf->subbufs_consumed % n_subbufs; + + write_offset = buf->offset > subbuf_size ? subbuf_size : buf->offset; + + if (consumed > produced) { + if ((produced > n_subbufs) && + (produced + n_subbufs - consumed <= n_subbufs)) + produced += n_subbufs; + } else if (consumed == produced) { + if (buf->offset > subbuf_size) { + produced += n_subbufs; + if (buf->subbufs_produced == buf->subbufs_consumed) + consumed += n_subbufs; } } - return read_pos; + if (buf->offset > subbuf_size) + bytes_produced = (produced - 1) * subbuf_size + write_offset; + else + bytes_produced = produced * subbuf_size + write_offset; + bytes_consumed = consumed * subbuf_size + buf->bytes_consumed; + + if (bytes_produced == bytes_consumed) + return 0; + + relayfs_read_consume(buf, read_pos, 0); + + return 1; } /** - * relayfs_read_end - return the end of available bytes to read - * - * If the read_pos is in the middle of a full sub-buffer, return - * the padding-adjusted end of that sub-buffer, otherwise return - * the position after the last byte written to the buffer. At - * most, 1 sub-buffer can be read at a time. - * + * relayfs_read_subbuf_avail - return bytes available in sub-buffer */ -static inline size_t relayfs_read_end(size_t read_pos, - size_t avail, - size_t start_subbuf, - struct rchan_buf *buf) +static size_t relayfs_read_subbuf_avail(size_t read_pos, + struct rchan_buf *buf) { - size_t padding, read_endpos, buf_offset; - size_t read_subbuf, adj_read_subbuf; + size_t padding, avail = 0; + size_t read_subbuf, read_offset, write_subbuf, write_offset; size_t subbuf_size = buf->chan->subbuf_size; - size_t n_subbufs = buf->chan->n_subbufs; - buf_offset = buf->offset > subbuf_size ? subbuf_size : buf->offset; + write_subbuf = (buf->data - buf->start) / subbuf_size; + write_offset = buf->offset > subbuf_size ? subbuf_size : buf->offset; read_subbuf = read_pos / subbuf_size; - adj_read_subbuf = (read_subbuf + start_subbuf) % n_subbufs; + read_offset = read_pos % subbuf_size; + padding = buf->padding[read_subbuf]; - if ((read_subbuf + 1) * subbuf_size <= avail) { - padding = buf->padding[adj_read_subbuf]; - read_endpos = (read_subbuf + 1) * subbuf_size - padding; + if (read_subbuf == write_subbuf) { + if (read_offset + padding < write_offset) + avail = write_offset - (read_offset + padding); } else - read_endpos = read_subbuf * subbuf_size + buf_offset; + avail = (subbuf_size - padding) - read_offset; - return read_endpos; + return avail; } /** - * relayfs_read_avail - return total available along with buffer start - * - * Because buffers are circular, the 'beginning' of the buffer - * depends on where the buffer was last written. If the writer - * has cycled around the buffer, the beginning is defined to be - * the beginning of the sub-buffer following the last sub-buffer - * written to, otherwise it's the beginning of sub-buffer 0. + * relayfs_read_start_pos - find the first available byte to read * + * If the read_pos is in the middle of padding, return the + * position of the first actually available byte, otherwise + * return the original value. */ -static inline size_t relayfs_read_avail(struct rchan_buf *buf, - size_t *start_subbuf) +static size_t relayfs_read_start_pos(size_t read_pos, + struct rchan_buf *buf) { - size_t avail, complete_subbufs, cur_subbuf, buf_offset; + size_t read_subbuf, padding, padding_start, padding_end; size_t subbuf_size = buf->chan->subbuf_size; size_t n_subbufs = buf->chan->n_subbufs; + + read_subbuf = read_pos / subbuf_size; + padding = buf->padding[read_subbuf]; + padding_start = (read_subbuf + 1) * subbuf_size - padding; + padding_end = (read_subbuf + 1) * subbuf_size; + if (read_pos >= padding_start && read_pos < padding_end) { + read_subbuf = (read_subbuf + 1) % n_subbufs; + read_pos = read_subbuf * subbuf_size; + } - buf_offset = buf->offset > subbuf_size ? subbuf_size : buf->offset; + return read_pos; +} - if (buf->subbufs_produced >= n_subbufs) { - complete_subbufs = n_subbufs - 1; - cur_subbuf = (buf->data - buf->start) / subbuf_size; - *start_subbuf = (cur_subbuf + 1) % n_subbufs; - } else { - complete_subbufs = buf->subbufs_produced; - *start_subbuf = 0; - } +/** + * relayfs_read_end_pos - return the new read position + */ +static size_t relayfs_read_end_pos(struct rchan_buf *buf, + size_t read_pos, + size_t count) +{ + size_t read_subbuf, padding, end_pos; + size_t subbuf_size = buf->chan->subbuf_size; + size_t n_subbufs = buf->chan->n_subbufs; - avail = complete_subbufs * subbuf_size + buf_offset; + read_subbuf = read_pos / subbuf_size; + padding = buf->padding[read_subbuf]; + if (read_pos % subbuf_size + count + padding == subbuf_size) + end_pos = (read_subbuf + 1) * subbuf_size; + else + end_pos = read_pos + count; + if (end_pos >= subbuf_size * n_subbufs) + end_pos = 0; - return avail; + return end_pos; } /** @@ -401,13 +443,6 @@ static inline size_t relayfs_read_avail( * * Reads count bytes or the number of bytes available in the * current sub-buffer being read, whichever is smaller. - * - * NOTE: The results of reading a relayfs file which is currently - * being written to are undefined. This is because the buffer is - * circular and an active writer in the kernel could be - * overwriting the data currently being read. Therefore read() - * is mainly useful for reading the contents of a buffer after - * logging has completed. */ static ssize_t relayfs_read(struct file *filp, char __user *buffer, @@ -416,33 +451,30 @@ static ssize_t relayfs_read(struct file { struct inode *inode = filp->f_dentry->d_inode; struct rchan_buf *buf = RELAYFS_I(inode)->buf; - size_t read_start, read_end, avail, start_subbuf; - size_t buf_size = buf->chan->subbuf_size * buf->chan->n_subbufs; + size_t read_start, avail; + ssize_t ret = 0; void *from; - avail = relayfs_read_avail(buf, &start_subbuf); - if (*ppos >= avail) - return 0; - - read_start = relayfs_read_start(*ppos, avail, start_subbuf, buf); - if (read_start == 0 && *ppos) - return 0; - - read_end = relayfs_read_end(read_start, avail, start_subbuf, buf); - if (read_end == read_start) - return 0; - - from = buf->start + start_subbuf * buf->chan->subbuf_size + read_start; - if (from >= buf->start + buf_size) - from -= buf_size; - - count = min(count, read_end - read_start); - if (copy_to_user(buffer, from, count)) - return -EFAULT; - - *ppos = read_start + count; - - return count; + down(&inode->i_sem); + if(!relayfs_read_avail(buf, *ppos)) + goto out; + + read_start = relayfs_read_start_pos(*ppos, buf); + avail = relayfs_read_subbuf_avail(read_start, buf); + if (!avail) + goto out; + + from = buf->start + read_start; + ret = count = min(count, avail); + if (copy_to_user(buffer, from, count)) { + ret = -EFAULT; + goto out; + } + relayfs_read_consume(buf, read_start, count); + *ppos = relayfs_read_end_pos(buf, read_start, count); +out: + up(&inode->i_sem); + return ret; } /** @@ -481,6 +513,7 @@ struct file_operations relayfs_file_oper .poll = relayfs_poll, .mmap = relayfs_mmap, .read = relayfs_read, + .llseek = no_llseek, .release = relayfs_release, }; diff -urpN -X dontdiff linux-2.6.13-rc6-mm2/fs/relayfs/relay.c linux-2.6.13-rc6-mm2-cur/fs/relayfs/relay.c --- linux-2.6.13-rc6-mm2/fs/relayfs/relay.c 2005-08-25 19:29:02.000000000 -0500 +++ linux-2.6.13-rc6-mm2-cur/fs/relayfs/relay.c 2005-08-25 21:12:29.000000000 -0500 @@ -58,6 +58,9 @@ static int subbuf_start_default_callback void *prev_subbuf, size_t prev_padding) { + if (relay_buf_full(buf)) + return 0; + return 1; } @@ -120,6 +123,7 @@ static inline void __relay_reset(struct buf->subbufs_produced = 0; buf->subbufs_consumed = 0; + buf->bytes_consumed = 0; buf->finalized = 0; buf->data = buf->start; buf->offset = 0; @@ -262,6 +266,7 @@ struct rchan *relay_open(const char *bas for_each_online_cpu(i) { sprintf(tmpname, "%s%d", base_filename, i); chan->buf[i] = relay_open_buf(chan, tmpname, parent); + chan->buf[i]->cpu = i; if (!chan->buf[i]) goto free_bufs; } @@ -328,7 +333,7 @@ size_t relay_switch_subbuf(struct rchan_ return length; toobig: - printk(KERN_WARNING "relayfs: event too large (%u)\n", length); + printk(KERN_WARNING "relayfs: event too large (%Zd)\n", length); WARN_ON(1); return 0; } diff -urpN -X dontdiff linux-2.6.13-rc6-mm2/include/linux/relayfs_fs.h linux-2.6.13-rc6-mm2-cur/include/linux/relayfs_fs.h --- linux-2.6.13-rc6-mm2/include/linux/relayfs_fs.h 2005-08-25 19:29:03.000000000 -0500 +++ linux-2.6.13-rc6-mm2-cur/include/linux/relayfs_fs.h 2005-08-24 00:16:37.000000000 -0500 @@ -22,7 +22,7 @@ /* * Tracks changes to rchan_buf struct */ -#define RELAYFS_CHANNEL_VERSION 4 +#define RELAYFS_CHANNEL_VERSION 5 /* * Per-cpu relay channel buffer @@ -44,6 +44,8 @@ struct rchan_buf unsigned int finalized; /* buffer has been finalized */ size_t *padding; /* padding counts per sub-buffer */ size_t prev_padding; /* temporary variable */ + size_t bytes_consumed; /* bytes consumed in cur read subbuf */ + unsigned int cpu; /* this buf's cpu */ } ____cacheline_aligned; /* - 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/
