Re: [GIT PULL] Additional firmware files for CA0132 HD-audio codec
Okay, just got a response from the guy at Creative. He said they'll try to sort it out this week. Just a heads up. :) On Wed, Oct 24, 2018 at 12:22 PM Connor McAdams wrote: > > Understood. I will see what I can do. I already had them contact > Takashi, but I will ask if they're willing to give a sign-off. If > they're having trouble, is it okay to have them contact you Josh? > > Let me know. > On Wed, Oct 24, 2018 at 9:44 AM Josh Boyer wrote: > > > > On Wed, Oct 24, 2018 at 3:37 AM Takashi Iwai wrote: > > > > > > On Wed, 10 Oct 2018 19:49:23 +0200, > > > Connor McAdams wrote: > > > > > > > > The following changes since commit > > > > c6b6265d718d118e28e1ce8f91769aa886b54c94: > > > > > > > > Merge tag 'iwlwifi-fw-2018-10-03' of > > > > git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware > > > > (2018-10-08 09:23:53 -0400) > > > > > > > > are available in the git repository at: > > > > > > > > g...@github.com:Conmanx360/linux-firmware.git > > > > > > > > for you to fetch changes up to cd8899323352e772c5989230f35b1c494e0ab196: > > > > > > > > linux-firmware: Add new firmware for Creative CA0132 HD-Audio Codec > > > > (2018-10-09 14:41:12 -0400) > > > > > > > > > > > > These are additional firmware files for CA0132 based sound cards. > > > > > > > > Creative has given me permission through email to send these to the > > > > linux-firmware repository under the same license as the previous ca0132 > > > > firmware. I asked my contact if they would sign-off on it, but they said > > > > to go ahead and submit it and that if there was any issue to contact > > > > them. > > > > > > > > If you need to contact them, or if there's more info needed, let me know > > > > and I can get see what I can do. > > > > I'd really like to see a Sign-off from someone at Creative. It's not > > that I don't trust your word that they agreed to it, but we have > > nothing to go back and verify they actually agreed to allow this to be > > redistributable, etc. > > > > josh > > > > > > > > > > Connor McAdams (1): > > > > linux-firmware: Add new firmware for Creative CA0132 HD-Audio > > > > Codec > > > > > > > > WHENCE| 2 ++ > > > > ctefx-desktop.bin | Bin 0 -> 655856 bytes > > > > ctefx-r3di.bin| Bin 0 -> 655816 bytes > > > > 3 files changed, 2 insertions(+) > > > > create mode 100644 ctefx-desktop.bin > > > > create mode 100644 ctefx-r3di.bin > > > > > > Please can anyone take a look at this pull request? > > > > > > It's mandatory for the CA0132-based cards that are now supported > > > better in the upstream kernel tree. > > > > > > > > > Thanks! > > > > > > Takashi
Re: [GIT PULL] Additional firmware files for CA0132 HD-audio codec
Okay, just got a response from the guy at Creative. He said they'll try to sort it out this week. Just a heads up. :) On Wed, Oct 24, 2018 at 12:22 PM Connor McAdams wrote: > > Understood. I will see what I can do. I already had them contact > Takashi, but I will ask if they're willing to give a sign-off. If > they're having trouble, is it okay to have them contact you Josh? > > Let me know. > On Wed, Oct 24, 2018 at 9:44 AM Josh Boyer wrote: > > > > On Wed, Oct 24, 2018 at 3:37 AM Takashi Iwai wrote: > > > > > > On Wed, 10 Oct 2018 19:49:23 +0200, > > > Connor McAdams wrote: > > > > > > > > The following changes since commit > > > > c6b6265d718d118e28e1ce8f91769aa886b54c94: > > > > > > > > Merge tag 'iwlwifi-fw-2018-10-03' of > > > > git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware > > > > (2018-10-08 09:23:53 -0400) > > > > > > > > are available in the git repository at: > > > > > > > > g...@github.com:Conmanx360/linux-firmware.git > > > > > > > > for you to fetch changes up to cd8899323352e772c5989230f35b1c494e0ab196: > > > > > > > > linux-firmware: Add new firmware for Creative CA0132 HD-Audio Codec > > > > (2018-10-09 14:41:12 -0400) > > > > > > > > > > > > These are additional firmware files for CA0132 based sound cards. > > > > > > > > Creative has given me permission through email to send these to the > > > > linux-firmware repository under the same license as the previous ca0132 > > > > firmware. I asked my contact if they would sign-off on it, but they said > > > > to go ahead and submit it and that if there was any issue to contact > > > > them. > > > > > > > > If you need to contact them, or if there's more info needed, let me know > > > > and I can get see what I can do. > > > > I'd really like to see a Sign-off from someone at Creative. It's not > > that I don't trust your word that they agreed to it, but we have > > nothing to go back and verify they actually agreed to allow this to be > > redistributable, etc. > > > > josh > > > > > > > > > > Connor McAdams (1): > > > > linux-firmware: Add new firmware for Creative CA0132 HD-Audio > > > > Codec > > > > > > > > WHENCE| 2 ++ > > > > ctefx-desktop.bin | Bin 0 -> 655856 bytes > > > > ctefx-r3di.bin| Bin 0 -> 655816 bytes > > > > 3 files changed, 2 insertions(+) > > > > create mode 100644 ctefx-desktop.bin > > > > create mode 100644 ctefx-r3di.bin > > > > > > Please can anyone take a look at this pull request? > > > > > > It's mandatory for the CA0132-based cards that are now supported > > > better in the upstream kernel tree. > > > > > > > > > Thanks! > > > > > > Takashi
Re: [PATCH v2] staging: olpc_dcon: olpc_dcon_xo_1.c: Switch to the gpio descriptor interface
Hi Nishad,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.19 next-20181102]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Nishad-Kamdar/staging-olpc_dcon-olpc_dcon_xo_1-c-Switch-to-the-gpio-descriptor-interface/20181104-041335
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
>> drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:23:2: error: expected identifier
>> before numeric constant
DCON_IRQ,
^
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:34:50: error: 'GPIO_ASIS'
undeclared here (not in a function); did you mean 'GPIOD_ASIS'?
[DCON_STAT0] = { .name = "dcon_stat0", .flags = GPIO_ASIS },
^
GPIOD_ASIS
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:37:3: error: 'DCON_LOAD'
undeclared here (not in a function); did you mean 'DCON_STAT1'?
[DCON_LOAD] = { .name = "dcon_load", .flags = GPIO_ASIS },
^
DCON_STAT1
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:37:3: error: array index in
initializer not of integer type
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:37:3: note: (near initialization
for 'gpios_asis')
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:38:3: error: 'DCON_BLANK'
undeclared here (not in a function); did you mean 'DCON_LOAD'?
[DCON_BLANK] = { .name = "dcon_blank", .flag = GPIO_ASIS },
^~
DCON_LOAD
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:38:3: error: array index in
initializer not of integer type
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:38:3: note: (near initialization
for 'gpios_asis')
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:38:42: error: 'const struct
dcon_gpio' has no member named 'flag'; did you mean 'flags'?
[DCON_BLANK] = { .name = "dcon_blank", .flag = GPIO_ASIS },
^~~~
flags
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c: In function 'dcon_init_xo_1':
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:47:26: warning: initialization
discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
struct dcon_gpio *pin = _asis[0];
^
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:42: error: dereferencing
pointer to incomplete type 'struct i2c_client'
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:42: error: request for member
'dev' in something not a structure or union
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:55: error: invalid type
argument of '->' (have 'struct dcon_gpio')
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:55: error: request for member
'name' in something not a structure or union
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:51:14: error: invalid type
argument of '->' (have 'struct dcon_gpio')
pin[i]->flags);
^~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:51:14: error: request for member
'flags' in something not a structure or union
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:29: error: passing argument 1
of 'devm_gpiod_get' from incompatible pointer type
[-Werror=incompatible-pointer-types]
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^
In file included from drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:14:0:
include/linux/gpio/consumer.h:87:55: note: expected 'struct device *' but
argument is of type 'const struct dcon_gpio (*)[1]'
struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
^~~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:49: error: passing argument 2
of 'devm_gpiod_get' from incompatible pointer type
[-Werror=incompatible-pointer-types]
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~~
In file included from drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:14:0:
include/linux/gpio/consumer.h:87:55: note: expected 'const char *' but
argument is of type 'const struct dcon_gpio *'
struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
^~~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:51:8: error: incompatible type
for argument 3 of 'devm_gpiod_get'
pin[i]->flags);
Re: [PATCH v2] staging: olpc_dcon: olpc_dcon_xo_1.c: Switch to the gpio descriptor interface
Hi Nishad,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.19 next-20181102]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Nishad-Kamdar/staging-olpc_dcon-olpc_dcon_xo_1-c-Switch-to-the-gpio-descriptor-interface/20181104-041335
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
>> drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:23:2: error: expected identifier
>> before numeric constant
DCON_IRQ,
^
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:34:50: error: 'GPIO_ASIS'
undeclared here (not in a function); did you mean 'GPIOD_ASIS'?
[DCON_STAT0] = { .name = "dcon_stat0", .flags = GPIO_ASIS },
^
GPIOD_ASIS
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:37:3: error: 'DCON_LOAD'
undeclared here (not in a function); did you mean 'DCON_STAT1'?
[DCON_LOAD] = { .name = "dcon_load", .flags = GPIO_ASIS },
^
DCON_STAT1
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:37:3: error: array index in
initializer not of integer type
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:37:3: note: (near initialization
for 'gpios_asis')
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:38:3: error: 'DCON_BLANK'
undeclared here (not in a function); did you mean 'DCON_LOAD'?
[DCON_BLANK] = { .name = "dcon_blank", .flag = GPIO_ASIS },
^~
DCON_LOAD
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:38:3: error: array index in
initializer not of integer type
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:38:3: note: (near initialization
for 'gpios_asis')
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:38:42: error: 'const struct
dcon_gpio' has no member named 'flag'; did you mean 'flags'?
[DCON_BLANK] = { .name = "dcon_blank", .flag = GPIO_ASIS },
^~~~
flags
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c: In function 'dcon_init_xo_1':
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:47:26: warning: initialization
discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
struct dcon_gpio *pin = _asis[0];
^
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:42: error: dereferencing
pointer to incomplete type 'struct i2c_client'
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:42: error: request for member
'dev' in something not a structure or union
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:55: error: invalid type
argument of '->' (have 'struct dcon_gpio')
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:55: error: request for member
'name' in something not a structure or union
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:51:14: error: invalid type
argument of '->' (have 'struct dcon_gpio')
pin[i]->flags);
^~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:51:14: error: request for member
'flags' in something not a structure or union
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:29: error: passing argument 1
of 'devm_gpiod_get' from incompatible pointer type
[-Werror=incompatible-pointer-types]
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^
In file included from drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:14:0:
include/linux/gpio/consumer.h:87:55: note: expected 'struct device *' but
argument is of type 'const struct dcon_gpio (*)[1]'
struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
^~~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:50:49: error: passing argument 2
of 'devm_gpiod_get' from incompatible pointer type
[-Werror=incompatible-pointer-types]
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~~
In file included from drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:14:0:
include/linux/gpio/consumer.h:87:55: note: expected 'const char *' but
argument is of type 'const struct dcon_gpio *'
struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
^~~
drivers/staging//olpc_dcon/olpc_dcon_xo_1.c:51:8: error: incompatible type
for argument 3 of 'devm_gpiod_get'
pin[i]->flags);
Re: [PATCH 4.19 00/24] 4.19.1-stable review
On Sat, 3 Nov 2018 at 00:06, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.19.1 release. > There are 24 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun Nov 4 18:28:10 UTC 2018. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.1-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.19.y > and the diffstat can be found below. > > thanks, > > greg k-h Results from Linaro’s test farm. No regressions on arm64, arm, x86_64 and i386. NOTE: kernel selftest version updated to 4.19 Summary kernel: 4.19.1-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.19.y git commit: 6eebea1ddae93f3931387c0672ff2d130a4888ce git describe: v4.19-25-g6eebea1ddae9 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-4.19-oe/build/v4.19-25-g6eebea1ddae9 No regressions (compared to build v4.19-25-ga3c575473f6c) and mainline.
Re: [PATCH 4.19 00/24] 4.19.1-stable review
On Sat, 3 Nov 2018 at 00:06, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.19.1 release. > There are 24 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun Nov 4 18:28:10 UTC 2018. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.1-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.19.y > and the diffstat can be found below. > > thanks, > > greg k-h Results from Linaro’s test farm. No regressions on arm64, arm, x86_64 and i386. NOTE: kernel selftest version updated to 4.19 Summary kernel: 4.19.1-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.19.y git commit: 6eebea1ddae93f3931387c0672ff2d130a4888ce git describe: v4.19-25-g6eebea1ddae9 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-4.19-oe/build/v4.19-25-g6eebea1ddae9 No regressions (compared to build v4.19-25-ga3c575473f6c) and mainline.
Re: [PATCH 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Taniya Das (2018-11-02 20:06:00)
> Hello Stephen,
>
> On 10/18/2018 5:02 AM, Stephen Boyd wrote:
> > Quoting Taniya Das (2018-10-11 04:36:01)
> >> --- a/drivers/cpufreq/Kconfig.arm
> >> +++ b/drivers/cpufreq/Kconfig.arm
> >> @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO
> >>
> >>If in doubt, say N.
> >>
> >> +config ARM_QCOM_CPUFREQ_HW
> >> + bool "QCOM CPUFreq HW driver"
> >
> > Is there any reason this can't be a module?
> >
>
> We do not have any use cases where we need to support it as module.
Ok, so it could easily be tristate then? Why not allow it?
>
> >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c
> >> b/drivers/cpufreq/qcom-cpufreq-hw.c
> >> new file mode 100644
> >> index 000..fe1c264
> >> --- /dev/null
> >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> >> @@ -0,0 +1,354 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> >> + */
[...]
> >> +
> >> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = {
> >
> > Is this going to change in the future?
> >
>
> Yes, they could change and that was the reason to introduce the offsets.
> This was discussed earlier too with Sudeep and was to add them.
>
> >> + [REG_ENABLE]= 0x0,
This is only used once? Maybe it could be removed.
> >> + [REG_LUT_TABLE] = 0x110,
And this is only used during probe to figure out the supported
frequencies. So we definitely don't need to store around the registers
after probe in an array of iomem pointers. The only one that we need
after probe is the one below.
> >> + [REG_PERF_STATE]= 0x920,
> >> +};
> >> +
> >> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
> >> +
> >> +static int
> >> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
> >> +unsigned int index)
> >> +{
> >> + struct cpufreq_qcom *c = policy->driver_data;
> >> +
> >> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
> >
> > Why can't we avoid the indirection here and store the perf_state pointer
> > in probe? Then we don't have to indirect through a table to perform the
> > register write.
> >
>
> As the offsets could change and that was the reason to add this.
With fast switching we can avoid incurring any extra instructions, so
please make another iomem pointer in the cpufreq_qcom struct just for
writing the index or if possible, just pass the iomem pointer that
points to the REG_PERF_STATE as the policy->driver_data variable here.
Then we have the address in hand without any extra load. If my
understanding is correct, we don't need to keep around anything besides
this register address anyway so we should be able to just load it and
write it immediately.
Re: [PATCH 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Taniya Das (2018-11-02 20:06:00)
> Hello Stephen,
>
> On 10/18/2018 5:02 AM, Stephen Boyd wrote:
> > Quoting Taniya Das (2018-10-11 04:36:01)
> >> --- a/drivers/cpufreq/Kconfig.arm
> >> +++ b/drivers/cpufreq/Kconfig.arm
> >> @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO
> >>
> >>If in doubt, say N.
> >>
> >> +config ARM_QCOM_CPUFREQ_HW
> >> + bool "QCOM CPUFreq HW driver"
> >
> > Is there any reason this can't be a module?
> >
>
> We do not have any use cases where we need to support it as module.
Ok, so it could easily be tristate then? Why not allow it?
>
> >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c
> >> b/drivers/cpufreq/qcom-cpufreq-hw.c
> >> new file mode 100644
> >> index 000..fe1c264
> >> --- /dev/null
> >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> >> @@ -0,0 +1,354 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> >> + */
[...]
> >> +
> >> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = {
> >
> > Is this going to change in the future?
> >
>
> Yes, they could change and that was the reason to introduce the offsets.
> This was discussed earlier too with Sudeep and was to add them.
>
> >> + [REG_ENABLE]= 0x0,
This is only used once? Maybe it could be removed.
> >> + [REG_LUT_TABLE] = 0x110,
And this is only used during probe to figure out the supported
frequencies. So we definitely don't need to store around the registers
after probe in an array of iomem pointers. The only one that we need
after probe is the one below.
> >> + [REG_PERF_STATE]= 0x920,
> >> +};
> >> +
> >> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
> >> +
> >> +static int
> >> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
> >> +unsigned int index)
> >> +{
> >> + struct cpufreq_qcom *c = policy->driver_data;
> >> +
> >> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
> >
> > Why can't we avoid the indirection here and store the perf_state pointer
> > in probe? Then we don't have to indirect through a table to perform the
> > register write.
> >
>
> As the offsets could change and that was the reason to add this.
With fast switching we can avoid incurring any extra instructions, so
please make another iomem pointer in the cpufreq_qcom struct just for
writing the index or if possible, just pass the iomem pointer that
points to the REG_PERF_STATE as the policy->driver_data variable here.
Then we have the address in hand without any extra load. If my
understanding is correct, we don't need to keep around anything besides
this register address anyway so we should be able to just load it and
write it immediately.
Re: [PATCH 4.18 000/150] 4.18.17-stable review
On Sat, 3 Nov 2018 at 00:08, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.18.17 release. > There are 150 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun Nov 4 18:28:05 UTC 2018. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.18.17-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.18.y > and the diffstat can be found below. > > thanks, > > greg k-h Results from Linaro’s test farm. No regressions on arm64, arm, x86_64 and i386. Summary kernel: 4.18.17-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.18.y git commit: 79190d1d2b752a03eced074e98b3aa2a03563370 git describe: v4.18.16-151-g79190d1d2b75 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-4.18-oe/build/v4.18.16-151-g79190d1d2b75 No regressions (compared to build v4.18.16-151-g8a21f68d3977)
Re: [PATCH 4.18 000/150] 4.18.17-stable review
On Sat, 3 Nov 2018 at 00:08, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.18.17 release. > There are 150 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun Nov 4 18:28:05 UTC 2018. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.18.17-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.18.y > and the diffstat can be found below. > > thanks, > > greg k-h Results from Linaro’s test farm. No regressions on arm64, arm, x86_64 and i386. Summary kernel: 4.18.17-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.18.y git commit: 79190d1d2b752a03eced074e98b3aa2a03563370 git describe: v4.18.16-151-g79190d1d2b75 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-4.18-oe/build/v4.18.16-151-g79190d1d2b75 No regressions (compared to build v4.18.16-151-g8a21f68d3977)
Re: [PATCH 4.14 000/143] 4.14.79-stable review
On Sat, 3 Nov 2018 at 20:34, Greg Kroah-Hartman
wrote:
>
> On Sat, Nov 03, 2018 at 07:31:42AM -0700, Guenter Roeck wrote:
> > On 11/2/18 11:33 AM, Greg Kroah-Hartman wrote:
> > > This is the start of the stable review cycle for the 4.14.79 release.
> > > There are 143 patches in this series, all will be posted as a response
> > > to this one. If anyone has any issues with these being applied, please
> > > let me know.
> > >
> > > Responses should be made by Sun Nov 4 18:27:59 UTC 2018.
> > > Anything received after that time might be too late.
> > >
> >
> > Build results:
> > total: 150 pass: 149 fail: 1
> > Failed builds:
> > xtensa:allmodconfig
> > Qemu test results:
> > total: 318 pass: 318 fail: 0\
> >
> > Build failure:
> >
> > In file included from include/linux/mlx5/port.h:36:0,
> > from include/linux/mlx5/driver.h: In function
> > ‘mlx5_get_vector_affinity_hint’:
> > include/linux/mlx5/driver.h:1208:13: error:
> > ‘struct irq_desc’ has no member named ‘affinity_hint’
> >
> > Caused by commit 19b743c448db ("net/mlx5: Fix mlx5_get_vector_affinity
> > function").
>
> Odd, this should be fixed by a later patch in the same queue, as 0 day
> also reported this.
>
> Yes, e3ca34880652 ("net/mlx5: Fix build break when CONFIG_SMP=n") in the
> 4.14 tree should resolve this. Ah, Sasha added it at the "last minute"
> after I did the -rc1 release. So this should be resolved now, I'll push
> out a -rc2 so that it can be verified...
Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64 and i386.
Summary
kernel: 4.14.79-rc2
git repo:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.14.y
git commit: b825fd9fbad594b1eb7f4ba22588e33f00bca345
git describe: v4.14.78-144-gb825fd9fbad5
Test details:
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.14-oe/build/v4.14.78-144-gb825fd9fbad5
No regressions (compared to build v4.14.78-144-g02f369a75b6e)
>
> thanks,
>
> greg k-h
Re: [PATCH 4.14 000/143] 4.14.79-stable review
On Sat, 3 Nov 2018 at 20:34, Greg Kroah-Hartman
wrote:
>
> On Sat, Nov 03, 2018 at 07:31:42AM -0700, Guenter Roeck wrote:
> > On 11/2/18 11:33 AM, Greg Kroah-Hartman wrote:
> > > This is the start of the stable review cycle for the 4.14.79 release.
> > > There are 143 patches in this series, all will be posted as a response
> > > to this one. If anyone has any issues with these being applied, please
> > > let me know.
> > >
> > > Responses should be made by Sun Nov 4 18:27:59 UTC 2018.
> > > Anything received after that time might be too late.
> > >
> >
> > Build results:
> > total: 150 pass: 149 fail: 1
> > Failed builds:
> > xtensa:allmodconfig
> > Qemu test results:
> > total: 318 pass: 318 fail: 0\
> >
> > Build failure:
> >
> > In file included from include/linux/mlx5/port.h:36:0,
> > from include/linux/mlx5/driver.h: In function
> > ‘mlx5_get_vector_affinity_hint’:
> > include/linux/mlx5/driver.h:1208:13: error:
> > ‘struct irq_desc’ has no member named ‘affinity_hint’
> >
> > Caused by commit 19b743c448db ("net/mlx5: Fix mlx5_get_vector_affinity
> > function").
>
> Odd, this should be fixed by a later patch in the same queue, as 0 day
> also reported this.
>
> Yes, e3ca34880652 ("net/mlx5: Fix build break when CONFIG_SMP=n") in the
> 4.14 tree should resolve this. Ah, Sasha added it at the "last minute"
> after I did the -rc1 release. So this should be resolved now, I'll push
> out a -rc2 so that it can be verified...
Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64 and i386.
Summary
kernel: 4.14.79-rc2
git repo:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.14.y
git commit: b825fd9fbad594b1eb7f4ba22588e33f00bca345
git describe: v4.14.78-144-gb825fd9fbad5
Test details:
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.14-oe/build/v4.14.78-144-gb825fd9fbad5
No regressions (compared to build v4.14.78-144-g02f369a75b6e)
>
> thanks,
>
> greg k-h
[GIT PULL] NTB patches for v4.20
Hello Linus, Here are a few NTB patches for v4.20. Fairly minor changes and bug fixes. Please consider pulling them. Thanks, Jon --- The following changes since commit 84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d: Linux 4.19 (2018-10-22 07:37:37 +0100) are available in the Git repository at: git://github.com/jonmason/ntb tags/ntb-4.20 for you to fetch changes up to a662315d8ad9e687fe648b6eea9bd35017f565dd: ntb: idt: Alter the driver info comments (2018-11-01 10:33:12 -0400) NTB IDT thermal changes and hook into hwmon, ntb_netdev clean-up of private struct, and a few bug fixes. Aaron Sierra (2): NTB: transport: Try harder to alloc an aligned MW buffer ntb_netdev: Simplify remove with client device drvdata Dave Jiang (1): ntb: intel: fix return value for ndev_vec_mask() Gustavo A. R. Silva (2): NTB: ntb_hw_idt: replace IS_ERR_OR_NULL with regular NULL checks ntb: ntb_transport: Mark expected switch fall-throughs Jon Mason (1): ntb_netdev: fix sleep time mismatch Serge Semin (5): ntb: idt: Set PCIe bus address to BARLIMITx ntb: idt: Alter temperature read method ntb: idt: Add basic hwmon sysfs interface ntb: idt: Discard temperature sensor IRQ handler ntb: idt: Alter the driver info comments drivers/net/ntb_netdev.c | 30 +--- drivers/ntb/hw/idt/Kconfig | 5 +- drivers/ntb/hw/idt/ntb_hw_idt.c| 327 +++-- drivers/ntb/hw/idt/ntb_hw_idt.h| 87 +- drivers/ntb/hw/intel/ntb_hw_gen1.c | 2 +- drivers/ntb/ntb_transport.c| 88 +++--- 6 files changed, 429 insertions(+), 110 deletions(-)
[GIT PULL] NTB patches for v4.20
Hello Linus, Here are a few NTB patches for v4.20. Fairly minor changes and bug fixes. Please consider pulling them. Thanks, Jon --- The following changes since commit 84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d: Linux 4.19 (2018-10-22 07:37:37 +0100) are available in the Git repository at: git://github.com/jonmason/ntb tags/ntb-4.20 for you to fetch changes up to a662315d8ad9e687fe648b6eea9bd35017f565dd: ntb: idt: Alter the driver info comments (2018-11-01 10:33:12 -0400) NTB IDT thermal changes and hook into hwmon, ntb_netdev clean-up of private struct, and a few bug fixes. Aaron Sierra (2): NTB: transport: Try harder to alloc an aligned MW buffer ntb_netdev: Simplify remove with client device drvdata Dave Jiang (1): ntb: intel: fix return value for ndev_vec_mask() Gustavo A. R. Silva (2): NTB: ntb_hw_idt: replace IS_ERR_OR_NULL with regular NULL checks ntb: ntb_transport: Mark expected switch fall-throughs Jon Mason (1): ntb_netdev: fix sleep time mismatch Serge Semin (5): ntb: idt: Set PCIe bus address to BARLIMITx ntb: idt: Alter temperature read method ntb: idt: Add basic hwmon sysfs interface ntb: idt: Discard temperature sensor IRQ handler ntb: idt: Alter the driver info comments drivers/net/ntb_netdev.c | 30 +--- drivers/ntb/hw/idt/Kconfig | 5 +- drivers/ntb/hw/idt/ntb_hw_idt.c| 327 +++-- drivers/ntb/hw/idt/ntb_hw_idt.h| 87 +- drivers/ntb/hw/intel/ntb_hw_gen1.c | 2 +- drivers/ntb/ntb_transport.c| 88 +++--- 6 files changed, 429 insertions(+), 110 deletions(-)
[PATCH v2] sched/core: Remove unnecessary check for next_task in push_{rt,dl}_task()
In push_{rt,dl}_task(), we call pick_next_pushable{_dl}_task() to
pick next_task before the check. If next_task and rq->curr are equal,
which will trigger BUG_ON() in pick_next_pushable{_dl}_task(). See
the following code in pick_next_pushable{_dl}_task().
static struct task_struct *pick_next_pushable{_dl}_task(struct rq *rq)
{
BUG_ON(task_current(rq, p));
}
The task_current() will check if task p and rq->curr are equal.
So, we can remove the unnecessary check in push_{rt,dl}_task().
Signed-off-by: Muchun Song
---
Changes in v2:
-also remove unnecessary check in deadline.c
-update commit message
---
kernel/sched/deadline.c | 5 -
kernel/sched/rt.c | 5 -
2 files changed, 10 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 470ba6b464fe..4b0189627318 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2042,11 +2042,6 @@ static int push_dl_task(struct rq *rq)
return 0;
retry:
- if (unlikely(next_task == rq->curr)) {
- WARN_ON(1);
- return 0;
- }
-
/*
* If next_task preempts rq->curr, and rq->curr
* can move away, it makes sense to just reschedule
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 481bb7aa25c5..ec097edea9ca 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1813,11 +1813,6 @@ static int push_rt_task(struct rq *rq)
return 0;
retry:
- if (unlikely(next_task == rq->curr)) {
- WARN_ON(1);
- return 0;
- }
-
/*
* It's possible that the next_task slipped in of
* higher priority than current. If that's the case
--
2.17.1
[PATCH v2] sched/core: Remove unnecessary check for next_task in push_{rt,dl}_task()
In push_{rt,dl}_task(), we call pick_next_pushable{_dl}_task() to
pick next_task before the check. If next_task and rq->curr are equal,
which will trigger BUG_ON() in pick_next_pushable{_dl}_task(). See
the following code in pick_next_pushable{_dl}_task().
static struct task_struct *pick_next_pushable{_dl}_task(struct rq *rq)
{
BUG_ON(task_current(rq, p));
}
The task_current() will check if task p and rq->curr are equal.
So, we can remove the unnecessary check in push_{rt,dl}_task().
Signed-off-by: Muchun Song
---
Changes in v2:
-also remove unnecessary check in deadline.c
-update commit message
---
kernel/sched/deadline.c | 5 -
kernel/sched/rt.c | 5 -
2 files changed, 10 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 470ba6b464fe..4b0189627318 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2042,11 +2042,6 @@ static int push_dl_task(struct rq *rq)
return 0;
retry:
- if (unlikely(next_task == rq->curr)) {
- WARN_ON(1);
- return 0;
- }
-
/*
* If next_task preempts rq->curr, and rq->curr
* can move away, it makes sense to just reschedule
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 481bb7aa25c5..ec097edea9ca 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1813,11 +1813,6 @@ static int push_rt_task(struct rq *rq)
return 0;
retry:
- if (unlikely(next_task == rq->curr)) {
- WARN_ON(1);
- return 0;
- }
-
/*
* It's possible that the next_task slipped in of
* higher priority than current. If that's the case
--
2.17.1
Re: [RFC] doc: rcu: remove note on smp_mb during synchronize_rcu
On Sat, Nov 03, 2018 at 04:22:59PM -0700, Paul E. McKenney wrote: > On Fri, Nov 02, 2018 at 10:12:26PM -0700, Joel Fernandes wrote: > > On Thu, Nov 01, 2018 at 09:13:07AM -0700, Paul E. McKenney wrote: > > > On Wed, Oct 31, 2018 at 10:00:19PM -0700, Joel Fernandes wrote: > > > > On Wed, Oct 31, 2018 at 11:17:48AM -0700, Paul E. McKenney wrote: > > > > > On Tue, Oct 30, 2018 at 06:11:19PM -0700, Joel Fernandes wrote: > > > > > > Hi Paul, > > > > > > > > > > > > On Tue, Oct 30, 2018 at 04:43:36PM -0700, Paul E. McKenney wrote: > > > > > > > On Tue, Oct 30, 2018 at 03:26:49PM -0700, Joel Fernandes wrote: > > > > > > > > Hi Paul, > > > > > > > > > > > > > > > > On Sat, Oct 27, 2018 at 09:30:46PM -0700, Joel Fernandes > > > > > > > > (Google) wrote: > > > > > > > > > As per this thread [1], it seems this smp_mb isn't needed > > > > > > > > > anymore: > > > > > > > > > "So the smp_mb() that I was trying to add doesn't need to be > > > > > > > > > there." > > > > > > > > > > > > > > > > > > So let us remove this part from the memory ordering > > > > > > > > > documentation. > > > > > > > > > > > > > > > > > > [1] https://lkml.org/lkml/2017/10/6/707 > > > > > > > > > > > > > > > > > > Signed-off-by: Joel Fernandes (Google) > > > > > > > > > > > > > > > > > > > > > > > > > I was just checking about this patch. Do you feel it is correct > > > > > > > > to remove > > > > > > > > this part from the docs? Are you satisified that a barrier > > > > > > > > isn't needed there > > > > > > > > now? Or did I miss something? > > > > > > > > > > > > > > Apologies, it got lost in the shuffle. I have now applied it > > > > > > > with a > > > > > > > bit of rework to the commit log, thank you! > > > > > > > > > > > > No worries, thanks for taking it! > > > > > > > > > > > > Just wanted to update you on my progress reading/correcting the > > > > > > docs. The > > > > > > 'Memory Ordering' is taking a bit of time so I paused that and I'm > > > > > > focusing > > > > > > on finishing all the other low hanging fruit. This activity is > > > > > > mostly during > > > > > > night hours after the baby is asleep but some times I also manage > > > > > > to sneak it > > > > > > into the day job ;-) > > > > > > > > > > If there is anything I can do to make this a more sustainable task for > > > > > you, please do not keep it a secret!!! > > > > > > > > Thanks a lot, that means a lot to me! Will do! > > > > > > > > > > BTW I do want to discuss about this smp_mb patch above with you at > > > > > > LPC if you > > > > > > had time, even though we are removing it from the documentation. I > > > > > > thought > > > > > > about it a few times, and I was not able to fully appreciate the > > > > > > need for the > > > > > > barrier (that is even assuming that complete() etc did not do the > > > > > > right > > > > > > thing). Specifically I was wondering same thing Peter said in the > > > > > > above > > > > > > thread I think that - if that rcu_read_unlock() triggered all the > > > > > > spin > > > > > > locking up the tree of nodes, then why is that locking not > > > > > > sufficient to > > > > > > prevent reads from the read-side section from bleeding out? That > > > > > > would > > > > > > prevent the reader that just unlocked from seeing anything that > > > > > > happens > > > > > > _after_ the synchronize_rcu. > > > > > > > > > > Actually, I recall an smp_mb() being added, but am not seeing it > > > > > anywhere > > > > > relevant to wait_for_completion(). So I might need to add the > > > > > smp_mb() > > > > > to synchronize_rcu() and remove the patch (retaining the typo fix). > > > > > :-/ > > > > > > > > No problem, I'm glad atleast the patch resurfaced the topic of the > > > > potential > > > > issue :-) > > > > > > And an smp_mb() is needed in Tree RCU's __wait_rcu_gp(). This is > > > because wait_for_completion() might get a "fly-by" wakeup, which would > > > mean no ordering for code naively thinking that it was ordered after a > > > grace period. > > > > > > > > The short form answer is that anything before a grace period on any > > > > > CPU > > > > > must be seen by any CPU as being before anything on any CPU after that > > > > > same grace period. This guarantee requires a rather big hammer. > > > > > > > > > > But yes, let's talk at LPC! > > > > > > > > Sounds great, looking forward to discussing this. > > > > > > Would it make sense to have an RCU-implementation BoF? > > > > > > > > > Also about GP memory ordering and RCU-tree-locking, I think you > > > > > > mentioned to > > > > > > me that the RCU reader-sections are virtually extended both forward > > > > > > and > > > > > > backward and whereever it ends, those paths do heavy-weight > > > > > > synchronization > > > > > > that should be sufficient to prevent memory ordering issues (such > > > > > > as those > > > > > > you mentioned in the Requierments document). That is exactly why we > > > > > > don't > > > > > > need
Re: [RFC] doc: rcu: remove note on smp_mb during synchronize_rcu
On Sat, Nov 03, 2018 at 04:22:59PM -0700, Paul E. McKenney wrote: > On Fri, Nov 02, 2018 at 10:12:26PM -0700, Joel Fernandes wrote: > > On Thu, Nov 01, 2018 at 09:13:07AM -0700, Paul E. McKenney wrote: > > > On Wed, Oct 31, 2018 at 10:00:19PM -0700, Joel Fernandes wrote: > > > > On Wed, Oct 31, 2018 at 11:17:48AM -0700, Paul E. McKenney wrote: > > > > > On Tue, Oct 30, 2018 at 06:11:19PM -0700, Joel Fernandes wrote: > > > > > > Hi Paul, > > > > > > > > > > > > On Tue, Oct 30, 2018 at 04:43:36PM -0700, Paul E. McKenney wrote: > > > > > > > On Tue, Oct 30, 2018 at 03:26:49PM -0700, Joel Fernandes wrote: > > > > > > > > Hi Paul, > > > > > > > > > > > > > > > > On Sat, Oct 27, 2018 at 09:30:46PM -0700, Joel Fernandes > > > > > > > > (Google) wrote: > > > > > > > > > As per this thread [1], it seems this smp_mb isn't needed > > > > > > > > > anymore: > > > > > > > > > "So the smp_mb() that I was trying to add doesn't need to be > > > > > > > > > there." > > > > > > > > > > > > > > > > > > So let us remove this part from the memory ordering > > > > > > > > > documentation. > > > > > > > > > > > > > > > > > > [1] https://lkml.org/lkml/2017/10/6/707 > > > > > > > > > > > > > > > > > > Signed-off-by: Joel Fernandes (Google) > > > > > > > > > > > > > > > > > > > > > > > > > I was just checking about this patch. Do you feel it is correct > > > > > > > > to remove > > > > > > > > this part from the docs? Are you satisified that a barrier > > > > > > > > isn't needed there > > > > > > > > now? Or did I miss something? > > > > > > > > > > > > > > Apologies, it got lost in the shuffle. I have now applied it > > > > > > > with a > > > > > > > bit of rework to the commit log, thank you! > > > > > > > > > > > > No worries, thanks for taking it! > > > > > > > > > > > > Just wanted to update you on my progress reading/correcting the > > > > > > docs. The > > > > > > 'Memory Ordering' is taking a bit of time so I paused that and I'm > > > > > > focusing > > > > > > on finishing all the other low hanging fruit. This activity is > > > > > > mostly during > > > > > > night hours after the baby is asleep but some times I also manage > > > > > > to sneak it > > > > > > into the day job ;-) > > > > > > > > > > If there is anything I can do to make this a more sustainable task for > > > > > you, please do not keep it a secret!!! > > > > > > > > Thanks a lot, that means a lot to me! Will do! > > > > > > > > > > BTW I do want to discuss about this smp_mb patch above with you at > > > > > > LPC if you > > > > > > had time, even though we are removing it from the documentation. I > > > > > > thought > > > > > > about it a few times, and I was not able to fully appreciate the > > > > > > need for the > > > > > > barrier (that is even assuming that complete() etc did not do the > > > > > > right > > > > > > thing). Specifically I was wondering same thing Peter said in the > > > > > > above > > > > > > thread I think that - if that rcu_read_unlock() triggered all the > > > > > > spin > > > > > > locking up the tree of nodes, then why is that locking not > > > > > > sufficient to > > > > > > prevent reads from the read-side section from bleeding out? That > > > > > > would > > > > > > prevent the reader that just unlocked from seeing anything that > > > > > > happens > > > > > > _after_ the synchronize_rcu. > > > > > > > > > > Actually, I recall an smp_mb() being added, but am not seeing it > > > > > anywhere > > > > > relevant to wait_for_completion(). So I might need to add the > > > > > smp_mb() > > > > > to synchronize_rcu() and remove the patch (retaining the typo fix). > > > > > :-/ > > > > > > > > No problem, I'm glad atleast the patch resurfaced the topic of the > > > > potential > > > > issue :-) > > > > > > And an smp_mb() is needed in Tree RCU's __wait_rcu_gp(). This is > > > because wait_for_completion() might get a "fly-by" wakeup, which would > > > mean no ordering for code naively thinking that it was ordered after a > > > grace period. > > > > > > > > The short form answer is that anything before a grace period on any > > > > > CPU > > > > > must be seen by any CPU as being before anything on any CPU after that > > > > > same grace period. This guarantee requires a rather big hammer. > > > > > > > > > > But yes, let's talk at LPC! > > > > > > > > Sounds great, looking forward to discussing this. > > > > > > Would it make sense to have an RCU-implementation BoF? > > > > > > > > > Also about GP memory ordering and RCU-tree-locking, I think you > > > > > > mentioned to > > > > > > me that the RCU reader-sections are virtually extended both forward > > > > > > and > > > > > > backward and whereever it ends, those paths do heavy-weight > > > > > > synchronization > > > > > > that should be sufficient to prevent memory ordering issues (such > > > > > > as those > > > > > > you mentioned in the Requierments document). That is exactly why we > > > > > > don't > > > > > > need
Re: [PATCH] net/9p: Fix iov_iter usage
On Sat, Nov 03, 2018 at 08:04:28PM -0700, Andy Lutomirski wrote: > Trying to use 9pfs causes QEMU to complain: commit 2cbfdf4df58330f6cb493500387427dae1c5551d Author: Marc Zyngier Date: Fri Nov 2 17:16:51 2018 + iov_iter: Fix 9p virtio breakage
Re: [PATCH] net/9p: Fix iov_iter usage
On Sat, Nov 03, 2018 at 08:04:28PM -0700, Andy Lutomirski wrote: > Trying to use 9pfs causes QEMU to complain: commit 2cbfdf4df58330f6cb493500387427dae1c5551d Author: Marc Zyngier Date: Fri Nov 2 17:16:51 2018 + iov_iter: Fix 9p virtio breakage
Re: [PATCH] arm64: dts: sdm845-mtp: Reserve reserved gpios
Quoting Bjorn Andersson (2018-11-02 14:45:32)
> With the introduction of commit 3edfb7bd76bd ("gpiolib: Show correct
> direction from the beginning") the gpiolib will attempt to read the
> direction of all pins, which triggers a read from protected register
> regions.
>
> The pins 0 through 3 and 81 through 84 are protected, so mark these as
> reserved.
>
> Signed-off-by: Bjorn Andersson
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH] arm64: dts: sdm845-mtp: Reserve reserved gpios
Quoting Bjorn Andersson (2018-11-02 14:45:32)
> With the introduction of commit 3edfb7bd76bd ("gpiolib: Show correct
> direction from the beginning") the gpiolib will attempt to read the
> direction of all pins, which triggers a read from protected register
> regions.
>
> The pins 0 through 3 and 81 through 84 are protected, so mark these as
> reserved.
>
> Signed-off-by: Bjorn Andersson
> ---
Reviewed-by: Stephen Boyd
[PATCH] net/9p: Fix iov_iter usage
Trying to use 9pfs causes QEMU to complain:
qemu-system-x86_64: virtio: bogus descriptor or out of resources
This happens because 9p was broken by the iov_iter refactoring because
a ! got lost. Put it back. The offending hunk was:
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 7728b0acde09..4d7d2070e9c8 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -322,7 +322,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
if (!iov_iter_count(data))
return 0;
- if (!(data->type & ITER_KVEC)) {
+ if (iov_iter_is_kvec(data)) {
Cc: David Howells
Cc: Al Viro
Fixes: 00e23707442a ("iov_iter: Use accessor function")
Signed-off-by: Andy Lutomirski
---
net/9p/trans_virtio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 91981970f542..b1d39cabf125 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -329,7 +329,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
if (!iov_iter_count(data))
return 0;
- if (iov_iter_is_kvec(data)) {
+ if (!iov_iter_is_kvec(data)) {
int n;
/*
* We allow only p9_max_pages pinned. We wait for the
--
2.17.2
[PATCH] net/9p: Fix iov_iter usage
Trying to use 9pfs causes QEMU to complain:
qemu-system-x86_64: virtio: bogus descriptor or out of resources
This happens because 9p was broken by the iov_iter refactoring because
a ! got lost. Put it back. The offending hunk was:
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 7728b0acde09..4d7d2070e9c8 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -322,7 +322,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
if (!iov_iter_count(data))
return 0;
- if (!(data->type & ITER_KVEC)) {
+ if (iov_iter_is_kvec(data)) {
Cc: David Howells
Cc: Al Viro
Fixes: 00e23707442a ("iov_iter: Use accessor function")
Signed-off-by: Andy Lutomirski
---
net/9p/trans_virtio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 91981970f542..b1d39cabf125 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -329,7 +329,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
if (!iov_iter_count(data))
return 0;
- if (iov_iter_is_kvec(data)) {
+ if (!iov_iter_is_kvec(data)) {
int n;
/*
* We allow only p9_max_pages pinned. We wait for the
--
2.17.2
Re: [PATCH v5 2/3] clk: meson: add DT documentation for emmc clock controller
Quoting Yixun Lan (2018-10-25 00:29:15)
> yes, I think the documentation need to be fixed
>
> for the final solution, we decide to make 'mmc-clkc' an independent node
> instead of being a sub-node of 'mmc', so both of them may exist in parallel..
>
> the DT part may like this:
>
> sd_emmc_c_clkc: clock-controller@7000 {
> compatible = "amlogic,axg-mmc-clkc", "syscon";
> reg = <0x0 0x7000 0x0 0x4>;
> ...
> };
>
> sd_emmc_c: mmc@7000 {
> compatible = "amlogic,axg-mmc";
> reg = <0x0 0x7000 0x0 0x800>;
> ...
> };
That's improper usage of DT. We don't want two devices at the same
register offset.
Re: [PATCH v5 2/3] clk: meson: add DT documentation for emmc clock controller
Quoting Yixun Lan (2018-10-25 00:29:15)
> yes, I think the documentation need to be fixed
>
> for the final solution, we decide to make 'mmc-clkc' an independent node
> instead of being a sub-node of 'mmc', so both of them may exist in parallel..
>
> the DT part may like this:
>
> sd_emmc_c_clkc: clock-controller@7000 {
> compatible = "amlogic,axg-mmc-clkc", "syscon";
> reg = <0x0 0x7000 0x0 0x4>;
> ...
> };
>
> sd_emmc_c: mmc@7000 {
> compatible = "amlogic,axg-mmc";
> reg = <0x0 0x7000 0x0 0x800>;
> ...
> };
That's improper usage of DT. We don't want two devices at the same
register offset.
Re: [PATCH v6 1/3] clk: meson: add emmc sub clock phase delay driver
Quoting Jianxin Pan (2018-11-01 09:30:53)
> diff --git a/drivers/clk/meson/clk-phase-delay.c
> b/drivers/clk/meson/clk-phase-delay.c
> new file mode 100644
> index 000..83e74ed
> --- /dev/null
> +++ b/drivers/clk/meson/clk-phase-delay.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Amlogic Meson MMC Sub Clock Controller Driver
> + *
> + * Copyright (c) 2017 Baylibre SAS.
> + * Author: Jerome Brunet
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Yixun Lan
> + * Author: Jianxin Pan
> + */
> +
> +#include
> +#include "clkc.h"
> +
> +static int meson_clk_phase_delay_get_phase(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_clk_phase_delay_data *ph =
> + meson_clk_get_phase_delay_data(clk);
Nitpick: Do this after declaring variables because it splits a line.
> + unsigned long period_ps, p, d;
> + int degrees;
> +
> + p = meson_parm_read(clk->map, >phase);
> + degrees = p * 360 / (1 << (ph->phase.width));
Nitpick: Remove useless parenthesis.
> +
> + period_ps = DIV_ROUND_UP((unsigned long)NSEC_PER_SEC * 1000,
Is the cast necessary?
> +clk_hw_get_rate(hw));
> +
> + d = meson_parm_read(clk->map, >delay);
> + degrees += d * ph->delay_step_ps * 360 / period_ps;
> + degrees %= 360;
> +
> + return degrees;
> +}
> +
> +static int meson_clk_phase_delay_set_phase(struct clk_hw *hw, int degrees)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_clk_phase_delay_data *ph =
> + meson_clk_get_phase_delay_data(clk);
> + unsigned long period_ps, d = 0, r;
> + u64 p;
> +
> + p = degrees % 360;
We don't allow phase to be larger than 360 so this isn't needed.
> + period_ps = DIV_ROUND_UP((unsigned long)NSEC_PER_SEC * 1000,
Drop the cast?
> +clk_hw_get_rate(hw));
> +
> + /* First compute the phase index (p), the remainder (r) is the
Nitpick: Please leave /* on it's own line.
> +* part we'll try to acheive using the delays (d).
> +*/
> + r = do_div(p, 360 / (1 << (ph->phase.width)));
Drop useless parenthesis please.
> + d = DIV_ROUND_CLOSEST(r * period_ps,
> + 360 * ph->delay_step_ps);
> + d = min(d, PMASK(ph->delay.width));
> +
> + meson_parm_write(clk->map, >phase, p);
> + meson_parm_write(clk->map, >delay, d);
> + return 0;
> +}
Re: [PATCH v6 1/3] clk: meson: add emmc sub clock phase delay driver
Quoting Jianxin Pan (2018-11-01 09:30:53)
> diff --git a/drivers/clk/meson/clk-phase-delay.c
> b/drivers/clk/meson/clk-phase-delay.c
> new file mode 100644
> index 000..83e74ed
> --- /dev/null
> +++ b/drivers/clk/meson/clk-phase-delay.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Amlogic Meson MMC Sub Clock Controller Driver
> + *
> + * Copyright (c) 2017 Baylibre SAS.
> + * Author: Jerome Brunet
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Yixun Lan
> + * Author: Jianxin Pan
> + */
> +
> +#include
> +#include "clkc.h"
> +
> +static int meson_clk_phase_delay_get_phase(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_clk_phase_delay_data *ph =
> + meson_clk_get_phase_delay_data(clk);
Nitpick: Do this after declaring variables because it splits a line.
> + unsigned long period_ps, p, d;
> + int degrees;
> +
> + p = meson_parm_read(clk->map, >phase);
> + degrees = p * 360 / (1 << (ph->phase.width));
Nitpick: Remove useless parenthesis.
> +
> + period_ps = DIV_ROUND_UP((unsigned long)NSEC_PER_SEC * 1000,
Is the cast necessary?
> +clk_hw_get_rate(hw));
> +
> + d = meson_parm_read(clk->map, >delay);
> + degrees += d * ph->delay_step_ps * 360 / period_ps;
> + degrees %= 360;
> +
> + return degrees;
> +}
> +
> +static int meson_clk_phase_delay_set_phase(struct clk_hw *hw, int degrees)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_clk_phase_delay_data *ph =
> + meson_clk_get_phase_delay_data(clk);
> + unsigned long period_ps, d = 0, r;
> + u64 p;
> +
> + p = degrees % 360;
We don't allow phase to be larger than 360 so this isn't needed.
> + period_ps = DIV_ROUND_UP((unsigned long)NSEC_PER_SEC * 1000,
Drop the cast?
> +clk_hw_get_rate(hw));
> +
> + /* First compute the phase index (p), the remainder (r) is the
Nitpick: Please leave /* on it's own line.
> +* part we'll try to acheive using the delays (d).
> +*/
> + r = do_div(p, 360 / (1 << (ph->phase.width)));
Drop useless parenthesis please.
> + d = DIV_ROUND_CLOSEST(r * period_ps,
> + 360 * ph->delay_step_ps);
> + d = min(d, PMASK(ph->delay.width));
> +
> + meson_parm_write(clk->map, >phase, p);
> + meson_parm_write(clk->map, >delay, d);
> + return 0;
> +}
[PATCH] cgroup: remove unnecessary unlikely()
WARN_ON() already contains an unlikely(), so it's not necessary to use
unlikely.
Signed-off-by: Yangtao Li
---
kernel/cgroup/cgroup.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 6aaf5dd5383b..2e5d90dfcb49 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5978,10 +5978,8 @@ static ssize_t show_delegatable_files(struct cftype
*files, char *buf,
ret += snprintf(buf + ret, size - ret, "%s\n", cft->name);
- if (unlikely(ret >= size)) {
- WARN_ON(1);
+ if (WARN_ON(ret >= size))
break;
- }
}
return ret;
--
2.17.0
[PATCH] cgroup: remove unnecessary unlikely()
WARN_ON() already contains an unlikely(), so it's not necessary to use
unlikely.
Signed-off-by: Yangtao Li
---
kernel/cgroup/cgroup.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 6aaf5dd5383b..2e5d90dfcb49 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5978,10 +5978,8 @@ static ssize_t show_delegatable_files(struct cftype
*files, char *buf,
ret += snprintf(buf + ret, size - ret, "%s\n", cft->name);
- if (unlikely(ret >= size)) {
- WARN_ON(1);
+ if (WARN_ON(ret >= size))
break;
- }
}
return ret;
--
2.17.0
Re: linux-next: Tree for Oct 31 (vboxguest)
On Sat, Nov 3, 2018 at 12:55 AM Arnd Bergmann wrote:
>
> On 11/2/18, Masahiro Yamada wrote:
> > On Thu, Nov 1, 2018 at 11:32 PM Changbin Du wrote:
> >> On Thu, Nov 01, 2018 at 12:32:48PM +0900, Masahiro Yamada wrote:
> >
> > How about clang?
> >
> > For clang, -Og might be equivalent to -O1 at this moment, but I am not
> > sure.
> >
> > In my understanding, Clang does not inline functions marked with 'static
> > inline'
> > for -Og (or -O1) optimization level.
> >
> > Theoretically, 'inline' keyword is a just hint for the compiler, after all.
>
> I think this means that we cannot build the kernel in that configuration,
> at least with CONFIG_OPTIMIZE_INLINING=y. Without that option,
> every 'inline' becomes 'always_inline'.
>
Sorry, I missed that fact.
At this moment of time, it is OK given the following:
- CONFIG_OPTIMIZE_INLINING is defined only for x86
- Clang cannot build x86 due to missing asm-goto
However, Clang with -Og
does not inline even such simple code like this:
-test code--
static inline int foo(int x)
{
return x;
}
int bar(int x)
{
return foo(x);
}
---
$ clang -Og -c -o bar.o bar.c
$ objdump -d bar.o
bar.o: file format elf64-x86-64
Disassembly of section .text:
:
0: eb 0ejmp10
2: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
9: 00 00 00
c: 0f 1f 40 00 nopl 0x0(%rax)
0010 :
10: 89 f8mov%edi,%eax
12: c3retq
--
Best Regards
Masahiro Yamada
Re: linux-next: Tree for Oct 31 (vboxguest)
On Sat, Nov 3, 2018 at 12:55 AM Arnd Bergmann wrote:
>
> On 11/2/18, Masahiro Yamada wrote:
> > On Thu, Nov 1, 2018 at 11:32 PM Changbin Du wrote:
> >> On Thu, Nov 01, 2018 at 12:32:48PM +0900, Masahiro Yamada wrote:
> >
> > How about clang?
> >
> > For clang, -Og might be equivalent to -O1 at this moment, but I am not
> > sure.
> >
> > In my understanding, Clang does not inline functions marked with 'static
> > inline'
> > for -Og (or -O1) optimization level.
> >
> > Theoretically, 'inline' keyword is a just hint for the compiler, after all.
>
> I think this means that we cannot build the kernel in that configuration,
> at least with CONFIG_OPTIMIZE_INLINING=y. Without that option,
> every 'inline' becomes 'always_inline'.
>
Sorry, I missed that fact.
At this moment of time, it is OK given the following:
- CONFIG_OPTIMIZE_INLINING is defined only for x86
- Clang cannot build x86 due to missing asm-goto
However, Clang with -Og
does not inline even such simple code like this:
-test code--
static inline int foo(int x)
{
return x;
}
int bar(int x)
{
return foo(x);
}
---
$ clang -Og -c -o bar.o bar.c
$ objdump -d bar.o
bar.o: file format elf64-x86-64
Disassembly of section .text:
:
0: eb 0ejmp10
2: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
9: 00 00 00
c: 0f 1f 40 00 nopl 0x0(%rax)
0010 :
10: 89 f8mov%edi,%eax
12: c3retq
--
Best Regards
Masahiro Yamada
Re: [PATCH v5 1/5] dt-bindings: phy-qcom-qmp: Fix register underspecification
Quoting Evan Green (2018-10-26 10:35:40)
> (or)
> @@ -150,3 +153,54 @@ Example:
> ...
> ...
> };
> +
> + phy@88eb000 {
> + compatible = "qcom,sdm845-qmp-usb3-uni-phy";
> + reg = <0x88eb000 0x18c>;
> + #clock-cells = <1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + clocks = < GCC_USB3_SEC_PHY_AUX_CLK>,
> +< GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +< GCC_USB3_SEC_CLKREF_CLK>,
> +< GCC_USB3_SEC_PHY_COM_AUX_CLK>;
> + clock-names = "aux", "cfg_ahb", "ref", "com_aux";
> +
> + resets = < GCC_USB3PHY_PHY_SEC_BCR>,
> +< GCC_USB3_PHY_SEC_BCR>;
> + reset-names = "phy", "common";
> +
> + lane@88eb200 {
> + reg = <0x88eb200 0x128>,
> + <0x88eb400 0x1fc>,
> + <0x88eb800 0x218>,
> + <0x88eb600 0x70>;
> + #phy-cells = <0>;
> + clocks = < GCC_USB3_SEC_PHY_PIPE_CLK>;
> + clock-names = "pipe0";
> + clock-output-names = "usb3_uni_phy_pipe_clk_src";
If this has clock-output-names then I would expect to see a #clock-cells
property, but that isn't here in this node. Are we relying on the same
property in the parent node?
> + };
> + };
Re: [PATCH v5 1/5] dt-bindings: phy-qcom-qmp: Fix register underspecification
Quoting Evan Green (2018-10-26 10:35:40)
> (or)
> @@ -150,3 +153,54 @@ Example:
> ...
> ...
> };
> +
> + phy@88eb000 {
> + compatible = "qcom,sdm845-qmp-usb3-uni-phy";
> + reg = <0x88eb000 0x18c>;
> + #clock-cells = <1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + clocks = < GCC_USB3_SEC_PHY_AUX_CLK>,
> +< GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +< GCC_USB3_SEC_CLKREF_CLK>,
> +< GCC_USB3_SEC_PHY_COM_AUX_CLK>;
> + clock-names = "aux", "cfg_ahb", "ref", "com_aux";
> +
> + resets = < GCC_USB3PHY_PHY_SEC_BCR>,
> +< GCC_USB3_PHY_SEC_BCR>;
> + reset-names = "phy", "common";
> +
> + lane@88eb200 {
> + reg = <0x88eb200 0x128>,
> + <0x88eb400 0x1fc>,
> + <0x88eb800 0x218>,
> + <0x88eb600 0x70>;
> + #phy-cells = <0>;
> + clocks = < GCC_USB3_SEC_PHY_PIPE_CLK>;
> + clock-names = "pipe0";
> + clock-output-names = "usb3_uni_phy_pipe_clk_src";
If this has clock-output-names then I would expect to see a #clock-cells
property, but that isn't here in this node. Are we relying on the same
property in the parent node?
> + };
> + };
[PATCH] clockevents: remove unnecessary unlikely()
WARN_ON() and WARN_ON_ONCE() already contains an unlikely(), so it's not
necessary to use unlikely.
Signed-off-by: Yangtao Li
---
kernel/time/clockevents.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 8c0e4092f661..af58898d9ebf 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -39,10 +39,8 @@ static u64 cev_delta2ns(unsigned long latch, struct
clock_event_device *evt,
u64 clc = (u64) latch << evt->shift;
u64 rnd;
- if (unlikely(!evt->mult)) {
+ if (WARN_ON(!evt->mult))
evt->mult = 1;
- WARN_ON(1);
- }
rnd = (u64) evt->mult - 1;
/*
@@ -164,10 +162,8 @@ void clockevents_switch_state(struct clock_event_device
*dev,
* on it, so fix it up and emit a warning:
*/
if (clockevent_state_oneshot(dev)) {
- if (unlikely(!dev->mult)) {
+ if (WARN_ON(!dev->mult))
dev->mult = 1;
- WARN_ON(1);
- }
}
}
}
@@ -315,10 +311,8 @@ int clockevents_program_event(struct clock_event_device
*dev, ktime_t expires,
int64_t delta;
int rc;
- if (unlikely(expires < 0)) {
- WARN_ON_ONCE(1);
+ if (WARN_ON_ONCE(expires < 0))
return -ETIME;
- }
dev->next_event = expires;
--
2.17.0
[PATCH] clockevents: remove unnecessary unlikely()
WARN_ON() and WARN_ON_ONCE() already contains an unlikely(), so it's not
necessary to use unlikely.
Signed-off-by: Yangtao Li
---
kernel/time/clockevents.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 8c0e4092f661..af58898d9ebf 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -39,10 +39,8 @@ static u64 cev_delta2ns(unsigned long latch, struct
clock_event_device *evt,
u64 clc = (u64) latch << evt->shift;
u64 rnd;
- if (unlikely(!evt->mult)) {
+ if (WARN_ON(!evt->mult))
evt->mult = 1;
- WARN_ON(1);
- }
rnd = (u64) evt->mult - 1;
/*
@@ -164,10 +162,8 @@ void clockevents_switch_state(struct clock_event_device
*dev,
* on it, so fix it up and emit a warning:
*/
if (clockevent_state_oneshot(dev)) {
- if (unlikely(!dev->mult)) {
+ if (WARN_ON(!dev->mult))
dev->mult = 1;
- WARN_ON(1);
- }
}
}
}
@@ -315,10 +311,8 @@ int clockevents_program_event(struct clock_event_device
*dev, ktime_t expires,
int64_t delta;
int rc;
- if (unlikely(expires < 0)) {
- WARN_ON_ONCE(1);
+ if (WARN_ON_ONCE(expires < 0))
return -ETIME;
- }
dev->next_event = expires;
--
2.17.0
Re: [PATCH 2/2] spi: spi-geni-qcom: Simplify probe function
Quoting Alok Chauhan (2018-10-25 09:40:29) > Re-arrange existing APIs in probe function to > avoid using goto and remove redundant variables. > > Signed-off-by: Alok Chauhan > --- Reviewed-by: Stephen Boyd
Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
On Sat, 3 Nov 2018 13:30:21 -0400
Steven Rostedt wrote:
> On Sun, 4 Nov 2018 01:34:30 +0900
> Masami Hiramatsu wrote:
> >
> > > I was thinking of a bitmask that represents the handlers, and use that
> > > to map which handler gets called for which shadow entry for a
> > > particular task.
> >
> > Hmm, I doubt that is too complicated and not scalable. I rather like to see
> > the open shadow entry...
>
> It can scale and not too complex (I already played a little with it).
> But that said, I'm not committed to it, and using the shadow stack is
> also an interesting idea.
>
> >
> > entry: [[original_retaddr][function][modified_retaddr]]
> >
> > So if there are many users on same function, the entries will be like this
> >
> > [[original_return_address][function][trampoline_A]]
> > [[trampline_A][function][trampoline_B]]
> > [[trampline_B][function][trampoline_C]]
> >
> > And on the top of the stack, there is trampline_C instead of
> > original_return_address.
> > In this case, return to trampoline_C(), it jumps back to trampline_B() and
> > then
> > it jumps back to trampline_A(). And eventually it jumps back to
> > original_return_address.
>
> Where are trampolines A, B, and C made? Do we also need to dynamically
> create them? If I register multiple function tracing ones, each one
> will need its own trampoline?
>
No, I think tramplines are very limited. currently we will only have ftrace
and kretprobe trampolines.
> > This way, we don't need allocate another bitmap/pages for the shadow stack.
> > We only need a shadow stack for each task.
> > Also, unwinder can easily find the trampline_C from the shadow stack and
> > restores
> > original_return_address. (of course trampline_A,B,C must be registered so
> > that
> > search function can skip it.)
>
> What I was thinking was to store a count and the functions to be called:
>
>
> [original_return_address]
> [function_A]
> [function_B]
> [function_C]
> [ 3 ]
>
> Then the trampoline that processes the return codes for ftrace (and
> kretprobes and everyone else) can simply do:
>
> count = pop_shadow_stack();
> for (i = 0; i < count; i++) {
> func = pop_shadow_stack();
> func(...);
> }
> return_address = pop_shadow_stack();
Ah, that's a good idea. I think we also have to store the called function
entry address with the number header, but basically I agree with you.
If we have a space to store a data with the function address, that is also
good to kretprobe. systemtap heavily uses "entry data" for saving some data
at function entry for exit handler.
> That way we only need to register a function to the return handler and
> it will be called, without worrying about making trampolines. There
> will just be a single trampoline that handles all the work.
OK, and could you make it independent from func graph tracer, so that
CONFIG_KPROBES=y but CONFIG_FUNCTION_GRAPH_TRACER=n kernel can support
kretprobes too.
Thank you,
--
Masami Hiramatsu
Re: [PATCH 2/2] spi: spi-geni-qcom: Simplify probe function
Quoting Alok Chauhan (2018-10-25 09:40:29) > Re-arrange existing APIs in probe function to > avoid using goto and remove redundant variables. > > Signed-off-by: Alok Chauhan > --- Reviewed-by: Stephen Boyd
Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
On Sat, 3 Nov 2018 13:30:21 -0400
Steven Rostedt wrote:
> On Sun, 4 Nov 2018 01:34:30 +0900
> Masami Hiramatsu wrote:
> >
> > > I was thinking of a bitmask that represents the handlers, and use that
> > > to map which handler gets called for which shadow entry for a
> > > particular task.
> >
> > Hmm, I doubt that is too complicated and not scalable. I rather like to see
> > the open shadow entry...
>
> It can scale and not too complex (I already played a little with it).
> But that said, I'm not committed to it, and using the shadow stack is
> also an interesting idea.
>
> >
> > entry: [[original_retaddr][function][modified_retaddr]]
> >
> > So if there are many users on same function, the entries will be like this
> >
> > [[original_return_address][function][trampoline_A]]
> > [[trampline_A][function][trampoline_B]]
> > [[trampline_B][function][trampoline_C]]
> >
> > And on the top of the stack, there is trampline_C instead of
> > original_return_address.
> > In this case, return to trampoline_C(), it jumps back to trampline_B() and
> > then
> > it jumps back to trampline_A(). And eventually it jumps back to
> > original_return_address.
>
> Where are trampolines A, B, and C made? Do we also need to dynamically
> create them? If I register multiple function tracing ones, each one
> will need its own trampoline?
>
No, I think tramplines are very limited. currently we will only have ftrace
and kretprobe trampolines.
> > This way, we don't need allocate another bitmap/pages for the shadow stack.
> > We only need a shadow stack for each task.
> > Also, unwinder can easily find the trampline_C from the shadow stack and
> > restores
> > original_return_address. (of course trampline_A,B,C must be registered so
> > that
> > search function can skip it.)
>
> What I was thinking was to store a count and the functions to be called:
>
>
> [original_return_address]
> [function_A]
> [function_B]
> [function_C]
> [ 3 ]
>
> Then the trampoline that processes the return codes for ftrace (and
> kretprobes and everyone else) can simply do:
>
> count = pop_shadow_stack();
> for (i = 0; i < count; i++) {
> func = pop_shadow_stack();
> func(...);
> }
> return_address = pop_shadow_stack();
Ah, that's a good idea. I think we also have to store the called function
entry address with the number header, but basically I agree with you.
If we have a space to store a data with the function address, that is also
good to kretprobe. systemtap heavily uses "entry data" for saving some data
at function entry for exit handler.
> That way we only need to register a function to the return handler and
> it will be called, without worrying about making trampolines. There
> will just be a single trampoline that handles all the work.
OK, and could you make it independent from func graph tracer, so that
CONFIG_KPROBES=y but CONFIG_FUNCTION_GRAPH_TRACER=n kernel can support
kretprobes too.
Thank you,
--
Masami Hiramatsu
[PATCH] tracing/fgraph: remove unnecessary unlikely()
WARN_ON() already contains an unlikely(), so it's not necessary to use
unlikely.
Signed-off-by: Yangtao Li
---
kernel/trace/trace_functions_graph.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c
b/kernel/trace/trace_functions_graph.c
index 169b3c44ee97..f8c2a08e4985 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -201,9 +201,8 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace,
unsigned long *ret,
if (index < 0)
index += FTRACE_NOTRACE_DEPTH;
- if (unlikely(index < 0 || index >= FTRACE_RETFUNC_DEPTH)) {
+ if (WARN_ON(index < 0 || index >= FTRACE_RETFUNC_DEPTH)) {
ftrace_graph_stop();
- WARN_ON(1);
/* Might as well panic, otherwise we have no where to go */
*ret = (unsigned long)panic;
return;
@@ -274,9 +273,8 @@ unsigned long ftrace_return_to_handler(unsigned long
frame_pointer)
*/
ftrace_graph_return();
- if (unlikely(!ret)) {
+ if (WARN_ON(!ret)) {
ftrace_graph_stop();
- WARN_ON(1);
/* Might as well panic. What else to do? */
ret = (unsigned long)panic;
}
--
2.17.0
[PATCH] tracing/fgraph: remove unnecessary unlikely()
WARN_ON() already contains an unlikely(), so it's not necessary to use
unlikely.
Signed-off-by: Yangtao Li
---
kernel/trace/trace_functions_graph.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c
b/kernel/trace/trace_functions_graph.c
index 169b3c44ee97..f8c2a08e4985 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -201,9 +201,8 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace,
unsigned long *ret,
if (index < 0)
index += FTRACE_NOTRACE_DEPTH;
- if (unlikely(index < 0 || index >= FTRACE_RETFUNC_DEPTH)) {
+ if (WARN_ON(index < 0 || index >= FTRACE_RETFUNC_DEPTH)) {
ftrace_graph_stop();
- WARN_ON(1);
/* Might as well panic, otherwise we have no where to go */
*ret = (unsigned long)panic;
return;
@@ -274,9 +273,8 @@ unsigned long ftrace_return_to_handler(unsigned long
frame_pointer)
*/
ftrace_graph_return();
- if (unlikely(!ret)) {
+ if (WARN_ON(!ret)) {
ftrace_graph_stop();
- WARN_ON(1);
/* Might as well panic. What else to do? */
ret = (unsigned long)panic;
}
--
2.17.0
Re: [PATCH 1/2] spi: spi-geni-qcom: fix nitpicks
Quoting Alok Chauhan (2018-10-25 09:40:28) > fixed the nitpicks. Agree with Doug, this commit text needs work. > > Signed-off-by: Alok Chauhan > --- Code is fine though. Reviewed-by: Stephen Boyd
Re: [PATCH 1/2] spi: spi-geni-qcom: fix nitpicks
Quoting Alok Chauhan (2018-10-25 09:40:28) > fixed the nitpicks. Agree with Doug, this commit text needs work. > > Signed-off-by: Alok Chauhan > --- Code is fine though. Reviewed-by: Stephen Boyd
Re: [RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments
On Sat, 3 Nov 2018 13:43:16 -0400
Steven Rostedt wrote:
> On Sun, 4 Nov 2018 01:03:34 +0900
> Masami Hiramatsu wrote:
>
> > Introduce symbol_offset data structure for avoiding symbol+offset
> > parsing when updating arguments.
> >
> > For kprobe events, "@symbol+offset" is supported, that requires
> > to be updated when new module is loaded because @symbol address
> > can be solved at that point. Currently kprobe events saves
> > the "symbol+offset" string and parse it repeatedly, but that
> > is inefficient.
>
> Do we care if it's inefficient? It's only done on module load and at
> the beginning. That's far from a fast path.
>
> If anything, the original solution saves memory, which is a bigger
> concern than speed in this case.
OK, and original one saves lines too. Please drop it :)
Thank you,
>
> -- Steve
>
>
> > This introduces symbol_offset data structure which can save the
> > offset and symbol separated, so that we don't need to parse it
> > anymore.
> >
> > Signed-off-by: Masami Hiramatsu
> > ---
> > kernel/trace/trace_probe.c | 49
> > +---
> > kernel/trace/trace_probe.h |7 +-
> > 2 files changed, 34 insertions(+), 22 deletions(-)
> >
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index bd30e9398d2a..0a3af7d6e133 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -201,6 +201,26 @@ static int parse_probe_vars(char *arg, const struct
> > fetch_type *t,
> > return ret;
> > }
> >
> > +static struct symbol_offset *allocate_symbol_offset(char *sym_offs_str)
> > +{
> > + int ret;
> > + long offset = 0;
> > + struct symbol_offset *sof;
> > +
> > + ret = traceprobe_split_symbol_offset(sym_offs_str, );
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + sof = kzalloc(sizeof(struct symbol_offset) + strlen(sym_offs_str) + 1,
> > + GFP_KERNEL);
> > + if (!sof)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + sof->offset = offset;
> > + strcpy(sof->symbol, sym_offs_str);
> > + return sof;
> > +}
> > +
> > /* Recursive argument parser */
> > static int
> > parse_probe_arg(char *arg, const struct fetch_type *type,
> > @@ -253,9 +273,9 @@ parse_probe_arg(char *arg, const struct fetch_type
> > *type,
> >
> > /* Preserve symbol for updating */
> > code->op = FETCH_NOP_SYMBOL;
> > - code->data = kstrdup(arg + 1, GFP_KERNEL);
> > - if (!code->data)
> > - return -ENOMEM;
> > + code->symoffs = allocate_symbol_offset(arg + 1);
> > + if (IS_ERR(code->symoffs))
> > + return PTR_ERR(code->symoffs);
> > if (++code == end)
> > return -E2BIG;
> >
> > @@ -483,7 +503,7 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> > if (ret) {
> > for (code = tmp; code < tmp + FETCH_INSN_MAX; code++)
> > if (code->op == FETCH_NOP_SYMBOL)
> > - kfree(code->data);
> > + kfree(code->symoffs);
> > }
> > kfree(tmp);
> >
> > @@ -513,7 +533,7 @@ void traceprobe_free_probe_arg(struct probe_arg *arg)
> >
> > while (code && code->op != FETCH_OP_END) {
> > if (code->op == FETCH_NOP_SYMBOL)
> > - kfree(code->data);
> > + kfree(code->symoffs);
> > code++;
> > }
> > kfree(arg->code);
> > @@ -525,31 +545,18 @@ void traceprobe_free_probe_arg(struct probe_arg *arg)
> > int traceprobe_update_arg(struct probe_arg *arg)
> > {
> > struct fetch_insn *code = arg->code;
> > - long offset;
> > - char *tmp;
> > - char c;
> > - int ret = 0;
> >
> > while (code && code->op != FETCH_OP_END) {
> > if (code->op == FETCH_NOP_SYMBOL) {
> > if (code[1].op != FETCH_OP_IMM)
> > return -EINVAL;
> >
> > - tmp = strpbrk(code->data, "+-");
> > - if (tmp)
> > - c = *tmp;
> > - ret = traceprobe_split_symbol_offset(code->data,
> > -);
> > - if (ret)
> > - return ret;
> > -
> > code[1].immediate =
> > - (unsigned long)kallsyms_lookup_name(code->data);
> > - if (tmp)
> > - *tmp = c;
> > + (unsigned long)kallsyms_lookup_name(
> > + code->symoffs->symbol);
> > if (!code[1].immediate)
> > return -ENOENT;
> > - code[1].immediate += offset;
> > + code[1].immediate += code->symoffs->offset;
> >
Re: [RFC PATCH] tracing/kprobes: Avoid parsing symbol+offset when updating arguments
On Sat, 3 Nov 2018 13:43:16 -0400
Steven Rostedt wrote:
> On Sun, 4 Nov 2018 01:03:34 +0900
> Masami Hiramatsu wrote:
>
> > Introduce symbol_offset data structure for avoiding symbol+offset
> > parsing when updating arguments.
> >
> > For kprobe events, "@symbol+offset" is supported, that requires
> > to be updated when new module is loaded because @symbol address
> > can be solved at that point. Currently kprobe events saves
> > the "symbol+offset" string and parse it repeatedly, but that
> > is inefficient.
>
> Do we care if it's inefficient? It's only done on module load and at
> the beginning. That's far from a fast path.
>
> If anything, the original solution saves memory, which is a bigger
> concern than speed in this case.
OK, and original one saves lines too. Please drop it :)
Thank you,
>
> -- Steve
>
>
> > This introduces symbol_offset data structure which can save the
> > offset and symbol separated, so that we don't need to parse it
> > anymore.
> >
> > Signed-off-by: Masami Hiramatsu
> > ---
> > kernel/trace/trace_probe.c | 49
> > +---
> > kernel/trace/trace_probe.h |7 +-
> > 2 files changed, 34 insertions(+), 22 deletions(-)
> >
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index bd30e9398d2a..0a3af7d6e133 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -201,6 +201,26 @@ static int parse_probe_vars(char *arg, const struct
> > fetch_type *t,
> > return ret;
> > }
> >
> > +static struct symbol_offset *allocate_symbol_offset(char *sym_offs_str)
> > +{
> > + int ret;
> > + long offset = 0;
> > + struct symbol_offset *sof;
> > +
> > + ret = traceprobe_split_symbol_offset(sym_offs_str, );
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + sof = kzalloc(sizeof(struct symbol_offset) + strlen(sym_offs_str) + 1,
> > + GFP_KERNEL);
> > + if (!sof)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + sof->offset = offset;
> > + strcpy(sof->symbol, sym_offs_str);
> > + return sof;
> > +}
> > +
> > /* Recursive argument parser */
> > static int
> > parse_probe_arg(char *arg, const struct fetch_type *type,
> > @@ -253,9 +273,9 @@ parse_probe_arg(char *arg, const struct fetch_type
> > *type,
> >
> > /* Preserve symbol for updating */
> > code->op = FETCH_NOP_SYMBOL;
> > - code->data = kstrdup(arg + 1, GFP_KERNEL);
> > - if (!code->data)
> > - return -ENOMEM;
> > + code->symoffs = allocate_symbol_offset(arg + 1);
> > + if (IS_ERR(code->symoffs))
> > + return PTR_ERR(code->symoffs);
> > if (++code == end)
> > return -E2BIG;
> >
> > @@ -483,7 +503,7 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> > if (ret) {
> > for (code = tmp; code < tmp + FETCH_INSN_MAX; code++)
> > if (code->op == FETCH_NOP_SYMBOL)
> > - kfree(code->data);
> > + kfree(code->symoffs);
> > }
> > kfree(tmp);
> >
> > @@ -513,7 +533,7 @@ void traceprobe_free_probe_arg(struct probe_arg *arg)
> >
> > while (code && code->op != FETCH_OP_END) {
> > if (code->op == FETCH_NOP_SYMBOL)
> > - kfree(code->data);
> > + kfree(code->symoffs);
> > code++;
> > }
> > kfree(arg->code);
> > @@ -525,31 +545,18 @@ void traceprobe_free_probe_arg(struct probe_arg *arg)
> > int traceprobe_update_arg(struct probe_arg *arg)
> > {
> > struct fetch_insn *code = arg->code;
> > - long offset;
> > - char *tmp;
> > - char c;
> > - int ret = 0;
> >
> > while (code && code->op != FETCH_OP_END) {
> > if (code->op == FETCH_NOP_SYMBOL) {
> > if (code[1].op != FETCH_OP_IMM)
> > return -EINVAL;
> >
> > - tmp = strpbrk(code->data, "+-");
> > - if (tmp)
> > - c = *tmp;
> > - ret = traceprobe_split_symbol_offset(code->data,
> > -);
> > - if (ret)
> > - return ret;
> > -
> > code[1].immediate =
> > - (unsigned long)kallsyms_lookup_name(code->data);
> > - if (tmp)
> > - *tmp = c;
> > + (unsigned long)kallsyms_lookup_name(
> > + code->symoffs->symbol);
> > if (!code[1].immediate)
> > return -ENOENT;
> > - code[1].immediate += offset;
> > + code[1].immediate += code->symoffs->offset;
> >
Re: [GIT PULL] scheduler fixes
On Sat, Nov 3, 2018 at 4:52 PM Ingo Molnar wrote: > > A memory (under-)allocation fix and a comment fix. Pulled, Linus
Re: [GIT PULL] scheduler fixes
On Sat, Nov 3, 2018 at 4:52 PM Ingo Molnar wrote: > > A memory (under-)allocation fix and a comment fix. Pulled, Linus
Re: [PATCH v2] staging: olpc_dcon: olpc_dcon_xo_1.c: Switch to the gpio descriptor interface
Hi Nishad,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.19 next-20181102]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Nishad-Kamdar/staging-olpc_dcon-olpc_dcon_xo_1-c-Switch-to-the-gpio-descriptor-interface/20181104-041335
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:18:0:
>> drivers/staging/olpc_dcon/olpc_dcon.h:58:33: error: expected identifier
>> before numeric constant
#define DCON_IRQ6
^
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:23:2: note: in expansion of macro
>> 'DCON_IRQ'
DCON_IRQ,
^~~~
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:34:50: error: 'GPIO_ASIS'
>> undeclared here (not in a function); did you mean 'GPIOD_ASIS'?
[DCON_STAT0] = { .name = "dcon_stat0", .flags = GPIO_ASIS },
^
GPIOD_ASIS
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:37:3: error: 'DCON_LOAD'
>> undeclared here (not in a function); did you mean 'DCON_STAT1'?
[DCON_LOAD] = { .name = "dcon_load", .flags = GPIO_ASIS },
^
DCON_STAT1
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:37:3: error: array index in
>> initializer not of integer type
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:37:3: note: (near initialization
for 'gpios_asis')
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:38:3: error: 'DCON_BLANK'
>> undeclared here (not in a function); did you mean 'DCON_LOAD'?
[DCON_BLANK] = { .name = "dcon_blank", .flag = GPIO_ASIS },
^~
DCON_LOAD
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:38:3: error: array index in
initializer not of integer type
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:38:3: note: (near initialization
for 'gpios_asis')
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:38:42: error: 'const struct
>> dcon_gpio' has no member named 'flag'; did you mean 'flags'?
[DCON_BLANK] = { .name = "dcon_blank", .flag = GPIO_ASIS },
^~~~
flags
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c: In function 'dcon_init_xo_1':
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:47:26: warning: initialization
>> discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
struct dcon_gpio *pin = _asis[0];
^
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:42: error: dereferencing
>> pointer to incomplete type 'struct i2c_client'
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:42: error: request for member
>> 'dev' in something not a structure or union
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:55: error: invalid type
>> argument of '->' (have 'struct dcon_gpio')
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:55: error: request for member
>> 'name' in something not a structure or union
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:51:14: error: invalid type
argument of '->' (have 'struct dcon_gpio')
pin[i]->flags);
^~
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:51:14: error: request for member
>> 'flags' in something not a structure or union
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:29: error: passing argument 1
>> of 'devm_gpiod_get' from incompatible pointer type
>> [-Werror=incompatible-pointer-types]
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^
In file included from drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:14:0:
include/linux/gpio/consumer.h:87:32: note: expected 'struct device *' but
argument is of type 'const struct dcon_gpio (*)[1]'
struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
^~
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:49: error: passing argument 2
of 'devm_gpiod_get' from incompatible pointer type
[-Werror=incompatible-pointer-types]
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~~
In file included from drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:14:0:
include/linux/gpio/consumer.h:87:32: note: expected 'const char *' but
argument is of type 'const struct dcon_gpio *'
struct gpio_desc *__must_check
Re: [PATCH v2] staging: olpc_dcon: olpc_dcon_xo_1.c: Switch to the gpio descriptor interface
Hi Nishad,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.19 next-20181102]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Nishad-Kamdar/staging-olpc_dcon-olpc_dcon_xo_1-c-Switch-to-the-gpio-descriptor-interface/20181104-041335
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:18:0:
>> drivers/staging/olpc_dcon/olpc_dcon.h:58:33: error: expected identifier
>> before numeric constant
#define DCON_IRQ6
^
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:23:2: note: in expansion of macro
>> 'DCON_IRQ'
DCON_IRQ,
^~~~
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:34:50: error: 'GPIO_ASIS'
>> undeclared here (not in a function); did you mean 'GPIOD_ASIS'?
[DCON_STAT0] = { .name = "dcon_stat0", .flags = GPIO_ASIS },
^
GPIOD_ASIS
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:37:3: error: 'DCON_LOAD'
>> undeclared here (not in a function); did you mean 'DCON_STAT1'?
[DCON_LOAD] = { .name = "dcon_load", .flags = GPIO_ASIS },
^
DCON_STAT1
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:37:3: error: array index in
>> initializer not of integer type
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:37:3: note: (near initialization
for 'gpios_asis')
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:38:3: error: 'DCON_BLANK'
>> undeclared here (not in a function); did you mean 'DCON_LOAD'?
[DCON_BLANK] = { .name = "dcon_blank", .flag = GPIO_ASIS },
^~
DCON_LOAD
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:38:3: error: array index in
initializer not of integer type
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:38:3: note: (near initialization
for 'gpios_asis')
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:38:42: error: 'const struct
>> dcon_gpio' has no member named 'flag'; did you mean 'flags'?
[DCON_BLANK] = { .name = "dcon_blank", .flag = GPIO_ASIS },
^~~~
flags
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c: In function 'dcon_init_xo_1':
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:47:26: warning: initialization
>> discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
struct dcon_gpio *pin = _asis[0];
^
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:42: error: dereferencing
>> pointer to incomplete type 'struct i2c_client'
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:42: error: request for member
>> 'dev' in something not a structure or union
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:55: error: invalid type
>> argument of '->' (have 'struct dcon_gpio')
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:55: error: request for member
>> 'name' in something not a structure or union
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:51:14: error: invalid type
argument of '->' (have 'struct dcon_gpio')
pin[i]->flags);
^~
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:51:14: error: request for member
>> 'flags' in something not a structure or union
>> drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:29: error: passing argument 1
>> of 'devm_gpiod_get' from incompatible pointer type
>> [-Werror=incompatible-pointer-types]
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^
In file included from drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:14:0:
include/linux/gpio/consumer.h:87:32: note: expected 'struct device *' but
argument is of type 'const struct dcon_gpio (*)[1]'
struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
^~
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:50:49: error: passing argument 2
of 'devm_gpiod_get' from incompatible pointer type
[-Werror=incompatible-pointer-types]
gpios[i] = devm_gpiod_get(>client->dev, pin[i]->name,
^~~
In file included from drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:14:0:
include/linux/gpio/consumer.h:87:32: note: expected 'const char *' but
argument is of type 'const struct dcon_gpio *'
struct gpio_desc *__must_check
Re: [GIT PULL] x86 fixes
On Sat, Nov 3, 2018 at 4:09 PM Ingo Molnar wrote: > > A number of fixes and some late updates: Pulled, Linus
Re: [GIT PULL] x86 fixes
On Sat, Nov 3, 2018 at 4:09 PM Ingo Molnar wrote: > > A number of fixes and some late updates: Pulled, Linus
Re: [GIT PULL] perf updates/fixes
On Sat, Nov 3, 2018 at 4:03 PM Ingo Molnar wrote: > > These are almost all tooling updates: 'perf top', 'perf trace' and 'perf > script' fixes and updates, an UAPI header sync with the merge window > versions, license marker updates, much improved Sparc support from David > Miller, and a number of fixes. Pulled, Linus
Re: [GIT PULL] perf updates/fixes
On Sat, Nov 3, 2018 at 4:03 PM Ingo Molnar wrote: > > These are almost all tooling updates: 'perf top', 'perf trace' and 'perf > script' fixes and updates, an UAPI header sync with the merge window > versions, license marker updates, much improved Sparc support from David > Miller, and a number of fixes. Pulled, Linus
Re: [GIT PULL] IRQ fixes
On Sat, Nov 3, 2018 at 3:59 PM Ingo Molnar wrote: > > An irqchip driver fix and a memory (over-)allocation fix. Pulled, Linus
Re: [GIT PULL] IRQ fixes
On Sat, Nov 3, 2018 at 3:59 PM Ingo Molnar wrote: > > An irqchip driver fix and a memory (over-)allocation fix. Pulled, Linus
[tip:sched/core] sched/core: Introduce set_next_task() helper for better code readability
Commit-ID: ff1cdc94de4d336be45336d70709dfcf3d682514
Gitweb: https://git.kernel.org/tip/ff1cdc94de4d336be45336d70709dfcf3d682514
Author: Muchun Song
AuthorDate: Fri, 26 Oct 2018 21:17:43 +0800
Committer: Ingo Molnar
CommitDate: Sun, 4 Nov 2018 00:59:24 +0100
sched/core: Introduce set_next_task() helper for better code readability
When we pick the next task, we will do the following for the task:
1) p->se.exec_start = rq_clock_task(rq);
2) dequeue_pushable(_dl)_task(rq, p);
When we call set_curr_task(), we also need to do the same thing
above. In rt.c, the code at 1) is in the _pick_next_task_rt()
and the code at 2) is in the pick_next_task_rt(). If we put two
operations in one function, maybe better. So, we introduce a new
function set_next_task(), which is responsible for doing the above.
By introducing the function we can get rid of calling the
dequeue_pushable(_dl)_task() directly(We can call set_next_task())
in pick_next_task() and have better code readability and reuse.
In set_curr_task_rt(), we also can call set_next_task().
Do this things such that we end up with:
static struct task_struct *pick_next_task(struct rq *rq,
struct task_struct *prev,
struct rq_flags *rf)
{
/* do something else ... */
put_prev_task(rq, prev);
/* pick next task p */
set_next_task(rq, p);
/* do something else ... */
}
put_prev_task() can match set_next_task(), which can make the
code more readable.
Signed-off-by: Muchun Song
Signed-off-by: Peter Zijlstra (Intel)
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Link: http://lkml.kernel.org/r/20181026131743.21786-1-smuc...@gmail.com
Signed-off-by: Ingo Molnar
---
kernel/sched/deadline.c | 19 ++-
kernel/sched/rt.c | 24 +++-
2 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 91e4202b0634..470ba6b464fe 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1695,6 +1695,14 @@ static void start_hrtick_dl(struct rq *rq, struct
task_struct *p)
}
#endif
+static inline void set_next_task(struct rq *rq, struct task_struct *p)
+{
+ p->se.exec_start = rq_clock_task(rq);
+
+ /* You can't push away the running task */
+ dequeue_pushable_dl_task(rq, p);
+}
+
static struct sched_dl_entity *pick_next_dl_entity(struct rq *rq,
struct dl_rq *dl_rq)
{
@@ -1750,10 +1758,8 @@ pick_next_task_dl(struct rq *rq, struct task_struct
*prev, struct rq_flags *rf)
BUG_ON(!dl_se);
p = dl_task_of(dl_se);
- p->se.exec_start = rq_clock_task(rq);
- /* Running task will never be pushed. */
- dequeue_pushable_dl_task(rq, p);
+ set_next_task(rq, p);
if (hrtick_enabled(rq))
start_hrtick_dl(rq, p);
@@ -1808,12 +1814,7 @@ static void task_fork_dl(struct task_struct *p)
static void set_curr_task_dl(struct rq *rq)
{
- struct task_struct *p = rq->curr;
-
- p->se.exec_start = rq_clock_task(rq);
-
- /* You can't push away the running task */
- dequeue_pushable_dl_task(rq, p);
+ set_next_task(rq, rq->curr);
}
#ifdef CONFIG_SMP
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index a21ea6021929..9aa3287ce301 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1498,6 +1498,14 @@ static void check_preempt_curr_rt(struct rq *rq, struct
task_struct *p, int flag
#endif
}
+static inline void set_next_task(struct rq *rq, struct task_struct *p)
+{
+ p->se.exec_start = rq_clock_task(rq);
+
+ /* The running task is never eligible for pushing */
+ dequeue_pushable_task(rq, p);
+}
+
static struct sched_rt_entity *pick_next_rt_entity(struct rq *rq,
struct rt_rq *rt_rq)
{
@@ -1518,7 +1526,6 @@ static struct sched_rt_entity *pick_next_rt_entity(struct
rq *rq,
static struct task_struct *_pick_next_task_rt(struct rq *rq)
{
struct sched_rt_entity *rt_se;
- struct task_struct *p;
struct rt_rq *rt_rq = >rt;
do {
@@ -1527,10 +1534,7 @@ static struct task_struct *_pick_next_task_rt(struct rq
*rq)
rt_rq = group_rt_rq(rt_se);
} while (rt_rq);
- p = rt_task_of(rt_se);
- p->se.exec_start = rq_clock_task(rq);
-
- return p;
+ return rt_task_of(rt_se);
}
static struct task_struct *
@@ -1573,8 +1577,7 @@ pick_next_task_rt(struct rq *rq, struct task_struct
*prev, struct rq_flags *rf)
p = _pick_next_task_rt(rq);
- /* The running task is never eligible for pushing */
- dequeue_pushable_task(rq, p);
+ set_next_task(rq, p);
rt_queue_push_tasks(rq);
@@ -2355,12 +2358,7 @@ static void task_tick_rt(struct rq *rq, struct
task_struct *p, int queued)
static void
[tip:sched/core] sched/core: Introduce set_next_task() helper for better code readability
Commit-ID: ff1cdc94de4d336be45336d70709dfcf3d682514
Gitweb: https://git.kernel.org/tip/ff1cdc94de4d336be45336d70709dfcf3d682514
Author: Muchun Song
AuthorDate: Fri, 26 Oct 2018 21:17:43 +0800
Committer: Ingo Molnar
CommitDate: Sun, 4 Nov 2018 00:59:24 +0100
sched/core: Introduce set_next_task() helper for better code readability
When we pick the next task, we will do the following for the task:
1) p->se.exec_start = rq_clock_task(rq);
2) dequeue_pushable(_dl)_task(rq, p);
When we call set_curr_task(), we also need to do the same thing
above. In rt.c, the code at 1) is in the _pick_next_task_rt()
and the code at 2) is in the pick_next_task_rt(). If we put two
operations in one function, maybe better. So, we introduce a new
function set_next_task(), which is responsible for doing the above.
By introducing the function we can get rid of calling the
dequeue_pushable(_dl)_task() directly(We can call set_next_task())
in pick_next_task() and have better code readability and reuse.
In set_curr_task_rt(), we also can call set_next_task().
Do this things such that we end up with:
static struct task_struct *pick_next_task(struct rq *rq,
struct task_struct *prev,
struct rq_flags *rf)
{
/* do something else ... */
put_prev_task(rq, prev);
/* pick next task p */
set_next_task(rq, p);
/* do something else ... */
}
put_prev_task() can match set_next_task(), which can make the
code more readable.
Signed-off-by: Muchun Song
Signed-off-by: Peter Zijlstra (Intel)
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Link: http://lkml.kernel.org/r/20181026131743.21786-1-smuc...@gmail.com
Signed-off-by: Ingo Molnar
---
kernel/sched/deadline.c | 19 ++-
kernel/sched/rt.c | 24 +++-
2 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 91e4202b0634..470ba6b464fe 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1695,6 +1695,14 @@ static void start_hrtick_dl(struct rq *rq, struct
task_struct *p)
}
#endif
+static inline void set_next_task(struct rq *rq, struct task_struct *p)
+{
+ p->se.exec_start = rq_clock_task(rq);
+
+ /* You can't push away the running task */
+ dequeue_pushable_dl_task(rq, p);
+}
+
static struct sched_dl_entity *pick_next_dl_entity(struct rq *rq,
struct dl_rq *dl_rq)
{
@@ -1750,10 +1758,8 @@ pick_next_task_dl(struct rq *rq, struct task_struct
*prev, struct rq_flags *rf)
BUG_ON(!dl_se);
p = dl_task_of(dl_se);
- p->se.exec_start = rq_clock_task(rq);
- /* Running task will never be pushed. */
- dequeue_pushable_dl_task(rq, p);
+ set_next_task(rq, p);
if (hrtick_enabled(rq))
start_hrtick_dl(rq, p);
@@ -1808,12 +1814,7 @@ static void task_fork_dl(struct task_struct *p)
static void set_curr_task_dl(struct rq *rq)
{
- struct task_struct *p = rq->curr;
-
- p->se.exec_start = rq_clock_task(rq);
-
- /* You can't push away the running task */
- dequeue_pushable_dl_task(rq, p);
+ set_next_task(rq, rq->curr);
}
#ifdef CONFIG_SMP
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index a21ea6021929..9aa3287ce301 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1498,6 +1498,14 @@ static void check_preempt_curr_rt(struct rq *rq, struct
task_struct *p, int flag
#endif
}
+static inline void set_next_task(struct rq *rq, struct task_struct *p)
+{
+ p->se.exec_start = rq_clock_task(rq);
+
+ /* The running task is never eligible for pushing */
+ dequeue_pushable_task(rq, p);
+}
+
static struct sched_rt_entity *pick_next_rt_entity(struct rq *rq,
struct rt_rq *rt_rq)
{
@@ -1518,7 +1526,6 @@ static struct sched_rt_entity *pick_next_rt_entity(struct
rq *rq,
static struct task_struct *_pick_next_task_rt(struct rq *rq)
{
struct sched_rt_entity *rt_se;
- struct task_struct *p;
struct rt_rq *rt_rq = >rt;
do {
@@ -1527,10 +1534,7 @@ static struct task_struct *_pick_next_task_rt(struct rq
*rq)
rt_rq = group_rt_rq(rt_se);
} while (rt_rq);
- p = rt_task_of(rt_se);
- p->se.exec_start = rq_clock_task(rq);
-
- return p;
+ return rt_task_of(rt_se);
}
static struct task_struct *
@@ -1573,8 +1577,7 @@ pick_next_task_rt(struct rq *rq, struct task_struct
*prev, struct rq_flags *rf)
p = _pick_next_task_rt(rq);
- /* The running task is never eligible for pushing */
- dequeue_pushable_task(rq, p);
+ set_next_task(rq, p);
rt_queue_push_tasks(rq);
@@ -2355,12 +2358,7 @@ static void task_tick_rt(struct rq *rq, struct
task_struct *p, int queued)
static void
[tip:sched/core] sched/fair: Clean up load_balance() condition
Commit-ID: 47b7aee14fd7e453370a5d15dfb11c958ca360f2 Gitweb: https://git.kernel.org/tip/47b7aee14fd7e453370a5d15dfb11c958ca360f2 Author: Valentin Schneider AuthorDate: Wed, 26 Sep 2018 16:12:06 +0100 Committer: Ingo Molnar CommitDate: Sun, 4 Nov 2018 00:59:22 +0100 sched/fair: Clean up load_balance() condition The alignment of the condition is off, clean that up. Also, logical operators have lower precedence than bitwise/relational operators, so remove one layer of parentheses to make the condition a bit simpler to follow. Signed-off-by: Valentin Schneider Signed-off-by: Peter Zijlstra (Intel) Cc: dietmar.eggem...@arm.com Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: patrick.bell...@arm.com Cc: vincent.guit...@linaro.org Link: http://lkml.kernel.org/r/1537974727-30788-1-git-send-email-valentin.schnei...@arm.com Signed-off-by: Ingo Molnar --- kernel/sched/fair.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index ee271bb661cc..4e298931a715 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -8877,9 +8877,9 @@ out_all_pinned: out_one_pinned: /* tune up the balancing interval */ - if (((env.flags & LBF_ALL_PINNED) && - sd->balance_interval < MAX_PINNED_INTERVAL) || - (sd->balance_interval < sd->max_interval)) + if ((env.flags & LBF_ALL_PINNED && +sd->balance_interval < MAX_PINNED_INTERVAL) || + sd->balance_interval < sd->max_interval) sd->balance_interval *= 2; ld_moved = 0;
[tip:sched/core] sched/fair: Don't increase sd->balance_interval on newidle balance
Commit-ID: 3f130a37c442d5c4d66531b240ebe9abfef426b5
Gitweb: https://git.kernel.org/tip/3f130a37c442d5c4d66531b240ebe9abfef426b5
Author: Valentin Schneider
AuthorDate: Wed, 26 Sep 2018 16:12:07 +0100
Committer: Ingo Molnar
CommitDate: Sun, 4 Nov 2018 00:59:23 +0100
sched/fair: Don't increase sd->balance_interval on newidle balance
When load_balance() fails to move some load because of task affinity,
we end up increasing sd->balance_interval to delay the next periodic
balance in the hopes that next time we look, that annoying pinned
task(s) will be gone.
However, idle_balance() pays no attention to sd->balance_interval, yet
it will still lead to an increase in balance_interval in case of
pinned tasks.
If we're going through several newidle balances (e.g. we have a
periodic task), this can lead to a huge increase of the
balance_interval in a very small amount of time.
To prevent that, don't increase the balance interval when going
through a newidle balance.
This is a similar approach to what is done in commit 58b26c4c0257
("sched: Increment cache_nice_tries only on periodic lb"), where we
disregard newidle balance and rely on periodic balance for more stable
results.
Signed-off-by: Valentin Schneider
Signed-off-by: Peter Zijlstra (Intel)
Cc: dietmar.eggem...@arm.com
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: patrick.bell...@arm.com
Cc: vincent.guit...@linaro.org
Link:
http://lkml.kernel.org/r/1537974727-30788-2-git-send-email-valentin.schnei...@arm.com
Signed-off-by: Ingo Molnar
---
kernel/sched/fair.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4e298931a715..a17ca4254427 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8876,13 +8876,22 @@ out_all_pinned:
sd->nr_balance_failed = 0;
out_one_pinned:
+ ld_moved = 0;
+
+ /*
+* idle_balance() disregards balance intervals, so we could repeatedly
+* reach this code, which would lead to balance_interval skyrocketting
+* in a short amount of time. Skip the balance_interval increase logic
+* to avoid that.
+*/
+ if (env.idle == CPU_NEWLY_IDLE)
+ goto out;
+
/* tune up the balancing interval */
if ((env.flags & LBF_ALL_PINNED &&
sd->balance_interval < MAX_PINNED_INTERVAL) ||
sd->balance_interval < sd->max_interval)
sd->balance_interval *= 2;
-
- ld_moved = 0;
out:
return ld_moved;
}
[tip:sched/core] sched/fair: Don't increase sd->balance_interval on newidle balance
Commit-ID: 3f130a37c442d5c4d66531b240ebe9abfef426b5
Gitweb: https://git.kernel.org/tip/3f130a37c442d5c4d66531b240ebe9abfef426b5
Author: Valentin Schneider
AuthorDate: Wed, 26 Sep 2018 16:12:07 +0100
Committer: Ingo Molnar
CommitDate: Sun, 4 Nov 2018 00:59:23 +0100
sched/fair: Don't increase sd->balance_interval on newidle balance
When load_balance() fails to move some load because of task affinity,
we end up increasing sd->balance_interval to delay the next periodic
balance in the hopes that next time we look, that annoying pinned
task(s) will be gone.
However, idle_balance() pays no attention to sd->balance_interval, yet
it will still lead to an increase in balance_interval in case of
pinned tasks.
If we're going through several newidle balances (e.g. we have a
periodic task), this can lead to a huge increase of the
balance_interval in a very small amount of time.
To prevent that, don't increase the balance interval when going
through a newidle balance.
This is a similar approach to what is done in commit 58b26c4c0257
("sched: Increment cache_nice_tries only on periodic lb"), where we
disregard newidle balance and rely on periodic balance for more stable
results.
Signed-off-by: Valentin Schneider
Signed-off-by: Peter Zijlstra (Intel)
Cc: dietmar.eggem...@arm.com
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: patrick.bell...@arm.com
Cc: vincent.guit...@linaro.org
Link:
http://lkml.kernel.org/r/1537974727-30788-2-git-send-email-valentin.schnei...@arm.com
Signed-off-by: Ingo Molnar
---
kernel/sched/fair.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4e298931a715..a17ca4254427 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8876,13 +8876,22 @@ out_all_pinned:
sd->nr_balance_failed = 0;
out_one_pinned:
+ ld_moved = 0;
+
+ /*
+* idle_balance() disregards balance intervals, so we could repeatedly
+* reach this code, which would lead to balance_interval skyrocketting
+* in a short amount of time. Skip the balance_interval increase logic
+* to avoid that.
+*/
+ if (env.idle == CPU_NEWLY_IDLE)
+ goto out;
+
/* tune up the balancing interval */
if ((env.flags & LBF_ALL_PINNED &&
sd->balance_interval < MAX_PINNED_INTERVAL) ||
sd->balance_interval < sd->max_interval)
sd->balance_interval *= 2;
-
- ld_moved = 0;
out:
return ld_moved;
}
[tip:sched/core] sched/fair: Clean up load_balance() condition
Commit-ID: 47b7aee14fd7e453370a5d15dfb11c958ca360f2 Gitweb: https://git.kernel.org/tip/47b7aee14fd7e453370a5d15dfb11c958ca360f2 Author: Valentin Schneider AuthorDate: Wed, 26 Sep 2018 16:12:06 +0100 Committer: Ingo Molnar CommitDate: Sun, 4 Nov 2018 00:59:22 +0100 sched/fair: Clean up load_balance() condition The alignment of the condition is off, clean that up. Also, logical operators have lower precedence than bitwise/relational operators, so remove one layer of parentheses to make the condition a bit simpler to follow. Signed-off-by: Valentin Schneider Signed-off-by: Peter Zijlstra (Intel) Cc: dietmar.eggem...@arm.com Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: patrick.bell...@arm.com Cc: vincent.guit...@linaro.org Link: http://lkml.kernel.org/r/1537974727-30788-1-git-send-email-valentin.schnei...@arm.com Signed-off-by: Ingo Molnar --- kernel/sched/fair.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index ee271bb661cc..4e298931a715 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -8877,9 +8877,9 @@ out_all_pinned: out_one_pinned: /* tune up the balancing interval */ - if (((env.flags & LBF_ALL_PINNED) && - sd->balance_interval < MAX_PINNED_INTERVAL) || - (sd->balance_interval < sd->max_interval)) + if ((env.flags & LBF_ALL_PINNED && +sd->balance_interval < MAX_PINNED_INTERVAL) || + sd->balance_interval < sd->max_interval) sd->balance_interval *= 2; ld_moved = 0;
[tip:sched/urgent] sched/core: Take the hotplug lock in sched_init_smp()
Commit-ID: 40fa3780bac2b654edf23f6b13f4e2dd550aea10
Gitweb: https://git.kernel.org/tip/40fa3780bac2b654edf23f6b13f4e2dd550aea10
Author: Valentin Schneider
AuthorDate: Tue, 23 Oct 2018 14:37:31 +0100
Committer: Ingo Molnar
CommitDate: Sun, 4 Nov 2018 00:57:44 +0100
sched/core: Take the hotplug lock in sched_init_smp()
When running on linux-next (8c60c36d0b8c ("Add linux-next specific files
for 20181019")) + CONFIG_PROVE_LOCKING=y on a big.LITTLE system (e.g.
Juno or HiKey960), we get the following report:
[0.748225] Call trace:
[0.750685] lockdep_assert_cpus_held+0x30/0x40
[0.755236] static_key_enable_cpuslocked+0x20/0xc8
[0.760137] build_sched_domains+0x1034/0x1108
[0.764601] sched_init_domains+0x68/0x90
[0.768628] sched_init_smp+0x30/0x80
[0.772309] kernel_init_freeable+0x278/0x51c
[0.776685] kernel_init+0x10/0x108
[0.780190] ret_from_fork+0x10/0x18
The static_key in question is 'sched_asym_cpucapacity' introduced by
commit:
df054e8445a4 ("sched/topology: Add static_key for asymmetric CPU capacity
optimizations")
In this particular case, we enable it because smp_prepare_cpus() will
end up fetching the capacity-dmips-mhz entry from the devicetree,
so we already have some asymmetry detected when entering sched_init_smp().
This didn't get detected in tip/sched/core because we were missing:
commit cb538267ea1e ("jump_label/lockdep: Assert we hold the hotplug lock for
_cpuslocked() operations")
Calls to build_sched_domains() post sched_init_smp() will hold the
hotplug lock, it just so happens that this very first call is a
special case. As stated by a comment in sched_init_smp(), "There's no
userspace yet to cause hotplug operations" so this is a harmless
warning.
However, to both respect the semantics of underlying
callees and make lockdep happy, take the hotplug lock in
sched_init_smp(). This also satisfies the comment atop
sched_init_domains() that says "Callers must hold the hotplug lock".
Reported-by: Sudeep Holla
Tested-by: Sudeep Holla
Signed-off-by: Valentin Schneider
Signed-off-by: Peter Zijlstra (Intel)
Cc: dietmar.eggem...@arm.com
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: morten.rasmus...@arm.com
Cc: quentin.per...@arm.com
Link:
http://lkml.kernel.org/r/1540301851-3048-1-git-send-email-valentin.schnei...@arm.com
Signed-off-by: Ingo Molnar
---
kernel/sched/core.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fd2fce8a001b..02a20ef196a6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5859,11 +5859,14 @@ void __init sched_init_smp(void)
/*
* There's no userspace yet to cause hotplug operations; hence all the
* CPU masks are stable and all blatant races in the below code cannot
-* happen.
+* happen. The hotplug lock is nevertheless taken to satisfy lockdep,
+* but there won't be any contention on it.
*/
+ cpus_read_lock();
mutex_lock(_domains_mutex);
sched_init_domains(cpu_active_mask);
mutex_unlock(_domains_mutex);
+ cpus_read_unlock();
/* Move init over to a non-isolated CPU */
if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_FLAG_DOMAIN))
< 0)
[tip:sched/urgent] sched/core: Take the hotplug lock in sched_init_smp()
Commit-ID: 40fa3780bac2b654edf23f6b13f4e2dd550aea10
Gitweb: https://git.kernel.org/tip/40fa3780bac2b654edf23f6b13f4e2dd550aea10
Author: Valentin Schneider
AuthorDate: Tue, 23 Oct 2018 14:37:31 +0100
Committer: Ingo Molnar
CommitDate: Sun, 4 Nov 2018 00:57:44 +0100
sched/core: Take the hotplug lock in sched_init_smp()
When running on linux-next (8c60c36d0b8c ("Add linux-next specific files
for 20181019")) + CONFIG_PROVE_LOCKING=y on a big.LITTLE system (e.g.
Juno or HiKey960), we get the following report:
[0.748225] Call trace:
[0.750685] lockdep_assert_cpus_held+0x30/0x40
[0.755236] static_key_enable_cpuslocked+0x20/0xc8
[0.760137] build_sched_domains+0x1034/0x1108
[0.764601] sched_init_domains+0x68/0x90
[0.768628] sched_init_smp+0x30/0x80
[0.772309] kernel_init_freeable+0x278/0x51c
[0.776685] kernel_init+0x10/0x108
[0.780190] ret_from_fork+0x10/0x18
The static_key in question is 'sched_asym_cpucapacity' introduced by
commit:
df054e8445a4 ("sched/topology: Add static_key for asymmetric CPU capacity
optimizations")
In this particular case, we enable it because smp_prepare_cpus() will
end up fetching the capacity-dmips-mhz entry from the devicetree,
so we already have some asymmetry detected when entering sched_init_smp().
This didn't get detected in tip/sched/core because we were missing:
commit cb538267ea1e ("jump_label/lockdep: Assert we hold the hotplug lock for
_cpuslocked() operations")
Calls to build_sched_domains() post sched_init_smp() will hold the
hotplug lock, it just so happens that this very first call is a
special case. As stated by a comment in sched_init_smp(), "There's no
userspace yet to cause hotplug operations" so this is a harmless
warning.
However, to both respect the semantics of underlying
callees and make lockdep happy, take the hotplug lock in
sched_init_smp(). This also satisfies the comment atop
sched_init_domains() that says "Callers must hold the hotplug lock".
Reported-by: Sudeep Holla
Tested-by: Sudeep Holla
Signed-off-by: Valentin Schneider
Signed-off-by: Peter Zijlstra (Intel)
Cc: dietmar.eggem...@arm.com
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: morten.rasmus...@arm.com
Cc: quentin.per...@arm.com
Link:
http://lkml.kernel.org/r/1540301851-3048-1-git-send-email-valentin.schnei...@arm.com
Signed-off-by: Ingo Molnar
---
kernel/sched/core.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fd2fce8a001b..02a20ef196a6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5859,11 +5859,14 @@ void __init sched_init_smp(void)
/*
* There's no userspace yet to cause hotplug operations; hence all the
* CPU masks are stable and all blatant races in the below code cannot
-* happen.
+* happen. The hotplug lock is nevertheless taken to satisfy lockdep,
+* but there won't be any contention on it.
*/
+ cpus_read_lock();
mutex_lock(_domains_mutex);
sched_init_domains(cpu_active_mask);
mutex_unlock(_domains_mutex);
+ cpus_read_unlock();
/* Move init over to a non-isolated CPU */
if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_FLAG_DOMAIN))
< 0)
RE: [PATCH v3] genirq/matrix: Choose CPU for managed IRQs based on how many of them are allocated
> Subject: Re: [PATCH v3] genirq/matrix: Choose CPU for managed IRQs based > on how many of them are allocated > > On Sat, 3 Nov 2018, Thomas Gleixner wrote: > > On Fri, 2 Nov 2018, Long Li wrote: > > > /** > > > * irq_matrix_assign_system - Assign system wide entry in the matrix > > > * @m: Matrix pointer > > > @@ -269,7 +291,7 @@ int irq_matrix_alloc_managed(struct irq_matrix > *m, const struct cpumask *msk, > > > if (cpumask_empty(msk)) > > > return -EINVAL; > > > > > > - cpu = matrix_find_best_cpu(m, msk); > > > + cpu = matrix_find_best_cpu_managed(m, msk); > > > if (cpu == UINT_MAX) > > > return -ENOSPC; > > > > > > @@ -282,6 +304,7 @@ int irq_matrix_alloc_managed(struct irq_matrix > *m, const struct cpumask *msk, > > > return -ENOSPC; > > > set_bit(bit, cm->alloc_map); > > > cm->allocated++; > > > + cm->managed_allocated++; > > > m->total_allocated++; > > > *mapped_cpu = cpu; > > > trace_irq_matrix_alloc_managed(bit, cpu, m, cm); > > > > so far so good. But what exactly decrements managed_allocated ? > > Another thing. If we add that counter, then it would be good to expose it in > the debugfs files as well. I will send an update to address those. Long > > Thanks, > > tglx
RE: [PATCH v3] genirq/matrix: Choose CPU for managed IRQs based on how many of them are allocated
> Subject: Re: [PATCH v3] genirq/matrix: Choose CPU for managed IRQs based > on how many of them are allocated > > On Sat, 3 Nov 2018, Thomas Gleixner wrote: > > On Fri, 2 Nov 2018, Long Li wrote: > > > /** > > > * irq_matrix_assign_system - Assign system wide entry in the matrix > > > * @m: Matrix pointer > > > @@ -269,7 +291,7 @@ int irq_matrix_alloc_managed(struct irq_matrix > *m, const struct cpumask *msk, > > > if (cpumask_empty(msk)) > > > return -EINVAL; > > > > > > - cpu = matrix_find_best_cpu(m, msk); > > > + cpu = matrix_find_best_cpu_managed(m, msk); > > > if (cpu == UINT_MAX) > > > return -ENOSPC; > > > > > > @@ -282,6 +304,7 @@ int irq_matrix_alloc_managed(struct irq_matrix > *m, const struct cpumask *msk, > > > return -ENOSPC; > > > set_bit(bit, cm->alloc_map); > > > cm->allocated++; > > > + cm->managed_allocated++; > > > m->total_allocated++; > > > *mapped_cpu = cpu; > > > trace_irq_matrix_alloc_managed(bit, cpu, m, cm); > > > > so far so good. But what exactly decrements managed_allocated ? > > Another thing. If we add that counter, then it would be good to expose it in > the debugfs files as well. I will send an update to address those. Long > > Thanks, > > tglx
[GIT PULL] scheduler fixes
Linus, Please pull the latest sched-urgent-for-linus git tree from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched-urgent-for-linus # HEAD: 993f0b0510dad98b4e6e39506834dab0d13fd539 sched/topology: Fix off by one bug A memory (under-)allocation fix and a comment fix. Thanks, Ingo --> Muchun Song (1): sched/rt: Update comment in pick_next_task_rt() Peter Zijlstra (1): sched/topology: Fix off by one bug kernel/sched/rt.c | 2 +- kernel/sched/topology.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 2e2955a8cf8f..a21ea6021929 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1561,7 +1561,7 @@ pick_next_task_rt(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) /* * We may dequeue prev's rt_rq in put_prev_task(). -* So, we update time before rt_nr_running check. +* So, we update time before rt_queued check. */ if (prev->sched_class == _sched_class) update_curr_rt(rq); diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c index 9d74371e4aad..8d7f15ba5916 100644 --- a/kernel/sched/topology.c +++ b/kernel/sched/topology.c @@ -1337,7 +1337,7 @@ void sched_init_numa(void) int level = 0; int i, j, k; - sched_domains_numa_distance = kzalloc(sizeof(int) * nr_node_ids, GFP_KERNEL); + sched_domains_numa_distance = kzalloc(sizeof(int) * (nr_node_ids + 1), GFP_KERNEL); if (!sched_domains_numa_distance) return;
[GIT PULL] scheduler fixes
Linus, Please pull the latest sched-urgent-for-linus git tree from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched-urgent-for-linus # HEAD: 993f0b0510dad98b4e6e39506834dab0d13fd539 sched/topology: Fix off by one bug A memory (under-)allocation fix and a comment fix. Thanks, Ingo --> Muchun Song (1): sched/rt: Update comment in pick_next_task_rt() Peter Zijlstra (1): sched/topology: Fix off by one bug kernel/sched/rt.c | 2 +- kernel/sched/topology.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 2e2955a8cf8f..a21ea6021929 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1561,7 +1561,7 @@ pick_next_task_rt(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) /* * We may dequeue prev's rt_rq in put_prev_task(). -* So, we update time before rt_nr_running check. +* So, we update time before rt_queued check. */ if (prev->sched_class == _sched_class) update_curr_rt(rq); diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c index 9d74371e4aad..8d7f15ba5916 100644 --- a/kernel/sched/topology.c +++ b/kernel/sched/topology.c @@ -1337,7 +1337,7 @@ void sched_init_numa(void) int level = 0; int i, j, k; - sched_domains_numa_distance = kzalloc(sizeof(int) * nr_node_ids, GFP_KERNEL); + sched_domains_numa_distance = kzalloc(sizeof(int) * (nr_node_ids + 1), GFP_KERNEL); if (!sched_domains_numa_distance) return;
[PATCH RFC v2 2/3] pstore: simplify ramoops_get_next_prz arguments
(1) remove type argument from ramoops_get_next_prz
Since we store the type of the prz when we initialize it, we no longer
need to pass it again in ramoops_get_next_prz since we can just use that
to setup the pstore record. So lets remove it from the argument list.
(2) remove max argument from ramoops_get_next_prz
>From the code flow, the 'max' checks are already being done on the prz
passed to ramoops_get_next_prz. Lets remove it to simplify this function
and reduce its arguments.
(3) further reduce ramoops_get_next_prz arguments by passing record
Both the id and type fields of a pstore_record are set by
ramoops_get_next_prz. So we can just pass a pointer to the pstore_record
instead of passing individual elements. This results in cleaner more
readable code and fewer lines.
In addition lets also remove the 'update' argument since we can detect
that. Changes are squashed into a single patch to reduce fixup conflicts.
Signed-off-by: Joel Fernandes (Google)
---
fs/pstore/ram.c | 48 ++--
1 file changed, 18 insertions(+), 30 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index b174d0fc009f..202eaa82bcc6 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -124,19 +124,17 @@ static int ramoops_pstore_open(struct pstore_info *psi)
}
static struct persistent_ram_zone *
-ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max,
-u64 *id,
-enum pstore_type_id *typep, enum pstore_type_id type,
-bool update)
+ramoops_get_next_prz(struct persistent_ram_zone *przs[], int id,
+struct pstore_record *record)
{
struct persistent_ram_zone *prz;
- int i = (*c)++;
+ bool update = (record->type == PSTORE_TYPE_DMESG);
/* Give up if we never existed or have hit the end. */
- if (!przs || i >= max)
+ if (!przs)
return NULL;
- prz = przs[i];
+ prz = przs[id];
if (!prz)
return NULL;
@@ -147,8 +145,8 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[],
uint *c, uint max,
if (!persistent_ram_old_size(prz))
return NULL;
- *typep = type;
- *id = i;
+ record->type = prz->type;
+ record->id = id;
return prz;
}
@@ -255,10 +253,8 @@ static ssize_t ramoops_pstore_read(struct pstore_record
*record)
/* Find the next valid persistent_ram_zone for DMESG */
while (cxt->dump_read_cnt < cxt->max_dump_cnt && !prz) {
- prz = ramoops_get_next_prz(cxt->dprzs, >dump_read_cnt,
- cxt->max_dump_cnt, >id,
- >type,
- PSTORE_TYPE_DMESG, 1);
+ prz = ramoops_get_next_prz(cxt->dprzs, cxt->dump_read_cnt++,
+ record);
if (!prz_ok(prz))
continue;
header_length = ramoops_read_kmsg_hdr(persistent_ram_old(prz),
@@ -272,22 +268,18 @@ static ssize_t ramoops_pstore_read(struct pstore_record
*record)
}
}
- if (!prz_ok(prz))
- prz = ramoops_get_next_prz(>cprz, >console_read_cnt,
- 1, >id, >type,
- PSTORE_TYPE_CONSOLE, 0);
+ if (!prz_ok(prz) && !cxt->console_read_cnt++)
+ prz = ramoops_get_next_prz(>cprz, 0 /* single */, record);
- if (!prz_ok(prz))
- prz = ramoops_get_next_prz(>mprz, >pmsg_read_cnt,
- 1, >id, >type,
- PSTORE_TYPE_PMSG, 0);
+ if (!prz_ok(prz) && !cxt->pmsg_read_cnt++)
+ prz = ramoops_get_next_prz(>mprz, 0 /* single */, record);
/* ftrace is last since it may want to dynamically allocate memory. */
if (!prz_ok(prz)) {
- if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU)) {
- prz = ramoops_get_next_prz(cxt->fprzs,
- >ftrace_read_cnt, 1, >id,
- >type, PSTORE_TYPE_FTRACE, 0);
+ if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) &&
+ !cxt->ftrace_read_cnt++) {
+ prz = ramoops_get_next_prz(cxt->fprzs, 0 /* single */,
+ record);
} else {
/*
* Build a new dummy record which combines all the
@@ -303,11 +295,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record
*record)
while (cxt->ftrace_read_cnt < cxt->max_ftrace_cnt) {
prz_next = ramoops_get_next_prz(cxt->fprzs,
- >ftrace_read_cnt,
-
[PATCH RFC v2 2/3] pstore: simplify ramoops_get_next_prz arguments
(1) remove type argument from ramoops_get_next_prz
Since we store the type of the prz when we initialize it, we no longer
need to pass it again in ramoops_get_next_prz since we can just use that
to setup the pstore record. So lets remove it from the argument list.
(2) remove max argument from ramoops_get_next_prz
>From the code flow, the 'max' checks are already being done on the prz
passed to ramoops_get_next_prz. Lets remove it to simplify this function
and reduce its arguments.
(3) further reduce ramoops_get_next_prz arguments by passing record
Both the id and type fields of a pstore_record are set by
ramoops_get_next_prz. So we can just pass a pointer to the pstore_record
instead of passing individual elements. This results in cleaner more
readable code and fewer lines.
In addition lets also remove the 'update' argument since we can detect
that. Changes are squashed into a single patch to reduce fixup conflicts.
Signed-off-by: Joel Fernandes (Google)
---
fs/pstore/ram.c | 48 ++--
1 file changed, 18 insertions(+), 30 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index b174d0fc009f..202eaa82bcc6 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -124,19 +124,17 @@ static int ramoops_pstore_open(struct pstore_info *psi)
}
static struct persistent_ram_zone *
-ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max,
-u64 *id,
-enum pstore_type_id *typep, enum pstore_type_id type,
-bool update)
+ramoops_get_next_prz(struct persistent_ram_zone *przs[], int id,
+struct pstore_record *record)
{
struct persistent_ram_zone *prz;
- int i = (*c)++;
+ bool update = (record->type == PSTORE_TYPE_DMESG);
/* Give up if we never existed or have hit the end. */
- if (!przs || i >= max)
+ if (!przs)
return NULL;
- prz = przs[i];
+ prz = przs[id];
if (!prz)
return NULL;
@@ -147,8 +145,8 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[],
uint *c, uint max,
if (!persistent_ram_old_size(prz))
return NULL;
- *typep = type;
- *id = i;
+ record->type = prz->type;
+ record->id = id;
return prz;
}
@@ -255,10 +253,8 @@ static ssize_t ramoops_pstore_read(struct pstore_record
*record)
/* Find the next valid persistent_ram_zone for DMESG */
while (cxt->dump_read_cnt < cxt->max_dump_cnt && !prz) {
- prz = ramoops_get_next_prz(cxt->dprzs, >dump_read_cnt,
- cxt->max_dump_cnt, >id,
- >type,
- PSTORE_TYPE_DMESG, 1);
+ prz = ramoops_get_next_prz(cxt->dprzs, cxt->dump_read_cnt++,
+ record);
if (!prz_ok(prz))
continue;
header_length = ramoops_read_kmsg_hdr(persistent_ram_old(prz),
@@ -272,22 +268,18 @@ static ssize_t ramoops_pstore_read(struct pstore_record
*record)
}
}
- if (!prz_ok(prz))
- prz = ramoops_get_next_prz(>cprz, >console_read_cnt,
- 1, >id, >type,
- PSTORE_TYPE_CONSOLE, 0);
+ if (!prz_ok(prz) && !cxt->console_read_cnt++)
+ prz = ramoops_get_next_prz(>cprz, 0 /* single */, record);
- if (!prz_ok(prz))
- prz = ramoops_get_next_prz(>mprz, >pmsg_read_cnt,
- 1, >id, >type,
- PSTORE_TYPE_PMSG, 0);
+ if (!prz_ok(prz) && !cxt->pmsg_read_cnt++)
+ prz = ramoops_get_next_prz(>mprz, 0 /* single */, record);
/* ftrace is last since it may want to dynamically allocate memory. */
if (!prz_ok(prz)) {
- if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU)) {
- prz = ramoops_get_next_prz(cxt->fprzs,
- >ftrace_read_cnt, 1, >id,
- >type, PSTORE_TYPE_FTRACE, 0);
+ if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) &&
+ !cxt->ftrace_read_cnt++) {
+ prz = ramoops_get_next_prz(cxt->fprzs, 0 /* single */,
+ record);
} else {
/*
* Build a new dummy record which combines all the
@@ -303,11 +295,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record
*record)
while (cxt->ftrace_read_cnt < cxt->max_ftrace_cnt) {
prz_next = ramoops_get_next_prz(cxt->fprzs,
- >ftrace_read_cnt,
-
[PATCH RFC v2 0/3] cleanups for pstore and ramoops
Here are some simple cleanups and fixes for ramoops in pstore. Let me know what you think, thanks. Joel Fernandes (Google) (3): pstore: map pstore types to names pstore: simplify ramoops_get_next_prz arguments pstore: donot treat empty buffers as valid fs/pstore/inode.c | 53 +- fs/pstore/ram.c| 52 +++-- fs/pstore/ram_core.c | 2 +- include/linux/pstore.h | 37 ++ include/linux/pstore_ram.h | 2 ++ 5 files changed, 67 insertions(+), 79 deletions(-) -- 2.19.1.930.g4563a0d9d0-goog
[PATCH RFC v2 1/3] pstore: map pstore types to names
In later patches we will need to map types to names, so create a table
for that which can also be used and reused in different parts of old and
new code. Also use it to save the type in the PRZ which will be useful
in later patches.
Signed-off-by: Joel Fernandes (Google)
---
fs/pstore/inode.c | 53 +-
fs/pstore/ram.c| 4 ++-
include/linux/pstore.h | 37 ++
include/linux/pstore_ram.h | 2 ++
4 files changed, 48 insertions(+), 48 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 8cf2218b46a7..c5c6b8b4b70a 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -304,6 +304,7 @@ int pstore_mkfile(struct dentry *root, struct pstore_record
*record)
struct dentry *dentry;
struct inode*inode;
int rc = 0;
+ enum pstore_type_id type;
charname[PSTORE_NAMELEN];
struct pstore_private *private, *pos;
unsigned long flags;
@@ -335,53 +336,11 @@ int pstore_mkfile(struct dentry *root, struct
pstore_record *record)
goto fail_alloc;
private->record = record;
- switch (record->type) {
- case PSTORE_TYPE_DMESG:
- scnprintf(name, sizeof(name), "dmesg-%s-%llu%s",
- record->psi->name, record->id,
- record->compressed ? ".enc.z" : "");
- break;
- case PSTORE_TYPE_CONSOLE:
- scnprintf(name, sizeof(name), "console-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_FTRACE:
- scnprintf(name, sizeof(name), "ftrace-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_MCE:
- scnprintf(name, sizeof(name), "mce-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PPC_RTAS:
- scnprintf(name, sizeof(name), "rtas-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PPC_OF:
- scnprintf(name, sizeof(name), "powerpc-ofw-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PPC_COMMON:
- scnprintf(name, sizeof(name), "powerpc-common-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PMSG:
- scnprintf(name, sizeof(name), "pmsg-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PPC_OPAL:
- scnprintf(name, sizeof(name), "powerpc-opal-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_UNKNOWN:
- scnprintf(name, sizeof(name), "unknown-%s-%llu",
- record->psi->name, record->id);
- break;
- default:
- scnprintf(name, sizeof(name), "type%d-%s-%llu",
- record->type, record->psi->name, record->id);
- break;
- }
+ scnprintf(name, sizeof(name), "%s-%s-%llu%s",
+ pstore_type_to_name(record->type),
+ record->psi->name, record->id,
+ (record->type == PSTORE_TYPE_DMESG
+&& record->compressed) ? ".enc.z" : "");
dentry = d_alloc_name(root, name);
if (!dentry)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 10ac4d23c423..b174d0fc009f 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -611,6 +611,7 @@ static int ramoops_init_przs(const char *name,
goto fail;
}
*paddr += zone_sz;
+ prz_ar[i]->type = pstore_name_to_type(name);
}
*przs = prz_ar;
@@ -650,6 +651,7 @@ static int ramoops_init_prz(const char *name,
}
*paddr += sz;
+ (*prz)->type = pstore_name_to_type(name);
return 0;
}
@@ -785,7 +787,7 @@ static int ramoops_probe(struct platform_device *pdev)
dump_mem_sz = cxt->size - cxt->console_size - cxt->ftrace_size
- cxt->pmsg_size;
- err = ramoops_init_przs("dump", dev, cxt, >dprzs, ,
+ err = ramoops_init_przs("dmesg", dev, cxt, >dprzs, ,
dump_mem_sz, cxt->record_size,
>max_dump_cnt, 0, 0);
if (err)
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index f46e5df76b58..caeeda0bbab3 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -44,9 +44,46 @@ enum pstore_type_id {
PSTORE_TYPE_PPC_COMMON = 6,
PSTORE_TYPE_PMSG= 7,
PSTORE_TYPE_PPC_OPAL= 8,
+
[PATCH RFC v2 3/3] pstore: donot treat empty buffers as valid
pstore currently calls persistent_ram_save_old even if a buffer is
empty. While this appears to work, it is does not seem like the right
thing to do and could lead to future bugs so lets avoid that. It also
prevent misleading prints in the logs which claim the buffer is valid.
I got something like:
found existing buffer, size 0, start 0
When I was expecting:
no valid data in buffer (sig = ...)
Signed-off-by: Joel Fernandes (Google)
---
Note that if you feel this patch is not necessary, then feel free to
drop it. I would say it is harmless and is a good clean up.
fs/pstore/ram_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index e6375439c5ac..196e4fd7ba8c 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -510,7 +510,7 @@ static int persistent_ram_post_init(struct
persistent_ram_zone *prz, u32 sig,
sig ^= PERSISTENT_RAM_SIG;
- if (prz->buffer->sig == sig) {
+ if (prz->buffer->sig == sig && buffer_size(prz)) {
if (buffer_size(prz) > prz->buffer_size ||
buffer_start(prz) > buffer_size(prz)) {
pr_info("found existing invalid buffer, size %zu, start
%zu\n",
--
2.19.1.930.g4563a0d9d0-goog
[PATCH RFC v2 0/3] cleanups for pstore and ramoops
Here are some simple cleanups and fixes for ramoops in pstore. Let me know what you think, thanks. Joel Fernandes (Google) (3): pstore: map pstore types to names pstore: simplify ramoops_get_next_prz arguments pstore: donot treat empty buffers as valid fs/pstore/inode.c | 53 +- fs/pstore/ram.c| 52 +++-- fs/pstore/ram_core.c | 2 +- include/linux/pstore.h | 37 ++ include/linux/pstore_ram.h | 2 ++ 5 files changed, 67 insertions(+), 79 deletions(-) -- 2.19.1.930.g4563a0d9d0-goog
[PATCH RFC v2 1/3] pstore: map pstore types to names
In later patches we will need to map types to names, so create a table
for that which can also be used and reused in different parts of old and
new code. Also use it to save the type in the PRZ which will be useful
in later patches.
Signed-off-by: Joel Fernandes (Google)
---
fs/pstore/inode.c | 53 +-
fs/pstore/ram.c| 4 ++-
include/linux/pstore.h | 37 ++
include/linux/pstore_ram.h | 2 ++
4 files changed, 48 insertions(+), 48 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 8cf2218b46a7..c5c6b8b4b70a 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -304,6 +304,7 @@ int pstore_mkfile(struct dentry *root, struct pstore_record
*record)
struct dentry *dentry;
struct inode*inode;
int rc = 0;
+ enum pstore_type_id type;
charname[PSTORE_NAMELEN];
struct pstore_private *private, *pos;
unsigned long flags;
@@ -335,53 +336,11 @@ int pstore_mkfile(struct dentry *root, struct
pstore_record *record)
goto fail_alloc;
private->record = record;
- switch (record->type) {
- case PSTORE_TYPE_DMESG:
- scnprintf(name, sizeof(name), "dmesg-%s-%llu%s",
- record->psi->name, record->id,
- record->compressed ? ".enc.z" : "");
- break;
- case PSTORE_TYPE_CONSOLE:
- scnprintf(name, sizeof(name), "console-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_FTRACE:
- scnprintf(name, sizeof(name), "ftrace-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_MCE:
- scnprintf(name, sizeof(name), "mce-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PPC_RTAS:
- scnprintf(name, sizeof(name), "rtas-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PPC_OF:
- scnprintf(name, sizeof(name), "powerpc-ofw-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PPC_COMMON:
- scnprintf(name, sizeof(name), "powerpc-common-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PMSG:
- scnprintf(name, sizeof(name), "pmsg-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_PPC_OPAL:
- scnprintf(name, sizeof(name), "powerpc-opal-%s-%llu",
- record->psi->name, record->id);
- break;
- case PSTORE_TYPE_UNKNOWN:
- scnprintf(name, sizeof(name), "unknown-%s-%llu",
- record->psi->name, record->id);
- break;
- default:
- scnprintf(name, sizeof(name), "type%d-%s-%llu",
- record->type, record->psi->name, record->id);
- break;
- }
+ scnprintf(name, sizeof(name), "%s-%s-%llu%s",
+ pstore_type_to_name(record->type),
+ record->psi->name, record->id,
+ (record->type == PSTORE_TYPE_DMESG
+&& record->compressed) ? ".enc.z" : "");
dentry = d_alloc_name(root, name);
if (!dentry)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 10ac4d23c423..b174d0fc009f 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -611,6 +611,7 @@ static int ramoops_init_przs(const char *name,
goto fail;
}
*paddr += zone_sz;
+ prz_ar[i]->type = pstore_name_to_type(name);
}
*przs = prz_ar;
@@ -650,6 +651,7 @@ static int ramoops_init_prz(const char *name,
}
*paddr += sz;
+ (*prz)->type = pstore_name_to_type(name);
return 0;
}
@@ -785,7 +787,7 @@ static int ramoops_probe(struct platform_device *pdev)
dump_mem_sz = cxt->size - cxt->console_size - cxt->ftrace_size
- cxt->pmsg_size;
- err = ramoops_init_przs("dump", dev, cxt, >dprzs, ,
+ err = ramoops_init_przs("dmesg", dev, cxt, >dprzs, ,
dump_mem_sz, cxt->record_size,
>max_dump_cnt, 0, 0);
if (err)
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index f46e5df76b58..caeeda0bbab3 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -44,9 +44,46 @@ enum pstore_type_id {
PSTORE_TYPE_PPC_COMMON = 6,
PSTORE_TYPE_PMSG= 7,
PSTORE_TYPE_PPC_OPAL= 8,
+
[PATCH RFC v2 3/3] pstore: donot treat empty buffers as valid
pstore currently calls persistent_ram_save_old even if a buffer is
empty. While this appears to work, it is does not seem like the right
thing to do and could lead to future bugs so lets avoid that. It also
prevent misleading prints in the logs which claim the buffer is valid.
I got something like:
found existing buffer, size 0, start 0
When I was expecting:
no valid data in buffer (sig = ...)
Signed-off-by: Joel Fernandes (Google)
---
Note that if you feel this patch is not necessary, then feel free to
drop it. I would say it is harmless and is a good clean up.
fs/pstore/ram_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index e6375439c5ac..196e4fd7ba8c 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -510,7 +510,7 @@ static int persistent_ram_post_init(struct
persistent_ram_zone *prz, u32 sig,
sig ^= PERSISTENT_RAM_SIG;
- if (prz->buffer->sig == sig) {
+ if (prz->buffer->sig == sig && buffer_size(prz)) {
if (buffer_size(prz) > prz->buffer_size ||
buffer_start(prz) > buffer_size(prz)) {
pr_info("found existing invalid buffer, size %zu, start
%zu\n",
--
2.19.1.930.g4563a0d9d0-goog
Re: [RFC] doc: rcu: remove note on smp_mb during synchronize_rcu
On Fri, Nov 02, 2018 at 10:12:26PM -0700, Joel Fernandes wrote: > On Thu, Nov 01, 2018 at 09:13:07AM -0700, Paul E. McKenney wrote: > > On Wed, Oct 31, 2018 at 10:00:19PM -0700, Joel Fernandes wrote: > > > On Wed, Oct 31, 2018 at 11:17:48AM -0700, Paul E. McKenney wrote: > > > > On Tue, Oct 30, 2018 at 06:11:19PM -0700, Joel Fernandes wrote: > > > > > Hi Paul, > > > > > > > > > > On Tue, Oct 30, 2018 at 04:43:36PM -0700, Paul E. McKenney wrote: > > > > > > On Tue, Oct 30, 2018 at 03:26:49PM -0700, Joel Fernandes wrote: > > > > > > > Hi Paul, > > > > > > > > > > > > > > On Sat, Oct 27, 2018 at 09:30:46PM -0700, Joel Fernandes (Google) > > > > > > > wrote: > > > > > > > > As per this thread [1], it seems this smp_mb isn't needed > > > > > > > > anymore: > > > > > > > > "So the smp_mb() that I was trying to add doesn't need to be > > > > > > > > there." > > > > > > > > > > > > > > > > So let us remove this part from the memory ordering > > > > > > > > documentation. > > > > > > > > > > > > > > > > [1] https://lkml.org/lkml/2017/10/6/707 > > > > > > > > > > > > > > > > Signed-off-by: Joel Fernandes (Google) > > > > > > > > > > > > > > I was just checking about this patch. Do you feel it is correct > > > > > > > to remove > > > > > > > this part from the docs? Are you satisified that a barrier isn't > > > > > > > needed there > > > > > > > now? Or did I miss something? > > > > > > > > > > > > Apologies, it got lost in the shuffle. I have now applied it with a > > > > > > bit of rework to the commit log, thank you! > > > > > > > > > > No worries, thanks for taking it! > > > > > > > > > > Just wanted to update you on my progress reading/correcting the docs. > > > > > The > > > > > 'Memory Ordering' is taking a bit of time so I paused that and I'm > > > > > focusing > > > > > on finishing all the other low hanging fruit. This activity is mostly > > > > > during > > > > > night hours after the baby is asleep but some times I also manage to > > > > > sneak it > > > > > into the day job ;-) > > > > > > > > If there is anything I can do to make this a more sustainable task for > > > > you, please do not keep it a secret!!! > > > > > > Thanks a lot, that means a lot to me! Will do! > > > > > > > > BTW I do want to discuss about this smp_mb patch above with you at > > > > > LPC if you > > > > > had time, even though we are removing it from the documentation. I > > > > > thought > > > > > about it a few times, and I was not able to fully appreciate the need > > > > > for the > > > > > barrier (that is even assuming that complete() etc did not do the > > > > > right > > > > > thing). Specifically I was wondering same thing Peter said in the > > > > > above > > > > > thread I think that - if that rcu_read_unlock() triggered all the spin > > > > > locking up the tree of nodes, then why is that locking not sufficient > > > > > to > > > > > prevent reads from the read-side section from bleeding out? That would > > > > > prevent the reader that just unlocked from seeing anything that > > > > > happens > > > > > _after_ the synchronize_rcu. > > > > > > > > Actually, I recall an smp_mb() being added, but am not seeing it > > > > anywhere > > > > relevant to wait_for_completion(). So I might need to add the smp_mb() > > > > to synchronize_rcu() and remove the patch (retaining the typo fix). :-/ > > > > > > No problem, I'm glad atleast the patch resurfaced the topic of the > > > potential > > > issue :-) > > > > And an smp_mb() is needed in Tree RCU's __wait_rcu_gp(). This is > > because wait_for_completion() might get a "fly-by" wakeup, which would > > mean no ordering for code naively thinking that it was ordered after a > > grace period. > > > > > > The short form answer is that anything before a grace period on any CPU > > > > must be seen by any CPU as being before anything on any CPU after that > > > > same grace period. This guarantee requires a rather big hammer. > > > > > > > > But yes, let's talk at LPC! > > > > > > Sounds great, looking forward to discussing this. > > > > Would it make sense to have an RCU-implementation BoF? > > > > > > > Also about GP memory ordering and RCU-tree-locking, I think you > > > > > mentioned to > > > > > me that the RCU reader-sections are virtually extended both forward > > > > > and > > > > > backward and whereever it ends, those paths do heavy-weight > > > > > synchronization > > > > > that should be sufficient to prevent memory ordering issues (such as > > > > > those > > > > > you mentioned in the Requierments document). That is exactly why we > > > > > don't > > > > > need explicit barriers during rcu_read_unlock. If I recall I asked > > > > > you why > > > > > those are not needed. So that answer made sense, but then now on going > > > > > through the 'Memory Ordering' document, I see that you mentioned > > > > > there is > > > > > reliance on the locking. Is that reliance on locking necessary to > > > > > maintain > > > > >
Re: [RFC] doc: rcu: remove note on smp_mb during synchronize_rcu
On Fri, Nov 02, 2018 at 10:12:26PM -0700, Joel Fernandes wrote: > On Thu, Nov 01, 2018 at 09:13:07AM -0700, Paul E. McKenney wrote: > > On Wed, Oct 31, 2018 at 10:00:19PM -0700, Joel Fernandes wrote: > > > On Wed, Oct 31, 2018 at 11:17:48AM -0700, Paul E. McKenney wrote: > > > > On Tue, Oct 30, 2018 at 06:11:19PM -0700, Joel Fernandes wrote: > > > > > Hi Paul, > > > > > > > > > > On Tue, Oct 30, 2018 at 04:43:36PM -0700, Paul E. McKenney wrote: > > > > > > On Tue, Oct 30, 2018 at 03:26:49PM -0700, Joel Fernandes wrote: > > > > > > > Hi Paul, > > > > > > > > > > > > > > On Sat, Oct 27, 2018 at 09:30:46PM -0700, Joel Fernandes (Google) > > > > > > > wrote: > > > > > > > > As per this thread [1], it seems this smp_mb isn't needed > > > > > > > > anymore: > > > > > > > > "So the smp_mb() that I was trying to add doesn't need to be > > > > > > > > there." > > > > > > > > > > > > > > > > So let us remove this part from the memory ordering > > > > > > > > documentation. > > > > > > > > > > > > > > > > [1] https://lkml.org/lkml/2017/10/6/707 > > > > > > > > > > > > > > > > Signed-off-by: Joel Fernandes (Google) > > > > > > > > > > > > > > I was just checking about this patch. Do you feel it is correct > > > > > > > to remove > > > > > > > this part from the docs? Are you satisified that a barrier isn't > > > > > > > needed there > > > > > > > now? Or did I miss something? > > > > > > > > > > > > Apologies, it got lost in the shuffle. I have now applied it with a > > > > > > bit of rework to the commit log, thank you! > > > > > > > > > > No worries, thanks for taking it! > > > > > > > > > > Just wanted to update you on my progress reading/correcting the docs. > > > > > The > > > > > 'Memory Ordering' is taking a bit of time so I paused that and I'm > > > > > focusing > > > > > on finishing all the other low hanging fruit. This activity is mostly > > > > > during > > > > > night hours after the baby is asleep but some times I also manage to > > > > > sneak it > > > > > into the day job ;-) > > > > > > > > If there is anything I can do to make this a more sustainable task for > > > > you, please do not keep it a secret!!! > > > > > > Thanks a lot, that means a lot to me! Will do! > > > > > > > > BTW I do want to discuss about this smp_mb patch above with you at > > > > > LPC if you > > > > > had time, even though we are removing it from the documentation. I > > > > > thought > > > > > about it a few times, and I was not able to fully appreciate the need > > > > > for the > > > > > barrier (that is even assuming that complete() etc did not do the > > > > > right > > > > > thing). Specifically I was wondering same thing Peter said in the > > > > > above > > > > > thread I think that - if that rcu_read_unlock() triggered all the spin > > > > > locking up the tree of nodes, then why is that locking not sufficient > > > > > to > > > > > prevent reads from the read-side section from bleeding out? That would > > > > > prevent the reader that just unlocked from seeing anything that > > > > > happens > > > > > _after_ the synchronize_rcu. > > > > > > > > Actually, I recall an smp_mb() being added, but am not seeing it > > > > anywhere > > > > relevant to wait_for_completion(). So I might need to add the smp_mb() > > > > to synchronize_rcu() and remove the patch (retaining the typo fix). :-/ > > > > > > No problem, I'm glad atleast the patch resurfaced the topic of the > > > potential > > > issue :-) > > > > And an smp_mb() is needed in Tree RCU's __wait_rcu_gp(). This is > > because wait_for_completion() might get a "fly-by" wakeup, which would > > mean no ordering for code naively thinking that it was ordered after a > > grace period. > > > > > > The short form answer is that anything before a grace period on any CPU > > > > must be seen by any CPU as being before anything on any CPU after that > > > > same grace period. This guarantee requires a rather big hammer. > > > > > > > > But yes, let's talk at LPC! > > > > > > Sounds great, looking forward to discussing this. > > > > Would it make sense to have an RCU-implementation BoF? > > > > > > > Also about GP memory ordering and RCU-tree-locking, I think you > > > > > mentioned to > > > > > me that the RCU reader-sections are virtually extended both forward > > > > > and > > > > > backward and whereever it ends, those paths do heavy-weight > > > > > synchronization > > > > > that should be sufficient to prevent memory ordering issues (such as > > > > > those > > > > > you mentioned in the Requierments document). That is exactly why we > > > > > don't > > > > > need explicit barriers during rcu_read_unlock. If I recall I asked > > > > > you why > > > > > those are not needed. So that answer made sense, but then now on going > > > > > through the 'Memory Ordering' document, I see that you mentioned > > > > > there is > > > > > reliance on the locking. Is that reliance on locking necessary to > > > > > maintain > > > > >
[GIT PULL] x86 fixes
Linus, Please pull the latest x86-urgent-for-linus git tree from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-for-linus # HEAD: 23a12ddee1ce28065b71f14ccc695b5a0c8a64ff Merge branch 'core/urgent' into x86/urgent, to pick up objtool fix A number of fixes and some late updates: - make in_compat_syscall() behavior on x86-32 similar to other platforms, this touches a number of generic files but is not intended to impact non-x86 platforms. - objtool fixes - PAT preemption fix - paravirt fixes/cleanups - cpufeatures updates for new instructions - earlyprintk quirk - make microcode version in sysfs world-readable (it is already world-readable in procfs) - minor cleanups and fixes out-of-topic modifications in x86-urgent-for-linus: - drivers/firmware/efi/efivars.c # 98f76206b335: compat: Cleanup in_compat_sy include/linux/compat.h # a846446b1914: x86/compat: Adjust in_compat kernel/time/time.c # 98f76206b335: compat: Cleanup in_compat_sy net/xfrm/xfrm_state.c # 98f76206b335: compat: Cleanup in_compat_sy net/xfrm/xfrm_user.c # 98f76206b335: compat: Cleanup in_compat_sy tools/objtool/check.c # 4a60aa05a063: objtool: Support per-functio tools/objtool/check.h # 4a60aa05a063: objtool: Support per-functio tools/objtool/elf.c# bcb6fb5da77c: objtool: Support GCC 9 cold # 4a60aa05a063: objtool: Support per-functio tools/objtool/elf.h# 4a60aa05a063: objtool: Support per-functio Thanks, Ingo --> Allan Xavier (1): objtool: Support per-function rodata sections Dave Jiang (1): x86/numa_emulation: Fix uniform-split numa emulation Dmitry Safonov (2): x86/compat: Adjust in_compat_syscall() to generic code under !COMPAT compat: Cleanup in_compat_syscall() callers Feng Tang (1): x86/earlyprintk: Add a force option for pciserial device Fenghua Yu (2): x86/cpufeatures: Enumerate MOVDIRI instruction x86/cpufeatures: Enumerate MOVDIR64B instruction Jacek Tomaka (1): x86/microcode: Make revision and processor flags world-readable Jordan Borgner (1): x86: Clean up 'sizeof x' => 'sizeof(x)' Josh Poimboeuf (1): objtool: Support GCC 9 cold subfunction naming scheme Juergen Gross (2): x86/paravirt: Remove GPL from pv_ops export x86/paravirt: Remove unused _paravirt_ident_32 Rasmus Villemoes (1): x86/traps: Use format string with panic() call Sebastian Andrzej Siewior (1): x86/mm/pat: Disable preemption around __flush_tlb_all() Documentation/admin-guide/kernel-parameters.txt | 6 +++- arch/x86/boot/cpucheck.c| 2 +- arch/x86/boot/early_serial_console.c| 4 +-- arch/x86/boot/edd.c | 6 ++-- arch/x86/boot/main.c| 4 +-- arch/x86/boot/memory.c | 2 +- arch/x86/boot/regs.c| 2 +- arch/x86/boot/video-vesa.c | 6 ++-- arch/x86/boot/video.c | 2 +- arch/x86/events/intel/core.c| 2 +- arch/x86/include/asm/compat.h | 9 +- arch/x86/include/asm/cpufeatures.h | 2 ++ arch/x86/include/asm/ftrace.h | 4 +-- arch/x86/include/asm/paravirt_types.h | 2 -- arch/x86/include/asm/tlbflush.h | 6 arch/x86/kernel/cpu/common.c| 4 +-- arch/x86/kernel/cpu/mcheck/mce.c| 2 +- arch/x86/kernel/cpu/microcode/core.c| 4 +-- arch/x86/kernel/cpu/mtrr/generic.c | 2 +- arch/x86/kernel/cpu/mtrr/if.c | 6 ++-- arch/x86/kernel/early_printk.c | 29 +++-- arch/x86/kernel/head64.c| 2 +- arch/x86/kernel/msr.c | 8 ++--- arch/x86/kernel/paravirt.c | 28 + arch/x86/kernel/paravirt_patch_32.c | 18 --- arch/x86/kernel/paravirt_patch_64.c | 20 arch/x86/kernel/process_64.c| 4 +-- arch/x86/kernel/sys_x86_64.c| 11 --- arch/x86/kernel/traps.c | 2 +- arch/x86/kvm/emulate.c | 22 ++--- arch/x86/kvm/lapic.c| 2 +- arch/x86/kvm/x86.c | 42 - arch/x86/mm/hugetlbpage.c | 4 +-- arch/x86/mm/mmap.c | 2 +- arch/x86/mm/numa_emulation.c| 12 +-- arch/x86/mm/pageattr.c | 6 +++- arch/x86/tools/relocs.c | 4 +--
[GIT PULL] x86 fixes
Linus, Please pull the latest x86-urgent-for-linus git tree from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-for-linus # HEAD: 23a12ddee1ce28065b71f14ccc695b5a0c8a64ff Merge branch 'core/urgent' into x86/urgent, to pick up objtool fix A number of fixes and some late updates: - make in_compat_syscall() behavior on x86-32 similar to other platforms, this touches a number of generic files but is not intended to impact non-x86 platforms. - objtool fixes - PAT preemption fix - paravirt fixes/cleanups - cpufeatures updates for new instructions - earlyprintk quirk - make microcode version in sysfs world-readable (it is already world-readable in procfs) - minor cleanups and fixes out-of-topic modifications in x86-urgent-for-linus: - drivers/firmware/efi/efivars.c # 98f76206b335: compat: Cleanup in_compat_sy include/linux/compat.h # a846446b1914: x86/compat: Adjust in_compat kernel/time/time.c # 98f76206b335: compat: Cleanup in_compat_sy net/xfrm/xfrm_state.c # 98f76206b335: compat: Cleanup in_compat_sy net/xfrm/xfrm_user.c # 98f76206b335: compat: Cleanup in_compat_sy tools/objtool/check.c # 4a60aa05a063: objtool: Support per-functio tools/objtool/check.h # 4a60aa05a063: objtool: Support per-functio tools/objtool/elf.c# bcb6fb5da77c: objtool: Support GCC 9 cold # 4a60aa05a063: objtool: Support per-functio tools/objtool/elf.h# 4a60aa05a063: objtool: Support per-functio Thanks, Ingo --> Allan Xavier (1): objtool: Support per-function rodata sections Dave Jiang (1): x86/numa_emulation: Fix uniform-split numa emulation Dmitry Safonov (2): x86/compat: Adjust in_compat_syscall() to generic code under !COMPAT compat: Cleanup in_compat_syscall() callers Feng Tang (1): x86/earlyprintk: Add a force option for pciserial device Fenghua Yu (2): x86/cpufeatures: Enumerate MOVDIRI instruction x86/cpufeatures: Enumerate MOVDIR64B instruction Jacek Tomaka (1): x86/microcode: Make revision and processor flags world-readable Jordan Borgner (1): x86: Clean up 'sizeof x' => 'sizeof(x)' Josh Poimboeuf (1): objtool: Support GCC 9 cold subfunction naming scheme Juergen Gross (2): x86/paravirt: Remove GPL from pv_ops export x86/paravirt: Remove unused _paravirt_ident_32 Rasmus Villemoes (1): x86/traps: Use format string with panic() call Sebastian Andrzej Siewior (1): x86/mm/pat: Disable preemption around __flush_tlb_all() Documentation/admin-guide/kernel-parameters.txt | 6 +++- arch/x86/boot/cpucheck.c| 2 +- arch/x86/boot/early_serial_console.c| 4 +-- arch/x86/boot/edd.c | 6 ++-- arch/x86/boot/main.c| 4 +-- arch/x86/boot/memory.c | 2 +- arch/x86/boot/regs.c| 2 +- arch/x86/boot/video-vesa.c | 6 ++-- arch/x86/boot/video.c | 2 +- arch/x86/events/intel/core.c| 2 +- arch/x86/include/asm/compat.h | 9 +- arch/x86/include/asm/cpufeatures.h | 2 ++ arch/x86/include/asm/ftrace.h | 4 +-- arch/x86/include/asm/paravirt_types.h | 2 -- arch/x86/include/asm/tlbflush.h | 6 arch/x86/kernel/cpu/common.c| 4 +-- arch/x86/kernel/cpu/mcheck/mce.c| 2 +- arch/x86/kernel/cpu/microcode/core.c| 4 +-- arch/x86/kernel/cpu/mtrr/generic.c | 2 +- arch/x86/kernel/cpu/mtrr/if.c | 6 ++-- arch/x86/kernel/early_printk.c | 29 +++-- arch/x86/kernel/head64.c| 2 +- arch/x86/kernel/msr.c | 8 ++--- arch/x86/kernel/paravirt.c | 28 + arch/x86/kernel/paravirt_patch_32.c | 18 --- arch/x86/kernel/paravirt_patch_64.c | 20 arch/x86/kernel/process_64.c| 4 +-- arch/x86/kernel/sys_x86_64.c| 11 --- arch/x86/kernel/traps.c | 2 +- arch/x86/kvm/emulate.c | 22 ++--- arch/x86/kvm/lapic.c| 2 +- arch/x86/kvm/x86.c | 42 - arch/x86/mm/hugetlbpage.c | 4 +-- arch/x86/mm/mmap.c | 2 +- arch/x86/mm/numa_emulation.c| 12 +-- arch/x86/mm/pageattr.c | 6 +++- arch/x86/tools/relocs.c | 4 +--
[GIT PULL] perf updates/fixes
Linus,
Please pull the latest perf-urgent-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
perf-urgent-for-linus
# HEAD: 29995d296e3e9ce4f9767963ecbef143ade26c36 Merge tag
'perf-urgent-for-mingo-4.20-20181031' of
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent
These are almost all tooling updates: 'perf top', 'perf trace' and 'perf
script' fixes and updates, an UAPI header sync with the merge window
versions, license marker updates, much improved Sparc support from David
Miller, and a number of fixes.
Thanks,
Ingo
-->
Adrian Hunter (19):
perf scripts python: call-graph-from-sql.py: Use SPDX license identifier
perf scripts python: call-graph-from-sql.py: Provide better default
column sizes
perf scripts python: call-graph-from-sql.py: Set a minimum window size
perf scripts python: call-graph-from-sql.py: Change icon
perf scripts python: call-graph-from-sql.py: Make a "Main" function
perf scripts python: call-graph-from-sql.py: Separate the database
details into a class
perf scripts python: call-graph-from-sql.py: Add a class for global data
perf scripts python: call-graph-from-sql.py: Remove use of setObjectName()
perf scripts python: call-graph-from-sql.py: Factor out CallGraphModel
from TreeModel
perf scripts python: call-graph-from-sql.py: Add data helper functions
perf scripts python: call-graph-from-sql.py: Refactor TreeItem class
perf scripts python: call-graph-from-sql.py: Rename to
exported-sql-viewer.py
perf scripts python: exported-sql-viewer.py: Add support for multiple
sub-windows
perf scripts python: exported-sql-viewer.py: Add ability to find symbols
in the call-graph
perf scripts python: exported-sql-viewer.py: Add ability to shrink /
enlarge font
perf scripts python: exported-sql-viewer.py: Add ability to display all
the database tables
perf scripts python: exported-sql-viewer.py: Add All branches report
perf intel-pt: Insert callchain context into synthesized callchains
perf intel-pt/bts: Calculate cpumode for synthesized samples
Alexey Budankov (1):
perf record: Encode -k clockid frequency into Perf trace
Andi Kleen (5):
perf script: Add --insn-trace for instruction decoding
perf script: Make itrace script default to all calls
tools script: Add --call-trace and --call-ret-trace
perf script: Implement --graph-function
perf script: Support total cycles count
Arnaldo Carvalho de Melo (28):
tools lib subcmd: Introduce OPTION_ULONG
perf trace: Introduce --max-events
perf evsel: Introduce per event max_events property
perf evsel: Mark a evsel as disabled when asking the kernel do disable it
perf trace: Drop addr_location refcounts
perf trace: Drop thread refcount in trace__event_handler()
perf trace: Introduce per-event maximum number of events property
tools include uapi: Grab a copy of linux/fs.h
perf beauty: Add a generator for MS_ mount/umount's flag constants
perf beauty: Switch from GPL v2.0 to LGPL v2.1
perf beauty: Introduce strarray__scnprintf_flags()
perf trace beauty: Allow syscalls to mask an argument before considering
it
perf trace beauty: Beautify mount/umount's 'flags' argument
perf trace: Consider syscall aliases too
perf trace: Beautify the umount's 'name' argument
perf trace: Beautify mount's first pathname arg
perf top: Allow disabling the overwrite mode
perf top: Do not use overwrite mode by default
tools include uapi: Update linux/fs.h copy
tools arch uapi: Update asm-generic/unistd.h and arm64 unistd.h copies
tools include uapi: Update asound.h copy
perf beauty: Add a generator for MAP_ mmap's flag constants
perf beauty: Wire up the mmap flags table generator to the Makefile
perf trace beauty: Use the mmap flags table generated from headers
tools include uapi: Update linux/mmap.h copy
tools headers: Sync the various kvm.h header copies
tools headers uapi: Update linux/netlink.h header copy
tools headers uapi: Update linux/if_link.h header copy
Colin Ian King (1):
perf/core: Clean up inconsisent indentation
David Miller (5):
perf annotate: Add Sparc support
perf jitdump: Add Sparc support.
perf symbols: Set PLT entry/header sizes properly on Sparc
perf top: Start display thread earlier
perf tools: Don't clone maps from parent when synthesizing forks
David S. Miller (1):
perf callchain: Honour the ordering of PERF_CONTEXT_{USER,KERNEL,etc}
Hongxu Jia (1):
perf arm64: Fix generate system call table failed with /tmp mounted with
noexec
Jiri Olsa (1):
perf stat: Poll for monitored tasks being alive
Leo Yan (1):
perf cs-etm: Correct CPU mode for samples
Milian Wolff (3):
perf
[GIT PULL] perf updates/fixes
Linus,
Please pull the latest perf-urgent-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
perf-urgent-for-linus
# HEAD: 29995d296e3e9ce4f9767963ecbef143ade26c36 Merge tag
'perf-urgent-for-mingo-4.20-20181031' of
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent
These are almost all tooling updates: 'perf top', 'perf trace' and 'perf
script' fixes and updates, an UAPI header sync with the merge window
versions, license marker updates, much improved Sparc support from David
Miller, and a number of fixes.
Thanks,
Ingo
-->
Adrian Hunter (19):
perf scripts python: call-graph-from-sql.py: Use SPDX license identifier
perf scripts python: call-graph-from-sql.py: Provide better default
column sizes
perf scripts python: call-graph-from-sql.py: Set a minimum window size
perf scripts python: call-graph-from-sql.py: Change icon
perf scripts python: call-graph-from-sql.py: Make a "Main" function
perf scripts python: call-graph-from-sql.py: Separate the database
details into a class
perf scripts python: call-graph-from-sql.py: Add a class for global data
perf scripts python: call-graph-from-sql.py: Remove use of setObjectName()
perf scripts python: call-graph-from-sql.py: Factor out CallGraphModel
from TreeModel
perf scripts python: call-graph-from-sql.py: Add data helper functions
perf scripts python: call-graph-from-sql.py: Refactor TreeItem class
perf scripts python: call-graph-from-sql.py: Rename to
exported-sql-viewer.py
perf scripts python: exported-sql-viewer.py: Add support for multiple
sub-windows
perf scripts python: exported-sql-viewer.py: Add ability to find symbols
in the call-graph
perf scripts python: exported-sql-viewer.py: Add ability to shrink /
enlarge font
perf scripts python: exported-sql-viewer.py: Add ability to display all
the database tables
perf scripts python: exported-sql-viewer.py: Add All branches report
perf intel-pt: Insert callchain context into synthesized callchains
perf intel-pt/bts: Calculate cpumode for synthesized samples
Alexey Budankov (1):
perf record: Encode -k clockid frequency into Perf trace
Andi Kleen (5):
perf script: Add --insn-trace for instruction decoding
perf script: Make itrace script default to all calls
tools script: Add --call-trace and --call-ret-trace
perf script: Implement --graph-function
perf script: Support total cycles count
Arnaldo Carvalho de Melo (28):
tools lib subcmd: Introduce OPTION_ULONG
perf trace: Introduce --max-events
perf evsel: Introduce per event max_events property
perf evsel: Mark a evsel as disabled when asking the kernel do disable it
perf trace: Drop addr_location refcounts
perf trace: Drop thread refcount in trace__event_handler()
perf trace: Introduce per-event maximum number of events property
tools include uapi: Grab a copy of linux/fs.h
perf beauty: Add a generator for MS_ mount/umount's flag constants
perf beauty: Switch from GPL v2.0 to LGPL v2.1
perf beauty: Introduce strarray__scnprintf_flags()
perf trace beauty: Allow syscalls to mask an argument before considering
it
perf trace beauty: Beautify mount/umount's 'flags' argument
perf trace: Consider syscall aliases too
perf trace: Beautify the umount's 'name' argument
perf trace: Beautify mount's first pathname arg
perf top: Allow disabling the overwrite mode
perf top: Do not use overwrite mode by default
tools include uapi: Update linux/fs.h copy
tools arch uapi: Update asm-generic/unistd.h and arm64 unistd.h copies
tools include uapi: Update asound.h copy
perf beauty: Add a generator for MAP_ mmap's flag constants
perf beauty: Wire up the mmap flags table generator to the Makefile
perf trace beauty: Use the mmap flags table generated from headers
tools include uapi: Update linux/mmap.h copy
tools headers: Sync the various kvm.h header copies
tools headers uapi: Update linux/netlink.h header copy
tools headers uapi: Update linux/if_link.h header copy
Colin Ian King (1):
perf/core: Clean up inconsisent indentation
David Miller (5):
perf annotate: Add Sparc support
perf jitdump: Add Sparc support.
perf symbols: Set PLT entry/header sizes properly on Sparc
perf top: Start display thread earlier
perf tools: Don't clone maps from parent when synthesizing forks
David S. Miller (1):
perf callchain: Honour the ordering of PERF_CONTEXT_{USER,KERNEL,etc}
Hongxu Jia (1):
perf arm64: Fix generate system call table failed with /tmp mounted with
noexec
Jiri Olsa (1):
perf stat: Poll for monitored tasks being alive
Leo Yan (1):
perf cs-etm: Correct CPU mode for samples
Milian Wolff (3):
perf
[GIT PULL] IRQ fixes
Linus,
Please pull the latest irq-urgent-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
irq-urgent-for-linus
# HEAD: 3424243e39e8ec138486926949e3668e7553125d irqchip/irq-mvebu-sei: Fix
a NULL vs IS_ERR() bug in probe function
An irqchip driver fix and a memory (over-)allocation fix.
Thanks,
Ingo
-->
Dan Carpenter (1):
irqchip/irq-mvebu-sei: Fix a NULL vs IS_ERR() bug in probe function
Michael Kelley (1):
irq/matrix: Fix memory overallocation
drivers/irqchip/irq-mvebu-sei.c | 4 ++--
kernel/irq/matrix.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-mvebu-sei.c b/drivers/irqchip/irq-mvebu-sei.c
index 566d69a2edbc..add4c9c934c8 100644
--- a/drivers/irqchip/irq-mvebu-sei.c
+++ b/drivers/irqchip/irq-mvebu-sei.c
@@ -384,9 +384,9 @@ static int mvebu_sei_probe(struct platform_device *pdev)
sei->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sei->base = devm_ioremap_resource(sei->dev, sei->res);
- if (!sei->base) {
+ if (IS_ERR(sei->base)) {
dev_err(sei->dev, "Failed to remap SEI resource\n");
- return -ENODEV;
+ return PTR_ERR(sei->base);
}
/* Retrieve the SEI capabilities with the interrupt ranges */
diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
index 6e6d467f3dec..1f0985adf193 100644
--- a/kernel/irq/matrix.c
+++ b/kernel/irq/matrix.c
@@ -8,7 +8,7 @@
#include
#include
-#define IRQ_MATRIX_SIZE(BITS_TO_LONGS(IRQ_MATRIX_BITS) *
sizeof(unsigned long))
+#define IRQ_MATRIX_SIZE(BITS_TO_LONGS(IRQ_MATRIX_BITS))
struct cpumap {
unsigned intavailable;
[GIT PULL] IRQ fixes
Linus,
Please pull the latest irq-urgent-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
irq-urgent-for-linus
# HEAD: 3424243e39e8ec138486926949e3668e7553125d irqchip/irq-mvebu-sei: Fix
a NULL vs IS_ERR() bug in probe function
An irqchip driver fix and a memory (over-)allocation fix.
Thanks,
Ingo
-->
Dan Carpenter (1):
irqchip/irq-mvebu-sei: Fix a NULL vs IS_ERR() bug in probe function
Michael Kelley (1):
irq/matrix: Fix memory overallocation
drivers/irqchip/irq-mvebu-sei.c | 4 ++--
kernel/irq/matrix.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-mvebu-sei.c b/drivers/irqchip/irq-mvebu-sei.c
index 566d69a2edbc..add4c9c934c8 100644
--- a/drivers/irqchip/irq-mvebu-sei.c
+++ b/drivers/irqchip/irq-mvebu-sei.c
@@ -384,9 +384,9 @@ static int mvebu_sei_probe(struct platform_device *pdev)
sei->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sei->base = devm_ioremap_resource(sei->dev, sei->res);
- if (!sei->base) {
+ if (IS_ERR(sei->base)) {
dev_err(sei->dev, "Failed to remap SEI resource\n");
- return -ENODEV;
+ return PTR_ERR(sei->base);
}
/* Retrieve the SEI capabilities with the interrupt ranges */
diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
index 6e6d467f3dec..1f0985adf193 100644
--- a/kernel/irq/matrix.c
+++ b/kernel/irq/matrix.c
@@ -8,7 +8,7 @@
#include
#include
-#define IRQ_MATRIX_SIZE(BITS_TO_LONGS(IRQ_MATRIX_BITS) *
sizeof(unsigned long))
+#define IRQ_MATRIX_SIZE(BITS_TO_LONGS(IRQ_MATRIX_BITS))
struct cpumap {
unsigned intavailable;
[PATCH v3 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 30 +++
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 8f79cccf4814..9c168b7410d0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,17 +34,29 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / 2^12 */
+ *val = 6283; /* mV */
+ *val2 = 12;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -55,7 +67,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
[PATCH v3 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Changes in v3: - Removed unconnected change in patch 1 (whitespace tidying up). - Added comment to patch 2's description regarding a code block that was moved in patch 4. - Corrected scale in patch 5, from 2Pi/(2^12 - 1) to 2Pi/2^12 and using IIO_VAL_FRACTIONAL_LOG2. Changes in v2: - Added my S-o-B in patch 5. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 53 ++- 1 file changed, 35 insertions(+), 18 deletions(-) -- 2.18.0
[PATCH v3 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 4908c8a95fad..54ad85bd9dc6 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -62,7 +62,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
[PATCH v3 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 30 +++
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 8f79cccf4814..9c168b7410d0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,17 +34,29 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / 2^12 */
+ *val = 6283; /* mV */
+ *val2 = 12;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -55,7 +67,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
[PATCH v3 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Changes in v3: - Removed unconnected change in patch 1 (whitespace tidying up). - Added comment to patch 2's description regarding a code block that was moved in patch 4. - Corrected scale in patch 5, from 2Pi/(2^12 - 1) to 2Pi/2^12 and using IIO_VAL_FRACTIONAL_LOG2. Changes in v2: - Added my S-o-B in patch 5. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 53 ++- 1 file changed, 35 insertions(+), 18 deletions(-) -- 2.18.0
[PATCH v3 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 4908c8a95fad..54ad85bd9dc6 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -62,7 +62,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
[PATCH v3 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 54ad85bd9dc6..8f79cccf4814 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -79,10 +79,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -93,7 +89,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
[PATCH v3 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 9c168b7410d0..3e257ac46f7a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / 2^12 */
--
2.18.0
[PATCH v3 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 54ad85bd9dc6..8f79cccf4814 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -79,10 +79,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -93,7 +89,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
[PATCH v3 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 9c168b7410d0..3e257ac46f7a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / 2^12 */
--
2.18.0
[PATCH v3 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code.
Note: The 'return ret' statement could be out of the 'if' block, but
this whole block will be moved up in the function in the patch:
'staging:iio:ad2s90: Move device registration to the end of probe'.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index ba55de29ef36..4908c8a95fad 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -86,7 +86,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
[PATCH v3 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..ba55de29ef36 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -36,11 +36,12 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
mutex_lock(>lock);
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
[PATCH v3 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code.
Note: The 'return ret' statement could be out of the 'if' block, but
this whole block will be moved up in the function in the patch:
'staging:iio:ad2s90: Move device registration to the end of probe'.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index ba55de29ef36..4908c8a95fad 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -86,7 +86,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
[PATCH v3 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..ba55de29ef36 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -36,11 +36,12 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
mutex_lock(>lock);
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
Re: [Ksummit-discuss] Call to Action Re: [PATCH 0/7] Code of Conduct: Fix some wording, and add an interpretation document
On Sun, Nov 04, 2018 at 08:06:41AM +1100, NeilBrown wrote: > On Sat, Nov 03 2018, Paul E. McKenney wrote: > > > On Sat, Nov 03, 2018 at 07:36:19PM +1100, NeilBrown wrote: > >> On Fri, Nov 02 2018, Paul E. McKenney wrote: > >> > >> > On Fri, Nov 02, 2018 at 08:50:11AM +1100, NeilBrown wrote: > >> >> On Thu, Nov 01 2018, Paul E. McKenney wrote: > >> >> > >> >> > On Sat, Oct 27, 2018 at 02:10:10AM +0100, Josh Triplett wrote: > >> >> >> On Fri, Oct 26, 2018 at 08:14:51AM +1100, NeilBrown wrote: > >> >> >> > On Wed, Oct 24 2018, Josh Triplett wrote: > >> >> >> > > >> >> >> > > On Tue, Oct 23, 2018 at 07:26:06AM +1100, NeilBrown wrote: > >> >> >> > >> On Sun, Oct 21 2018, Josh Triplett wrote: > >> >> >> > >> > >> >> >> > >> > On Mon, Oct 22, 2018 at 08:20:11AM +1100, NeilBrown wrote: > >> >> >> > >> >> I call on you, Greg: > >> >> >> > >> >> - to abandon this divisive attempt to impose a "Code of > >> >> >> > >> >> Conduct" > >> >> >> > >> >> - to revert 8a104f8b5867c68 > >> >> >> > >> >> - to return to your core competence of building a great > >> >> >> > >> >> team around > >> >> >> > >> >>a great kernel > >> >> >> > >> >> > >> >> >> > >> >> #Isupportreversion > >> >> >> > >> >> > >> >> >> > >> >> I call on the community to consider what *does* need to be > >> >> >> > >> >> said, about > >> >> >> > >> >> conduct, to people outside the community and who have > >> >> >> > >> >> recently joined. > >> >> >> > >> >> What is the document that you would have liked to have read > >> >> >> > >> >> as you were > >> >> >> > >> >> starting out? It is all too long ago for me to remember > >> >> >> > >> >> clearly, and so > >> >> >> > >> >> much has changed. > >> >> >> > >> > > >> >> >> > >> > The document I would have liked to have read when starting > >> >> >> > >> > out is > >> >> >> > >> > currently checked into the source tree in > >> >> >> > >> > Documentation/process/code-of-conduct.rst . > >> >> >> > >> > >> >> >> > >> I'm curious - what would you have gained by reading that > >> >> >> > >> document? > >> >> >> > > > >> >> >> > > I would have then had rather less of a pervasive feeling of "if > >> >> >> > > I make > >> >> >> > > even a single mistake I get made an example of in ways that will > >> >> >> > > feed > >> >> >> > > people's quotes files for years to come". > >> >> >> > > >> >> >> > Thanks for your reply. Certainly feeling safe is important, and > >> >> >> > having > >> >> >> > clear statements that the community values and promotes > >> >> >> > psychological > >> >> >> > safety is valuable. > >> >> >> > > >> >> >> > The old "code of conflict" said > >> >> >> >If however, anyone feels personally abused, threatened, or > >> >> >> > otherwise > >> >> >> >uncomfortable due to this process, that is not acceptable. > >> >> >> > > >> >> >> > would you have not found this a strong enough statement to ward > >> >> >> > off that > >> >> >> > pervasive feeling? > >> >> >> > >> >> >> Not when that document started out effectively saying, in an > >> >> >> elaborate > >> >> >> way, "code > people". > >> >> > > >> >> > Interesting. > >> >> > > >> >> > I am curious what leads you to your "code > people" statement. Of > >> >> > course, > >> >> > one could argue that this does not really matter given that the code > >> >> > of > >> >> > conflict is no longer. However, I would like to understand for future > >> >> > reference, if for no other reason. > >> >> > > >> >> > One possibility is that you are restricting the "people" to only those > >> >> > people directly contributing in one way or another. But those using > >> >> > the > >> >> > kernel (both directly and indirectly) are important as well, and it is > >> >> > exactly this group that is served by "the most robust operating system > >> >> > kernel ever", the chest-beating sentiment notwithstanding. Which is > >> >> > in > >> >> > fact why I must reject (or rework or whatever) any patch that might > >> >> > result > >> >> > in too-short RCU grace periods: The needs of the patch's submitter > >> >> > are > >> >> > quite emphatically outweighed by the needs of the kernel's many users, > >> >> > and many of the various technical requirements and restrictions are in > >> >> > fact proxies for the needs of these users. > >> >> > > >> >> > But you knew that already. > >> >> > > >> >> > Similarly for the Linux kernel's various code-style strictures, which > >> >> > serve the surprisingly large group of people reading the kernel's > >> >> > code. > >> >> > Including the stricture that I most love to hate, which is the one > >> >> > stating that single-line do/for/if/while statements must not be > >> >> > enclosed > >> >> > in braces, which sometimes causes me trouble when inserting debug > >> >> > code, > >> >> > but which also makes more code fit into a window of a given size. ;-) > >> >> > > >> >> > But you knew that already, too. > >> >> > > >> >> > The maintainability requirements can be argued to mostly
Re: [Ksummit-discuss] Call to Action Re: [PATCH 0/7] Code of Conduct: Fix some wording, and add an interpretation document
On Sun, Nov 04, 2018 at 08:06:41AM +1100, NeilBrown wrote: > On Sat, Nov 03 2018, Paul E. McKenney wrote: > > > On Sat, Nov 03, 2018 at 07:36:19PM +1100, NeilBrown wrote: > >> On Fri, Nov 02 2018, Paul E. McKenney wrote: > >> > >> > On Fri, Nov 02, 2018 at 08:50:11AM +1100, NeilBrown wrote: > >> >> On Thu, Nov 01 2018, Paul E. McKenney wrote: > >> >> > >> >> > On Sat, Oct 27, 2018 at 02:10:10AM +0100, Josh Triplett wrote: > >> >> >> On Fri, Oct 26, 2018 at 08:14:51AM +1100, NeilBrown wrote: > >> >> >> > On Wed, Oct 24 2018, Josh Triplett wrote: > >> >> >> > > >> >> >> > > On Tue, Oct 23, 2018 at 07:26:06AM +1100, NeilBrown wrote: > >> >> >> > >> On Sun, Oct 21 2018, Josh Triplett wrote: > >> >> >> > >> > >> >> >> > >> > On Mon, Oct 22, 2018 at 08:20:11AM +1100, NeilBrown wrote: > >> >> >> > >> >> I call on you, Greg: > >> >> >> > >> >> - to abandon this divisive attempt to impose a "Code of > >> >> >> > >> >> Conduct" > >> >> >> > >> >> - to revert 8a104f8b5867c68 > >> >> >> > >> >> - to return to your core competence of building a great > >> >> >> > >> >> team around > >> >> >> > >> >>a great kernel > >> >> >> > >> >> > >> >> >> > >> >> #Isupportreversion > >> >> >> > >> >> > >> >> >> > >> >> I call on the community to consider what *does* need to be > >> >> >> > >> >> said, about > >> >> >> > >> >> conduct, to people outside the community and who have > >> >> >> > >> >> recently joined. > >> >> >> > >> >> What is the document that you would have liked to have read > >> >> >> > >> >> as you were > >> >> >> > >> >> starting out? It is all too long ago for me to remember > >> >> >> > >> >> clearly, and so > >> >> >> > >> >> much has changed. > >> >> >> > >> > > >> >> >> > >> > The document I would have liked to have read when starting > >> >> >> > >> > out is > >> >> >> > >> > currently checked into the source tree in > >> >> >> > >> > Documentation/process/code-of-conduct.rst . > >> >> >> > >> > >> >> >> > >> I'm curious - what would you have gained by reading that > >> >> >> > >> document? > >> >> >> > > > >> >> >> > > I would have then had rather less of a pervasive feeling of "if > >> >> >> > > I make > >> >> >> > > even a single mistake I get made an example of in ways that will > >> >> >> > > feed > >> >> >> > > people's quotes files for years to come". > >> >> >> > > >> >> >> > Thanks for your reply. Certainly feeling safe is important, and > >> >> >> > having > >> >> >> > clear statements that the community values and promotes > >> >> >> > psychological > >> >> >> > safety is valuable. > >> >> >> > > >> >> >> > The old "code of conflict" said > >> >> >> >If however, anyone feels personally abused, threatened, or > >> >> >> > otherwise > >> >> >> >uncomfortable due to this process, that is not acceptable. > >> >> >> > > >> >> >> > would you have not found this a strong enough statement to ward > >> >> >> > off that > >> >> >> > pervasive feeling? > >> >> >> > >> >> >> Not when that document started out effectively saying, in an > >> >> >> elaborate > >> >> >> way, "code > people". > >> >> > > >> >> > Interesting. > >> >> > > >> >> > I am curious what leads you to your "code > people" statement. Of > >> >> > course, > >> >> > one could argue that this does not really matter given that the code > >> >> > of > >> >> > conflict is no longer. However, I would like to understand for future > >> >> > reference, if for no other reason. > >> >> > > >> >> > One possibility is that you are restricting the "people" to only those > >> >> > people directly contributing in one way or another. But those using > >> >> > the > >> >> > kernel (both directly and indirectly) are important as well, and it is > >> >> > exactly this group that is served by "the most robust operating system > >> >> > kernel ever", the chest-beating sentiment notwithstanding. Which is > >> >> > in > >> >> > fact why I must reject (or rework or whatever) any patch that might > >> >> > result > >> >> > in too-short RCU grace periods: The needs of the patch's submitter > >> >> > are > >> >> > quite emphatically outweighed by the needs of the kernel's many users, > >> >> > and many of the various technical requirements and restrictions are in > >> >> > fact proxies for the needs of these users. > >> >> > > >> >> > But you knew that already. > >> >> > > >> >> > Similarly for the Linux kernel's various code-style strictures, which > >> >> > serve the surprisingly large group of people reading the kernel's > >> >> > code. > >> >> > Including the stricture that I most love to hate, which is the one > >> >> > stating that single-line do/for/if/while statements must not be > >> >> > enclosed > >> >> > in braces, which sometimes causes me trouble when inserting debug > >> >> > code, > >> >> > but which also makes more code fit into a window of a given size. ;-) > >> >> > > >> >> > But you knew that already, too. > >> >> > > >> >> > The maintainability requirements can be argued to mostly
