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.
----------------------------------------------------------------------
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
