Re: [PATCH] mm/swap: add function get_total_swap_pages to expose total_swap_pages

[Michal Hocko]

On Tue 30-01-18 02:56:51, He, Roger wrote:
> Hi Michal:
> 
> We need a API to tell TTM module the system totally has how many swap
> cache.  Then TTM module can use it to restrict how many the swap cache
> it can use to prevent triggering OOM.  For Now we set the threshold of
> swap size TTM used as 1/2 * total size and leave the rest for others
> use.

Why do you so much memory? Are you going to use TB of memory on large
systems? What about memory hotplug when the memory is added/released?
 
> But get_nr_swap_pages is the only API we can accessed from other
> module now.  It can't cover the case of the dynamic swap size
> increment.  I mean: user can use "swapon" to enable new swap file or
> swap disk dynamically or "swapoff" to disable swap space.

Exactly. Your scaling configuration based on get_nr_swap_pages or the
available memory simply sounds wrong.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm/swap: add function get_total_swap_pages to expose total_swap_pages

[Michal Hocko]

On Tue 30-01-18 02:56:51, He, Roger wrote:
> Hi Michal:
> 
> We need a API to tell TTM module the system totally has how many swap
> cache.  Then TTM module can use it to restrict how many the swap cache
> it can use to prevent triggering OOM.  For Now we set the threshold of
> swap size TTM used as 1/2 * total size and leave the rest for others
> use.

Why do you so much memory? Are you going to use TB of memory on large
systems? What about memory hotplug when the memory is added/released?
 
> But get_nr_swap_pages is the only API we can accessed from other
> module now.  It can't cover the case of the dynamic swap size
> increment.  I mean: user can use "swapon" to enable new swap file or
> swap disk dynamically or "swapoff" to disable swap space.

Exactly. Your scaling configuration based on get_nr_swap_pages or the
available memory simply sounds wrong.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

[Maxime Ripard]

Hi,

On Mon, Jan 29, 2018 at 03:34:02PM +0100, Arnd Bergmann wrote:
> On Mon, Jan 29, 2018 at 10:25 AM, Linus Walleij
>  wrote:
> > On Mon, Jan 29, 2018 at 9:25 AM, Maxime Ripard
> >  wrote:
> >> On Sat, Jan 27, 2018 at 05:14:26PM +0100, Linus Walleij wrote:
> >> However, in DT systems, that
> >> field is filled only with the parent's node dma-ranges property. In
> >> our case, and since the DT parenthood is based on the "control" bus,
> >> and not the "data" bus, our parent node would be the AXI bus, and not
> >> the memory bus that enforce those constraints.
> >>
> >> And other devices doing DMA through regular DMA accesses won't have
> >> that mapping, so we definitely shouldn't enforce it for all the
> >> devices there, but only the one connected to the separate memory bus.
> >>
> >> tl; dr: the DT is not really an option to store that info.
> >>
> >> I suggested setting dma_pfn_offset at probe, but Arnd didn't seem too
> >> fond of that approach either at the time.
> >>
> >> So, well, I guess we could do better. We just have no idea how :)
> >
> > Usually of Arnd cannot suggest something smart, neither can I :(
> >
> > If some aspect of the memory layout of the system *really* doesn't
> > fit into DT because of the assumptions that DT is doing about
> > how systems work, we have a problem.
> >
> > Is the problem that DT's model assumes that devices have one and
> > only one data port to the system memory, and that is how it
> > populates the Linux devices?
> >
> > I guess, if nothing else works, I would use auxdata in the board file.
> > It is our definitive last resort when DT doesn't hold.
> >
> > I.e. I would go into struct of_dev_auxdata
> > (from ) and add a
> > dma_pfn_offset field that gets set into the device's dma_pfn_offset
> > in drivers/of/platform.c overriding anything else and use that to hammer
> > it down in the boardfile.
> >
> > But I bet some DT people are going to have other ideas.
> 
> At one point we had discussed adding a 'dma-masters' property that
> lists all the buses on which a device can be a dma master, and
> the respective properties of those masters (iommu, coherency,
> offset, ...).
>
> IIRC at the time we decided that we could live without that complexity,
> but perhaps we cannot.

Are you talking about this ?
https://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/dma/dma.txt#L41

It doesn't seem to be related to that issue to me. And in our
particular cases, all the devices are DMA masters, the RAM is just
mapped to another address.

> Another local hack that we can do here is to have two separate
> device nodes and let the device driver bind to one device and then
> look up the other one through a phandle to look up a platform_device
> for it, which it can then use with the DMA mapping interface.

We have multiple devices doing that, a couple of them already merged
(mostly on the display side). I'd rather not do that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

[Maxime Ripard]

Hi,

On Mon, Jan 29, 2018 at 03:34:02PM +0100, Arnd Bergmann wrote:
> On Mon, Jan 29, 2018 at 10:25 AM, Linus Walleij
>  wrote:
> > On Mon, Jan 29, 2018 at 9:25 AM, Maxime Ripard
> >  wrote:
> >> On Sat, Jan 27, 2018 at 05:14:26PM +0100, Linus Walleij wrote:
> >> However, in DT systems, that
> >> field is filled only with the parent's node dma-ranges property. In
> >> our case, and since the DT parenthood is based on the "control" bus,
> >> and not the "data" bus, our parent node would be the AXI bus, and not
> >> the memory bus that enforce those constraints.
> >>
> >> And other devices doing DMA through regular DMA accesses won't have
> >> that mapping, so we definitely shouldn't enforce it for all the
> >> devices there, but only the one connected to the separate memory bus.
> >>
> >> tl; dr: the DT is not really an option to store that info.
> >>
> >> I suggested setting dma_pfn_offset at probe, but Arnd didn't seem too
> >> fond of that approach either at the time.
> >>
> >> So, well, I guess we could do better. We just have no idea how :)
> >
> > Usually of Arnd cannot suggest something smart, neither can I :(
> >
> > If some aspect of the memory layout of the system *really* doesn't
> > fit into DT because of the assumptions that DT is doing about
> > how systems work, we have a problem.
> >
> > Is the problem that DT's model assumes that devices have one and
> > only one data port to the system memory, and that is how it
> > populates the Linux devices?
> >
> > I guess, if nothing else works, I would use auxdata in the board file.
> > It is our definitive last resort when DT doesn't hold.
> >
> > I.e. I would go into struct of_dev_auxdata
> > (from ) and add a
> > dma_pfn_offset field that gets set into the device's dma_pfn_offset
> > in drivers/of/platform.c overriding anything else and use that to hammer
> > it down in the boardfile.
> >
> > But I bet some DT people are going to have other ideas.
> 
> At one point we had discussed adding a 'dma-masters' property that
> lists all the buses on which a device can be a dma master, and
> the respective properties of those masters (iommu, coherency,
> offset, ...).
>
> IIRC at the time we decided that we could live without that complexity,
> but perhaps we cannot.

Are you talking about this ?
https://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/dma/dma.txt#L41

It doesn't seem to be related to that issue to me. And in our
particular cases, all the devices are DMA masters, the RAM is just
mapped to another address.

> Another local hack that we can do here is to have two separate
> device nodes and let the device driver bind to one device and then
> look up the other one through a phandle to look up a platform_device
> for it, which it can then use with the DMA mapping interface.

We have multiple devices doing that, a couple of them already merged
(mostly on the display side). I'd rather not do that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [netfilter-core] kernel panic: Out of memory and no killable processes... (2)

[Michal Hocko]

On Mon 29-01-18 23:35:22, Florian Westphal wrote:
> Kirill A. Shutemov  wrote:
[...]
> > I hate what I'm saying, but I guess we need some tunable here.
> > Not sure what exactly.
> 
> Would memcg help?

That really depends. I would have to check whether vmalloc path obeys
__GFP_ACCOUNT (I suspect it does except for page tables allocations but
that shouldn't be a big deal). But then the other potential problem is
the life time of the xt_table_info (or other potentially large) data
structures. Are they bound to any process life time. Because if they are
not then the OOM killer will not help. The OOM panic earlier in this
thread suggests it doesn't because the test case managed to eat all the
available memory and killed all the eligible tasks which didn't help.

So in some sense the memcg would help to stop the excessive allocation,
but it wouldn't resolve it other than kill all tasks in the affected
memcg/container. Whether this is sufficient or not, I dunno. It sounds
quite suboptimal to me. But it is true this would be less tricky then
adding a global knob...
-- 
Michal Hocko
SUSE Labs

Re: [netfilter-core] kernel panic: Out of memory and no killable processes... (2)

[Michal Hocko]

On Mon 29-01-18 23:35:22, Florian Westphal wrote:
> Kirill A. Shutemov  wrote:
[...]
> > I hate what I'm saying, but I guess we need some tunable here.
> > Not sure what exactly.
> 
> Would memcg help?

That really depends. I would have to check whether vmalloc path obeys
__GFP_ACCOUNT (I suspect it does except for page tables allocations but
that shouldn't be a big deal). But then the other potential problem is
the life time of the xt_table_info (or other potentially large) data
structures. Are they bound to any process life time. Because if they are
not then the OOM killer will not help. The OOM panic earlier in this
thread suggests it doesn't because the test case managed to eat all the
available memory and killed all the eligible tasks which didn't help.

So in some sense the memcg would help to stop the excessive allocation,
but it wouldn't resolve it other than kill all tasks in the affected
memcg/container. Whether this is sufficient or not, I dunno. It sounds
quite suboptimal to me. But it is true this would be less tricky then
adding a global knob...
-- 
Michal Hocko
SUSE Labs

[PATCH] RISC-V: Enable IRQ during exception handling

[Zong Li]

Interrupt is allowed during exception handling.
There are warning messages if the kernel enables the configuration
'CONFIG_DEBUG_ATOMIC_SLEEP=y'.

BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:23
in_atomic(): 0, irqs_disabled(): 1, pid: 43, name: ash
CPU: 0 PID: 43 Comm: ash Tainted:  GW
4.15.0-rc8-00089-g89ffdae-dirty #17
Call Trace:
[<9abb1587>] walk_stackframe+0x0/0x7a
[] ___might_sleep+0x102/0x11a
[] down_read+0x18/0x28
[<0289ec01>] do_page_fault+0x86/0x2f6
[<012441f6>] _do_fork+0x1b4/0x1e0
[] ret_from_syscall+0xa/0xe

Signed-off-by: Zong Li 
---
 arch/riscv/kernel/entry.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 7404ec2..61f063e 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -169,6 +169,9 @@ ENTRY(handle_exception)
move a1, sp /* pt_regs */
tail do_IRQ
 1:
+   /* Exceptions run with interrupts enabled */
+   csrs sstatus, SR_SIE
+
/* Handle syscalls */
li t0, EXC_SYSCALL
beq s4, t0, handle_syscall
@@ -195,8 +198,6 @@ handle_syscall:
 */
addi s2, s2, 0x4
REG_S s2, PT_SEPC(sp)
-   /* System calls run with interrupts enabled */
-   csrs sstatus, SR_SIE
/* Trace syscalls, but only if requested by the user. */
REG_L t0, TASK_TI_FLAGS(tp)
andi t0, t0, _TIF_SYSCALL_TRACE
-- 
2.9.3

[PATCH] RISC-V: Enable IRQ during exception handling

[Zong Li]

Interrupt is allowed during exception handling.
There are warning messages if the kernel enables the configuration
'CONFIG_DEBUG_ATOMIC_SLEEP=y'.

BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:23
in_atomic(): 0, irqs_disabled(): 1, pid: 43, name: ash
CPU: 0 PID: 43 Comm: ash Tainted:  GW
4.15.0-rc8-00089-g89ffdae-dirty #17
Call Trace:
[<9abb1587>] walk_stackframe+0x0/0x7a
[] ___might_sleep+0x102/0x11a
[] down_read+0x18/0x28
[<0289ec01>] do_page_fault+0x86/0x2f6
[<012441f6>] _do_fork+0x1b4/0x1e0
[] ret_from_syscall+0xa/0xe

Signed-off-by: Zong Li 
---
 arch/riscv/kernel/entry.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 7404ec2..61f063e 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -169,6 +169,9 @@ ENTRY(handle_exception)
move a1, sp /* pt_regs */
tail do_IRQ
 1:
+   /* Exceptions run with interrupts enabled */
+   csrs sstatus, SR_SIE
+
/* Handle syscalls */
li t0, EXC_SYSCALL
beq s4, t0, handle_syscall
@@ -195,8 +198,6 @@ handle_syscall:
 */
addi s2, s2, 0x4
REG_S s2, PT_SEPC(sp)
-   /* System calls run with interrupts enabled */
-   csrs sstatus, SR_SIE
/* Trace syscalls, but only if requested by the user. */
REG_L t0, TASK_TI_FLAGS(tp)
andi t0, t0, _TIF_SYSCALL_TRACE
-- 
2.9.3

Re: [PATCHv1] Add Intel Stratix10 service layer driver

[Greg KH]

On Mon, Jan 29, 2018 at 08:08:11PM -0600, Richard Gong wrote:
> Hi Greg,
> 
> Many thanks for your reviews.
> 
> 
> On 01/25/2018 10:53 AM, Greg KH wrote:
> > On Thu, Jan 25, 2018 at 10:39:03AM -0600, richard.g...@linux.intel.com 
> > wrote:
> > > From: Richard Gong 
> > > 
> > > Intel Stratix10 SoC is composed of a 64 bit quad-core ARM Cortex A53 hard
> > > processor system (HPS) and Secure Device Manager (SDM). SDM is the 
> > > hardware
> > > which does the FPGA configuration, QSPI, Crypto and warm reset.
> > > 
> > > When the FPGA is configured from HPS, there needs to be a way for HPS to
> > > notify SDM the location and size of the configuration data. Then SDM will
> > > get the configuration data from that location and perform the FPGA 
> > > configuration.
> > > 
> > > To meet the whole system security needs and support virtual machine
> > > requesting communication with SDM, only the secure world of software (EL3,
> > > Exception Level 3) can interface with SDM. All software entities running
> > > on other exception levels must channel through the EL3 software whenever 
> > > it
> > > needs service from SDM.
> > > 
> > > Intel Stratix10 service layer driver is added to provide the service for
> > > FPGA configuration. Running at privileged exception level (EL1, Exception
> > > Level 1), Intel Stratix10 service layer driver interfaces with the service
> > > provider at EL1 (Intel Stratix10 FPGA Manager) and manages secure monitor
> > > call (SMC) to communicate with secure monitor software at secure monitor
> > > exception level (EL3).
> > > 
> > > Later the Intel Stratix10 service layer driver will be extended to provide
> > > services for QSPI, Crypto and warm reset.
> > > 
> > > Richard Gong (1):
> > >driver: misc: add Intel Stratix10 service layer driver
> > > 
> > >   drivers/misc/Kconfig   |   3 +-
> > >   drivers/misc/Makefile  |   3 +-
> > >   drivers/misc/intel-service/Kconfig |   9 +
> > >   drivers/misc/intel-service/Makefile|   2 +
> > >   drivers/misc/intel-service/intel_service.c | 703 
> > > +
> > >   include/linux/intel-service-client.h   | 227 ++
> > >   include/linux/intel-service.h  | 122 +
> > >   include/linux/intel-smc.h  | 246 ++
> > Simple questions first:
> >   - why do you have 3 different .h files for a single .c file?
> This is because service layer driver interface with both the service
> provider and secure monitor SW.
> intel-service-client.h is created to define interface between service
> providers (FPGA manager is one of them) and service layer. Alan Tull's FPGA
> manager .c file includes this header file
> intel-smc.h defines the secure monitor call (SMC) message protocols used for
> service layer driver in normal world (EL1) to communicate with secure
> monitor SW in secure monitor exception level 3 (EL3). Also this header file
> is shared with firmware since both (FW, service layer) utilizes the same SMC
> message protocol.
> intel-sevice.h is created to define service layer's own data structures
> (service controller, channel for communicating with service provider, shared
> memory region, private data etc)

That's very complex for a single patch submission, don't you think?

Please do not add new apis / interfaces for code that is not part of
your patch series, otherwise we don't know what the future is going to
hold :)

This feels like it should be a series of patches, to properly explain
this and hook up all of the new interfaces you are adding, right?

thanks,

greg k-h

Re: [PATCHv1] Add Intel Stratix10 service layer driver

[Greg KH]

On Mon, Jan 29, 2018 at 08:08:11PM -0600, Richard Gong wrote:
> Hi Greg,
> 
> Many thanks for your reviews.
> 
> 
> On 01/25/2018 10:53 AM, Greg KH wrote:
> > On Thu, Jan 25, 2018 at 10:39:03AM -0600, richard.g...@linux.intel.com 
> > wrote:
> > > From: Richard Gong 
> > > 
> > > Intel Stratix10 SoC is composed of a 64 bit quad-core ARM Cortex A53 hard
> > > processor system (HPS) and Secure Device Manager (SDM). SDM is the 
> > > hardware
> > > which does the FPGA configuration, QSPI, Crypto and warm reset.
> > > 
> > > When the FPGA is configured from HPS, there needs to be a way for HPS to
> > > notify SDM the location and size of the configuration data. Then SDM will
> > > get the configuration data from that location and perform the FPGA 
> > > configuration.
> > > 
> > > To meet the whole system security needs and support virtual machine
> > > requesting communication with SDM, only the secure world of software (EL3,
> > > Exception Level 3) can interface with SDM. All software entities running
> > > on other exception levels must channel through the EL3 software whenever 
> > > it
> > > needs service from SDM.
> > > 
> > > Intel Stratix10 service layer driver is added to provide the service for
> > > FPGA configuration. Running at privileged exception level (EL1, Exception
> > > Level 1), Intel Stratix10 service layer driver interfaces with the service
> > > provider at EL1 (Intel Stratix10 FPGA Manager) and manages secure monitor
> > > call (SMC) to communicate with secure monitor software at secure monitor
> > > exception level (EL3).
> > > 
> > > Later the Intel Stratix10 service layer driver will be extended to provide
> > > services for QSPI, Crypto and warm reset.
> > > 
> > > Richard Gong (1):
> > >driver: misc: add Intel Stratix10 service layer driver
> > > 
> > >   drivers/misc/Kconfig   |   3 +-
> > >   drivers/misc/Makefile  |   3 +-
> > >   drivers/misc/intel-service/Kconfig |   9 +
> > >   drivers/misc/intel-service/Makefile|   2 +
> > >   drivers/misc/intel-service/intel_service.c | 703 
> > > +
> > >   include/linux/intel-service-client.h   | 227 ++
> > >   include/linux/intel-service.h  | 122 +
> > >   include/linux/intel-smc.h  | 246 ++
> > Simple questions first:
> >   - why do you have 3 different .h files for a single .c file?
> This is because service layer driver interface with both the service
> provider and secure monitor SW.
> intel-service-client.h is created to define interface between service
> providers (FPGA manager is one of them) and service layer. Alan Tull's FPGA
> manager .c file includes this header file
> intel-smc.h defines the secure monitor call (SMC) message protocols used for
> service layer driver in normal world (EL1) to communicate with secure
> monitor SW in secure monitor exception level 3 (EL3). Also this header file
> is shared with firmware since both (FW, service layer) utilizes the same SMC
> message protocol.
> intel-sevice.h is created to define service layer's own data structures
> (service controller, channel for communicating with service provider, shared
> memory region, private data etc)

That's very complex for a single patch submission, don't you think?

Please do not add new apis / interfaces for code that is not part of
your patch series, otherwise we don't know what the future is going to
hold :)

This feels like it should be a series of patches, to properly explain
this and hook up all of the new interfaces you are adding, right?

thanks,

greg k-h

Re: [PATCH 3.18 00/52] 3.18.93-stable review

[Greg Kroah-Hartman]

On Mon, Jan 29, 2018 at 04:58:05PM -0700, Shuah Khan wrote:
> On 01/29/2018 05:56 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 3.18.93 release.
> > There are 52 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Wed Jan 31 12:36:07 UTC 2018.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.93-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-3.18.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Compiled and booted on my test system. No dmesg regressions.

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 3.18 00/52] 3.18.93-stable review

[Greg Kroah-Hartman]

On Tue, Jan 30, 2018 at 05:09:07AM +, Harsh Shandilya wrote:
> On Tue 30 Jan, 2018, 2:20 AM Greg Kroah-Hartman, 
> wrote:
> 
> > This is the start of the stable review cycle for the 3.18.93 release.
> > There are 52 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Wed Jan 31 12:36:07 UTC 2018.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> >
> > kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.93-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > linux-3.18.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
> >
> 
> Builds and boots on the OnePlus 3, no dmesg or userspace regressions.

Yeah, it's still working!  :)

thanks for testing and letting me know.

greg k-h

Re: [PATCH 3.18 00/52] 3.18.93-stable review

[Greg Kroah-Hartman]

On Mon, Jan 29, 2018 at 04:58:05PM -0700, Shuah Khan wrote:
> On 01/29/2018 05:56 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 3.18.93 release.
> > There are 52 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Wed Jan 31 12:36:07 UTC 2018.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.93-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-3.18.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Compiled and booted on my test system. No dmesg regressions.

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 3.18 00/52] 3.18.93-stable review

[Greg Kroah-Hartman]

On Tue, Jan 30, 2018 at 05:09:07AM +, Harsh Shandilya wrote:
> On Tue 30 Jan, 2018, 2:20 AM Greg Kroah-Hartman, 
> wrote:
> 
> > This is the start of the stable review cycle for the 3.18.93 release.
> > There are 52 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Wed Jan 31 12:36:07 UTC 2018.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> >
> > kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.93-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > linux-3.18.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
> >
> 
> Builds and boots on the OnePlus 3, no dmesg or userspace regressions.

Yeah, it's still working!  :)

thanks for testing and letting me know.

greg k-h

Re: [PATCH 4.4 00/74] 4.4.114-stable review

[Greg Kroah-Hartman]

On Mon, Jan 29, 2018 at 02:30:53PM -0700, Nathan Chancellor wrote:
> On Mon, Jan 29, 2018 at 01:56:05PM +0100, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.4.114 release.
> > There are 74 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Wed Jan 31 12:38:21 UTC 2018.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.114-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.4.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> >
> 
> Merged, compiled, and flashed onto my Pixel 2 XL and OnePlus 5.
> 
> No conflicts, no initial issues noticed in dmesg or general usage.
> 
> Reference trees for Android: https://github.com/android-linux-stable

Very nice, thanks for testing and letting me know.

greg k-h

Re: [PATCH 4.4 00/74] 4.4.114-stable review

[Greg Kroah-Hartman]

On Mon, Jan 29, 2018 at 02:30:53PM -0700, Nathan Chancellor wrote:
> On Mon, Jan 29, 2018 at 01:56:05PM +0100, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.4.114 release.
> > There are 74 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Wed Jan 31 12:38:21 UTC 2018.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.114-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.4.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> >
> 
> Merged, compiled, and flashed onto my Pixel 2 XL and OnePlus 5.
> 
> No conflicts, no initial issues noticed in dmesg or general usage.
> 
> Reference trees for Android: https://github.com/android-linux-stable

Very nice, thanks for testing and letting me know.

greg k-h

Re: [PATCH 8/8] platform: vivid-cec: fix potential integer overflow in vivid_cec_pin_adap_events

[Hans Verkuil]

Hi Gustavo,

On 01/30/2018 01:33 AM, Gustavo A. R. Silva wrote:
> Cast len to const u64 in order to avoid a potential integer
> overflow. This variable is being used in a context that expects
> an expression of type const u64.
> 
> Addresses-Coverity-ID: 1454996 ("Unintentional integer overflow")
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/media/platform/vivid/vivid-cec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vivid/vivid-cec.c 
> b/drivers/media/platform/vivid/vivid-cec.c
> index b55d278..30240ab 100644
> --- a/drivers/media/platform/vivid/vivid-cec.c
> +++ b/drivers/media/platform/vivid/vivid-cec.c
> @@ -83,7 +83,7 @@ static void vivid_cec_pin_adap_events(struct cec_adapter 
> *adap, ktime_t ts,
>   if (adap == NULL)
>   return;
>   ts = ktime_sub_us(ts, (CEC_TIM_START_BIT_TOTAL +
> -len * 10 * CEC_TIM_DATA_BIT_TOTAL));
> +(const u64)len * 10 * CEC_TIM_DATA_BIT_TOTAL));

This makes no sense. Certainly the const part is pointless. And given that
len is always <= 16 there definitely is no overflow.

I don't really want this cast in the code.

Sorry,

Hans

>   cec_queue_pin_cec_event(adap, false, ts);
>   ts = ktime_add_us(ts, CEC_TIM_START_BIT_LOW);
>   cec_queue_pin_cec_event(adap, true, ts);
> 

Re: [PATCH 8/8] platform: vivid-cec: fix potential integer overflow in vivid_cec_pin_adap_events

[Hans Verkuil]

Hi Gustavo,

On 01/30/2018 01:33 AM, Gustavo A. R. Silva wrote:
> Cast len to const u64 in order to avoid a potential integer
> overflow. This variable is being used in a context that expects
> an expression of type const u64.
> 
> Addresses-Coverity-ID: 1454996 ("Unintentional integer overflow")
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/media/platform/vivid/vivid-cec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vivid/vivid-cec.c 
> b/drivers/media/platform/vivid/vivid-cec.c
> index b55d278..30240ab 100644
> --- a/drivers/media/platform/vivid/vivid-cec.c
> +++ b/drivers/media/platform/vivid/vivid-cec.c
> @@ -83,7 +83,7 @@ static void vivid_cec_pin_adap_events(struct cec_adapter 
> *adap, ktime_t ts,
>   if (adap == NULL)
>   return;
>   ts = ktime_sub_us(ts, (CEC_TIM_START_BIT_TOTAL +
> -len * 10 * CEC_TIM_DATA_BIT_TOTAL));
> +(const u64)len * 10 * CEC_TIM_DATA_BIT_TOTAL));

This makes no sense. Certainly the const part is pointless. And given that
len is always <= 16 there definitely is no overflow.

I don't really want this cast in the code.

Sorry,

Hans

>   cec_queue_pin_cec_event(adap, false, ts);
>   ts = ktime_add_us(ts, CEC_TIM_START_BIT_LOW);
>   cec_queue_pin_cec_event(adap, true, ts);
> 

Re: [PATCH] PCI: Add SPDX GPL-2.0+ to replace implicit GPL v2 or later statement

[Greg Kroah-Hartman]

On Mon, Jan 29, 2018 at 06:40:52PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas 
> 
> 7441b0627e22 ("s390/pci: PCI hotplug support via SCLP") added
> s390_pci_hpc.c, which included this license information:
> 
>   +MODULE_LICENSE("GPL");
> 
> Based on "git show 7441b0627e22:include/linux/module.h", that "GPL" string
> means "GPL v2 or later":
> 
>*  "GPL"   [GNU Public License v2 or later]
> 
> 0729dcf24832 ("s390: hotplug: make pci_hpc explicitly non-modular")
> subsequently replaced the MODULE_LICENSE() with a "License: GPL" comment.
> 
> Add SPDX GPL-2.0+ and remove the "License: GPL" comment, relying on the
> assertion in b24413180f56 ("License cleanup: add SPDX GPL-2.0 license
> identifier to files with no license") that the SPDX identifier may be used
> instead of the full boilerplate text.
> 
> Signed-off-by: Bjorn Helgaas 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH] PCI: Add SPDX GPL-2.0+ to replace implicit GPL v2 or later statement

[Greg Kroah-Hartman]

On Mon, Jan 29, 2018 at 06:40:52PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas 
> 
> 7441b0627e22 ("s390/pci: PCI hotplug support via SCLP") added
> s390_pci_hpc.c, which included this license information:
> 
>   +MODULE_LICENSE("GPL");
> 
> Based on "git show 7441b0627e22:include/linux/module.h", that "GPL" string
> means "GPL v2 or later":
> 
>*  "GPL"   [GNU Public License v2 or later]
> 
> 0729dcf24832 ("s390: hotplug: make pci_hpc explicitly non-modular")
> subsequently replaced the MODULE_LICENSE() with a "License: GPL" comment.
> 
> Add SPDX GPL-2.0+ and remove the "License: GPL" comment, relying on the
> assertion in b24413180f56 ("License cleanup: add SPDX GPL-2.0 license
> identifier to files with no license") that the SPDX identifier may be used
> instead of the full boilerplate text.
> 
> Signed-off-by: Bjorn Helgaas 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence

[Ingo Molnar]

* Dan Williams  wrote:

> > The flip side is that if the MFENCE stalls the STAC that is ahead of it 
> > could be
> > processed for 'free' - while it's always post barrier with my suggestion.
> 
> This 'for free' aspect is what I aiming for.

Ok.

> >
> > But in any case it would be nice to see a discussion of this aspect in the
> > changelog, even if the patch does not change.
> 
> I'll add a note to the changelog that having the fence after the
> 'stac' hopefully allows some overlap of the cost of 'stac' and the
> flushing of the instruction pipeline.

Perfect!

Thanks,

Ingo

Re: [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence

[Ingo Molnar]

* Dan Williams  wrote:

> > The flip side is that if the MFENCE stalls the STAC that is ahead of it 
> > could be
> > processed for 'free' - while it's always post barrier with my suggestion.
> 
> This 'for free' aspect is what I aiming for.

Ok.

> >
> > But in any case it would be nice to see a discussion of this aspect in the
> > changelog, even if the patch does not change.
> 
> I'll add a note to the changelog that having the fence after the
> 'stac' hopefully allows some overlap of the cost of 'stac' and the
> flushing of the instruction pipeline.

Perfect!

Thanks,

Ingo

Re: [PATCH net-next] ptr_ring: fix integer overflow

[Jason Wang]

On 2018年01月30日 01:01, David Miller wrote:

From: Jason Wang 
Date: Thu, 25 Jan 2018 15:31:42 +0800


We try to allocate one more entry for lockless peeking. The adding
operation may overflow which causes zero to be passed to kmalloc().
In this case, it returns ZERO_SIZE_PTR without any notice by ptr
ring. Try to do producing or consuming on such ring will lead NULL
dereference. Fix this detect and fail early.

Fixes: bcecb4bbf88a ("net: ptr_ring: otherwise safe empty checks can overrun array 
bounds")
Reported-by: syzbot+87678bcf753b44c39...@syzkaller.appspotmail.com
Cc: John Fastabend 
Signed-off-by: Jason Wang 

I'm dropping this because I am to understand that Michael Tsirkin's patch
series covers this issue.


Yes.



Let me know if I still need to apply this.

Thanks.


No need for this.

Thanks

Re: [PATCH net-next] ptr_ring: fix integer overflow

[Jason Wang]

On 2018年01月30日 01:01, David Miller wrote:

From: Jason Wang 
Date: Thu, 25 Jan 2018 15:31:42 +0800


We try to allocate one more entry for lockless peeking. The adding
operation may overflow which causes zero to be passed to kmalloc().
In this case, it returns ZERO_SIZE_PTR without any notice by ptr
ring. Try to do producing or consuming on such ring will lead NULL
dereference. Fix this detect and fail early.

Fixes: bcecb4bbf88a ("net: ptr_ring: otherwise safe empty checks can overrun array 
bounds")
Reported-by: syzbot+87678bcf753b44c39...@syzkaller.appspotmail.com
Cc: John Fastabend 
Signed-off-by: Jason Wang 

I'm dropping this because I am to understand that Michael Tsirkin's patch
series covers this issue.


Yes.



Let me know if I still need to apply this.

Thanks.


No need for this.

Thanks

Re: [PATCH 1/1] scsi: ufs-qcom: remove broken hci version quirk

[Asutosh Das (asd)]

On 1/30/2018 11:33 AM, Vivek Gautam wrote:

Hi Asutosh,


On 1/30/2018 10:11 AM, Asutosh Das wrote:

From: Subhash Jadavani 

UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION is only applicable for QCOM UFS host
controller version 2.x.y and this has been fixed from version 3.x.y
onwards, hence this change removes this quirk for version 3.x.y onwards.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Asutosh Das 
---


This patch and all other ufs patches that you have posted recently,
do they all fall under one 'ufs-qcom fixes' patch series for fixes that
we would want to do?
If it is so, then please club them together in a series, so that
it's easy for reviewers and maintainers to review, and keep track
of all the patches that can get-in after the reviews.
If they belong to two or more separate patch-series then please
create such patch series.
It's difficult to read through a lot of [PATCH 1/1] ... patch.

Sure Vivek - Makes sense. Will resend it.



Regards
Vivek


  drivers/scsi/ufs/ufs-qcom.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2..221820a 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1098,7 +1098,7 @@ static void ufs_qcom_advertise_quirks(struct 
ufs_hba *hba)

  hba->quirks |= UFSHCD_QUIRK_BROKEN_LCC;
  }
-    if (host->hw_ver.major >= 0x2) {
+    if (host->hw_ver.major == 0x2) {
  hba->quirks |= UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION;
  if (!ufs_qcom_cap_qunipro(host))





--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH 1/1] scsi: ufs-qcom: remove broken hci version quirk

[Asutosh Das (asd)]

On 1/30/2018 11:33 AM, Vivek Gautam wrote:

Hi Asutosh,


On 1/30/2018 10:11 AM, Asutosh Das wrote:

From: Subhash Jadavani 

UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION is only applicable for QCOM UFS host
controller version 2.x.y and this has been fixed from version 3.x.y
onwards, hence this change removes this quirk for version 3.x.y onwards.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Asutosh Das 
---


This patch and all other ufs patches that you have posted recently,
do they all fall under one 'ufs-qcom fixes' patch series for fixes that
we would want to do?
If it is so, then please club them together in a series, so that
it's easy for reviewers and maintainers to review, and keep track
of all the patches that can get-in after the reviews.
If they belong to two or more separate patch-series then please
create such patch series.
It's difficult to read through a lot of [PATCH 1/1] ... patch.

Sure Vivek - Makes sense. Will resend it.



Regards
Vivek


  drivers/scsi/ufs/ufs-qcom.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2..221820a 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1098,7 +1098,7 @@ static void ufs_qcom_advertise_quirks(struct 
ufs_hba *hba)

  hba->quirks |= UFSHCD_QUIRK_BROKEN_LCC;
  }
-    if (host->hw_ver.major >= 0x2) {
+    if (host->hw_ver.major == 0x2) {
  hba->quirks |= UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION;
  if (!ufs_qcom_cap_qunipro(host))





--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH v2] dmaengine: dmatest: fix container_of member in dmatest_callback

[Vinod Koul]

On Mon, Jan 29, 2018 at 02:40:11PM +0800, Yang Shunyong wrote:
> The type of arg passed to dmatest_callback is struct dmatest_done.
> It refers to test_done in struct dmatest_thread, not done_wait.

Applied, thanks

-- 
~Vinod

Re: [PATCH v2] dmaengine: dmatest: fix container_of member in dmatest_callback

[Vinod Koul]

On Mon, Jan 29, 2018 at 02:40:11PM +0800, Yang Shunyong wrote:
> The type of arg passed to dmatest_callback is struct dmatest_done.
> It refers to test_done in struct dmatest_thread, not done_wait.

Applied, thanks

-- 
~Vinod

[PATCH 2/2] x86/mm/64: Add vsyscall page to /proc/kcore conditionally

[Jia Zhang]

The vsyscall page should be visible only if
vsyscall=emulate/native when dumping /proc/kcore.

Signed-off-by: Jia Zhang 
---
 arch/x86/mm/init_64.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index dab78f6..3d4cf33 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1186,8 +1186,9 @@ void __init mem_init(void)
register_page_bootmem_info();
 
/* Register memory areas for /proc/kcore */
-   kclist_add(_vsyscall, (void *)VSYSCALL_ADDR,
-  PAGE_SIZE, KCORE_USER);
+   if (get_gate_vma(_mm))
+   kclist_add(_vsyscall, (void *)VSYSCALL_ADDR,
+  PAGE_SIZE, KCORE_USER);
 
mem_init_print_info(NULL);
 }
-- 
1.8.3.1

[PATCH 1/2] /proc/kcore: Fix SMAP violation when dumping vsyscall user page

[Jia Zhang]

The commit df04abfd181a
("fs/proc/kcore.c: Add bounce buffer for ktext data") introduces a
bounce buffer to work around CONFIG_HARDENED_USERCOPY=y. However,
accessing vsyscall user page will cause SMAP violation in this way.

In order to fix this issue, simply replace memcpy() with copy_from_user()
may work, but using a common way to handle this sort of user page may be
useful for future.

Currently, only vsyscall page requires KCORE_USER.

Signed-off-by: Jia Zhang 
---
 arch/x86/mm/init_64.c | 2 +-
 fs/proc/kcore.c   | 4 
 include/linux/kcore.h | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 4a83728..dab78f6 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1187,7 +1187,7 @@ void __init mem_init(void)
 
/* Register memory areas for /proc/kcore */
kclist_add(_vsyscall, (void *)VSYSCALL_ADDR,
-PAGE_SIZE, KCORE_OTHER);
+  PAGE_SIZE, KCORE_USER);
 
mem_init_print_info(NULL);
 }
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index 4bc85cb..e4b0204 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -510,6 +510,10 @@ static void elf_kcore_store_hdr(char *bufp, int nphdr, int 
dataoff)
/* we have to zero-fill user buffer even if no read */
if (copy_to_user(buffer, buf, tsz))
return -EFAULT;
+   } else if (m->type == KCORE_USER) {
+   /* user page is handled prior to normal kernel page */
+   if (copy_to_user(buffer, (char *)start, tsz))
+   return -EFAULT;
} else {
if (kern_addr_valid(start)) {
unsigned long n;
diff --git a/include/linux/kcore.h b/include/linux/kcore.h
index 7ff25a8..80db19d 100644
--- a/include/linux/kcore.h
+++ b/include/linux/kcore.h
@@ -10,6 +10,7 @@ enum kcore_type {
KCORE_VMALLOC,
KCORE_RAM,
KCORE_VMEMMAP,
+   KCORE_USER,
KCORE_OTHER,
 };
 
-- 
1.8.3.1

[PATCH 2/2] x86/mm/64: Add vsyscall page to /proc/kcore conditionally

[Jia Zhang]

The vsyscall page should be visible only if
vsyscall=emulate/native when dumping /proc/kcore.

Signed-off-by: Jia Zhang 
---
 arch/x86/mm/init_64.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index dab78f6..3d4cf33 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1186,8 +1186,9 @@ void __init mem_init(void)
register_page_bootmem_info();
 
/* Register memory areas for /proc/kcore */
-   kclist_add(_vsyscall, (void *)VSYSCALL_ADDR,
-  PAGE_SIZE, KCORE_USER);
+   if (get_gate_vma(_mm))
+   kclist_add(_vsyscall, (void *)VSYSCALL_ADDR,
+  PAGE_SIZE, KCORE_USER);
 
mem_init_print_info(NULL);
 }
-- 
1.8.3.1

[PATCH 1/2] /proc/kcore: Fix SMAP violation when dumping vsyscall user page

[Jia Zhang]

The commit df04abfd181a
("fs/proc/kcore.c: Add bounce buffer for ktext data") introduces a
bounce buffer to work around CONFIG_HARDENED_USERCOPY=y. However,
accessing vsyscall user page will cause SMAP violation in this way.

In order to fix this issue, simply replace memcpy() with copy_from_user()
may work, but using a common way to handle this sort of user page may be
useful for future.

Currently, only vsyscall page requires KCORE_USER.

Signed-off-by: Jia Zhang 
---
 arch/x86/mm/init_64.c | 2 +-
 fs/proc/kcore.c   | 4 
 include/linux/kcore.h | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 4a83728..dab78f6 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1187,7 +1187,7 @@ void __init mem_init(void)
 
/* Register memory areas for /proc/kcore */
kclist_add(_vsyscall, (void *)VSYSCALL_ADDR,
-PAGE_SIZE, KCORE_OTHER);
+  PAGE_SIZE, KCORE_USER);
 
mem_init_print_info(NULL);
 }
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index 4bc85cb..e4b0204 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -510,6 +510,10 @@ static void elf_kcore_store_hdr(char *bufp, int nphdr, int 
dataoff)
/* we have to zero-fill user buffer even if no read */
if (copy_to_user(buffer, buf, tsz))
return -EFAULT;
+   } else if (m->type == KCORE_USER) {
+   /* user page is handled prior to normal kernel page */
+   if (copy_to_user(buffer, (char *)start, tsz))
+   return -EFAULT;
} else {
if (kern_addr_valid(start)) {
unsigned long n;
diff --git a/include/linux/kcore.h b/include/linux/kcore.h
index 7ff25a8..80db19d 100644
--- a/include/linux/kcore.h
+++ b/include/linux/kcore.h
@@ -10,6 +10,7 @@ enum kcore_type {
KCORE_VMALLOC,
KCORE_RAM,
KCORE_VMEMMAP,
+   KCORE_USER,
KCORE_OTHER,
 };
 
-- 
1.8.3.1

Re: [PATCH RFC 01/16] prcu: Add PRCU implementation

[Boqun Feng]

On Tue, Jan 30, 2018 at 05:34:03AM +, zhangheng (AC) wrote:
[...]
> >> > +static void prcu_handler(void *info) {
> >> > +struct prcu_local_struct *local;
> >> > +
> >> > +local = this_cpu_ptr(_local);
> >> > +if (!local->locked)
> >
> >And I think a smp_mb() is needed here, because in the following case:
> >
> > CPU 0 CPU 1
> > ==  ==
> > {X is initially 0}
> >
> > WRITE_ONCE(X, 1);
> >
> >   prcu_read_unlock(void):
> >   if (locked) {
> >   synchronize_prcu(void):
> > ...
> > 
> > local->locked--;
> >   # switch to IPI
> > WRITE_ONCE(local->version,)
> >  > latest>
> >   
> >
> >   r1 = READ_ONCE(X);
> >
> >r1 could be 0, which breaks RCU guarantees.
> >
> 
> Thank you.
> As I know,
> it guarantees that the interrupt to be handled after all write instructions 
> issued before have complete in x86 arch.
> So the smp_mb is meaningless in x86 arch.

Sure. x86 is TSO, and we are talking about reordering of two stores
here, and that can not happen on TSO.

> But I am not sure whether other archs guarantee this feature. If not, we do 
> need a smp_mb here.
> 

I think most of the weak memory model don't have this gaurantee, so you
need a smp_mb() or use smp_store_release().

> >> > +WRITE_ONCE(local->version, 
> >> > atomic64_read(>global_version));
> >> > +}
> >> > +
> >> > +void synchronize_prcu(void)
> >> > +{
> >> > +int cpu;
> >> > +cpumask_t cpus;
> >> > +unsigned long long version;
> >> > +struct prcu_local_struct *local;
> >> > +
> >> > +version = atomic64_add_return(1, >global_version);
> >> > +mutex_lock(>mtx);
> >> > +
> >> > +local = get_cpu_ptr(_local);
> >> > +local->version = version;
> >> > +put_cpu_ptr(_local);
> >> > +
> >> > +cpumask_clear();
> >> > +for_each_possible_cpu(cpu) {
> >> > +local = per_cpu_ptr(_local, cpu);
> >> > +if (!READ_ONCE(local->online))
> >> > +continue;
> >> > +if (READ_ONCE(local->version) < version) {
> >> 
> >> On 32-bit systems, given that ->version is long long, you might see 
> >> load tearing.  And on some 32-bit systems, the cmpxchg() in 
> >> prcu_hander() might not build.
> >> 
> >
> >/me curious about why an atomic64_t is used here for global version. I think 
> >maybe 32bit global version still suffices.
> >
> >Regards,
> >Boqun
> 
> Because the synchronization latency is low, it can have higher gp frequency.
> It seems that 32bit can only correctly work for several years if there are 
> 20+ gps per second.
> 

Because PRCU doesn't handle gp number overflow? May I ask why this is
difficult? Currently RCU could tolerate counter wrap for grace period:

https://lwn.net/Articles/652677/ (Details in "Parallelism facts of 
life")

Is there any subtle difference I'm missing?

Regards,
Boqun

> >
> >> Or is the idea that only prcu_handler() updates ->version?  But in 
> >> that case, you wouldn't need the READ_ONCE() above.  What am I missing 
> >> here?
> >> 
> >> > +smp_call_function_single(cpu, prcu_handler, 
> >> > NULL, 0);
> >> > +cpumask_set_cpu(cpu, );
> >> > +}
> >> > +}
> >> > +
> >> > +for_each_cpu(cpu, ) {
> >> > +local = per_cpu_ptr(_local, cpu);
> >> > +while (READ_ONCE(local->version) < version)
> >> 
> >> This ->version read can also tear on some 32-bit systems, and this one 
> >> most definitely can race with the prcu_handler() above.  Does the 
> >> algorithm operate correctly in that case?  (It doesn't look that way 
> >> to me, but I might be missing something.) Or are 32-bit systems excluded?
> >> 
> >> > +cpu_relax();
> >> > +}
> >> 
> >> I might be missing something, but I believe we need a memory barrier 
> >> here on non-TSO systems.  Without that, couldn't we miss a preemption?
> >> 
> >> > +
> >> > +if (atomic_read(>active_ctr))
> >> > +wait_event(prcu->wait_q, 
> >> > !atomic_read(>active_ctr));
> >> > +
> >> > +mutex_unlock(>mtx);
> >> > +}
> >> > +EXPORT_SYMBOL(synchronize_prcu);
> >> > +
> >> > +void prcu_note_context_switch(void) {
> >> > +struct prcu_local_struct *local;
> >> > +
> >> > +local = get_cpu_ptr(_local);
> >> > +if (local->locked) {
> >> > +atomic_add(local->locked, >active_ctr);
> >> > +local->locked = 0;
> >> > + 

Re: [PATCH RFC 01/16] prcu: Add PRCU implementation

[Boqun Feng]

On Tue, Jan 30, 2018 at 05:34:03AM +, zhangheng (AC) wrote:
[...]
> >> > +static void prcu_handler(void *info) {
> >> > +struct prcu_local_struct *local;
> >> > +
> >> > +local = this_cpu_ptr(_local);
> >> > +if (!local->locked)
> >
> >And I think a smp_mb() is needed here, because in the following case:
> >
> > CPU 0 CPU 1
> > ==  ==
> > {X is initially 0}
> >
> > WRITE_ONCE(X, 1);
> >
> >   prcu_read_unlock(void):
> >   if (locked) {
> >   synchronize_prcu(void):
> > ...
> > 
> > local->locked--;
> >   # switch to IPI
> > WRITE_ONCE(local->version,)
> >  > latest>
> >   
> >
> >   r1 = READ_ONCE(X);
> >
> >r1 could be 0, which breaks RCU guarantees.
> >
> 
> Thank you.
> As I know,
> it guarantees that the interrupt to be handled after all write instructions 
> issued before have complete in x86 arch.
> So the smp_mb is meaningless in x86 arch.

Sure. x86 is TSO, and we are talking about reordering of two stores
here, and that can not happen on TSO.

> But I am not sure whether other archs guarantee this feature. If not, we do 
> need a smp_mb here.
> 

I think most of the weak memory model don't have this gaurantee, so you
need a smp_mb() or use smp_store_release().

> >> > +WRITE_ONCE(local->version, 
> >> > atomic64_read(>global_version));
> >> > +}
> >> > +
> >> > +void synchronize_prcu(void)
> >> > +{
> >> > +int cpu;
> >> > +cpumask_t cpus;
> >> > +unsigned long long version;
> >> > +struct prcu_local_struct *local;
> >> > +
> >> > +version = atomic64_add_return(1, >global_version);
> >> > +mutex_lock(>mtx);
> >> > +
> >> > +local = get_cpu_ptr(_local);
> >> > +local->version = version;
> >> > +put_cpu_ptr(_local);
> >> > +
> >> > +cpumask_clear();
> >> > +for_each_possible_cpu(cpu) {
> >> > +local = per_cpu_ptr(_local, cpu);
> >> > +if (!READ_ONCE(local->online))
> >> > +continue;
> >> > +if (READ_ONCE(local->version) < version) {
> >> 
> >> On 32-bit systems, given that ->version is long long, you might see 
> >> load tearing.  And on some 32-bit systems, the cmpxchg() in 
> >> prcu_hander() might not build.
> >> 
> >
> >/me curious about why an atomic64_t is used here for global version. I think 
> >maybe 32bit global version still suffices.
> >
> >Regards,
> >Boqun
> 
> Because the synchronization latency is low, it can have higher gp frequency.
> It seems that 32bit can only correctly work for several years if there are 
> 20+ gps per second.
> 

Because PRCU doesn't handle gp number overflow? May I ask why this is
difficult? Currently RCU could tolerate counter wrap for grace period:

https://lwn.net/Articles/652677/ (Details in "Parallelism facts of 
life")

Is there any subtle difference I'm missing?

Regards,
Boqun

> >
> >> Or is the idea that only prcu_handler() updates ->version?  But in 
> >> that case, you wouldn't need the READ_ONCE() above.  What am I missing 
> >> here?
> >> 
> >> > +smp_call_function_single(cpu, prcu_handler, 
> >> > NULL, 0);
> >> > +cpumask_set_cpu(cpu, );
> >> > +}
> >> > +}
> >> > +
> >> > +for_each_cpu(cpu, ) {
> >> > +local = per_cpu_ptr(_local, cpu);
> >> > +while (READ_ONCE(local->version) < version)
> >> 
> >> This ->version read can also tear on some 32-bit systems, and this one 
> >> most definitely can race with the prcu_handler() above.  Does the 
> >> algorithm operate correctly in that case?  (It doesn't look that way 
> >> to me, but I might be missing something.) Or are 32-bit systems excluded?
> >> 
> >> > +cpu_relax();
> >> > +}
> >> 
> >> I might be missing something, but I believe we need a memory barrier 
> >> here on non-TSO systems.  Without that, couldn't we miss a preemption?
> >> 
> >> > +
> >> > +if (atomic_read(>active_ctr))
> >> > +wait_event(prcu->wait_q, 
> >> > !atomic_read(>active_ctr));
> >> > +
> >> > +mutex_unlock(>mtx);
> >> > +}
> >> > +EXPORT_SYMBOL(synchronize_prcu);
> >> > +
> >> > +void prcu_note_context_switch(void) {
> >> > +struct prcu_local_struct *local;
> >> > +
> >> > +local = get_cpu_ptr(_local);
> >> > +if (local->locked) {
> >> > +atomic_add(local->locked, >active_ctr);
> >> > +local->locked = 0;
> >> > + 

Re: [RFC PATCH 5/8] media: Document the media request API

[Alexandre Courbot]

On Tue, Jan 30, 2018 at 1:04 AM, Hans Verkuil  wrote:
> On 01/26/2018 07:02 AM, Alexandre Courbot wrote:
>> From: Laurent Pinchart 
>>
>> The media request API is made of a new ioctl to implement request
>> management. Document it.
>>
>> Signed-off-by: Laurent Pinchart 
>> [acour...@chromium.org: adapt for newest API]
>> Signed-off-by: Alexandre Courbot 
>> ---
>>  Documentation/media/uapi/mediactl/media-funcs.rst  |   1 +
>>  .../media/uapi/mediactl/media-ioc-request-cmd.rst  | 140 
>> +
>>  2 files changed, 141 insertions(+)
>>  create mode 100644 
>> Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>>
>> diff --git a/Documentation/media/uapi/mediactl/media-funcs.rst 
>> b/Documentation/media/uapi/mediactl/media-funcs.rst
>> index 076856501cdb..e3a45d82ffcb 100644
>> --- a/Documentation/media/uapi/mediactl/media-funcs.rst
>> +++ b/Documentation/media/uapi/mediactl/media-funcs.rst
>> @@ -15,4 +15,5 @@ Function Reference
>>  media-ioc-g-topology
>>  media-ioc-enum-entities
>>  media-ioc-enum-links
>> +media-ioc-request-cmd
>>  media-ioc-setup-link
>> diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst 
>> b/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>> new file mode 100644
>> index ..17223e8170e9
>> --- /dev/null
>> +++ b/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>> @@ -0,0 +1,140 @@
>> +.. -*- coding: utf-8; mode: rst -*-
>> +
>> +.. _media_ioc_request_cmd:
>> +
>> +***
>> +ioctl MEDIA_IOC_REQUEST_CMD
>> +***
>> +
>> +Name
>> +
>> +
>> +MEDIA_IOC_REQUEST_CMD - Manage media device requests
>> +
>> +
>> +Synopsis
>> +
>> +
>> +.. c:function:: int ioctl( int fd, MEDIA_IOC_REQUEST_CMD, struct 
>> media_request_cmd *argp )
>> +:name: MEDIA_IOC_REQUEST_CMD
>> +
>> +
>> +Arguments
>> +=
>> +
>> +``fd``
>> +File descriptor returned by :ref:`open() `.
>> +
>> +``argp``
>> +
>> +
>> +Description
>> +===
>> +
>> +The MEDIA_IOC_REQUEST_CMD ioctl allow applications to manage media device
>> +requests. A request is an object that can group media device configuration
>> +parameters, including subsystem-specific parameters, in order to apply all 
>> the
>> +parameters atomically. Applications are responsible for allocating and
>> +deleting requests, filling them with configuration parameters submitting 
>> them.
>> +
>> +Request operations are performed by calling the MEDIA_IOC_REQUEST_CMD ioctl
>> +with a pointer to a struct :c:type:`media_request_cmd` with the cmd field 
>> set
>> +to the appropriate command. :ref:`media-request-command` lists the commands
>> +supported by the ioctl.
>> +
>> +The struct :c:type:`media_request_cmd` request field contains the file
>> +descriptorof the request on which the command operates. For the
>> +``MEDIA_REQ_CMD_ALLOC`` command the field is set to zero by applications and
>> +filled by the driver. For all other commands the field is set by 
>> applications
>> +and left untouched by the driver.
>> +
>> +To allocate a new request applications use the ``MEDIA_REQ_CMD_ALLOC``
>> +command. The driver will allocate a new request and return its ID in the
>
> ID -> FD

Indeed, this is a leftover from the original documentation.

>
>> +request field.
>> +
>> +Requests are reference-counted. A newly allocated request is referenced
>> +by the returned file descriptor, and can be later referenced by
>> +subsystem-specific operations. Requests will thus be automatically deleted
>> +when they're not longer used after the returned file descriptor is closed.
>> +
>> +If a request isn't needed applications can delete it by calling ``close()``
>> +on it. The driver will drop the file handle reference. The request will not
>> +be usable through the MEDIA_IOC_REQUEST_CMD ioctl anymore, but will only be
>> +deleted when the last reference is released. If no other reference exists 
>> when
>> +``close()`` is invoked the request will be deleted immediately.
>> +
>> +After creating a request applications should fill it with configuration
>> +parameters. This is performed through subsystem-specific request APIs 
>> outside
>> +the scope of the media controller API. See the appropriate subsystem APIs 
>> for
>> +more information, including how they interact with the MEDIA_IOC_REQUEST_CMD
>> +ioctl.
>> +
>> +Once a request contains all the desired configuration parameters it can be
>> +submitted using the ``MEDIA_REQ_CMD_SUBMIT`` command. This will let the
>> +buffers queued for the request be passed to their respective drivers, which
>> +will then apply the request's parameters before processing them.
>> +
>> +Once a request has been queued applications are not allowed to modify its
>> +configuration parameters until the request has been fully processed. Any
>> +attempt to do so 

Re: [RFC PATCH 5/8] media: Document the media request API

[Alexandre Courbot]

On Tue, Jan 30, 2018 at 1:04 AM, Hans Verkuil  wrote:
> On 01/26/2018 07:02 AM, Alexandre Courbot wrote:
>> From: Laurent Pinchart 
>>
>> The media request API is made of a new ioctl to implement request
>> management. Document it.
>>
>> Signed-off-by: Laurent Pinchart 
>> [acour...@chromium.org: adapt for newest API]
>> Signed-off-by: Alexandre Courbot 
>> ---
>>  Documentation/media/uapi/mediactl/media-funcs.rst  |   1 +
>>  .../media/uapi/mediactl/media-ioc-request-cmd.rst  | 140 
>> +
>>  2 files changed, 141 insertions(+)
>>  create mode 100644 
>> Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>>
>> diff --git a/Documentation/media/uapi/mediactl/media-funcs.rst 
>> b/Documentation/media/uapi/mediactl/media-funcs.rst
>> index 076856501cdb..e3a45d82ffcb 100644
>> --- a/Documentation/media/uapi/mediactl/media-funcs.rst
>> +++ b/Documentation/media/uapi/mediactl/media-funcs.rst
>> @@ -15,4 +15,5 @@ Function Reference
>>  media-ioc-g-topology
>>  media-ioc-enum-entities
>>  media-ioc-enum-links
>> +media-ioc-request-cmd
>>  media-ioc-setup-link
>> diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst 
>> b/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>> new file mode 100644
>> index ..17223e8170e9
>> --- /dev/null
>> +++ b/Documentation/media/uapi/mediactl/media-ioc-request-cmd.rst
>> @@ -0,0 +1,140 @@
>> +.. -*- coding: utf-8; mode: rst -*-
>> +
>> +.. _media_ioc_request_cmd:
>> +
>> +***
>> +ioctl MEDIA_IOC_REQUEST_CMD
>> +***
>> +
>> +Name
>> +
>> +
>> +MEDIA_IOC_REQUEST_CMD - Manage media device requests
>> +
>> +
>> +Synopsis
>> +
>> +
>> +.. c:function:: int ioctl( int fd, MEDIA_IOC_REQUEST_CMD, struct 
>> media_request_cmd *argp )
>> +:name: MEDIA_IOC_REQUEST_CMD
>> +
>> +
>> +Arguments
>> +=
>> +
>> +``fd``
>> +File descriptor returned by :ref:`open() `.
>> +
>> +``argp``
>> +
>> +
>> +Description
>> +===
>> +
>> +The MEDIA_IOC_REQUEST_CMD ioctl allow applications to manage media device
>> +requests. A request is an object that can group media device configuration
>> +parameters, including subsystem-specific parameters, in order to apply all 
>> the
>> +parameters atomically. Applications are responsible for allocating and
>> +deleting requests, filling them with configuration parameters submitting 
>> them.
>> +
>> +Request operations are performed by calling the MEDIA_IOC_REQUEST_CMD ioctl
>> +with a pointer to a struct :c:type:`media_request_cmd` with the cmd field 
>> set
>> +to the appropriate command. :ref:`media-request-command` lists the commands
>> +supported by the ioctl.
>> +
>> +The struct :c:type:`media_request_cmd` request field contains the file
>> +descriptorof the request on which the command operates. For the
>> +``MEDIA_REQ_CMD_ALLOC`` command the field is set to zero by applications and
>> +filled by the driver. For all other commands the field is set by 
>> applications
>> +and left untouched by the driver.
>> +
>> +To allocate a new request applications use the ``MEDIA_REQ_CMD_ALLOC``
>> +command. The driver will allocate a new request and return its ID in the
>
> ID -> FD

Indeed, this is a leftover from the original documentation.

>
>> +request field.
>> +
>> +Requests are reference-counted. A newly allocated request is referenced
>> +by the returned file descriptor, and can be later referenced by
>> +subsystem-specific operations. Requests will thus be automatically deleted
>> +when they're not longer used after the returned file descriptor is closed.
>> +
>> +If a request isn't needed applications can delete it by calling ``close()``
>> +on it. The driver will drop the file handle reference. The request will not
>> +be usable through the MEDIA_IOC_REQUEST_CMD ioctl anymore, but will only be
>> +deleted when the last reference is released. If no other reference exists 
>> when
>> +``close()`` is invoked the request will be deleted immediately.
>> +
>> +After creating a request applications should fill it with configuration
>> +parameters. This is performed through subsystem-specific request APIs 
>> outside
>> +the scope of the media controller API. See the appropriate subsystem APIs 
>> for
>> +more information, including how they interact with the MEDIA_IOC_REQUEST_CMD
>> +ioctl.
>> +
>> +Once a request contains all the desired configuration parameters it can be
>> +submitted using the ``MEDIA_REQ_CMD_SUBMIT`` command. This will let the
>> +buffers queued for the request be passed to their respective drivers, which
>> +will then apply the request's parameters before processing them.
>> +
>> +Once a request has been queued applications are not allowed to modify its
>> +configuration parameters until the request has been fully processed. Any
>> +attempt to do so will result in the related subsystem API returning an 
>> error.
>> +The application that submitted the request can wait for its 

Re: [RFC PATCH 0/8] [media] Request API, take three

[Alexandre Courbot]

Hi Hans,

On Mon, Jan 29, 2018 at 8:21 PM, Hans Verkuil  wrote:
> On 01/26/2018 07:02 AM, Alexandre Courbot wrote:
>> Howdy. Here is your bi-weekly request API redesign! ;)
>>
>> Again, this is a simple version that only implements the flow of requests,
>> without applying controls. The intent is to get an agreement on a base to 
>> work
>> on, since the previous versions went straight back to the redesign board.
>>
>> Highlights of this version:
>>
>> * As requested by Hans, request-bound buffers are now passed earlier to 
>> drivers,
>> as early as the request itself is submitted. Doing it earlier is not be 
>> useful
>> since the driver would not know the state of the request, and thus cannot do
>> anything with the buffer. Drivers are now responsible for applying request
>> parameters themselves.
>>
>> * As a consequence, there is no such thing as a "request queue" anymore. The
>> flow of buffers decides the order in which requests are processed. Individual
>> devices of the same pipeline can implement synchronization if needed, but 
>> this
>> is beyond this first version.
>>
>> * VB2 code is still a bit shady. Some of it will interfere with the fences
>> series, so I am waiting for the latter to land to do it properly.
>>
>> * Requests that are not yet submitted effectively act as fences on the 
>> buffers
>> they own, resulting in the buffer queue being blocked until the request is
>> submitted. An alternate design would be to only block the
>> not-submitted-request's buffer and let further buffers pass before it, but 
>> since
>> buffer order is becoming increasingly important I have decided to just block 
>> the
>> queue. This is open to discussion though.
>
> I don't think we should mess with the order.

Agreed, let's keep it that way then.

>
>>
>> * Documentation! Also described usecases for codec and simple (i.e. not part 
>> of
>> a complex pipeline) capture device.
>
> I'll concentrate on reviewing that.
>
>>
>> Still remaining to do:
>>
>> * As pointed out by Hans on the previous revision, do not assume that drivers
>> using v4l2_fh have a per-handle state. I have not yet found a good way to
>> differenciate both usages.
>
> I suspect we need to add a flag or something for this.

I hope we don't need to, let's see if I can find a pattern...

>
>> * Integrate Hans' patchset for control handling: as said above, this is 
>> futile
>> unless we can agree on the basic design, which I hope we can do this time.
>> Chrome OS needs this really badly now and will have to move forward no matter
>> what, so I hope this will be considered good enough for a common base of 
>> work.
>
> I am not sure there is any reason to not move forward with the control 
> handling.
> You need this anyway IMHO, regardless of any public API considerations.

Only reasons are my lazyness and because I wanted to focus on the
request flow first. But you're right. I have a version with your
control framework changes integrated (they worked on the first attempt
btw! :)), let me create a clean patchset from this and send another
RFC.

>
>> A few thoughts/questions that emerged when writing this patchset:
>>
>> * Since requests are exposed as file descriptors, we could easily move the
>> MEDIA_REQ_CMD_SUBMIT and MEDIA_REQ_CMD_REININT commands as ioctls on the
>> requests themselves, instead of having to perform them on the media device 
>> that
>> provided the request in the first place. That would add a bit more 
>> flexibility
>> if/when passing requests around and means the media device only needs to 
>> handle
>> MEDIA_REQ_CMD_ALLOC. Conceptually speaking, this seems to make sense to me.
>> Any comment for/against that?
>
> Makes sense IMHO.

Glad to hear it, that was my preferred design. :)

>
>> * For the codec usecase, I have experimented a bit marking CAPTURE buffers 
>> with
>> the request FD that produced them (vim2m acts that way). This seems to work
>> well, however FDs are process-local and could be closed before the CAPTURE
>> buffer is dequeued, rendering that information less meaningful, if not
>> dangerous.
>
> I don't follow this. Once the fd is passed to the kernel its refcount should 
> be
> increased so the data it represents won't be released if userspace closes the 
> fd.

The refcount of the request is increased. The refcount of the FD is
not, since it is only a userspace reference to the request.

>
> Obviously if userspace closes the fd it cannot do anything with it anymore, 
> but
> it shouldn't be 'dangerous' AFAICT.

It userspace later gets that closed FD back from a DQBUF call, and
decides to use it again, then we would have a problem. I agree that it
is userspace responsibility to be careful here, but making things
foolproof never hurts.

>
>> Wouldn't it be better/safer to use a global request ID for
>> such information instead? That ID would be returned upon MEDIA_REQ_CMD_ALLOC 
>> so
>> user-space knows which request ID a FD refers to.
>
> I think it is not a good idea to 

Re: [RFC PATCH 6/8] v4l2: document the request API interface

[Alexandre Courbot]

On Tue, Jan 30, 2018 at 1:03 AM, Hans Verkuil  wrote:
> On 01/26/2018 07:02 AM, Alexandre Courbot wrote:
>> Document how the request API can be used along with the existing V4L2
>> interface.
>>
>> Signed-off-by: Alexandre Courbot 
>> ---
>>  Documentation/media/uapi/v4l/buffer.rst  |  10 +-
>>  Documentation/media/uapi/v4l/common.rst  |   1 +
>>  Documentation/media/uapi/v4l/request-api.rst | 194 
>> +++
>>  Documentation/media/uapi/v4l/vidioc-qbuf.rst |  21 +++
>>  4 files changed, 223 insertions(+), 3 deletions(-)
>>  create mode 100644 Documentation/media/uapi/v4l/request-api.rst
>>
>> diff --git a/Documentation/media/uapi/v4l/buffer.rst 
>> b/Documentation/media/uapi/v4l/buffer.rst
>> index ae6ee73f151c..9d082784081d 100644
>> --- a/Documentation/media/uapi/v4l/buffer.rst
>> +++ b/Documentation/media/uapi/v4l/buffer.rst
>> @@ -301,10 +301,14 @@ struct v4l2_buffer
>>   elements in the ``planes`` array. The driver will fill in the
>>   actual number of valid elements in that array.
>>  * - __u32
>> -  - ``reserved2``
>> +  - ``request_fd``
>>-
>> -  - A place holder for future extensions. Drivers and applications
>> - must set this to 0.
>> +  - The file descriptor of the request associated with this buffer.
>> + user-space can set this when calling :ref:`VIDIOC_QBUF`, and drivers
>> + will return the request used when processing a buffer (if any) upon
>> + :ref:`VIDIOC_DQBUF`.
>> +
>> + A value of 0 means the buffer is not associated with any request.
>>  * - __u32
>>- ``reserved``
>>-
>> diff --git a/Documentation/media/uapi/v4l/common.rst 
>> b/Documentation/media/uapi/v4l/common.rst
>> index 13f2ed3fc5a6..a4aa0059d45a 100644
>> --- a/Documentation/media/uapi/v4l/common.rst
>> +++ b/Documentation/media/uapi/v4l/common.rst
>> @@ -44,3 +44,4 @@ applicable to all devices.
>>  crop
>>  selection-api
>>  streaming-par
>> +request-api
>> diff --git a/Documentation/media/uapi/v4l/request-api.rst 
>> b/Documentation/media/uapi/v4l/request-api.rst
>> new file mode 100644
>> index ..a758a5fd3ca0
>> --- /dev/null
>> +++ b/Documentation/media/uapi/v4l/request-api.rst
>> @@ -0,0 +1,194 @@
>> +.. -*- coding: utf-8; mode: rst -*-
>> +
>> +.. _media-request-api:
>> +
>> +Request API
>> +===
>> +
>> +The Request API has been designed to allow V4L2 to deal with requirements of
>> +modern IPs (stateless codecs, MIPI cameras, ...) and APIs (Android Codec 
>> v2).
>> +One such requirement is the ability for devices belonging to the same 
>> pipeline
>> +to reconfigure and collaborate closely on a per-frame basis. Another is
>> +efficient support of stateless codecs, which need per-frame controls to be 
>> set
>> +asynchronously in order to be efficiently used.
>> +
>> +Supporting these features without the Request API is possible but terribly
>> +inefficient: user-space would have to flush all activity on the media 
>> pipeline,
>> +reconfigure it for the next frame, queue the buffers to be processed with 
>> that
>> +configuration, and wait until they are all available for dequeing before
>> +considering the next frame. This defeats the purpose of having buffer queues
>> +since in practice only one buffer would be queued at a time.
>> +
>> +The Request API allows a specific configuration of the pipeline (media
>> +controller topology + controls for each device) to be associated with 
>> specific
>> +buffers. The parameters are applied by each participating device as buffers
>> +associated to a request flow in. This allows user-space to schedule several
>> +tasks ("requests") with different parameters in advance, knowing that the
>> +parameters will be applied when needed to get the expected result. Controls
>> +values at the time of request completion are also available for reading.
>> +
>> +Usage
>> +=
>> +
>> +The Request API is used on top of standard media controller and V4L2 calls,
>> +which are augmented with an extra ``request_fd`` parameter. All operations 
>> on
>> +requests themselves are performed using the command parameter of the
>> +:c:func:`MEDIA_IOC_REQUEST_CMD` ioctl.
>> +
>> +Allocation
>> +--
>> +
>> +User-space allocates requests using the ``MEDIA_REQ_CMD_ALLOC`` command on
>> +an opened media device. This returns a file descriptor representing the
>> +request. Typically, several such requests will be allocated.
>> +
>> +Request Preparation
>> +---
>> +
>> +Standard V4L2 ioctls can then receive a request file descriptor to express 
>> the
>> +fact that the ioctl is part of said request, and is not to be applied
>> +immediately. V4L2 ioctls supporting this are :c:func:`VIDIOC_S_EXT_CTRLS` 
>> and
>> +:c:func:`VIDIOC_QBUF`. Controls set with a request parameter are stored 
>> instead
>> +of being immediately applied, and queued buffers will block the buffer queue
>> +until the request 

Re: [RFC PATCH 0/8] [media] Request API, take three

[Alexandre Courbot]

Hi Hans,

On Mon, Jan 29, 2018 at 8:21 PM, Hans Verkuil  wrote:
> On 01/26/2018 07:02 AM, Alexandre Courbot wrote:
>> Howdy. Here is your bi-weekly request API redesign! ;)
>>
>> Again, this is a simple version that only implements the flow of requests,
>> without applying controls. The intent is to get an agreement on a base to 
>> work
>> on, since the previous versions went straight back to the redesign board.
>>
>> Highlights of this version:
>>
>> * As requested by Hans, request-bound buffers are now passed earlier to 
>> drivers,
>> as early as the request itself is submitted. Doing it earlier is not be 
>> useful
>> since the driver would not know the state of the request, and thus cannot do
>> anything with the buffer. Drivers are now responsible for applying request
>> parameters themselves.
>>
>> * As a consequence, there is no such thing as a "request queue" anymore. The
>> flow of buffers decides the order in which requests are processed. Individual
>> devices of the same pipeline can implement synchronization if needed, but 
>> this
>> is beyond this first version.
>>
>> * VB2 code is still a bit shady. Some of it will interfere with the fences
>> series, so I am waiting for the latter to land to do it properly.
>>
>> * Requests that are not yet submitted effectively act as fences on the 
>> buffers
>> they own, resulting in the buffer queue being blocked until the request is
>> submitted. An alternate design would be to only block the
>> not-submitted-request's buffer and let further buffers pass before it, but 
>> since
>> buffer order is becoming increasingly important I have decided to just block 
>> the
>> queue. This is open to discussion though.
>
> I don't think we should mess with the order.

Agreed, let's keep it that way then.

>
>>
>> * Documentation! Also described usecases for codec and simple (i.e. not part 
>> of
>> a complex pipeline) capture device.
>
> I'll concentrate on reviewing that.
>
>>
>> Still remaining to do:
>>
>> * As pointed out by Hans on the previous revision, do not assume that drivers
>> using v4l2_fh have a per-handle state. I have not yet found a good way to
>> differenciate both usages.
>
> I suspect we need to add a flag or something for this.

I hope we don't need to, let's see if I can find a pattern...

>
>> * Integrate Hans' patchset for control handling: as said above, this is 
>> futile
>> unless we can agree on the basic design, which I hope we can do this time.
>> Chrome OS needs this really badly now and will have to move forward no matter
>> what, so I hope this will be considered good enough for a common base of 
>> work.
>
> I am not sure there is any reason to not move forward with the control 
> handling.
> You need this anyway IMHO, regardless of any public API considerations.

Only reasons are my lazyness and because I wanted to focus on the
request flow first. But you're right. I have a version with your
control framework changes integrated (they worked on the first attempt
btw! :)), let me create a clean patchset from this and send another
RFC.

>
>> A few thoughts/questions that emerged when writing this patchset:
>>
>> * Since requests are exposed as file descriptors, we could easily move the
>> MEDIA_REQ_CMD_SUBMIT and MEDIA_REQ_CMD_REININT commands as ioctls on the
>> requests themselves, instead of having to perform them on the media device 
>> that
>> provided the request in the first place. That would add a bit more 
>> flexibility
>> if/when passing requests around and means the media device only needs to 
>> handle
>> MEDIA_REQ_CMD_ALLOC. Conceptually speaking, this seems to make sense to me.
>> Any comment for/against that?
>
> Makes sense IMHO.

Glad to hear it, that was my preferred design. :)

>
>> * For the codec usecase, I have experimented a bit marking CAPTURE buffers 
>> with
>> the request FD that produced them (vim2m acts that way). This seems to work
>> well, however FDs are process-local and could be closed before the CAPTURE
>> buffer is dequeued, rendering that information less meaningful, if not
>> dangerous.
>
> I don't follow this. Once the fd is passed to the kernel its refcount should 
> be
> increased so the data it represents won't be released if userspace closes the 
> fd.

The refcount of the request is increased. The refcount of the FD is
not, since it is only a userspace reference to the request.

>
> Obviously if userspace closes the fd it cannot do anything with it anymore, 
> but
> it shouldn't be 'dangerous' AFAICT.

It userspace later gets that closed FD back from a DQBUF call, and
decides to use it again, then we would have a problem. I agree that it
is userspace responsibility to be careful here, but making things
foolproof never hurts.

>
>> Wouldn't it be better/safer to use a global request ID for
>> such information instead? That ID would be returned upon MEDIA_REQ_CMD_ALLOC 
>> so
>> user-space knows which request ID a FD refers to.
>
> I think it is not a good idea to have both an fd and 

Re: [RFC PATCH 6/8] v4l2: document the request API interface

[Alexandre Courbot]

On Tue, Jan 30, 2018 at 1:03 AM, Hans Verkuil  wrote:
> On 01/26/2018 07:02 AM, Alexandre Courbot wrote:
>> Document how the request API can be used along with the existing V4L2
>> interface.
>>
>> Signed-off-by: Alexandre Courbot 
>> ---
>>  Documentation/media/uapi/v4l/buffer.rst  |  10 +-
>>  Documentation/media/uapi/v4l/common.rst  |   1 +
>>  Documentation/media/uapi/v4l/request-api.rst | 194 
>> +++
>>  Documentation/media/uapi/v4l/vidioc-qbuf.rst |  21 +++
>>  4 files changed, 223 insertions(+), 3 deletions(-)
>>  create mode 100644 Documentation/media/uapi/v4l/request-api.rst
>>
>> diff --git a/Documentation/media/uapi/v4l/buffer.rst 
>> b/Documentation/media/uapi/v4l/buffer.rst
>> index ae6ee73f151c..9d082784081d 100644
>> --- a/Documentation/media/uapi/v4l/buffer.rst
>> +++ b/Documentation/media/uapi/v4l/buffer.rst
>> @@ -301,10 +301,14 @@ struct v4l2_buffer
>>   elements in the ``planes`` array. The driver will fill in the
>>   actual number of valid elements in that array.
>>  * - __u32
>> -  - ``reserved2``
>> +  - ``request_fd``
>>-
>> -  - A place holder for future extensions. Drivers and applications
>> - must set this to 0.
>> +  - The file descriptor of the request associated with this buffer.
>> + user-space can set this when calling :ref:`VIDIOC_QBUF`, and drivers
>> + will return the request used when processing a buffer (if any) upon
>> + :ref:`VIDIOC_DQBUF`.
>> +
>> + A value of 0 means the buffer is not associated with any request.
>>  * - __u32
>>- ``reserved``
>>-
>> diff --git a/Documentation/media/uapi/v4l/common.rst 
>> b/Documentation/media/uapi/v4l/common.rst
>> index 13f2ed3fc5a6..a4aa0059d45a 100644
>> --- a/Documentation/media/uapi/v4l/common.rst
>> +++ b/Documentation/media/uapi/v4l/common.rst
>> @@ -44,3 +44,4 @@ applicable to all devices.
>>  crop
>>  selection-api
>>  streaming-par
>> +request-api
>> diff --git a/Documentation/media/uapi/v4l/request-api.rst 
>> b/Documentation/media/uapi/v4l/request-api.rst
>> new file mode 100644
>> index ..a758a5fd3ca0
>> --- /dev/null
>> +++ b/Documentation/media/uapi/v4l/request-api.rst
>> @@ -0,0 +1,194 @@
>> +.. -*- coding: utf-8; mode: rst -*-
>> +
>> +.. _media-request-api:
>> +
>> +Request API
>> +===
>> +
>> +The Request API has been designed to allow V4L2 to deal with requirements of
>> +modern IPs (stateless codecs, MIPI cameras, ...) and APIs (Android Codec 
>> v2).
>> +One such requirement is the ability for devices belonging to the same 
>> pipeline
>> +to reconfigure and collaborate closely on a per-frame basis. Another is
>> +efficient support of stateless codecs, which need per-frame controls to be 
>> set
>> +asynchronously in order to be efficiently used.
>> +
>> +Supporting these features without the Request API is possible but terribly
>> +inefficient: user-space would have to flush all activity on the media 
>> pipeline,
>> +reconfigure it for the next frame, queue the buffers to be processed with 
>> that
>> +configuration, and wait until they are all available for dequeing before
>> +considering the next frame. This defeats the purpose of having buffer queues
>> +since in practice only one buffer would be queued at a time.
>> +
>> +The Request API allows a specific configuration of the pipeline (media
>> +controller topology + controls for each device) to be associated with 
>> specific
>> +buffers. The parameters are applied by each participating device as buffers
>> +associated to a request flow in. This allows user-space to schedule several
>> +tasks ("requests") with different parameters in advance, knowing that the
>> +parameters will be applied when needed to get the expected result. Controls
>> +values at the time of request completion are also available for reading.
>> +
>> +Usage
>> +=
>> +
>> +The Request API is used on top of standard media controller and V4L2 calls,
>> +which are augmented with an extra ``request_fd`` parameter. All operations 
>> on
>> +requests themselves are performed using the command parameter of the
>> +:c:func:`MEDIA_IOC_REQUEST_CMD` ioctl.
>> +
>> +Allocation
>> +--
>> +
>> +User-space allocates requests using the ``MEDIA_REQ_CMD_ALLOC`` command on
>> +an opened media device. This returns a file descriptor representing the
>> +request. Typically, several such requests will be allocated.
>> +
>> +Request Preparation
>> +---
>> +
>> +Standard V4L2 ioctls can then receive a request file descriptor to express 
>> the
>> +fact that the ioctl is part of said request, and is not to be applied
>> +immediately. V4L2 ioctls supporting this are :c:func:`VIDIOC_S_EXT_CTRLS` 
>> and
>> +:c:func:`VIDIOC_QBUF`. Controls set with a request parameter are stored 
>> instead
>> +of being immediately applied, and queued buffers will block the buffer queue
>> +until the request becomes active.
>> +
>> +RFC Note: currently 

Re: [GIT pull] Timer core updates for 4.16

[Ingo Molnar]

* Linus Torvalds  wrote:

> On Mon, Jan 29, 2018 at 12:48 AM, Thomas Gleixner  wrote:
> >
> >   - A rather large rework of the hrtimer infrastructure which introduces
> > softirq based hrtimers to replace the spread of hrtimer/tasklet combos
> > which force the actual callback execution into softirq context.
> 
> I really would have liked to see more of the rationale for this - now
> I'm left with just two example drivers, and a "you'll see the cleanups
> this allows in future driver pulls".
> 
> But pulled, since the code doesn't look disgusting.

Sorry, this is my fault: from Anna-Maria's original, full softirq-hrtimers 
series 
I didn't apply many of the usecases, because we didn't get any review feedback 
from the networking folks:

 can/bcm: Replace hrtimer_tasklet with softirq based hrtimer

   net/can/bcm.c | 156 
--
   1 file changed, 52 insertions(+), 104 deletions(-)

 mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer

   drivers/net/wireless/mac80211_hwsim.c | 44 
---
   1 file changed, 20 insertions(+), 24 deletions(-)

 xfrm: Replace hrtimer tasklet with softirq hrtimer

   include/net/xfrm.h|  2 +-
   net/xfrm/xfrm_state.c | 30 ++
   2 files changed, 19 insertions(+), 13 deletions(-)

 net/mvpp2: Replace tasklet with softirq hrtimer

   drivers/net/ethernet/marvell/mvpp2.c | 62 
+++-
   1 file changed, 25 insertions(+), 37 deletions(-)

And I felt uneasy about applying this in one go, so we decided to apply it in 
two 
phases.

These are in cases significant driver simplifications, but they also enable the 
real deal, the elimination of the hrtimer tasklet:

 softirq: Remove tasklet_hrtimer

   include/linux/interrupt.h | 25 ---
   kernel/softirq.c  | 51 
---
   2 files changed, 76 deletions(-)

... which is a pretty nice thing in itself even without the driver 
simplifications!

Plus the _real_ secret motivation behind it all is the -rt kernel and 
CONFIG_PREEMPT_RT=y and the ability to push most of the hrtimer processing into 
softirq context - while it still keeps the main hrtimer machinery capable to 
run 
in hard-RT hardirq domain. Turns out it was possible to implement this duality 
via 
the softirq-hrtimers, with a good chunk of benefits to non-rt upstream as well.

Thanks,

Ingo

Re: [GIT pull] Timer core updates for 4.16

[Ingo Molnar]

* Linus Torvalds  wrote:

> On Mon, Jan 29, 2018 at 12:48 AM, Thomas Gleixner  wrote:
> >
> >   - A rather large rework of the hrtimer infrastructure which introduces
> > softirq based hrtimers to replace the spread of hrtimer/tasklet combos
> > which force the actual callback execution into softirq context.
> 
> I really would have liked to see more of the rationale for this - now
> I'm left with just two example drivers, and a "you'll see the cleanups
> this allows in future driver pulls".
> 
> But pulled, since the code doesn't look disgusting.

Sorry, this is my fault: from Anna-Maria's original, full softirq-hrtimers 
series 
I didn't apply many of the usecases, because we didn't get any review feedback 
from the networking folks:

 can/bcm: Replace hrtimer_tasklet with softirq based hrtimer

   net/can/bcm.c | 156 
--
   1 file changed, 52 insertions(+), 104 deletions(-)

 mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer

   drivers/net/wireless/mac80211_hwsim.c | 44 
---
   1 file changed, 20 insertions(+), 24 deletions(-)

 xfrm: Replace hrtimer tasklet with softirq hrtimer

   include/net/xfrm.h|  2 +-
   net/xfrm/xfrm_state.c | 30 ++
   2 files changed, 19 insertions(+), 13 deletions(-)

 net/mvpp2: Replace tasklet with softirq hrtimer

   drivers/net/ethernet/marvell/mvpp2.c | 62 
+++-
   1 file changed, 25 insertions(+), 37 deletions(-)

And I felt uneasy about applying this in one go, so we decided to apply it in 
two 
phases.

These are in cases significant driver simplifications, but they also enable the 
real deal, the elimination of the hrtimer tasklet:

 softirq: Remove tasklet_hrtimer

   include/linux/interrupt.h | 25 ---
   kernel/softirq.c  | 51 
---
   2 files changed, 76 deletions(-)

... which is a pretty nice thing in itself even without the driver 
simplifications!

Plus the _real_ secret motivation behind it all is the -rt kernel and 
CONFIG_PREEMPT_RT=y and the ability to push most of the hrtimer processing into 
softirq context - while it still keeps the main hrtimer machinery capable to 
run 
in hard-RT hardirq domain. Turns out it was possible to implement this duality 
via 
the softirq-hrtimers, with a good chunk of benefits to non-rt upstream as well.

Thanks,

Ingo

Re: [PATCH v5 02/12] array_idx: sanitize speculative array de-references

[Dan Williams]

On Sun, Jan 28, 2018 at 10:36 AM, Thomas Gleixner  wrote:
> On Sun, 28 Jan 2018, Dan Williams wrote:
>> On Sun, Jan 28, 2018 at 12:55 AM, Ingo Molnar  wrote:
>> >> + */
>> >> +#define array_idx(idx, sz)   \
>> >> +({   \
>> >> + typeof(idx) _i = (idx); \
>> >> + typeof(sz) _s = (sz);   \
>> >> + unsigned long _mask = array_idx_mask(_i, _s);   \
>> >> + \
>> >> + BUILD_BUG_ON(sizeof(_i) > sizeof(long));\
>> >> + BUILD_BUG_ON(sizeof(_s) > sizeof(long));\
>> >> + \
>> >> + _i &= _mask;\
>> >> + _i; \
>> >> +})
>> >> +#endif /* __NOSPEC_H__ */
>> >
>> > For heaven's sake, please name a size variable as 'size', not 'sz'. We 
>> > don't have
>> > a shortage of characters and can deobfuscate common primitives, can we?
>> >
>> > Also, beyond the nits, I also hate the namespace here. We have a new 
>> > generic
>> > header providing two new methods:
>> >
>> > #include 
>> >
>> > array_idx_mask()
>> > array_idx()
>> >
>> > which is then optimized for x86 in asm/barrier.h. That's already a 
>> > non-sequitor.
>> >
>> > Then we introduce uaccess API variants with a _nospec() postfix.
>> >
>> > Then we add ifence() to x86.
>> >
>> > There's no naming coherency to this.
>>
>> Ingo, I love you, but please take the incredulity down a bit,
>> especially when I had 'nospec' in all the names in v1. Thomas, Peter,
>> and Alexei wanted s/nospec_barrier/ifence/ and
>
> Sorry, I never was involved in that discussion.
>
>> s/array_idx_nospec/array_idx/. You can always follow on with a patch
>> to fix up the names and placements to your liking. While they'll pick
>> on my name choices, they won't pick on yours, because I simply can't
>> be bothered to care about a bikeshed color at this point after being
>> bounced around for 5 revisions of this patch set.
>
> Oh well, we really need this kind of attitude right now. We are all fed up
> with that mess, but Ingo and I care about the details, consistency and
> general code quality beyond the current rush to get stuff solved. It's our
> damned job as maintainers.

Of course, and everything about the technical feedback and suggestions
was completely valid, on point, and made the patches that much better.
What wasn't appreciated and what I am tired of grinning and bearing is
the idea that it's only the maintainer that can show emotion, that
it's only the maintainer that can be exasperated and burnt out.

For example, I would have spun v6 the same day (same hour?) had the
mail started, "hey I'm late to the party, why aren't all these helpers
in a new _nospec namespace?". I might have pointed to the mails from
Peter about ifence being his preferred name for a speculation barrier
and Alexei's request to drop nospec [1] and moved on because naming is
a maintainer's prerogative.

> If you decide you don't care anymore, please let me know, so I can try to
> free up some cycles to pick up the stuff from where you decided to dump it.

Care is a two way street. I respect yours and Ingo's workload, please
respect mine.

[1]: https://lkml.org/lkml/2018/1/9/1232

Re: [PATCH v5 02/12] array_idx: sanitize speculative array de-references

[Dan Williams]

On Sun, Jan 28, 2018 at 10:36 AM, Thomas Gleixner  wrote:
> On Sun, 28 Jan 2018, Dan Williams wrote:
>> On Sun, Jan 28, 2018 at 12:55 AM, Ingo Molnar  wrote:
>> >> + */
>> >> +#define array_idx(idx, sz)   \
>> >> +({   \
>> >> + typeof(idx) _i = (idx); \
>> >> + typeof(sz) _s = (sz);   \
>> >> + unsigned long _mask = array_idx_mask(_i, _s);   \
>> >> + \
>> >> + BUILD_BUG_ON(sizeof(_i) > sizeof(long));\
>> >> + BUILD_BUG_ON(sizeof(_s) > sizeof(long));\
>> >> + \
>> >> + _i &= _mask;\
>> >> + _i; \
>> >> +})
>> >> +#endif /* __NOSPEC_H__ */
>> >
>> > For heaven's sake, please name a size variable as 'size', not 'sz'. We 
>> > don't have
>> > a shortage of characters and can deobfuscate common primitives, can we?
>> >
>> > Also, beyond the nits, I also hate the namespace here. We have a new 
>> > generic
>> > header providing two new methods:
>> >
>> > #include 
>> >
>> > array_idx_mask()
>> > array_idx()
>> >
>> > which is then optimized for x86 in asm/barrier.h. That's already a 
>> > non-sequitor.
>> >
>> > Then we introduce uaccess API variants with a _nospec() postfix.
>> >
>> > Then we add ifence() to x86.
>> >
>> > There's no naming coherency to this.
>>
>> Ingo, I love you, but please take the incredulity down a bit,
>> especially when I had 'nospec' in all the names in v1. Thomas, Peter,
>> and Alexei wanted s/nospec_barrier/ifence/ and
>
> Sorry, I never was involved in that discussion.
>
>> s/array_idx_nospec/array_idx/. You can always follow on with a patch
>> to fix up the names and placements to your liking. While they'll pick
>> on my name choices, they won't pick on yours, because I simply can't
>> be bothered to care about a bikeshed color at this point after being
>> bounced around for 5 revisions of this patch set.
>
> Oh well, we really need this kind of attitude right now. We are all fed up
> with that mess, but Ingo and I care about the details, consistency and
> general code quality beyond the current rush to get stuff solved. It's our
> damned job as maintainers.

Of course, and everything about the technical feedback and suggestions
was completely valid, on point, and made the patches that much better.
What wasn't appreciated and what I am tired of grinning and bearing is
the idea that it's only the maintainer that can show emotion, that
it's only the maintainer that can be exasperated and burnt out.

For example, I would have spun v6 the same day (same hour?) had the
mail started, "hey I'm late to the party, why aren't all these helpers
in a new _nospec namespace?". I might have pointed to the mails from
Peter about ifence being his preferred name for a speculation barrier
and Alexei's request to drop nospec [1] and moved on because naming is
a maintainer's prerogative.

> If you decide you don't care anymore, please let me know, so I can try to
> free up some cycles to pick up the stuff from where you decided to dump it.

Care is a two way street. I respect yours and Ingo's workload, please
respect mine.

[1]: https://lkml.org/lkml/2018/1/9/1232

RE: [PATCH RFC 01/16] prcu: Add PRCU implementation

[zhangheng (AC)]

>-Original Message-
>From: jiangshan...@gmail.com [mailto:jiangshan...@gmail.com] On Behalf Of Lai 
>Jiangshan
>Sent: 2018年1月29日 17:11
>To: liangli...@huawei.com
>Cc: Paul E. McKenney ; Guohanjun (Hanjun Guo) 
>; zhangheng (AC) ; Chenhaibo (Haibo, 
>OS Lab) ; lihao.li...@gmail.com; LKML 
>
>Subject: Re: [PATCH RFC 01/16] prcu: Add PRCU implementation
>
>On Tue, Jan 23, 2018 at 3:59 PM,   wrote:
>> From: Heng Zhang 
>>
>> This RCU implementation (PRCU) is based on a fast consensus protocol 
>> published in the following paper:
>>
>> Fast Consensus Using Bounded Staleness for Scalable Read-mostly 
>> Synchronization.
>> Haibo Chen, Heng Zhang, Ran Liu, Binyu Zang, and Haibing Guan.
>> IEEE Transactions on Parallel and Distributed Systems (TPDS), 2016.
>> https://dl.acm.org/citation.cfm?id=3024114.3024143
>>
>> Signed-off-by: Heng Zhang 
>> Signed-off-by: Lihao Liang 
>> ---
>>  include/linux/prcu.h |  37 +++
>>  kernel/rcu/Makefile  |   2 +-
>>  kernel/rcu/prcu.c| 125 
>> +++
>>  kernel/sched/core.c  |   2 +
>>  4 files changed, 165 insertions(+), 1 deletion(-)  create mode 100644 
>> include/linux/prcu.h  create mode 100644 kernel/rcu/prcu.c
>>
>> diff --git a/include/linux/prcu.h b/include/linux/prcu.h new file mode 
>> 100644 index ..653b4633
>> --- /dev/null
>> +++ b/include/linux/prcu.h
>> @@ -0,0 +1,37 @@
>> +#ifndef __LINUX_PRCU_H
>> +#define __LINUX_PRCU_H
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define CONFIG_PRCU
>> +
>> +struct prcu_local_struct {
>> +   unsigned int locked;
>> +   unsigned int online;
>> +   unsigned long long version;
>> +};
>> +
>> +struct prcu_struct {
>> +   atomic64_t global_version;
>> +   atomic_t active_ctr;
>> +   struct mutex mtx;
>> +   wait_queue_head_t wait_q;
>> +};
>> +
>> +#ifdef CONFIG_PRCU
>> +void prcu_read_lock(void);
>> +void prcu_read_unlock(void);
>> +void synchronize_prcu(void);
>> +void prcu_note_context_switch(void);
>> +
>> +#else /* #ifdef CONFIG_PRCU */
>> +
>> +#define prcu_read_lock() do {} while (0) #define prcu_read_unlock() 
>> +do {} while (0) #define synchronize_prcu() do {} while (0) #define 
>> +prcu_note_context_switch() do {} while (0)
>> +
>> +#endif /* #ifdef CONFIG_PRCU */
>> +#endif /* __LINUX_PRCU_H */
>> diff --git a/kernel/rcu/Makefile b/kernel/rcu/Makefile index 
>> 23803c7d..8791419c 100644
>> --- a/kernel/rcu/Makefile
>> +++ b/kernel/rcu/Makefile
>> @@ -2,7 +2,7 @@
>>  # and is generally not a function of system call inputs.
>>  KCOV_INSTRUMENT := n
>>
>> -obj-y += update.o sync.o
>> +obj-y += update.o sync.o prcu.o
>>  obj-$(CONFIG_CLASSIC_SRCU) += srcu.o
>>  obj-$(CONFIG_TREE_SRCU) += srcutree.o
>>  obj-$(CONFIG_TINY_SRCU) += srcutiny.o diff --git a/kernel/rcu/prcu.c 
>> b/kernel/rcu/prcu.c new file mode 100644 index ..a00b9420
>> --- /dev/null
>> +++ b/kernel/rcu/prcu.c
>> @@ -0,0 +1,125 @@
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +DEFINE_PER_CPU_SHARED_ALIGNED(struct prcu_local_struct, prcu_local);
>> +
>> +struct prcu_struct global_prcu = {
>> +   .global_version = ATOMIC64_INIT(0),
>> +   .active_ctr = ATOMIC_INIT(0),
>> +   .mtx = __MUTEX_INITIALIZER(global_prcu.mtx),
>> +   .wait_q = __WAIT_QUEUE_HEAD_INITIALIZER(global_prcu.wait_q)
>> +};
>> +struct prcu_struct *prcu = _prcu;
>> +
>> +static inline void prcu_report(struct prcu_local_struct *local) {
>> +   unsigned long long global_version;
>> +   unsigned long long local_version;
>> +
>> +   global_version = atomic64_read(>global_version);
>> +   local_version = local->version;
>> +   if (global_version > local_version)
>> +   cmpxchg(>version, local_version, 
>> + global_version);
>
>It is called with irq-disabled, and local->version can't be modified on other 
>cpu. why cmpxchg is needed?

No, it will also be called by prcu_read_unlock in this implementation.

>> +}
>> +
>> +void prcu_read_lock(void)
>> +{
>> +   struct prcu_local_struct *local;
>> +
>> +   local = get_cpu_ptr(_local);
>> +   if (!local->online) {
>> +   WRITE_ONCE(local->online, 1);
>> +   smp_mb();
>
>What's is the paired code?

It is paired with the mutex_lock in synchronize_prcu.
It is used to ensure that if writer see the online is false, there must be no 
online reader on this core.

>
>> +   }
>> +
>> +   local->locked++;
>> +   put_cpu_ptr(_local);
>> +}
>> +EXPORT_SYMBOL(prcu_read_lock);
>> +
>> +void prcu_read_unlock(void)
>> +{
>> +   int locked;
>> +   struct prcu_local_struct *local;
>> +
>> +   barrier();
>> +   local = get_cpu_ptr(_local);
>> +   locked = local->locked;
>> +   if 

RE: [PATCH RFC 01/16] prcu: Add PRCU implementation

[zhangheng (AC)]

>-Original Message-
>From: jiangshan...@gmail.com [mailto:jiangshan...@gmail.com] On Behalf Of Lai 
>Jiangshan
>Sent: 2018年1月29日 17:11
>To: liangli...@huawei.com
>Cc: Paul E. McKenney ; Guohanjun (Hanjun Guo) 
>; zhangheng (AC) ; Chenhaibo (Haibo, 
>OS Lab) ; lihao.li...@gmail.com; LKML 
>
>Subject: Re: [PATCH RFC 01/16] prcu: Add PRCU implementation
>
>On Tue, Jan 23, 2018 at 3:59 PM,   wrote:
>> From: Heng Zhang 
>>
>> This RCU implementation (PRCU) is based on a fast consensus protocol 
>> published in the following paper:
>>
>> Fast Consensus Using Bounded Staleness for Scalable Read-mostly 
>> Synchronization.
>> Haibo Chen, Heng Zhang, Ran Liu, Binyu Zang, and Haibing Guan.
>> IEEE Transactions on Parallel and Distributed Systems (TPDS), 2016.
>> https://dl.acm.org/citation.cfm?id=3024114.3024143
>>
>> Signed-off-by: Heng Zhang 
>> Signed-off-by: Lihao Liang 
>> ---
>>  include/linux/prcu.h |  37 +++
>>  kernel/rcu/Makefile  |   2 +-
>>  kernel/rcu/prcu.c| 125 
>> +++
>>  kernel/sched/core.c  |   2 +
>>  4 files changed, 165 insertions(+), 1 deletion(-)  create mode 100644 
>> include/linux/prcu.h  create mode 100644 kernel/rcu/prcu.c
>>
>> diff --git a/include/linux/prcu.h b/include/linux/prcu.h new file mode 
>> 100644 index ..653b4633
>> --- /dev/null
>> +++ b/include/linux/prcu.h
>> @@ -0,0 +1,37 @@
>> +#ifndef __LINUX_PRCU_H
>> +#define __LINUX_PRCU_H
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define CONFIG_PRCU
>> +
>> +struct prcu_local_struct {
>> +   unsigned int locked;
>> +   unsigned int online;
>> +   unsigned long long version;
>> +};
>> +
>> +struct prcu_struct {
>> +   atomic64_t global_version;
>> +   atomic_t active_ctr;
>> +   struct mutex mtx;
>> +   wait_queue_head_t wait_q;
>> +};
>> +
>> +#ifdef CONFIG_PRCU
>> +void prcu_read_lock(void);
>> +void prcu_read_unlock(void);
>> +void synchronize_prcu(void);
>> +void prcu_note_context_switch(void);
>> +
>> +#else /* #ifdef CONFIG_PRCU */
>> +
>> +#define prcu_read_lock() do {} while (0) #define prcu_read_unlock() 
>> +do {} while (0) #define synchronize_prcu() do {} while (0) #define 
>> +prcu_note_context_switch() do {} while (0)
>> +
>> +#endif /* #ifdef CONFIG_PRCU */
>> +#endif /* __LINUX_PRCU_H */
>> diff --git a/kernel/rcu/Makefile b/kernel/rcu/Makefile index 
>> 23803c7d..8791419c 100644
>> --- a/kernel/rcu/Makefile
>> +++ b/kernel/rcu/Makefile
>> @@ -2,7 +2,7 @@
>>  # and is generally not a function of system call inputs.
>>  KCOV_INSTRUMENT := n
>>
>> -obj-y += update.o sync.o
>> +obj-y += update.o sync.o prcu.o
>>  obj-$(CONFIG_CLASSIC_SRCU) += srcu.o
>>  obj-$(CONFIG_TREE_SRCU) += srcutree.o
>>  obj-$(CONFIG_TINY_SRCU) += srcutiny.o diff --git a/kernel/rcu/prcu.c 
>> b/kernel/rcu/prcu.c new file mode 100644 index ..a00b9420
>> --- /dev/null
>> +++ b/kernel/rcu/prcu.c
>> @@ -0,0 +1,125 @@
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +DEFINE_PER_CPU_SHARED_ALIGNED(struct prcu_local_struct, prcu_local);
>> +
>> +struct prcu_struct global_prcu = {
>> +   .global_version = ATOMIC64_INIT(0),
>> +   .active_ctr = ATOMIC_INIT(0),
>> +   .mtx = __MUTEX_INITIALIZER(global_prcu.mtx),
>> +   .wait_q = __WAIT_QUEUE_HEAD_INITIALIZER(global_prcu.wait_q)
>> +};
>> +struct prcu_struct *prcu = _prcu;
>> +
>> +static inline void prcu_report(struct prcu_local_struct *local) {
>> +   unsigned long long global_version;
>> +   unsigned long long local_version;
>> +
>> +   global_version = atomic64_read(>global_version);
>> +   local_version = local->version;
>> +   if (global_version > local_version)
>> +   cmpxchg(>version, local_version, 
>> + global_version);
>
>It is called with irq-disabled, and local->version can't be modified on other 
>cpu. why cmpxchg is needed?

No, it will also be called by prcu_read_unlock in this implementation.

>> +}
>> +
>> +void prcu_read_lock(void)
>> +{
>> +   struct prcu_local_struct *local;
>> +
>> +   local = get_cpu_ptr(_local);
>> +   if (!local->online) {
>> +   WRITE_ONCE(local->online, 1);
>> +   smp_mb();
>
>What's is the paired code?

It is paired with the mutex_lock in synchronize_prcu.
It is used to ensure that if writer see the online is false, there must be no 
online reader on this core.

>
>> +   }
>> +
>> +   local->locked++;
>> +   put_cpu_ptr(_local);
>> +}
>> +EXPORT_SYMBOL(prcu_read_lock);
>> +
>> +void prcu_read_unlock(void)
>> +{
>> +   int locked;
>> +   struct prcu_local_struct *local;
>> +
>> +   barrier();
>> +   local = get_cpu_ptr(_local);
>> +   locked = local->locked;
>> +   if (locked) {
>> +   local->locked--;
>> +   if (locked == 1)
>> +   prcu_report(local);
>> +   put_cpu_ptr(_local);
>> +   } else {
>> +   

Re: Question about dmesg/sysfs output when retpoline config is disabled

[Dou Liyang]

Hi Misono-san,

At 01/30/2018 12:52 PM, Misono, Tomohiro wrote:

Hello,

I think dmesg/sysfs output messages are not suitable if retpoline config is off:

I intentionally compiled the kernel 4.15.0 with CONFIG_RETPOLINE=n for test and
boot it with the following kernel command line option to check dmesg/sysfs:

(a) no command line option or "spectre_v2=on" or "spectre_v2=auto"
$ dmesg | grep -i spectre
[0.017714] Spectre V2 mitigation: Vulnerable: Minimal generic ASM retpoline
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Minimal generic ASM retpoline

(b) "spectre_v2=off"
$ dmesg | grep -i spectre
[0.017002] Spectre V2 mitigation: disabled on command line.
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable

(c) "spectre_v2=retpoline"
$ dmesg | grep -i spectre
[0.018002] Spectre V2 mitigation: kernel not compiled with retpoline; no 
mitigation available!
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable

I think the output of (c) is correct for this case, or are these outputs 
actually right?

Also, the output of (a) is the same with following condition:
  (1) CONFIG_RETPOLINE=n, and
  (2) CONFIG_RETPOLINE=y but the compiler did not support retpoline
These cannot be distinguished unless option of (c) is explicitly used.



+Cc maintainers and x86 mail list... first.

IMO, Selecting 'on' or 'auto' to "spectre_v2=" should also consider the
setting of the CONFIG_RETPOLINE configuration option.

So, check if CONFIG_RETPOLINE is y before setup CPU capability.

Thanks,
dou.

--8<-
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 390b3dc3d438..10188f856099 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -193,8 +193,9 @@ static void __init spectre_v2_select_mitigation(void)
case SPECTRE_V2_CMD_FORCE:
/* FALLTRHU */
case SPECTRE_V2_CMD_AUTO:
-   goto retpoline_auto;
-
+   if (IS_ENABLED(CONFIG_RETPOLINE))
+   goto retpoline_auto;
+   break;
case SPECTRE_V2_CMD_RETPOLINE_AMD:
if (IS_ENABLED(CONFIG_RETPOLINE))
goto retpoline_amd;


Regards,
Tomohiro Misono

Re: Question about dmesg/sysfs output when retpoline config is disabled

[Dou Liyang]

Hi Misono-san,

At 01/30/2018 12:52 PM, Misono, Tomohiro wrote:

Hello,

I think dmesg/sysfs output messages are not suitable if retpoline config is off:

I intentionally compiled the kernel 4.15.0 with CONFIG_RETPOLINE=n for test and
boot it with the following kernel command line option to check dmesg/sysfs:

(a) no command line option or "spectre_v2=on" or "spectre_v2=auto"
$ dmesg | grep -i spectre
[0.017714] Spectre V2 mitigation: Vulnerable: Minimal generic ASM retpoline
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Minimal generic ASM retpoline

(b) "spectre_v2=off"
$ dmesg | grep -i spectre
[0.017002] Spectre V2 mitigation: disabled on command line.
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable

(c) "spectre_v2=retpoline"
$ dmesg | grep -i spectre
[0.018002] Spectre V2 mitigation: kernel not compiled with retpoline; no 
mitigation available!
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable

I think the output of (c) is correct for this case, or are these outputs 
actually right?

Also, the output of (a) is the same with following condition:
  (1) CONFIG_RETPOLINE=n, and
  (2) CONFIG_RETPOLINE=y but the compiler did not support retpoline
These cannot be distinguished unless option of (c) is explicitly used.



+Cc maintainers and x86 mail list... first.

IMO, Selecting 'on' or 'auto' to "spectre_v2=" should also consider the
setting of the CONFIG_RETPOLINE configuration option.

So, check if CONFIG_RETPOLINE is y before setup CPU capability.

Thanks,
dou.

--8<-
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 390b3dc3d438..10188f856099 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -193,8 +193,9 @@ static void __init spectre_v2_select_mitigation(void)
case SPECTRE_V2_CMD_FORCE:
/* FALLTRHU */
case SPECTRE_V2_CMD_AUTO:
-   goto retpoline_auto;
-
+   if (IS_ENABLED(CONFIG_RETPOLINE))
+   goto retpoline_auto;
+   break;
case SPECTRE_V2_CMD_RETPOLINE_AMD:
if (IS_ENABLED(CONFIG_RETPOLINE))
goto retpoline_amd;


Regards,
Tomohiro Misono

[PATCH] ACPI: Parse entire table as a term_list for Dell XPS 9570 and Precision M5530

[Kai-Heng Feng]

The i2c touchpad on Dell XPS 9570 and Precision M5530 doesn't work out
of box.

The touchpad relies on its _INI method to update its _HID value from
 to SYNA2393.
Also, the _STA relies on value of I2CN to report correct status.

Set acpi_gbl_parse_table_as_term_list so the value of I2CN can be
correctly set up, and _INI can get run. The ACPI table in this machine
is designed to get parsed this way.

Also, change the quirk table to a more generic name.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198515
Cc: Mario Limonciello 
Signed-off-by: Kai-Heng Feng 
---
v2: Andy's suggestion, merge to a single quirk table.

 drivers/acpi/bus.c | 35 ---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 4d0979e02a28..3999e175b6f4 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -65,9 +65,38 @@ static int set_copy_dsdt(const struct dmi_system_id *id)
acpi_gbl_copy_dsdt_locally = 1;
return 0;
 }
+
+static int set_gbl_term_list(const struct dmi_system_id *id)
+{
+   pr_notice("%s detected - parse the entire table as a term_list\n",
+ id->ident);
+   acpi_gbl_parse_table_as_term_list = 1;
+   return 0;
+}
 #endif
 
-static const struct dmi_system_id dsdt_dmi_table[] __initconst = {
+static const struct dmi_system_id acpi_quirks_dmi_table[] __initconst = {
+   /*
+* Touchpad on Dell XPS 9570/Precision M5530 doesn't work under I2C
+* mode.
+* https://bugzilla.kernel.org/show_bug.cgi?id=198515
+*/
+   {
+   .callback = set_gbl_term_list,
+   .ident = "Dell Precision M5530",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Precision M5530"),
+   },
+   },
+   {
+   .callback = set_gbl_term_list,
+   .ident = "Dell XPS 15 9570",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9570"),
+   },
+   },
/*
 * Invoke DSDT corruption work-around on all Toshiba Satellite.
 * https://bugzilla.kernel.org/show_bug.cgi?id=14679
@@ -83,7 +112,7 @@ static const struct dmi_system_id dsdt_dmi_table[] 
__initconst = {
{}
 };
 #else
-static const struct dmi_system_id dsdt_dmi_table[] __initconst = {
+static const struct dmi_system_id acpi_quirks_dmi_table[] __initconst = {
{}
 };
 #endif
@@ -1005,7 +1034,7 @@ void __init acpi_early_init(void)
 * If the machine falls into the DMI check table,
 * DSDT will be copied to memory
 */
-   dmi_check_system(dsdt_dmi_table);
+   dmi_check_system(acpi_quirks_dmi_table);
 
status = acpi_reallocate_root_table();
if (ACPI_FAILURE(status)) {
-- 
2.15.1

[PATCH] ACPI: Parse entire table as a term_list for Dell XPS 9570 and Precision M5530

[Kai-Heng Feng]

The i2c touchpad on Dell XPS 9570 and Precision M5530 doesn't work out
of box.

The touchpad relies on its _INI method to update its _HID value from
 to SYNA2393.
Also, the _STA relies on value of I2CN to report correct status.

Set acpi_gbl_parse_table_as_term_list so the value of I2CN can be
correctly set up, and _INI can get run. The ACPI table in this machine
is designed to get parsed this way.

Also, change the quirk table to a more generic name.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198515
Cc: Mario Limonciello 
Signed-off-by: Kai-Heng Feng 
---
v2: Andy's suggestion, merge to a single quirk table.

 drivers/acpi/bus.c | 35 ---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 4d0979e02a28..3999e175b6f4 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -65,9 +65,38 @@ static int set_copy_dsdt(const struct dmi_system_id *id)
acpi_gbl_copy_dsdt_locally = 1;
return 0;
 }
+
+static int set_gbl_term_list(const struct dmi_system_id *id)
+{
+   pr_notice("%s detected - parse the entire table as a term_list\n",
+ id->ident);
+   acpi_gbl_parse_table_as_term_list = 1;
+   return 0;
+}
 #endif
 
-static const struct dmi_system_id dsdt_dmi_table[] __initconst = {
+static const struct dmi_system_id acpi_quirks_dmi_table[] __initconst = {
+   /*
+* Touchpad on Dell XPS 9570/Precision M5530 doesn't work under I2C
+* mode.
+* https://bugzilla.kernel.org/show_bug.cgi?id=198515
+*/
+   {
+   .callback = set_gbl_term_list,
+   .ident = "Dell Precision M5530",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Precision M5530"),
+   },
+   },
+   {
+   .callback = set_gbl_term_list,
+   .ident = "Dell XPS 15 9570",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9570"),
+   },
+   },
/*
 * Invoke DSDT corruption work-around on all Toshiba Satellite.
 * https://bugzilla.kernel.org/show_bug.cgi?id=14679
@@ -83,7 +112,7 @@ static const struct dmi_system_id dsdt_dmi_table[] 
__initconst = {
{}
 };
 #else
-static const struct dmi_system_id dsdt_dmi_table[] __initconst = {
+static const struct dmi_system_id acpi_quirks_dmi_table[] __initconst = {
{}
 };
 #endif
@@ -1005,7 +1034,7 @@ void __init acpi_early_init(void)
 * If the machine falls into the DMI check table,
 * DSDT will be copied to memory
 */
-   dmi_check_system(dsdt_dmi_table);
+   dmi_check_system(acpi_quirks_dmi_table);
 
status = acpi_reallocate_root_table();
if (ACPI_FAILURE(status)) {
-- 
2.15.1

Re: [PATCH 1/1] scsi: ufs-qcom: remove broken hci version quirk

[Vivek Gautam]

Hi Asutosh,


On 1/30/2018 10:11 AM, Asutosh Das wrote:

From: Subhash Jadavani 

UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION is only applicable for QCOM UFS host
controller version 2.x.y and this has been fixed from version 3.x.y
onwards, hence this change removes this quirk for version 3.x.y onwards.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Asutosh Das 
---


This patch and all other ufs patches that you have posted recently,
do they all fall under one 'ufs-qcom fixes' patch series for fixes that
we would want to do?
If it is so, then please club them together in a series, so that
it's easy for reviewers and maintainers to review, and keep track
of all the patches that can get-in after the reviews.
If they belong to two or more separate patch-series then please
create such patch series.
It's difficult to read through a lot of [PATCH 1/1] ... patch.

Regards
Vivek


  drivers/scsi/ufs/ufs-qcom.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2..221820a 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1098,7 +1098,7 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
hba->quirks |= UFSHCD_QUIRK_BROKEN_LCC;
}
  
-	if (host->hw_ver.major >= 0x2) {

+   if (host->hw_ver.major == 0x2) {
hba->quirks |= UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION;
  
  		if (!ufs_qcom_cap_qunipro(host))

Re: [PATCH 1/1] scsi: ufs-qcom: remove broken hci version quirk

[Vivek Gautam]

Hi Asutosh,


On 1/30/2018 10:11 AM, Asutosh Das wrote:

From: Subhash Jadavani 

UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION is only applicable for QCOM UFS host
controller version 2.x.y and this has been fixed from version 3.x.y
onwards, hence this change removes this quirk for version 3.x.y onwards.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Asutosh Das 
---


This patch and all other ufs patches that you have posted recently,
do they all fall under one 'ufs-qcom fixes' patch series for fixes that
we would want to do?
If it is so, then please club them together in a series, so that
it's easy for reviewers and maintainers to review, and keep track
of all the patches that can get-in after the reviews.
If they belong to two or more separate patch-series then please
create such patch series.
It's difficult to read through a lot of [PATCH 1/1] ... patch.

Regards
Vivek


  drivers/scsi/ufs/ufs-qcom.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2..221820a 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1098,7 +1098,7 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
hba->quirks |= UFSHCD_QUIRK_BROKEN_LCC;
}
  
-	if (host->hw_ver.major >= 0x2) {

+   if (host->hw_ver.major == 0x2) {
hba->quirks |= UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION;
  
  		if (!ufs_qcom_cap_qunipro(host))

[PATCH 2/2] fsi: core: Add check for master property no-scan-on-init

[Joel Stanley]

From: Christopher Bostic 

Prior to scanning a master check if the optional property
no-scan-on-init is present.  If it is then avoid scanning.  This is
necessary in cases where a master scan could interfere with another
FSI master on the same bus.

Signed-off-by: Christopher Bostic 
Acked-by: Jeremy Kerr 
Signed-off-by: Joel Stanley 
---
 drivers/fsi/fsi-core.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 8d8b25809452..4c03d6933646 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -901,6 +901,7 @@ static DEVICE_ATTR(break, 0200, NULL, master_break_store);
 int fsi_master_register(struct fsi_master *master)
 {
int rc;
+   struct device_node *np;
 
if (!master)
return -EINVAL;
@@ -928,7 +929,9 @@ int fsi_master_register(struct fsi_master *master)
return rc;
}
 
-   fsi_master_scan(master);
+   np = dev_of_node(>dev);
+   if (!of_property_read_bool(np, "no-scan-on-init"))
+   fsi_master_scan(master);
 
return 0;
 }
-- 
2.15.1

[PATCH 2/2] fsi: core: Add check for master property no-scan-on-init

[Joel Stanley]

From: Christopher Bostic 

Prior to scanning a master check if the optional property
no-scan-on-init is present.  If it is then avoid scanning.  This is
necessary in cases where a master scan could interfere with another
FSI master on the same bus.

Signed-off-by: Christopher Bostic 
Acked-by: Jeremy Kerr 
Signed-off-by: Joel Stanley 
---
 drivers/fsi/fsi-core.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 8d8b25809452..4c03d6933646 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -901,6 +901,7 @@ static DEVICE_ATTR(break, 0200, NULL, master_break_store);
 int fsi_master_register(struct fsi_master *master)
 {
int rc;
+   struct device_node *np;
 
if (!master)
return -EINVAL;
@@ -928,7 +929,9 @@ int fsi_master_register(struct fsi_master *master)
return rc;
}
 
-   fsi_master_scan(master);
+   np = dev_of_node(>dev);
+   if (!of_property_read_bool(np, "no-scan-on-init"))
+   fsi_master_scan(master);
 
return 0;
 }
-- 
2.15.1

[PATCH 1/2] dt-bindings: fsi: Add optional property no-scan-on-init

[Joel Stanley]

From: Christopher Bostic 

Add an optional FSI master property 'no-scan-on-init.  This
can be specified to indicate that a master should not be
automatically scanned at init time.  This is required in cases
where a scan could interfere with another FSI master on the same
bus.

Signed-off-by: Christopher Bostic 
Acked-by: Jeremy Kerr 
Signed-off-by: Joel Stanley 
---
 Documentation/devicetree/bindings/fsi/fsi.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt 
b/Documentation/devicetree/bindings/fsi/fsi.txt
index 4eaf488d4015..ab516c673a4b 100644
--- a/Documentation/devicetree/bindings/fsi/fsi.txt
+++ b/Documentation/devicetree/bindings/fsi/fsi.txt
@@ -56,6 +56,13 @@ addresses (link index and slave ID), and no size:
 #address-cells = <2>;
 #size-cells = <0>;
 
+An optional boolean property can be added to indicate that a particular master
+should not scan for connected devices at initialization time.  This is
+necessary in cases where a scan could cause arbitration issues with other
+masters that may be present on the bus.
+
+no-scan-on-init;
+
 FSI slaves
 --
 
-- 
2.15.1

[PATCH 1/2] dt-bindings: fsi: Add optional property no-scan-on-init

[Joel Stanley]

From: Christopher Bostic 

Add an optional FSI master property 'no-scan-on-init.  This
can be specified to indicate that a master should not be
automatically scanned at init time.  This is required in cases
where a scan could interfere with another FSI master on the same
bus.

Signed-off-by: Christopher Bostic 
Acked-by: Jeremy Kerr 
Signed-off-by: Joel Stanley 
---
 Documentation/devicetree/bindings/fsi/fsi.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt 
b/Documentation/devicetree/bindings/fsi/fsi.txt
index 4eaf488d4015..ab516c673a4b 100644
--- a/Documentation/devicetree/bindings/fsi/fsi.txt
+++ b/Documentation/devicetree/bindings/fsi/fsi.txt
@@ -56,6 +56,13 @@ addresses (link index and slave ID), and no size:
 #address-cells = <2>;
 #size-cells = <0>;
 
+An optional boolean property can be added to indicate that a particular master
+should not scan for connected devices at initialization time.  This is
+necessary in cases where a scan could cause arbitration issues with other
+masters that may be present on the bus.
+
+no-scan-on-init;
+
 FSI slaves
 --
 
-- 
2.15.1

[PATCH 0/2] fsi: add property to avoid scanning at boot

[Joel Stanley]

These two patches from Chris add an optional property that says the
FSI attached hardware cannot cope with being probed unless the state of
that hardware is known.

This allows the driver to eg. defer to userspace which can make this
decision.

I am collecting patches for a FSI tree to send to Greg, so I am just after
review from the device tree people.

Christopher Bostic (2):
  dt-bindings: fsi: Add optional property no-scan-on-init
  fsi: core: Add check for master property no-scan-on-init

 Documentation/devicetree/bindings/fsi/fsi.txt | 7 +++
 drivers/fsi/fsi-core.c| 5 -
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
2.15.1

[PATCH 0/2] fsi: add property to avoid scanning at boot

[Joel Stanley]

These two patches from Chris add an optional property that says the
FSI attached hardware cannot cope with being probed unless the state of
that hardware is known.

This allows the driver to eg. defer to userspace which can make this
decision.

I am collecting patches for a FSI tree to send to Greg, so I am just after
review from the device tree people.

Christopher Bostic (2):
  dt-bindings: fsi: Add optional property no-scan-on-init
  fsi: core: Add check for master property no-scan-on-init

 Documentation/devicetree/bindings/fsi/fsi.txt | 7 +++
 drivers/fsi/fsi-core.c| 5 -
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
2.15.1

Re: [perf] perf probe fails sometimes on 4.9

[Masami Hiramatsu]

On Mon, 29 Jan 2018 22:00:52 +0530
Pintu Kumar  wrote:

> Dear Masami,
> 
> Thank you so much for your reply.
> Please find some of my answers inline.
> 
> 
> On Mon, Jan 29, 2018 at 7:47 PM, Masami Hiramatsu  wrote:
> > On Mon, 29 Jan 2018 13:40:34 +0530
> > Pintu Kumar  wrote:
> >
> >> Hi All,
> >>
> >> 'perf probe' is failing sometimes on 4.9.20 with AMD-64.
> >> # perf probe --add schedule
> >> schedule is out of .text, skip it.
> >>   Error: Failed to add events.
> >>
> >> If any one have come across this problem please let me know the cause.
> >
> > Hi Pintu,
> >
> > Could you run it with --vv?
> >
> Ok, I will send verbose output by tomorrow.
> 
> >>
> >> Note: I don't have CONFIG_DEBUG_INFO enabled in kernel. Is this the 
> >> problem?
> >
> > Without it, you can not probe source-level probe nor trace local variable.
> >
> 
> Currently I am facing problem in enabling DEBUG_INFO in our kernel 4.9.20
> However, I will try to manually include "-g" option during compilation.
> 
> >> However, I manually copied the vmlinux file to /boot/ directory, but
> >> still it does not work.
> >
> > That doesn't work.
> > CONFIG_DEBUG_INFO option enables gcc to compile kernel with extra debuginfo.
> > Without that option, debuginfo is not generated with vmlinux.
> >
> >>
> >> I checked upstream patches until 4.15 but could not find any clue.
> >> Please let me know if there is any fixes available for this.
> >
> > Could you also ensure that you run perf by root user?
> >
> 
> Yes I am running with root user.
> 
> My concern is sometimes it works but sometimes it fails.

What I thought was that your kernel enables kptr_strict(but maybe not.)
Can you also try to find "schedule" and "_etext" functions in
/proc/kallsyms?

> I still needs to figure out, in which condition it works and which
> condition it fails.

Yeah, it is important.

> Usually I noticed that in fresh reboot case it works.

So, after a while, it doesn't work again? If so, it sounds like a daemon
process changes settings in background.

Thank you,

> 
> 
> > Thank you,
> >
> >
> >>
> >>
> >> Thank You!
> >> Regards,
> >> Pintu
> >>
> >>
> >> On Thu, Jan 25, 2018 at 7:09 PM, Pintu Kumar  wrote:
> >> > Hi,
> >> >
> >> > ** Changed the subject now, since these issues are related to general
> >> > perf commands.
> >> >
> >> > Following are the issues:
> >> >
> >> > 1) perf probe --add schedule - FAILED
> >> > output:
> >> > schedule is out of .text, skip it.
> >> >   Error: Failed to add events.
> >> >
> >> > what is the issue here?
> >> > Sometimes it pass and sometimes it fails...
> >> > Similar is the case of 'perf inject' as well.
> >> >
> >> > 2) perf test - 1 FAILURE
> >> > 37.1: Test basic BPF filtering   : FAILED!
> >> > 37.2: Test BPF prologue generation   : Skip
> >> > 37.3: Test BPF relocation checker: Skip
> >> >
> >> > bpf: config program 'func=SyS_epoll_wait'
> >> > symbol:SyS_epoll_wait file:(null) line:0 offset:0 return:0 lazy:(null)
> >> > bpf: config 'func=SyS_epoll_wait' is ok
> >> > Looking at the vmlinux_path (8 entries long)
> >> > Using /boot/vmlinux for symbols
> >> > Could not open debuginfo. Try to use symbols.
> >> > SyS_epoll_wait is out of .text, skip it.
> >> > bpf_probe: failed to convert perf probe eventsFailed to add events
> >> > selected by BPF
> >> > test child finished with -1
> >> >  end 
> >> > Test BPF filter subtest 0: FAILED!
> >> >
> >> > Looks like both 1,2 are related.
> >> > Since, CONFIG_DEBUG_INFO is not enabled, I manually copied the vmlinux
> >> > to /boot/ folder.
> >> >
> >> > ---
> >> > Some more info:
> >> >
> >> > Kernel build dir is set to /lib/modules/4.9.20-sc-amd-x86-64/build
> >> > set env: KBUILD_DIR=/lib/modules/4.9.20-sc-amd-x86-64/build
> >> > unset env: KBUILD_OPTS
> >> > include option is set to  -nostdinc -isystem
> >> > /usr/lib/gcc/x86_64-linux-gnu/5/include -I./arch/x86/include
> >> > -I./arch/x86/include/generated/uapi -I./arch/x86/include/generated
> >> > -I./include -I./arch/x86/include/uapi -I./include/uapi
> >> > -I./include/generated/uapi -include ./include/linux/kconfig.h
> >> > set env: NR_CPUS=8
> >> > set env: LINUX_VERSION_CODE=0x40914
> >> > set env: CLANG_EXEC=/usr/bin/clang
> >> > set env: CLANG_OPTIONS=-xc
> >> > set env: KERNEL_INC_OPTIONS= -nostdinc -isystem
> >> > /usr/lib/gcc/x86_64-linux-gnu/5/include -I./arch/x86/include
> >> > -I./arch/x86/include/generated/uapi -I./arch/x86/include/generated
> >> > -I./include -I./arch/x86/include/uapi -I./include/uapi
> >> > -I./include/generated/uapi -include ./include/linux/kconfig.h
> >> > set env: WORKING_DIR=/lib/modules/4.9.20-sc-amd-x86-64/build
> >> >
> >> >
> >> > If you have any clue about these failure please help me.
> >> >
> >> >
> >> > Thanks,
> >> > Pintu
> >> >
> >> >
> >> > On Wed, Jan 24, 2018 at 8:23 PM, Pintu Kumar 

Re: [perf] perf probe fails sometimes on 4.9

[Masami Hiramatsu]

On Mon, 29 Jan 2018 22:00:52 +0530
Pintu Kumar  wrote:

> Dear Masami,
> 
> Thank you so much for your reply.
> Please find some of my answers inline.
> 
> 
> On Mon, Jan 29, 2018 at 7:47 PM, Masami Hiramatsu  wrote:
> > On Mon, 29 Jan 2018 13:40:34 +0530
> > Pintu Kumar  wrote:
> >
> >> Hi All,
> >>
> >> 'perf probe' is failing sometimes on 4.9.20 with AMD-64.
> >> # perf probe --add schedule
> >> schedule is out of .text, skip it.
> >>   Error: Failed to add events.
> >>
> >> If any one have come across this problem please let me know the cause.
> >
> > Hi Pintu,
> >
> > Could you run it with --vv?
> >
> Ok, I will send verbose output by tomorrow.
> 
> >>
> >> Note: I don't have CONFIG_DEBUG_INFO enabled in kernel. Is this the 
> >> problem?
> >
> > Without it, you can not probe source-level probe nor trace local variable.
> >
> 
> Currently I am facing problem in enabling DEBUG_INFO in our kernel 4.9.20
> However, I will try to manually include "-g" option during compilation.
> 
> >> However, I manually copied the vmlinux file to /boot/ directory, but
> >> still it does not work.
> >
> > That doesn't work.
> > CONFIG_DEBUG_INFO option enables gcc to compile kernel with extra debuginfo.
> > Without that option, debuginfo is not generated with vmlinux.
> >
> >>
> >> I checked upstream patches until 4.15 but could not find any clue.
> >> Please let me know if there is any fixes available for this.
> >
> > Could you also ensure that you run perf by root user?
> >
> 
> Yes I am running with root user.
> 
> My concern is sometimes it works but sometimes it fails.

What I thought was that your kernel enables kptr_strict(but maybe not.)
Can you also try to find "schedule" and "_etext" functions in
/proc/kallsyms?

> I still needs to figure out, in which condition it works and which
> condition it fails.

Yeah, it is important.

> Usually I noticed that in fresh reboot case it works.

So, after a while, it doesn't work again? If so, it sounds like a daemon
process changes settings in background.

Thank you,

> 
> 
> > Thank you,
> >
> >
> >>
> >>
> >> Thank You!
> >> Regards,
> >> Pintu
> >>
> >>
> >> On Thu, Jan 25, 2018 at 7:09 PM, Pintu Kumar  wrote:
> >> > Hi,
> >> >
> >> > ** Changed the subject now, since these issues are related to general
> >> > perf commands.
> >> >
> >> > Following are the issues:
> >> >
> >> > 1) perf probe --add schedule - FAILED
> >> > output:
> >> > schedule is out of .text, skip it.
> >> >   Error: Failed to add events.
> >> >
> >> > what is the issue here?
> >> > Sometimes it pass and sometimes it fails...
> >> > Similar is the case of 'perf inject' as well.
> >> >
> >> > 2) perf test - 1 FAILURE
> >> > 37.1: Test basic BPF filtering   : FAILED!
> >> > 37.2: Test BPF prologue generation   : Skip
> >> > 37.3: Test BPF relocation checker: Skip
> >> >
> >> > bpf: config program 'func=SyS_epoll_wait'
> >> > symbol:SyS_epoll_wait file:(null) line:0 offset:0 return:0 lazy:(null)
> >> > bpf: config 'func=SyS_epoll_wait' is ok
> >> > Looking at the vmlinux_path (8 entries long)
> >> > Using /boot/vmlinux for symbols
> >> > Could not open debuginfo. Try to use symbols.
> >> > SyS_epoll_wait is out of .text, skip it.
> >> > bpf_probe: failed to convert perf probe eventsFailed to add events
> >> > selected by BPF
> >> > test child finished with -1
> >> >  end 
> >> > Test BPF filter subtest 0: FAILED!
> >> >
> >> > Looks like both 1,2 are related.
> >> > Since, CONFIG_DEBUG_INFO is not enabled, I manually copied the vmlinux
> >> > to /boot/ folder.
> >> >
> >> > ---
> >> > Some more info:
> >> >
> >> > Kernel build dir is set to /lib/modules/4.9.20-sc-amd-x86-64/build
> >> > set env: KBUILD_DIR=/lib/modules/4.9.20-sc-amd-x86-64/build
> >> > unset env: KBUILD_OPTS
> >> > include option is set to  -nostdinc -isystem
> >> > /usr/lib/gcc/x86_64-linux-gnu/5/include -I./arch/x86/include
> >> > -I./arch/x86/include/generated/uapi -I./arch/x86/include/generated
> >> > -I./include -I./arch/x86/include/uapi -I./include/uapi
> >> > -I./include/generated/uapi -include ./include/linux/kconfig.h
> >> > set env: NR_CPUS=8
> >> > set env: LINUX_VERSION_CODE=0x40914
> >> > set env: CLANG_EXEC=/usr/bin/clang
> >> > set env: CLANG_OPTIONS=-xc
> >> > set env: KERNEL_INC_OPTIONS= -nostdinc -isystem
> >> > /usr/lib/gcc/x86_64-linux-gnu/5/include -I./arch/x86/include
> >> > -I./arch/x86/include/generated/uapi -I./arch/x86/include/generated
> >> > -I./include -I./arch/x86/include/uapi -I./include/uapi
> >> > -I./include/generated/uapi -include ./include/linux/kconfig.h
> >> > set env: WORKING_DIR=/lib/modules/4.9.20-sc-amd-x86-64/build
> >> >
> >> >
> >> > If you have any clue about these failure please help me.
> >> >
> >> >
> >> > Thanks,
> >> > Pintu
> >> >
> >> >
> >> > On Wed, Jan 24, 2018 at 8:23 PM, Pintu Kumar  
> >> > wrote:
> >> >> Hi,
> >> >>
> >> >> Thanks for your help.
> >> >> Yes it was a sub version issue.

RE: [PATCH RFC 01/16] prcu: Add PRCU implementation

[zhangheng (AC)]

-Original Message-
>From: Boqun Feng [mailto:boqun.f...@gmail.com] 
>Sent: 2018年1月25日 15:31
>To: Paul E. McKenney 
>Cc: liangli...@huawei.com; Guohanjun (Hanjun Guo) ; 
>zhangheng (AC) ; Chenhaibo (Haibo, OS Lab) 
>; lihao.li...@gmail.com; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH RFC 01/16] prcu: Add PRCU implementation
>
>On Wed, Jan 24, 2018 at 10:16:18PM -0800, Paul E. McKenney wrote:
>> On Tue, Jan 23, 2018 at 03:59:26PM +0800, liangli...@huawei.com wrote:
>> > From: Heng Zhang 
>> > 
>> > This RCU implementation (PRCU) is based on a fast consensus protocol 
>> > published in the following paper:
>> > 
>> > Fast Consensus Using Bounded Staleness for Scalable Read-mostly 
>> > Synchronization.
>> > Haibo Chen, Heng Zhang, Ran Liu, Binyu Zang, and Haibing Guan.
>> > IEEE Transactions on Parallel and Distributed Systems (TPDS), 2016.
>> > https://dl.acm.org/citation.cfm?id=3024114.3024143
>> > 
>> > Signed-off-by: Heng Zhang 
>> > Signed-off-by: Lihao Liang 
>> 
>> A few comments and questions interspersed.
>> 
>>  Thanx, Paul
>> 
>> > ---
>> >  include/linux/prcu.h |  37 +++
>> >  kernel/rcu/Makefile  |   2 +-
>> >  kernel/rcu/prcu.c| 125 
>> > +++
>> >  kernel/sched/core.c  |   2 +
>> >  4 files changed, 165 insertions(+), 1 deletion(-)  create mode 
>> > 100644 include/linux/prcu.h  create mode 100644 kernel/rcu/prcu.c
>> > 
>> > diff --git a/include/linux/prcu.h b/include/linux/prcu.h new file 
>> > mode 100644 index ..653b4633
>> > --- /dev/null
>> > +++ b/include/linux/prcu.h
>> > @@ -0,0 +1,37 @@
>> > +#ifndef __LINUX_PRCU_H
>> > +#define __LINUX_PRCU_H
>> > +
>> > +#include 
>> > +#include 
>> > +#include 
>> > +
>> > +#define CONFIG_PRCU
>> > +
>> > +struct prcu_local_struct {
>> > +  unsigned int locked;
>> > +  unsigned int online;
>> > +  unsigned long long version;
>> > +};
>> > +
>> > +struct prcu_struct {
>> > +  atomic64_t global_version;
>> > +  atomic_t active_ctr;
>> > +  struct mutex mtx;
>> > +  wait_queue_head_t wait_q;
>> > +};
>> > +
>> > +#ifdef CONFIG_PRCU
>> > +void prcu_read_lock(void);
>> > +void prcu_read_unlock(void);
>> > +void synchronize_prcu(void);
>> > +void prcu_note_context_switch(void);
>> > +
>> > +#else /* #ifdef CONFIG_PRCU */
>> > +
>> > +#define prcu_read_lock() do {} while (0) #define prcu_read_unlock() 
>> > +do {} while (0) #define synchronize_prcu() do {} while (0) #define 
>> > +prcu_note_context_switch() do {} while (0)
>> 
>> If CONFIG_PRCU=n and some code is built that uses PRCU, shouldn't you 
>> get a build error rather than an error-free but inoperative PRCU?
>> 
>> Of course, Peter's question about purpose of the patch set applies 
>> here as well.
>> 
>> > +
>> > +#endif /* #ifdef CONFIG_PRCU */
>> > +#endif /* __LINUX_PRCU_H */
>> > diff --git a/kernel/rcu/Makefile b/kernel/rcu/Makefile index 
>> > 23803c7d..8791419c 100644
>> > --- a/kernel/rcu/Makefile
>> > +++ b/kernel/rcu/Makefile
>> > @@ -2,7 +2,7 @@
>> >  # and is generally not a function of system call inputs.
>> >  KCOV_INSTRUMENT := n
>> > 
>> > -obj-y += update.o sync.o
>> > +obj-y += update.o sync.o prcu.o
>> >  obj-$(CONFIG_CLASSIC_SRCU) += srcu.o
>> >  obj-$(CONFIG_TREE_SRCU) += srcutree.o
>> >  obj-$(CONFIG_TINY_SRCU) += srcutiny.o diff --git 
>> > a/kernel/rcu/prcu.c b/kernel/rcu/prcu.c new file mode 100644 index 
>> > ..a00b9420
>> > --- /dev/null
>> > +++ b/kernel/rcu/prcu.c
>> > @@ -0,0 +1,125 @@
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +
>> > +#include 
>> > +
>> > +DEFINE_PER_CPU_SHARED_ALIGNED(struct prcu_local_struct, 
>> > +prcu_local);
>> > +
>> > +struct prcu_struct global_prcu = {
>> > +  .global_version = ATOMIC64_INIT(0),
>> > +  .active_ctr = ATOMIC_INIT(0),
>> > +  .mtx = __MUTEX_INITIALIZER(global_prcu.mtx),
>> > +  .wait_q = __WAIT_QUEUE_HEAD_INITIALIZER(global_prcu.wait_q)
>> > +};
>> > +struct prcu_struct *prcu = _prcu;
>> > +
>> > +static inline void prcu_report(struct prcu_local_struct *local) {
>> > +  unsigned long long global_version;
>> > +  unsigned long long local_version;
>> > +
>> > +  global_version = atomic64_read(>global_version);
>> > +  local_version = local->version;
>> > +  if (global_version > local_version)
>> > +  cmpxchg(>version, local_version, global_version); }
>> > +
>> > +void prcu_read_lock(void)
>> > +{
>> > +  struct prcu_local_struct *local;
>> > +
>> > +  local = get_cpu_ptr(_local);
>> > +  if (!local->online) {
>> > +  WRITE_ONCE(local->online, 1);
>> > +  smp_mb();
>> > +  }
>> > +
>> > +  local->locked++;
>> > +  put_cpu_ptr(_local);
>> > +}
>> > +EXPORT_SYMBOL(prcu_read_lock);
>> > +
>> > +void prcu_read_unlock(void)
>> > +{
>> > +  int locked;
>> > +  struct 

RE: [PATCH RFC 01/16] prcu: Add PRCU implementation

[zhangheng (AC)]

-Original Message-
>From: Boqun Feng [mailto:boqun.f...@gmail.com] 
>Sent: 2018年1月25日 15:31
>To: Paul E. McKenney 
>Cc: liangli...@huawei.com; Guohanjun (Hanjun Guo) ; 
>zhangheng (AC) ; Chenhaibo (Haibo, OS Lab) 
>; lihao.li...@gmail.com; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH RFC 01/16] prcu: Add PRCU implementation
>
>On Wed, Jan 24, 2018 at 10:16:18PM -0800, Paul E. McKenney wrote:
>> On Tue, Jan 23, 2018 at 03:59:26PM +0800, liangli...@huawei.com wrote:
>> > From: Heng Zhang 
>> > 
>> > This RCU implementation (PRCU) is based on a fast consensus protocol 
>> > published in the following paper:
>> > 
>> > Fast Consensus Using Bounded Staleness for Scalable Read-mostly 
>> > Synchronization.
>> > Haibo Chen, Heng Zhang, Ran Liu, Binyu Zang, and Haibing Guan.
>> > IEEE Transactions on Parallel and Distributed Systems (TPDS), 2016.
>> > https://dl.acm.org/citation.cfm?id=3024114.3024143
>> > 
>> > Signed-off-by: Heng Zhang 
>> > Signed-off-by: Lihao Liang 
>> 
>> A few comments and questions interspersed.
>> 
>>  Thanx, Paul
>> 
>> > ---
>> >  include/linux/prcu.h |  37 +++
>> >  kernel/rcu/Makefile  |   2 +-
>> >  kernel/rcu/prcu.c| 125 
>> > +++
>> >  kernel/sched/core.c  |   2 +
>> >  4 files changed, 165 insertions(+), 1 deletion(-)  create mode 
>> > 100644 include/linux/prcu.h  create mode 100644 kernel/rcu/prcu.c
>> > 
>> > diff --git a/include/linux/prcu.h b/include/linux/prcu.h new file 
>> > mode 100644 index ..653b4633
>> > --- /dev/null
>> > +++ b/include/linux/prcu.h
>> > @@ -0,0 +1,37 @@
>> > +#ifndef __LINUX_PRCU_H
>> > +#define __LINUX_PRCU_H
>> > +
>> > +#include 
>> > +#include 
>> > +#include 
>> > +
>> > +#define CONFIG_PRCU
>> > +
>> > +struct prcu_local_struct {
>> > +  unsigned int locked;
>> > +  unsigned int online;
>> > +  unsigned long long version;
>> > +};
>> > +
>> > +struct prcu_struct {
>> > +  atomic64_t global_version;
>> > +  atomic_t active_ctr;
>> > +  struct mutex mtx;
>> > +  wait_queue_head_t wait_q;
>> > +};
>> > +
>> > +#ifdef CONFIG_PRCU
>> > +void prcu_read_lock(void);
>> > +void prcu_read_unlock(void);
>> > +void synchronize_prcu(void);
>> > +void prcu_note_context_switch(void);
>> > +
>> > +#else /* #ifdef CONFIG_PRCU */
>> > +
>> > +#define prcu_read_lock() do {} while (0) #define prcu_read_unlock() 
>> > +do {} while (0) #define synchronize_prcu() do {} while (0) #define 
>> > +prcu_note_context_switch() do {} while (0)
>> 
>> If CONFIG_PRCU=n and some code is built that uses PRCU, shouldn't you 
>> get a build error rather than an error-free but inoperative PRCU?
>> 
>> Of course, Peter's question about purpose of the patch set applies 
>> here as well.
>> 
>> > +
>> > +#endif /* #ifdef CONFIG_PRCU */
>> > +#endif /* __LINUX_PRCU_H */
>> > diff --git a/kernel/rcu/Makefile b/kernel/rcu/Makefile index 
>> > 23803c7d..8791419c 100644
>> > --- a/kernel/rcu/Makefile
>> > +++ b/kernel/rcu/Makefile
>> > @@ -2,7 +2,7 @@
>> >  # and is generally not a function of system call inputs.
>> >  KCOV_INSTRUMENT := n
>> > 
>> > -obj-y += update.o sync.o
>> > +obj-y += update.o sync.o prcu.o
>> >  obj-$(CONFIG_CLASSIC_SRCU) += srcu.o
>> >  obj-$(CONFIG_TREE_SRCU) += srcutree.o
>> >  obj-$(CONFIG_TINY_SRCU) += srcutiny.o diff --git 
>> > a/kernel/rcu/prcu.c b/kernel/rcu/prcu.c new file mode 100644 index 
>> > ..a00b9420
>> > --- /dev/null
>> > +++ b/kernel/rcu/prcu.c
>> > @@ -0,0 +1,125 @@
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +
>> > +#include 
>> > +
>> > +DEFINE_PER_CPU_SHARED_ALIGNED(struct prcu_local_struct, 
>> > +prcu_local);
>> > +
>> > +struct prcu_struct global_prcu = {
>> > +  .global_version = ATOMIC64_INIT(0),
>> > +  .active_ctr = ATOMIC_INIT(0),
>> > +  .mtx = __MUTEX_INITIALIZER(global_prcu.mtx),
>> > +  .wait_q = __WAIT_QUEUE_HEAD_INITIALIZER(global_prcu.wait_q)
>> > +};
>> > +struct prcu_struct *prcu = _prcu;
>> > +
>> > +static inline void prcu_report(struct prcu_local_struct *local) {
>> > +  unsigned long long global_version;
>> > +  unsigned long long local_version;
>> > +
>> > +  global_version = atomic64_read(>global_version);
>> > +  local_version = local->version;
>> > +  if (global_version > local_version)
>> > +  cmpxchg(>version, local_version, global_version); }
>> > +
>> > +void prcu_read_lock(void)
>> > +{
>> > +  struct prcu_local_struct *local;
>> > +
>> > +  local = get_cpu_ptr(_local);
>> > +  if (!local->online) {
>> > +  WRITE_ONCE(local->online, 1);
>> > +  smp_mb();
>> > +  }
>> > +
>> > +  local->locked++;
>> > +  put_cpu_ptr(_local);
>> > +}
>> > +EXPORT_SYMBOL(prcu_read_lock);
>> > +
>> > +void prcu_read_unlock(void)
>> > +{
>> > +  int locked;
>> > +  struct prcu_local_struct *local;
>> > +
>> > +  barrier();
>> > +  local = get_cpu_ptr(_local);
>> > +  locked = local->locked;
>> > +  if (locked) {
>> > +  

[PATCH 2/2] perf trace: Fix call-graph output

[Ravi Bangoria]

Recently, Arnaldo fixed global vs event specific --max-stack usage
with commit bd3dda9ab0fb ("perf trace: Allow overriding global
--max-stack per event"). This commit is having a regression when
we don't use --max-stack at all with perf trace. Ex,

  $ ./perf trace record -g ls
  $ ./perf trace -i perf.data
 0.076 ( 0.002 ms): ls/9109 brk(
 0.196 ( 0.008 ms): ls/9109 access(filename: 0x9f998b70, mode: R
 0.209 ( 0.031 ms): ls/9109 open(filename: 0x9f998978, flags: CLOEXEC

This is missing call-traces.
After patch:

  $ ./perf trace -i perf.data
 0.076 ( 0.002 ms): ls/9109 brk(
do_syscall_trace_leave ([kernel.kallsyms])
[0] ([unknown])
syscall_exit_work ([kernel.kallsyms])
brk (/usr/lib64/ld-2.17.so)
_dl_sysdep_start (/usr/lib64/ld-2.17.so)
_dl_start_final (/usr/lib64/ld-2.17.so)
_dl_start (/usr/lib64/ld-2.17.so)
_start (/usr/lib64/ld-2.17.so)
 0.196 ( 0.008 ms): ls/9109 access(filename: 0x9f998b70, mode: R
do_syscall_trace_leave ([kernel.kallsyms])
[0] ([unknown])

Fixes: bd3dda9ab0fb ("perf trace: Allow overriding global --max-stack per 
event")
Signed-off-by: Ravi Bangoria 
---
 tools/perf/builtin-trace.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 17d11de..e90f5c3 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1661,9 +1661,12 @@ static int trace__resolve_callchain(struct trace *trace, 
struct perf_evsel *evse
struct callchain_cursor *cursor)
 {
struct addr_location al;
+   int max_stack = evsel->attr.sample_max_stack ?
+   evsel->attr.sample_max_stack :
+   trace->max_stack;
 
if (machine__resolve(trace->host, , sample) < 0 ||
-   thread__resolve_callchain(al.thread, cursor, evsel, sample, NULL, 
NULL, evsel->attr.sample_max_stack))
+   thread__resolve_callchain(al.thread, cursor, evsel, sample, NULL, 
NULL, max_stack))
return -1;
 
return 0;
-- 
1.8.3.1

Re: [PATCH] macintosh: Add module license to ans-lcd

[Daniel Axtens]

Hi,

That matches the SPDX identifier from the top of the file, so:

Reviewed-by: Daniel Axtens 

Regards,
Daniel

Larry Finger  writes:

> In kernel 4.15, the modprobe step on my PowerBook G5 started complaining that
> there was no module license for ans-lcd.
>
> Signed-off-by: Larry Finger 
> ---
>  drivers/macintosh/ans-lcd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/macintosh/ans-lcd.c b/drivers/macintosh/ans-lcd.c
> index 1de81d922d8a..c8e078b911c7 100644
> --- a/drivers/macintosh/ans-lcd.c
> +++ b/drivers/macintosh/ans-lcd.c
> @@ -201,3 +201,4 @@ anslcd_exit(void)
>  
>  module_init(anslcd_init);
>  module_exit(anslcd_exit);
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.16.1

[PATCH 2/2] perf trace: Fix call-graph output

[Ravi Bangoria]

Recently, Arnaldo fixed global vs event specific --max-stack usage
with commit bd3dda9ab0fb ("perf trace: Allow overriding global
--max-stack per event"). This commit is having a regression when
we don't use --max-stack at all with perf trace. Ex,

  $ ./perf trace record -g ls
  $ ./perf trace -i perf.data
 0.076 ( 0.002 ms): ls/9109 brk(
 0.196 ( 0.008 ms): ls/9109 access(filename: 0x9f998b70, mode: R
 0.209 ( 0.031 ms): ls/9109 open(filename: 0x9f998978, flags: CLOEXEC

This is missing call-traces.
After patch:

  $ ./perf trace -i perf.data
 0.076 ( 0.002 ms): ls/9109 brk(
do_syscall_trace_leave ([kernel.kallsyms])
[0] ([unknown])
syscall_exit_work ([kernel.kallsyms])
brk (/usr/lib64/ld-2.17.so)
_dl_sysdep_start (/usr/lib64/ld-2.17.so)
_dl_start_final (/usr/lib64/ld-2.17.so)
_dl_start (/usr/lib64/ld-2.17.so)
_start (/usr/lib64/ld-2.17.so)
 0.196 ( 0.008 ms): ls/9109 access(filename: 0x9f998b70, mode: R
do_syscall_trace_leave ([kernel.kallsyms])
[0] ([unknown])

Fixes: bd3dda9ab0fb ("perf trace: Allow overriding global --max-stack per 
event")
Signed-off-by: Ravi Bangoria 
---
 tools/perf/builtin-trace.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 17d11de..e90f5c3 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1661,9 +1661,12 @@ static int trace__resolve_callchain(struct trace *trace, 
struct perf_evsel *evse
struct callchain_cursor *cursor)
 {
struct addr_location al;
+   int max_stack = evsel->attr.sample_max_stack ?
+   evsel->attr.sample_max_stack :
+   trace->max_stack;
 
if (machine__resolve(trace->host, , sample) < 0 ||
-   thread__resolve_callchain(al.thread, cursor, evsel, sample, NULL, 
NULL, evsel->attr.sample_max_stack))
+   thread__resolve_callchain(al.thread, cursor, evsel, sample, NULL, 
NULL, max_stack))
return -1;
 
return 0;
-- 
1.8.3.1

Re: [PATCH] macintosh: Add module license to ans-lcd

[Daniel Axtens]

Hi,

That matches the SPDX identifier from the top of the file, so:

Reviewed-by: Daniel Axtens 

Regards,
Daniel

Larry Finger  writes:

> In kernel 4.15, the modprobe step on my PowerBook G5 started complaining that
> there was no module license for ans-lcd.
>
> Signed-off-by: Larry Finger 
> ---
>  drivers/macintosh/ans-lcd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/macintosh/ans-lcd.c b/drivers/macintosh/ans-lcd.c
> index 1de81d922d8a..c8e078b911c7 100644
> --- a/drivers/macintosh/ans-lcd.c
> +++ b/drivers/macintosh/ans-lcd.c
> @@ -201,3 +201,4 @@ anslcd_exit(void)
>  
>  module_init(anslcd_init);
>  module_exit(anslcd_exit);
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.16.1

[PATCH 1/2] perf tools: Add trace/beauty/generated/ into .gitignore

[Ravi Bangoria]

No functionality changes.

Signed-off-by: Ravi Bangoria 
---
 tools/perf/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/.gitignore b/tools/perf/.gitignore
index 643cc4ba..3e5135d 100644
--- a/tools/perf/.gitignore
+++ b/tools/perf/.gitignore
@@ -31,5 +31,6 @@ config.mak.autogen
 .config-detected
 util/intel-pt-decoder/inat-tables.c
 arch/*/include/generated/
+trace/beauty/generated/
 pmu-events/pmu-events.c
 pmu-events/jevents
-- 
1.8.3.1

[PATCH 1/2] perf tools: Add trace/beauty/generated/ into .gitignore

[Ravi Bangoria]

No functionality changes.

Signed-off-by: Ravi Bangoria 
---
 tools/perf/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/.gitignore b/tools/perf/.gitignore
index 643cc4ba..3e5135d 100644
--- a/tools/perf/.gitignore
+++ b/tools/perf/.gitignore
@@ -31,5 +31,6 @@ config.mak.autogen
 .config-detected
 util/intel-pt-decoder/inat-tables.c
 arch/*/include/generated/
+trace/beauty/generated/
 pmu-events/pmu-events.c
 pmu-events/jevents
-- 
1.8.3.1

[PATCH 0/2] perf trace: Two trivial fixes

[Ravi Bangoria]

Two independent fixes:
First adds 'generated' directory into .gitignore
Second fixes call-graph output with perf trace

Ravi Bangoria (2):
  perf tools: Add trace/beauty/generated/ into .gitignore
  perf trace: Fix call-graph output

 tools/perf/.gitignore  | 1 +
 tools/perf/builtin-trace.c | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[PATCH 0/2] perf trace: Two trivial fixes

[Ravi Bangoria]

Two independent fixes:
First adds 'generated' directory into .gitignore
Second fixes call-graph output with perf trace

Ravi Bangoria (2):
  perf tools: Add trace/beauty/generated/ into .gitignore
  perf trace: Fix call-graph output

 tools/perf/.gitignore  | 1 +
 tools/perf/builtin-trace.c | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH 0/3] vfio: ccw: basic channel path event handling

[Dong Jia Shi]

Halil Pasic  writes:
--text follows this line--
Hi Halil,
--text follows this line--
AS you may noticed, Conny replied to this thread on my mail. Some of her
comments there could answer your questions. If that applies, I will just
say "See Conny's mail" in the following, and you can reply to that mail,
so we can consolidate further discussion.

>>> * When a channel path malfunctions a CRW is generated and a machine
>>> check channel report pending is generated. Same goes for
>>> channel paths popping up (on HW level). Should not these get
>>> propagated?
>> AFAIR, channel path related CRW is not that reliable. I mean it seems
>> that it's not mandatory to generate CRWs for channel path malfunctions.
>> AFAIU, it depneds on machine models. For example, we won't get
>> CRW_ERC_INIT for a "path has come" event on many models I believe. And
>> that's why nobody noticed the misuse of CRW_ERC_IPARM and CRW_ERC_INIT
>> in the CRW handling logic for channel path CRWs.
>> Ref:
>> 2daace78a8c9 s390: chp: handle CRW_ERC_INIT for channel-path status change
>> 
>
> Honestly, I forgot about this discussion. I've refreshed my memories by
> now, but I could not find why is it supposed to be architecturally OK
> to loose CRWs when for instance a chp goes away.
>
>> So my understanding for this is: it really a design decision for us to
>> propagate all of the channel path CRWs, or we just use other ways (like
>> using PNO and PNOM as this series shows).
>
> From what I read, CRWs did not seem optional.
> I wonder what should I read in order to change my mind. I'm not
> talking about the hardware in the wild -- but that could be buggy
> hardware.
>
Ah, can you point out the chapter that says CRWs is mandatory?

AFAIU, PoP doesn't say, for example, chp gone will lead to a CRW, but it
only says when we got a chp gone CRW it means a chp has been gone.

[See Conny's mail pls, and we can discuss there.]

>> 
>> Of course, CRW propagation is good to have. But as a discussion result
>> of a former thread, that is something we can consider only after we have
>> a good channel path re-modelling. That is the problem. We can try to
>> trigger CRW event in some work of machine checks handling enhancement
>> later.
>> 
>>> * Kind of instead of the CRW you introduce a per device interrupt
>>> for channel path events on the qemu/kvm boundary. AFAIU for a chp
>>> event you do an STSCH for each (affected?) subchannel
>> Until here, yes and right.
>> 
>>> and generate an irq. Right? Why is this a good idea.
>> This is not right. This series does not generate an irq for the guest.
>
> You misunderstood this. The word 'irq' this sentence is a short version
> of 'per device interrupt for channel path events on the qemu/kvm boundary'
> form the previous sentence. It's not an irq injected into the guest but
> a notification (which you call irq in '[RFC PATCH 2/3] vfio: ccw:
> introduce channel path irq') for QEMU.
>
I see now. I misunderstood.

>> In QEMU, when it gets the notification of a channel path status change
>> event, it read the newest SCHIB from the schib region, and update the
>> SCHIB (patch related parts) for the virtual subchannel. The guest will
>> get the new SCHIB whenever it calls STSCH then.
>
> We are in agreement. I just wanted to say, if let's say 42 vfio-ccw devices
> are using the same chp and it goes away, you will generate 42 notifications.
See reply below #LabelA.

>
>> 
>> I think this is a good idea, because:
>> 1. This is complies the Linux implementation. Path status change could
>>be noticed by Linux.
>> 2. This provides enhancement with a small work. On the contrary, channel
>>path re-modelling needs a lot of work.
>
> I thing your answer is based on the misunderstanding explained above.
I see now.

>
> My idea was to be lazy in a different way. Instead of being lazy about
> reading the subsection, I was wondering why not do an STSCH in the host
> as a response to one being done in the guest.
>
> That means: if there is no activity on the passed trough devices, nothing
> needs to be done. (Except maybe the CRWs).
>
#LabelA:

I see your point. This is an interesting idea.

Pros:
- code simplification
- no chp event notification and handling

Cons:
- have to read schib region (or issue STSCH on host) for each STSCH
  request from a guest

I think code siplification is very acctractive, and performance is not
the most important issue. Any comments? @Conny

> The things aren't observable by the guest if it does not do STSCH anyway.
Nod. This is the idea that this series is based on.

>
>
>> 
>>> * SCHIB.PMCW provides path information relevant for a given device.
>>> This information is retrieved using store subchannel. Is your series
>>> sufficient for making store subchannel work properly (correct and
>>> reasonably up to date)?
>> Introducing a SCHIB region is the basis of further STSCH hanlding work.
>> This series depends on it, and only focuses on the update of the channel
>> 

Re: [Qemu-devel] [RFC PATCH 0/3] vfio: ccw: basic channel path event handling

[Dong Jia Shi]

Halil Pasic  writes:
--text follows this line--
Hi Halil,
--text follows this line--
AS you may noticed, Conny replied to this thread on my mail. Some of her
comments there could answer your questions. If that applies, I will just
say "See Conny's mail" in the following, and you can reply to that mail,
so we can consolidate further discussion.

>>> * When a channel path malfunctions a CRW is generated and a machine
>>> check channel report pending is generated. Same goes for
>>> channel paths popping up (on HW level). Should not these get
>>> propagated?
>> AFAIR, channel path related CRW is not that reliable. I mean it seems
>> that it's not mandatory to generate CRWs for channel path malfunctions.
>> AFAIU, it depneds on machine models. For example, we won't get
>> CRW_ERC_INIT for a "path has come" event on many models I believe. And
>> that's why nobody noticed the misuse of CRW_ERC_IPARM and CRW_ERC_INIT
>> in the CRW handling logic for channel path CRWs.
>> Ref:
>> 2daace78a8c9 s390: chp: handle CRW_ERC_INIT for channel-path status change
>> 
>
> Honestly, I forgot about this discussion. I've refreshed my memories by
> now, but I could not find why is it supposed to be architecturally OK
> to loose CRWs when for instance a chp goes away.
>
>> So my understanding for this is: it really a design decision for us to
>> propagate all of the channel path CRWs, or we just use other ways (like
>> using PNO and PNOM as this series shows).
>
> From what I read, CRWs did not seem optional.
> I wonder what should I read in order to change my mind. I'm not
> talking about the hardware in the wild -- but that could be buggy
> hardware.
>
Ah, can you point out the chapter that says CRWs is mandatory?

AFAIU, PoP doesn't say, for example, chp gone will lead to a CRW, but it
only says when we got a chp gone CRW it means a chp has been gone.

[See Conny's mail pls, and we can discuss there.]

>> 
>> Of course, CRW propagation is good to have. But as a discussion result
>> of a former thread, that is something we can consider only after we have
>> a good channel path re-modelling. That is the problem. We can try to
>> trigger CRW event in some work of machine checks handling enhancement
>> later.
>> 
>>> * Kind of instead of the CRW you introduce a per device interrupt
>>> for channel path events on the qemu/kvm boundary. AFAIU for a chp
>>> event you do an STSCH for each (affected?) subchannel
>> Until here, yes and right.
>> 
>>> and generate an irq. Right? Why is this a good idea.
>> This is not right. This series does not generate an irq for the guest.
>
> You misunderstood this. The word 'irq' this sentence is a short version
> of 'per device interrupt for channel path events on the qemu/kvm boundary'
> form the previous sentence. It's not an irq injected into the guest but
> a notification (which you call irq in '[RFC PATCH 2/3] vfio: ccw:
> introduce channel path irq') for QEMU.
>
I see now. I misunderstood.

>> In QEMU, when it gets the notification of a channel path status change
>> event, it read the newest SCHIB from the schib region, and update the
>> SCHIB (patch related parts) for the virtual subchannel. The guest will
>> get the new SCHIB whenever it calls STSCH then.
>
> We are in agreement. I just wanted to say, if let's say 42 vfio-ccw devices
> are using the same chp and it goes away, you will generate 42 notifications.
See reply below #LabelA.

>
>> 
>> I think this is a good idea, because:
>> 1. This is complies the Linux implementation. Path status change could
>>be noticed by Linux.
>> 2. This provides enhancement with a small work. On the contrary, channel
>>path re-modelling needs a lot of work.
>
> I thing your answer is based on the misunderstanding explained above.
I see now.

>
> My idea was to be lazy in a different way. Instead of being lazy about
> reading the subsection, I was wondering why not do an STSCH in the host
> as a response to one being done in the guest.
>
> That means: if there is no activity on the passed trough devices, nothing
> needs to be done. (Except maybe the CRWs).
>
#LabelA:

I see your point. This is an interesting idea.

Pros:
- code simplification
- no chp event notification and handling

Cons:
- have to read schib region (or issue STSCH on host) for each STSCH
  request from a guest

I think code siplification is very acctractive, and performance is not
the most important issue. Any comments? @Conny

> The things aren't observable by the guest if it does not do STSCH anyway.
Nod. This is the idea that this series is based on.

>
>
>> 
>>> * SCHIB.PMCW provides path information relevant for a given device.
>>> This information is retrieved using store subchannel. Is your series
>>> sufficient for making store subchannel work properly (correct and
>>> reasonably up to date)?
>> Introducing a SCHIB region is the basis of further STSCH hanlding work.
>> This series depends on it, and only focuses on the update of the channel
>> path related parts of it. 

linux-next: Tree for Jan 30

[Stephen Rothwell]

Hi all,

Please do not add any v4.17 material to your linux-next included branches
until after v4.16-rc1 has been released.

Changes since 20180129:

The userns tree still had its build failure for which I added a patch.

Non-merge commits (relative to Linus' tree): 10187
 10050 files changed, 426072 insertions(+), 280932 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a
multi_v7_defconfig for arm and a native build of tools/perf. After
the final fixups (if any), I do an x86_64 modules_install followed by
builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit),
ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc
and sparc64 defconfig. And finally, a simple boot test of the powerpc
pseries_le_defconfig kernel in qemu (with and without kvm enabled).

Below is a summary of the state of the merge.

I am currently merging 256 trees (counting Linus' and 44 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (0a4b6e2f80aa Merge branch 'for-4.16/block' of 
git://git.kernel.dk/linux-block)
Merging fixes/master (820bf5c419e4 Merge tag 'scsi-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi)
Merging kbuild-current/fixes (36c1681678b5 genksyms: drop *.hash.c from 
.gitignore)
Merging arc-current/for-curr (a46f24acf8bc ARC: boot log: Fix trailing 
semicolon)
Merging arm-current/fixes (091f02483df7 ARM: net: bpf: clarify tail_call index)
Merging m68k-current/for-linus (2334b1ac1235 MAINTAINERS: Add NuBus subsystem 
entry)
Merging metag-fixes/fixes (b884a190afce metag/usercopy: Add missing fixups)
Merging powerpc-fixes/fixes (1b689a95ce74 powerpc/pseries: include 
linux/types.h in asm/hvcall.h)
Merging sparc/master (aebb48f5e465 sparc64: fix typo in 
CONFIG_CRYPTO_DES_SPARC64 => CONFIG_CRYPTO_CAMELLIA_SPARC64)
Merging fscrypt-current/for-stable (ae64f9bd1d36 Linux 4.15-rc2)
Merging net/master (ba804bb4b72e Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net)
Merging bpf/master (ba804bb4b72e Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net)
Merging ipsec/master (545d8ae7afff xfrm: fix boolean assignment in 
xfrm_get_type_offload)
Merging netfilter/master (da17c73b6eb7 netfilter: x_tables: avoid out-of-bounds 
reads in xt_request_find_{match|target})
Merging ipvs/master (f7fb77fc1235 netfilter: nft_compat: check extension hook 
mask only if set)
Merging wireless-drivers/master (a9e6d44ddecc ssb: Do not disable PCI host on 
non-Mips)
Merging mac80211/master (e58edaa48635 net/mlx5e: Fix fixpoint divide exception 
in mlx5e_am_stats_compare)
Merging rdma-fixes/for-rc (ae59c3f0b6cf RDMA/mlx5: Fix out-of-bound access 
while querying AH)
Merging sound-current/for-linus (1c9609e3a8cf ALSA: hda - Reduce the suspend 
time consumption for ALC256)
Merging pci-current/for-linus (838cda369707 x86/PCI: Enable AMD 64-bit window 
on resume)
Merging driver-core.current/driver-core-linus (30a7acd57389 Linux 4.15-rc6)
Merging tty.current/tty-linus (30a7acd57389 Linux 4.15-rc6)
Merging usb.current/usb-linus (a8750ddca918 Linux 4.15-rc8)
Merging usb-gadget-fixes/fixes (b2cd1df66037 Linux 4.15-rc7)
Merging usb-serial-fixes/usb-linus (d14ac576d10f USB: serial: cp210x: add new 
device ID ELV ALC 8xxx)
Merging usb-chipidea-fixes/ci-for-usb-stable (964728f9f407 USB: chipidea: msm: 
fix ulpi-node lookup)
Merging phy/fixes (2b88212c4cc6 phy: rcar-gen3-usb2: select USB_COMMON)
Merging staging.current/staging-linus (a8750ddca918 Linux 4.15-rc8)
Merging char-misc.current/char-misc-linus (a8750ddca918 Linux 4.15-rc8)
Merging input-current/for-linus (060403f34008 Revert "Input: synaptics_rmi4 - 
use devm_device_add_group() for attributes in F01")
Merging crypto-current/master (2d55807b7f7b crypto: picoxcell - Fix error 
handling in 

linux-next: Tree for Jan 30

[Stephen Rothwell]

Hi all,

Please do not add any v4.17 material to your linux-next included branches
until after v4.16-rc1 has been released.

Changes since 20180129:

The userns tree still had its build failure for which I added a patch.

Non-merge commits (relative to Linus' tree): 10187
 10050 files changed, 426072 insertions(+), 280932 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a
multi_v7_defconfig for arm and a native build of tools/perf. After
the final fixups (if any), I do an x86_64 modules_install followed by
builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit),
ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc
and sparc64 defconfig. And finally, a simple boot test of the powerpc
pseries_le_defconfig kernel in qemu (with and without kvm enabled).

Below is a summary of the state of the merge.

I am currently merging 256 trees (counting Linus' and 44 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (0a4b6e2f80aa Merge branch 'for-4.16/block' of 
git://git.kernel.dk/linux-block)
Merging fixes/master (820bf5c419e4 Merge tag 'scsi-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi)
Merging kbuild-current/fixes (36c1681678b5 genksyms: drop *.hash.c from 
.gitignore)
Merging arc-current/for-curr (a46f24acf8bc ARC: boot log: Fix trailing 
semicolon)
Merging arm-current/fixes (091f02483df7 ARM: net: bpf: clarify tail_call index)
Merging m68k-current/for-linus (2334b1ac1235 MAINTAINERS: Add NuBus subsystem 
entry)
Merging metag-fixes/fixes (b884a190afce metag/usercopy: Add missing fixups)
Merging powerpc-fixes/fixes (1b689a95ce74 powerpc/pseries: include 
linux/types.h in asm/hvcall.h)
Merging sparc/master (aebb48f5e465 sparc64: fix typo in 
CONFIG_CRYPTO_DES_SPARC64 => CONFIG_CRYPTO_CAMELLIA_SPARC64)
Merging fscrypt-current/for-stable (ae64f9bd1d36 Linux 4.15-rc2)
Merging net/master (ba804bb4b72e Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net)
Merging bpf/master (ba804bb4b72e Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net)
Merging ipsec/master (545d8ae7afff xfrm: fix boolean assignment in 
xfrm_get_type_offload)
Merging netfilter/master (da17c73b6eb7 netfilter: x_tables: avoid out-of-bounds 
reads in xt_request_find_{match|target})
Merging ipvs/master (f7fb77fc1235 netfilter: nft_compat: check extension hook 
mask only if set)
Merging wireless-drivers/master (a9e6d44ddecc ssb: Do not disable PCI host on 
non-Mips)
Merging mac80211/master (e58edaa48635 net/mlx5e: Fix fixpoint divide exception 
in mlx5e_am_stats_compare)
Merging rdma-fixes/for-rc (ae59c3f0b6cf RDMA/mlx5: Fix out-of-bound access 
while querying AH)
Merging sound-current/for-linus (1c9609e3a8cf ALSA: hda - Reduce the suspend 
time consumption for ALC256)
Merging pci-current/for-linus (838cda369707 x86/PCI: Enable AMD 64-bit window 
on resume)
Merging driver-core.current/driver-core-linus (30a7acd57389 Linux 4.15-rc6)
Merging tty.current/tty-linus (30a7acd57389 Linux 4.15-rc6)
Merging usb.current/usb-linus (a8750ddca918 Linux 4.15-rc8)
Merging usb-gadget-fixes/fixes (b2cd1df66037 Linux 4.15-rc7)
Merging usb-serial-fixes/usb-linus (d14ac576d10f USB: serial: cp210x: add new 
device ID ELV ALC 8xxx)
Merging usb-chipidea-fixes/ci-for-usb-stable (964728f9f407 USB: chipidea: msm: 
fix ulpi-node lookup)
Merging phy/fixes (2b88212c4cc6 phy: rcar-gen3-usb2: select USB_COMMON)
Merging staging.current/staging-linus (a8750ddca918 Linux 4.15-rc8)
Merging char-misc.current/char-misc-linus (a8750ddca918 Linux 4.15-rc8)
Merging input-current/for-linus (060403f34008 Revert "Input: synaptics_rmi4 - 
use devm_device_add_group() for attributes in F01")
Merging crypto-current/master (2d55807b7f7b crypto: picoxcell - Fix error 
handling in 

Re: [RFC] mm/migrate: Add new migration reason MR_HUGETLB

[Anshuman Khandual]

On 01/30/2018 08:37 AM, Anshuman Khandual wrote:
> @@ -7621,8 +7622,13 @@ static int __alloc_contig_migrate_range(struct 
> compact_control *cc,
>   >migratepages);
>   cc->nr_migratepages -= nr_reclaimed;
>  
> + if (migratetype == MIGRATE_CMA)
> + migrate_reason = MR_CMA;
> + else
> + migrate_reason = MR_HUGETLB;
> +
>   ret = migrate_pages(>migratepages, new_page_alloc_contig,

Oops, this is on top of the changes from the other RFC regarding migration
helper functions.

Re: [RFC] mm/migrate: Add new migration reason MR_HUGETLB

[Anshuman Khandual]

On 01/30/2018 08:37 AM, Anshuman Khandual wrote:
> @@ -7621,8 +7622,13 @@ static int __alloc_contig_migrate_range(struct 
> compact_control *cc,
>   >migratepages);
>   cc->nr_migratepages -= nr_reclaimed;
>  
> + if (migratetype == MIGRATE_CMA)
> + migrate_reason = MR_CMA;
> + else
> + migrate_reason = MR_HUGETLB;
> +
>   ret = migrate_pages(>migratepages, new_page_alloc_contig,

Oops, this is on top of the changes from the other RFC regarding migration
helper functions.

RE: [PATCH] mm/swap: add function get_total_swap_pages to expose total_swap_pages

[He, Roger]

get_nr_swap_pages is the only API we can accessed from other module now.
It can't cover the case of the dynamic swap size increment.
I mean: user can use "swapon" to enable new swap file or swap disk 
dynamically or "swapoff" to disable swap space.

Above is why we always to get swap cache size rather than getting it once at 
module initialization time.
That is internal in TTM. Please ignore that.

And why TTM needs get_total_swap_pages instead of using get_nr_swap_pages 
directly. That because
even though the TTM buffer has been swapped out, at the start they also stay in 
system memory by shmem. Later at some point when
Under high memory pressure, Those buffers all are flushed into swap disk and 
used more swap disk size or even use up all swap size. That is not what we want 
and still has random OOM. So we need a API to get total swap size and control 
the swap size used by TTM very accurately.

Thanks
Roger(Hongbo.He)
-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
He, Roger
Sent: Tuesday, January 30, 2018 10:57 AM
To: Michal Hocko 
Cc: linux...@kvack.org; linux-kernel@vger.kernel.org; 
dri-de...@lists.freedesktop.org; Koenig, Christian 
Subject: RE: [PATCH] mm/swap: add function get_total_swap_pages to expose 
total_swap_pages

Hi Michal:

We need a API to tell TTM module the system totally has how many swap cache.
Then TTM module can use it to restrict how many the swap cache it can use to 
prevent triggering OOM.
For Now we set the threshold of swap size TTM used as 1/2 * total size and 
leave the rest for others use.


get_nr_swap_pages is the only API we can accessed from other module now.
It can't cover the case of the dynamic swap size increment.
I mean: user can use "swapon" to enable new swap file or swap disk 
dynamically or "swapoff" to disable swap space.

Thanks
Roger(Hongbo.He)

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Michal Hocko
Sent: Tuesday, January 30, 2018 12:31 AM
To: He, Roger 
Cc: linux...@kvack.org; linux-kernel@vger.kernel.org; 
dri-de...@lists.freedesktop.org; Koenig, Christian 
Subject: Re: [PATCH] mm/swap: add function get_total_swap_pages to expose 
total_swap_pages

On Mon 29-01-18 16:29:42, Roger He wrote:
> ttm module needs it to determine its internal parameter setting.

Could you be more specific why?

> Signed-off-by: Roger He 
> ---
>  include/linux/swap.h |  6 ++
>  mm/swapfile.c| 15 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h index 
> c2b8128..708d66f 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -484,6 +484,7 @@ extern int try_to_free_swap(struct page *); struct 
> backing_dev_info;  extern int init_swap_address_space(unsigned int 
> type, unsigned long nr_pages);  extern void 
> exit_swap_address_space(unsigned int type);
> +extern long get_total_swap_pages(void);
>  
>  #else /* CONFIG_SWAP */
>  
> @@ -516,6 +517,11 @@ static inline void show_swap_cache_info(void)  { 
> }
>  
> +long get_total_swap_pages(void)
> +{
> + return 0;
> +}
> +
>  #define free_swap_and_cache(e) ({(is_migration_entry(e) ||
> is_device_private_entry(e));})  #define swapcache_prepare(e)
> ({(is_migration_entry(e) || is_device_private_entry(e));})
>  
> diff --git a/mm/swapfile.c b/mm/swapfile.c index 3074b02..a0062eb
> 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -98,6 +98,21 @@ static atomic_t proc_poll_event = ATOMIC_INIT(0);
>  
>  atomic_t nr_rotate_swap = ATOMIC_INIT(0);
>  
> +/*
> + * expose this value for others use
> + */
> +long get_total_swap_pages(void)
> +{
> + long ret;
> +
> + spin_lock(_lock);
> + ret = total_swap_pages;
> + spin_unlock(_lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(get_total_swap_pages);
> +
>  static inline unsigned char swap_count(unsigned char ent)  {
>   return ent & ~SWAP_HAS_CACHE;   /* may include SWAP_HAS_CONT flag */
> --
> 2.7.4

--
Michal Hocko
SUSE Labs
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH] mm/swap: add function get_total_swap_pages to expose total_swap_pages

[He, Roger]

get_nr_swap_pages is the only API we can accessed from other module now.
It can't cover the case of the dynamic swap size increment.
I mean: user can use "swapon" to enable new swap file or swap disk 
dynamically or "swapoff" to disable swap space.

Above is why we always to get swap cache size rather than getting it once at 
module initialization time.
That is internal in TTM. Please ignore that.

And why TTM needs get_total_swap_pages instead of using get_nr_swap_pages 
directly. That because
even though the TTM buffer has been swapped out, at the start they also stay in 
system memory by shmem. Later at some point when
Under high memory pressure, Those buffers all are flushed into swap disk and 
used more swap disk size or even use up all swap size. That is not what we want 
and still has random OOM. So we need a API to get total swap size and control 
the swap size used by TTM very accurately.

Thanks
Roger(Hongbo.He)
-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
He, Roger
Sent: Tuesday, January 30, 2018 10:57 AM
To: Michal Hocko 
Cc: linux...@kvack.org; linux-kernel@vger.kernel.org; 
dri-de...@lists.freedesktop.org; Koenig, Christian 
Subject: RE: [PATCH] mm/swap: add function get_total_swap_pages to expose 
total_swap_pages

Hi Michal:

We need a API to tell TTM module the system totally has how many swap cache.
Then TTM module can use it to restrict how many the swap cache it can use to 
prevent triggering OOM.
For Now we set the threshold of swap size TTM used as 1/2 * total size and 
leave the rest for others use.


get_nr_swap_pages is the only API we can accessed from other module now.
It can't cover the case of the dynamic swap size increment.
I mean: user can use "swapon" to enable new swap file or swap disk 
dynamically or "swapoff" to disable swap space.

Thanks
Roger(Hongbo.He)

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Michal Hocko
Sent: Tuesday, January 30, 2018 12:31 AM
To: He, Roger 
Cc: linux...@kvack.org; linux-kernel@vger.kernel.org; 
dri-de...@lists.freedesktop.org; Koenig, Christian 
Subject: Re: [PATCH] mm/swap: add function get_total_swap_pages to expose 
total_swap_pages

On Mon 29-01-18 16:29:42, Roger He wrote:
> ttm module needs it to determine its internal parameter setting.

Could you be more specific why?

> Signed-off-by: Roger He 
> ---
>  include/linux/swap.h |  6 ++
>  mm/swapfile.c| 15 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h index 
> c2b8128..708d66f 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -484,6 +484,7 @@ extern int try_to_free_swap(struct page *); struct 
> backing_dev_info;  extern int init_swap_address_space(unsigned int 
> type, unsigned long nr_pages);  extern void 
> exit_swap_address_space(unsigned int type);
> +extern long get_total_swap_pages(void);
>  
>  #else /* CONFIG_SWAP */
>  
> @@ -516,6 +517,11 @@ static inline void show_swap_cache_info(void)  { 
> }
>  
> +long get_total_swap_pages(void)
> +{
> + return 0;
> +}
> +
>  #define free_swap_and_cache(e) ({(is_migration_entry(e) ||
> is_device_private_entry(e));})  #define swapcache_prepare(e)
> ({(is_migration_entry(e) || is_device_private_entry(e));})
>  
> diff --git a/mm/swapfile.c b/mm/swapfile.c index 3074b02..a0062eb
> 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -98,6 +98,21 @@ static atomic_t proc_poll_event = ATOMIC_INIT(0);
>  
>  atomic_t nr_rotate_swap = ATOMIC_INIT(0);
>  
> +/*
> + * expose this value for others use
> + */
> +long get_total_swap_pages(void)
> +{
> + long ret;
> +
> + spin_lock(_lock);
> + ret = total_swap_pages;
> + spin_unlock(_lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(get_total_swap_pages);
> +
>  static inline unsigned char swap_count(unsigned char ent)  {
>   return ent & ~SWAP_HAS_CACHE;   /* may include SWAP_HAS_CONT flag */
> --
> 2.7.4

--
Michal Hocko
SUSE Labs
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] leaking_addresses: add 32-bit support

[Tobin C. Harding]

On Tue, Jan 30, 2018 at 09:14:49AM +0530, kaiwan.billimo...@gmail.com wrote:
> Hi Tobin,
> 
> On Mon, 2018-01-29 at 15:51 +1100, Tobin C. Harding wrote:
> > Currently script only supports x86_64 and ppc64.  It would be nice to be
> > able to scan 32-bit machines also.  We can add support for
> > 32-bit architectures by modifying how we check for false positives,
> > taking advantage of the page offset used by the kernel, and using the
> > correct regular expression.
> > 
> > Support for 32-bit machines is enabled by the observation the kernel
> > addresses on 32-bit machines are larger than the page offset.  We can
> > use this to filter false positives when scanning the kernel for leaking
> > addresses.
> > 
> > Programmatic determination of the running architecture is not
> > immediately obvious.  We therefore provide a flag to enable scanning of
> > 32-bit kernels.  Also we can check the kernel config file for the offset
> > and if not found default to 0xc000.  A command line option to parse
> > in the page offset is also provided.  We do automatically detect
> > architecture if running on ix86.
> > 
> > Add support for 32-bit kernels.  Add a command line option for page
> > offset.
> > 
> > Suggested-by: Kaiwan N Billimoria 
> > Signed-off-by: Tobin C. Harding 
> > ---
> > 
> > The basis for this patch has been in development for a while by Kaiwan
> > but didn't get finished before the merge window opened.  I'd like to
> > fast track this and get it to Linus this merge window (considering
> > Spectre/Meltdown).  I have finished this work off and added the
> > Suggested-by tag.  Kaiwan I hope you are not upset by this, extra
> > ordinary circumstances seemed to require this action.
> Definitely not; I understand and am glad you're on it a 100%. Apologies
> that I couldn't work on this right now.. will try and keep track too.

Thanks for the response.

Tobin

Re: [PATCH] leaking_addresses: add 32-bit support

[Tobin C. Harding]

On Tue, Jan 30, 2018 at 09:14:49AM +0530, kaiwan.billimo...@gmail.com wrote:
> Hi Tobin,
> 
> On Mon, 2018-01-29 at 15:51 +1100, Tobin C. Harding wrote:
> > Currently script only supports x86_64 and ppc64.  It would be nice to be
> > able to scan 32-bit machines also.  We can add support for
> > 32-bit architectures by modifying how we check for false positives,
> > taking advantage of the page offset used by the kernel, and using the
> > correct regular expression.
> > 
> > Support for 32-bit machines is enabled by the observation the kernel
> > addresses on 32-bit machines are larger than the page offset.  We can
> > use this to filter false positives when scanning the kernel for leaking
> > addresses.
> > 
> > Programmatic determination of the running architecture is not
> > immediately obvious.  We therefore provide a flag to enable scanning of
> > 32-bit kernels.  Also we can check the kernel config file for the offset
> > and if not found default to 0xc000.  A command line option to parse
> > in the page offset is also provided.  We do automatically detect
> > architecture if running on ix86.
> > 
> > Add support for 32-bit kernels.  Add a command line option for page
> > offset.
> > 
> > Suggested-by: Kaiwan N Billimoria 
> > Signed-off-by: Tobin C. Harding 
> > ---
> > 
> > The basis for this patch has been in development for a while by Kaiwan
> > but didn't get finished before the merge window opened.  I'd like to
> > fast track this and get it to Linus this merge window (considering
> > Spectre/Meltdown).  I have finished this work off and added the
> > Suggested-by tag.  Kaiwan I hope you are not upset by this, extra
> > ordinary circumstances seemed to require this action.
> Definitely not; I understand and am glad you're on it a 100%. Apologies
> that I couldn't work on this right now.. will try and keep track too.

Thanks for the response.

Tobin

[RFC] mm/migrate: Consolidate page allocation helper functions

[Anshuman Khandual]

Allocation helper functions for migrate_pages() remmain scattered with
similar names making them really confusing. Rename these functions based
on the context for the migration and move them all into common migration
header. Functionality remains unchanged.

Signed-off-by: Anshuman Khandual 
---
- Based on earlier discussion (https://lkml.org/lkml/2018/1/3/128)
- Wondering if we can still further factorize these helpers functions

 include/linux/migrate.h| 112 -
 include/linux/page-isolation.h |   2 -
 mm/internal.h  |   1 -
 mm/memory-failure.c|  11 +---
 mm/memory_hotplug.c|  19 +--
 mm/mempolicy.c |  69 +
 mm/migrate.c   |  19 +--
 mm/page_alloc.c|   2 +-
 mm/page_isolation.c|   4 --
 9 files changed, 119 insertions(+), 120 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 0c6fe904bc97..a732598fcf83 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -3,6 +3,7 @@
 #define _LINUX_MIGRATE_H
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -31,6 +32,84 @@ enum migrate_reason {
 /* In mm/debug.c; also keep sync with include/trace/events/migrate.h */
 extern char *migrate_reason_names[MR_TYPES];
 
+#ifdef CONFIG_MIGRATION
+/*
+ * Allocate a new page for page migration based on vma policy.
+ * Start by assuming the page is mapped by the same vma as contains @start.
+ * Search forward from there, if not.  N.B., this assumes that the
+ * list of pages handed to migrate_pages()--which is how we get here--
+ * is in virtual address order.
+ */
+static inline struct page *new_page_alloc_mbind(struct page *page, unsigned 
long start)
+{
+   struct vm_area_struct *vma;
+   unsigned long uninitialized_var(address);
+
+   vma = find_vma(current->mm, start);
+   while (vma) {
+   address = page_address_in_vma(page, vma);
+   if (address != -EFAULT)
+   break;
+   vma = vma->vm_next;
+   }
+
+   if (PageHuge(page)) {
+   return alloc_huge_page_vma(page_hstate(compound_head(page)),
+   vma, address);
+   } else if (PageTransHuge(page)) {
+   struct page *thp;
+
+   thp = alloc_hugepage_vma(GFP_TRANSHUGE, vma, address,
+HPAGE_PMD_ORDER);
+   if (!thp)
+   return NULL;
+   prep_transhuge_page(thp);
+   return thp;
+   }
+   /*
+* if !vma, alloc_page_vma() will use task or system default policy
+*/
+   return alloc_page_vma(GFP_HIGHUSER_MOVABLE | __GFP_RETRY_MAYFAIL,
+   vma, address);
+}
+
+/* page allocation callback for NUMA node migration */
+static inline struct page *new_page_alloc_syscall(struct page *page, unsigned 
long node)
+{
+   if (PageHuge(page))
+   return alloc_huge_page_node(page_hstate(compound_head(page)),
+   node);
+   else if (PageTransHuge(page)) {
+   struct page *thp;
+
+   thp = alloc_pages_node(node,
+   (GFP_TRANSHUGE | __GFP_THISNODE),
+   HPAGE_PMD_ORDER);
+   if (!thp)
+   return NULL;
+   prep_transhuge_page(thp);
+   return thp;
+   } else
+   return __alloc_pages_node(node, GFP_HIGHUSER_MOVABLE |
+   __GFP_THISNODE, 0);
+}
+
+
+static inline struct page *new_page_alloc_misplaced(struct page *page,
+  unsigned long data)
+{
+   int nid = (int) data;
+   struct page *newpage;
+
+   newpage = __alloc_pages_node(nid,
+(GFP_HIGHUSER_MOVABLE |
+ __GFP_THISNODE | __GFP_NOMEMALLOC |
+ __GFP_NORETRY | __GFP_NOWARN) &
+~__GFP_RECLAIM, 0);
+
+   return newpage;
+}
+
 static inline struct page *new_page_nodemask(struct page *page,
int preferred_nid, nodemask_t *nodemask)
 {
@@ -59,7 +138,34 @@ static inline struct page *new_page_nodemask(struct page 
*page,
return new_page;
 }
 
-#ifdef CONFIG_MIGRATION
+static inline struct page *new_page_alloc_failure(struct page *p, unsigned 
long private)
+{
+   int nid = page_to_nid(p);
+
+   return new_page_nodemask(p, nid, _states[N_MEMORY]);
+}
+
+/*
+ * Try to allocate from a different node but reuse this node if there
+ * are no other online nodes to be used (e.g. we are offlining a part
+ * of the only existing node).
+ */
+static inline struct page *new_page_alloc_hotplug(struct page *page, unsigned 
long 

[RFC] mm/migrate: Consolidate page allocation helper functions

[Anshuman Khandual]

Allocation helper functions for migrate_pages() remmain scattered with
similar names making them really confusing. Rename these functions based
on the context for the migration and move them all into common migration
header. Functionality remains unchanged.

Signed-off-by: Anshuman Khandual 
---
- Based on earlier discussion (https://lkml.org/lkml/2018/1/3/128)
- Wondering if we can still further factorize these helpers functions

 include/linux/migrate.h| 112 -
 include/linux/page-isolation.h |   2 -
 mm/internal.h  |   1 -
 mm/memory-failure.c|  11 +---
 mm/memory_hotplug.c|  19 +--
 mm/mempolicy.c |  69 +
 mm/migrate.c   |  19 +--
 mm/page_alloc.c|   2 +-
 mm/page_isolation.c|   4 --
 9 files changed, 119 insertions(+), 120 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 0c6fe904bc97..a732598fcf83 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -3,6 +3,7 @@
 #define _LINUX_MIGRATE_H
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -31,6 +32,84 @@ enum migrate_reason {
 /* In mm/debug.c; also keep sync with include/trace/events/migrate.h */
 extern char *migrate_reason_names[MR_TYPES];
 
+#ifdef CONFIG_MIGRATION
+/*
+ * Allocate a new page for page migration based on vma policy.
+ * Start by assuming the page is mapped by the same vma as contains @start.
+ * Search forward from there, if not.  N.B., this assumes that the
+ * list of pages handed to migrate_pages()--which is how we get here--
+ * is in virtual address order.
+ */
+static inline struct page *new_page_alloc_mbind(struct page *page, unsigned 
long start)
+{
+   struct vm_area_struct *vma;
+   unsigned long uninitialized_var(address);
+
+   vma = find_vma(current->mm, start);
+   while (vma) {
+   address = page_address_in_vma(page, vma);
+   if (address != -EFAULT)
+   break;
+   vma = vma->vm_next;
+   }
+
+   if (PageHuge(page)) {
+   return alloc_huge_page_vma(page_hstate(compound_head(page)),
+   vma, address);
+   } else if (PageTransHuge(page)) {
+   struct page *thp;
+
+   thp = alloc_hugepage_vma(GFP_TRANSHUGE, vma, address,
+HPAGE_PMD_ORDER);
+   if (!thp)
+   return NULL;
+   prep_transhuge_page(thp);
+   return thp;
+   }
+   /*
+* if !vma, alloc_page_vma() will use task or system default policy
+*/
+   return alloc_page_vma(GFP_HIGHUSER_MOVABLE | __GFP_RETRY_MAYFAIL,
+   vma, address);
+}
+
+/* page allocation callback for NUMA node migration */
+static inline struct page *new_page_alloc_syscall(struct page *page, unsigned 
long node)
+{
+   if (PageHuge(page))
+   return alloc_huge_page_node(page_hstate(compound_head(page)),
+   node);
+   else if (PageTransHuge(page)) {
+   struct page *thp;
+
+   thp = alloc_pages_node(node,
+   (GFP_TRANSHUGE | __GFP_THISNODE),
+   HPAGE_PMD_ORDER);
+   if (!thp)
+   return NULL;
+   prep_transhuge_page(thp);
+   return thp;
+   } else
+   return __alloc_pages_node(node, GFP_HIGHUSER_MOVABLE |
+   __GFP_THISNODE, 0);
+}
+
+
+static inline struct page *new_page_alloc_misplaced(struct page *page,
+  unsigned long data)
+{
+   int nid = (int) data;
+   struct page *newpage;
+
+   newpage = __alloc_pages_node(nid,
+(GFP_HIGHUSER_MOVABLE |
+ __GFP_THISNODE | __GFP_NOMEMALLOC |
+ __GFP_NORETRY | __GFP_NOWARN) &
+~__GFP_RECLAIM, 0);
+
+   return newpage;
+}
+
 static inline struct page *new_page_nodemask(struct page *page,
int preferred_nid, nodemask_t *nodemask)
 {
@@ -59,7 +138,34 @@ static inline struct page *new_page_nodemask(struct page 
*page,
return new_page;
 }
 
-#ifdef CONFIG_MIGRATION
+static inline struct page *new_page_alloc_failure(struct page *p, unsigned 
long private)
+{
+   int nid = page_to_nid(p);
+
+   return new_page_nodemask(p, nid, _states[N_MEMORY]);
+}
+
+/*
+ * Try to allocate from a different node but reuse this node if there
+ * are no other online nodes to be used (e.g. we are offlining a part
+ * of the only existing node).
+ */
+static inline struct page *new_page_alloc_hotplug(struct page *page, unsigned 
long private)
+{
+   int nid = 

Re: [PATCH v3 2/4] drivers: firmware: xilinx: Add ZynqMP firmware driver

[Shubhrajyoti Datta]

Hi,

Thanks for the patch.
A few questions below.


On Thu, Jan 25, 2018 at 4:51 AM, Jolly Shah  wrote:
> This patch is adding communication layer with firmware.
> Firmware driver provides an interface to firmware APIs.
> Interface APIs can be used by any driver to communicate to
> PMUFW(Platform Management Unit). All requests go through ATF.
>
> Signed-off-by: Jolly Shah 
> Signed-off-by: Rajan Vaja 
> ---

>

> +/**
> + * zynqmp_pm_clock_enable - Enable the clock for given id
> + * @clock_id:  ID of the clock to be enabled

Does it enable all the parents also if they are disabled?

> + *
> + * This function is used by master to enable the clock
> + * including peripherals and PLL clocks.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_enable(u32 clock_id)
> +{
> +   return invoke_pm_fn(PM_CLOCK_ENABLE, clock_id, 0, 0, 0, NULL);
> +}
> +
> +/**
> + * zynqmp_pm_clock_disable - Disable the clock for given id
> + * @clock_id:  ID of the clock to be disable
> + *
> + * This function is used by master to disable the clock
> + * including peripherals and PLL clocks.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_disable(u32 clock_id)
> +{
> +   return invoke_pm_fn(PM_CLOCK_DISABLE, clock_id, 0, 0, 0, NULL);
> +}
> +
> +/**
> + * zynqmp_pm_clock_getstate - Get the clock state for given id
> + * @clock_id:  ID of the clock to be queried
> + * @state: 1/0 (Enabled/Disabled)
> + *
> + * This function is used by master to get the state of clock
> + * including peripherals and PLL clocks.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_getstate(u32 clock_id, u32 *state)
> +{
> +   u32 ret_payload[PAYLOAD_ARG_CNT];
> +   int ret;
> +
> +   ret = invoke_pm_fn(PM_CLOCK_GETSTATE, clock_id, 0, 0, 0, ret_payload);
> +   *state = ret_payload[1];
> +
> +   return ret;
> +}
> +
> +/**
> + * zynqmp_pm_clock_setdivider - Set the clock divider for given id
> + * @clock_id:  ID of the clock
> + * @div_type:  TYPE_DIV1: div1
> + * TYPE_DIV2: div2
div type didnt see in the signature.



> + * @divider:   divider value.
> + *
> + * This function is used by master to set divider for any clock
> + * to achieve desired rate.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_setdivider(u32 clock_id, u32 divider)
> +{
> +   return invoke_pm_fn(PM_CLOCK_SETDIVIDER, clock_id, divider, 0, 0, 
> NULL);
> +}
> +
> +/**
> + * zynqmp_pm_clock_getdivider - Get the clock divider for given id
> + * @clock_id:  ID of the clock
> + * @div_type:  TYPE_DIV1: div1
> + * TYPE_DIV2: div2
didnt see this  below.

> + * @divider:   divider value.
> + *
> + * This function is used by master to get divider values
> + * for any clock.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_getdivider(u32 clock_id, u32 *divider)
> +{
> +   u32 ret_payload[PAYLOAD_ARG_CNT];
> +   int ret;
> +
> +   ret = invoke_pm_fn(PM_CLOCK_GETDIVIDER, clock_id, 0, 0, 0, 
> ret_payload);
> +   *divider = ret_payload[1];
> +
> +   return ret;
> +}
> +
> +/**
> + * zynqmp_pm_clock_setrate - Set the clock rate for given id
> + * @clock_id:  ID of the clock
> + * @rate:  rate value in hz
> + *
> + * This function is used by master to set rate for any clock.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
So this can set rate only 4G max ?

> +static int zynqmp_pm_clock_setrate(u32 clock_id, u32 rate)
> +{
> +   return invoke_pm_fn(PM_CLOCK_SETRATE, clock_id, rate, 0, 0, NULL);
> +}
> +
> +/**
> + * zynqmp_pm_clock_getrate - Get the clock rate for given id
> + * @clock_id:  ID of the clock
> + * @rate:  rate value in hz
> + *
> + * This function is used by master to get rate
> + * for any clock.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
Same question here?

> +static int zynqmp_pm_clock_getrate(u32 clock_id, u32 *rate)
> +{
> +   u32 ret_payload[PAYLOAD_ARG_CNT];
> +   int ret;
> +
> +   ret = invoke_pm_fn(PM_CLOCK_GETRATE, clock_id, 0, 0, 0, ret_payload);
> +   *rate = ret_payload[1];
> +
> +   return ret;
> +}
> +
Also  what is the difference between set rate and set divider?

Re: [PATCH v3 2/4] drivers: firmware: xilinx: Add ZynqMP firmware driver

[Shubhrajyoti Datta]

Hi,

Thanks for the patch.
A few questions below.


On Thu, Jan 25, 2018 at 4:51 AM, Jolly Shah  wrote:
> This patch is adding communication layer with firmware.
> Firmware driver provides an interface to firmware APIs.
> Interface APIs can be used by any driver to communicate to
> PMUFW(Platform Management Unit). All requests go through ATF.
>
> Signed-off-by: Jolly Shah 
> Signed-off-by: Rajan Vaja 
> ---

>

> +/**
> + * zynqmp_pm_clock_enable - Enable the clock for given id
> + * @clock_id:  ID of the clock to be enabled

Does it enable all the parents also if they are disabled?

> + *
> + * This function is used by master to enable the clock
> + * including peripherals and PLL clocks.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_enable(u32 clock_id)
> +{
> +   return invoke_pm_fn(PM_CLOCK_ENABLE, clock_id, 0, 0, 0, NULL);
> +}
> +
> +/**
> + * zynqmp_pm_clock_disable - Disable the clock for given id
> + * @clock_id:  ID of the clock to be disable
> + *
> + * This function is used by master to disable the clock
> + * including peripherals and PLL clocks.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_disable(u32 clock_id)
> +{
> +   return invoke_pm_fn(PM_CLOCK_DISABLE, clock_id, 0, 0, 0, NULL);
> +}
> +
> +/**
> + * zynqmp_pm_clock_getstate - Get the clock state for given id
> + * @clock_id:  ID of the clock to be queried
> + * @state: 1/0 (Enabled/Disabled)
> + *
> + * This function is used by master to get the state of clock
> + * including peripherals and PLL clocks.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_getstate(u32 clock_id, u32 *state)
> +{
> +   u32 ret_payload[PAYLOAD_ARG_CNT];
> +   int ret;
> +
> +   ret = invoke_pm_fn(PM_CLOCK_GETSTATE, clock_id, 0, 0, 0, ret_payload);
> +   *state = ret_payload[1];
> +
> +   return ret;
> +}
> +
> +/**
> + * zynqmp_pm_clock_setdivider - Set the clock divider for given id
> + * @clock_id:  ID of the clock
> + * @div_type:  TYPE_DIV1: div1
> + * TYPE_DIV2: div2
div type didnt see in the signature.



> + * @divider:   divider value.
> + *
> + * This function is used by master to set divider for any clock
> + * to achieve desired rate.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_setdivider(u32 clock_id, u32 divider)
> +{
> +   return invoke_pm_fn(PM_CLOCK_SETDIVIDER, clock_id, divider, 0, 0, 
> NULL);
> +}
> +
> +/**
> + * zynqmp_pm_clock_getdivider - Get the clock divider for given id
> + * @clock_id:  ID of the clock
> + * @div_type:  TYPE_DIV1: div1
> + * TYPE_DIV2: div2
didnt see this  below.

> + * @divider:   divider value.
> + *
> + * This function is used by master to get divider values
> + * for any clock.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
> +static int zynqmp_pm_clock_getdivider(u32 clock_id, u32 *divider)
> +{
> +   u32 ret_payload[PAYLOAD_ARG_CNT];
> +   int ret;
> +
> +   ret = invoke_pm_fn(PM_CLOCK_GETDIVIDER, clock_id, 0, 0, 0, 
> ret_payload);
> +   *divider = ret_payload[1];
> +
> +   return ret;
> +}
> +
> +/**
> + * zynqmp_pm_clock_setrate - Set the clock rate for given id
> + * @clock_id:  ID of the clock
> + * @rate:  rate value in hz
> + *
> + * This function is used by master to set rate for any clock.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
So this can set rate only 4G max ?

> +static int zynqmp_pm_clock_setrate(u32 clock_id, u32 rate)
> +{
> +   return invoke_pm_fn(PM_CLOCK_SETRATE, clock_id, rate, 0, 0, NULL);
> +}
> +
> +/**
> + * zynqmp_pm_clock_getrate - Get the clock rate for given id
> + * @clock_id:  ID of the clock
> + * @rate:  rate value in hz
> + *
> + * This function is used by master to get rate
> + * for any clock.
> + *
> + * Return: Returns status, either success or error+reason.
> + */
Same question here?

> +static int zynqmp_pm_clock_getrate(u32 clock_id, u32 *rate)
> +{
> +   u32 ret_payload[PAYLOAD_ARG_CNT];
> +   int ret;
> +
> +   ret = invoke_pm_fn(PM_CLOCK_GETRATE, clock_id, 0, 0, 0, ret_payload);
> +   *rate = ret_payload[1];
> +
> +   return ret;
> +}
> +
Also  what is the difference between set rate and set divider?

Question about dmesg/sysfs output when retpoline config is disabled

[Misono, Tomohiro]

Hello,

I think dmesg/sysfs output messages are not suitable if retpoline config is off:

I intentionally compiled the kernel 4.15.0 with CONFIG_RETPOLINE=n for test and 
boot it with the following kernel command line option to check dmesg/sysfs:

(a) no command line option or "spectre_v2=on" or "spectre_v2=auto"
$ dmesg | grep -i spectre
[0.017714] Spectre V2 mitigation: Vulnerable: Minimal generic ASM retpoline
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Minimal generic ASM retpoline

(b) "spectre_v2=off"
$ dmesg | grep -i spectre
[0.017002] Spectre V2 mitigation: disabled on command line.
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable

(c) "spectre_v2=retpoline"
$ dmesg | grep -i spectre
[0.018002] Spectre V2 mitigation: kernel not compiled with retpoline; no 
mitigation available! 
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable

I think the output of (c) is correct for this case, or are these outputs 
actually right?

Also, the output of (a) is the same with following condition:
 (1) CONFIG_RETPOLINE=n, and
 (2) CONFIG_RETPOLINE=y but the compiler did not support retpoline
These cannot be distinguished unless option of (c) is explicitly used.

Regards,
Tomohiro Misono

Question about dmesg/sysfs output when retpoline config is disabled

[Misono, Tomohiro]

Hello,

I think dmesg/sysfs output messages are not suitable if retpoline config is off:

I intentionally compiled the kernel 4.15.0 with CONFIG_RETPOLINE=n for test and 
boot it with the following kernel command line option to check dmesg/sysfs:

(a) no command line option or "spectre_v2=on" or "spectre_v2=auto"
$ dmesg | grep -i spectre
[0.017714] Spectre V2 mitigation: Vulnerable: Minimal generic ASM retpoline
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Minimal generic ASM retpoline

(b) "spectre_v2=off"
$ dmesg | grep -i spectre
[0.017002] Spectre V2 mitigation: disabled on command line.
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable

(c) "spectre_v2=retpoline"
$ dmesg | grep -i spectre
[0.018002] Spectre V2 mitigation: kernel not compiled with retpoline; no 
mitigation available! 
$ cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable

I think the output of (c) is correct for this case, or are these outputs 
actually right?

Also, the output of (a) is the same with following condition:
 (1) CONFIG_RETPOLINE=n, and
 (2) CONFIG_RETPOLINE=y but the compiler did not support retpoline
These cannot be distinguished unless option of (c) is explicitly used.

Regards,
Tomohiro Misono

[PATCH 1/1] scsi: ufs: make sure all interrupts are processed

[Asutosh Das]

From: Venkat Gopalakrishnan 

As multiple requests are submitted to the ufs host controller in
parallel there could be instances where the command completion
interrupt arrives later for a request that is already processed
earlier as the corresponding doorbell was cleared when handling
the previous interrupt. Read the interrupt status in a loop after
processing the received interrupt to catch such interrupts and
handle it.

Signed-off-by: Venkat Gopalakrishnan 
Signed-off-by: Asutosh Das 
---
 drivers/scsi/ufs/ufshcd.c | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8af2af3..58d81de 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5357,19 +5357,30 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba)
u32 intr_status, enabled_intr_status;
irqreturn_t retval = IRQ_NONE;
struct ufs_hba *hba = __hba;
+   int retries = hba->nutrs;
 
spin_lock(hba->host->host_lock);
intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
-   enabled_intr_status =
-   intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
 
-   if (intr_status)
-   ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
+   /*
+* There could be max of hba->nutrs reqs in flight and in worst case
+* if the reqs get finished 1 by 1 after the interrupt status is
+* read, make sure we handle them by checking the interrupt status
+* again in a loop until we process all of the reqs before returning.
+*/
+   do {
+   enabled_intr_status =
+   intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+   if (intr_status)
+   ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
+   if (enabled_intr_status) {
+   ufshcd_sl_intr(hba, enabled_intr_status);
+   retval = IRQ_HANDLED;
+   }
+
+   intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
+   } while (intr_status && --retries);
 
-   if (enabled_intr_status) {
-   ufshcd_sl_intr(hba, enabled_intr_status);
-   retval = IRQ_HANDLED;
-   }
spin_unlock(hba->host->host_lock);
return retval;
 }
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project.

[PATCH 1/1] scsi: ufs: make sure all interrupts are processed

[Asutosh Das]

From: Venkat Gopalakrishnan 

As multiple requests are submitted to the ufs host controller in
parallel there could be instances where the command completion
interrupt arrives later for a request that is already processed
earlier as the corresponding doorbell was cleared when handling
the previous interrupt. Read the interrupt status in a loop after
processing the received interrupt to catch such interrupts and
handle it.

Signed-off-by: Venkat Gopalakrishnan 
Signed-off-by: Asutosh Das 
---
 drivers/scsi/ufs/ufshcd.c | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8af2af3..58d81de 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5357,19 +5357,30 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba)
u32 intr_status, enabled_intr_status;
irqreturn_t retval = IRQ_NONE;
struct ufs_hba *hba = __hba;
+   int retries = hba->nutrs;
 
spin_lock(hba->host->host_lock);
intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
-   enabled_intr_status =
-   intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
 
-   if (intr_status)
-   ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
+   /*
+* There could be max of hba->nutrs reqs in flight and in worst case
+* if the reqs get finished 1 by 1 after the interrupt status is
+* read, make sure we handle them by checking the interrupt status
+* again in a loop until we process all of the reqs before returning.
+*/
+   do {
+   enabled_intr_status =
+   intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+   if (intr_status)
+   ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
+   if (enabled_intr_status) {
+   ufshcd_sl_intr(hba, enabled_intr_status);
+   retval = IRQ_HANDLED;
+   }
+
+   intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
+   } while (intr_status && --retries);
 
-   if (enabled_intr_status) {
-   ufshcd_sl_intr(hba, enabled_intr_status);
-   retval = IRQ_HANDLED;
-   }
spin_unlock(hba->host->host_lock);
return retval;
 }
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH v2 3/3] fsi/master-gpio: Add external mode

[Joel Stanley]

Hi Jeremy,

On Thu, Jun 22, 2017 at 7:40 AM, Jeremy Kerr  wrote:
> This change introduces an 'external mode' for GPIO-based FSI masters,
> allowing the clock and data lines to be driven by an external source.
> For example, external mode is selected by a user when an external debug
> device is attached to the FSI pins.
>
> To do this, we need to set specific states for the trans, mux and enable
> gpios, and prevent access to clk & data from the FSI core code (by
> returning EBUSY).
>
> External mode is controlled by a sysfs attribute, so add the relevent
> information to Documentation/ABI/
>
> Signed-off-by: Jeremy Kerr 
> Reviewed-by: Joel Stanley 

This one never made it into Greg's tree (I assume because we didn't cc him).

Can you resend with Greg on cc please?

Cheers,

Joel

> ---
>  .../ABI/testing/sysfs-driver-fsi-master-gpio   | 10 +++
>  drivers/fsi/fsi-master-gpio.c  | 78 
> +-
>  2 files changed, 86 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio 
> b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
> new file mode 100644
> index 000..9667bb4
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
> @@ -0,0 +1,10 @@
> +What:   /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
> +Date:   June 2017
> +KernelVersion:  4.12
> +Contact:j...@ozlabs.org
> +Description:
> +Controls access arbitration for GPIO-based FSI master. A
> +   value of 0 (the default) sets normal mode, where the
> +   driver performs FSI bus transactions, 1 sets external mode,
> +   where the FSI bus is driven externally (for example, by
> +   a debug device).
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index a6d602e..b54c213 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -59,6 +59,7 @@ struct fsi_master_gpio {
> struct gpio_desc*gpio_trans;/* Voltage translator */
> struct gpio_desc*gpio_enable;   /* FSI enable */
> struct gpio_desc*gpio_mux;  /* Mux control */
> +   boolexternal_mode;
>  };
>
>  #define CREATE_TRACE_POINTS
> @@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio 
> *master, uint8_t slave,
> int rc;
>
> spin_lock_irqsave(>cmd_lock, flags);
> +
> +   if (master->external_mode) {
> +   spin_unlock_irqrestore(>cmd_lock, flags);
> +   return -EBUSY;
> +   }
> +
> serial_out(master, cmd);
> echo_delay(master);
> rc = poll_for_response(master, slave, resp_len, resp);
> @@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master 
> *_master, int link)
> trace_fsi_master_gpio_break(master);
>
> spin_lock_irqsave(>cmd_lock, flags);
> +   if (master->external_mode) {
> +   spin_unlock_irqrestore(>cmd_lock, flags);
> +   return -EBUSY;
> +   }
> set_sda_output(master, 1);
> sda_out(master, 1);
> clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
> @@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio 
> *master)
> clock_zeros(master, FSI_INIT_CLOCKS);
>  }
>
> +static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
> +{
> +   gpiod_direction_output(master->gpio_mux, 0);
> +   gpiod_direction_output(master->gpio_trans, 0);
> +   gpiod_direction_output(master->gpio_enable, 1);
> +   gpiod_direction_input(master->gpio_clk);
> +   gpiod_direction_input(master->gpio_data);
> +}
> +
>  static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
>  {
> struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> unsigned long flags;
> +   int rc = -EBUSY;
>
> if (link != 0)
> return -ENODEV;
>
> spin_lock_irqsave(>cmd_lock, flags);
> -   gpiod_set_value(master->gpio_enable, 1);
> +   if (!master->external_mode) {
> +   gpiod_set_value(master->gpio_enable, 1);
> +   rc = 0;
> +   }
> spin_unlock_irqrestore(>cmd_lock, flags);
>
> -   return 0;
> +   return rc;
> +}
> +
> +static ssize_t external_mode_show(struct device *dev,
> +   struct device_attribute *attr, char *buf)
> +{
> +   struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +
> +   return snprintf(buf, PAGE_SIZE - 1, "%u\n",
> +   master->external_mode ? 1 : 0);
> +}
> +
> +static ssize_t external_mode_store(struct device *dev,
> +   struct device_attribute *attr, const char *buf, size_t count)
> +{
> +   struct fsi_master_gpio *master = 

Re: [PATCH v2 3/3] fsi/master-gpio: Add external mode

[Joel Stanley]

Hi Jeremy,

On Thu, Jun 22, 2017 at 7:40 AM, Jeremy Kerr  wrote:
> This change introduces an 'external mode' for GPIO-based FSI masters,
> allowing the clock and data lines to be driven by an external source.
> For example, external mode is selected by a user when an external debug
> device is attached to the FSI pins.
>
> To do this, we need to set specific states for the trans, mux and enable
> gpios, and prevent access to clk & data from the FSI core code (by
> returning EBUSY).
>
> External mode is controlled by a sysfs attribute, so add the relevent
> information to Documentation/ABI/
>
> Signed-off-by: Jeremy Kerr 
> Reviewed-by: Joel Stanley 

This one never made it into Greg's tree (I assume because we didn't cc him).

Can you resend with Greg on cc please?

Cheers,

Joel

> ---
>  .../ABI/testing/sysfs-driver-fsi-master-gpio   | 10 +++
>  drivers/fsi/fsi-master-gpio.c  | 78 
> +-
>  2 files changed, 86 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio 
> b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
> new file mode 100644
> index 000..9667bb4
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
> @@ -0,0 +1,10 @@
> +What:   /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
> +Date:   June 2017
> +KernelVersion:  4.12
> +Contact:j...@ozlabs.org
> +Description:
> +Controls access arbitration for GPIO-based FSI master. A
> +   value of 0 (the default) sets normal mode, where the
> +   driver performs FSI bus transactions, 1 sets external mode,
> +   where the FSI bus is driven externally (for example, by
> +   a debug device).
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index a6d602e..b54c213 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -59,6 +59,7 @@ struct fsi_master_gpio {
> struct gpio_desc*gpio_trans;/* Voltage translator */
> struct gpio_desc*gpio_enable;   /* FSI enable */
> struct gpio_desc*gpio_mux;  /* Mux control */
> +   boolexternal_mode;
>  };
>
>  #define CREATE_TRACE_POINTS
> @@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio 
> *master, uint8_t slave,
> int rc;
>
> spin_lock_irqsave(>cmd_lock, flags);
> +
> +   if (master->external_mode) {
> +   spin_unlock_irqrestore(>cmd_lock, flags);
> +   return -EBUSY;
> +   }
> +
> serial_out(master, cmd);
> echo_delay(master);
> rc = poll_for_response(master, slave, resp_len, resp);
> @@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master 
> *_master, int link)
> trace_fsi_master_gpio_break(master);
>
> spin_lock_irqsave(>cmd_lock, flags);
> +   if (master->external_mode) {
> +   spin_unlock_irqrestore(>cmd_lock, flags);
> +   return -EBUSY;
> +   }
> set_sda_output(master, 1);
> sda_out(master, 1);
> clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
> @@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio 
> *master)
> clock_zeros(master, FSI_INIT_CLOCKS);
>  }
>
> +static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
> +{
> +   gpiod_direction_output(master->gpio_mux, 0);
> +   gpiod_direction_output(master->gpio_trans, 0);
> +   gpiod_direction_output(master->gpio_enable, 1);
> +   gpiod_direction_input(master->gpio_clk);
> +   gpiod_direction_input(master->gpio_data);
> +}
> +
>  static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
>  {
> struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> unsigned long flags;
> +   int rc = -EBUSY;
>
> if (link != 0)
> return -ENODEV;
>
> spin_lock_irqsave(>cmd_lock, flags);
> -   gpiod_set_value(master->gpio_enable, 1);
> +   if (!master->external_mode) {
> +   gpiod_set_value(master->gpio_enable, 1);
> +   rc = 0;
> +   }
> spin_unlock_irqrestore(>cmd_lock, flags);
>
> -   return 0;
> +   return rc;
> +}
> +
> +static ssize_t external_mode_show(struct device *dev,
> +   struct device_attribute *attr, char *buf)
> +{
> +   struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +
> +   return snprintf(buf, PAGE_SIZE - 1, "%u\n",
> +   master->external_mode ? 1 : 0);
> +}
> +
> +static ssize_t external_mode_store(struct device *dev,
> +   struct device_attribute *attr, const char *buf, size_t count)
> +{
> +   struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +   unsigned long flags, val;
> 

[PATCH 1/1] scsi: ufs-qcom: remove broken hci version quirk

[Asutosh Das]

From: Subhash Jadavani 

UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION is only applicable for QCOM UFS host
controller version 2.x.y and this has been fixed from version 3.x.y
onwards, hence this change removes this quirk for version 3.x.y onwards.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Asutosh Das 
---
 drivers/scsi/ufs/ufs-qcom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2..221820a 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1098,7 +1098,7 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
hba->quirks |= UFSHCD_QUIRK_BROKEN_LCC;
}
 
-   if (host->hw_ver.major >= 0x2) {
+   if (host->hw_ver.major == 0x2) {
hba->quirks |= UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION;
 
if (!ufs_qcom_cap_qunipro(host))
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project.

[PATCH 1/1] scsi: ufs-qcom: remove broken hci version quirk

[Asutosh Das]

From: Subhash Jadavani 

UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION is only applicable for QCOM UFS host
controller version 2.x.y and this has been fixed from version 3.x.y
onwards, hence this change removes this quirk for version 3.x.y onwards.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Asutosh Das 
---
 drivers/scsi/ufs/ufs-qcom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2..221820a 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1098,7 +1098,7 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
hba->quirks |= UFSHCD_QUIRK_BROKEN_LCC;
}
 
-   if (host->hw_ver.major >= 0x2) {
+   if (host->hw_ver.major == 0x2) {
hba->quirks |= UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION;
 
if (!ufs_qcom_cap_qunipro(host))
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH AUTOSEL for 3.18 36/40] powerpc/xmon: Avoid tripping SMP hardlockup watchdog

[Michael Ellerman]

alexander.le...@verizon.com writes:

> On Thu, Dec 14, 2017 at 12:10:39AM +1100, Michael Ellerman wrote:
>>alexander.le...@verizon.com writes:
>>
>>> From: Nicholas Piggin 
>>>
>>> [ Upstream commit 064996d62a33ffe10264b5af5dca92d54f60f806 ]
>>>
>>> The SMP hardlockup watchdog cross-checks other CPUs for lockups, which
>>> causes xmon headaches because it's assuming interrupts hard disabled
>>> means no watchdog troubles. Try to improve that by calling
>>> touch_nmi_watchdog() in obvious places where secondaries are spinning.
>>>
>>> Also annotate these spin loops with spin_begin/end calls.
>>
>>These macros didn't exist until 4.13, and haven't been backported AFAIK.
>
> But the touch_nmi_watchdog() bits are something we want in stable, right?

I don't think you need them unless you've also back ported
arch/powerpc/kernel/watchdog.c, which I don't think you have.

Maybe Nick can confirm?


Also, I thought 3.18 was EOL?

cheers

Re: [PATCH AUTOSEL for 3.18 36/40] powerpc/xmon: Avoid tripping SMP hardlockup watchdog

[Michael Ellerman]

alexander.le...@verizon.com writes:

> On Thu, Dec 14, 2017 at 12:10:39AM +1100, Michael Ellerman wrote:
>>alexander.le...@verizon.com writes:
>>
>>> From: Nicholas Piggin 
>>>
>>> [ Upstream commit 064996d62a33ffe10264b5af5dca92d54f60f806 ]
>>>
>>> The SMP hardlockup watchdog cross-checks other CPUs for lockups, which
>>> causes xmon headaches because it's assuming interrupts hard disabled
>>> means no watchdog troubles. Try to improve that by calling
>>> touch_nmi_watchdog() in obvious places where secondaries are spinning.
>>>
>>> Also annotate these spin loops with spin_begin/end calls.
>>
>>These macros didn't exist until 4.13, and haven't been backported AFAIK.
>
> But the touch_nmi_watchdog() bits are something we want in stable, right?

I don't think you need them unless you've also back ported
arch/powerpc/kernel/watchdog.c, which I don't think you have.

Maybe Nick can confirm?


Also, I thought 3.18 was EOL?

cheers

Re: [RFC PATCH 1/9] media: add request API core and UAPI

[Alexandre Courbot]

Hi Sakari, thanks for the review!

The version you reviewed is not the latest one, but I suppose most of
your comments still apply.

On Fri, Jan 26, 2018 at 5:39 PM, Sakari Ailus  wrote:
> Hi Alexandre,
>
> I remember it was discussed that the work after the V4L2 jobs API would
> continue from the existing request API patches. I see that at least the
> rather important support for events is missing in this version. Why was it
> left out?

Request completion is signaled by polling on the request FD, so we
don't need to rely on V4L2 events to signal this anymore. If we want
to signal different kinds of events on requests we could implement a
more sophisticated event system on top of that, but for our current
needs polling is sufficient.

What other kind of event besides completion could we want to deliver
to user-space from a request?

>
> I also see that variable size IOCTL argument support is no longer included.

Do we need this for the request API?

>
> On Fri, Dec 15, 2017 at 04:56:17PM +0900, Alexandre Courbot wrote:
>> The request API provides a way to group buffers and device parameters
>> into units of work to be queued and executed. This patch introduces the
>> UAPI and core framework.
>>
>> This patch is based on the previous work by Laurent Pinchart. The core
>> has changed considerably, but the UAPI is mostly untouched.
>>
>> Signed-off-by: Alexandre Courbot 
>> ---
>>  drivers/media/Makefile   |   3 +-
>>  drivers/media/media-device.c |   6 +
>>  drivers/media/media-request.c| 390 
>> +++
>>  drivers/media/v4l2-core/v4l2-ioctl.c |   2 +-
>>  include/media/media-device.h |   3 +
>>  include/media/media-entity.h |   6 +
>>  include/media/media-request.h| 269 
>>  include/uapi/linux/media.h   |  11 +
>>  8 files changed, 688 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/media/media-request.c
>>  create mode 100644 include/media/media-request.h
>>
>> diff --git a/drivers/media/Makefile b/drivers/media/Makefile
>> index 594b462ddf0e..985d35ec6b29 100644
>> --- a/drivers/media/Makefile
>> +++ b/drivers/media/Makefile
>> @@ -3,7 +3,8 @@
>>  # Makefile for the kernel multimedia device drivers.
>>  #
>>
>> -media-objs   := media-device.o media-devnode.o media-entity.o
>> +media-objs   := media-device.o media-devnode.o media-entity.o \
>> +media-request.o
>>
>>  #
>>  # I2C drivers should come before other drivers, otherwise they'll fail
>> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
>> index e79f72b8b858..045cec7d2de9 100644
>> --- a/drivers/media/media-device.c
>> +++ b/drivers/media/media-device.c
>> @@ -32,6 +32,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #ifdef CONFIG_MEDIA_CONTROLLER
>>
>> @@ -407,6 +408,7 @@ static const struct media_ioctl_info ioctl_info[] = {
>>   MEDIA_IOC(ENUM_LINKS, media_device_enum_links, 
>> MEDIA_IOC_FL_GRAPH_MUTEX),
>>   MEDIA_IOC(SETUP_LINK, media_device_setup_link, 
>> MEDIA_IOC_FL_GRAPH_MUTEX),
>>   MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, 
>> MEDIA_IOC_FL_GRAPH_MUTEX),
>> + MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
>>  };
>>
>>  static long media_device_ioctl(struct file *filp, unsigned int cmd,
>> @@ -688,6 +690,10 @@ EXPORT_SYMBOL_GPL(media_device_init);
>>
>>  void media_device_cleanup(struct media_device *mdev)
>>  {
>> + if (mdev->req_queue) {
>> + mdev->req_queue->ops->release(mdev->req_queue);
>> + mdev->req_queue = NULL;
>> + }
>>   ida_destroy(>entity_internal_idx);
>>   mdev->entity_internal_idx_max = 0;
>>   media_graph_walk_cleanup(>pm_count_walk);
>> diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
>> new file mode 100644
>> index ..15dc65ddfe41
>> --- /dev/null
>> +++ b/drivers/media/media-request.c
>> @@ -0,0 +1,390 @@
>> +/*
>> + * Request and request queue base management
>> + *
>> + * Copyright (C) 2017, The Chromium OS Authors.  All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>
> Alphabetic order, please.

Fixed.

>
>> +
>> +const struct file_operations request_fops;
>> +
>> +void media_request_get(struct media_request *req)
>
> Please return the media request, too.

Fixed.

>
>> +{
>> + kref_get(>kref);
>> +}
>> 

Re: [RFC PATCH 1/9] media: add request API core and UAPI

[Alexandre Courbot]

Hi Sakari, thanks for the review!

The version you reviewed is not the latest one, but I suppose most of
your comments still apply.

On Fri, Jan 26, 2018 at 5:39 PM, Sakari Ailus  wrote:
> Hi Alexandre,
>
> I remember it was discussed that the work after the V4L2 jobs API would
> continue from the existing request API patches. I see that at least the
> rather important support for events is missing in this version. Why was it
> left out?

Request completion is signaled by polling on the request FD, so we
don't need to rely on V4L2 events to signal this anymore. If we want
to signal different kinds of events on requests we could implement a
more sophisticated event system on top of that, but for our current
needs polling is sufficient.

What other kind of event besides completion could we want to deliver
to user-space from a request?

>
> I also see that variable size IOCTL argument support is no longer included.

Do we need this for the request API?

>
> On Fri, Dec 15, 2017 at 04:56:17PM +0900, Alexandre Courbot wrote:
>> The request API provides a way to group buffers and device parameters
>> into units of work to be queued and executed. This patch introduces the
>> UAPI and core framework.
>>
>> This patch is based on the previous work by Laurent Pinchart. The core
>> has changed considerably, but the UAPI is mostly untouched.
>>
>> Signed-off-by: Alexandre Courbot 
>> ---
>>  drivers/media/Makefile   |   3 +-
>>  drivers/media/media-device.c |   6 +
>>  drivers/media/media-request.c| 390 
>> +++
>>  drivers/media/v4l2-core/v4l2-ioctl.c |   2 +-
>>  include/media/media-device.h |   3 +
>>  include/media/media-entity.h |   6 +
>>  include/media/media-request.h| 269 
>>  include/uapi/linux/media.h   |  11 +
>>  8 files changed, 688 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/media/media-request.c
>>  create mode 100644 include/media/media-request.h
>>
>> diff --git a/drivers/media/Makefile b/drivers/media/Makefile
>> index 594b462ddf0e..985d35ec6b29 100644
>> --- a/drivers/media/Makefile
>> +++ b/drivers/media/Makefile
>> @@ -3,7 +3,8 @@
>>  # Makefile for the kernel multimedia device drivers.
>>  #
>>
>> -media-objs   := media-device.o media-devnode.o media-entity.o
>> +media-objs   := media-device.o media-devnode.o media-entity.o \
>> +media-request.o
>>
>>  #
>>  # I2C drivers should come before other drivers, otherwise they'll fail
>> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
>> index e79f72b8b858..045cec7d2de9 100644
>> --- a/drivers/media/media-device.c
>> +++ b/drivers/media/media-device.c
>> @@ -32,6 +32,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #ifdef CONFIG_MEDIA_CONTROLLER
>>
>> @@ -407,6 +408,7 @@ static const struct media_ioctl_info ioctl_info[] = {
>>   MEDIA_IOC(ENUM_LINKS, media_device_enum_links, 
>> MEDIA_IOC_FL_GRAPH_MUTEX),
>>   MEDIA_IOC(SETUP_LINK, media_device_setup_link, 
>> MEDIA_IOC_FL_GRAPH_MUTEX),
>>   MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, 
>> MEDIA_IOC_FL_GRAPH_MUTEX),
>> + MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
>>  };
>>
>>  static long media_device_ioctl(struct file *filp, unsigned int cmd,
>> @@ -688,6 +690,10 @@ EXPORT_SYMBOL_GPL(media_device_init);
>>
>>  void media_device_cleanup(struct media_device *mdev)
>>  {
>> + if (mdev->req_queue) {
>> + mdev->req_queue->ops->release(mdev->req_queue);
>> + mdev->req_queue = NULL;
>> + }
>>   ida_destroy(>entity_internal_idx);
>>   mdev->entity_internal_idx_max = 0;
>>   media_graph_walk_cleanup(>pm_count_walk);
>> diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
>> new file mode 100644
>> index ..15dc65ddfe41
>> --- /dev/null
>> +++ b/drivers/media/media-request.c
>> @@ -0,0 +1,390 @@
>> +/*
>> + * Request and request queue base management
>> + *
>> + * Copyright (C) 2017, The Chromium OS Authors.  All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>
> Alphabetic order, please.

Fixed.

>
>> +
>> +const struct file_operations request_fops;
>> +
>> +void media_request_get(struct media_request *req)
>
> Please return the media request, too.

Fixed.

>
>> +{
>> + kref_get(>kref);
>> +}
>> +EXPORT_SYMBOL_GPL(media_request_get);
>> +
>> +struct 

[PATCH 1/1] scsi: ufs: add reference counting for scsi block requests

[Asutosh Das]

From: Subhash Jadavani 

Currently we call the scsi_block_requests()/scsi_unblock_requests()
whenever we want to block/unblock scsi requests but as there is no
reference counting, nesting of these calls could leave us in undesired
state sometime. Consider following call flow sequence:
1. func1() calls scsi_block_requests() but calls func2() before
   calling scsi_unblock_requests()
2. func2() calls scsi_block_requests()
3. func2() calls scsi_unblock_requests()
4. func1() calls scsi_unblock_requests()

As there is no reference counting, we will have scsi requests unblocked
after #3 instead of it to be unblocked only after #4. Though we may not
have failures seen with this, we might run into some failures in future.
Better solution would be to fix this by adding reference counting.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Can Guo 
Signed-off-by: Asutosh Das 
---
 drivers/scsi/ufs/ufshcd.c | 44 +---
 drivers/scsi/ufs/ufshcd.h |  5 +
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8af2af3..a9ebc8d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -264,6 +264,36 @@ static inline void ufshcd_disable_irq(struct ufs_hba *hba)
}
 }
 
+void ufshcd_scsi_unblock_requests(struct ufs_hba *hba)
+{
+   unsigned long flags;
+   bool unblock = false;
+
+   spin_lock_irqsave(hba->host->host_lock, flags);
+   hba->scsi_block_reqs_cnt--;
+   unblock = !hba->scsi_block_reqs_cnt;
+   spin_unlock_irqrestore(hba->host->host_lock, flags);
+   if (unblock)
+   scsi_unblock_requests(hba->host);
+}
+EXPORT_SYMBOL(ufshcd_scsi_unblock_requests);
+
+static inline void __ufshcd_scsi_block_requests(struct ufs_hba *hba)
+{
+   if (!hba->scsi_block_reqs_cnt++)
+   scsi_block_requests(hba->host);
+}
+
+void ufshcd_scsi_block_requests(struct ufs_hba *hba)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(hba->host->host_lock, flags);
+   __ufshcd_scsi_block_requests(hba);
+   spin_unlock_irqrestore(hba->host->host_lock, flags);
+}
+EXPORT_SYMBOL(ufshcd_scsi_block_requests);
+
 /* replace non-printable or non-ASCII characters with spaces */
 static inline void ufshcd_remove_non_printable(char *val)
 {
@@ -1079,12 +1109,12 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba 
*hba)
 * make sure that there are no outstanding requests when
 * clock scaling is in progress
 */
-   scsi_block_requests(hba->host);
+   ufshcd_scsi_block_requests(hba);
down_write(>clk_scaling_lock);
if (ufshcd_wait_for_doorbell_clr(hba, DOORBELL_CLR_TOUT_US)) {
ret = -EBUSY;
up_write(>clk_scaling_lock);
-   scsi_unblock_requests(hba->host);
+   ufshcd_scsi_unblock_requests(hba);
}
 
return ret;
@@ -1093,7 +1123,7 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba 
*hba)
 static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba)
 {
up_write(>clk_scaling_lock);
-   scsi_unblock_requests(hba->host);
+   ufshcd_scsi_unblock_requests(hba);
 }
 
 /**
@@ -1413,7 +1443,7 @@ static void ufshcd_ungate_work(struct work_struct *work)
hba->clk_gating.is_suspended = false;
}
 unblock_reqs:
-   scsi_unblock_requests(hba->host);
+   ufshcd_scsi_unblock_requests(hba);
 }
 
 /**
@@ -1469,7 +1499,7 @@ int ufshcd_hold(struct ufs_hba *hba, bool async)
 * work and to enable clocks.
 */
case CLKS_OFF:
-   scsi_block_requests(hba->host);
+   __ufshcd_scsi_block_requests(hba);
hba->clk_gating.state = REQ_CLKS_ON;
trace_ufshcd_clk_gating(dev_name(hba->dev),
hba->clk_gating.state);
@@ -5178,7 +5208,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 
 out:
spin_unlock_irqrestore(hba->host->host_lock, flags);
-   scsi_unblock_requests(hba->host);
+   ufshcd_scsi_unblock_requests(hba);
ufshcd_release(hba);
pm_runtime_put_sync(hba->dev);
 }
@@ -5280,7 +5310,7 @@ static void ufshcd_check_errors(struct ufs_hba *hba)
/* handle fatal errors only when link is functional */
if (hba->ufshcd_state == UFSHCD_STATE_OPERATIONAL) {
/* block commands from scsi mid-layer */
-   scsi_block_requests(hba->host);
+   __ufshcd_scsi_block_requests(hba);
 
hba->ufshcd_state = UFSHCD_STATE_EH_SCHEDULED;
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 1332e54..84f4175 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -498,6 +498,7 @@ struct ufs_stats {
  * 

[PATCH 1/1] scsi: ufs: add reference counting for scsi block requests

[Asutosh Das]

From: Subhash Jadavani 

Currently we call the scsi_block_requests()/scsi_unblock_requests()
whenever we want to block/unblock scsi requests but as there is no
reference counting, nesting of these calls could leave us in undesired
state sometime. Consider following call flow sequence:
1. func1() calls scsi_block_requests() but calls func2() before
   calling scsi_unblock_requests()
2. func2() calls scsi_block_requests()
3. func2() calls scsi_unblock_requests()
4. func1() calls scsi_unblock_requests()

As there is no reference counting, we will have scsi requests unblocked
after #3 instead of it to be unblocked only after #4. Though we may not
have failures seen with this, we might run into some failures in future.
Better solution would be to fix this by adding reference counting.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Can Guo 
Signed-off-by: Asutosh Das 
---
 drivers/scsi/ufs/ufshcd.c | 44 +---
 drivers/scsi/ufs/ufshcd.h |  5 +
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8af2af3..a9ebc8d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -264,6 +264,36 @@ static inline void ufshcd_disable_irq(struct ufs_hba *hba)
}
 }
 
+void ufshcd_scsi_unblock_requests(struct ufs_hba *hba)
+{
+   unsigned long flags;
+   bool unblock = false;
+
+   spin_lock_irqsave(hba->host->host_lock, flags);
+   hba->scsi_block_reqs_cnt--;
+   unblock = !hba->scsi_block_reqs_cnt;
+   spin_unlock_irqrestore(hba->host->host_lock, flags);
+   if (unblock)
+   scsi_unblock_requests(hba->host);
+}
+EXPORT_SYMBOL(ufshcd_scsi_unblock_requests);
+
+static inline void __ufshcd_scsi_block_requests(struct ufs_hba *hba)
+{
+   if (!hba->scsi_block_reqs_cnt++)
+   scsi_block_requests(hba->host);
+}
+
+void ufshcd_scsi_block_requests(struct ufs_hba *hba)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(hba->host->host_lock, flags);
+   __ufshcd_scsi_block_requests(hba);
+   spin_unlock_irqrestore(hba->host->host_lock, flags);
+}
+EXPORT_SYMBOL(ufshcd_scsi_block_requests);
+
 /* replace non-printable or non-ASCII characters with spaces */
 static inline void ufshcd_remove_non_printable(char *val)
 {
@@ -1079,12 +1109,12 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba 
*hba)
 * make sure that there are no outstanding requests when
 * clock scaling is in progress
 */
-   scsi_block_requests(hba->host);
+   ufshcd_scsi_block_requests(hba);
down_write(>clk_scaling_lock);
if (ufshcd_wait_for_doorbell_clr(hba, DOORBELL_CLR_TOUT_US)) {
ret = -EBUSY;
up_write(>clk_scaling_lock);
-   scsi_unblock_requests(hba->host);
+   ufshcd_scsi_unblock_requests(hba);
}
 
return ret;
@@ -1093,7 +1123,7 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba 
*hba)
 static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba)
 {
up_write(>clk_scaling_lock);
-   scsi_unblock_requests(hba->host);
+   ufshcd_scsi_unblock_requests(hba);
 }
 
 /**
@@ -1413,7 +1443,7 @@ static void ufshcd_ungate_work(struct work_struct *work)
hba->clk_gating.is_suspended = false;
}
 unblock_reqs:
-   scsi_unblock_requests(hba->host);
+   ufshcd_scsi_unblock_requests(hba);
 }
 
 /**
@@ -1469,7 +1499,7 @@ int ufshcd_hold(struct ufs_hba *hba, bool async)
 * work and to enable clocks.
 */
case CLKS_OFF:
-   scsi_block_requests(hba->host);
+   __ufshcd_scsi_block_requests(hba);
hba->clk_gating.state = REQ_CLKS_ON;
trace_ufshcd_clk_gating(dev_name(hba->dev),
hba->clk_gating.state);
@@ -5178,7 +5208,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 
 out:
spin_unlock_irqrestore(hba->host->host_lock, flags);
-   scsi_unblock_requests(hba->host);
+   ufshcd_scsi_unblock_requests(hba);
ufshcd_release(hba);
pm_runtime_put_sync(hba->dev);
 }
@@ -5280,7 +5310,7 @@ static void ufshcd_check_errors(struct ufs_hba *hba)
/* handle fatal errors only when link is functional */
if (hba->ufshcd_state == UFSHCD_STATE_OPERATIONAL) {
/* block commands from scsi mid-layer */
-   scsi_block_requests(hba->host);
+   __ufshcd_scsi_block_requests(hba);
 
hba->ufshcd_state = UFSHCD_STATE_EH_SCHEDULED;
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 1332e54..84f4175 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -498,6 +498,7 @@ struct ufs_stats {
  * @urgent_bkops_lvl: keeps track of urgent bkops level for device
  * @is_urgent_bkops_lvl_checked: keeps track 

Re: [PATCH 5/5] USB: serial: f81232: fix bulk_in/out size

[Johan Hovold]

On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Fix Fintek F81232 bulk_in/out size to 64/16 according to the spec.
> http://html.alldatasheet.com/html-pdf/406315/FINTEK/F81232/1762/8/F81232.html
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) 
> ---
>  drivers/usb/serial/f81232.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index a054f69446fd..f3ee537d643c 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
>   },
>   .id_table = id_table,
>   .num_ports =1,
> - .bulk_in_size = 256,
> - .bulk_out_size =256,
> + .bulk_out_size =16,

These fields control the URB buffer sizes and defaults to the corresponding
endpoint max-packet size, which would be 16 for all endpoints according
to the datasheet above.

So it seems you should really be setting bulk_in_size to 64 here (and
possibly leave bulk_out_size unset) as that would appear to match your
device buffer sizes.

Johan

Re: [PATCH 5/5] USB: serial: f81232: fix bulk_in/out size

[Johan Hovold]

On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Fix Fintek F81232 bulk_in/out size to 64/16 according to the spec.
> http://html.alldatasheet.com/html-pdf/406315/FINTEK/F81232/1762/8/F81232.html
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) 
> ---
>  drivers/usb/serial/f81232.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index a054f69446fd..f3ee537d643c 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
>   },
>   .id_table = id_table,
>   .num_ports =1,
> - .bulk_in_size = 256,
> - .bulk_out_size =256,
> + .bulk_out_size =16,

These fields control the URB buffer sizes and defaults to the corresponding
endpoint max-packet size, which would be 16 for all endpoints according
to the datasheet above.

So it seems you should really be setting bulk_in_size to 64 here (and
possibly leave bulk_out_size unset) as that would appear to match your
device buffer sizes.

Johan