Am Sonntag, 26. November 2006 20:01 schrieb Alan Stern:
> On Sun, 26 Nov 2006, Oliver Neukum wrote:
>
> > > Go ahead and try to write it. You may find it's a little tricky to create
> > > the sysfs_buffer_collection in the first place (and to delete it at the
> > > end). It may also be hard to acquire the locks for the dentry, the
> > > buffer, and the buffer_collection in the right order.
> >
> > Very well. This boots and survives a few tests.
>
> It looks pretty good. I have only a few comments.
>
> First, you definitely should use list_for_each_entry() instead of
> list_for_each().
It shall be done.
> Second, instead of putting a lock in the buffer_collection you could just
> use the inode's lock.
That's a layering violation.
Plus that lock is used for other things, plus it might be held a pretty long
time (eg the full control timeout)
> Third, I would have written add_to_collection() and
> remove_from_collection() inline. But that's nothing more than personal
> taste.
Separation of logic and implementation is good.
> Fourth, you might consider deallocating the sysfs_buffer_collection as
> soon as the last buffer is removed from its list. I don't know if this
> really matters...
That would mean having to use the inode's lock.
> Fifth, instead of allocating buffer_collections as needed, you might
> simply put the list_head directly into the kobject. It would complicate
> orphan_all_buffers a little bit because the list would then contain the
> buffers for all the attributes of the kobject. (Yes, this contradicts the
> previous recommendation.)
Exactly. Simplicity is a virtue. In addition, the majority of attributes is
never read.
Regards
Oliver
-------------------------------------------------------------------------
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
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
