On Wed, Jan 14, 2009 at 03:31:15PM -0500, Tony Battersby wrote: > 4) Add a variation of kref_get() that uses atomic_inc_not_zero(). Ick. > When I was designing my previous patches, I anticipated running into > these complications with kref, which is one reason I avoided it. Heh. > Other people have experienced similar problems with the kref interface > (search "kref_get_not_zero" or "cgroups: fix probable race with > put_css_set[_taskexit] and find_css_set"). If you still want to use > kref, I think that we are either going to have to change the behavior > of /proc/scsi/sg/*, or else use atomic_inc_not_zero(). Since I still > prefer not to change valid user-visible behavior in a patch that fixes > so many possible oopses, I will go with atomic_inc_not_zero(). And, > since I am going that route, I will use it for sg_get_dev() also, > so that sg_put_dev() doesn't need to acquire sg_index_lock. > > For the purposes of this patch, I added the local function > sg_kref_get_not_zero() to sg.c. It would be better to add > kref_get_not_zero() to kref.c, but I see in the mailing list archives > that there has been some resistance to that idea because it complicates > the kref interface. However, since it has come up more than once, > perhaps it would be better to go ahead and make it an official part > of the interface. If anyone wants to support that idea, I will break > it out into a separate patch. I'd be interested in seeing how you propose such a change so that it works properly for people, because as you have noted, others have had this same problem. I strongly object to the use of sg_kref_get_not_zero(), as you are just providing a private version of this function, which you shouldn't be doing. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
