On Tue, 23 Sep 2014 00:55:49 -0400 Tejun Heo <t...@kernel.org> wrote:
> Hello, Neil. > > On Tue, Sep 23, 2014 at 02:46:50PM +1000, NeilBrown wrote: > > seqfile is only safe for reads. sysfs via kernfs uses seq_read(), so there > > is only a single allocation on the first read. > > > > It doesn't really related to fixing writes, except to point out that only > > writes need to be "fixed". Reads already work. > > Oh, I meant the buffer seqfile read op writes to, so it depends on the > fact that the allocation is only on the first read? That seems > extremely brittle to me, especially for an issue which tends to be > difficult to reproduce. It is easy for user-space to ensure they read once before any critical time.. > > > Separately: > > > > > Ugh... :( If this can't be avoided at all, I'd much prefer it to be > > > something explicit - a flag marking the file as needing a persistent > > > write buffer which is allocated on open. "Small" writes on stack > > > feels way to implicit to me. > > > > How about if we add seq_getbuf() and seq_putbuf() to seqfile > > which takes a 'struct seq_file' and a size and returns the ->buf > > after making sure it is big enough. > > It also claims and releases the seqfile ->lock. > > > > Then we would be using the same buffer for reads and write. > > > > Does that sound suitable? It uses existing infrastructure and avoids having > > to identify in advance which attributes it is important for. > > I'd much rather keep things direct and make it explicitly allocate r/w > buffer(s) on open and disallow seq_file operations on such files. As far as I can tell, seq_read is used on all sysfs files that are readable except for 'binary' files. Are you suggesting all files that might need to be accessed without a kmalloc have to be binary files? Having to identify those files which are important in advance seems the more "brittle" approach to me. I would much rather it "just worked" Would you prefer a new per-attribute flag which directed sysfs to pre-allocate a full page, or a 'max_size' attribute which caused a buffer of that size to be allocated on open? The same size would be used to pre-allocate the seqfile buf (like single_open_size does) if reads were supported. Thanks, NeilBrown
signature.asc
Description: PGP signature
