On 2018-06-11 18:12:55 [+0100], John Garry wrote: > On 11/06/2018 15:40, Sebastian Andrzej Siewior wrote: > > This is the repost of the two patches I posted in earlier this month: > > > > - [PATCH 1/2] libsas: remove irq save in sas_ata_qc_issue() > > Received feedback but nothing really changed. I explained that this is > > not about "local_irqsave() + spin_lock()" *but* "local_irq_save() + > > spin_unlock()". This seemed to have been overseen twice. > > Also there were two opinions about the TODO comment: > > /* TODO: audit callers to ensure they are ready for qc_issue to > > * unconditionally re-enable interrupts > > It is unclear to me if this comment should be removed because it > > makes no sense or if the intention was indeed to audit callers code > > for a possible "spin_unlock_irq(ap->lock);". > > Hi, > > As I said previously, since it is not clear now what the comment meant, then > removing the irq save/restore calls will only make it even less clear, and > should be fixed. how should it be fixed? The discussion went forth and back. The comment as of now does not match the code. It disables interrupts which are already disabled. It does not enable them at any point. > Cheers, > John Sebastian
