Messages in this thread | | | Date | Mon, 21 Oct 2013 18:35:39 +0800 | From | Vaughan Cao <> | Subject | Re: [PATCH] [SCSI] sg: late O_EXCL fix for lk 3.12-rc |
| |
On 2013年10月21日 07:00, Douglas Gilbert wrote: > On 13-10-20 01:31 PM, Bart Van Assche wrote: >> On 10/20/13 18:09, Douglas Gilbert wrote: >>> Given that lk 3.12.0 release is not far away, the safest path >>> may still be to revert Vaughan Cao's patch. I'll leave that >>> decision to the maintainers. >> >> Hello Doug, >> >> Thanks for looking into this. But I would appreciate it if you could >> address the >> whitespace errors reported by checkpatch: >> >> ERROR: space prohibited after that '!' (ctx:BxW) >> #24: FILE: drivers/scsi/sg.c:241: >> + (excl_case ? (! sdp->exclude) : sfds_list_empty(sdp)))); >> ^ >> >> ERROR: space prohibited after that '!' (ctx:BxW) >> #55: FILE: drivers/scsi/sg.c:289: >> + if (! alone) { >> ^ >> >> ERROR: code indent should use tabs where possible >> #59: FILE: drivers/scsi/sg.c:292: >> + }$ >> >> WARNING: please, no spaces at the start of a line >> #59: FILE: drivers/scsi/sg.c:292: >> + }$ >> >> ERROR: space prohibited after that '!' (ctx:BxW) >> #73: FILE: drivers/scsi/sg.c:301: >> + while (! alone) { >> ^ >> >> WARNING: suspect code indent for conditional statements (8, 12) >> #144: FILE: drivers/scsi/sg.c:375: >> + if (excl || sfds_list_empty(sdp)) >> + wake_up_interruptible(&sdp->open_wait); >> > > I'd prefer people to test the patch or find logical flaws. > > Doug Gilbert > Hi Doug,
Will the lines below conflict with the meaning of NONBLOCK? >+ down(&sdp->or_sem); >+ alone = sfds_list_empty(sdp); > if (!((flags & O_NONBLOCK) || > scsi_block_when_processing_errors(sdp->device))) {
Assume one thread holds the or_sem and waiting in scsi_block_when_processing_errors for the underlying scsi device to complete error recovery, another thread with O_NONBLOCK call sg_open().
I'm also curious why we can skip checking _processing_errors() when called with O_NONBLOCK?...Though it has been there from the very beginning. In other words, since scsi device may go into a error recovery state at random time, why we only check here?
Thanks, Vaughan
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |