On Tue, 05 Oct 2010 16:00:29 +0530 Suresh Jayaraman <sjayaraman@xxxxxxx> wrote: > On 10/04/2010 11:22 PM, Jeff Layton wrote: > > To prevent races we need to be able to do an atomic_dec_and_lock with > > it, and that won't work with a rwlock. > > It's not clear to me where a potential race window is. Could you please > elaborate? > > > Thanks, > The specific race window that I was thinking about doesn't exist today as such, but that's just because the way that filehandles are managed is so different. When we do refcounting with atomic_t's though we need to take care that we don't act on them without taking steps to prevent races. For instance, suppose I had left the code using an rwlock_t and did this in cifsFileInfo_put: if (atomic_dec_and_test(&open_file->count)) return; write_lock(&GlobalSMBSeslock) ...and then tear down the cifsFileInfo. There would always be a possibility that the count increased after I checked it but before taking the lock, which could result in a use-after-free. Now that I think about this though...it might be better to just make the count non-atomic, and simply move it under the protection of the lock. You generally need the lock anyway when you're searching for a file so there's little benefit to using an atomic here. -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
