On Thu, Jun 14, 2018 at 7:30 AM, Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote: > On 2018-06-12 08:46:38 [-0700], Dan Williams wrote: >> On Tue, Jun 12, 2018 at 8:04 AM, John Garry <john.garry@xxxxxxxxxx> wrote: >> >> We had this comment for 6 years or so and nothing happend. What makes >> >> you think that an updated version of that comment will motivate someone >> >> to make change here in the near future? >> > >> > Updating the comment is not in itself something to motivate someone to >> > change, but we should keep the comment reasonably accurate or get rid. >> > >> >> It looks to me like a stale comment which won't change a thing because >> >> it does not point out the benefit of doing so (re-enabling interrupts >> >> while dropping the lock) and the price, that is paid for not doing so >> >> (keeping the code as it is) is small enough to not bother. >> >> >> >> So if updating the comment as suggested instead of keeping it as-is or >> >> removing it is the blocker *here* then I can send an updated version. >> >> Any comments? >> > >> > >> > I'd prefer an updated comment. >> > >> >> I think we should try to remove the unlock completely. I agree with >> Sebastian that the audit is never coming. As it is libsas is the only >> ata_port_operations implementation that drops the host_lock while >> running ->qc_issue(). > > Dan, so in meantime I update the comment in patch #1 [0] to say "we > should try to remove unlock completely"? > > [0] https://lkml.kernel.org/r/20180611144053.18294-2-bigeasy@xxxxxxxxxxxxx > Up to you, I'd ack that patch either way... I just wanted to propose the idea that unlock/relock flows tend to have hidden bugs and would be worthwhile to revisit why libsas needs that arrangement.
