Re: [PATCH 0/2] sg: fix races during device removal (v2)

[Tony Battersby <tonyb@cybernetics.com>]

FUJITA Tomonori wrote:
>> Besides holding the lock during kref_put(), I also considered two
>> other simple ways to avoid this race:
>> 1) Do idr_remove() from sg_remove().
>> 2) Return NULL in sg_get_dev() if sdp->detached.
>>
>> However, both of these options would have changed the behavior of
>> the /proc/scsi/sg/* functions that show information for devices that
>> are in the process of being detached.  I wanted to fix bugs without
>> changing other behavior, so I chose to call kref_put() under lock in
>> my previous patches.
>>     
>
> How about doing 2) and accessing to /proc/scsi/sg/* with
> sg_index_lock (don't use sg_get_device for it).
>
>   
That's an excellent idea.  If it works, I can forget this whole
atomic_inc_not_zero() business and the pain that goes with it.
> What /proc/scsi/sg/* doing is abnormal from the perspective of the ref
> counting (accessing to something that is going away).
>
> >From the perspective of the ref counting, the best way is calling
> idr_remove in from sg_remove but as you said, it's not nice to change
> the behavior.
>
>   
Exactly.
> If we don't use sg_get_device for /proc/scsi/sg/*, then we use
> sg_get_device for only sg_open So we can do 2) without changing the
> behavior.
>
>   
I will try to implement it this way today.  Thanks for your input.

Tony