On Wed, May 7, 2014 at 11:50 PM, Paolo Bonzini <[email protected]> wrote:
> Il 07/05/2014 16:34, Ming Lei ha scritto:
>>   *
>> - * An interesting effect of this policy is that only writes to req_vq need 
>> to
>> - * take the tgt_lock.  Read can be done outside the lock because:
>> - *
>> - * - writes of req_vq only occur when atomic_inc_return(&tgt->reqs) returns 
>> 1.
>> - *   In that case, no other CPU is reading req_vq: even if they were in
>> - *   virtscsi_queuecommand_multi, they would be spinning on tgt_lock.
>> - *
>> - * - reads of req_vq only occur when the target is not idle (reqs != 0).
>> - *   A CPU that enters virtscsi_queuecommand_multi will not modify req_vq.
>> + * tgt_lock is held to serialize reading and writing req_vq. Reading req_vq
>> + * could be done locklessly, but we do not do it yet.
>>   *
>>   * Similarly, decrements of reqs are never concurrent with writes of req_vq.
>>   * Thus they can happen outside the tgt_lock, provided of course we make 
>> reqs
>
> The part you deleted explains _why_ decrements of reqs are never
> concurrent with writes of req_vq.  Perhaps:

The below part might not need, since the 1st paragraph has
explained the basic principle, also looks current virtscsi_pick_vq()
isn't very difficult to understand.

>
>  * An interesting effect of this policy is that only increments to reqs and 
> writes
>  * to req_vq need to take the tgt_lock.  Reads of req_vq and decrements or 
> req_vq
>  * can be done outside the lock because:
>  *
>  * - writes of req_vq only occur when atomic_inc_return(&tgt->reqs) returns 1.
>  *   In that case, no other CPU is reading req_vq: even if they were in
>  *   virtscsi_queuecommand_multi, they would be spinning on tgt_lock.
>  *
>  * - reads of req_vq only occur when the target is not idle (reqs != 0).
>  *   A CPU that enters virtscsi_queuecommand_multi will not modify req_vq.

The above two parts should be very clear from current code in
virtscsi_pick_vq(), so I am not sure if we need the description.

>  *
>  * - likewise, decrements of reqs only occur when reqs != 0.  If the 
> decremented
>  *   value is zero, the first CPU that enters virtscsi_queuecommand_multi will
>  *   modify req_vq and the others will spin on tgt_lock.

The fact should be obvious too, :-)

>  *
>  * We do not try to read req_vq locklessly for simplicity, so tgt_lock is used
>  * to serialize reads of req_vq too.

Maybe it is better to just mention it isn't done yet, and maybe someone
will figure out simple way to support lockless reading req_vq.


Thanks,
-- 
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to