Re: [PATCH 1/3] dt-bindings: added new pwm-sifive driver documentation

[Thierry Reding]

On Fri, Apr 27, 2018 at 03:59:56PM -0700, Wesley W. Terpstra wrote:
> Document new PWM device tree bindings for SiFive SoCs.
> 
> Signed-off-by: Wesley W. Terpstra 
> ---
>  .../devicetree/bindings/pwm/pwm-sifive.txt | 28 
> ++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt 
> b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> new file mode 100644
> index 000..7cea20d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> @@ -0,0 +1,28 @@
> +SiFive PWM controller
> +
> +Unlike most other PWM controllers, the SiFive PWM controller currently only
> +supports one period for all channels in the PWM. This is set globally in DTS.
> +The period also has significant restrictions on the values it can achieve,
> +which the driver rounds to the nearest achievable frequency.

How about you encode this in the driver rather than DT? We have several
drivers with similar restrictions and they usually allow the first PWM
channel to choose an arbitrary period and return an error if subsequent
channels request a period that can't be supported.

I think something similar could work with that second restriction. Why
not return an error if the exact period can't be set? Or perhaps allow
some percentage of deviation.

If the exact period is achieved in a deterministic way, it should be
possible for board designers to specify it exactly, so the consumer's
pwms property can simply take the correct period.

> +Required properties:
> +- compatible: should be "sifive,pwm0"

Why not simply "sifive,pwm"? If this is supposed to be some sort of
version number, then it is more customary to use the name of the first
SoC that integrates the IP. There are some exceptions, like for example
when the IP is third-party and is integrated in a number of different
SoCs. In such cases the IP is often properly versioned. But that doesn't
seem to be the case here.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH 1/3] dt-bindings: added new pwm-sifive driver documentation

[Thierry Reding]

On Fri, Apr 27, 2018 at 03:59:56PM -0700, Wesley W. Terpstra wrote:
> Document new PWM device tree bindings for SiFive SoCs.
> 
> Signed-off-by: Wesley W. Terpstra 
> ---
>  .../devicetree/bindings/pwm/pwm-sifive.txt | 28 
> ++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt 
> b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> new file mode 100644
> index 000..7cea20d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> @@ -0,0 +1,28 @@
> +SiFive PWM controller
> +
> +Unlike most other PWM controllers, the SiFive PWM controller currently only
> +supports one period for all channels in the PWM. This is set globally in DTS.
> +The period also has significant restrictions on the values it can achieve,
> +which the driver rounds to the nearest achievable frequency.

How about you encode this in the driver rather than DT? We have several
drivers with similar restrictions and they usually allow the first PWM
channel to choose an arbitrary period and return an error if subsequent
channels request a period that can't be supported.

I think something similar could work with that second restriction. Why
not return an error if the exact period can't be set? Or perhaps allow
some percentage of deviation.

If the exact period is achieved in a deterministic way, it should be
possible for board designers to specify it exactly, so the consumer's
pwms property can simply take the correct period.

> +Required properties:
> +- compatible: should be "sifive,pwm0"

Why not simply "sifive,pwm"? If this is supposed to be some sort of
version number, then it is more customary to use the name of the first
SoC that integrates the IP. There are some exceptions, like for example
when the IP is third-party and is integrated in a number of different
SoCs. In such cases the IP is often properly versioned. But that doesn't
seem to be the case here.

Thierry


signature.asc
Description: PGP signature

Re: LICENSES: Missing ISC text & possibly a category ("Not recommended" vs. "Preferred licenses")

[Greg Kroah-Hartman]

On Sat, Apr 28, 2018 at 11:25:17PM +0200, Rafał Miłecki wrote:
> Hi,
> 
> Due to some maintainers *preferring* BSD-compatible license for DTS
> files [0], I was writing mine using ISC. I had no very special reason
> for it: I was choosing between BSD-2-Clause, MIT and ISC. I've chosen
> ISC as I read about its "removal of language deemed unnecessary".
> 
> I took a moment to look at the new SPDX thing and noticed that:
> 1) File license-rules.rst provides "LICENSES/other/ISC" as an example

Yeah, bad example, we should fix that text up.  Care to send a patch? :)

> 2) License file LICENSES/other/ISC doesn't exist
> 3) ISC is listed as an *example* under the "Not recommended licenses"

Yes, please don't use it if at all possible.

> First of all, as ISC is used by some files in the Linux kernel, I
> think it's worth adding to the LICENSE/*/ISC.

I see it is only used in a very small number of dts files.  Why not just
use BSD-2-Clause instead?  What do you find in ISC that is not available
to you with just BSD?

> Secondly, it isn't 100% clear to me if ISC is preferred or not
> recommended. File license-rules.rst suggests the later by listing it
> as an example for "Not recommended". It's just an example though, so
> I'm not 100% sure without seeing it in either: "preferred" or "other"
> directories. Also if anyone finds it "Not recommended", can we get a
> short explanation why is it so, please?

The license is functionally equalivant to BSD-2, so why would you want
to add more complexity here and have two licenses that are the same be
"recommended"?

thanks,

greg k-h

Re: LICENSES: Missing ISC text & possibly a category ("Not recommended" vs. "Preferred licenses")

[Greg Kroah-Hartman]

On Sat, Apr 28, 2018 at 11:25:17PM +0200, Rafał Miłecki wrote:
> Hi,
> 
> Due to some maintainers *preferring* BSD-compatible license for DTS
> files [0], I was writing mine using ISC. I had no very special reason
> for it: I was choosing between BSD-2-Clause, MIT and ISC. I've chosen
> ISC as I read about its "removal of language deemed unnecessary".
> 
> I took a moment to look at the new SPDX thing and noticed that:
> 1) File license-rules.rst provides "LICENSES/other/ISC" as an example

Yeah, bad example, we should fix that text up.  Care to send a patch? :)

> 2) License file LICENSES/other/ISC doesn't exist
> 3) ISC is listed as an *example* under the "Not recommended licenses"

Yes, please don't use it if at all possible.

> First of all, as ISC is used by some files in the Linux kernel, I
> think it's worth adding to the LICENSE/*/ISC.

I see it is only used in a very small number of dts files.  Why not just
use BSD-2-Clause instead?  What do you find in ISC that is not available
to you with just BSD?

> Secondly, it isn't 100% clear to me if ISC is preferred or not
> recommended. File license-rules.rst suggests the later by listing it
> as an example for "Not recommended". It's just an example though, so
> I'm not 100% sure without seeing it in either: "preferred" or "other"
> directories. Also if anyone finds it "Not recommended", can we get a
> short explanation why is it so, please?

The license is functionally equalivant to BSD-2, so why would you want
to add more complexity here and have two licenses that are the same be
"recommended"?

thanks,

greg k-h

Re: [RFD] x86: The future of MPX

[Eric W. Biederman]

Ingo Molnar  writes:

> * Linus Torvalds  wrote:
>
>> On Sat, Apr 28, 2018 at 2:18 AM Ingo Molnar  wrote:
>> 
>> > I just tried the MPX testcases with the latest kernel:
>> 
>> >   deimos:~/tip/tools/testing/selftests/x86> ./mpx-mini-test_64
>> 
>> They need to be run as root. They try to access
>> 
>>  /sys/kernel/debug/tracing/set_ftrace_pid
>>  /sys/kernel/debug/tracing/trace
>> 
>> and that's root-only.
>
> Indeed - but note that they are crashing as root as well here:
>
>   root@deimos:/home/mingo/tip/tools/testing/selftests/x86# ./mpx-mini-test_64 
>   [...]
>   Aborted (core dumped)
>
> ... because I don't even have /sys/kernel/debug/tracing/set_ftrace_pid... :-/
>
> That's despite having ftrace enabled.
>
> After some digging I found out that 'set_ftrace_pid' is dependent on 
> CONFIG_FUNCTION_TRACING=y (not just CONFIG_TRACING=y), which I didn't
> have enabled on this kernel.
>
> After enabling it I got the 64-bit testcase to work:
>
>   #
>   ...
>   ./mpx-mini-test_64 completed successfully
>
> But the 32-bit testcase is erroring out:
>
>   root@deimos:/home/mingo/tip/tools/testing/selftests/x86# ./mpx-mini-test_32 
>   XSAVE is supported by HW & OS
>   XSAVE processor supported state mask: 0x21f
>   XSAVE OS supported state mask: 0x21f
>BNDREGS: size: 64 user: 1 supervisor: 0 aligned: 0
> BNDCSR: size: 64 user: 1 supervisor: 0 aligned: 0
>   executing unmaptest
>   mpx dig (  1) complete, SUCCESS (   0 /0)
>   #BR status == 2, missing bounds table,kernel should have handled!!
>
> v4.17-rc2 based kernel, so it should have the latest MPX code.

If you remove this which tree will this be going into?

I have some refactoring and cleanups of the x86 siginfo code I am
hoping to get in later this development cycle and part of that was
refactoring the mpx code so it works more like everything else.

If the codes away I can just that part of the patchset.

Eric

Re: [RFD] x86: The future of MPX

[Eric W. Biederman]

Ingo Molnar  writes:

> * Linus Torvalds  wrote:
>
>> On Sat, Apr 28, 2018 at 2:18 AM Ingo Molnar  wrote:
>> 
>> > I just tried the MPX testcases with the latest kernel:
>> 
>> >   deimos:~/tip/tools/testing/selftests/x86> ./mpx-mini-test_64
>> 
>> They need to be run as root. They try to access
>> 
>>  /sys/kernel/debug/tracing/set_ftrace_pid
>>  /sys/kernel/debug/tracing/trace
>> 
>> and that's root-only.
>
> Indeed - but note that they are crashing as root as well here:
>
>   root@deimos:/home/mingo/tip/tools/testing/selftests/x86# ./mpx-mini-test_64 
>   [...]
>   Aborted (core dumped)
>
> ... because I don't even have /sys/kernel/debug/tracing/set_ftrace_pid... :-/
>
> That's despite having ftrace enabled.
>
> After some digging I found out that 'set_ftrace_pid' is dependent on 
> CONFIG_FUNCTION_TRACING=y (not just CONFIG_TRACING=y), which I didn't
> have enabled on this kernel.
>
> After enabling it I got the 64-bit testcase to work:
>
>   #
>   ...
>   ./mpx-mini-test_64 completed successfully
>
> But the 32-bit testcase is erroring out:
>
>   root@deimos:/home/mingo/tip/tools/testing/selftests/x86# ./mpx-mini-test_32 
>   XSAVE is supported by HW & OS
>   XSAVE processor supported state mask: 0x21f
>   XSAVE OS supported state mask: 0x21f
>BNDREGS: size: 64 user: 1 supervisor: 0 aligned: 0
> BNDCSR: size: 64 user: 1 supervisor: 0 aligned: 0
>   executing unmaptest
>   mpx dig (  1) complete, SUCCESS (   0 /0)
>   #BR status == 2, missing bounds table,kernel should have handled!!
>
> v4.17-rc2 based kernel, so it should have the latest MPX code.

If you remove this which tree will this be going into?

I have some refactoring and cleanups of the x86 siginfo code I am
hoping to get in later this development cycle and part of that was
refactoring the mpx code so it works more like everything else.

If the codes away I can just that part of the patchset.

Eric

Re: [PATCH 2/2] hwmon: (k10temp) Use API function to access System Management Network

[Gabriel C]

2018-04-29 3:54 GMT+02:00 Guenter Roeck :
> The SMN (System Management Network) on Family 17h AMD CPUs is also accessed
> from other drivers, specifically EDAC. Accessing it directly is racy.
> On top of that, accessing the SMN through root bridge 00:00 is wrong on
> multi-die CPUs and may result in reading the temperature from the wrong
> die. Use available API functions to fix the problem.
>
> For this to work, also change the Raven Ridge PCI device ID to point to
> Data Fabric Function 3, since this ID is used by the API functions to
> find the CPU node.
>
> Signed-off-by: Guenter Roeck 
> ---
>  drivers/hwmon/k10temp.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index b06bb1f90853..00e785afae0d 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  MODULE_DESCRIPTION("AMD Family 10h+ CPU core temperature monitor");
> @@ -40,8 +41,8 @@ static DEFINE_MUTEX(nb_smu_ind_mutex);
>  #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
>  #endif
>
> -#ifndef PCI_DEVICE_ID_AMD_17H_RR_NB
> -#define PCI_DEVICE_ID_AMD_17H_RR_NB0x15d0
> +#ifndef PCI_DEVICE_ID_AMD_17H_RR_DF_F3
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x14eb
>  #endif
>
>  /* CPUID function 0x8001, ebx */
> @@ -136,8 +137,8 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 
> *regval)
>
>  static void read_tempreg_nb_f17(struct pci_dev *pdev, u32 *regval)
>  {
> -   amd_nb_index_read(pdev, PCI_DEVFN(0, 0), 0x60,
> - F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
> +   amd_smn_read(amd_pci_dev_to_node_id(pdev),
> +F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
>  }
>
>  static ssize_t temp1_input_show(struct device *dev,
> @@ -323,7 +324,7 @@ static const struct pci_device_id k10temp_id_table[] = {
> { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
> { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
> { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
> -   { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_RR_NB) },
> +   { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
> {}
>  };
>  MODULE_DEVICE_TABLE(pci, k10temp_id_table);
> --
> 2.7.4
>

Works fine for me on top stable and on top v4.17-rc2-398-gcdface520934

Tested-by: Gabriel Craciunescu 

Regards

Re: [PATCH 2/2] hwmon: (k10temp) Use API function to access System Management Network

[Gabriel C]

2018-04-29 3:54 GMT+02:00 Guenter Roeck :
> The SMN (System Management Network) on Family 17h AMD CPUs is also accessed
> from other drivers, specifically EDAC. Accessing it directly is racy.
> On top of that, accessing the SMN through root bridge 00:00 is wrong on
> multi-die CPUs and may result in reading the temperature from the wrong
> die. Use available API functions to fix the problem.
>
> For this to work, also change the Raven Ridge PCI device ID to point to
> Data Fabric Function 3, since this ID is used by the API functions to
> find the CPU node.
>
> Signed-off-by: Guenter Roeck 
> ---
>  drivers/hwmon/k10temp.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index b06bb1f90853..00e785afae0d 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  MODULE_DESCRIPTION("AMD Family 10h+ CPU core temperature monitor");
> @@ -40,8 +41,8 @@ static DEFINE_MUTEX(nb_smu_ind_mutex);
>  #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
>  #endif
>
> -#ifndef PCI_DEVICE_ID_AMD_17H_RR_NB
> -#define PCI_DEVICE_ID_AMD_17H_RR_NB0x15d0
> +#ifndef PCI_DEVICE_ID_AMD_17H_RR_DF_F3
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x14eb
>  #endif
>
>  /* CPUID function 0x8001, ebx */
> @@ -136,8 +137,8 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 
> *regval)
>
>  static void read_tempreg_nb_f17(struct pci_dev *pdev, u32 *regval)
>  {
> -   amd_nb_index_read(pdev, PCI_DEVFN(0, 0), 0x60,
> - F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
> +   amd_smn_read(amd_pci_dev_to_node_id(pdev),
> +F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
>  }
>
>  static ssize_t temp1_input_show(struct device *dev,
> @@ -323,7 +324,7 @@ static const struct pci_device_id k10temp_id_table[] = {
> { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
> { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
> { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
> -   { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_RR_NB) },
> +   { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
> {}
>  };
>  MODULE_DEVICE_TABLE(pci, k10temp_id_table);
> --
> 2.7.4
>

Works fine for me on top stable and on top v4.17-rc2-398-gcdface520934

Tested-by: Gabriel Craciunescu 

Regards

Re: [PATCH net-next 0/2 v4] netns: uevent filtering

[Eric W. Biederman]

Christian Brauner  writes:

> Hey everyone,
>
> This is the new approach to uevent filtering as discussed (see the
> threads in [1], [2], and [3]). It only contains *non-functional
> changes*.
>
> This series deals with with fixing up uevent filtering logic:
> - uevent filtering logic is simplified
> - locking time on uevent_sock_list is minimized
> - tagged and untagged kobjects are handled in separate codepaths
> - permissions for userspace are fixed for network device uevents in
>   network namespaces owned by non-initial user namespaces
>   Udev is now able to see those events correctly which it wasn't before.
>   For example, moving a physical device into a network namespace not
>   owned by the initial user namespaces before gave:
>
>   root@xen1:~# udevadm --debug monitor -k
>   calling: monitor
>   monitor will print the received events for:
>   KERNEL - the kernel uevent
>
>   sender uid=65534, message ignored
>   sender uid=65534, message ignored
>   sender uid=65534, message ignored
>   sender uid=65534, message ignored
>   sender uid=65534, message ignored
>
>   and now after the discussion and solution in [3] correctly gives:
>
>   root@xen1:~# udevadm --debug monitor -k
>   calling: monitor
>   monitor will print the received events for:
>   KERNEL - the kernel uevent
>
>   KERNEL[625.301042] add  
> /devices/pci:00/:00:02.0/:01:00.1/net/enp1s0f1 (net)
>   KERNEL[625.301109] move 
> /devices/pci:00/:00:02.0/:01:00.1/net/enp1s0f1 (net)
>   KERNEL[625.301138] move 
> /devices/pci:00/:00:02.0/:01:00.1/net/eth1 (net)
>   KERNEL[655.333272] remove 
> /devices/pci:00/:00:02.0/:01:00.1/net/eth1 (net)
>
> Thanks!
> Christian
>
> [1]: https://lkml.org/lkml/2018/4/4/739
> [2]: https://lkml.org/lkml/2018/4/26/767
> [3]: https://lkml.org/lkml/2018/4/26/738

Again ovearall ack.  One last nit that might be worth addressing.

Acked-by: "Eric W. Biederman" 

Eric

Re: [PATCH net-next 0/2 v4] netns: uevent filtering

[Eric W. Biederman]

Christian Brauner  writes:

> Hey everyone,
>
> This is the new approach to uevent filtering as discussed (see the
> threads in [1], [2], and [3]). It only contains *non-functional
> changes*.
>
> This series deals with with fixing up uevent filtering logic:
> - uevent filtering logic is simplified
> - locking time on uevent_sock_list is minimized
> - tagged and untagged kobjects are handled in separate codepaths
> - permissions for userspace are fixed for network device uevents in
>   network namespaces owned by non-initial user namespaces
>   Udev is now able to see those events correctly which it wasn't before.
>   For example, moving a physical device into a network namespace not
>   owned by the initial user namespaces before gave:
>
>   root@xen1:~# udevadm --debug monitor -k
>   calling: monitor
>   monitor will print the received events for:
>   KERNEL - the kernel uevent
>
>   sender uid=65534, message ignored
>   sender uid=65534, message ignored
>   sender uid=65534, message ignored
>   sender uid=65534, message ignored
>   sender uid=65534, message ignored
>
>   and now after the discussion and solution in [3] correctly gives:
>
>   root@xen1:~# udevadm --debug monitor -k
>   calling: monitor
>   monitor will print the received events for:
>   KERNEL - the kernel uevent
>
>   KERNEL[625.301042] add  
> /devices/pci:00/:00:02.0/:01:00.1/net/enp1s0f1 (net)
>   KERNEL[625.301109] move 
> /devices/pci:00/:00:02.0/:01:00.1/net/enp1s0f1 (net)
>   KERNEL[625.301138] move 
> /devices/pci:00/:00:02.0/:01:00.1/net/eth1 (net)
>   KERNEL[655.333272] remove 
> /devices/pci:00/:00:02.0/:01:00.1/net/eth1 (net)
>
> Thanks!
> Christian
>
> [1]: https://lkml.org/lkml/2018/4/4/739
> [2]: https://lkml.org/lkml/2018/4/26/767
> [3]: https://lkml.org/lkml/2018/4/26/738

Again ovearall ack.  One last nit that might be worth addressing.

Acked-by: "Eric W. Biederman" 

Eric

Re: [PATCH net-next 2/2 v4] netns: restrict uevents

[Eric W. Biederman]

> + /* fix credentials */
> + if (owning_user_ns != _user_ns) {
> + struct netlink_skb_parms *parms = _CB(skb);
> + kuid_t root_uid;
> + kgid_t root_gid;
> +
> + /* fix uid */
> + root_uid = make_kuid(owning_user_ns, 0);
> + if (!uid_valid(root_uid))
> + root_uid = GLOBAL_ROOT_UID;
> + parms->creds.uid = root_uid;
> +
> + /* fix gid */
> + root_gid = make_kgid(owning_user_ns, 0);
> + if (!gid_valid(root_gid))
> + root_gid = GLOBAL_ROOT_GID;
> + parms->creds.gid = root_gid;

One last nit:

You can only make the assignment if the uid is valid.
Leaving it GLBOAL_ROOT_UID if the composed uid is invalid.
AKA

/* fix uid */
root_uid = make_kuid(owning_user_ns, 0);
if (uid_valid(root_uid))
parms->creds.uid = root_uid;

/* fix gid */
root_gid = make_kgid(owning_user_ns, 0);
if (gid_valid(root_gid))
params->creds.gid = root_gid;


One line shorter and I think a little clearer.  I suspect
it even results in better code.

Eric

Re: [PATCH net-next 2/2 v4] netns: restrict uevents

[Eric W. Biederman]

> + /* fix credentials */
> + if (owning_user_ns != _user_ns) {
> + struct netlink_skb_parms *parms = _CB(skb);
> + kuid_t root_uid;
> + kgid_t root_gid;
> +
> + /* fix uid */
> + root_uid = make_kuid(owning_user_ns, 0);
> + if (!uid_valid(root_uid))
> + root_uid = GLOBAL_ROOT_UID;
> + parms->creds.uid = root_uid;
> +
> + /* fix gid */
> + root_gid = make_kgid(owning_user_ns, 0);
> + if (!gid_valid(root_gid))
> + root_gid = GLOBAL_ROOT_GID;
> + parms->creds.gid = root_gid;

One last nit:

You can only make the assignment if the uid is valid.
Leaving it GLBOAL_ROOT_UID if the composed uid is invalid.
AKA

/* fix uid */
root_uid = make_kuid(owning_user_ns, 0);
if (uid_valid(root_uid))
parms->creds.uid = root_uid;

/* fix gid */
root_gid = make_kgid(owning_user_ns, 0);
if (gid_valid(root_gid))
params->creds.gid = root_gid;


One line shorter and I think a little clearer.  I suspect
it even results in better code.

Eric

Re: [PATCH 1/2] x86/amd_nb: Add support for Raven Ridge CPUs

[Gabriel C]

2018-04-29 3:54 GMT+02:00 Guenter Roeck :
> Add Raven Ridge root bridge and data fabric PCI IDs.
> This is required for amd_pci_dev_to_node_id() and amd_smn_read().
>
> Signed-off-by: Guenter Roeck 
> ---
> This patch is a prerequisite for the second patch in the series.
> I'll be happy to apply both patches through hwmon if that is acceptable
> (and Cc: stable for 4.16+). If not, I'll be happy to wait for this patch
> to be available upstream.
>
> Since that there is no public documentation available for Raven Ridge,
> PCI IDs are derived from output of lspci.
>
>  arch/x86/kernel/amd_nb.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index c88e0b127810..bd33613ecb7c 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -14,8 +14,11 @@
>  #include 
>
>  #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450
> +#define PCI_DEVICE_ID_AMD_17H_RR_ROOT  0x15d0
>  #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
>  #define PCI_DEVICE_ID_AMD_17H_DF_F40x1464
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x15eb
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F4 0x15ec
>
>  /* Protect the PCI config register pairs used for SMN and DF indirect 
> access. */
>  static DEFINE_MUTEX(smn_mutex);
> @@ -24,6 +27,7 @@ static u32 *flush_words;
>
>  static const struct pci_device_id amd_root_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_ROOT) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_ROOT) },
> {}
>  };
>
> @@ -39,6 +43,7 @@ const struct pci_device_id amd_nb_misc_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
> {}
>  };
> @@ -51,6 +56,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
> {}
>  };
> --
> 2.7.4
>

Works fine for me on top stable and on top v4.17-rc2-398-gcdface520934

Tested-by: Gabriel Craciunescu 


Regards

Re: [PATCH 1/2] x86/amd_nb: Add support for Raven Ridge CPUs

[Gabriel C]

2018-04-29 3:54 GMT+02:00 Guenter Roeck :
> Add Raven Ridge root bridge and data fabric PCI IDs.
> This is required for amd_pci_dev_to_node_id() and amd_smn_read().
>
> Signed-off-by: Guenter Roeck 
> ---
> This patch is a prerequisite for the second patch in the series.
> I'll be happy to apply both patches through hwmon if that is acceptable
> (and Cc: stable for 4.16+). If not, I'll be happy to wait for this patch
> to be available upstream.
>
> Since that there is no public documentation available for Raven Ridge,
> PCI IDs are derived from output of lspci.
>
>  arch/x86/kernel/amd_nb.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index c88e0b127810..bd33613ecb7c 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -14,8 +14,11 @@
>  #include 
>
>  #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450
> +#define PCI_DEVICE_ID_AMD_17H_RR_ROOT  0x15d0
>  #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
>  #define PCI_DEVICE_ID_AMD_17H_DF_F40x1464
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x15eb
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F4 0x15ec
>
>  /* Protect the PCI config register pairs used for SMN and DF indirect 
> access. */
>  static DEFINE_MUTEX(smn_mutex);
> @@ -24,6 +27,7 @@ static u32 *flush_words;
>
>  static const struct pci_device_id amd_root_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_ROOT) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_ROOT) },
> {}
>  };
>
> @@ -39,6 +43,7 @@ const struct pci_device_id amd_nb_misc_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
> {}
>  };
> @@ -51,6 +56,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
> {}
>  };
> --
> 2.7.4
>

Works fine for me on top stable and on top v4.17-rc2-398-gcdface520934

Tested-by: Gabriel Craciunescu 


Regards

Re: [PATCH 1/2] x86/amd_nb: Add support for Raven Ridge CPUs

[Gabriel C]

2018-04-29 3:54 GMT+02:00 Guenter Roeck :
> Add Raven Ridge root bridge and data fabric PCI IDs.
> This is required for amd_pci_dev_to_node_id() and amd_smn_read().
>
> Signed-off-by: Guenter Roeck 
> ---
> This patch is a prerequisite for the second patch in the series.
> I'll be happy to apply both patches through hwmon if that is acceptable
> (and Cc: stable for 4.16+). If not, I'll be happy to wait for this patch
> to be available upstream.
>
> Since that there is no public documentation available for Raven Ridge,
> PCI IDs are derived from output of lspci.
>
>  arch/x86/kernel/amd_nb.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index c88e0b127810..bd33613ecb7c 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -14,8 +14,11 @@
>  #include 
>
>  #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450
> +#define PCI_DEVICE_ID_AMD_17H_RR_ROOT  0x15d0
>  #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
>  #define PCI_DEVICE_ID_AMD_17H_DF_F40x1464
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x15eb
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F4 0x15ec
>
>  /* Protect the PCI config register pairs used for SMN and DF indirect 
> access. */
>  static DEFINE_MUTEX(smn_mutex);
> @@ -24,6 +27,7 @@ static u32 *flush_words;
>
>  static const struct pci_device_id amd_root_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_ROOT) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_ROOT) },
> {}
>  };
>
> @@ -39,6 +43,7 @@ const struct pci_device_id amd_nb_misc_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
> {}
>  };
> @@ -51,6 +56,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
> {}
>  };
> --
> 2.7.4
>

Works fine for me on top stable and on top v4.17-rc2-398-gcdface520934

Tested-by: Gabriel Craciunescu 

Regards

Re: [PATCH 1/2] x86/amd_nb: Add support for Raven Ridge CPUs

[Gabriel C]

2018-04-29 3:54 GMT+02:00 Guenter Roeck :
> Add Raven Ridge root bridge and data fabric PCI IDs.
> This is required for amd_pci_dev_to_node_id() and amd_smn_read().
>
> Signed-off-by: Guenter Roeck 
> ---
> This patch is a prerequisite for the second patch in the series.
> I'll be happy to apply both patches through hwmon if that is acceptable
> (and Cc: stable for 4.16+). If not, I'll be happy to wait for this patch
> to be available upstream.
>
> Since that there is no public documentation available for Raven Ridge,
> PCI IDs are derived from output of lspci.
>
>  arch/x86/kernel/amd_nb.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index c88e0b127810..bd33613ecb7c 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -14,8 +14,11 @@
>  #include 
>
>  #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450
> +#define PCI_DEVICE_ID_AMD_17H_RR_ROOT  0x15d0
>  #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
>  #define PCI_DEVICE_ID_AMD_17H_DF_F40x1464
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x15eb
> +#define PCI_DEVICE_ID_AMD_17H_RR_DF_F4 0x15ec
>
>  /* Protect the PCI config register pairs used for SMN and DF indirect 
> access. */
>  static DEFINE_MUTEX(smn_mutex);
> @@ -24,6 +27,7 @@ static u32 *flush_words;
>
>  static const struct pci_device_id amd_root_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_ROOT) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_ROOT) },
> {}
>  };
>
> @@ -39,6 +43,7 @@ const struct pci_device_id amd_nb_misc_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
> {}
>  };
> @@ -51,6 +56,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) },
> +   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F4) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
> {}
>  };
> --
> 2.7.4
>

Works fine for me on top stable and on top v4.17-rc2-398-gcdface520934

Tested-by: Gabriel Craciunescu 

Regards

Re: [PATCH 3/3] genalloc: selftest

[Randy Dunlap]

On 04/28/2018 07:45 PM, Igor Stoppa wrote:
> Introduce a set of macros for writing concise test cases for genalloc.
> 
> The test cases are meant to provide regression testing, when working on
> new functionality for genalloc.
> 
> Primarily they are meant to confirm that the various allocation strategy
> will continue to work as expected.
> 
> The execution of the self testing is controlled through a Kconfig option.
> 
> The testing takes place in the very early stages of main.c, to ensure
> that failures in genalloc are caught before they can cause unexplained
> erratic behavior in any of genalloc users.
> 
> Therefore, it would not be advisable to implement it as module.
> 
> Signed-off-by: Igor Stoppa 

Hi,

> ---
>  init/main.c |   2 +
>  lib/Kconfig |  15 ++
>  lib/Makefile|   1 +
>  lib/test_genalloc.c | 410 
> 
>  4 files changed, 428 insertions(+)
>  create mode 100644 lib/test_genalloc.c
> 
> diff --git a/init/main.c b/init/main.c
> index b795aa341a3a..b3b319d91b0e 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -91,6 +91,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -679,6 +680,7 @@ asmlinkage __visible void __init start_kernel(void)
>*/
>   mem_encrypt_init();
>  
> + test_genalloc();

Is there a stub for test_genalloc() when its config option is not enabled?
I don't see it.

>  #ifdef CONFIG_BLK_DEV_INITRD
>   if (initrd_start && !initrd_below_start_ok &&
>   page_to_pfn(virt_to_page((void *)initrd_start)) < min_low_pfn) {
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 09565d779324..2bf89af50728 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -303,6 +303,21 @@ config DECOMPRESS_LZ4
>  config GENERIC_ALLOCATOR
>   bool
>  

These TEST_ kconfig symbols should be in lib/Kconfig.debug, not lib/Kconfig.


> +config TEST_GENERIC_ALLOCATOR
> + bool "genalloc tester"
> + default n
> + select GENERIC_ALLOCATOR

This should depend on GENERIC_ALLOCATOR, not select it.

See TEST_PARMAN, TEST_BPF, TEST_FIRMWARE, TEST_SYSCTL, TEST_DEBUG_VIRTUAL
in lib/Kconfig.debug.


> + help
> +   Enable automated testing of the generic allocator.
> +   The testing is primarily for the tracking of allocated space.
> +
> +config TEST_GENERIC_ALLOCATOR_VERBOSE
> + bool "make the genalloc tester more verbose"
> + default n
> + select TEST_GENERIC_ALLOCATOR

depends on TEST_GENERIC_ALLOCATOR

> + help
> +   More information will be displayed during the self-testing.
> +
>  #
>  # reed solomon support is select'ed if needed
>  #

> diff --git a/lib/test_genalloc.c b/lib/test_genalloc.c
> new file mode 100644
> index ..ab9984861517
> --- /dev/null
> +++ b/lib/test_genalloc.c
> @@ -0,0 +1,410 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * test_genalloc.c
> + *
> + * (C) Copyright 2017 Huawei Technologies Co. Ltd.
> + * Author: Igor Stoppa 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +
> +/*
> + * In case of failure of any of these tests, memory corruption is almost
> + * guarranteed; allowing the boot to continue means risking to corrupt

  guaranteed;

> + * also any filesystem/block device accessed write mode.
> + * Therefore, BUG_ON() is used, when testing.
> + */


-- 
~Randy

Re: [PATCH 3/3] genalloc: selftest

[Randy Dunlap]

On 04/28/2018 07:45 PM, Igor Stoppa wrote:
> Introduce a set of macros for writing concise test cases for genalloc.
> 
> The test cases are meant to provide regression testing, when working on
> new functionality for genalloc.
> 
> Primarily they are meant to confirm that the various allocation strategy
> will continue to work as expected.
> 
> The execution of the self testing is controlled through a Kconfig option.
> 
> The testing takes place in the very early stages of main.c, to ensure
> that failures in genalloc are caught before they can cause unexplained
> erratic behavior in any of genalloc users.
> 
> Therefore, it would not be advisable to implement it as module.
> 
> Signed-off-by: Igor Stoppa 

Hi,

> ---
>  init/main.c |   2 +
>  lib/Kconfig |  15 ++
>  lib/Makefile|   1 +
>  lib/test_genalloc.c | 410 
> 
>  4 files changed, 428 insertions(+)
>  create mode 100644 lib/test_genalloc.c
> 
> diff --git a/init/main.c b/init/main.c
> index b795aa341a3a..b3b319d91b0e 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -91,6 +91,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -679,6 +680,7 @@ asmlinkage __visible void __init start_kernel(void)
>*/
>   mem_encrypt_init();
>  
> + test_genalloc();

Is there a stub for test_genalloc() when its config option is not enabled?
I don't see it.

>  #ifdef CONFIG_BLK_DEV_INITRD
>   if (initrd_start && !initrd_below_start_ok &&
>   page_to_pfn(virt_to_page((void *)initrd_start)) < min_low_pfn) {
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 09565d779324..2bf89af50728 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -303,6 +303,21 @@ config DECOMPRESS_LZ4
>  config GENERIC_ALLOCATOR
>   bool
>  

These TEST_ kconfig symbols should be in lib/Kconfig.debug, not lib/Kconfig.


> +config TEST_GENERIC_ALLOCATOR
> + bool "genalloc tester"
> + default n
> + select GENERIC_ALLOCATOR

This should depend on GENERIC_ALLOCATOR, not select it.

See TEST_PARMAN, TEST_BPF, TEST_FIRMWARE, TEST_SYSCTL, TEST_DEBUG_VIRTUAL
in lib/Kconfig.debug.


> + help
> +   Enable automated testing of the generic allocator.
> +   The testing is primarily for the tracking of allocated space.
> +
> +config TEST_GENERIC_ALLOCATOR_VERBOSE
> + bool "make the genalloc tester more verbose"
> + default n
> + select TEST_GENERIC_ALLOCATOR

depends on TEST_GENERIC_ALLOCATOR

> + help
> +   More information will be displayed during the self-testing.
> +
>  #
>  # reed solomon support is select'ed if needed
>  #

> diff --git a/lib/test_genalloc.c b/lib/test_genalloc.c
> new file mode 100644
> index ..ab9984861517
> --- /dev/null
> +++ b/lib/test_genalloc.c
> @@ -0,0 +1,410 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * test_genalloc.c
> + *
> + * (C) Copyright 2017 Huawei Technologies Co. Ltd.
> + * Author: Igor Stoppa 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +
> +/*
> + * In case of failure of any of these tests, memory corruption is almost
> + * guarranteed; allowing the boot to continue means risking to corrupt

  guaranteed;

> + * also any filesystem/block device accessed write mode.
> + * Therefore, BUG_ON() is used, when testing.
> + */


-- 
~Randy

Re: [llc_ui_release] BUG: unable to handle kernel NULL pointer dereference at 0000000000000004

[Linus Torvalds]

On Sat, Apr 28, 2018 at 7:12 PM Fengguang Wu  wrote:

> FYI this happens in mainline kernel 4.17.0-rc2.
> It looks like a new regression.

> It occurs in 5 out of 5 boots.

> [main] 375 sockets created based on info from socket cachefile.
> [main] Generating file descriptors
> [main] Added 83 filenames from /dev
> udevd[507]: failed to execute '/sbin/modprobe' '/sbin/modprobe -bv
platform:regulatory': No such file or directory
> [  372.057947] caif:caif_disconnect_client(): nothing to disconnect
> [  372.082415] BUG: unable to handle kernel NULL pointer dereference at
0004

I think this is fixed by commit 3a04ce7130a7 ("llc: fix NULL pointer deref
for SOCK_ZAPPED")

Liunus

Re: [llc_ui_release] BUG: unable to handle kernel NULL pointer dereference at 0000000000000004

[Linus Torvalds]

On Sat, Apr 28, 2018 at 7:12 PM Fengguang Wu  wrote:

> FYI this happens in mainline kernel 4.17.0-rc2.
> It looks like a new regression.

> It occurs in 5 out of 5 boots.

> [main] 375 sockets created based on info from socket cachefile.
> [main] Generating file descriptors
> [main] Added 83 filenames from /dev
> udevd[507]: failed to execute '/sbin/modprobe' '/sbin/modprobe -bv
platform:regulatory': No such file or directory
> [  372.057947] caif:caif_disconnect_client(): nothing to disconnect
> [  372.082415] BUG: unable to handle kernel NULL pointer dereference at
0004

I think this is fixed by commit 3a04ce7130a7 ("llc: fix NULL pointer deref
for SOCK_ZAPPED")

Liunus

drivers/gpu/drm/bridge/sil-sii8620.c:2405: undefined reference to `extcon_unregister_notifier'

[kbuild test robot]

Hi Maciej,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   19b9ad67310ed2f685062a00aec602bec33835f0
commit: 688838442147d9dd94c2ef7c2c31a35cf150c5fa drm/bridge/sii8620: use 
micro-USB cable detection logic to detect MHL
date:   7 weeks ago
config: x86_64-randconfig-s4-04290805 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
git checkout 688838442147d9dd94c2ef7c2c31a35cf150c5fa
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/bridge/sil-sii8620.o: In function `sii8620_remove':
>> drivers/gpu/drm/bridge/sil-sii8620.c:2405: undefined reference to 
>> `extcon_unregister_notifier'
   drivers/gpu/drm/bridge/sil-sii8620.o: In function `sii8620_extcon_init':
>> drivers/gpu/drm/bridge/sil-sii8620.c:2229: undefined reference to 
>> `extcon_find_edev_by_node'
>> drivers/gpu/drm/bridge/sil-sii8620.c:2241: undefined reference to 
>> `extcon_register_notifier'
   drivers/gpu/drm/bridge/sil-sii8620.o: In function `sii8620_extcon_work':
>> drivers/gpu/drm/bridge/sil-sii8620.c:2189: undefined reference to 
>> `extcon_get_state'

vim +2405 drivers/gpu/drm/bridge/sil-sii8620.c

  2212  
  2213  static int sii8620_extcon_init(struct sii8620 *ctx)
  2214  {
  2215  struct extcon_dev *edev;
  2216  struct device_node *musb, *muic;
  2217  int ret;
  2218  
  2219  /* get micro-USB connector node */
  2220  musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
  2221  /* next get micro-USB Interface Controller node */
    muic = of_get_next_parent(musb);
  2223  
  2224  if (!muic) {
  2225  dev_info(ctx->dev, "no extcon found, switching to 
'always on' mode\n");
  2226  return 0;
  2227  }
  2228  
> 2229  edev = extcon_find_edev_by_node(muic);
  2230  of_node_put(muic);
  2231  if (IS_ERR(edev)) {
  2232  if (PTR_ERR(edev) == -EPROBE_DEFER)
  2233  return -EPROBE_DEFER;
  2234  dev_err(ctx->dev, "Invalid or missing extcon\n");
  2235  return PTR_ERR(edev);
  2236  }
  2237  
  2238  ctx->extcon = edev;
  2239  ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
  2240  INIT_WORK(>extcon_wq, sii8620_extcon_work);
> 2241  ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, 
> >extcon_nb);
  2242  if (ret) {
  2243  dev_err(ctx->dev, "failed to register notifier for 
MHL\n");
  2244  return ret;
  2245  }
  2246  
  2247  return 0;
  2248  }
  2249  
  2250  static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge 
*bridge)
  2251  {
  2252  return container_of(bridge, struct sii8620, bridge);
  2253  }
  2254  
  2255  static int sii8620_attach(struct drm_bridge *bridge)
  2256  {
  2257  struct sii8620 *ctx = bridge_to_sii8620(bridge);
  2258  
  2259  sii8620_init_rcp_input_dev(ctx);
  2260  
  2261  return sii8620_clear_error(ctx);
  2262  }
  2263  
  2264  static void sii8620_detach(struct drm_bridge *bridge)
  2265  {
  2266  struct sii8620 *ctx = bridge_to_sii8620(bridge);
  2267  
  2268  rc_unregister_device(ctx->rc_dev);
  2269  }
  2270  
  2271  static enum drm_mode_status sii8620_mode_valid(struct drm_bridge 
*bridge,
  2272   const struct drm_display_mode 
*mode)
  2273  {
  2274  struct sii8620 *ctx = bridge_to_sii8620(bridge);
  2275  bool can_pack = ctx->devcap[MHL_DCAP_VID_LINK_MODE] &
  2276  MHL_DCAP_VID_LINK_PPIXEL;
  2277  unsigned int max_pclk = sii8620_is_mhl3(ctx) ? MHL3_MAX_LCLK :
  2278 MHL1_MAX_LCLK;
  2279  max_pclk /= can_pack ? 2 : 3;
  2280  
  2281  return (mode->clock > max_pclk) ? MODE_CLOCK_HIGH : MODE_OK;
  2282  }
  2283  
  2284  static bool sii8620_mode_fixup(struct drm_bridge *bridge,
  2285 const struct drm_display_mode *mode,
  2286 struct drm_display_mode *adjusted_mode)
  2287  {
  2288  struct sii8620 *ctx = bridge_to_sii8620(bridge);
  2289  int max_lclk;
  2290  bool ret = true;
  2291  
  2292  mutex_lock(>lock);
  2293  
  2294  max_lclk = sii8620_is_mhl3(ctx) ? MHL3_MAX_LCLK : MHL1_MAX_LCLK;
  2295  if (max_lclk > 3 * adjusted_mode->clock) {
  2296  ctx->use_packed_pixel = 0;
  2297  goto end;
  2298  }
  2299  if ((ctx->devcap[MHL_DCAP_VID_LINK_MODE] & 
MHL_DCAP_VID_LINK_PPIXEL) &&
  2300  max_lclk > 2 * adjusted_mode->clock) {
  2301  ctx->use_packed_pixel = 1;
  2302   

drivers/gpu/drm/bridge/sil-sii8620.c:2405: undefined reference to `extcon_unregister_notifier'

[kbuild test robot]

Hi Maciej,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   19b9ad67310ed2f685062a00aec602bec33835f0
commit: 688838442147d9dd94c2ef7c2c31a35cf150c5fa drm/bridge/sii8620: use 
micro-USB cable detection logic to detect MHL
date:   7 weeks ago
config: x86_64-randconfig-s4-04290805 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
git checkout 688838442147d9dd94c2ef7c2c31a35cf150c5fa
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/bridge/sil-sii8620.o: In function `sii8620_remove':
>> drivers/gpu/drm/bridge/sil-sii8620.c:2405: undefined reference to 
>> `extcon_unregister_notifier'
   drivers/gpu/drm/bridge/sil-sii8620.o: In function `sii8620_extcon_init':
>> drivers/gpu/drm/bridge/sil-sii8620.c:2229: undefined reference to 
>> `extcon_find_edev_by_node'
>> drivers/gpu/drm/bridge/sil-sii8620.c:2241: undefined reference to 
>> `extcon_register_notifier'
   drivers/gpu/drm/bridge/sil-sii8620.o: In function `sii8620_extcon_work':
>> drivers/gpu/drm/bridge/sil-sii8620.c:2189: undefined reference to 
>> `extcon_get_state'

vim +2405 drivers/gpu/drm/bridge/sil-sii8620.c

  2212  
  2213  static int sii8620_extcon_init(struct sii8620 *ctx)
  2214  {
  2215  struct extcon_dev *edev;
  2216  struct device_node *musb, *muic;
  2217  int ret;
  2218  
  2219  /* get micro-USB connector node */
  2220  musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
  2221  /* next get micro-USB Interface Controller node */
    muic = of_get_next_parent(musb);
  2223  
  2224  if (!muic) {
  2225  dev_info(ctx->dev, "no extcon found, switching to 
'always on' mode\n");
  2226  return 0;
  2227  }
  2228  
> 2229  edev = extcon_find_edev_by_node(muic);
  2230  of_node_put(muic);
  2231  if (IS_ERR(edev)) {
  2232  if (PTR_ERR(edev) == -EPROBE_DEFER)
  2233  return -EPROBE_DEFER;
  2234  dev_err(ctx->dev, "Invalid or missing extcon\n");
  2235  return PTR_ERR(edev);
  2236  }
  2237  
  2238  ctx->extcon = edev;
  2239  ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
  2240  INIT_WORK(>extcon_wq, sii8620_extcon_work);
> 2241  ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, 
> >extcon_nb);
  2242  if (ret) {
  2243  dev_err(ctx->dev, "failed to register notifier for 
MHL\n");
  2244  return ret;
  2245  }
  2246  
  2247  return 0;
  2248  }
  2249  
  2250  static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge 
*bridge)
  2251  {
  2252  return container_of(bridge, struct sii8620, bridge);
  2253  }
  2254  
  2255  static int sii8620_attach(struct drm_bridge *bridge)
  2256  {
  2257  struct sii8620 *ctx = bridge_to_sii8620(bridge);
  2258  
  2259  sii8620_init_rcp_input_dev(ctx);
  2260  
  2261  return sii8620_clear_error(ctx);
  2262  }
  2263  
  2264  static void sii8620_detach(struct drm_bridge *bridge)
  2265  {
  2266  struct sii8620 *ctx = bridge_to_sii8620(bridge);
  2267  
  2268  rc_unregister_device(ctx->rc_dev);
  2269  }
  2270  
  2271  static enum drm_mode_status sii8620_mode_valid(struct drm_bridge 
*bridge,
  2272   const struct drm_display_mode 
*mode)
  2273  {
  2274  struct sii8620 *ctx = bridge_to_sii8620(bridge);
  2275  bool can_pack = ctx->devcap[MHL_DCAP_VID_LINK_MODE] &
  2276  MHL_DCAP_VID_LINK_PPIXEL;
  2277  unsigned int max_pclk = sii8620_is_mhl3(ctx) ? MHL3_MAX_LCLK :
  2278 MHL1_MAX_LCLK;
  2279  max_pclk /= can_pack ? 2 : 3;
  2280  
  2281  return (mode->clock > max_pclk) ? MODE_CLOCK_HIGH : MODE_OK;
  2282  }
  2283  
  2284  static bool sii8620_mode_fixup(struct drm_bridge *bridge,
  2285 const struct drm_display_mode *mode,
  2286 struct drm_display_mode *adjusted_mode)
  2287  {
  2288  struct sii8620 *ctx = bridge_to_sii8620(bridge);
  2289  int max_lclk;
  2290  bool ret = true;
  2291  
  2292  mutex_lock(>lock);
  2293  
  2294  max_lclk = sii8620_is_mhl3(ctx) ? MHL3_MAX_LCLK : MHL1_MAX_LCLK;
  2295  if (max_lclk > 3 * adjusted_mode->clock) {
  2296  ctx->use_packed_pixel = 0;
  2297  goto end;
  2298  }
  2299  if ((ctx->devcap[MHL_DCAP_VID_LINK_MODE] & 
MHL_DCAP_VID_LINK_PPIXEL) &&
  2300  max_lclk > 2 * adjusted_mode->clock) {
  2301  ctx->use_packed_pixel = 1;
  2302   

Re: [PATCH 0/3] linux-next: mm: hardening: Track genalloc allocations

[Matthew Wilcox]

On Sun, Apr 29, 2018 at 06:45:39AM +0400, Igor Stoppa wrote:
> This patchset was created as part of an older version of pmalloc, however
> it has value per-se, as it hardens the memory management for the generic
> allocator genalloc.
> 
> Genalloc does not currently track the size of the allocations it hands out.
> 
> Either by mistake, or due to an attack, it is possible that more memory
> than what was initially allocated is freed, leaving behind dangling
> pointers, ready for an use-after-free attack.

This is a good point.  It is worth hardening genalloc.
But I still don't like the cost of the bitmap.  I've been
reading about allocators and I found Bonwick's paper from 2001:
https://www.usenix.org/legacy/event/usenix01/full_papers/bonwick/bonwick.pdf
Section 4 describes the vmem allocator which would seem to have superior
performance and lower memory overhead than the current genalloc allocator,
never mind the hardened allocator.

Maybe there's been an advnace in resource allocator technology since
then that someone more familiar with CS research can point out.

Re: [PATCH 0/3] linux-next: mm: hardening: Track genalloc allocations

[Matthew Wilcox]

On Sun, Apr 29, 2018 at 06:45:39AM +0400, Igor Stoppa wrote:
> This patchset was created as part of an older version of pmalloc, however
> it has value per-se, as it hardens the memory management for the generic
> allocator genalloc.
> 
> Genalloc does not currently track the size of the allocations it hands out.
> 
> Either by mistake, or due to an attack, it is possible that more memory
> than what was initially allocated is freed, leaving behind dangling
> pointers, ready for an use-after-free attack.

This is a good point.  It is worth hardening genalloc.
But I still don't like the cost of the bitmap.  I've been
reading about allocators and I found Bonwick's paper from 2001:
https://www.usenix.org/legacy/event/usenix01/full_papers/bonwick/bonwick.pdf
Section 4 describes the vmem allocator which would seem to have superior
performance and lower memory overhead than the current genalloc allocator,
never mind the hardened allocator.

Maybe there's been an advnace in resource allocator technology since
then that someone more familiar with CS research can point out.

Re: [crng_reseed] WARNING: inconsistent lock state

[Linus Torvalds]

On Sat, Apr 28, 2018 at 7:26 PM Fengguang Wu  wrote:

> FYI this happens in mainline kernel 4.17.0-rc2.
> It looks like a new regression.

> It occurs in 3 out of 3 boots.

> There is another "[  294.642506] BUG: sleeping function called from
> invalid context at mm/slab.h:421" at the bottom of this long dmesg:

This should be fixed by commit 6c1e851c4edc ("random: fix possible sleeping
allocation from irq context").

 Linus

Re: [crng_reseed] WARNING: inconsistent lock state

[Linus Torvalds]

On Sat, Apr 28, 2018 at 7:26 PM Fengguang Wu  wrote:

> FYI this happens in mainline kernel 4.17.0-rc2.
> It looks like a new regression.

> It occurs in 3 out of 3 boots.

> There is another "[  294.642506] BUG: sleeping function called from
> invalid context at mm/slab.h:421" at the bottom of this long dmesg:

This should be fixed by commit 6c1e851c4edc ("random: fix possible sleeping
allocation from irq context").

 Linus

[GIT PULL] ext4 fixes for 4.17-rc3

[Theodore Y. Ts'o]

The following changes since commit e40ff213898502d299351cc2fe1e350cd186f0d3:

  ext4: force revalidation of directory pointer after seekdir(2) (2018-04-01 
23:21:03 -0400)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git 
tags/for_linus_stable

for you to fetch changes up to 7ef79ad52136712172eb0525bf0b462516bf2f93:

  ext4: add MODULE_SOFTDEP to ensure crc32c is included in the initramfs 
(2018-04-26 00:44:46 -0400)


Fix misc. bugs and a regression for ext4.


Eric Biggers (1):
  ext4: prevent right-shifting extents beyond EXT_MAX_BLOCKS

Lukas Czerner (1):
  ext4: fix bitmap position validation

Theodore Ts'o (2):
  ext4: set h_journal if there is a failure starting a reserved handle
  ext4: add MODULE_SOFTDEP to ensure crc32c is included in the initramfs

 fs/ext4/balloc.c  |  9 +
 fs/ext4/extents.c | 16 +++-
 fs/ext4/super.c   |  1 +
 fs/jbd2/transaction.c |  1 +
 4 files changed, 18 insertions(+), 9 deletions(-)

[GIT PULL] ext4 fixes for 4.17-rc3

[Theodore Y. Ts'o]

The following changes since commit e40ff213898502d299351cc2fe1e350cd186f0d3:

  ext4: force revalidation of directory pointer after seekdir(2) (2018-04-01 
23:21:03 -0400)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git 
tags/for_linus_stable

for you to fetch changes up to 7ef79ad52136712172eb0525bf0b462516bf2f93:

  ext4: add MODULE_SOFTDEP to ensure crc32c is included in the initramfs 
(2018-04-26 00:44:46 -0400)


Fix misc. bugs and a regression for ext4.


Eric Biggers (1):
  ext4: prevent right-shifting extents beyond EXT_MAX_BLOCKS

Lukas Czerner (1):
  ext4: fix bitmap position validation

Theodore Ts'o (2):
  ext4: set h_journal if there is a failure starting a reserved handle
  ext4: add MODULE_SOFTDEP to ensure crc32c is included in the initramfs

 fs/ext4/balloc.c  |  9 +
 fs/ext4/extents.c | 16 +++-
 fs/ext4/super.c   |  1 +
 fs/jbd2/transaction.c |  1 +
 4 files changed, 18 insertions(+), 9 deletions(-)

[PATCH 3/3] genalloc: selftest

[Igor Stoppa]

Introduce a set of macros for writing concise test cases for genalloc.

The test cases are meant to provide regression testing, when working on
new functionality for genalloc.

Primarily they are meant to confirm that the various allocation strategy
will continue to work as expected.

The execution of the self testing is controlled through a Kconfig option.

The testing takes place in the very early stages of main.c, to ensure
that failures in genalloc are caught before they can cause unexplained
erratic behavior in any of genalloc users.

Therefore, it would not be advisable to implement it as module.

Signed-off-by: Igor Stoppa 
---
 init/main.c |   2 +
 lib/Kconfig |  15 ++
 lib/Makefile|   1 +
 lib/test_genalloc.c | 410 
 4 files changed, 428 insertions(+)
 create mode 100644 lib/test_genalloc.c

diff --git a/init/main.c b/init/main.c
index b795aa341a3a..b3b319d91b0e 100644
--- a/init/main.c
+++ b/init/main.c
@@ -91,6 +91,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -679,6 +680,7 @@ asmlinkage __visible void __init start_kernel(void)
 */
mem_encrypt_init();
 
+   test_genalloc();
 #ifdef CONFIG_BLK_DEV_INITRD
if (initrd_start && !initrd_below_start_ok &&
page_to_pfn(virt_to_page((void *)initrd_start)) < min_low_pfn) {
diff --git a/lib/Kconfig b/lib/Kconfig
index 09565d779324..2bf89af50728 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -303,6 +303,21 @@ config DECOMPRESS_LZ4
 config GENERIC_ALLOCATOR
bool
 
+config TEST_GENERIC_ALLOCATOR
+   bool "genalloc tester"
+   default n
+   select GENERIC_ALLOCATOR
+   help
+ Enable automated testing of the generic allocator.
+ The testing is primarily for the tracking of allocated space.
+
+config TEST_GENERIC_ALLOCATOR_VERBOSE
+   bool "make the genalloc tester more verbose"
+   default n
+   select TEST_GENERIC_ALLOCATOR
+   help
+ More information will be displayed during the self-testing.
+
 #
 # reed solomon support is select'ed if needed
 #
diff --git a/lib/Makefile b/lib/Makefile
index 384713ff70d3..2c66346ab246 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -114,6 +114,7 @@ obj-$(CONFIG_LIBCRC32C) += libcrc32c.o
 obj-$(CONFIG_CRC8) += crc8.o
 obj-$(CONFIG_XXHASH)   += xxhash.o
 obj-$(CONFIG_GENERIC_ALLOCATOR) += genalloc.o
+obj-$(CONFIG_TEST_GENERIC_ALLOCATOR) += test_genalloc.o
 
 obj-$(CONFIG_842_COMPRESS) += 842/
 obj-$(CONFIG_842_DECOMPRESS) += 842/
diff --git a/lib/test_genalloc.c b/lib/test_genalloc.c
new file mode 100644
index ..ab9984861517
--- /dev/null
+++ b/lib/test_genalloc.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * test_genalloc.c
+ *
+ * (C) Copyright 2017 Huawei Technologies Co. Ltd.
+ * Author: Igor Stoppa 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+
+/*
+ * In case of failure of any of these tests, memory corruption is almost
+ * guarranteed; allowing the boot to continue means risking to corrupt
+ * also any filesystem/block device accessed write mode.
+ * Therefore, BUG_ON() is used, when testing.
+ */
+
+
+/*
+ * Keep the bitmap small, while including case of cross-ulong mapping.
+ * For simplicity, the test cases use only 1 chunk of memory.
+ */
+#define BITMAP_SIZE_C 16
+#define ALLOC_ORDER 0
+
+#define ULONG_SIZE (sizeof(unsigned long))
+#define BITMAP_SIZE_UL (BITMAP_SIZE_C / ULONG_SIZE)
+#define MIN_ALLOC_SIZE (1 << ALLOC_ORDER)
+#define ENTRIES (BITMAP_SIZE_C * 8)
+#define CHUNK_SIZE  (MIN_ALLOC_SIZE * ENTRIES)
+
+#ifndef CONFIG_TEST_GENERIC_ALLOCATOR_VERBOSE
+
+static inline void print_first_chunk_bitmap(struct gen_pool *pool) {}
+
+#else
+
+static void print_first_chunk_bitmap(struct gen_pool *pool)
+{
+   struct gen_pool_chunk *chunk;
+   char bitmap[BITMAP_SIZE_C * 2 + 1];
+   unsigned long i;
+   char *bm = bitmap;
+   char *entry;
+
+   if (unlikely(pool == NULL || pool->chunks.next == NULL))
+   return;
+
+   chunk = container_of(pool->chunks.next, struct gen_pool_chunk,
+next_chunk);
+   entry = (void *)chunk->entries;
+   for (i = 1; i <= BITMAP_SIZE_C; i++)
+   bm += snprintf(bm, 3, "%02hhx", entry[BITMAP_SIZE_C - i]);
+   *bm = '\0';
+   pr_notice("chunk: %pbitmap: 0x%s\n", chunk, bitmap);
+
+}
+
+#endif
+
+enum test_commands {
+   CMD_ALLOCATOR,
+   CMD_ALLOCATE,
+   CMD_FLUSH,
+   CMD_FREE,
+   CMD_NUMBER,
+   CMD_END = CMD_NUMBER,
+};
+
+struct null_struct {
+   void *null;
+};
+
+struct test_allocator {
+   genpool_algo_t algo;
+   union {
+   struct genpool_data_align align;
+   struct genpool_data_fixed offset;
+   struct null_struct null;
+   } data;
+};
+
+struct 

[PATCH 3/3] genalloc: selftest

[Igor Stoppa]

Introduce a set of macros for writing concise test cases for genalloc.

The test cases are meant to provide regression testing, when working on
new functionality for genalloc.

Primarily they are meant to confirm that the various allocation strategy
will continue to work as expected.

The execution of the self testing is controlled through a Kconfig option.

The testing takes place in the very early stages of main.c, to ensure
that failures in genalloc are caught before they can cause unexplained
erratic behavior in any of genalloc users.

Therefore, it would not be advisable to implement it as module.

Signed-off-by: Igor Stoppa 
---
 init/main.c |   2 +
 lib/Kconfig |  15 ++
 lib/Makefile|   1 +
 lib/test_genalloc.c | 410 
 4 files changed, 428 insertions(+)
 create mode 100644 lib/test_genalloc.c

diff --git a/init/main.c b/init/main.c
index b795aa341a3a..b3b319d91b0e 100644
--- a/init/main.c
+++ b/init/main.c
@@ -91,6 +91,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -679,6 +680,7 @@ asmlinkage __visible void __init start_kernel(void)
 */
mem_encrypt_init();
 
+   test_genalloc();
 #ifdef CONFIG_BLK_DEV_INITRD
if (initrd_start && !initrd_below_start_ok &&
page_to_pfn(virt_to_page((void *)initrd_start)) < min_low_pfn) {
diff --git a/lib/Kconfig b/lib/Kconfig
index 09565d779324..2bf89af50728 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -303,6 +303,21 @@ config DECOMPRESS_LZ4
 config GENERIC_ALLOCATOR
bool
 
+config TEST_GENERIC_ALLOCATOR
+   bool "genalloc tester"
+   default n
+   select GENERIC_ALLOCATOR
+   help
+ Enable automated testing of the generic allocator.
+ The testing is primarily for the tracking of allocated space.
+
+config TEST_GENERIC_ALLOCATOR_VERBOSE
+   bool "make the genalloc tester more verbose"
+   default n
+   select TEST_GENERIC_ALLOCATOR
+   help
+ More information will be displayed during the self-testing.
+
 #
 # reed solomon support is select'ed if needed
 #
diff --git a/lib/Makefile b/lib/Makefile
index 384713ff70d3..2c66346ab246 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -114,6 +114,7 @@ obj-$(CONFIG_LIBCRC32C) += libcrc32c.o
 obj-$(CONFIG_CRC8) += crc8.o
 obj-$(CONFIG_XXHASH)   += xxhash.o
 obj-$(CONFIG_GENERIC_ALLOCATOR) += genalloc.o
+obj-$(CONFIG_TEST_GENERIC_ALLOCATOR) += test_genalloc.o
 
 obj-$(CONFIG_842_COMPRESS) += 842/
 obj-$(CONFIG_842_DECOMPRESS) += 842/
diff --git a/lib/test_genalloc.c b/lib/test_genalloc.c
new file mode 100644
index ..ab9984861517
--- /dev/null
+++ b/lib/test_genalloc.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * test_genalloc.c
+ *
+ * (C) Copyright 2017 Huawei Technologies Co. Ltd.
+ * Author: Igor Stoppa 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+
+/*
+ * In case of failure of any of these tests, memory corruption is almost
+ * guarranteed; allowing the boot to continue means risking to corrupt
+ * also any filesystem/block device accessed write mode.
+ * Therefore, BUG_ON() is used, when testing.
+ */
+
+
+/*
+ * Keep the bitmap small, while including case of cross-ulong mapping.
+ * For simplicity, the test cases use only 1 chunk of memory.
+ */
+#define BITMAP_SIZE_C 16
+#define ALLOC_ORDER 0
+
+#define ULONG_SIZE (sizeof(unsigned long))
+#define BITMAP_SIZE_UL (BITMAP_SIZE_C / ULONG_SIZE)
+#define MIN_ALLOC_SIZE (1 << ALLOC_ORDER)
+#define ENTRIES (BITMAP_SIZE_C * 8)
+#define CHUNK_SIZE  (MIN_ALLOC_SIZE * ENTRIES)
+
+#ifndef CONFIG_TEST_GENERIC_ALLOCATOR_VERBOSE
+
+static inline void print_first_chunk_bitmap(struct gen_pool *pool) {}
+
+#else
+
+static void print_first_chunk_bitmap(struct gen_pool *pool)
+{
+   struct gen_pool_chunk *chunk;
+   char bitmap[BITMAP_SIZE_C * 2 + 1];
+   unsigned long i;
+   char *bm = bitmap;
+   char *entry;
+
+   if (unlikely(pool == NULL || pool->chunks.next == NULL))
+   return;
+
+   chunk = container_of(pool->chunks.next, struct gen_pool_chunk,
+next_chunk);
+   entry = (void *)chunk->entries;
+   for (i = 1; i <= BITMAP_SIZE_C; i++)
+   bm += snprintf(bm, 3, "%02hhx", entry[BITMAP_SIZE_C - i]);
+   *bm = '\0';
+   pr_notice("chunk: %pbitmap: 0x%s\n", chunk, bitmap);
+
+}
+
+#endif
+
+enum test_commands {
+   CMD_ALLOCATOR,
+   CMD_ALLOCATE,
+   CMD_FLUSH,
+   CMD_FREE,
+   CMD_NUMBER,
+   CMD_END = CMD_NUMBER,
+};
+
+struct null_struct {
+   void *null;
+};
+
+struct test_allocator {
+   genpool_algo_t algo;
+   union {
+   struct genpool_data_align align;
+   struct genpool_data_fixed offset;
+   struct null_struct null;
+   } data;
+};
+
+struct test_action {
+   unsigned int location;
+   char 

[PATCH 2/3] Add label and license to genalloc.rst

[Igor Stoppa]

Add SPDX license to genalloc.rst, then a label, to allow cross-referencing.

Signed-off-by: Igor Stoppa 
---
 Documentation/core-api/genalloc.rst | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/core-api/genalloc.rst 
b/Documentation/core-api/genalloc.rst
index 6b38a39fab24..0b5ade832ee8 100644
--- a/Documentation/core-api/genalloc.rst
+++ b/Documentation/core-api/genalloc.rst
@@ -1,3 +1,7 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _genalloc:
+
 The genalloc/genpool subsystem
 ==
 
-- 
2.14.1

[PATCH 1/3] genalloc: track beginning of allocations

[Igor Stoppa]

The genalloc library is only capable of tracking if a certain unit of
allocation is in use or not.

It is not capable of discerning where the memory associated to an
allocation request begins and where it ends.

The reason is that units of allocations are tracked by using a bitmap,
where each bit represents that the unit is either allocated (1) or
available (0).

The user of the API must keep track of how much space was requested, if
it ever needs to be freed.

This can cause errors being undetected.
Examples:
* Only a subset of the memory provided to an allocation request is freed
* The memory from a subsequent allocation is freed
* The memory being freed doesn't start at the beginning of an
  allocation.

The bitmap is used because it allows to perform lockless read/write
access, where this is supported by hw through cmpxchg.
Similarly, it is possible to scan the bitmap for a sufficiently long
sequence of zeros, to identify zones available for allocation.

This patch doubles the space reserved in the bitmap for each allocation,
to track their beginning.

For details, see the documentation inside lib/genalloc.c

The primary effect of this patch is that code using the gen_alloc
library does not need anymore to keep track of the size of the
allocations it makes.

Prior to this patch, it was necessary to keep track of the size of the
allocation, so that it would be possible, later on, to know how much
space should be freed.

Now, users of the api can choose to etiher still specify explicitly the
size, or let the library determine it, by giving a value of 0.

However, even when the value is specified, the library still uses its on
understanding of the space associated with a certain allocation, to
confirm that they are consistent.

This verification also confirms that the patch works correctly.

Eventually, the extra parameter (and the corresponding verification)
could be dropped, in favor of a simplified API.

Signed-off-by: Igor Stoppa 
---
 include/linux/genalloc.h | 112 +++
 lib/genalloc.c   | 742 ++-
 2 files changed, 599 insertions(+), 255 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 872f930f1b06..ff7229520656 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -32,7 +32,7 @@
 
 #include 
 #include 
-#include 
+#include 
 
 struct device;
 struct device_node;
@@ -76,7 +76,7 @@ struct gen_pool_chunk {
phys_addr_t phys_addr;  /* physical starting address of memory 
chunk */
unsigned long start_addr;   /* start address of memory chunk */
unsigned long end_addr; /* end address of memory chunk 
(inclusive) */
-   unsigned long bits[0];  /* bitmap for allocating memory chunk */
+   unsigned long entries[0];   /* bitmap for allocating memory chunk */
 };
 
 /*
@@ -93,74 +93,82 @@ struct genpool_data_fixed {
unsigned long offset;   /* The offset of the specific region */
 };
 
-extern struct gen_pool *gen_pool_create(int, int);
-extern phys_addr_t gen_pool_virt_to_phys(struct gen_pool *pool, unsigned long);
-extern int gen_pool_add_virt(struct gen_pool *, unsigned long, phys_addr_t,
-size_t, int);
-/**
- * gen_pool_add - add a new chunk of special memory to the pool
- * @pool: pool to add new memory chunk to
- * @addr: starting address of memory chunk to add to pool
- * @size: size in bytes of the memory chunk to add to pool
- * @nid: node id of the node the chunk structure and bitmap should be
- *   allocated on, or -1
- *
- * Add a new chunk of special memory to the specified pool.
- *
- * Returns 0 on success or a -ve errno on failure.
- */
+struct gen_pool *gen_pool_create(int min_alloc_order, int nid);
+
+int gen_pool_add_virt(struct gen_pool *pool, unsigned long virt,
+ phys_addr_t phys, size_t size, int nid);
+
+
 static inline int gen_pool_add(struct gen_pool *pool, unsigned long addr,
   size_t size, int nid)
 {
return gen_pool_add_virt(pool, addr, -1, size, nid);
 }
-extern void gen_pool_destroy(struct gen_pool *);
-extern unsigned long gen_pool_alloc(struct gen_pool *, size_t);
-extern unsigned long gen_pool_alloc_algo(struct gen_pool *, size_t,
-   genpool_algo_t algo, void *data);
-extern void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size,
-   dma_addr_t *dma);
-extern void gen_pool_free(struct gen_pool *, unsigned long, size_t);
-extern void gen_pool_for_each_chunk(struct gen_pool *,
-   void (*)(struct gen_pool *, struct gen_pool_chunk *, void *), void *);
-extern size_t gen_pool_avail(struct gen_pool *);
-extern size_t gen_pool_size(struct gen_pool *);
 
-extern void gen_pool_set_algo(struct gen_pool *pool, genpool_algo_t algo,
-   void *data);
+phys_addr_t gen_pool_virt_to_phys(struct gen_pool *pool, unsigned long addr);
 
-extern unsigned 

[PATCH 0/3] linux-next: mm: hardening: Track genalloc allocations

[Igor Stoppa]

This patchset was created as part of an older version of pmalloc, however
it has value per-se, as it hardens the memory management for the generic
allocator genalloc.

Genalloc does not currently track the size of the allocations it hands out.

Either by mistake, or due to an attack, it is possible that more memory
than what was initially allocated is freed, leaving behind dangling
pointers, ready for an use-after-free attack.

With this patch, genalloc becomes capable of tracking the size of each
allocation it has handed out, when it's time to free it.

It can either verify that the size received, when free is invoked, is
correct, or it can decide autonomously how much memory to free, if the
value received for the size parameter is 0.

These patches are proposed for beign merged into linux-next, to verify
that they do not introduce regressions, by comparing the value received
from the callers of the free function with the internal tracking.

Later on, the "size" parameter can be dropped, and each caller can be
adjusted accordingly.

Signed-off-by: Igor Stoppa 

Igor Stoppa (3):
  genalloc: track beginning of allocations
  Add label and license to genalloc.rst
  genalloc: selftest

 Documentation/core-api/genalloc.rst |   4 +
 include/linux/genalloc.h| 112 +++---
 init/main.c |   2 +
 lib/Kconfig |  15 +
 lib/Makefile|   1 +
 lib/genalloc.c  | 742 ++--
 lib/test_genalloc.c | 410 
 7 files changed, 1031 insertions(+), 255 deletions(-)
 create mode 100644 lib/test_genalloc.c

-- 
2.14.1

[PATCH 2/3] Add label and license to genalloc.rst

[Igor Stoppa]

Add SPDX license to genalloc.rst, then a label, to allow cross-referencing.

Signed-off-by: Igor Stoppa 
---
 Documentation/core-api/genalloc.rst | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/core-api/genalloc.rst 
b/Documentation/core-api/genalloc.rst
index 6b38a39fab24..0b5ade832ee8 100644
--- a/Documentation/core-api/genalloc.rst
+++ b/Documentation/core-api/genalloc.rst
@@ -1,3 +1,7 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _genalloc:
+
 The genalloc/genpool subsystem
 ==
 
-- 
2.14.1

[PATCH 1/3] genalloc: track beginning of allocations

[Igor Stoppa]

The genalloc library is only capable of tracking if a certain unit of
allocation is in use or not.

It is not capable of discerning where the memory associated to an
allocation request begins and where it ends.

The reason is that units of allocations are tracked by using a bitmap,
where each bit represents that the unit is either allocated (1) or
available (0).

The user of the API must keep track of how much space was requested, if
it ever needs to be freed.

This can cause errors being undetected.
Examples:
* Only a subset of the memory provided to an allocation request is freed
* The memory from a subsequent allocation is freed
* The memory being freed doesn't start at the beginning of an
  allocation.

The bitmap is used because it allows to perform lockless read/write
access, where this is supported by hw through cmpxchg.
Similarly, it is possible to scan the bitmap for a sufficiently long
sequence of zeros, to identify zones available for allocation.

This patch doubles the space reserved in the bitmap for each allocation,
to track their beginning.

For details, see the documentation inside lib/genalloc.c

The primary effect of this patch is that code using the gen_alloc
library does not need anymore to keep track of the size of the
allocations it makes.

Prior to this patch, it was necessary to keep track of the size of the
allocation, so that it would be possible, later on, to know how much
space should be freed.

Now, users of the api can choose to etiher still specify explicitly the
size, or let the library determine it, by giving a value of 0.

However, even when the value is specified, the library still uses its on
understanding of the space associated with a certain allocation, to
confirm that they are consistent.

This verification also confirms that the patch works correctly.

Eventually, the extra parameter (and the corresponding verification)
could be dropped, in favor of a simplified API.

Signed-off-by: Igor Stoppa 
---
 include/linux/genalloc.h | 112 +++
 lib/genalloc.c   | 742 ++-
 2 files changed, 599 insertions(+), 255 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 872f930f1b06..ff7229520656 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -32,7 +32,7 @@
 
 #include 
 #include 
-#include 
+#include 
 
 struct device;
 struct device_node;
@@ -76,7 +76,7 @@ struct gen_pool_chunk {
phys_addr_t phys_addr;  /* physical starting address of memory 
chunk */
unsigned long start_addr;   /* start address of memory chunk */
unsigned long end_addr; /* end address of memory chunk 
(inclusive) */
-   unsigned long bits[0];  /* bitmap for allocating memory chunk */
+   unsigned long entries[0];   /* bitmap for allocating memory chunk */
 };
 
 /*
@@ -93,74 +93,82 @@ struct genpool_data_fixed {
unsigned long offset;   /* The offset of the specific region */
 };
 
-extern struct gen_pool *gen_pool_create(int, int);
-extern phys_addr_t gen_pool_virt_to_phys(struct gen_pool *pool, unsigned long);
-extern int gen_pool_add_virt(struct gen_pool *, unsigned long, phys_addr_t,
-size_t, int);
-/**
- * gen_pool_add - add a new chunk of special memory to the pool
- * @pool: pool to add new memory chunk to
- * @addr: starting address of memory chunk to add to pool
- * @size: size in bytes of the memory chunk to add to pool
- * @nid: node id of the node the chunk structure and bitmap should be
- *   allocated on, or -1
- *
- * Add a new chunk of special memory to the specified pool.
- *
- * Returns 0 on success or a -ve errno on failure.
- */
+struct gen_pool *gen_pool_create(int min_alloc_order, int nid);
+
+int gen_pool_add_virt(struct gen_pool *pool, unsigned long virt,
+ phys_addr_t phys, size_t size, int nid);
+
+
 static inline int gen_pool_add(struct gen_pool *pool, unsigned long addr,
   size_t size, int nid)
 {
return gen_pool_add_virt(pool, addr, -1, size, nid);
 }
-extern void gen_pool_destroy(struct gen_pool *);
-extern unsigned long gen_pool_alloc(struct gen_pool *, size_t);
-extern unsigned long gen_pool_alloc_algo(struct gen_pool *, size_t,
-   genpool_algo_t algo, void *data);
-extern void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size,
-   dma_addr_t *dma);
-extern void gen_pool_free(struct gen_pool *, unsigned long, size_t);
-extern void gen_pool_for_each_chunk(struct gen_pool *,
-   void (*)(struct gen_pool *, struct gen_pool_chunk *, void *), void *);
-extern size_t gen_pool_avail(struct gen_pool *);
-extern size_t gen_pool_size(struct gen_pool *);
 
-extern void gen_pool_set_algo(struct gen_pool *pool, genpool_algo_t algo,
-   void *data);
+phys_addr_t gen_pool_virt_to_phys(struct gen_pool *pool, unsigned long addr);
 
-extern unsigned long 

[PATCH 0/3] linux-next: mm: hardening: Track genalloc allocations

[Igor Stoppa]

This patchset was created as part of an older version of pmalloc, however
it has value per-se, as it hardens the memory management for the generic
allocator genalloc.

Genalloc does not currently track the size of the allocations it hands out.

Either by mistake, or due to an attack, it is possible that more memory
than what was initially allocated is freed, leaving behind dangling
pointers, ready for an use-after-free attack.

With this patch, genalloc becomes capable of tracking the size of each
allocation it has handed out, when it's time to free it.

It can either verify that the size received, when free is invoked, is
correct, or it can decide autonomously how much memory to free, if the
value received for the size parameter is 0.

These patches are proposed for beign merged into linux-next, to verify
that they do not introduce regressions, by comparing the value received
from the callers of the free function with the internal tracking.

Later on, the "size" parameter can be dropped, and each caller can be
adjusted accordingly.

Signed-off-by: Igor Stoppa 

Igor Stoppa (3):
  genalloc: track beginning of allocations
  Add label and license to genalloc.rst
  genalloc: selftest

 Documentation/core-api/genalloc.rst |   4 +
 include/linux/genalloc.h| 112 +++---
 init/main.c |   2 +
 lib/Kconfig |  15 +
 lib/Makefile|   1 +
 lib/genalloc.c  | 742 ++--
 lib/test_genalloc.c | 410 
 7 files changed, 1031 insertions(+), 255 deletions(-)
 create mode 100644 lib/test_genalloc.c

-- 
2.14.1

Re: ed74ae0342 ("blk-mq: Avoid that a completion can be ignored .."): BUG: kernel hang in test stage

[Fengguang Wu]

Hi Jens,

On Fri, Apr 27, 2018 at 06:52:58PM -0600, Jens Axboe wrote:

On 4/24/18 3:00 PM, kernel test robot wrote:

Greetings,

0day kernel testing robot got the below dmesg and the first bad commit is

https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-linus

commit ed74ae03424684a6ad8a973c3fa727c6b4162432
Author: Bart Van Assche 
AuthorDate: Thu Apr 19 09:43:53 2018 -0700
Commit: Jens Axboe 
CommitDate: Thu Apr 19 14:21:47 2018 -0600

blk-mq: Avoid that a completion can be ignored for BLK_EH_RESET_TIMER



Any chance you can try with the newer version?

https://github.com/bvanassche/linux/commit/4acd555fa13087


That works!

Tested-by: Fengguang Wu 

Re: ed74ae0342 ("blk-mq: Avoid that a completion can be ignored .."): BUG: kernel hang in test stage

[Fengguang Wu]

Hi Jens,

On Fri, Apr 27, 2018 at 06:52:58PM -0600, Jens Axboe wrote:

On 4/24/18 3:00 PM, kernel test robot wrote:

Greetings,

0day kernel testing robot got the below dmesg and the first bad commit is

https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-linus

commit ed74ae03424684a6ad8a973c3fa727c6b4162432
Author: Bart Van Assche 
AuthorDate: Thu Apr 19 09:43:53 2018 -0700
Commit: Jens Axboe 
CommitDate: Thu Apr 19 14:21:47 2018 -0600

blk-mq: Avoid that a completion can be ignored for BLK_EH_RESET_TIMER



Any chance you can try with the newer version?

https://github.com/bvanassche/linux/commit/4acd555fa13087


That works!

Tested-by: Fengguang Wu 

[RFC patch] Documenation: don't build media .rst files when not building Documentation/media/

[Randy Dunlap]

Don't build media .rst files from header files when Documentation/media/
is not in the target SPHINXDIRS.

It looks like the dependencies in Documentation/media/Makefile should
take care of running parse-headers.pl to generated .rst files from the
header files, but it's not happening and I don't see why not.

---
 Documentation/Makefile |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- linux-next-20180426.orig/Documentation/Makefile
+++ linux-next-20180426/Documentation/Makefile
@@ -52,8 +52,7 @@ loop_cmd = $(echo-cmd) $(cmd_$(1)) || ex
 #e.g. "media" for the linux-tv book-set at ./Documentation/media
 
 quiet_cmd_sphinx = SPHINX  $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
-  cmd_sphinx = $(MAKE) BUILDDIR=$(abspath $(BUILDDIR)) 
$(build)=Documentation/media $2 && \
-   PYTHONDONTWRITEBYTECODE=1 \
+  cmd_sphinx = PYTHONDONTWRITEBYTECODE=1 \
BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath 
$(srctree)/$(src)/$5/$(SPHINX_CONF)) \
$(SPHINXBUILD) \
-b $2 \

[RFC patch] Documenation: don't build media .rst files when not building Documentation/media/

[Randy Dunlap]

Don't build media .rst files from header files when Documentation/media/
is not in the target SPHINXDIRS.

It looks like the dependencies in Documentation/media/Makefile should
take care of running parse-headers.pl to generated .rst files from the
header files, but it's not happening and I don't see why not.

---
 Documentation/Makefile |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- linux-next-20180426.orig/Documentation/Makefile
+++ linux-next-20180426/Documentation/Makefile
@@ -52,8 +52,7 @@ loop_cmd = $(echo-cmd) $(cmd_$(1)) || ex
 #e.g. "media" for the linux-tv book-set at ./Documentation/media
 
 quiet_cmd_sphinx = SPHINX  $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
-  cmd_sphinx = $(MAKE) BUILDDIR=$(abspath $(BUILDDIR)) 
$(build)=Documentation/media $2 && \
-   PYTHONDONTWRITEBYTECODE=1 \
+  cmd_sphinx = PYTHONDONTWRITEBYTECODE=1 \
BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath 
$(srctree)/$(src)/$5/$(SPHINX_CONF)) \
$(SPHINXBUILD) \
-b $2 \

[PATCH 1/2] x86/amd_nb: Add support for Raven Ridge CPUs

[Guenter Roeck]

Add Raven Ridge root bridge and data fabric PCI IDs.
This is required for amd_pci_dev_to_node_id() and amd_smn_read().

Signed-off-by: Guenter Roeck 
---
This patch is a prerequisite for the second patch in the series.
I'll be happy to apply both patches through hwmon if that is acceptable
(and Cc: stable for 4.16+). If not, I'll be happy to wait for this patch
to be available upstream.

Since that there is no public documentation available for Raven Ridge,
PCI IDs are derived from output of lspci.

 arch/x86/kernel/amd_nb.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index c88e0b127810..bd33613ecb7c 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -14,8 +14,11 @@
 #include 
 
 #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450
+#define PCI_DEVICE_ID_AMD_17H_RR_ROOT  0x15d0
 #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
 #define PCI_DEVICE_ID_AMD_17H_DF_F40x1464
+#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x15eb
+#define PCI_DEVICE_ID_AMD_17H_RR_DF_F4 0x15ec
 
 /* Protect the PCI config register pairs used for SMN and DF indirect access. 
*/
 static DEFINE_MUTEX(smn_mutex);
@@ -24,6 +27,7 @@ static u32 *flush_words;
 
 static const struct pci_device_id amd_root_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_ROOT) },
+   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_ROOT) },
{}
 };
 
@@ -39,6 +43,7 @@ const struct pci_device_id amd_nb_misc_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
+   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
{}
 };
@@ -51,6 +56,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) },
+   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
{}
 };
-- 
2.7.4

[PATCH 2/2] hwmon: (k10temp) Use API function to access System Management Network

[Guenter Roeck]

The SMN (System Management Network) on Family 17h AMD CPUs is also accessed
from other drivers, specifically EDAC. Accessing it directly is racy.
On top of that, accessing the SMN through root bridge 00:00 is wrong on
multi-die CPUs and may result in reading the temperature from the wrong
die. Use available API functions to fix the problem.

For this to work, also change the Raven Ridge PCI device ID to point to
Data Fabric Function 3, since this ID is used by the API functions to
find the CPU node.

Signed-off-by: Guenter Roeck 
---
 drivers/hwmon/k10temp.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index b06bb1f90853..00e785afae0d 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 MODULE_DESCRIPTION("AMD Family 10h+ CPU core temperature monitor");
@@ -40,8 +41,8 @@ static DEFINE_MUTEX(nb_smu_ind_mutex);
 #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
 #endif
 
-#ifndef PCI_DEVICE_ID_AMD_17H_RR_NB
-#define PCI_DEVICE_ID_AMD_17H_RR_NB0x15d0
+#ifndef PCI_DEVICE_ID_AMD_17H_RR_DF_F3
+#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x14eb
 #endif
 
 /* CPUID function 0x8001, ebx */
@@ -136,8 +137,8 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 
*regval)
 
 static void read_tempreg_nb_f17(struct pci_dev *pdev, u32 *regval)
 {
-   amd_nb_index_read(pdev, PCI_DEVFN(0, 0), 0x60,
- F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
+   amd_smn_read(amd_pci_dev_to_node_id(pdev),
+F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
 }
 
 static ssize_t temp1_input_show(struct device *dev,
@@ -323,7 +324,7 @@ static const struct pci_device_id k10temp_id_table[] = {
{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
-   { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_RR_NB) },
+   { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
{}
 };
 MODULE_DEVICE_TABLE(pci, k10temp_id_table);
-- 
2.7.4

[PATCH 1/2] x86/amd_nb: Add support for Raven Ridge CPUs

[Guenter Roeck]

Add Raven Ridge root bridge and data fabric PCI IDs.
This is required for amd_pci_dev_to_node_id() and amd_smn_read().

Signed-off-by: Guenter Roeck 
---
This patch is a prerequisite for the second patch in the series.
I'll be happy to apply both patches through hwmon if that is acceptable
(and Cc: stable for 4.16+). If not, I'll be happy to wait for this patch
to be available upstream.

Since that there is no public documentation available for Raven Ridge,
PCI IDs are derived from output of lspci.

 arch/x86/kernel/amd_nb.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index c88e0b127810..bd33613ecb7c 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -14,8 +14,11 @@
 #include 
 
 #define PCI_DEVICE_ID_AMD_17H_ROOT 0x1450
+#define PCI_DEVICE_ID_AMD_17H_RR_ROOT  0x15d0
 #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
 #define PCI_DEVICE_ID_AMD_17H_DF_F40x1464
+#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x15eb
+#define PCI_DEVICE_ID_AMD_17H_RR_DF_F4 0x15ec
 
 /* Protect the PCI config register pairs used for SMN and DF indirect access. 
*/
 static DEFINE_MUTEX(smn_mutex);
@@ -24,6 +27,7 @@ static u32 *flush_words;
 
 static const struct pci_device_id amd_root_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_ROOT) },
+   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_ROOT) },
{}
 };
 
@@ -39,6 +43,7 @@ const struct pci_device_id amd_nb_misc_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
+   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
{}
 };
@@ -51,6 +56,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_NB_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) },
+   { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
{}
 };
-- 
2.7.4

[PATCH 2/2] hwmon: (k10temp) Use API function to access System Management Network

[Guenter Roeck]

The SMN (System Management Network) on Family 17h AMD CPUs is also accessed
from other drivers, specifically EDAC. Accessing it directly is racy.
On top of that, accessing the SMN through root bridge 00:00 is wrong on
multi-die CPUs and may result in reading the temperature from the wrong
die. Use available API functions to fix the problem.

For this to work, also change the Raven Ridge PCI device ID to point to
Data Fabric Function 3, since this ID is used by the API functions to
find the CPU node.

Signed-off-by: Guenter Roeck 
---
 drivers/hwmon/k10temp.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index b06bb1f90853..00e785afae0d 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 MODULE_DESCRIPTION("AMD Family 10h+ CPU core temperature monitor");
@@ -40,8 +41,8 @@ static DEFINE_MUTEX(nb_smu_ind_mutex);
 #define PCI_DEVICE_ID_AMD_17H_DF_F30x1463
 #endif
 
-#ifndef PCI_DEVICE_ID_AMD_17H_RR_NB
-#define PCI_DEVICE_ID_AMD_17H_RR_NB0x15d0
+#ifndef PCI_DEVICE_ID_AMD_17H_RR_DF_F3
+#define PCI_DEVICE_ID_AMD_17H_RR_DF_F3 0x14eb
 #endif
 
 /* CPUID function 0x8001, ebx */
@@ -136,8 +137,8 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 
*regval)
 
 static void read_tempreg_nb_f17(struct pci_dev *pdev, u32 *regval)
 {
-   amd_nb_index_read(pdev, PCI_DEVFN(0, 0), 0x60,
- F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
+   amd_smn_read(amd_pci_dev_to_node_id(pdev),
+F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
 }
 
 static ssize_t temp1_input_show(struct device *dev,
@@ -323,7 +324,7 @@ static const struct pci_device_id k10temp_id_table[] = {
{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_NB_F3) },
{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F3) },
{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
-   { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_RR_NB) },
+   { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_RR_DF_F3) },
{}
 };
 MODULE_DEVICE_TABLE(pci, k10temp_id_table);
-- 
2.7.4

Re: FROM: "MRS PATRICIA WILLIAMS" linux-kernel@vger.kernel.org

[Mrs Patricia Williams]

FYI: About My Previous Message

Hi,

Am Mrs Patricia William, i just want to know if you receive my 
previous email i sent to you last three (3) days ago.

Is your email still Active? If YES; please can you email me back, 
i have something very important to discuss with you.

Awaits your reply soon..


Best Regard
Mrs. Patricia Williams

Re: FROM: "MRS PATRICIA WILLIAMS" linux-kernel@vger.kernel.org

[Mrs Patricia Williams]

FYI: About My Previous Message

Hi,

Am Mrs Patricia William, i just want to know if you receive my 
previous email i sent to you last three (3) days ago.

Is your email still Active? If YES; please can you email me back, 
i have something very important to discuss with you.

Awaits your reply soon..


Best Regard
Mrs. Patricia Williams

Re: [PATCH v2 1/2] drm/ttm: Only allocate huge pages with new flag TTM_PAGE_FLAG_TRANSHUGE

[Ilia Mirkin]

On Sat, Apr 28, 2018 at 7:02 PM, Michel Dänzer  wrote:
> On 2018-04-28 06:30 PM, Ilia Mirkin wrote:
>> On Fri, Apr 27, 2018 at 9:08 AM, Michel Dänzer  wrote:
>>> From: Michel Dänzer 
>>>
>>> Previously, TTM would always (with CONFIG_TRANSPARENT_HUGEPAGE enabled)
>>> try to allocate huge pages. However, not all drivers can take advantage
>>> of huge pages, but they would incur the overhead for allocating and
>>> freeing them anyway.
>>>
>>> Now, drivers which can take advantage of huge pages need to set the new
>>> flag TTM_PAGE_FLAG_TRANSHUGE to get them. Drivers not setting this flag
>>> no longer incur any overhead for allocating or freeing huge pages.
>>>
>>> v2:
>>> * Also guard swapping of consecutive pages in ttm_get_pages
>>> * Reword commit log, hopefully clearer now
>>>
>>> Cc: sta...@vger.kernel.org
>>> Signed-off-by: Michel Dänzer 
>>
>> Both I and lots of other people, based on reports, are still seeing
>> plenty of issues with this as late as 4.16.4.
>
> "lots of other people", "plenty of issues" sounds a bit exaggerated from
> what I've seen. FWIW, while I did see the original messages myself, I
> haven't seen any since Christian's original fix (see below), neither
> with amdgpu nor radeon, even before this patch you followed up to.

Probably a half-dozen reports of it with nouveau, in addition to
another bunch of people talking about it on the bug you mention below,
along with email threads on dri-devel.

I figured I didn't have to raise my own since it was identical to the
others, and, I assumed, was being handled.

>> Admittedly I'm on nouveau, but others have reported issues with
>> radeon/amdgpu as well. It's been going on since the feature was merged
>> in v4.15, with what seems like little investigation from the authors
>> introducing the feature.
>
> That's not a fair assessment. See
> https://bugs.freedesktop.org/show_bug.cgi?id=104082#c40 and following
> comments.
>
> Christian fixed the original issue in
> d0bc0c2a31c95002d37c3cc511ffdcab851b3256 "swiotlb: suppress warning when
> __GFP_NOWARN is set". Christian did his best to try and get the fix in
> before 4.15 final, but for reasons beyond his control, it was delayed
> until 4.16-rc1 and then backported to 4.15.5.

In case it's unclear, let me state this explicitly -- I totally get
that despite best intentions, bugs get introduced. I do it myself.
What I'm having trouble with is the handling once the issue is
discovered.

>
> Unfortunately, there was an swiotlb regression (not directly related to
> Christian's work) shortly after this fix, also in 4.16-rc1, which is now
> fixed in 4.17-rc1 and will be backported to 4.16.y.

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=v4.16.5=2c9dacf5bfe1e45d96dfe97cb71d2b717786a7b9

This guy? Didn't help. I'm running 4.16.4 right now.

> It looks like there's at least one more bug left, but it's not clear yet
> when that was introduced, whether it's directly related to Christian's
> work, or indeed what the impact is. Let's not get ahead of ourselves.

Whether it is directly related to that work or not, the issue
persists. There are two options:

 - When declaring things fixed, no serious attempt was actually made
at reproducing the underlying issues.
 - The authors truly can't reproduce the underlying issues users are
seeing and are taking stabs in the dark.

Given that a number of people are reporting problems, in either
scenario, the reasonable thing is to disable the feature, and figure
out what is going on. Maybe condition it on !CONFIG_SWIOTLB.

>> We now have *two* broken releases, v4.15 and v4.16 (anything that
>> spews error messages and stack traces ad-infinitum in dmesg is, by
>> definition, broken).
>
> I haven't seen any evidence that there's still an issue in 4.15, is
> there any?

Well, I did have a late 4.15 rc kernel in addition to the 'suppress
warning' patch. Now I'm questioning my memory of whether the issue was
resolved there or not. I'm pretty sure that 'not', but no longer 100%.
Either way, I think we all agree v4.15 was broken and more importantly
was *known* to be broken well in advance of the release. A reasonable
option would have been to disable the feature until the other bits
fell into place.

>> You're putting this behind a flag now (finally),
>
> I wrote this patch because I realized due to some remark I happened to
> see you make this week on IRC that the huge page support in TTM was
> enabled for all drivers. Instead of making that kind of remark on IRC,
> it would have been more constructive, and more conducive to quick
> implementation, to suggest making the feature not active for drivers
> which don't need it in a mailing list post.

I see IRC as a much faster and direct way of reaching the authors
and/or people who need to know an issue. As there was already a bug
filed about it and the issue was known about, I didn't really see a

Re: [PATCH v2 1/2] drm/ttm: Only allocate huge pages with new flag TTM_PAGE_FLAG_TRANSHUGE

[Ilia Mirkin]

On Sat, Apr 28, 2018 at 7:02 PM, Michel Dänzer  wrote:
> On 2018-04-28 06:30 PM, Ilia Mirkin wrote:
>> On Fri, Apr 27, 2018 at 9:08 AM, Michel Dänzer  wrote:
>>> From: Michel Dänzer 
>>>
>>> Previously, TTM would always (with CONFIG_TRANSPARENT_HUGEPAGE enabled)
>>> try to allocate huge pages. However, not all drivers can take advantage
>>> of huge pages, but they would incur the overhead for allocating and
>>> freeing them anyway.
>>>
>>> Now, drivers which can take advantage of huge pages need to set the new
>>> flag TTM_PAGE_FLAG_TRANSHUGE to get them. Drivers not setting this flag
>>> no longer incur any overhead for allocating or freeing huge pages.
>>>
>>> v2:
>>> * Also guard swapping of consecutive pages in ttm_get_pages
>>> * Reword commit log, hopefully clearer now
>>>
>>> Cc: sta...@vger.kernel.org
>>> Signed-off-by: Michel Dänzer 
>>
>> Both I and lots of other people, based on reports, are still seeing
>> plenty of issues with this as late as 4.16.4.
>
> "lots of other people", "plenty of issues" sounds a bit exaggerated from
> what I've seen. FWIW, while I did see the original messages myself, I
> haven't seen any since Christian's original fix (see below), neither
> with amdgpu nor radeon, even before this patch you followed up to.

Probably a half-dozen reports of it with nouveau, in addition to
another bunch of people talking about it on the bug you mention below,
along with email threads on dri-devel.

I figured I didn't have to raise my own since it was identical to the
others, and, I assumed, was being handled.

>> Admittedly I'm on nouveau, but others have reported issues with
>> radeon/amdgpu as well. It's been going on since the feature was merged
>> in v4.15, with what seems like little investigation from the authors
>> introducing the feature.
>
> That's not a fair assessment. See
> https://bugs.freedesktop.org/show_bug.cgi?id=104082#c40 and following
> comments.
>
> Christian fixed the original issue in
> d0bc0c2a31c95002d37c3cc511ffdcab851b3256 "swiotlb: suppress warning when
> __GFP_NOWARN is set". Christian did his best to try and get the fix in
> before 4.15 final, but for reasons beyond his control, it was delayed
> until 4.16-rc1 and then backported to 4.15.5.

In case it's unclear, let me state this explicitly -- I totally get
that despite best intentions, bugs get introduced. I do it myself.
What I'm having trouble with is the handling once the issue is
discovered.

>
> Unfortunately, there was an swiotlb regression (not directly related to
> Christian's work) shortly after this fix, also in 4.16-rc1, which is now
> fixed in 4.17-rc1 and will be backported to 4.16.y.

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=v4.16.5=2c9dacf5bfe1e45d96dfe97cb71d2b717786a7b9

This guy? Didn't help. I'm running 4.16.4 right now.

> It looks like there's at least one more bug left, but it's not clear yet
> when that was introduced, whether it's directly related to Christian's
> work, or indeed what the impact is. Let's not get ahead of ourselves.

Whether it is directly related to that work or not, the issue
persists. There are two options:

 - When declaring things fixed, no serious attempt was actually made
at reproducing the underlying issues.
 - The authors truly can't reproduce the underlying issues users are
seeing and are taking stabs in the dark.

Given that a number of people are reporting problems, in either
scenario, the reasonable thing is to disable the feature, and figure
out what is going on. Maybe condition it on !CONFIG_SWIOTLB.

>> We now have *two* broken releases, v4.15 and v4.16 (anything that
>> spews error messages and stack traces ad-infinitum in dmesg is, by
>> definition, broken).
>
> I haven't seen any evidence that there's still an issue in 4.15, is
> there any?

Well, I did have a late 4.15 rc kernel in addition to the 'suppress
warning' patch. Now I'm questioning my memory of whether the issue was
resolved there or not. I'm pretty sure that 'not', but no longer 100%.
Either way, I think we all agree v4.15 was broken and more importantly
was *known* to be broken well in advance of the release. A reasonable
option would have been to disable the feature until the other bits
fell into place.

>> You're putting this behind a flag now (finally),
>
> I wrote this patch because I realized due to some remark I happened to
> see you make this week on IRC that the huge page support in TTM was
> enabled for all drivers. Instead of making that kind of remark on IRC,
> it would have been more constructive, and more conducive to quick
> implementation, to suggest making the feature not active for drivers
> which don't need it in a mailing list post.

I see IRC as a much faster and direct way of reaching the authors
and/or people who need to know an issue. As there was already a bug
filed about it and the issue was known about, I didn't really see a
reason to pile on (until now).

> At least, please do more research before making this 

[PATCH] media: staging: atomisp: fix a potential missing-check bug

[Wenwen Wang]

At the end of atomisp_subdev_set_selection(), the function
atomisp_subdev_get_rect() is invoked to get the pointer to v4l2_rect. Since
this function may return a NULL pointer, it is firstly invoked to check
the returned pointer. If the returned pointer is not NULL, then the
function is invoked again to obtain the pointer and the memory content
at the location of the returned pointer is copied to the memory location of
r. In most cases, the pointers returned by the two invocations are same.
However, given that the pointer returned by the function
atomisp_subdev_get_rect() is not a constant, it is possible that the two
invocations return two different pointers. For example, another thread may
race to modify the related pointers during the two invocations. In that
case, even if the first returned pointer is not null, the second returned
pointer might be null, which will cause issues such as null pointer
dereference.

This patch saves the pointer returned by the first invocation and removes
the second invocation. If the returned pointer is not NULL, the memory
content is copied according to the original code.

Signed-off-by: Wenwen Wang 
---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
index 49a9973..d5fa513 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
@@ -366,6 +366,7 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd,
unsigned int i;
unsigned int padding_w = pad_w;
unsigned int padding_h = pad_h;
+   struct v4l2_rect *p;
 
stream_id = atomisp_source_pad_to_stream_id(isp_sd, vdev_pad);
 
@@ -536,9 +537,10 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd,
ffmt[pad]->height = comp[pad]->height;
}
 
-   if (!atomisp_subdev_get_rect(sd, cfg, which, pad, target))
+   p = atomisp_subdev_get_rect(sd, cfg, which, pad, target);
+   if (!p)
return -EINVAL;
-   *r = *atomisp_subdev_get_rect(sd, cfg, which, pad, target);
+   *r = *p;
 
dev_dbg(isp->dev, "sel actual: l %d t %d w %d h %d\n",
r->left, r->top, r->width, r->height);
-- 
2.7.4

[PATCH] media: staging: atomisp: fix a potential missing-check bug

[Wenwen Wang]

At the end of atomisp_subdev_set_selection(), the function
atomisp_subdev_get_rect() is invoked to get the pointer to v4l2_rect. Since
this function may return a NULL pointer, it is firstly invoked to check
the returned pointer. If the returned pointer is not NULL, then the
function is invoked again to obtain the pointer and the memory content
at the location of the returned pointer is copied to the memory location of
r. In most cases, the pointers returned by the two invocations are same.
However, given that the pointer returned by the function
atomisp_subdev_get_rect() is not a constant, it is possible that the two
invocations return two different pointers. For example, another thread may
race to modify the related pointers during the two invocations. In that
case, even if the first returned pointer is not null, the second returned
pointer might be null, which will cause issues such as null pointer
dereference.

This patch saves the pointer returned by the first invocation and removes
the second invocation. If the returned pointer is not NULL, the memory
content is copied according to the original code.

Signed-off-by: Wenwen Wang 
---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
index 49a9973..d5fa513 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
@@ -366,6 +366,7 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd,
unsigned int i;
unsigned int padding_w = pad_w;
unsigned int padding_h = pad_h;
+   struct v4l2_rect *p;
 
stream_id = atomisp_source_pad_to_stream_id(isp_sd, vdev_pad);
 
@@ -536,9 +537,10 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd,
ffmt[pad]->height = comp[pad]->height;
}
 
-   if (!atomisp_subdev_get_rect(sd, cfg, which, pad, target))
+   p = atomisp_subdev_get_rect(sd, cfg, which, pad, target);
+   if (!p)
return -EINVAL;
-   *r = *atomisp_subdev_get_rect(sd, cfg, which, pad, target);
+   *r = *p;
 
dev_dbg(isp->dev, "sel actual: l %d t %d w %d h %d\n",
r->left, r->top, r->width, r->height);
-- 
2.7.4

Re: [PATCH v2 1/2] drm/ttm: Only allocate huge pages with new flag TTM_PAGE_FLAG_TRANSHUGE

[Michel Dänzer]

On 2018-04-28 06:30 PM, Ilia Mirkin wrote:
> On Fri, Apr 27, 2018 at 9:08 AM, Michel Dänzer  wrote:
>> From: Michel Dänzer 
>>
>> Previously, TTM would always (with CONFIG_TRANSPARENT_HUGEPAGE enabled)
>> try to allocate huge pages. However, not all drivers can take advantage
>> of huge pages, but they would incur the overhead for allocating and
>> freeing them anyway.
>>
>> Now, drivers which can take advantage of huge pages need to set the new
>> flag TTM_PAGE_FLAG_TRANSHUGE to get them. Drivers not setting this flag
>> no longer incur any overhead for allocating or freeing huge pages.
>>
>> v2:
>> * Also guard swapping of consecutive pages in ttm_get_pages
>> * Reword commit log, hopefully clearer now
>>
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Michel Dänzer 
> 
> Both I and lots of other people, based on reports, are still seeing
> plenty of issues with this as late as 4.16.4.

"lots of other people", "plenty of issues" sounds a bit exaggerated from
what I've seen. FWIW, while I did see the original messages myself, I
haven't seen any since Christian's original fix (see below), neither
with amdgpu nor radeon, even before this patch you followed up to.


> Admittedly I'm on nouveau, but others have reported issues with
> radeon/amdgpu as well. It's been going on since the feature was merged
> in v4.15, with what seems like little investigation from the authors
> introducing the feature.

That's not a fair assessment. See
https://bugs.freedesktop.org/show_bug.cgi?id=104082#c40 and following
comments.

Christian fixed the original issue in
d0bc0c2a31c95002d37c3cc511ffdcab851b3256 "swiotlb: suppress warning when
__GFP_NOWARN is set". Christian did his best to try and get the fix in
before 4.15 final, but for reasons beyond his control, it was delayed
until 4.16-rc1 and then backported to 4.15.5.

Unfortunately, there was an swiotlb regression (not directly related to
Christian's work) shortly after this fix, also in 4.16-rc1, which is now
fixed in 4.17-rc1 and will be backported to 4.16.y.

It looks like there's at least one more bug left, but it's not clear yet
when that was introduced, whether it's directly related to Christian's
work, or indeed what the impact is. Let's not get ahead of ourselves.


> We now have *two* broken releases, v4.15 and v4.16 (anything that
> spews error messages and stack traces ad-infinitum in dmesg is, by
> definition, broken).

I haven't seen any evidence that there's still an issue in 4.15, is
there any?


> You're putting this behind a flag now (finally),

I wrote this patch because I realized due to some remark I happened to
see you make this week on IRC that the huge page support in TTM was
enabled for all drivers. Instead of making that kind of remark on IRC,
it would have been more constructive, and more conducive to quick
implementation, to suggest making the feature not active for drivers
which don't need it in a mailing list post.


At least, please do more research before making this kind of negative
post.

P.S. You might also want to look into whether nouveau really should be
hitting swiotlb in these cases.

-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

Re: [PATCH v2 1/2] drm/ttm: Only allocate huge pages with new flag TTM_PAGE_FLAG_TRANSHUGE

[Michel Dänzer]

On 2018-04-28 06:30 PM, Ilia Mirkin wrote:
> On Fri, Apr 27, 2018 at 9:08 AM, Michel Dänzer  wrote:
>> From: Michel Dänzer 
>>
>> Previously, TTM would always (with CONFIG_TRANSPARENT_HUGEPAGE enabled)
>> try to allocate huge pages. However, not all drivers can take advantage
>> of huge pages, but they would incur the overhead for allocating and
>> freeing them anyway.
>>
>> Now, drivers which can take advantage of huge pages need to set the new
>> flag TTM_PAGE_FLAG_TRANSHUGE to get them. Drivers not setting this flag
>> no longer incur any overhead for allocating or freeing huge pages.
>>
>> v2:
>> * Also guard swapping of consecutive pages in ttm_get_pages
>> * Reword commit log, hopefully clearer now
>>
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Michel Dänzer 
> 
> Both I and lots of other people, based on reports, are still seeing
> plenty of issues with this as late as 4.16.4.

"lots of other people", "plenty of issues" sounds a bit exaggerated from
what I've seen. FWIW, while I did see the original messages myself, I
haven't seen any since Christian's original fix (see below), neither
with amdgpu nor radeon, even before this patch you followed up to.


> Admittedly I'm on nouveau, but others have reported issues with
> radeon/amdgpu as well. It's been going on since the feature was merged
> in v4.15, with what seems like little investigation from the authors
> introducing the feature.

That's not a fair assessment. See
https://bugs.freedesktop.org/show_bug.cgi?id=104082#c40 and following
comments.

Christian fixed the original issue in
d0bc0c2a31c95002d37c3cc511ffdcab851b3256 "swiotlb: suppress warning when
__GFP_NOWARN is set". Christian did his best to try and get the fix in
before 4.15 final, but for reasons beyond his control, it was delayed
until 4.16-rc1 and then backported to 4.15.5.

Unfortunately, there was an swiotlb regression (not directly related to
Christian's work) shortly after this fix, also in 4.16-rc1, which is now
fixed in 4.17-rc1 and will be backported to 4.16.y.

It looks like there's at least one more bug left, but it's not clear yet
when that was introduced, whether it's directly related to Christian's
work, or indeed what the impact is. Let's not get ahead of ourselves.


> We now have *two* broken releases, v4.15 and v4.16 (anything that
> spews error messages and stack traces ad-infinitum in dmesg is, by
> definition, broken).

I haven't seen any evidence that there's still an issue in 4.15, is
there any?


> You're putting this behind a flag now (finally),

I wrote this patch because I realized due to some remark I happened to
see you make this week on IRC that the huge page support in TTM was
enabled for all drivers. Instead of making that kind of remark on IRC,
it would have been more constructive, and more conducive to quick
implementation, to suggest making the feature not active for drivers
which don't need it in a mailing list post.


At least, please do more research before making this kind of negative
post.

P.S. You might also want to look into whether nouveau really should be
hitting swiotlb in these cases.

-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

[PATCH v2] spi: meson-spicc: Fix error handling in meson_spicc_probe()

[Alexey Khoroshilov]

If devm_spi_register_master() fails in meson_spicc_probe(),
spicc->core is left undisabled. The patch fixes that.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
Reviewed-by: Neil Armstrong 
---
v2: Fix subject as Neil Armstrong noted.

 drivers/spi/spi-meson-spicc.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-meson-spicc.c b/drivers/spi/spi-meson-spicc.c
index 5c82910e3480..7fe4488ace57 100644
--- a/drivers/spi/spi-meson-spicc.c
+++ b/drivers/spi/spi-meson-spicc.c
@@ -574,10 +574,15 @@ static int meson_spicc_probe(struct platform_device *pdev)
master->max_speed_hz = rate >> 2;
 
ret = devm_spi_register_master(>dev, master);
-   if (!ret)
-   return 0;
+   if (ret) {
+   dev_err(>dev, "spi master registration failed\n");
+   goto out_clk;
+   }
 
-   dev_err(>dev, "spi master registration failed\n");
+   return 0;
+
+out_clk:
+   clk_disable_unprepare(spicc->core);
 
 out_master:
spi_master_put(master);
-- 
2.7.4

[PATCH v2] spi: meson-spicc: Fix error handling in meson_spicc_probe()

[Alexey Khoroshilov]

If devm_spi_register_master() fails in meson_spicc_probe(),
spicc->core is left undisabled. The patch fixes that.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
Reviewed-by: Neil Armstrong 
---
v2: Fix subject as Neil Armstrong noted.

 drivers/spi/spi-meson-spicc.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-meson-spicc.c b/drivers/spi/spi-meson-spicc.c
index 5c82910e3480..7fe4488ace57 100644
--- a/drivers/spi/spi-meson-spicc.c
+++ b/drivers/spi/spi-meson-spicc.c
@@ -574,10 +574,15 @@ static int meson_spicc_probe(struct platform_device *pdev)
master->max_speed_hz = rate >> 2;
 
ret = devm_spi_register_master(>dev, master);
-   if (!ret)
-   return 0;
+   if (ret) {
+   dev_err(>dev, "spi master registration failed\n");
+   goto out_clk;
+   }
 
-   dev_err(>dev, "spi master registration failed\n");
+   return 0;
+
+out_clk:
+   clk_disable_unprepare(spicc->core);
 
 out_master:
spi_master_put(master);
-- 
2.7.4

[PATCH] power: supply: ltc2941-battery-gauge: Release device_node in ltc294x_i2c_probe()

[Alexey Khoroshilov]

There is of_node_get(client->dev.of_node) in ltc294x_i2c_probe(),
but these is no of_node_put() somethere in the driver.

The patch adds one on error and normal paths.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/power/supply/ltc2941-battery-gauge.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/ltc2941-battery-gauge.c 
b/drivers/power/supply/ltc2941-battery-gauge.c
index 4f129bb4c972..7854a89b3332 100644
--- a/drivers/power/supply/ltc2941-battery-gauge.c
+++ b/drivers/power/supply/ltc2941-battery-gauge.c
@@ -481,7 +481,7 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(>dev,
"Could not find lltc,resistor-sense in devicetree\n");
-   return ret;
+   goto err_node_put;
}
info->r_sense = r_sense;
 
@@ -511,7 +511,7 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(>dev,
"Could not read status register\n");
-   return ret;
+   goto err_node_put;
}
if (status & LTC2941_REG_STATUS_CHIP_ID)
info->id = LTC2941_ID;
@@ -550,19 +550,25 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
ret = ltc294x_reset(info, prescaler_exp);
if (ret < 0) {
dev_err(>dev, "Communication with chip failed\n");
-   return ret;
+   goto err_node_put;
}
 
info->supply = power_supply_register(>dev, >supply_desc,
 _cfg);
if (IS_ERR(info->supply)) {
dev_err(>dev, "failed to register ltc2941\n");
-   return PTR_ERR(info->supply);
+   ret = PTR_ERR(info->supply);
+   goto err_node_put;
} else {
schedule_delayed_work(>work, LTC294X_WORK_DELAY * HZ);
}
 
+   of_node_put(np);
return 0;
+
+err_node_put:
+   of_node_put(np);
+   return ret;
 }
 
 static void ltc294x_i2c_shutdown(struct i2c_client *client)
-- 
2.7.4

[PATCH] power: supply: ltc2941-battery-gauge: Release device_node in ltc294x_i2c_probe()

[Alexey Khoroshilov]

There is of_node_get(client->dev.of_node) in ltc294x_i2c_probe(),
but these is no of_node_put() somethere in the driver.

The patch adds one on error and normal paths.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/power/supply/ltc2941-battery-gauge.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/ltc2941-battery-gauge.c 
b/drivers/power/supply/ltc2941-battery-gauge.c
index 4f129bb4c972..7854a89b3332 100644
--- a/drivers/power/supply/ltc2941-battery-gauge.c
+++ b/drivers/power/supply/ltc2941-battery-gauge.c
@@ -481,7 +481,7 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(>dev,
"Could not find lltc,resistor-sense in devicetree\n");
-   return ret;
+   goto err_node_put;
}
info->r_sense = r_sense;
 
@@ -511,7 +511,7 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(>dev,
"Could not read status register\n");
-   return ret;
+   goto err_node_put;
}
if (status & LTC2941_REG_STATUS_CHIP_ID)
info->id = LTC2941_ID;
@@ -550,19 +550,25 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
ret = ltc294x_reset(info, prescaler_exp);
if (ret < 0) {
dev_err(>dev, "Communication with chip failed\n");
-   return ret;
+   goto err_node_put;
}
 
info->supply = power_supply_register(>dev, >supply_desc,
 _cfg);
if (IS_ERR(info->supply)) {
dev_err(>dev, "failed to register ltc2941\n");
-   return PTR_ERR(info->supply);
+   ret = PTR_ERR(info->supply);
+   goto err_node_put;
} else {
schedule_delayed_work(>work, LTC294X_WORK_DELAY * HZ);
}
 
+   of_node_put(np);
return 0;
+
+err_node_put:
+   of_node_put(np);
+   return ret;
 }
 
 static void ltc294x_i2c_shutdown(struct i2c_client *client)
-- 
2.7.4

[PATCH] kbuild: "make tools/help" does not need syncconfig

[Randy Dunlap]

From: Randy Dunlap 

Fix "make tools/help" to skip "make syncconfig".

"make tools/help" currently tries to run "make syncconfig" even
though there is no .config file and one is not needed just to get
help text output.  With no .config file, make says:

*** Configuration file ".config" not found!
*** Please run some configurator (e.g. "make oldconfig" or
*** "make menuconfig" or "make xconfig").
scripts/kconfig/Makefile:40: recipe for target 'syncconfig' failed
make[2]: *** [syncconfig] Error 1
Makefile:525: recipe for target 'syncconfig' failed

and then goes on to run:
make LDFLAGS= MAKEFLAGS=" " O=/linux/linux-next-20180426 subdir=tools -C 
./tools/ help

which produces the requested help text.  However, the "syncconfig"
efforts are not needed, so skip them by making "tools/help" be part
of the no-dot-config-targets rule.

Signed-off-by: Randy Dunlap 
---
 Makefile |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-next-20180426.orig/Makefile
+++ linux-next-20180426/Makefile
@@ -224,7 +224,7 @@ clean-targets := %clean mrproper cleando
 no-dot-config-targets := $(clean-targets) \
 cscope gtags TAGS tags help% %docs check% coccicheck \
 $(version_h) headers_% archheaders archscripts \
-kernelversion %src-pkg
+kernelversion %src-pkg tools/help
 
 config-targets := 0
 mixed-targets  := 0

[PATCH] kbuild: "make tools/help" does not need syncconfig

[Randy Dunlap]

From: Randy Dunlap 

Fix "make tools/help" to skip "make syncconfig".

"make tools/help" currently tries to run "make syncconfig" even
though there is no .config file and one is not needed just to get
help text output.  With no .config file, make says:

*** Configuration file ".config" not found!
*** Please run some configurator (e.g. "make oldconfig" or
*** "make menuconfig" or "make xconfig").
scripts/kconfig/Makefile:40: recipe for target 'syncconfig' failed
make[2]: *** [syncconfig] Error 1
Makefile:525: recipe for target 'syncconfig' failed

and then goes on to run:
make LDFLAGS= MAKEFLAGS=" " O=/linux/linux-next-20180426 subdir=tools -C 
./tools/ help

which produces the requested help text.  However, the "syncconfig"
efforts are not needed, so skip them by making "tools/help" be part
of the no-dot-config-targets rule.

Signed-off-by: Randy Dunlap 
---
 Makefile |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-next-20180426.orig/Makefile
+++ linux-next-20180426/Makefile
@@ -224,7 +224,7 @@ clean-targets := %clean mrproper cleando
 no-dot-config-targets := $(clean-targets) \
 cscope gtags TAGS tags help% %docs check% coccicheck \
 $(version_h) headers_% archheaders archscripts \
-kernelversion %src-pkg
+kernelversion %src-pkg tools/help
 
 config-targets := 0
 mixed-targets  := 0

[PATCH] wireless: ipw2100: fix spelling mistake: "decsribed" -> "described"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in comment and in the ord_data text

Signed-off-by: Colin Ian King 
---
 drivers/net/wireless/intel/ipw2x00/ipw2100.c | 2 +-
 drivers/net/wireless/intel/ipw2x00/ipw2100.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2100.c 
b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
index 236b52423506..7c4f550a1475 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
@@ -3732,7 +3732,7 @@ IPW2100_ORD(STAT_TX_HOST_REQUESTS, "requested Host Tx's 
(MSDU)"),
IPW2100_ORD(ASSOCIATED_AP_PTR,
"0 if not associated, else pointer to AP table 
entry"),
IPW2100_ORD(AVAILABLE_AP_CNT,
-   "AP's decsribed in the AP table"),
+   "AP's described in the AP table"),
IPW2100_ORD(AP_LIST_PTR, "Ptr to list of available APs"),
IPW2100_ORD(STAT_AP_ASSNS, "associations"),
IPW2100_ORD(STAT_ASSN_FAIL, "association failures"),
diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2100.h 
b/drivers/net/wireless/intel/ipw2x00/ipw2100.h
index 193947865efd..ce3e35f6b60f 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2100.h
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2100.h
@@ -1009,7 +1009,7 @@ typedef enum _ORDINAL_TABLE_1 {   // NS - means Not 
Supported by FW
IPW_ORD_STAT_PERCENT_RETRIES,   // current calculation of % missed tx 
retries
IPW_ORD_ASSOCIATED_AP_PTR,  // If associated, this is ptr to the 
associated
// AP table entry. set to 0 if not associated
-   IPW_ORD_AVAILABLE_AP_CNT,   // # of AP's decsribed in the AP table
+   IPW_ORD_AVAILABLE_AP_CNT,   // # of AP's described in the AP table
IPW_ORD_AP_LIST_PTR,// Ptr to list of available APs
IPW_ORD_STAT_AP_ASSNS,  // # of associations
IPW_ORD_STAT_ASSN_FAIL, // # of association failures
-- 
2.17.0

[PATCH] wireless: ipw2100: fix spelling mistake: "decsribed" -> "described"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in comment and in the ord_data text

Signed-off-by: Colin Ian King 
---
 drivers/net/wireless/intel/ipw2x00/ipw2100.c | 2 +-
 drivers/net/wireless/intel/ipw2x00/ipw2100.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2100.c 
b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
index 236b52423506..7c4f550a1475 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
@@ -3732,7 +3732,7 @@ IPW2100_ORD(STAT_TX_HOST_REQUESTS, "requested Host Tx's 
(MSDU)"),
IPW2100_ORD(ASSOCIATED_AP_PTR,
"0 if not associated, else pointer to AP table 
entry"),
IPW2100_ORD(AVAILABLE_AP_CNT,
-   "AP's decsribed in the AP table"),
+   "AP's described in the AP table"),
IPW2100_ORD(AP_LIST_PTR, "Ptr to list of available APs"),
IPW2100_ORD(STAT_AP_ASSNS, "associations"),
IPW2100_ORD(STAT_ASSN_FAIL, "association failures"),
diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2100.h 
b/drivers/net/wireless/intel/ipw2x00/ipw2100.h
index 193947865efd..ce3e35f6b60f 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2100.h
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2100.h
@@ -1009,7 +1009,7 @@ typedef enum _ORDINAL_TABLE_1 {   // NS - means Not 
Supported by FW
IPW_ORD_STAT_PERCENT_RETRIES,   // current calculation of % missed tx 
retries
IPW_ORD_ASSOCIATED_AP_PTR,  // If associated, this is ptr to the 
associated
// AP table entry. set to 0 if not associated
-   IPW_ORD_AVAILABLE_AP_CNT,   // # of AP's decsribed in the AP table
+   IPW_ORD_AVAILABLE_AP_CNT,   // # of AP's described in the AP table
IPW_ORD_AP_LIST_PTR,// Ptr to list of available APs
IPW_ORD_STAT_AP_ASSNS,  // # of associations
IPW_ORD_STAT_ASSN_FAIL, // # of association failures
-- 
2.17.0

[PATCH] drm/amd/powerplay: fix spelling mistake: "contruct" -> "construct"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in PP_ASSERT_WITH_CODE message text

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 26fbeafc3c96..726994cf03ea 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -1229,7 +1229,7 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 
tmp_result = smu7_construct_voltage_tables(hwmgr);
PP_ASSERT_WITH_CODE((0 == tmp_result),
-   "Failed to contruct voltage tables!",
+   "Failed to construct voltage tables!",
result = tmp_result);
}
smum_initialize_mc_reg_table(hwmgr);
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 7cbb56ba6fab..ced1c2aab7a9 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -2829,7 +2829,7 @@ static int vega10_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 
tmp_result = vega10_construct_voltage_tables(hwmgr);
PP_ASSERT_WITH_CODE(!tmp_result,
-   "Failed to contruct voltage tables!",
+   "Failed to construct voltage tables!",
result = tmp_result);
 
tmp_result = vega10_init_smc_table(hwmgr);
-- 
2.17.0

[PATCH] drm/amd/powerplay: fix spelling mistake: "contruct" -> "construct"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in PP_ASSERT_WITH_CODE message text

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 26fbeafc3c96..726994cf03ea 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -1229,7 +1229,7 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 
tmp_result = smu7_construct_voltage_tables(hwmgr);
PP_ASSERT_WITH_CODE((0 == tmp_result),
-   "Failed to contruct voltage tables!",
+   "Failed to construct voltage tables!",
result = tmp_result);
}
smum_initialize_mc_reg_table(hwmgr);
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 7cbb56ba6fab..ced1c2aab7a9 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -2829,7 +2829,7 @@ static int vega10_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 
tmp_result = vega10_construct_voltage_tables(hwmgr);
PP_ASSERT_WITH_CODE(!tmp_result,
-   "Failed to contruct voltage tables!",
+   "Failed to construct voltage tables!",
result = tmp_result);
 
tmp_result = vega10_init_smc_table(hwmgr);
-- 
2.17.0

[PATCH] can: cc770: fix spelling mistake: "comptibility" -> "compatibility"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in module parameter description text

Signed-off-by: Colin Ian King 
---
 drivers/net/can/cc770/cc770.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
index d4dd4da23997..da636a22c542 100644
--- a/drivers/net/can/cc770/cc770.c
+++ b/drivers/net/can/cc770/cc770.c
@@ -73,7 +73,7 @@ MODULE_PARM_DESC(msgobj15_eff, "Extended 29-bit frames for 
message object 15 "
 
 static int i82527_compat;
 module_param(i82527_compat, int, 0444);
-MODULE_PARM_DESC(i82527_compat, "Strict Intel 82527 comptibility mode "
+MODULE_PARM_DESC(i82527_compat, "Strict Intel 82527 compatibility mode "
 "without using additional functions");
 
 /*
-- 
2.17.0

[PATCH] can: cc770: fix spelling mistake: "comptibility" -> "compatibility"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in module parameter description text

Signed-off-by: Colin Ian King 
---
 drivers/net/can/cc770/cc770.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
index d4dd4da23997..da636a22c542 100644
--- a/drivers/net/can/cc770/cc770.c
+++ b/drivers/net/can/cc770/cc770.c
@@ -73,7 +73,7 @@ MODULE_PARM_DESC(msgobj15_eff, "Extended 29-bit frames for 
message object 15 "
 
 static int i82527_compat;
 module_param(i82527_compat, int, 0444);
-MODULE_PARM_DESC(i82527_compat, "Strict Intel 82527 comptibility mode "
+MODULE_PARM_DESC(i82527_compat, "Strict Intel 82527 compatibility mode "
 "without using additional functions");
 
 /*
-- 
2.17.0

[PATCH] [media] media/usbvision: fix spelling mistake: "compresion" -> "compression"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in proc text string

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/usbvision/usbvision-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index 3f87fbc80be2..7138c2b606cc 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1857,7 +1857,7 @@ int usbvision_stream_interrupt(struct usb_usbvision 
*usbvision)
 
 static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
 {
-   static const char proc[] = "usbvision_set_compresion_params: ";
+   static const char proc[] = "usbvision_set_compression_params: ";
int rc;
unsigned char *value = usbvision->ctrl_urb_buffer;
 
-- 
2.17.0

[PATCH] [media] media/usbvision: fix spelling mistake: "compresion" -> "compression"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in proc text string

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/usbvision/usbvision-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index 3f87fbc80be2..7138c2b606cc 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1857,7 +1857,7 @@ int usbvision_stream_interrupt(struct usb_usbvision 
*usbvision)
 
 static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
 {
-   static const char proc[] = "usbvision_set_compresion_params: ";
+   static const char proc[] = "usbvision_set_compression_params: ";
int rc;
unsigned char *value = usbvision->ctrl_urb_buffer;
 
-- 
2.17.0

[PATCH][V2] dmaengine: stm32-mdma: fix spelling mistake: "avalaible" -> "available"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in dev_err error message text and make
channel plural.

Signed-off-by: Colin Ian King 
---
 drivers/dma/stm32-mdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c
index 1ac775f93d9e..90d9f6bd905b 100644
--- a/drivers/dma/stm32-mdma.c
+++ b/drivers/dma/stm32-mdma.c
@@ -1521,7 +1521,7 @@ static struct dma_chan *stm32_mdma_of_xlate(struct 
of_phandle_args *dma_spec,
 
c = dma_get_any_slave_channel(>ddev);
if (!c) {
-   dev_err(mdma2dev(dmadev), "No more channel avalaible\n");
+   dev_err(mdma2dev(dmadev), "No more channels available\n");
return NULL;
}
 
-- 
2.17.0

[PATCH][V2] dmaengine: stm32-mdma: fix spelling mistake: "avalaible" -> "available"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in dev_err error message text and make
channel plural.

Signed-off-by: Colin Ian King 
---
 drivers/dma/stm32-mdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c
index 1ac775f93d9e..90d9f6bd905b 100644
--- a/drivers/dma/stm32-mdma.c
+++ b/drivers/dma/stm32-mdma.c
@@ -1521,7 +1521,7 @@ static struct dma_chan *stm32_mdma_of_xlate(struct 
of_phandle_args *dma_spec,
 
c = dma_get_any_slave_channel(>ddev);
if (!c) {
-   dev_err(mdma2dev(dmadev), "No more channel avalaible\n");
+   dev_err(mdma2dev(dmadev), "No more channels available\n");
return NULL;
}
 
-- 
2.17.0

Re: PROBLEM: error due to conflicting types during build of kselftests

[Randy Dunlap]

On 04/28/2018 02:42 PM, Jeffrin Thalakkottoor wrote:
> anyway my current is gcc (Debian 7.3.0-16) 7.3.0
> 
> may be you can try ... make -C tools/testing/selftests 2> error.txt

I have a much older gcc, so newer must be better.  :)


> On Sat, Apr 28, 2018 at 5:08 AM, Randy Dunlap  wrote:
>> On 04/20/2018 11:03 AM, Jeffrin Thalakkottoor wrote:
>>> hello,
>>>
>>> the following is the error found...
>>> ---
>>> protection_keys.c:421:5: error: conflicting types for ‘pkey_set’
>>>  int pkey_set(int pkey, unsigned long rights, unsigned long flags)
>>>  ^~~~
>>> 
>>>
>>> to reproduce this error...
>>> make -C tools/testing/selftests
>>
>> Hi,
>>
>> I can't reproduce this problem.  What is your gcc --version?
>>
>> Thanks.
>>
>>> Details about software:
>>>
>>> Linux debian 4.17.0-rc1+ #1 SMP Thu Apr 19 18:59:45 IST 2018 x86_64 
>>> GNU/Linux
>>>
>>> GNU Make 4.2.1
>>> Binutils 2.30
>>> Util-linux   2.31.1
>>> Mount2.31.1
>>> Linux C Library  2.27
>>> Dynamic linker (ldd) 2.27
>>> readlink: missing operand
>>> Try 'readlink --help' for more information.
>>> Procps   3.3.14
>>> Kbd  2.0.4
>>> Console-tools2.0.4
>>> Sh-utils 8.28
>>> Udev 238


-- 
~Randy

Re: PROBLEM: error due to conflicting types during build of kselftests

[Randy Dunlap]

On 04/28/2018 02:42 PM, Jeffrin Thalakkottoor wrote:
> anyway my current is gcc (Debian 7.3.0-16) 7.3.0
> 
> may be you can try ... make -C tools/testing/selftests 2> error.txt

I have a much older gcc, so newer must be better.  :)


> On Sat, Apr 28, 2018 at 5:08 AM, Randy Dunlap  wrote:
>> On 04/20/2018 11:03 AM, Jeffrin Thalakkottoor wrote:
>>> hello,
>>>
>>> the following is the error found...
>>> ---
>>> protection_keys.c:421:5: error: conflicting types for ‘pkey_set’
>>>  int pkey_set(int pkey, unsigned long rights, unsigned long flags)
>>>  ^~~~
>>> 
>>>
>>> to reproduce this error...
>>> make -C tools/testing/selftests
>>
>> Hi,
>>
>> I can't reproduce this problem.  What is your gcc --version?
>>
>> Thanks.
>>
>>> Details about software:
>>>
>>> Linux debian 4.17.0-rc1+ #1 SMP Thu Apr 19 18:59:45 IST 2018 x86_64 
>>> GNU/Linux
>>>
>>> GNU Make 4.2.1
>>> Binutils 2.30
>>> Util-linux   2.31.1
>>> Mount2.31.1
>>> Linux C Library  2.27
>>> Dynamic linker (ldd) 2.27
>>> readlink: missing operand
>>> Try 'readlink --help' for more information.
>>> Procps   3.3.14
>>> Kbd  2.0.4
>>> Console-tools2.0.4
>>> Sh-utils 8.28
>>> Udev 238


-- 
~Randy

[PATCH] Revert "dmaengine: pl330: add DMA_PAUSE feature"

[Frank Mori Hess]

This reverts commit 88987d2c7534a0269f567fb101e6d71a08f0f01d.

The pl330.c pause implementation violates the dmaengine requirement
for no data loss, since it relies on the DMAKILL
instruction.  However, DMAKILL discards in-flight data from the
dma controller's fifo.  This is documented in the dma-330 manual
and I have observed it with hardware doing device-to-memory burst
transfers.  The discarded data may or may not show up in the
residue count, depending on timing (resulting in data corruption
effectively).

Signed-off-by: Frank Mori Hess 
---
 drivers/dma/pl330.c | 28 
 1 file changed, 28 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 6237069001c4..f802bd3b0481 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2290,33 +2290,6 @@ static int pl330_terminate_all(struct dma_chan *chan)
return 0;
 }
 
-/*
- * We don't support DMA_RESUME command because of hardware
- * limitations, so after pausing the channel we cannot restore
- * it to active state. We have to terminate channel and setup
- * DMA transfer again. This pause feature was implemented to
- * allow safely read residue before channel termination.
- */
-static int pl330_pause(struct dma_chan *chan)
-{
-   struct dma_pl330_chan *pch = to_pchan(chan);
-   struct pl330_dmac *pl330 = pch->dmac;
-   unsigned long flags;
-
-   pm_runtime_get_sync(pl330->ddma.dev);
-   spin_lock_irqsave(>lock, flags);
-
-   spin_lock(>lock);
-   _stop(pch->thread);
-   spin_unlock(>lock);
-
-   spin_unlock_irqrestore(>lock, flags);
-   pm_runtime_mark_last_busy(pl330->ddma.dev);
-   pm_runtime_put_autosuspend(pl330->ddma.dev);
-
-   return 0;
-}
-
 static void pl330_free_chan_resources(struct dma_chan *chan)
 {
struct dma_pl330_chan *pch = to_pchan(chan);
@@ -3027,7 +3000,6 @@ pl330_probe(struct amba_device *adev, const struct 
amba_id *id)
pd->device_tx_status = pl330_tx_status;
pd->device_prep_slave_sg = pl330_prep_slave_sg;
pd->device_config = pl330_config;
-   pd->device_pause = pl330_pause;
pd->device_terminate_all = pl330_terminate_all;
pd->device_issue_pending = pl330_issue_pending;
pd->src_addr_widths = PL330_DMA_BUSWIDTHS;
-- 
2.11.0

[PATCH] Revert "dmaengine: pl330: add DMA_PAUSE feature"

[Frank Mori Hess]

This reverts commit 88987d2c7534a0269f567fb101e6d71a08f0f01d.

The pl330.c pause implementation violates the dmaengine requirement
for no data loss, since it relies on the DMAKILL
instruction.  However, DMAKILL discards in-flight data from the
dma controller's fifo.  This is documented in the dma-330 manual
and I have observed it with hardware doing device-to-memory burst
transfers.  The discarded data may or may not show up in the
residue count, depending on timing (resulting in data corruption
effectively).

Signed-off-by: Frank Mori Hess 
---
 drivers/dma/pl330.c | 28 
 1 file changed, 28 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 6237069001c4..f802bd3b0481 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2290,33 +2290,6 @@ static int pl330_terminate_all(struct dma_chan *chan)
return 0;
 }
 
-/*
- * We don't support DMA_RESUME command because of hardware
- * limitations, so after pausing the channel we cannot restore
- * it to active state. We have to terminate channel and setup
- * DMA transfer again. This pause feature was implemented to
- * allow safely read residue before channel termination.
- */
-static int pl330_pause(struct dma_chan *chan)
-{
-   struct dma_pl330_chan *pch = to_pchan(chan);
-   struct pl330_dmac *pl330 = pch->dmac;
-   unsigned long flags;
-
-   pm_runtime_get_sync(pl330->ddma.dev);
-   spin_lock_irqsave(>lock, flags);
-
-   spin_lock(>lock);
-   _stop(pch->thread);
-   spin_unlock(>lock);
-
-   spin_unlock_irqrestore(>lock, flags);
-   pm_runtime_mark_last_busy(pl330->ddma.dev);
-   pm_runtime_put_autosuspend(pl330->ddma.dev);
-
-   return 0;
-}
-
 static void pl330_free_chan_resources(struct dma_chan *chan)
 {
struct dma_pl330_chan *pch = to_pchan(chan);
@@ -3027,7 +3000,6 @@ pl330_probe(struct amba_device *adev, const struct 
amba_id *id)
pd->device_tx_status = pl330_tx_status;
pd->device_prep_slave_sg = pl330_prep_slave_sg;
pd->device_config = pl330_config;
-   pd->device_pause = pl330_pause;
pd->device_terminate_all = pl330_terminate_all;
pd->device_issue_pending = pl330_issue_pending;
pd->src_addr_widths = PL330_DMA_BUSWIDTHS;
-- 
2.11.0

Re: PROBLEM: error due to conflicting types during build of kselftests

[Jeffrin Thalakkottoor]

anyway my current is gcc (Debian 7.3.0-16) 7.3.0

may be you can try ... make -C tools/testing/selftests 2> error.txt

On Sat, Apr 28, 2018 at 5:08 AM, Randy Dunlap  wrote:
> On 04/20/2018 11:03 AM, Jeffrin Thalakkottoor wrote:
>> hello,
>>
>> the following is the error found...
>> ---
>> protection_keys.c:421:5: error: conflicting types for ‘pkey_set’
>>  int pkey_set(int pkey, unsigned long rights, unsigned long flags)
>>  ^~~~
>> 
>>
>> to reproduce this error...
>> make -C tools/testing/selftests
>
> Hi,
>
> I can't reproduce this problem.  What is your gcc --version?
>
> Thanks.
>
>> Details about software:
>>
>> Linux debian 4.17.0-rc1+ #1 SMP Thu Apr 19 18:59:45 IST 2018 x86_64 GNU/Linux
>>
>> GNU Make 4.2.1
>> Binutils 2.30
>> Util-linux   2.31.1
>> Mount2.31.1
>> Linux C Library  2.27
>> Dynamic linker (ldd) 2.27
>> readlink: missing operand
>> Try 'readlink --help' for more information.
>> Procps   3.3.14
>> Kbd  2.0.4
>> Console-tools2.0.4
>> Sh-utils 8.28
>> Udev 238
>>
>
>
> --
> ~Randy



-- 
software engineer
rajagiri school of engineering and technology

Re: PROBLEM: error due to conflicting types during build of kselftests

[Jeffrin Thalakkottoor]

anyway my current is gcc (Debian 7.3.0-16) 7.3.0

may be you can try ... make -C tools/testing/selftests 2> error.txt

On Sat, Apr 28, 2018 at 5:08 AM, Randy Dunlap  wrote:
> On 04/20/2018 11:03 AM, Jeffrin Thalakkottoor wrote:
>> hello,
>>
>> the following is the error found...
>> ---
>> protection_keys.c:421:5: error: conflicting types for ‘pkey_set’
>>  int pkey_set(int pkey, unsigned long rights, unsigned long flags)
>>  ^~~~
>> 
>>
>> to reproduce this error...
>> make -C tools/testing/selftests
>
> Hi,
>
> I can't reproduce this problem.  What is your gcc --version?
>
> Thanks.
>
>> Details about software:
>>
>> Linux debian 4.17.0-rc1+ #1 SMP Thu Apr 19 18:59:45 IST 2018 x86_64 GNU/Linux
>>
>> GNU Make 4.2.1
>> Binutils 2.30
>> Util-linux   2.31.1
>> Mount2.31.1
>> Linux C Library  2.27
>> Dynamic linker (ldd) 2.27
>> readlink: missing operand
>> Try 'readlink --help' for more information.
>> Procps   3.3.14
>> Kbd  2.0.4
>> Console-tools2.0.4
>> Sh-utils 8.28
>> Udev 238
>>
>
>
> --
> ~Randy



-- 
software engineer
rajagiri school of engineering and technology

Re: [PATCH 3.18 18/52] HID: i2c-hid: fix size check and type usage

[John Smith]

> - if (ret_size > size) {
> + if ((ret_size > size) || (ret_size <= 2)) {
> dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
> __func__, size, ret_size);
> return;


This change spams in the dmesg output. Could you please undo this change? Or

- if ((ret_size > size) || (ret_size <= 2)) {
+ if ((ret_size > size) || (ret_size < 2)) {


$ dmesg

[ 1781.266353] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.273898] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.281464] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.289064] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.296607] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.304131] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.311751] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.319315] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.326882] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)

Re: [PATCH 3.18 18/52] HID: i2c-hid: fix size check and type usage

[John Smith]

> - if (ret_size > size) {
> + if ((ret_size > size) || (ret_size <= 2)) {
> dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
> __func__, size, ret_size);
> return;


This change spams in the dmesg output. Could you please undo this change? Or

- if ((ret_size > size) || (ret_size <= 2)) {
+ if ((ret_size > size) || (ret_size < 2)) {


$ dmesg

[ 1781.266353] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.273898] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.281464] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.289064] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.296607] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.304131] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.311751] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.319315] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)
[ 1781.326882] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)

LICENSES: Missing ISC text & possibly a category ("Not recommended" vs. "Preferred licenses")

[Rafał Miłecki]

Hi,

Due to some maintainers *preferring* BSD-compatible license for DTS
files [0], I was writing mine using ISC. I had no very special reason
for it: I was choosing between BSD-2-Clause, MIT and ISC. I've chosen
ISC as I read about its "removal of language deemed unnecessary".

I took a moment to look at the new SPDX thing and noticed that:
1) File license-rules.rst provides "LICENSES/other/ISC" as an example
2) License file LICENSES/other/ISC doesn't exist
3) ISC is listed as an *example* under the "Not recommended licenses"

First of all, as ISC is used by some files in the Linux kernel, I
think it's worth adding to the LICENSE/*/ISC.

Secondly, it isn't 100% clear to me if ISC is preferred or not
recommended. File license-rules.rst suggests the later by listing it
as an example for "Not recommended". It's just an example though, so
I'm not 100% sure without seeing it in either: "preferred" or "other"
directories. Also if anyone finds it "Not recommended", can we get a
short explanation why is it so, please?

[0] https://lkml.org/lkml/2016/5/4/707

-- 
Rafał

LICENSES: Missing ISC text & possibly a category ("Not recommended" vs. "Preferred licenses")

[Rafał Miłecki]

Hi,

Due to some maintainers *preferring* BSD-compatible license for DTS
files [0], I was writing mine using ISC. I had no very special reason
for it: I was choosing between BSD-2-Clause, MIT and ISC. I've chosen
ISC as I read about its "removal of language deemed unnecessary".

I took a moment to look at the new SPDX thing and noticed that:
1) File license-rules.rst provides "LICENSES/other/ISC" as an example
2) License file LICENSES/other/ISC doesn't exist
3) ISC is listed as an *example* under the "Not recommended licenses"

First of all, as ISC is used by some files in the Linux kernel, I
think it's worth adding to the LICENSE/*/ISC.

Secondly, it isn't 100% clear to me if ISC is preferred or not
recommended. File license-rules.rst suggests the later by listing it
as an example for "Not recommended". It's just an example though, so
I'm not 100% sure without seeing it in either: "preferred" or "other"
directories. Also if anyone finds it "Not recommended", can we get a
short explanation why is it so, please?

[0] https://lkml.org/lkml/2016/5/4/707

-- 
Rafał

Re: [PATCH v2 2/2] x86/mm: implement free pmd/pte page interfaces

[Kani, Toshi]

On Sat, 2018-04-28 at 11:02 +0200, j...@8bytes.org wrote:
> On Fri, Apr 27, 2018 at 02:31:51PM +, Kani, Toshi wrote:
> > So, we can add the step 2 on top of this patch.
> >  1. Clear pud/pmd entry.
> >  2. System wide TLB flush <-- TO BE ADDED BY NEW PATCH
> >  3. Free its underlining pmd/pte page.
> 
> This still lacks the page-table synchronization and will thus not fix
> the BUG_ON being triggered.

The BUG_ON issue is specific to PAE that it syncs at the pmd level.
x86/64 does not have this issue since it syncs at the pgd or p4d level.

> > We do not need to revert this patch.  We can make the above change I
> > mentioned.
> 
> Please note that we are not in the merge window anymore and that any fix
> needs to be simple and obviously correct.

Understood.  Changing the x86/32 sync point is risky.  So, I am going to
revert the free page handling for PAE.   

Thanks,
-Toshi

Re: [PATCH v2 2/2] x86/mm: implement free pmd/pte page interfaces

[Kani, Toshi]

On Sat, 2018-04-28 at 11:02 +0200, j...@8bytes.org wrote:
> On Fri, Apr 27, 2018 at 02:31:51PM +, Kani, Toshi wrote:
> > So, we can add the step 2 on top of this patch.
> >  1. Clear pud/pmd entry.
> >  2. System wide TLB flush <-- TO BE ADDED BY NEW PATCH
> >  3. Free its underlining pmd/pte page.
> 
> This still lacks the page-table synchronization and will thus not fix
> the BUG_ON being triggered.

The BUG_ON issue is specific to PAE that it syncs at the pmd level.
x86/64 does not have this issue since it syncs at the pgd or p4d level.

> > We do not need to revert this patch.  We can make the above change I
> > mentioned.
> 
> Please note that we are not in the merge window anymore and that any fix
> needs to be simple and obviously correct.

Understood.  Changing the x86/32 sync point is risky.  So, I am going to
revert the free page handling for PAE.   

Thanks,
-Toshi

Re: [PATCH net-next v2 4/7] net: mscc: Add initial Ocelot switch support

[kbuild test robot]

Hi Alexandre,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Alexandre-Belloni/Microsemi-Ocelot-Ethernet-switch-support/20180429-024136
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/net/ethernet/mscc/ocelot_board.c:23:26: sparse: Expected ) at end of 
cast operator
   drivers/net/ethernet/mscc/ocelot_board.c:23:26: sparse: got _be32
   drivers/net/ethernet/mscc/ocelot_board.c:23:26: sparse: cast from unknown 
type
   In file included from include/linux/swab.h:5:0,
from include/uapi/linux/byteorder/little_endian.h:13,
from include/linux/byteorder/little_endian.h:5,
from arch/x86/include/uapi/asm/byteorder.h:5,
from include/asm-generic/bitops/le.h:6,
from arch/x86/include/asm/bitops.h:521,
from include/linux/bitops.h:38,
from include/linux/kernel.h:11,
from include/linux/interrupt.h:6,
from drivers/net/ethernet/mscc/ocelot_board.c:7:
   drivers/net/ethernet/mscc/ocelot_board.c: In function 'ocelot_parse_ifh':
>> drivers/net/ethernet/mscc/ocelot_board.c:23:27: error: '_be32' undeclared 
>> (first use in this function); did you mean '__be32'?
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:114:54: note: in definition of macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
 ^
>> include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
>> '__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
>> include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
>> '___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
>> drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
>> 'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:27: note: each undeclared 
identifier is reported only once for each function it appears in
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:114:54: note: in definition of macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
 ^
>> include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
>> '__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
>> include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
>> '___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
>> drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
>> 'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
>> drivers/net/ethernet/mscc/ocelot_board.c:23:33: error: expected ')' before 
>> 'ifh'
  ifh[i] = ntohl((__force _be32)ifh[i]);
^
   include/uapi/linux/swab.h:114:54: note: in definition of macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
 ^
>> include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
>> '__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
>> include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
>> '___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
>> drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
>> 'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
--
   In file included from include/linux/swab.h:5:0,
from include/uapi/linux/byteorder/little_endian.h:13,
from include/linux/byteorder/little_endian.h:5,
from arch/x86/include/uapi/asm/byteorder.h:5,
from include/asm-generic/bitops/le.h:6,
from arch/x86/include/asm/bitops.h:521,
from include/linux/bitops.h:38,
from include/linux/kernel.h:11,
from include/linux/interrupt.h:6,
from drivers/net//ethernet/mscc/ocelot_board.c:7:
   drivers/net//ethernet/mscc/ocelot_board.c: In function 'ocelot_parse_ifh':
   drivers/net//ethernet/mscc/ocelot_board.c:23:27: error: '_be32' undeclared 
(first use in this function); did you mean '__be32'?
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:114:54: note: in definition of macro '__swab32'

Re: [PATCH net-next v2 4/7] net: mscc: Add initial Ocelot switch support

[kbuild test robot]

Hi Alexandre,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Alexandre-Belloni/Microsemi-Ocelot-Ethernet-switch-support/20180429-024136
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/net/ethernet/mscc/ocelot_board.c:23:26: sparse: Expected ) at end of 
cast operator
   drivers/net/ethernet/mscc/ocelot_board.c:23:26: sparse: got _be32
   drivers/net/ethernet/mscc/ocelot_board.c:23:26: sparse: cast from unknown 
type
   In file included from include/linux/swab.h:5:0,
from include/uapi/linux/byteorder/little_endian.h:13,
from include/linux/byteorder/little_endian.h:5,
from arch/x86/include/uapi/asm/byteorder.h:5,
from include/asm-generic/bitops/le.h:6,
from arch/x86/include/asm/bitops.h:521,
from include/linux/bitops.h:38,
from include/linux/kernel.h:11,
from include/linux/interrupt.h:6,
from drivers/net/ethernet/mscc/ocelot_board.c:7:
   drivers/net/ethernet/mscc/ocelot_board.c: In function 'ocelot_parse_ifh':
>> drivers/net/ethernet/mscc/ocelot_board.c:23:27: error: '_be32' undeclared 
>> (first use in this function); did you mean '__be32'?
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:114:54: note: in definition of macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
 ^
>> include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
>> '__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
>> include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
>> '___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
>> drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
>> 'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:27: note: each undeclared 
identifier is reported only once for each function it appears in
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:114:54: note: in definition of macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
 ^
>> include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
>> '__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
>> include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
>> '___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
>> drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
>> 'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
>> drivers/net/ethernet/mscc/ocelot_board.c:23:33: error: expected ')' before 
>> 'ifh'
  ifh[i] = ntohl((__force _be32)ifh[i]);
^
   include/uapi/linux/swab.h:114:54: note: in definition of macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
 ^
>> include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
>> '__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
>> include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
>> '___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
>> drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
>> 'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
--
   In file included from include/linux/swab.h:5:0,
from include/uapi/linux/byteorder/little_endian.h:13,
from include/linux/byteorder/little_endian.h:5,
from arch/x86/include/uapi/asm/byteorder.h:5,
from include/asm-generic/bitops/le.h:6,
from arch/x86/include/asm/bitops.h:521,
from include/linux/bitops.h:38,
from include/linux/kernel.h:11,
from include/linux/interrupt.h:6,
from drivers/net//ethernet/mscc/ocelot_board.c:7:
   drivers/net//ethernet/mscc/ocelot_board.c: In function 'ocelot_parse_ifh':
   drivers/net//ethernet/mscc/ocelot_board.c:23:27: error: '_be32' undeclared 
(first use in this function); did you mean '__be32'?
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:114:54: note: in definition of macro '__swab32'

Re: [LSF/MM TOPIC NOTES] x86 ZONE_DMA love

[Matthew Wilcox]

On Sat, Apr 28, 2018 at 09:46:52PM +0200, Julia Lawall wrote:
> FWIW, here is my semantic patch and the output - it reports on things that
> appear to be too small and things that it doesn't know about.
> 
> What are the relevant pci wrappers?  I didn't find them.

Basically all of the functions in include/linux/pci-dma-compat.h

> too small: drivers/gpu/drm/i915/i915_drv.c:1138: 30
> too small: drivers/net/wireless/broadcom/b43/dma.c:1068: 30
> unknown: sound/pci/ctxfi/cthw20k2.c:2033: DMA_BIT_MASK(dma_bits)
> unknown: sound/pci/ctxfi/cthw20k2.c:2034: DMA_BIT_MASK(dma_bits)

This one's good:

const unsigned int dma_bits = BITS_PER_LONG;

> unknown: drivers/scsi/megaraid/megaraid_sas_base.c:6036: consistent_mask

and this one:
consistent_mask = (instance->adapter_type == VENTURA_SERIES) ?
DMA_BIT_MASK(64) : DMA_BIT_MASK(32);

> unknown: drivers/net/wireless/ath/wil6210/txrx.c:200: 
> DMA_BIT_MASK(wil->dma_addr_size)

if (wil->dma_addr_size > 32)
dma_set_mask_and_coherent(dev,
  DMA_BIT_MASK(wil->dma_addr_size));

> unknown: drivers/net/ethernet/netronome/nfp/nfp_main.c:452: 
> DMA_BIT_MASK(NFP_NET_MAX_DMA_BITS)

drivers/net/ethernet/netronome/nfp/nfp_net.h:#define NFP_NET_MAX_DMA_BITS   
40

> unknown: drivers/gpu/host1x/dev.c:199: host->info->dma_mask

Looks safe ...

drivers/gpu/host1x/bus.c:   device->dev.coherent_dma_mask = 
host1x->dev->coherent_dma_mask;
drivers/gpu/host1x/bus.c:   device->dev.dma_mask = 
>dev.coherent_dma_mask;
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(32),
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(32),
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(34),
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(34),
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(34),
drivers/gpu/host1x/dev.c:   dma_set_mask_and_coherent(host->dev, 
host->info->dma_mask);
drivers/gpu/host1x/dev.h:   u64 dma_mask; /* mask of addressable memory */

... but that reminds us that maybe some drivers aren't using dma_set_mask()
but rather touching dma_mask directly.

... 57 more to look at ...

Re: [LSF/MM TOPIC NOTES] x86 ZONE_DMA love

[Matthew Wilcox]

On Sat, Apr 28, 2018 at 09:46:52PM +0200, Julia Lawall wrote:
> FWIW, here is my semantic patch and the output - it reports on things that
> appear to be too small and things that it doesn't know about.
> 
> What are the relevant pci wrappers?  I didn't find them.

Basically all of the functions in include/linux/pci-dma-compat.h

> too small: drivers/gpu/drm/i915/i915_drv.c:1138: 30
> too small: drivers/net/wireless/broadcom/b43/dma.c:1068: 30
> unknown: sound/pci/ctxfi/cthw20k2.c:2033: DMA_BIT_MASK(dma_bits)
> unknown: sound/pci/ctxfi/cthw20k2.c:2034: DMA_BIT_MASK(dma_bits)

This one's good:

const unsigned int dma_bits = BITS_PER_LONG;

> unknown: drivers/scsi/megaraid/megaraid_sas_base.c:6036: consistent_mask

and this one:
consistent_mask = (instance->adapter_type == VENTURA_SERIES) ?
DMA_BIT_MASK(64) : DMA_BIT_MASK(32);

> unknown: drivers/net/wireless/ath/wil6210/txrx.c:200: 
> DMA_BIT_MASK(wil->dma_addr_size)

if (wil->dma_addr_size > 32)
dma_set_mask_and_coherent(dev,
  DMA_BIT_MASK(wil->dma_addr_size));

> unknown: drivers/net/ethernet/netronome/nfp/nfp_main.c:452: 
> DMA_BIT_MASK(NFP_NET_MAX_DMA_BITS)

drivers/net/ethernet/netronome/nfp/nfp_net.h:#define NFP_NET_MAX_DMA_BITS   
40

> unknown: drivers/gpu/host1x/dev.c:199: host->info->dma_mask

Looks safe ...

drivers/gpu/host1x/bus.c:   device->dev.coherent_dma_mask = 
host1x->dev->coherent_dma_mask;
drivers/gpu/host1x/bus.c:   device->dev.dma_mask = 
>dev.coherent_dma_mask;
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(32),
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(32),
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(34),
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(34),
drivers/gpu/host1x/dev.c:   .dma_mask = DMA_BIT_MASK(34),
drivers/gpu/host1x/dev.c:   dma_set_mask_and_coherent(host->dev, 
host->info->dma_mask);
drivers/gpu/host1x/dev.h:   u64 dma_mask; /* mask of addressable memory */

... but that reminds us that maybe some drivers aren't using dma_set_mask()
but rather touching dma_mask directly.

... 57 more to look at ...

Re: [PATCH v2] efi: Ignore unrealistically large option roms

[Hans de Goede]

Hi,

On 28-04-18 08:40, Ard Biesheuvel wrote:

Hi Hans,

On 27 April 2018 at 23:35, Hans de Goede  wrote:

setup_efi_pci() tries to save a copy of each PCI option ROM as this may
be necessary for the device driver for the PCI device to have access too.

On some systems the efi_pci_io_protocol_64's romimage and romsize fields
contain invalid data, which looks a bit like pointers pointing back into
other EFI code or data. Interpreting these pointers as romsize leads to
a very large value and if we then try to alloc this amount of memory to
save a copy the alloc call fails.

This leads to a "Failed to alloc mem for rom" error being printed on the
EFI console for each PCI device.

This commit avoids the printing of these errors, by checking romsize
before doing the alloc and if it is larger then 256M silently ignore the
ROM fields instead of trying to alloc mem and fail.



The UEFI spec limits the size of option ROMs to 16 MiB, so I'd prefer
we use that as the upper bound instead.

With that changed,

Reviewed-by: Ard Biesheuvel 


Thanks for the review, fixed for v3 which I'm about to send.


or I can take it via the EFI tree if desired.


I've no preference for how this goes upstream. x86 folks, please let
us know if you will take this, or if you would prefer for this to
go upstream through the EFI tree.

Regards,

Hans

Re: [PATCH v2] efi: Ignore unrealistically large option roms

[Hans de Goede]

Hi,

On 28-04-18 08:40, Ard Biesheuvel wrote:

Hi Hans,

On 27 April 2018 at 23:35, Hans de Goede  wrote:

setup_efi_pci() tries to save a copy of each PCI option ROM as this may
be necessary for the device driver for the PCI device to have access too.

On some systems the efi_pci_io_protocol_64's romimage and romsize fields
contain invalid data, which looks a bit like pointers pointing back into
other EFI code or data. Interpreting these pointers as romsize leads to
a very large value and if we then try to alloc this amount of memory to
save a copy the alloc call fails.

This leads to a "Failed to alloc mem for rom" error being printed on the
EFI console for each PCI device.

This commit avoids the printing of these errors, by checking romsize
before doing the alloc and if it is larger then 256M silently ignore the
ROM fields instead of trying to alloc mem and fail.



The UEFI spec limits the size of option ROMs to 16 MiB, so I'd prefer
we use that as the upper bound instead.

With that changed,

Reviewed-by: Ard Biesheuvel 


Thanks for the review, fixed for v3 which I'm about to send.


or I can take it via the EFI tree if desired.


I've no preference for how this goes upstream. x86 folks, please let
us know if you will take this, or if you would prefer for this to
go upstream through the EFI tree.

Regards,

Hans

[PATCH v3] dmaengine: axi-dmac: Request IRQ with IRQF_SHARED

[Moritz Fischer]

Request IRQ with IRQF_SHARED flag to enable setups with multiple
instances of the core sharing a single IRQ line.
This works out since the IRQ handler already checks if there is
an actual IRQ pending and returns IRQ_NONE otherwise.

Acked-by: Lars-Peter Clausen 
Signed-off-by: Moritz Fischer 
---

Changes from v2:
- Reworded commit message

Changes from v1:
- Added Lars' Acked-by
- Dropped patch [1/2] and changed accordingly

---
 drivers/dma/dma-axi-dmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
index 2419fe524daa..15b2453d2647 100644
--- a/drivers/dma/dma-axi-dmac.c
+++ b/drivers/dma/dma-axi-dmac.c
@@ -687,7 +687,7 @@ static int axi_dmac_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_device;
 
-   ret = request_irq(dmac->irq, axi_dmac_interrupt_handler, 0,
+   ret = request_irq(dmac->irq, axi_dmac_interrupt_handler, IRQF_SHARED,
dev_name(>dev), dmac);
if (ret)
goto err_unregister_of;
-- 
2.17.0

[PATCH v3] dmaengine: axi-dmac: Request IRQ with IRQF_SHARED

[Moritz Fischer]

Request IRQ with IRQF_SHARED flag to enable setups with multiple
instances of the core sharing a single IRQ line.
This works out since the IRQ handler already checks if there is
an actual IRQ pending and returns IRQ_NONE otherwise.

Acked-by: Lars-Peter Clausen 
Signed-off-by: Moritz Fischer 
---

Changes from v2:
- Reworded commit message

Changes from v1:
- Added Lars' Acked-by
- Dropped patch [1/2] and changed accordingly

---
 drivers/dma/dma-axi-dmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
index 2419fe524daa..15b2453d2647 100644
--- a/drivers/dma/dma-axi-dmac.c
+++ b/drivers/dma/dma-axi-dmac.c
@@ -687,7 +687,7 @@ static int axi_dmac_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_device;
 
-   ret = request_irq(dmac->irq, axi_dmac_interrupt_handler, 0,
+   ret = request_irq(dmac->irq, axi_dmac_interrupt_handler, IRQF_SHARED,
dev_name(>dev), dmac);
if (ret)
goto err_unregister_of;
-- 
2.17.0

Re: [greybus-dev] [PATCH v2] staging: greybus: Use gpio_is_valid()

[Alex Elder]

On 04/27/2018 11:35 PM, Arvind Yadav wrote:
> Replace the manual validity checks for the GPIO with the
> gpio_is_valid().
> 
> Signed-off-by: Arvind Yadav 

Looks good.

Reviewed-by: Alex Elder 

> ---
> chnage in v2 :
>  Returning invalid gpio as error instead of -ENODEV.
> 
>  drivers/staging/greybus/arche-platform.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/arche-platform.c 
> b/drivers/staging/greybus/arche-platform.c
> index 83254a7..c3a7da5 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -448,7 +448,7 @@ static int arche_platform_probe(struct platform_device 
> *pdev)
>   arche_pdata->svc_reset_gpio = of_get_named_gpio(np,
>   "svc,reset-gpio",
>   0);
> - if (arche_pdata->svc_reset_gpio < 0) {
> + if (!gpio_is_valid(arche_pdata->svc_reset_gpio)) {
>   dev_err(dev, "failed to get reset-gpio\n");
>   return arche_pdata->svc_reset_gpio;
>   }
> @@ -468,7 +468,7 @@ static int arche_platform_probe(struct platform_device 
> *pdev)
>   arche_pdata->svc_sysboot_gpio = of_get_named_gpio(np,
> "svc,sysboot-gpio",
> 0);
> - if (arche_pdata->svc_sysboot_gpio < 0) {
> + if (!gpio_is_valid(arche_pdata->svc_sysboot_gpio)) {
>   dev_err(dev, "failed to get sysboot gpio\n");
>   return arche_pdata->svc_sysboot_gpio;
>   }
> @@ -487,7 +487,7 @@ static int arche_platform_probe(struct platform_device 
> *pdev)
>   arche_pdata->svc_refclk_req = of_get_named_gpio(np,
>   "svc,refclk-req-gpio",
>   0);
> - if (arche_pdata->svc_refclk_req < 0) {
> + if (!gpio_is_valid(arche_pdata->svc_refclk_req)) {
>   dev_err(dev, "failed to get svc clock-req gpio\n");
>   return arche_pdata->svc_refclk_req;
>   }
> 

Re: [greybus-dev] [PATCH v2] staging: greybus: Use gpio_is_valid()

[Alex Elder]

On 04/27/2018 11:35 PM, Arvind Yadav wrote:
> Replace the manual validity checks for the GPIO with the
> gpio_is_valid().
> 
> Signed-off-by: Arvind Yadav 

Looks good.

Reviewed-by: Alex Elder 

> ---
> chnage in v2 :
>  Returning invalid gpio as error instead of -ENODEV.
> 
>  drivers/staging/greybus/arche-platform.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/arche-platform.c 
> b/drivers/staging/greybus/arche-platform.c
> index 83254a7..c3a7da5 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -448,7 +448,7 @@ static int arche_platform_probe(struct platform_device 
> *pdev)
>   arche_pdata->svc_reset_gpio = of_get_named_gpio(np,
>   "svc,reset-gpio",
>   0);
> - if (arche_pdata->svc_reset_gpio < 0) {
> + if (!gpio_is_valid(arche_pdata->svc_reset_gpio)) {
>   dev_err(dev, "failed to get reset-gpio\n");
>   return arche_pdata->svc_reset_gpio;
>   }
> @@ -468,7 +468,7 @@ static int arche_platform_probe(struct platform_device 
> *pdev)
>   arche_pdata->svc_sysboot_gpio = of_get_named_gpio(np,
> "svc,sysboot-gpio",
> 0);
> - if (arche_pdata->svc_sysboot_gpio < 0) {
> + if (!gpio_is_valid(arche_pdata->svc_sysboot_gpio)) {
>   dev_err(dev, "failed to get sysboot gpio\n");
>   return arche_pdata->svc_sysboot_gpio;
>   }
> @@ -487,7 +487,7 @@ static int arche_platform_probe(struct platform_device 
> *pdev)
>   arche_pdata->svc_refclk_req = of_get_named_gpio(np,
>   "svc,refclk-req-gpio",
>   0);
> - if (arche_pdata->svc_refclk_req < 0) {
> + if (!gpio_is_valid(arche_pdata->svc_refclk_req)) {
>   dev_err(dev, "failed to get svc clock-req gpio\n");
>   return arche_pdata->svc_refclk_req;
>   }
> 

[PATCH] ext4: add crc32c soft dependency

[Dmitry V. Levin]

Starting with commit a45403b51582, ext4 unconditionally requires
a crc32c provider, which broke certain mkinitrd setups.

Add a soft dependency on crc32c so that mkinitrd, dracut, and other
initrd image generators could include the required helper module.

The dependency on crc32c is quite hard but there seems to be no
mechanism to specify a dependency on an alias, so let's use this
soft dependency as a workaround.

Suggested-by: Alexey Gladkov 
Fixes: a45403b51582 ("ext4: always initialize the crc32c checksum driver")
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry V. Levin 
---
 fs/ext4/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 185f7e6..eb104e8 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5886,5 +5886,6 @@ static void __exit ext4_exit_fs(void)
 MODULE_AUTHOR("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, 
Theodore Ts'o and others");
 MODULE_DESCRIPTION("Fourth Extended Filesystem");
 MODULE_LICENSE("GPL");
+MODULE_SOFTDEP("pre: crc32c");
 module_init(ext4_init_fs)
 module_exit(ext4_exit_fs)

-- 
ldv

[PATCH] ext4: add crc32c soft dependency

[Dmitry V. Levin]

Starting with commit a45403b51582, ext4 unconditionally requires
a crc32c provider, which broke certain mkinitrd setups.

Add a soft dependency on crc32c so that mkinitrd, dracut, and other
initrd image generators could include the required helper module.

The dependency on crc32c is quite hard but there seems to be no
mechanism to specify a dependency on an alias, so let's use this
soft dependency as a workaround.

Suggested-by: Alexey Gladkov 
Fixes: a45403b51582 ("ext4: always initialize the crc32c checksum driver")
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry V. Levin 
---
 fs/ext4/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 185f7e6..eb104e8 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5886,5 +5886,6 @@ static void __exit ext4_exit_fs(void)
 MODULE_AUTHOR("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, 
Theodore Ts'o and others");
 MODULE_DESCRIPTION("Fourth Extended Filesystem");
 MODULE_LICENSE("GPL");
+MODULE_SOFTDEP("pre: crc32c");
 module_init(ext4_init_fs)
 module_exit(ext4_exit_fs)

-- 
ldv

Re: [PATCH] thunderbolt: Handle NULL boot ACL entries properly

[Yehezkel Bernat]

On Fri, Apr 27, 2018 at 6:58 PM Mika Westerberg <
mika.westerb...@linux.intel.com> wrote:

> If the boot ACL entry is already NULL we should not fill in the upper
> two DWs with 0xf. Otherwise they are not shown as empty entries
> when the sysfs attribute is read.
> Fixes: 9aaa3b8b4c56 ("thunderbolt: Add support for preboot ACL")
> Signed-off-by: Mika Westerberg 
> ---
>drivers/thunderbolt/icm.c | 2 +-
>1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
> index 2d2ceda9aa26..aec6a82989d8 100644
> --- a/drivers/thunderbolt/icm.c
> +++ b/drivers/thunderbolt/icm.c
> @@ -1255,7 +1255,7 @@ static int icm_ar_get_boot_acl(struct tb *tb,
uuid_t *uuids, size_t nuuids)
>   /* Map empty entries to null UUID */
>   uuid[0] = 0;
>   uuid[1] = 0;
> -   } else {
> +   } else if (uuid[0] != 0 && uuid[1] != 0) {

Maybe || instead of &&?
I'd think it's enough for one of them to be non-zero to be a valid UUID.

>   /* Upper two DWs are always one's */
>   uuid[2] = 0x;
>   uuid[3] = 0x;
> --
> 2.17.0

Re: [PATCH] thunderbolt: Handle NULL boot ACL entries properly

[Yehezkel Bernat]

On Fri, Apr 27, 2018 at 6:58 PM Mika Westerberg <
mika.westerb...@linux.intel.com> wrote:

> If the boot ACL entry is already NULL we should not fill in the upper
> two DWs with 0xf. Otherwise they are not shown as empty entries
> when the sysfs attribute is read.
> Fixes: 9aaa3b8b4c56 ("thunderbolt: Add support for preboot ACL")
> Signed-off-by: Mika Westerberg 
> ---
>drivers/thunderbolt/icm.c | 2 +-
>1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
> index 2d2ceda9aa26..aec6a82989d8 100644
> --- a/drivers/thunderbolt/icm.c
> +++ b/drivers/thunderbolt/icm.c
> @@ -1255,7 +1255,7 @@ static int icm_ar_get_boot_acl(struct tb *tb,
uuid_t *uuids, size_t nuuids)
>   /* Map empty entries to null UUID */
>   uuid[0] = 0;
>   uuid[1] = 0;
> -   } else {
> +   } else if (uuid[0] != 0 && uuid[1] != 0) {

Maybe || instead of &&?
I'd think it's enough for one of them to be non-zero to be a valid UUID.

>   /* Upper two DWs are always one's */
>   uuid[2] = 0x;
>   uuid[3] = 0x;
> --
> 2.17.0

Re: [LSF/MM TOPIC NOTES] x86 ZONE_DMA love

[Julia Lawall]

On Sat, 28 Apr 2018, Luis R. Rodriguez wrote:

> On Sat, Apr 28, 2018 at 01:42:21AM -0700, Christoph Hellwig wrote:
> > On Fri, Apr 27, 2018 at 04:14:56PM +, Luis R. Rodriguez wrote:
> > > Do we have a list of users for x86 with a small DMA mask?
> > > Or, given that I'm not aware of a tool to be able to look
> > > for this in an easy way, would it be good to find out which
> > > x86 drivers do have a small mask?
> >
> > Basically you'll have to grep for calls to dma_set_mask/
> > dma_set_coherent_mask/dma_set_mask_and_coherent and their pci_*
> > wrappers with masks smaller 32-bit.  Some use numeric values,
> > some use DMA_BIT_MASK and various places uses local variables
> > or struct members to parse them, so finding them will be a bit
> > more work.  Nothing a coccinelle expert couldn't solve, though :)
>
> Thing is unless we have a specific flag used consistently I don't believe we
> can do this search with Coccinelle. ie, if we have local variables and based 
> on
> some series of variables things are set, this makes the grammatical expression
> difficult to express.  So Cocinelle is not designed for this purpose.
>
> But I believe smatch [0] is intended exactly for this sort of purpose, is that
> right Dan? I gave a cursory look and I think it'd take me significant time to
> get such hunt down.
>
> [0] https://lwn.net/Articles/691882/

FWIW, here is my semantic patch and the output - it reports on things that
appear to be too small and things that it doesn't know about.

What are the relevant pci wrappers?  I didn't find them.

julia

@initialize:ocaml@
@@

let clean s = String.concat "" (Str.split (Str.regexp " ") s)

let shorten s = List.nth (Str.split (Str.regexp "linux-next/") s) 1

@bad1 exists@
identifier i,x;
expression e;
position p;
@@

x = DMA_BIT_MASK(i)
...
\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)(e,x)

@bad2@
identifier i;
expression e;
position p;
@@

\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)
   (e,DMA_BIT_MASK(i))

@ok1 exists@
identifier x;
expression e;
constant c;
position p != bad1.p;
@@

x = \(DMA_BIT_MASK(c)\|0x\)
...
\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)(e,x)

@script:ocaml@
p << ok1.p;
c << ok1.c;
@@

let c = int_of_string c in
if c < 32
then
  let p = List.hd p in
  Printf.printf "too small: %s:%d: %d\n" (shorten p.file) p.line c

@ok2@
expression e;
constant c;
position p != bad2.p;
@@

\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)
   (e,\(DMA_BIT_MASK(c)\|0x\))

@script:ocaml@
p << ok2.p;
c << ok2.c;
@@

let c = int_of_string c in
if c < 32
then
  let p = List.hd p in
  Printf.printf "too small: %s:%d: %d\n" (shorten p.file) p.line c

@unk@
expression e,e1 != ATA_DMA_MASK;
position p != {ok1.p,ok2.p};
@@

\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)(e,e1)

@script:ocaml@
p << unk.p;
e1 << unk.e1;
@@

let p = List.hd p in
Printf.printf "unknown: %s:%d: %s\n" (shorten p.file) p.line (clean e1)

-

too small: drivers/gpu/drm/i915/i915_drv.c:1138: 30
too small: drivers/net/wireless/broadcom/b43/dma.c:1068: 30
unknown: sound/pci/ctxfi/cthw20k2.c:2033: DMA_BIT_MASK(dma_bits)
unknown: sound/pci/ctxfi/cthw20k2.c:2034: DMA_BIT_MASK(dma_bits)
unknown: drivers/scsi/megaraid/megaraid_sas_base.c:6036: consistent_mask
unknown: drivers/net/wireless/ath/wil6210/txrx.c:200: 
DMA_BIT_MASK(wil->dma_addr_size)
unknown: drivers/net/ethernet/netronome/nfp/nfp_main.c:452: 
DMA_BIT_MASK(NFP_NET_MAX_DMA_BITS)
unknown: drivers/gpu/host1x/dev.c:199: host->info->dma_mask
unknown: drivers/iommu/arm-smmu-v3.c:2691: DMA_BIT_MASK(smmu->oas)
too small: sound/pci/es1968.c:2692: 28
too small: sound/pci/es1968.c:2693: 28
too small: drivers/net/wireless/broadcom/b43legacy/dma.c:809: 30
unknown: drivers/virtio/virtio_mmio.c:573: DMA_BIT_MASK(32+PAGE_SHIFT)
unknown: drivers/ata/sata_nv.c:762: pp->adma_dma_mask
unknown: drivers/dma/mmp_pdma.c:1094: pdev->dev->coherent_dma_mask
too small: sound/pci/maestro3.c:2557: 28
too small: sound/pci/maestro3.c:2558: 28
too small: sound/pci/ice1712/ice1712.c:2533: 28
too small: sound/pci/ice1712/ice1712.c:2534: 28
unknown: drivers/net/wireless/ath/wil6210/pmc.c:132: 
DMA_BIT_MASK(wil->dma_addr_size)
unknown: drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c:313: 
DMA_BIT_MASK(tdev->func->iommu_bit)
unknown: drivers/net/ethernet/synopsys/dwc-xlgmac-common.c:96: 
DMA_BIT_MASK(pdata->hw_feat.dma_width)
too small: sound/pci/als4000.c:874: 24
too small: sound/pci/als4000.c:875: 24
unknown: drivers/hwtracing/coresight/coresight-tmc.c:335: DMA_BIT_MASK(dma_mask)
unknown: drivers/dma/xilinx/xilinx_dma.c:2634: DMA_BIT_MASK(addr_width)
too small: sound/pci/sonicvibes.c:1262: 24
too small: sound/pci/sonicvibes.c:1263: 24
too small: sound/pci/es1938.c:1600: 24
too small: sound/pci/es1938.c:1601: 24
unknown: drivers/crypto/ccree/cc_driver.c:260: dma_mask
unknown: sound/pci/hda/hda_intel.c:1888: 

Re: [LSF/MM TOPIC NOTES] x86 ZONE_DMA love

[Julia Lawall]

On Sat, 28 Apr 2018, Luis R. Rodriguez wrote:

> On Sat, Apr 28, 2018 at 01:42:21AM -0700, Christoph Hellwig wrote:
> > On Fri, Apr 27, 2018 at 04:14:56PM +, Luis R. Rodriguez wrote:
> > > Do we have a list of users for x86 with a small DMA mask?
> > > Or, given that I'm not aware of a tool to be able to look
> > > for this in an easy way, would it be good to find out which
> > > x86 drivers do have a small mask?
> >
> > Basically you'll have to grep for calls to dma_set_mask/
> > dma_set_coherent_mask/dma_set_mask_and_coherent and their pci_*
> > wrappers with masks smaller 32-bit.  Some use numeric values,
> > some use DMA_BIT_MASK and various places uses local variables
> > or struct members to parse them, so finding them will be a bit
> > more work.  Nothing a coccinelle expert couldn't solve, though :)
>
> Thing is unless we have a specific flag used consistently I don't believe we
> can do this search with Coccinelle. ie, if we have local variables and based 
> on
> some series of variables things are set, this makes the grammatical expression
> difficult to express.  So Cocinelle is not designed for this purpose.
>
> But I believe smatch [0] is intended exactly for this sort of purpose, is that
> right Dan? I gave a cursory look and I think it'd take me significant time to
> get such hunt down.
>
> [0] https://lwn.net/Articles/691882/

FWIW, here is my semantic patch and the output - it reports on things that
appear to be too small and things that it doesn't know about.

What are the relevant pci wrappers?  I didn't find them.

julia

@initialize:ocaml@
@@

let clean s = String.concat "" (Str.split (Str.regexp " ") s)

let shorten s = List.nth (Str.split (Str.regexp "linux-next/") s) 1

@bad1 exists@
identifier i,x;
expression e;
position p;
@@

x = DMA_BIT_MASK(i)
...
\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)(e,x)

@bad2@
identifier i;
expression e;
position p;
@@

\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)
   (e,DMA_BIT_MASK(i))

@ok1 exists@
identifier x;
expression e;
constant c;
position p != bad1.p;
@@

x = \(DMA_BIT_MASK(c)\|0x\)
...
\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)(e,x)

@script:ocaml@
p << ok1.p;
c << ok1.c;
@@

let c = int_of_string c in
if c < 32
then
  let p = List.hd p in
  Printf.printf "too small: %s:%d: %d\n" (shorten p.file) p.line c

@ok2@
expression e;
constant c;
position p != bad2.p;
@@

\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)
   (e,\(DMA_BIT_MASK(c)\|0x\))

@script:ocaml@
p << ok2.p;
c << ok2.c;
@@

let c = int_of_string c in
if c < 32
then
  let p = List.hd p in
  Printf.printf "too small: %s:%d: %d\n" (shorten p.file) p.line c

@unk@
expression e,e1 != ATA_DMA_MASK;
position p != {ok1.p,ok2.p};
@@

\(dma_set_mask@p\|dma_set_coherent_mask@p\|dma_set_mask_and_coherent@p\)(e,e1)

@script:ocaml@
p << unk.p;
e1 << unk.e1;
@@

let p = List.hd p in
Printf.printf "unknown: %s:%d: %s\n" (shorten p.file) p.line (clean e1)

-

too small: drivers/gpu/drm/i915/i915_drv.c:1138: 30
too small: drivers/net/wireless/broadcom/b43/dma.c:1068: 30
unknown: sound/pci/ctxfi/cthw20k2.c:2033: DMA_BIT_MASK(dma_bits)
unknown: sound/pci/ctxfi/cthw20k2.c:2034: DMA_BIT_MASK(dma_bits)
unknown: drivers/scsi/megaraid/megaraid_sas_base.c:6036: consistent_mask
unknown: drivers/net/wireless/ath/wil6210/txrx.c:200: 
DMA_BIT_MASK(wil->dma_addr_size)
unknown: drivers/net/ethernet/netronome/nfp/nfp_main.c:452: 
DMA_BIT_MASK(NFP_NET_MAX_DMA_BITS)
unknown: drivers/gpu/host1x/dev.c:199: host->info->dma_mask
unknown: drivers/iommu/arm-smmu-v3.c:2691: DMA_BIT_MASK(smmu->oas)
too small: sound/pci/es1968.c:2692: 28
too small: sound/pci/es1968.c:2693: 28
too small: drivers/net/wireless/broadcom/b43legacy/dma.c:809: 30
unknown: drivers/virtio/virtio_mmio.c:573: DMA_BIT_MASK(32+PAGE_SHIFT)
unknown: drivers/ata/sata_nv.c:762: pp->adma_dma_mask
unknown: drivers/dma/mmp_pdma.c:1094: pdev->dev->coherent_dma_mask
too small: sound/pci/maestro3.c:2557: 28
too small: sound/pci/maestro3.c:2558: 28
too small: sound/pci/ice1712/ice1712.c:2533: 28
too small: sound/pci/ice1712/ice1712.c:2534: 28
unknown: drivers/net/wireless/ath/wil6210/pmc.c:132: 
DMA_BIT_MASK(wil->dma_addr_size)
unknown: drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c:313: 
DMA_BIT_MASK(tdev->func->iommu_bit)
unknown: drivers/net/ethernet/synopsys/dwc-xlgmac-common.c:96: 
DMA_BIT_MASK(pdata->hw_feat.dma_width)
too small: sound/pci/als4000.c:874: 24
too small: sound/pci/als4000.c:875: 24
unknown: drivers/hwtracing/coresight/coresight-tmc.c:335: DMA_BIT_MASK(dma_mask)
unknown: drivers/dma/xilinx/xilinx_dma.c:2634: DMA_BIT_MASK(addr_width)
too small: sound/pci/sonicvibes.c:1262: 24
too small: sound/pci/sonicvibes.c:1263: 24
too small: sound/pci/es1938.c:1600: 24
too small: sound/pci/es1938.c:1601: 24
unknown: drivers/crypto/ccree/cc_driver.c:260: dma_mask
unknown: sound/pci/hda/hda_intel.c:1888: 

Re: [PATCH net-next v2 4/7] net: mscc: Add initial Ocelot switch support

[kbuild test robot]

Hi Alexandre,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Alexandre-Belloni/Microsemi-Ocelot-Ethernet-switch-support/20180429-024136
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 

All warnings (new ones prefixed by >>):

   In file included from include/linux/swab.h:5:0,
from include/uapi/linux/byteorder/little_endian.h:13,
from include/linux/byteorder/little_endian.h:5,
from arch/sh/include/uapi/asm/byteorder.h:6,
from arch/sh/include/asm/bitops.h:12,
from include/linux/bitops.h:38,
from include/linux/kernel.h:11,
from include/linux/interrupt.h:6,
from drivers/net/ethernet/mscc/ocelot_board.c:7:
   drivers/net/ethernet/mscc/ocelot_board.c: In function 'ocelot_parse_ifh':
   drivers/net/ethernet/mscc/ocelot_board.c:23:27: error: '_be32' undeclared 
(first use in this function); did you mean '__be32'?
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:117:32: note: in definition of macro '__swab32'
 (__builtin_constant_p((__u32)(x)) ? \
   ^
   include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
'__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
   include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
'___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
   drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:27: note: each undeclared 
identifier is reported only once for each function it appears in
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:117:32: note: in definition of macro '__swab32'
 (__builtin_constant_p((__u32)(x)) ? \
   ^
   include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
'__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
   include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
'___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
   drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:33: error: expected ')' before 
'ifh'
  ifh[i] = ntohl((__force _be32)ifh[i]);
^
   include/uapi/linux/swab.h:117:32: note: in definition of macro '__swab32'
 (__builtin_constant_p((__u32)(x)) ? \
   ^
   include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
'__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
   include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
'___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
   drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:33: error: expected ')' before 
'ifh'
  ifh[i] = ntohl((__force _be32)ifh[i]);
^
   include/uapi/linux/swab.h:18:12: note: in definition of macro 
'___constant_swab32'
 (((__u32)(x) & (__u32)0x00ffUL) << 24) |  \
   ^
>> include/uapi/linux/byteorder/little_endian.h:40:26: note: in expansion of 
>> macro '__swab32'
#define __be32_to_cpu(x) __swab32((__force __u32)(__be32)(x))
 ^~~~
   include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
'__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
   include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
'___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
   drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:33: error: expected ')' before 
'ifh'
  ifh[i] = ntohl((__force _be32)ifh[i]);
^
   include/uapi/linux/swab.h:19:12: note: in definition of macro 
'___constant_swab32'
 (((__u32)(x) & 

Re: [PATCH net-next v2 4/7] net: mscc: Add initial Ocelot switch support

[kbuild test robot]

Hi Alexandre,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Alexandre-Belloni/Microsemi-Ocelot-Ethernet-switch-support/20180429-024136
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 

All warnings (new ones prefixed by >>):

   In file included from include/linux/swab.h:5:0,
from include/uapi/linux/byteorder/little_endian.h:13,
from include/linux/byteorder/little_endian.h:5,
from arch/sh/include/uapi/asm/byteorder.h:6,
from arch/sh/include/asm/bitops.h:12,
from include/linux/bitops.h:38,
from include/linux/kernel.h:11,
from include/linux/interrupt.h:6,
from drivers/net/ethernet/mscc/ocelot_board.c:7:
   drivers/net/ethernet/mscc/ocelot_board.c: In function 'ocelot_parse_ifh':
   drivers/net/ethernet/mscc/ocelot_board.c:23:27: error: '_be32' undeclared 
(first use in this function); did you mean '__be32'?
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:117:32: note: in definition of macro '__swab32'
 (__builtin_constant_p((__u32)(x)) ? \
   ^
   include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
'__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
   include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
'___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
   drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:27: note: each undeclared 
identifier is reported only once for each function it appears in
  ifh[i] = ntohl((__force _be32)ifh[i]);
  ^
   include/uapi/linux/swab.h:117:32: note: in definition of macro '__swab32'
 (__builtin_constant_p((__u32)(x)) ? \
   ^
   include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
'__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
   include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
'___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
   drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:33: error: expected ')' before 
'ifh'
  ifh[i] = ntohl((__force _be32)ifh[i]);
^
   include/uapi/linux/swab.h:117:32: note: in definition of macro '__swab32'
 (__builtin_constant_p((__u32)(x)) ? \
   ^
   include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
'__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
   include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
'___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
   drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:33: error: expected ')' before 
'ifh'
  ifh[i] = ntohl((__force _be32)ifh[i]);
^
   include/uapi/linux/swab.h:18:12: note: in definition of macro 
'___constant_swab32'
 (((__u32)(x) & (__u32)0x00ffUL) << 24) |  \
   ^
>> include/uapi/linux/byteorder/little_endian.h:40:26: note: in expansion of 
>> macro '__swab32'
#define __be32_to_cpu(x) __swab32((__force __u32)(__be32)(x))
 ^~~~
   include/linux/byteorder/generic.h:136:21: note: in expansion of macro 
'__be32_to_cpu'
#define ___ntohl(x) __be32_to_cpu(x)
^
   include/linux/byteorder/generic.h:140:18: note: in expansion of macro 
'___ntohl'
#define ntohl(x) ___ntohl(x)
 ^~~~
   drivers/net/ethernet/mscc/ocelot_board.c:23:12: note: in expansion of macro 
'ntohl'
  ifh[i] = ntohl((__force _be32)ifh[i]);
   ^
   drivers/net/ethernet/mscc/ocelot_board.c:23:33: error: expected ')' before 
'ifh'
  ifh[i] = ntohl((__force _be32)ifh[i]);
^
   include/uapi/linux/swab.h:19:12: note: in definition of macro 
'___constant_swab32'
 (((__u32)(x) & 

Re: [PATCH v2] dmaengine: axi-dmac: Request IRQ with IRQF_SHARED

[Moritz Fischer]

On Sat, Apr 28, 2018 at 10:40:43AM +0530, Vinod Koul wrote:
> On Fri, Apr 27, 2018 at 05:18:29PM +0200, Lars-Peter Clausen wrote:
> > On 04/27/2018 05:15 PM, Moritz Fischer wrote:
> > > Hi Vinod,
> > > 
> > > On Fri, Apr 27, 2018 at 12:08 AM, Vinod Koul  wrote:
> > >> On Fri, Apr 27, 2018 at 08:53:39AM +0200, Lars-Peter Clausen wrote:
> > >>> On 04/27/2018 07:11 AM, Vinod Koul wrote:
> >  On Thu, Apr 26, 2018 at 10:40:00AM -0700, Moritz Fischer wrote:
> > > Request IRQ with IRQF_SHARED flag. This works since the interrupt
> > > handler already checks if there is an actual IRQ pending and returns
> > > IRQ_NONE otherwise.
> > 
> >  hmmm what are we trying to fix here? Is your device on a shared line 
> >  or not?
> > >>>
> > >>> IRQF_SHARED does not mean that the IRQ is on a shared line. It means 
> > >>> that
> > >>> the driver can handle it if the IRQ is on a shared line. Since the 
> > >>> driver
> > >>> can handle it setting the flag is a good idea since this enables 
> > >>> usecases
> > >>> where the line is shared.
> > >>
> > >> Yes that is correct indeed, but what is the motivation for the change.
> > >>
> > >> If you never expect this to be in shared environment why to do this?
> > >> Sorry but "it works" is not a good enough reason for this change, to 
> > >> enable
> > >> usecases where the line is shared is a good reason :)
> > > 
> > > Remember, this is an FPGA soft core. I happen to have a design [1] where 
> > > it
> > > is hooked up with multiple of them on one IRQ line, so to make this work,
> > > I need this change.
> > 
> > I think what Vinod is asking for is a change to the commit message saying
> > that "this change enables the driver to be used with devices where the
> > interrupt line is shared".
> 
> Correct, changelog need to reflect why a change was made, down the line
> people need to know the reasons, sometimes it might be even you..

Ok, communication is hard :) Will resubmit with fixed commit message,

Thanks for your feedback,

Moritz

Re: [PATCH v2] dmaengine: axi-dmac: Request IRQ with IRQF_SHARED

[Moritz Fischer]

On Sat, Apr 28, 2018 at 10:40:43AM +0530, Vinod Koul wrote:
> On Fri, Apr 27, 2018 at 05:18:29PM +0200, Lars-Peter Clausen wrote:
> > On 04/27/2018 05:15 PM, Moritz Fischer wrote:
> > > Hi Vinod,
> > > 
> > > On Fri, Apr 27, 2018 at 12:08 AM, Vinod Koul  wrote:
> > >> On Fri, Apr 27, 2018 at 08:53:39AM +0200, Lars-Peter Clausen wrote:
> > >>> On 04/27/2018 07:11 AM, Vinod Koul wrote:
> >  On Thu, Apr 26, 2018 at 10:40:00AM -0700, Moritz Fischer wrote:
> > > Request IRQ with IRQF_SHARED flag. This works since the interrupt
> > > handler already checks if there is an actual IRQ pending and returns
> > > IRQ_NONE otherwise.
> > 
> >  hmmm what are we trying to fix here? Is your device on a shared line 
> >  or not?
> > >>>
> > >>> IRQF_SHARED does not mean that the IRQ is on a shared line. It means 
> > >>> that
> > >>> the driver can handle it if the IRQ is on a shared line. Since the 
> > >>> driver
> > >>> can handle it setting the flag is a good idea since this enables 
> > >>> usecases
> > >>> where the line is shared.
> > >>
> > >> Yes that is correct indeed, but what is the motivation for the change.
> > >>
> > >> If you never expect this to be in shared environment why to do this?
> > >> Sorry but "it works" is not a good enough reason for this change, to 
> > >> enable
> > >> usecases where the line is shared is a good reason :)
> > > 
> > > Remember, this is an FPGA soft core. I happen to have a design [1] where 
> > > it
> > > is hooked up with multiple of them on one IRQ line, so to make this work,
> > > I need this change.
> > 
> > I think what Vinod is asking for is a change to the commit message saying
> > that "this change enables the driver to be used with devices where the
> > interrupt line is shared".
> 
> Correct, changelog need to reflect why a change was made, down the line
> people need to know the reasons, sometimes it might be even you..

Ok, communication is hard :) Will resubmit with fixed commit message,

Thanks for your feedback,

Moritz