lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e18f571-d848-84a-13e5-47315c2a5f5@telegraphics.com.au>
Date:   Tue, 9 Feb 2021 16:11:40 +1100 (AEDT)
From:   Finn Thain <fthain@...egraphics.com.au>
To:     tanxiaofei <tanxiaofei@...wei.com>
cc:     jejb@...ux.ibm.com, martin.petersen@...cle.com,
        linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        linuxarm@...neuler.org, linux-m68k@...r.kernel.org
Subject: Re: [PATCH for-next 00/32] spin lock usage optimization for SCSI
 drivers

On Tue, 9 Feb 2021, tanxiaofei wrote:

> Hi Finn,
> Thanks for reviewing the patch set.
> 
> On 2021/2/8 15:57, Finn Thain wrote:
> > On Sun, 7 Feb 2021, Xiaofei Tan wrote:
> > 
> > > Replace spin_lock_irqsave with spin_lock in hard IRQ of SCSI drivers.
> > > There are no function changes, but may speed up if interrupt happen too
> > > often.
> > 
> > This change doesn't necessarily work on platforms that support nested
> > interrupts.
> > 
> 
> Linux doesn't support nested interrupts anymore after the following 
> patch, so please don't worry this.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e58aa3d2d0cc
> 

Clearly that patch did not disable interrupts. It removed a statement that 
enabled them.

> > Were you able to measure any benefit from this change on some other 
> > platform?
> > 
> 
> It's hard to measure the benefit of this change. 

It's hard to see any benefit. But it's easy to see risk, when there's no 
indication that you've confirmed that the affected drivers do not rely on 
the irq lock, nor tested them for regressions, nor checked whether the 
affected platforms meet your assumuptions.

> Hmm, you could take this patch set as cleanup. thanks.
> 

A "cleanup" does not change program behaviour. Can you demonstrate that 
program behaviour is unchanged?

> > Please see also,
> > https://lore.kernel.org/linux-scsi/89c5cb05cb844939ae684db0077f675f@h3c.com/
> > 
> > .
> > 
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ