LWN: Comments on "Driver porting: The seq_file interface"

Driver porting: The seq_file interface

rtwertw — 2010-08-11T16:06:03+00:00

author need to learn to express his thoughts so not only himself, but ppl around could understand it. After reading it i only got more confused. Comments helped a bit. Thanks ppl.

Driver porting: The seq_file interface

iw2lsi — 2008-12-12T11:26:27+00:00

Hi all

is it normal to have more than one stop while reading a seq file ?

actually I have a list with 37 items but I get a "spontaneous" stop while processing element nine, and than a new start with the element ten (*pos=10).

after some hours (ok, two day :D) of debugging, it's all working now.. but it was really difficult for me to understand seq_file interface :D

thanks

Giampaolo

Driver porting: The seq_file interface

arafel — 2008-01-14T10:04:31+00:00

Hi,

Just wanted to say thanks for the comment. You've probably saved me a good few minutes trying
to work out why seq_file kept calling my code.

It must be said, the seq_file interface is one of the uglier APIs I've seen in the kernel.
Clear and easy to use it is not.

Driver porting: The seq_file interface

guest — 2005-04-01T17:40:45+00:00

This article describes how to generate an infinite series of integers. It would have been much more practical if this article had talked about generating finite series of integers ... I had trouble getting the seq_file iterator to stop calling my functions.

I read this article and http://www.kernelnewbies.org/documents/seq_file_howto.txt. Nevertheless, I still wasted a lot of time trying to get it to work.

My notes:

you write the routines your_start, your_stop, your_next, and your_show

The pos parameter to your_start and your_next is an index into the set of items that you wish to enumerate. your_next must increment this index, as in ++*pos.

your_start must know the number of items in your set so that it can check the validity of *pos. your_start will be called again, even after your_next returns NULL and your_stop has been called.

Let's say that you want to count from 0 through 9. your_start is called with *pos==0. It returns a pointer to your iterator data structure. your_show is called, then your_next is called. This happens until your_next returns NULL or ERR_PTR(some-negative-error-code). your_stop is then called. At this point, your_start will be called *again*, with *pos==10 ... seems strange to me, but that is the behavior.

The iterator pointer returned by your_start/your_next can be whatever you want, including integer values. However, be advised that the values in the range -1000 <= n <= 0 are interpreted as error conditions/NULL. Therefore, you probably want to return a pointer.

The kernelnewbies.org seq_file_howto.txt document says your_start should return error codes like EACCES if the *pos position is out of bounds. Note that if you want to do this, then you must return -EACCES, not EACCES. Maybe that was obvious to experienced kernel hackers, but it certainly was not obvious to me. Do this by returning ERR_PTR(-EACCES). If this value is not negative, then the seq_file code will assume that you have returned a valid iterator pointer and it will call your_show.

It is probably easier to simply return NULL on error conditions (like *pos out of bounds) and not worry about it. When I tried to worry about it and do the right thing, it just caused trouble for me.

I spent some time looking at the seq_file code. Frankly, I was quite disappointed ... the implementation looked pretty ugly to me.

Michael

Driver porting: The seq_file interface

gcc — 2004-01-30T23:08:17+00:00

Users should be clear about the difference between a position and an iterator (this confused me for a while). The position is your means of communication with the seq_file interface, telling it where you are, and for it to tell you where to go.

The iterator is an opaque value, about which seq_file understands nothing, except that you return it from your start and next handlers, and it will give it back to you in next, stop and show.

Examples of iterators given in the article are a simple index (using the pointer as an integer, i.e. it doesn't really point to anything), or a pointer to a linked list entry or an array member. However, if you have to navigate some more complex structure, such as a hash table, it can be useful to have a struct which holds several values, for example:

struct my_pos {              
	my_hash_table *table;              
	int hash;              
	my_ht_entry *entry;              
}

This allows a single value (your iterator, void *v) to tell you everything you need to know in order to walk the hashtable. In this case, as in the /proc/sequence example, v remains the same throughout the walk.

On the other hand, the position, loff_t *pos, is a strange beast. If you start reading the /proc file from the beginning, then start will be called with position 0, 1, 2, etc. until it returns NULL. At this point the user gets EOF, and they can then seek around and start reading again, which results in your start handler being called again.

However, if the user skips more than the length of the file, (e.g. dd if=/proc/my_file bs=1G skip=1, then your code will be called with position 0 twice! The user doesn't get what they were expecting (an empty file), but the whole file, as if the seek never happened. strace shows that the seek appears to succeed. I think this behavious is wrong and constitutes a mental health hazard.

If you want to use the same set of operations for multiple files in /proc, you need a way for them to distinguish what object they operate on. The article mentions s->private, but you can only use this after you have called seq_open, because the seq_file object doesn't exist before that. On the 2.6 kernel, it appears that a good place to store this private data is in the struct inode *inode which is passed to your open handler:

static int my_open (struct inode *inode, struct file          
*file)

In particular, 2.6 has a function called PROC_I which converts your struct inode into a

struct              
proc_inode

, which has a member called pde, which is your struct proc_dir_entry.

So you can create the proc_dir_entry, initialise its data member with a pointer to your private data, and then in the open handler you can get to it from inode->pde. For example:

// module init              
static int __init init()              
{              
	my_proc_entry = create_proc_entry(filename, 0, NULL);              
	/* ... check for failure ... */              
	my_proc_entry->data = my_data;              
}              
              
static int my_open (struct inode *inode, struct file *file)              
{              
	struct seq_file *s;       
	result = seq_open(file, &seq_ops);       
	/* ... check for failure ... */       
	/* file->private_data was initialised by seq_open */       
	s = (struct seq_file *)file->private_data;       
	my_data = PROC_I(inode)->pde->data;              
	s->private = my_data;              
}              
              
static int my_seq_start (...) {              
	my_data = s->private;              
}

I'm still looking for a good way to do this on 2.4, which lacks PROC_I and struct proc_inode. Can anyone help?

Driver porting: The seq_file interface

giraffedata — 2003-11-15T01:53:45+00:00

I am now kfree()ing in ct_seq_next(), just before it returns with NULL at the end of the table

Seems like that would be a problem if the user chooses not to read all the way to EOF.

This just sounds like a basic bug in the seq_file interface. If ct_seq_start() fails, it should be ct_seq_start's responsibility to not change any state, and thus ct_seq_stop doesn't need to be, and should not be, called. After all, does a POSIX program call close(-1) when open() fails?

Driver porting: The seq_file interface

laf0rge — 2003-11-14T13:13:24+00:00

After using this article as an example to port the /proc/net/ip_conntrack interface over to seq_file, and about 5 hours of crashing/rebooting/debugging, I have to admit that there are some shortcomings in it.

Some hints for other people, so they don't fall into the same pits as I did:

1) If you allocate something in ct_seq_start(), the place to free it is _NOT_ in ct_seq_stop(). This is because ct_seq_stop() is even called if ct_seq_start() returns an error (Like ERR_PTR(-ENOMEM)). You would then end up calling kfree(ERR_PTR(-ENOMEM)) which your mm subsystem doesn't really like. I am now kfree()ing in ct_seq_next(), just before it returns with NULL at the end of the table.

2) If you take a lock in ct_seq_start(), do it unconditionally as the first thing. Even if ct_seq_start() fails, ct_seq_stop() is called. In ct_seq_stop() you have no idea of knowing if ct_seq_start() failed or not - so you will unconditionally unlock.