[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Minor synchronisation quibble in scsifront
I've been having a look through scsifront again, and I saw this bit: ring_req->timeout_per_command = (sc->timeout_per_command / HZ); ring_req->nr_segments = 0; spin_unlock_irq(host->host_lock); scsifront_do_request(info); wait_event_interruptible(info->shadow[ring_req->rqid].wq_reset, info->shadow[ring_req->rqid].wait_reset); in scsifront_dev_reset_handler(). Looking at scsifront_do_request(): static void scsifront_do_request(struct vscsifrnt_info *info) { struct vscsiif_front_ring *ring = &(info->ring); unsigned int irq = info->irq; int notify; RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(ring, notify); if (notify) notify_remote_via_irq(irq); } scsifront_do_request() is also called from scsifront_queuecommand(), where it's under the host lock. I can't see any other relevant synchronisation, so I think that you might be able to end up with several processors pushing requests at the same time. I'm not sure what'll happen then, but I doubt it's a good idea. The issue could be avoided if you just swapped two lines in scsifront_dev_reset_handler(): ring_req->timeout_per_command = (sc->timeout_per_command / HZ); ring_req->nr_segments = 0; scsifront_do_request(info); spin_unlock_irq(host->host_lock); wait_event_interruptible(info->shadow[ring_req->rqid].wq_reset, info->shadow[ring_req->rqid].wait_reset); Does that sound sane? On the plus side, that's the only strange bit I could see in current scsifront. :) Steven. Attachment:
signature.asc _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |