Il 06/05/2014 11:26, Ming Lei ha scritto:
Hi Paolo and All,
One question is about ACCESS_ONCE() in virtscsi_pick_vq(),
looks it needn't since both reading and writing tgt->req_vq holds
tgt->tgt_lock.
You're right. It should be possible to avoid the lock in
virtscsi_pick_vq like this:
value = atomic_read(&tgt->reqs);
retry:
if (value != 0) {
old_value = atomic_cmpxchg(&tgt->regs, value, value + 1)
if (old_value != value) {
value = old_value;
goto retry;
}
smp_mb__after_atomic_cmpxchg(); // you get the idea :)
vq = ACCESS_ONCE(tgt->req_vq);
} else {
spin_lock_irqsave(&tgt->tgt_lock, flags);
// tgt->reqs may not be 0 anymore, need to recheck
value = atomic_read(&tgt->reqs);
if (atomic_read(&tgt->reqs) != 0) {
spin_unlock_irqrestore(&tgt->tgt_lock, flags);
goto retry;
}
// tgt->reqs now will remain fixed to 0.
...
tgt->req_vq = vq = ...;
smp_wmb();
atomic_set(&tgt->reqs, 1);
spin_unlock_irqrestore(&tgt->tgt_lock, flags);
}
return vq;
and then you would need the ACCESS_ONCE but I'm not sure it's worthwhile.
Another one is about the comment in virtscsi_req_done(), which
said smp_read_barrier_depends() is needed for avoiding
out of order between reading req_vq and decreasing tgt->reqs.
But if I understand correctly, in virtscsi_req_done(), req_vq is
read from vscsi->req_vqs[vq->index - VIRTIO_SCSI_VQ_BASE],
instead of tgt->req_vq, and the former won't change wrt.
inc/dec tgt->reqs, so can the barrier be removed?
Right. The comment is obsolete and dates to before vq->index existed.
Paolo
Any comments about the above?
Thanks,
--
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
