Hello, Paul.
On Thu, Jun 29, 2017 at 11:10:57AM -0700, Paul E. McKenney wrote:
> If this code fragment doesn't deadlock, then CPU 0's spin_unlock_wait()
> must have executed before CPU 1's spin_lock(). However, even on x86,
> CPU 0's prior writes can be reordered with its subsequent reads, which
> means that r1 == 0 is possible, which means that the above condition
> could hold, even on x86.
I see. Ah, that's a mind bender.
> One of the uses of spin_unlock_wait() is in ata_scsi_cmd_error_handler()
> in the file drivers/ata/libata-eh.c. Your commit ad9e27624479b
> ("libata-eh-fw: update ata_scsi_error() for new EH") last touched it,
> though it predates that commit.
>
> My question to you is whether the code in ata_scsi_cmd_error_handler()
> needs release semantics. If it does, my recommendation is to replace
> the spin_unlock_wait(ap->lock) with this (adding the needed curly braces,
> of course):
>
> spin_lock(ap->lock);
> spin_unlock(ap->lock);
>
> If the code only needs acquire semantics, no change required.
>
> If your code requires release semantics, and there is some reason why
> my suggested replacement above is a bad idea, please let me know!
That part of the code should be dead now. I don't think we no longer
have any driver which doesn't have error handler set. I should rip
out that if/else. Also, ACQUIRE semantics should be enough there.
Nothing changes from the EH side there.
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
