linux-kernel - Re: [RESEND] serial: 8250_bcm7271: move spin_lock_irqsave to spin

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] [day] [month] [year] [list]

Message-ID: <YxSj4wExrNxUxlEU@kroah.com>
Date:   Sun, 4 Sep 2022 15:10:59 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     tuo cao <91tuocao@...il.com>
Cc:     alcooperx@...il.com, bcm-kernel-feedback-list@...adcom.com,
        jirislaby@...nel.org, linux-serial@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RESEND] serial: 8250_bcm7271: move spin_lock_irqsave to
 spin_lock in interrupt handler

On Sun, Sep 04, 2022 at 04:48:13PM +0800, tuo cao wrote:
> Greg KH <gregkh@...uxfoundation.org> 于2022年8月30日周二 19:35写道：
> >
> > On Sat, Aug 27, 2022 at 05:42:19PM +0800, tuo cao wrote:
> > > No, whether it's spin_lock_irqsave() or spin_lock(), the security is
> > > the same. Since this commit:e58aa3d2d0cc01ad8d6f7f640a0670433f794922,
> > > interrupt nesting is disabled, which means interrupts has disabled in
> > > the interrupt handlers. So, it is unnecessary to call
> > > spin_lock_irqsave in a interrupt handler. And it takes less time
> > > obviously to use spin_lock()，so I think this change is needed.
> >
> > I have no context at all here, please never top-post :(
> >
> Sorry for causing you trouble. It should be OK this time.
> 
> > And have you measured the time difference?  Is it a real thing?
> >
> Yes, sir. I have measured it, it is a read thing. The test code and
> log have been put on Github, please check:
> https://github.com/tuocao1991/api_test

Did you test it for this code change?

And remember, those calls are being made inside of an IRQ handler, did
you measure that time difference?

And that link does not show much, sorry, you are doing no real work at
all, and again, not operating in an irq handler.

Can you see a measurable difference with your patch applied and without
it?  If so, great, provide that informatin in the changelog text.  If
not, be very careful about changing code in stuff like this.

thanks,

greg k-h