On Tue, 2006-10-10 at 14:58 -0700, Joel Becker wrote: > On Tue, Oct 10, 2006 at 02:31:43PM -0700, Paul Menage wrote: > > > NAK. This forces a complex and inappropriate interface on the > > >majority of users, and doesn't honor configfs' simplicity-first design. > > > > How is the seq_file interface complex and inappropriate? For the > > configfs clients it's basically a drop-in replacement for sprintf(), > > as Chandra's patches show. > > Well, they now have to learn seq_file. They now get to assume
If they are simple users, they don't have to "learn" seq_file semantics, they would just replace their sprintf's with seq_printfs (as my changes in OCFS2 show). IMO, seq_file interface is not that complex to learn either. > that "spewing large amounts of junk" is the default rather than "single > attribute", which is correct. None of it is relevant for the majority > of correct users. "char *" can also be used to spew out large amount of data (ok, maybe up to PAGESIZE in configfs's case :). My point is that changing char * to seq_file doesn't necessarily "introduce" the issue (of spewing large amounts of data). > It exposes the "I'm a file" knowledge down to the client module. > The entire point of configfs is that the "filesystem bits" are > independant of the "client bits". To the client, it's an item > hierarchy. To the user, the interface happens to be a filesystem. This issue is moot, unless you have intentions of changing the user interface of configfs to be anything other than a file system, isn't it ? > Technically, the seq_printf() as a drop-in replacement seems to > be functional. I'm worried about lifetiming, but I think it's OK (what > do I mean? If I open the file, I'd better not be able to remove the > client module until everything is torn down. If I close the file, it > had better get all torn down before module_put() so that when > ->release() returns, te module can safely be removed. I *think* this > change satisfies these worries, but it's something that absolutely has > to be done right. Yes, I'm very paranoid about this). > My bigger worry is that we haven't solved the write side. How > does one *set* a large attribute? It had better not be multiple > attributes. I know that your module doesn't set it, but hey, we don't > codify that requirement. Perhaps a patch where we say "if you are a > large display attribute, we'll use seq_file and error on write because > it isn't allowed" but that leaves the old buffer-based approach for > normal-sized read-write attributes. Now we are in need of *large* reads. We can add this feature and let it evolve to the next level later when somebody needs to *set* a large attribute. Also, these changes do not result in any change in the user level interface. So, we can afford this interface changes to change again later. > Joel > -- ---------------------------------------------------------------------- Chandra Seetharaman | Be careful what you choose.... - [EMAIL PROTECTED] | .......you may get it. ---------------------------------------------------------------------- ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ ckrm-tech mailing list https://lists.sourceforge.net/lists/listinfo/ckrm-tech