[FYI, I'm not ignoring you guys.  I haven't come up with a good solution,
 but I'm really busy, so I haven't had time to focus on it.  I do intend
 to come up with one, becuase ckrm isn't the only subsystem that would
 benefit.]


On Mon, Aug 07, 2006 at 09:42:59PM -0400, Shailabh Nagar wrote:
> Agreed. If file->private_data is not used as a config_buffer,
> seq_file can be used as is to do the reading part.

        Not sure I'm happy with this naked seq_file usage, but that's
not the biggest issue.
 
> 1. Don't manage the write operation at a configfs level. Let the
> subsystem provide a write operation that reads the user supplied buffer.
> Subsystem can worry about how to sync between the seq_* and write ops.

        This won't happen.  I'm not allowing generic plug-anything-in
APIs that have a lifetime uncontrolled by configfs.  We want an
interface where the client module doesn't have to worry about lifetimes
of the file tree.  At all.  This is an intentional and important
property.

> 2. Add iterator functions
>  ->store_attribute_next , ->store_attribute_last
> to configfs_item_operations. Calling ->store_attribute_next tells the
> subsystem to expect more data as part of the same write,
> ->store_attribute_last indicates the end so it can do whatever it now
> does for store_attribute. So you don't end up with a seq_write_file but
> effectively do something like it. Granularity of data passed in one
> ->store_attribute_next can be chosen to be PAGE_SIZE for convenience
> though the count field can make a static choice unnecessary.

        This is more like what it might be, but I'm still debating how
it should all work.  Too many function pointers is a pain.  Still
thinking about it :-)

> That semantic is anyway exported/enforced by the subsystem and not really
> by configfs even now, right ? I mean, the ->store_attribute can choose
> to do whatever it likes with the data (looking at file->f_pos or not)
> even though configfs uses generic_ll_seek to advance the file->f_pos ?

        No, store() takes a buffer, and there is no f_pos of any
relevance.  If you look at file->f_pos, there is no guarantee that it
means anything.  It's undefined here.  This is from sysfs, and it is
also intentional.  It's setting an attribute, nothing more.

> Thats fine. The problem here seems to be the way configfs is trying to
> help simplify the buffer management (using ->store and ->show). The
> use of configfs_buffer and the unnecessarily constrained way in which it
> calls the subsystem's callbacks only once (instead of multiple times if
> needed) seems to be the root of the problem ?

        Yes and no.  In general, a simple attribute needs nothing more.
It's simple, and simply contained.  I'd call it a necessary constraint,
as we don't have any reason for even PAGE_SIZE attributes for most
things.
        However, you (and, eg, device-mapper tables) have a type of
attribute that is large.  It may not even be worth setting in a one-shot
method (a large buffer for ->store()).  Something incremental (ala your
set_next_thingy() function above) might be a better fit.  Certainly, you
can just put it in sysfs as I originally suggested, but it does fit
conceptually under the configfs tree for the object, which is why I'm
still here.

Joel

-- 

"To fall in love is to create a religion that has a fallible god."
        -Jorge Luis Borges

Joel Becker
Principal Software Developer
Oracle
E-mail: [EMAIL PROTECTED]
Phone: (650) 506-8127

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
ckrm-tech mailing list
https://lists.sourceforge.net/lists/listinfo/ckrm-tech

Reply via email to