Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1676363

Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for storing hotkey scancodes

From Michał Kępień <kernel@kempniu.pl>
Newsgroups linux.kernel
Subject Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for storing hotkey scancodes
Date 2017-06-28 06:40 +0200
Message-ID <tXcVH-27M-5@gated-at.bofh.it> (permalink)
References <tSRmN-3Ii-3@gated-at.bofh.it> <tSRmN-3Ii-9@gated-at.bofh.it> <tUSop-1kz-3@gated-at.bofh.it> <tVHhf-K6-1@gated-at.bofh.it> <tWMeS-X0-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


> On Sat, Jun 24, 2017 at 02:25:46AM +0200, Rafael Wysocki wrote:
> > On Wednesday, June 21, 2017 11:15:43 AM Darren Hart wrote:
> > > On Fri, Jun 16, 2017 at 06:40:52AM +0200, Michał Kępień wrote:
> > > > All ACPI device notify callbacks are invoked using acpi_os_execute(),
> > > > which causes the supplied callback to be queued to a static workqueue
> > > > which always executes on CPU 0.  This means that there is no possibility
> > > > for any ACPI device notify callback to be concurrently executed on
> > > > multiple CPUs, which in the case of fujitsu-laptop means that using a
> > > > locked kfifo for handling hotkeys is redundant: as hotkey scancodes are
> > > > only pushed and popped from within acpi_fujitsu_laptop_notify(), no risk
> > > > of concurrent pushing and popping exists.
> > > 
> > > Was the kfifo causing a problem currently or for the migration to separate
> > > modules? Is this purely a simplification?
> > > 
> > > Rafael, the above rationale appears sound to me. Do you have any concerns?
> > 
> > I actually do.
> > 
> > While this is the case today, making the driver code depend on it in a hard way
> > sort of makes it difficult to change in the future if need be.
> 
> OK, if we aren't guaranteed for this to run on CPU 0 in the future, and this
> will be annoying to debug if it does changes, let's skip the kfifo change.
> 
> I have removed this patch, and fixed up the merge conflicts of the remaining 6
> patches here:
> 
> http://git.infradead.org/linux-platform-drivers-x86.git/shortlog/refs/heads/fujitsu
> 
> Michal / Jonathan, would you please review and let me know if this is what you
> would have done / approve the rebase?

The only issue I can see is that the push/pop debug messages in the last
patch contain the word "buffer" instead of the original "ringbuffer".
The dropped kfifo patch changed the wording from "ringbuffer" to
"buffer" as applying it means there is no ringbuffer any more, but since
it was not applied in the end, I guess the original wording should stay
in place.

The rest looks good to me.

-- 
Best regards,
Michał Kępień

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Darren Hart <dvhart@infradead.org> - 2017-06-21 20:20 +0200
  Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Jonathan Woithe <jwoithe@just42.net> - 2017-06-22 02:00 +0200
    Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Darren Hart <dvhart@infradead.org> - 2017-06-22 04:50 +0200
      Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Jonathan Woithe <jwoithe@just42.net> - 2017-06-22 05:10 +0200
        Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Michał Kępień <kernel@kempniu.pl> - 2017-06-22 22:50 +0200
          Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Darren Hart <dvhart@infradead.org> - 2017-06-23 02:00 +0200
            Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Jonathan Woithe <jwoithe@just42.net> - 2017-06-23 02:20 +0200
              Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Darren Hart <dvhart@infradead.org> - 2017-06-23 08:00 +0200
  Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Michał Kępień <kernel@kempniu.pl> - 2017-06-22 22:10 +0200
  Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for storing hotkey scancodes "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-24 02:40 +0200
    Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Darren Hart <dvhart@infradead.org> - 2017-06-27 02:10 +0200
      Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Jonathan Woithe <jwoithe@just42.net> - 2017-06-27 14:20 +0200
      Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Michał Kępień <kernel@kempniu.pl> - 2017-06-28 06:40 +0200
        Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for  storing hotkey scancodes Darren Hart <dvhart@infradead.org> - 2017-06-28 18:10 +0200

csiph-web