Am Samstag, 25. November 2006 17:23 schrieb Alan Stern:
> On Sat, 25 Nov 2006, Oliver Neukum wrote:
> 
> > > > OK, a bit more elaborate.
> > > > 
> > > > 1. add a list to struct attribute
> > > > 2. add any struct sysfs_buffer opened to that list
> > > > 3. in class_device_remove_file walk this list and mark all buffers on a 
> > > > list orphaned
> > > > 4. in fill_read_buffer check this flag an fail if need (the operation 
> > > > is already under lock)
> > > 
> > > Actually, I think that might work.  You'd have to add a pointer from
> > > the sysfs_buffer to the struct file or the dentry, so that you could tell
> > > which buffers belonged to the device whose file was being removed.  And
> > 
> > Yes, there needs to be a back pointer for close(), but why not to
> > struct attribute?
> 
> Let's say you have two devices (D1 and D2) plugged in, both managed by the
> X driver.  They share the single struct attribute (A) in the driver code,
> but they have separate file, dentry, and sysfs_buffer structures.  Now

OK, I see. I didn't think about that usage.

> device D2 is unplugged, and the X driver's disconnect routine calls
> driver_remove_file(D2, &A).  So you walk through the list of sysfs_buffers
> attached to A -- how do you tell which ones are for D1 and which are for
> D2?  The pointer to the struct device is accessed from the dentry, but the
> sysfs_buffer doesn't contain any pointers to the file or the dentry.
> 
> > > you'd have to synchronize walking the list with opening & closing
> > > attribute files, which could turn out to be prohibitively difficult.
> > 
> > Not trivial. It seems to me that a simple locking scheme would deadlock.
> 
> I don't see why it would.
> 
> > How about refcounting struct attribute? It would mean that they could not
> > be allocated statically, but it would solve the problem cleanly and there'd
> > still be an upper bound on memory consumption due to the limit on open
> > files.
> 
> You're not thinking of the right problem.  The struct attribute doesn't 
> matter; the issue is the new list you're proposing.

Right, for the reason above I wanted to house it in struct attribute.
This is obviously impractical.

> Consider the example above.  Let's say a user process closes the attribute
> file for D2 at the same time as device_remove_file() is called.  The user
> process will unlink the sysfs_buffer from the list attached to A while
> device_remove_file() is walking the list.  Somehow you need to protect the 
> list pointers.

OK. How about:

struct sysfs_buffer_collection {
        struct list_head                associated_buffers;
        struct mutex            lock;
};

struct sysfs_buffer {
        size_t                  count;
        loff_t                  pos;
        char                    * page;
        struct sysfs_buffer_collection  * set;
        struct sysfs_ops        * ops;
        struct semaphore        sem;
        int                     orphaned;
        int                     needs_read_fill;
        int                     event;
};

close() would take the lock of sysfs_buffer_collection, then remove the buffer 
from the list
device_remove_file() takes the lock, walks the elemnts, takes their locks and 
sets orphaned,
then it proceeds normally
actual IO to the buffers takes the locks and checks orphaned

For validity of sysfs_buffer_collection itself, we depend on the vfs

        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

Reply via email to