Re: [Ksummit-discuss] bug-introducing patches

[Willy Tarreau]

On Sat, Jul 14, 2018 at 11:09:13PM +0200, Pavel Machek wrote:
> > For anyone interested in making sure that obscure (whatever that means)
> > drivers are tested for stable releases, but does not want to spend time on 
> > it,
> > all I can recommend is to implement qemu support for it and let me know,
> > and I'll be happy to add a respective test to my test farm.
> 
> Umm. Yes, qemu support for every driver would be nice, but will not happen.

Well, I would argue that driver code changes much less than core code
between kernel versions, and that most of the changes in drivers are
mostly infrastructure changes. Drivers don't evolve much in general,
they are written, tested, merged, they receive fixes and then they
only receive infrastructure changes that touch all drivers in the same
category.

When you backport fixes to drivers, it is very common that the code
looks almost the same between even a very old kernel and mainline, and
when not, the adaptations generally look quite straightforward, and if
not it means the driver changed significantly and in this case we don't
backport the fix as we don't even know if it is relevant.

I've always had much more difficulties backporting fixes under the arch/
subdir where stuff changes all the time. Sometimes a patch applies but
doesn't even compile. I learned not to play black magic in this area
because some patches are subtle and if the code changed you often need
the author and/or maintainers to double-check. Some subsystems like KVM
improve a lot over time and are difficult to backport to as well, and
even if you manage to properly backport a fix you're uncertain how to
verify you backported it well. Similarly you don't want to improvise
yourself the backporter of the year in this area.

Drivers are often OK and are the ones harder to test, so in the end
you don't miss much by your limited ability to test a backport there.

What I can certainly say as a stable kernel user is that the regression
rate is so low compared to the fix rate that I never have any problem
upgrading to a more recent version in the same branch, because the
number of problems that will be fixed is much higher than the risk of
a single regression.

As Guenter says, we can always improve, but the most important is to
deliver fixes in a timely manner. When you see that any LTS branch
accumulates around 5000 fixes over time, you understand that any
single new kernel being released contains around 5000 bugs left to be
found. Fixing them quickly is much more important to me (as a user)
than ensuring that I will not reach 5001 by inheriting from a poorly
tested backport.

My hope is that thanks to all the automated testing in place we can
further accelerate the backport rate so that a stable kernel reaches
in 2 months the level of quality that we previously used to reach only
after one year. And I think we're already about there, as both 4.4.x
and 4.9.x in their early versions (x < 10) were already very good for
various use cases. 4.17.5 I'm using on this PC looks pretty slick as
well. Overall it means that we can provide a clean upgrade path for
users so that they don't stick to bogus or insecure kernels by fear
of upgrading. We can always argue that a bug may appear once in a
while but for me while technically this is true, stastistically this
is just FUD and is not relevant to end users' real usage.

Willy

Re: [Ksummit-discuss] bug-introducing patches

[Willy Tarreau]

On Sat, Jul 14, 2018 at 11:09:13PM +0200, Pavel Machek wrote:
> > For anyone interested in making sure that obscure (whatever that means)
> > drivers are tested for stable releases, but does not want to spend time on 
> > it,
> > all I can recommend is to implement qemu support for it and let me know,
> > and I'll be happy to add a respective test to my test farm.
> 
> Umm. Yes, qemu support for every driver would be nice, but will not happen.

Well, I would argue that driver code changes much less than core code
between kernel versions, and that most of the changes in drivers are
mostly infrastructure changes. Drivers don't evolve much in general,
they are written, tested, merged, they receive fixes and then they
only receive infrastructure changes that touch all drivers in the same
category.

When you backport fixes to drivers, it is very common that the code
looks almost the same between even a very old kernel and mainline, and
when not, the adaptations generally look quite straightforward, and if
not it means the driver changed significantly and in this case we don't
backport the fix as we don't even know if it is relevant.

I've always had much more difficulties backporting fixes under the arch/
subdir where stuff changes all the time. Sometimes a patch applies but
doesn't even compile. I learned not to play black magic in this area
because some patches are subtle and if the code changed you often need
the author and/or maintainers to double-check. Some subsystems like KVM
improve a lot over time and are difficult to backport to as well, and
even if you manage to properly backport a fix you're uncertain how to
verify you backported it well. Similarly you don't want to improvise
yourself the backporter of the year in this area.

Drivers are often OK and are the ones harder to test, so in the end
you don't miss much by your limited ability to test a backport there.

What I can certainly say as a stable kernel user is that the regression
rate is so low compared to the fix rate that I never have any problem
upgrading to a more recent version in the same branch, because the
number of problems that will be fixed is much higher than the risk of
a single regression.

As Guenter says, we can always improve, but the most important is to
deliver fixes in a timely manner. When you see that any LTS branch
accumulates around 5000 fixes over time, you understand that any
single new kernel being released contains around 5000 bugs left to be
found. Fixing them quickly is much more important to me (as a user)
than ensuring that I will not reach 5001 by inheriting from a poorly
tested backport.

My hope is that thanks to all the automated testing in place we can
further accelerate the backport rate so that a stable kernel reaches
in 2 months the level of quality that we previously used to reach only
after one year. And I think we're already about there, as both 4.4.x
and 4.9.x in their early versions (x < 10) were already very good for
various use cases. 4.17.5 I'm using on this PC looks pretty slick as
well. Overall it means that we can provide a clean upgrade path for
users so that they don't stick to bogus or insecure kernels by fear
of upgrading. We can always argue that a bug may appear once in a
while but for me while technically this is true, stastistically this
is just FUD and is not relevant to end users' real usage.

Willy

Re: [PATCH v5 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices

[Sean Wang]

On Sat, 2018-07-14 at 18:32 +0200, Marcel Holtmann wrote:
> Hi Sean,
> 
> > This adds a driver to run on the top of btuart driver for the MediaTek
> > serial protocol based on running H:4, which can enable the built-in
> > Bluetooth device inside MT7622 SoC.
> > 
> > Signed-off-by: Sean Wang 
> > ---
> > drivers/bluetooth/Kconfig |  11 ++
> > drivers/bluetooth/Makefile|   2 +
> > drivers/bluetooth/btmtkuart.c | 352 
> > ++
> > drivers/bluetooth/btmtkuart.h | 116 ++
> > drivers/bluetooth/btuart.c|  18 +++
> > 5 files changed, 499 insertions(+)
> > create mode 100644 drivers/bluetooth/btmtkuart.c
> > create mode 100644 drivers/bluetooth/btmtkuart.h
> > 
> > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > index 00fdf5f..4d7d640 100644
> > --- a/drivers/bluetooth/Kconfig
> > +++ b/drivers/bluetooth/Kconfig
> > @@ -85,6 +85,17 @@ config BT_HCIBTUART
> >   Say Y here to compile support for Bluetooth UART devices into the
> >   kernel or say M to compile it as module (btuart).
> > 
> > +config BT_HCIBTUART_MTK
> > +   tristate "MediaTek HCI UART driver"
> > +   depends on BT_HCIBTUART
> > +   help
> > + MediaTek Bluetooth HCI UART driver.
> > + This driver is required if you want to use MediaTek Bluetooth
> > + with serial interface.
> > +
> > + Say Y here to compile support for MediaTek Bluetooth UART devices
> > + into the kernel or say M to compile it as module (btmtkuart).
> > +
> > config BT_HCIUART
> > tristate "HCI UART driver"
> > depends on SERIAL_DEV_BUS || !SERIAL_DEV_BUS
> > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> > index 60a19cb..c9a8926 100644
> > --- a/drivers/bluetooth/Makefile
> > +++ b/drivers/bluetooth/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_BT_BCM)  += btbcm.o
> > obj-$(CONFIG_BT_RTL)+= btrtl.o
> > obj-$(CONFIG_BT_QCA)+= btqca.o
> > 
> > +obj-$(CONFIG_BT_HCIBTUART_MTK) += btmtkuart.o
> > +
> > obj-$(CONFIG_BT_HCIUART_NOKIA)  += hci_nokia.o
> > 
> > obj-$(CONFIG_BT_HCIRSI) += btrsi.o
> > diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c
> > new file mode 100644
> > index 000..9eed21c
> > --- /dev/null
> > +++ b/drivers/bluetooth/btmtkuart.c
> > @@ -0,0 +1,352 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2018 MediaTek Inc.
> > +
> > +/*
> > + * Bluetooth support for MediaTek serial devices
> > + *
> > + * Author: Sean Wang 
> > + *
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +#include "btmtkuart.h"
> > +#include "btuart.h"
> > +#include "h4_recv.h"
> > +
> > +static void mtk_stp_reset(struct mtk_stp_splitter *sp)
> > +{
> > +   sp->cursor = 2;
> > +   sp->dlen = 0;
> > +}
> > +
> > +static const unsigned char *
> > +mtk_stp_split(struct btuart_dev *bdev, struct mtk_stp_splitter *sp,
> > + const unsigned char *data, int count, int *sz_h4)
> > +{
> > +   struct mtk_stp_hdr *shdr;
> > +
> > +   /* The cursor is reset when all the data of STP is consumed out. */
> > +   if (!sp->dlen && sp->cursor >= 6)
> > +   sp->cursor = 0;
> > +
> > +   /* Filling pad until all STP info is obtained. */
> > +   while (sp->cursor < 6 && count > 0) {
> > +   sp->pad[sp->cursor] = *data;
> > +   sp->cursor++;
> > +   data++;
> > +   count--;
> > +   }
> > +
> > +   /* Retrieve STP info and have a sanity check. */
> > +   if (!sp->dlen && sp->cursor >= 6) {
> > +   shdr = (struct mtk_stp_hdr *)>pad[2];
> > +   sp->dlen = shdr->dlen1 << 8 | shdr->dlen2;
> > +
> > +   /* Resync STP when unexpected data is being read. */
> > +   if (shdr->prefix != 0x80 || sp->dlen > 2048) {
> > +   bt_dev_err(bdev->hdev, "stp format unexpect (%d, %d)",
> > +  shdr->prefix, sp->dlen);
> > +   mtk_stp_reset(sp);
> > +   }
> > +   }
> > +
> > +   /* Directly quit when there's no data found for H4 can process. */
> > +   if (count <= 0)
> > +   return NULL;
> > +
> > +   /* Tranlate to how much the size of data H4 can handle so far. */
> > +   *sz_h4 = min_t(int, count, sp->dlen);
> > +   /* Update the remaining size of STP packet. */
> > +   sp->dlen -= *sz_h4;
> > +
> > +   /* Data points to STP payload which can be handled by H4. */
> > +   return data;
> > +}
> > +
> > +static int mtk_stp_send(struct btuart_dev *bdev, struct sk_buff *skb)
> > +{
> > +   struct mtk_stp_hdr *shdr;
> > +   struct sk_buff *new_skb;
> > +   int dlen;
> > +
> > +   memcpy(skb_push(skb, 1), _skb_pkt_type(skb), 1);
> > +   dlen = skb->len;
> > +
> > +   /* Make sure of STP header at least has 4-bytes free space to fill. */
> > +   if (unlikely(skb_headroom(skb) < sizeof(*shdr))) {
> > +   new_skb = 

Re: [PATCH v5 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices

[Sean Wang]

On Sat, 2018-07-14 at 18:32 +0200, Marcel Holtmann wrote:
> Hi Sean,
> 
> > This adds a driver to run on the top of btuart driver for the MediaTek
> > serial protocol based on running H:4, which can enable the built-in
> > Bluetooth device inside MT7622 SoC.
> > 
> > Signed-off-by: Sean Wang 
> > ---
> > drivers/bluetooth/Kconfig |  11 ++
> > drivers/bluetooth/Makefile|   2 +
> > drivers/bluetooth/btmtkuart.c | 352 
> > ++
> > drivers/bluetooth/btmtkuart.h | 116 ++
> > drivers/bluetooth/btuart.c|  18 +++
> > 5 files changed, 499 insertions(+)
> > create mode 100644 drivers/bluetooth/btmtkuart.c
> > create mode 100644 drivers/bluetooth/btmtkuart.h
> > 
> > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > index 00fdf5f..4d7d640 100644
> > --- a/drivers/bluetooth/Kconfig
> > +++ b/drivers/bluetooth/Kconfig
> > @@ -85,6 +85,17 @@ config BT_HCIBTUART
> >   Say Y here to compile support for Bluetooth UART devices into the
> >   kernel or say M to compile it as module (btuart).
> > 
> > +config BT_HCIBTUART_MTK
> > +   tristate "MediaTek HCI UART driver"
> > +   depends on BT_HCIBTUART
> > +   help
> > + MediaTek Bluetooth HCI UART driver.
> > + This driver is required if you want to use MediaTek Bluetooth
> > + with serial interface.
> > +
> > + Say Y here to compile support for MediaTek Bluetooth UART devices
> > + into the kernel or say M to compile it as module (btmtkuart).
> > +
> > config BT_HCIUART
> > tristate "HCI UART driver"
> > depends on SERIAL_DEV_BUS || !SERIAL_DEV_BUS
> > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> > index 60a19cb..c9a8926 100644
> > --- a/drivers/bluetooth/Makefile
> > +++ b/drivers/bluetooth/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_BT_BCM)  += btbcm.o
> > obj-$(CONFIG_BT_RTL)+= btrtl.o
> > obj-$(CONFIG_BT_QCA)+= btqca.o
> > 
> > +obj-$(CONFIG_BT_HCIBTUART_MTK) += btmtkuart.o
> > +
> > obj-$(CONFIG_BT_HCIUART_NOKIA)  += hci_nokia.o
> > 
> > obj-$(CONFIG_BT_HCIRSI) += btrsi.o
> > diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c
> > new file mode 100644
> > index 000..9eed21c
> > --- /dev/null
> > +++ b/drivers/bluetooth/btmtkuart.c
> > @@ -0,0 +1,352 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2018 MediaTek Inc.
> > +
> > +/*
> > + * Bluetooth support for MediaTek serial devices
> > + *
> > + * Author: Sean Wang 
> > + *
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +#include "btmtkuart.h"
> > +#include "btuart.h"
> > +#include "h4_recv.h"
> > +
> > +static void mtk_stp_reset(struct mtk_stp_splitter *sp)
> > +{
> > +   sp->cursor = 2;
> > +   sp->dlen = 0;
> > +}
> > +
> > +static const unsigned char *
> > +mtk_stp_split(struct btuart_dev *bdev, struct mtk_stp_splitter *sp,
> > + const unsigned char *data, int count, int *sz_h4)
> > +{
> > +   struct mtk_stp_hdr *shdr;
> > +
> > +   /* The cursor is reset when all the data of STP is consumed out. */
> > +   if (!sp->dlen && sp->cursor >= 6)
> > +   sp->cursor = 0;
> > +
> > +   /* Filling pad until all STP info is obtained. */
> > +   while (sp->cursor < 6 && count > 0) {
> > +   sp->pad[sp->cursor] = *data;
> > +   sp->cursor++;
> > +   data++;
> > +   count--;
> > +   }
> > +
> > +   /* Retrieve STP info and have a sanity check. */
> > +   if (!sp->dlen && sp->cursor >= 6) {
> > +   shdr = (struct mtk_stp_hdr *)>pad[2];
> > +   sp->dlen = shdr->dlen1 << 8 | shdr->dlen2;
> > +
> > +   /* Resync STP when unexpected data is being read. */
> > +   if (shdr->prefix != 0x80 || sp->dlen > 2048) {
> > +   bt_dev_err(bdev->hdev, "stp format unexpect (%d, %d)",
> > +  shdr->prefix, sp->dlen);
> > +   mtk_stp_reset(sp);
> > +   }
> > +   }
> > +
> > +   /* Directly quit when there's no data found for H4 can process. */
> > +   if (count <= 0)
> > +   return NULL;
> > +
> > +   /* Tranlate to how much the size of data H4 can handle so far. */
> > +   *sz_h4 = min_t(int, count, sp->dlen);
> > +   /* Update the remaining size of STP packet. */
> > +   sp->dlen -= *sz_h4;
> > +
> > +   /* Data points to STP payload which can be handled by H4. */
> > +   return data;
> > +}
> > +
> > +static int mtk_stp_send(struct btuart_dev *bdev, struct sk_buff *skb)
> > +{
> > +   struct mtk_stp_hdr *shdr;
> > +   struct sk_buff *new_skb;
> > +   int dlen;
> > +
> > +   memcpy(skb_push(skb, 1), _skb_pkt_type(skb), 1);
> > +   dlen = skb->len;
> > +
> > +   /* Make sure of STP header at least has 4-bytes free space to fill. */
> > +   if (unlikely(skb_headroom(skb) < sizeof(*shdr))) {
> > +   new_skb = 

Re: [PATCH] mm: avoid bothering interrupted task when charge memcg in softirq

[Yafang Shao]

On Sun, Jul 15, 2018 at 12:25 PM, Shakeel Butt  wrote:
> On Sat, Jul 14, 2018 at 7:10 PM Yafang Shao  wrote:
>>
>> On Sat, Jul 14, 2018 at 11:38 PM, Shakeel Butt  wrote:
>> > On Sat, Jul 14, 2018 at 1:32 AM Yafang Shao  wrote:
>> >>
>> >> try_charge maybe executed in packet receive path, which is in interrupt
>> >> context.
>> >> In this situation, the 'current' is the interrupted task, which may has
>> >> no relation to the rx softirq, So it is nonsense to use 'current'.
>> >>
>> >
>> > Have you actually seen this occurring?
>>
>> Hi Shakeel,
>>
>> I'm trying to produce this issue, but haven't find it occur yet.
>>
>> > I am not very familiar with the
>> > network code but I can think of two ways try_charge() can be called
>> > from network code. Either through kmem charging or through
>> > mem_cgroup_charge_skmem() and both locations correctly handle
>> > interrupt context.
>> >
>>
>> Why do you say that mem_cgroup_charge_skmem() correctly hanle
>> interrupt context ?
>>
>> Let me show you why mem_cgroup_charge_skmem isn't hanling interrupt
>> context correctly.
>>
>> mem_cgroup_charge_skmem() is calling  try_charge() twice.
>> The first one is with GFP_NOWAIT as the gfp_mask, and the second one
>> is with  (GFP_NOWAIT |  __GFP_NOFAIL) as the gfp_mask.
>>
>> If page_counter_try_charge() failes at the first time, -ENOMEM is returned.
>> Then mem_cgroup_charge_skmem() will call try_charge() once more with
>> __GFP_NOFAIL set, and this time if If page_counter_try_charge() failes
>> again the '
>> force' label in  try_charge() will be executed and 0 is returned.
>>
>> No matter what, the 'current' will be used and touched, that is
>> meaning mem_cgroup_charge_skmem() isn't hanling the interrupt context
>> correctly.
>>
>
> Hi Yafang,
>
> If you check mem_cgroup_charge_skmem(), the memcg passed is not
> 'current' but is from the sock object i.e. sk->sk_memcg for which the
> network buffer is allocated for.
>

That's correct, the memcg if from the sock object.
But the point is, in this situation why 'current' is used in try_charge() ?
As 'current' is not related with the memcg, which is just a interrupted task.


> If the network buffers is allocated through kmem interface, the
> charging is bypassed altogether (through memcg_kmem_bypass()) for
> interrupt context.
>

Yes.

Thanks
Yafang

Re: [PATCH] mm: avoid bothering interrupted task when charge memcg in softirq

[Yafang Shao]

On Sun, Jul 15, 2018 at 12:25 PM, Shakeel Butt  wrote:
> On Sat, Jul 14, 2018 at 7:10 PM Yafang Shao  wrote:
>>
>> On Sat, Jul 14, 2018 at 11:38 PM, Shakeel Butt  wrote:
>> > On Sat, Jul 14, 2018 at 1:32 AM Yafang Shao  wrote:
>> >>
>> >> try_charge maybe executed in packet receive path, which is in interrupt
>> >> context.
>> >> In this situation, the 'current' is the interrupted task, which may has
>> >> no relation to the rx softirq, So it is nonsense to use 'current'.
>> >>
>> >
>> > Have you actually seen this occurring?
>>
>> Hi Shakeel,
>>
>> I'm trying to produce this issue, but haven't find it occur yet.
>>
>> > I am not very familiar with the
>> > network code but I can think of two ways try_charge() can be called
>> > from network code. Either through kmem charging or through
>> > mem_cgroup_charge_skmem() and both locations correctly handle
>> > interrupt context.
>> >
>>
>> Why do you say that mem_cgroup_charge_skmem() correctly hanle
>> interrupt context ?
>>
>> Let me show you why mem_cgroup_charge_skmem isn't hanling interrupt
>> context correctly.
>>
>> mem_cgroup_charge_skmem() is calling  try_charge() twice.
>> The first one is with GFP_NOWAIT as the gfp_mask, and the second one
>> is with  (GFP_NOWAIT |  __GFP_NOFAIL) as the gfp_mask.
>>
>> If page_counter_try_charge() failes at the first time, -ENOMEM is returned.
>> Then mem_cgroup_charge_skmem() will call try_charge() once more with
>> __GFP_NOFAIL set, and this time if If page_counter_try_charge() failes
>> again the '
>> force' label in  try_charge() will be executed and 0 is returned.
>>
>> No matter what, the 'current' will be used and touched, that is
>> meaning mem_cgroup_charge_skmem() isn't hanling the interrupt context
>> correctly.
>>
>
> Hi Yafang,
>
> If you check mem_cgroup_charge_skmem(), the memcg passed is not
> 'current' but is from the sock object i.e. sk->sk_memcg for which the
> network buffer is allocated for.
>

That's correct, the memcg if from the sock object.
But the point is, in this situation why 'current' is used in try_charge() ?
As 'current' is not related with the memcg, which is just a interrupted task.


> If the network buffers is allocated through kmem interface, the
> charging is bypassed altogether (through memcg_kmem_bypass()) for
> interrupt context.
>

Yes.

Thanks
Yafang

Re: [PATCH v5 1/7] dt-bindings: net: bluetooth: Add mediatek-bluetooth

[Sean Wang]

On Sat, 2018-07-14 at 18:26 +0200, Marcel Holtmann wrote:
> Hi Sean,
> 
> > Add binding document for a SoC built-in device using MediaTek protocol.
> > Which could be found on MT7622 SoC or other similar MediaTek SoCs.
> > 
> > Signed-off-by: Sean Wang 
> > Reviewed-by: Rob Herring 
> > ---
> > .../devicetree/bindings/net/mediatek-bluetooth.txt | 35 
> > ++
> > 1 file changed, 35 insertions(+)
> > create mode 100644 
> > Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt 
> > b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> > new file mode 100644
> > index 000..1335429
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> > @@ -0,0 +1,35 @@
> > +MediaTek SoC built-in Bluetooth Devices
> > +==
> > +
> > +This device is a serial attached device to BTIF device and thus it must be 
> > a
> > +child node of the serial node with BTIF. The dt-bindings details for BTIF
> > +device can be known via Documentation/devicetree/bindings/serial/8250.txt.
> > +
> > +Required properties:
> > +
> > +- compatible:  Must be one of
> > + "mediatek,mt7622-bluetooth"": for MT7622 SoC
> 
> this does not match with the example below. And one of, should be normally be 
> a list.
> 

Thanks! I'll remove the words "one of" from the compatible description,
and the extra " being added accidentally.

And the below example fully shows the bluetooth device and its attached
bus (mediatek,mt7622-btif) to let people know clearly how to enable the
bluetooth device.

The current document just describes the bluetooth device and as for the
attached bus, it is already present at
Documentation/devicetree/bindings/serial/8250.txt.

Sean


> > +- clocks:  Should be the clock specifiers corresponding to the entry in
> > +   clock-names property.
> > +- clock-names: Should contain "ref" entries.
> > +- power-domains: Phandle to the power domain that the device is part of
> > +
> > +Example:
> > +
> > +   btif: serial@1100c000 {
> > +   compatible = "mediatek,mt7622-btif",
> > +"mediatek,mtk-btif";
> > +   reg = <0 0x1100c000 0 0x1000>;
> > +   interrupts = ;
> > +   clocks = < CLK_PERI_BTIF_PD>;
> > +   clock-names = "main";
> > +   reg-shift = <2>;
> > +   reg-io-width = <4>;
> > +
> > +   bluetooth {
> > +   compatible = "mediatek,mt7622-bluetooth";
> > +   power-domains = < MT7622_POWER_DOMAIN_WB>;
> > +   clocks = <>;
> > +   clock-names = "ref";
> > +   };
> > +   };
> 
> Regards
> 
> Marcel
> 

Re: [PATCH v5 1/7] dt-bindings: net: bluetooth: Add mediatek-bluetooth

[Sean Wang]

On Sat, 2018-07-14 at 18:26 +0200, Marcel Holtmann wrote:
> Hi Sean,
> 
> > Add binding document for a SoC built-in device using MediaTek protocol.
> > Which could be found on MT7622 SoC or other similar MediaTek SoCs.
> > 
> > Signed-off-by: Sean Wang 
> > Reviewed-by: Rob Herring 
> > ---
> > .../devicetree/bindings/net/mediatek-bluetooth.txt | 35 
> > ++
> > 1 file changed, 35 insertions(+)
> > create mode 100644 
> > Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt 
> > b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> > new file mode 100644
> > index 000..1335429
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> > @@ -0,0 +1,35 @@
> > +MediaTek SoC built-in Bluetooth Devices
> > +==
> > +
> > +This device is a serial attached device to BTIF device and thus it must be 
> > a
> > +child node of the serial node with BTIF. The dt-bindings details for BTIF
> > +device can be known via Documentation/devicetree/bindings/serial/8250.txt.
> > +
> > +Required properties:
> > +
> > +- compatible:  Must be one of
> > + "mediatek,mt7622-bluetooth"": for MT7622 SoC
> 
> this does not match with the example below. And one of, should be normally be 
> a list.
> 

Thanks! I'll remove the words "one of" from the compatible description,
and the extra " being added accidentally.

And the below example fully shows the bluetooth device and its attached
bus (mediatek,mt7622-btif) to let people know clearly how to enable the
bluetooth device.

The current document just describes the bluetooth device and as for the
attached bus, it is already present at
Documentation/devicetree/bindings/serial/8250.txt.

Sean


> > +- clocks:  Should be the clock specifiers corresponding to the entry in
> > +   clock-names property.
> > +- clock-names: Should contain "ref" entries.
> > +- power-domains: Phandle to the power domain that the device is part of
> > +
> > +Example:
> > +
> > +   btif: serial@1100c000 {
> > +   compatible = "mediatek,mt7622-btif",
> > +"mediatek,mtk-btif";
> > +   reg = <0 0x1100c000 0 0x1000>;
> > +   interrupts = ;
> > +   clocks = < CLK_PERI_BTIF_PD>;
> > +   clock-names = "main";
> > +   reg-shift = <2>;
> > +   reg-io-width = <4>;
> > +
> > +   bluetooth {
> > +   compatible = "mediatek,mt7622-bluetooth";
> > +   power-domains = < MT7622_POWER_DOMAIN_WB>;
> > +   clocks = <>;
> > +   clock-names = "ref";
> > +   };
> > +   };
> 
> Regards
> 
> Marcel
> 

Re: [PATCH] mm: avoid bothering interrupted task when charge memcg in softirq

[Shakeel Butt]

On Sat, Jul 14, 2018 at 7:10 PM Yafang Shao  wrote:
>
> On Sat, Jul 14, 2018 at 11:38 PM, Shakeel Butt  wrote:
> > On Sat, Jul 14, 2018 at 1:32 AM Yafang Shao  wrote:
> >>
> >> try_charge maybe executed in packet receive path, which is in interrupt
> >> context.
> >> In this situation, the 'current' is the interrupted task, which may has
> >> no relation to the rx softirq, So it is nonsense to use 'current'.
> >>
> >
> > Have you actually seen this occurring?
>
> Hi Shakeel,
>
> I'm trying to produce this issue, but haven't find it occur yet.
>
> > I am not very familiar with the
> > network code but I can think of two ways try_charge() can be called
> > from network code. Either through kmem charging or through
> > mem_cgroup_charge_skmem() and both locations correctly handle
> > interrupt context.
> >
>
> Why do you say that mem_cgroup_charge_skmem() correctly hanle
> interrupt context ?
>
> Let me show you why mem_cgroup_charge_skmem isn't hanling interrupt
> context correctly.
>
> mem_cgroup_charge_skmem() is calling  try_charge() twice.
> The first one is with GFP_NOWAIT as the gfp_mask, and the second one
> is with  (GFP_NOWAIT |  __GFP_NOFAIL) as the gfp_mask.
>
> If page_counter_try_charge() failes at the first time, -ENOMEM is returned.
> Then mem_cgroup_charge_skmem() will call try_charge() once more with
> __GFP_NOFAIL set, and this time if If page_counter_try_charge() failes
> again the '
> force' label in  try_charge() will be executed and 0 is returned.
>
> No matter what, the 'current' will be used and touched, that is
> meaning mem_cgroup_charge_skmem() isn't hanling the interrupt context
> correctly.
>

Hi Yafang,

If you check mem_cgroup_charge_skmem(), the memcg passed is not
'current' but is from the sock object i.e. sk->sk_memcg for which the
network buffer is allocated for.

If the network buffers is allocated through kmem interface, the
charging is bypassed altogether (through memcg_kmem_bypass()) for
interrupt context.

regards,
Shakeel

Re: [PATCH] mm: avoid bothering interrupted task when charge memcg in softirq

[Shakeel Butt]

On Sat, Jul 14, 2018 at 7:10 PM Yafang Shao  wrote:
>
> On Sat, Jul 14, 2018 at 11:38 PM, Shakeel Butt  wrote:
> > On Sat, Jul 14, 2018 at 1:32 AM Yafang Shao  wrote:
> >>
> >> try_charge maybe executed in packet receive path, which is in interrupt
> >> context.
> >> In this situation, the 'current' is the interrupted task, which may has
> >> no relation to the rx softirq, So it is nonsense to use 'current'.
> >>
> >
> > Have you actually seen this occurring?
>
> Hi Shakeel,
>
> I'm trying to produce this issue, but haven't find it occur yet.
>
> > I am not very familiar with the
> > network code but I can think of two ways try_charge() can be called
> > from network code. Either through kmem charging or through
> > mem_cgroup_charge_skmem() and both locations correctly handle
> > interrupt context.
> >
>
> Why do you say that mem_cgroup_charge_skmem() correctly hanle
> interrupt context ?
>
> Let me show you why mem_cgroup_charge_skmem isn't hanling interrupt
> context correctly.
>
> mem_cgroup_charge_skmem() is calling  try_charge() twice.
> The first one is with GFP_NOWAIT as the gfp_mask, and the second one
> is with  (GFP_NOWAIT |  __GFP_NOFAIL) as the gfp_mask.
>
> If page_counter_try_charge() failes at the first time, -ENOMEM is returned.
> Then mem_cgroup_charge_skmem() will call try_charge() once more with
> __GFP_NOFAIL set, and this time if If page_counter_try_charge() failes
> again the '
> force' label in  try_charge() will be executed and 0 is returned.
>
> No matter what, the 'current' will be used and touched, that is
> meaning mem_cgroup_charge_skmem() isn't hanling the interrupt context
> correctly.
>

Hi Yafang,

If you check mem_cgroup_charge_skmem(), the memcg passed is not
'current' but is from the sock object i.e. sk->sk_memcg for which the
network buffer is allocated for.

If the network buffers is allocated through kmem interface, the
charging is bypassed altogether (through memcg_kmem_bypass()) for
interrupt context.

regards,
Shakeel

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Bart Van Assche]

On Sat, 2018-07-14 at 18:59 -0700, Linus Torvalds wrote:
> On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds 
>  wrote:
> > Honestly, I'd like to just encourage people to get the sparse update
> > from Luc Van Oostenryck instead.
> > 
> > For a while there it looked like Chris Li would just pull from Luc,
> > and we'd have timely releases, but that really doesn't seem to have
> > ended up happening after all. So right now it's probably just best to
> > get Luc's tree instead from
> > 
> > https://github.com/lucvoo/sparse-dev
> > 
> > which also ends up fixing a lot of other issues.
> 
> Oh,
>  and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.

(+ Konstantin)

Something that is confusing for kernel developers is that the sparse wiki
(https://sparse.wiki.kernel.org/index.php/Main_Page) refers kernel developers
to an outdated sparse tree. Should something be done about this inconsistency?
Should the sparse wiki point at one of Luc's sparse trees or should perhaps
Luc be granted access to the tree that wiki points at
(https://git.kernel.org/pub/scm/devel/sparse/sparse.git)?

Thanks,

Bart.

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Bart Van Assche]

On Sat, 2018-07-14 at 18:59 -0700, Linus Torvalds wrote:
> On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds 
>  wrote:
> > Honestly, I'd like to just encourage people to get the sparse update
> > from Luc Van Oostenryck instead.
> > 
> > For a while there it looked like Chris Li would just pull from Luc,
> > and we'd have timely releases, but that really doesn't seem to have
> > ended up happening after all. So right now it's probably just best to
> > get Luc's tree instead from
> > 
> > https://github.com/lucvoo/sparse-dev
> > 
> > which also ends up fixing a lot of other issues.
> 
> Oh,
>  and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.

(+ Konstantin)

Something that is confusing for kernel developers is that the sparse wiki
(https://sparse.wiki.kernel.org/index.php/Main_Page) refers kernel developers
to an outdated sparse tree. Should something be done about this inconsistency?
Should the sparse wiki point at one of Luc's sparse trees or should perhaps
Luc be granted access to the tree that wiki points at
(https://git.kernel.org/pub/scm/devel/sparse/sparse.git)?

Thanks,

Bart.

[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Olof Johansson]

There's some use in printing out what the implementer and part numbers
decode to for cases where they're known.

I filled in the table based on public information; mostly from ARM TRMs
and other tools (and some of the SSBD tables in the kernel, etc).

Apple IDs came from
https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h

Signed-off-by: Olof Johansson 
---
 arch/arm64/kernel/cpuinfo.c | 79 +++--
 1 file changed, 76 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index e9ab7b3..9a7c25d 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -121,6 +121,67 @@ static const char *const compat_hwcap2_str[] = {
 };
 #endif /* CONFIG_COMPAT */
 
+struct midr_info {
+   unsigned int mask;
+   unsigned int val;
+   char *id;
+};
+
+static const struct midr_info midr_part[] = {
+   // ARM
+   { .mask = 0xff00fff0, .val = 0x4100d030, .id = "Cortex-A53" },
+   { .mask = 0xff00fff0, .val = 0x4100d040, .id = "Cortex-A35" },
+   { .mask = 0xff00fff0, .val = 0x4100d070, .id = "Cortex-A57" },
+   { .mask = 0xff00fff0, .val = 0x4100d080, .id = "Cortex-A72" },
+   { .mask = 0xff00fff0, .val = 0x4100d090, .id = "Cortex-A73" },
+   { .mask = 0xff00fff0, .val = 0x4100d0a0, .id = "Cortex-A75" },
+   { .mask = 0xff00fff0, .val = 0x4100d0f0, .id = "Cortex-A55" },
+   // Broadcom
+   { .mask = 0xff00fff0, .val = 0x42001000, .id = "Brahma-B53" },
+   { .mask = 0xff00fff0, .val = 0x42005160, .id = "ThunderX2" },
+   // Cavium
+   { .mask = 0xff00fff0, .val = 0x43000a00, .id = "ThunderX" },
+   { .mask = 0xff00fff0, .val = 0x43000a10, .id = "ThunderX 88xx" },
+   { .mask = 0xff00fff0, .val = 0x43000a20, .id = "ThunderX 81xx" },
+   { .mask = 0xff00fff0, .val = 0x43000a30, .id = "ThunderX 83xx" },
+   { .mask = 0xff00fff0, .val = 0x43000af0, .id = "ThunderX2 99xx" },
+   // Nvidia
+   { .mask = 0xff00fff0, .val = 0x4e00, .id = "Denver" },
+   { .mask = 0xff00fff0, .val = 0x4e30, .id = "Denver 2" },
+   // Applied Micro
+   { .mask = 0xff00fff0, .val = 0x5000, .id = "X-Gene" },
+   // Qualcomm
+   { .mask = 0xff00fff0, .val = 0x51002010, .id = "Kryo" },
+   { .mask = 0xff00fff0, .val = 0x51002050, .id = "Kryo" },
+   { .mask = 0xff00fff0, .val = 0x51002110, .id = "Kryo" },
+   { .mask = 0xff00fff0, .val = 0x51008000, .id = "Falkor V1/Kryo" },
+   { .mask = 0xff00fff0, .val = 0x51008010, .id = "Kryo V2" },
+   { .mask = 0xff00fff0, .val = 0x5100c000, .id = "Falkor" },
+   { .mask = 0xff00fff0, .val = 0x5100c010, .id = "Saphira" },
+   // Samsung
+   { .mask = 0xff00fff0, .val = 0x5310, .id = "M1" },
+   // Apple
+   { .mask = 0xff00fff0, .val = 0x6110, .id = "Cyclone" },
+   { .mask = 0xff00fff0, .val = 0x6120, .id = "Typhoon" },
+   { .mask = 0xff00fff0, .val = 0x6130, .id = "Typhoon/Capri" },
+   { .mask = 0xff00fff0, .val = 0x6140, .id = "Twister" },
+   { .mask = 0xff00fff0, .val = 0x6150, .id = "Twister/Elba/Malta" },
+   { .mask = 0xff00fff0, .val = 0x6160, .id = "Hurricane" },
+   { .mask = 0xff00fff0, .val = 0x6170, .id = "Hurricane/Myst" },
+};
+
+static const struct midr_info midr_impl[] = {
+   { .mask = 0xff00, .val = 0x4100, .id = "ARM" },
+   { .mask = 0xff00, .val = 0x4200, .id = "Broadcom" },
+   { .mask = 0xff00, .val = 0x4300, .id = "Cavium" },
+   { .mask = 0xff00, .val = 0x4d00, .id = "Motorola" },
+   { .mask = 0xff00, .val = 0x4e00, .id = "Nvidia" },
+   { .mask = 0xff00, .val = 0x5000, .id = "Applied Micro" },
+   { .mask = 0xff00, .val = 0x5100, .id = "Qualcomm" },
+   { .mask = 0xff00, .val = 0x5300, .id = "Samsung" },
+   { .mask = 0xff00, .val = 0x6100, .id = "Apple" },
+};
+
 static int c_show(struct seq_file *m, void *v)
 {
int i, j;
@@ -129,6 +190,16 @@ static int c_show(struct seq_file *m, void *v)
for_each_online_cpu(i) {
struct cpuinfo_arm64 *cpuinfo = _cpu(cpu_data, i);
u32 midr = cpuinfo->reg_midr;
+   char *impl = NULL;
+   char *part = NULL;
+
+   for (j = 0; !impl && j < ARRAY_SIZE(midr_impl); j++)
+   if ((midr & midr_impl[j].mask) == midr_impl[j].val)
+   impl = midr_impl[j].id;
+
+   for (j = 0; !part && j < ARRAY_SIZE(midr_part); j++)
+   if ((midr & midr_part[j].mask) == midr_part[j].val)
+   part = midr_part[j].id;
 
/*
 * glibc reads /proc/cpuinfo to determine the number of
@@ -168,11 +239,12 @@ static int c_show(struct seq_file *m, void *v)
}
seq_puts(m, "\n");
 
-   seq_printf(m, 

[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Olof Johansson]

There's some use in printing out what the implementer and part numbers
decode to for cases where they're known.

I filled in the table based on public information; mostly from ARM TRMs
and other tools (and some of the SSBD tables in the kernel, etc).

Apple IDs came from
https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h

Signed-off-by: Olof Johansson 
---
 arch/arm64/kernel/cpuinfo.c | 79 +++--
 1 file changed, 76 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index e9ab7b3..9a7c25d 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -121,6 +121,67 @@ static const char *const compat_hwcap2_str[] = {
 };
 #endif /* CONFIG_COMPAT */
 
+struct midr_info {
+   unsigned int mask;
+   unsigned int val;
+   char *id;
+};
+
+static const struct midr_info midr_part[] = {
+   // ARM
+   { .mask = 0xff00fff0, .val = 0x4100d030, .id = "Cortex-A53" },
+   { .mask = 0xff00fff0, .val = 0x4100d040, .id = "Cortex-A35" },
+   { .mask = 0xff00fff0, .val = 0x4100d070, .id = "Cortex-A57" },
+   { .mask = 0xff00fff0, .val = 0x4100d080, .id = "Cortex-A72" },
+   { .mask = 0xff00fff0, .val = 0x4100d090, .id = "Cortex-A73" },
+   { .mask = 0xff00fff0, .val = 0x4100d0a0, .id = "Cortex-A75" },
+   { .mask = 0xff00fff0, .val = 0x4100d0f0, .id = "Cortex-A55" },
+   // Broadcom
+   { .mask = 0xff00fff0, .val = 0x42001000, .id = "Brahma-B53" },
+   { .mask = 0xff00fff0, .val = 0x42005160, .id = "ThunderX2" },
+   // Cavium
+   { .mask = 0xff00fff0, .val = 0x43000a00, .id = "ThunderX" },
+   { .mask = 0xff00fff0, .val = 0x43000a10, .id = "ThunderX 88xx" },
+   { .mask = 0xff00fff0, .val = 0x43000a20, .id = "ThunderX 81xx" },
+   { .mask = 0xff00fff0, .val = 0x43000a30, .id = "ThunderX 83xx" },
+   { .mask = 0xff00fff0, .val = 0x43000af0, .id = "ThunderX2 99xx" },
+   // Nvidia
+   { .mask = 0xff00fff0, .val = 0x4e00, .id = "Denver" },
+   { .mask = 0xff00fff0, .val = 0x4e30, .id = "Denver 2" },
+   // Applied Micro
+   { .mask = 0xff00fff0, .val = 0x5000, .id = "X-Gene" },
+   // Qualcomm
+   { .mask = 0xff00fff0, .val = 0x51002010, .id = "Kryo" },
+   { .mask = 0xff00fff0, .val = 0x51002050, .id = "Kryo" },
+   { .mask = 0xff00fff0, .val = 0x51002110, .id = "Kryo" },
+   { .mask = 0xff00fff0, .val = 0x51008000, .id = "Falkor V1/Kryo" },
+   { .mask = 0xff00fff0, .val = 0x51008010, .id = "Kryo V2" },
+   { .mask = 0xff00fff0, .val = 0x5100c000, .id = "Falkor" },
+   { .mask = 0xff00fff0, .val = 0x5100c010, .id = "Saphira" },
+   // Samsung
+   { .mask = 0xff00fff0, .val = 0x5310, .id = "M1" },
+   // Apple
+   { .mask = 0xff00fff0, .val = 0x6110, .id = "Cyclone" },
+   { .mask = 0xff00fff0, .val = 0x6120, .id = "Typhoon" },
+   { .mask = 0xff00fff0, .val = 0x6130, .id = "Typhoon/Capri" },
+   { .mask = 0xff00fff0, .val = 0x6140, .id = "Twister" },
+   { .mask = 0xff00fff0, .val = 0x6150, .id = "Twister/Elba/Malta" },
+   { .mask = 0xff00fff0, .val = 0x6160, .id = "Hurricane" },
+   { .mask = 0xff00fff0, .val = 0x6170, .id = "Hurricane/Myst" },
+};
+
+static const struct midr_info midr_impl[] = {
+   { .mask = 0xff00, .val = 0x4100, .id = "ARM" },
+   { .mask = 0xff00, .val = 0x4200, .id = "Broadcom" },
+   { .mask = 0xff00, .val = 0x4300, .id = "Cavium" },
+   { .mask = 0xff00, .val = 0x4d00, .id = "Motorola" },
+   { .mask = 0xff00, .val = 0x4e00, .id = "Nvidia" },
+   { .mask = 0xff00, .val = 0x5000, .id = "Applied Micro" },
+   { .mask = 0xff00, .val = 0x5100, .id = "Qualcomm" },
+   { .mask = 0xff00, .val = 0x5300, .id = "Samsung" },
+   { .mask = 0xff00, .val = 0x6100, .id = "Apple" },
+};
+
 static int c_show(struct seq_file *m, void *v)
 {
int i, j;
@@ -129,6 +190,16 @@ static int c_show(struct seq_file *m, void *v)
for_each_online_cpu(i) {
struct cpuinfo_arm64 *cpuinfo = _cpu(cpu_data, i);
u32 midr = cpuinfo->reg_midr;
+   char *impl = NULL;
+   char *part = NULL;
+
+   for (j = 0; !impl && j < ARRAY_SIZE(midr_impl); j++)
+   if ((midr & midr_impl[j].mask) == midr_impl[j].val)
+   impl = midr_impl[j].id;
+
+   for (j = 0; !part && j < ARRAY_SIZE(midr_part); j++)
+   if ((midr & midr_part[j].mask) == midr_part[j].val)
+   part = midr_part[j].id;
 
/*
 * glibc reads /proc/cpuinfo to determine the number of
@@ -168,11 +239,12 @@ static int c_show(struct seq_file *m, void *v)
}
seq_puts(m, "\n");
 
-   seq_printf(m, 

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Bart Van Assche]

On Sat, 2018-07-14 at 18:59 -0700, Linus Torvalds wrote:
> On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds
>  wrote:
> > 
> > Honestly, I'd like to just encourage people to get the sparse update
> > from Luc Van Oostenryck instead.
> > 
> > For a while there it looked like Chris Li would just pull from Luc,
> > and we'd have timely releases, but that really doesn't seem to have
> > ended up happening after all. So right now it's probably just best to
> > get Luc's tree instead from
> > 
> > https://github.com/lucvoo/sparse-dev
> > 
> > which also ends up fixing a lot of other issues.
> > 
> > Linus
>
> Oh,
>  and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.
> 
> Linus

Hello everyone,

I will switch to Luc's sparse tree.

Thank you for the feedback.

Bart.

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Bart Van Assche]

On Sat, 2018-07-14 at 18:59 -0700, Linus Torvalds wrote:
> On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds
>  wrote:
> > 
> > Honestly, I'd like to just encourage people to get the sparse update
> > from Luc Van Oostenryck instead.
> > 
> > For a while there it looked like Chris Li would just pull from Luc,
> > and we'd have timely releases, but that really doesn't seem to have
> > ended up happening after all. So right now it's probably just best to
> > get Luc's tree instead from
> > 
> > https://github.com/lucvoo/sparse-dev
> > 
> > which also ends up fixing a lot of other issues.
> > 
> > Linus
>
> Oh,
>  and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.
> 
> Linus

Hello everyone,

I will switch to Luc's sparse tree.

Thank you for the feedback.

Bart.

[PATCH] x86/boot: Fix if_changed build flip/flop

[Kees Cook]

The if_changed kbuild function can only be used once per target. If not
it will effectively always trigger, flipping back and forth between the
two commands getting recorded. Instead, merge the two commands into a
single function to get stable build artifacts (i.e. .vmlinux.cmd).

Reported-by: Dirk Gouders 
Suggested-by: Masahiro Yamada 
Fixes: 98f78525371b ("x86/boot: Refuse to build with data relocations")
Signed-off-by: Kees Cook 
---
 arch/x86/boot/compressed/Makefile | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index fa42f895fdde..169c2feda14a 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -106,9 +106,13 @@ define cmd_check_data_rel
done
 endef
 
+# We need to run two commands under "if_changed", so merge them into a
+# single invocation.
+quiet_cmd_check-and-link-vmlinux = LD  $@
+  cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
+
 $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
-   $(call if_changed,check_data_rel)
-   $(call if_changed,ld)
+   $(call if_changed,check-and-link-vmlinux)
 
 OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
 $(obj)/vmlinux.bin: vmlinux FORCE
-- 
2.17.1


-- 
Kees Cook
Pixel Security

[PATCH] x86/boot: Fix if_changed build flip/flop

[Kees Cook]

The if_changed kbuild function can only be used once per target. If not
it will effectively always trigger, flipping back and forth between the
two commands getting recorded. Instead, merge the two commands into a
single function to get stable build artifacts (i.e. .vmlinux.cmd).

Reported-by: Dirk Gouders 
Suggested-by: Masahiro Yamada 
Fixes: 98f78525371b ("x86/boot: Refuse to build with data relocations")
Signed-off-by: Kees Cook 
---
 arch/x86/boot/compressed/Makefile | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index fa42f895fdde..169c2feda14a 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -106,9 +106,13 @@ define cmd_check_data_rel
done
 endef
 
+# We need to run two commands under "if_changed", so merge them into a
+# single invocation.
+quiet_cmd_check-and-link-vmlinux = LD  $@
+  cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
+
 $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
-   $(call if_changed,check_data_rel)
-   $(call if_changed,ld)
+   $(call if_changed,check-and-link-vmlinux)
 
 OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
 $(obj)/vmlinux.bin: vmlinux FORCE
-- 
2.17.1


-- 
Kees Cook
Pixel Security

Re: [PATCH] mm: avoid bothering interrupted task when charge memcg in softirq

[Yafang Shao]

On Sun, Jul 15, 2018 at 10:10 AM, Yafang Shao  wrote:
> On Sat, Jul 14, 2018 at 11:38 PM, Shakeel Butt  wrote:
>> On Sat, Jul 14, 2018 at 1:32 AM Yafang Shao  wrote:
>>>
>>> try_charge maybe executed in packet receive path, which is in interrupt
>>> context.
>>> In this situation, the 'current' is the interrupted task, which may has
>>> no relation to the rx softirq, So it is nonsense to use 'current'.
>>>
>>
>> Have you actually seen this occurring?
>
> Hi Shakeel,
>
> I'm trying to produce this issue, but haven't find it occur yet.
>
>> I am not very familiar with the
>> network code but I can think of two ways try_charge() can be called
>> from network code. Either through kmem charging or through
>> mem_cgroup_charge_skmem() and both locations correctly handle
>> interrupt context.
>>
>
> Why do you say that mem_cgroup_charge_skmem() correctly hanle
> interrupt context ?
>
> Let me show you why mem_cgroup_charge_skmem isn't hanling interrupt
> context correctly.
>
> mem_cgroup_charge_skmem() is calling  try_charge() twice.
> The first one is with GFP_NOWAIT as the gfp_mask, and the second one
> is with  (GFP_NOWAIT |  __GFP_NOFAIL) as the gfp_mask.
>
> If page_counter_try_charge() failes at the first time, -ENOMEM is returned.
> Then mem_cgroup_charge_skmem() will call try_charge() once more with
> __GFP_NOFAIL set, and this time if If page_counter_try_charge() failes
> again the '
> force' label in  try_charge() will be executed and 0 is returned.
>
> No matter what, the 'current' will be used and touched, that is
> meaning mem_cgroup_charge_skmem() isn't hanling the interrupt context
> correctly.
>
> Pls. let me know if I miss something.
>
>

Maybe bellow change is better,
@@ -2123,6 +2123,9 @@ static int try_charge(struct mem_cgroup *memcg,
gfp_t gfp_mask,
goto retry;
}

+   if (!gfpflags_allow_blocking(gfp_mask))
+   goto nomem;
+
/*
 * Unlike in global OOM situations, memcg is not in a physical
 * memory shortage.  Allow dying and OOM-killed tasks to
@@ -2146,9 +2149,6 @@ static int try_charge(struct mem_cgroup *memcg,
gfp_t gfp_mask,
if (unlikely(task_in_memcg_oom(current)))
goto nomem;

-   if (!gfpflags_allow_blocking(gfp_mask))
-   goto nomem;

Thanks
Yafang

Re: [PATCH 1/1] f2fs: checkpoint disabling

[Jaegeuk Kim]

On 07/11, Daniel Rosenberg wrote:
> This adds a lightweight non-persistent snapshotting scheme to f2fs.
> 
> To use, mount with the option checkpoint=disable, and to return to
> normal operation, remount with checkpoint=enable. If the filesystem
> is shut down before remounting with checkpoint=enable, it will revert
> back to its apparent state when it was first mounted with
> checkpoint=disable. This is useful for situations where you wish to be
> able to roll back the state of the disk in case of some critical
> failure.
> 
> Signed-off-by: Daniel Rosenberg 
> ---
> 
> This probably needs some work in the mount/remount areas to ensure it
> plays nicely with all combinations of other options.
> I'm also unsure how it should interact with statfs.
> 
> It currently handles accounting for free space in checkpoint disabled
> mode by setting up addition tracking for free data blocks, node blocks,
> and segments. These are used in inc_valid_block_cnt and inc_valid_node_cnt
> to track what the state will be once the blocks are actually allocated.
> We choose new current segments in SSR mode first to avoid the edge case
> where the disk is not yet full, but we only have dirty segments remaining
> that happen to not be of the right type. We also agressively add segments
> to the dirty list instead of pre-free when it is possible to reuse them to
> allow us to continue without a checkpoint as long as possible.
> 
>  Documentation/filesystems/f2fs.txt |   5 ++
>  fs/f2fs/data.c |  21 ++
>  fs/f2fs/f2fs.h |  63 +++-
>  fs/f2fs/file.c |  18 +
>  fs/f2fs/gc.c   |   4 +
>  fs/f2fs/segment.c  |  96 +++-
>  fs/f2fs/segment.h  |  66 +
>  fs/f2fs/super.c| 115 +++--
>  8 files changed, 326 insertions(+), 62 deletions(-)
> 
> diff --git a/Documentation/filesystems/f2fs.txt 
> b/Documentation/filesystems/f2fs.txt
> index 69f8de9957397..a026b353a99d4 100644
> --- a/Documentation/filesystems/f2fs.txt
> +++ b/Documentation/filesystems/f2fs.txt
> @@ -193,6 +193,11 @@ fsync_mode=%s  Control the policy of fsync. 
> Currently supports "posix",
> non-atomic files likewise "nobarrier" mount option.
>  test_dummy_encryption  Enable dummy encryption, which provides a fake fscrypt
> context. The fake fscrypt context is used by xfstests.
> +checkpoint=%s  Set to "disable" to turn off checkpointing. Set to 
> "enable"
> +   to reenable checkpointing. Is enabled by default. 
> While
> +   disabled, any unmounting or unexpected shutdowns will 
> cause
> +   the filesystem contents to appear as they did when the
> +   filesystem was mounted with that option.
>  
>  
> 
>  DEBUGFS ENTRIES
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 83d4cff445f53..b3fa713fd42bf 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1654,9 +1654,20 @@ bool f2fs_should_update_inplace(struct inode *inode, 
> struct f2fs_io_info *fio)
>  bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info 
> *fio)
>  {
>   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> + struct seg_entry *se;
> + unsigned int segno, offset;
>  
>   if (test_opt(sbi, LFS))
>   return true;
> + if (test_opt(sbi, DISABLE_CHECKPOINT)) {

struct seg_entry *se;
unsigned int segno, offset;

> + if (fio->old_blkaddr == NULL_ADDR)
-
NEW_ADDR

> + return true;
> + segno = GET_SEGNO(sbi, fio->old_blkaddr);
> + se = get_seg_entry(sbi, segno);
> + offset = GET_BLKOFF_FROM_SEG0(sbi, fio->old_blkaddr);
> + if (f2fs_test_bit(offset, se->ckpt_valid_map))
> + return true;
> + }
>   if (S_ISDIR(inode->i_mode))
>   return true;
>   if (f2fs_is_atomic_file(inode))
> @@ -1684,9 +1695,12 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>  {
>   struct page *page = fio->page;
>   struct inode *inode = page->mapping->host;
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>   struct dnode_of_data dn;
>   struct extent_info ei = {0,0,0};
>   bool ipu_force = false;
> + bool need_tmp_grab = test_opt(sbi, DISABLE_CHECKPOINT);
> + blkcnt_t tmp_block = 1;
>   int err = 0;
>  
>   set_new_dnode(, inode, NULL, NULL, 0);
> @@ -1750,6 +1764,11 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>   if (err)
>   goto out_writepage;
>  
> + if (need_tmp_grab) {
> + err = inc_valid_block_count(sbi, dn.inode, _block);

Re: [PATCH] mm: avoid bothering interrupted task when charge memcg in softirq

[Yafang Shao]

On Sun, Jul 15, 2018 at 10:10 AM, Yafang Shao  wrote:
> On Sat, Jul 14, 2018 at 11:38 PM, Shakeel Butt  wrote:
>> On Sat, Jul 14, 2018 at 1:32 AM Yafang Shao  wrote:
>>>
>>> try_charge maybe executed in packet receive path, which is in interrupt
>>> context.
>>> In this situation, the 'current' is the interrupted task, which may has
>>> no relation to the rx softirq, So it is nonsense to use 'current'.
>>>
>>
>> Have you actually seen this occurring?
>
> Hi Shakeel,
>
> I'm trying to produce this issue, but haven't find it occur yet.
>
>> I am not very familiar with the
>> network code but I can think of two ways try_charge() can be called
>> from network code. Either through kmem charging or through
>> mem_cgroup_charge_skmem() and both locations correctly handle
>> interrupt context.
>>
>
> Why do you say that mem_cgroup_charge_skmem() correctly hanle
> interrupt context ?
>
> Let me show you why mem_cgroup_charge_skmem isn't hanling interrupt
> context correctly.
>
> mem_cgroup_charge_skmem() is calling  try_charge() twice.
> The first one is with GFP_NOWAIT as the gfp_mask, and the second one
> is with  (GFP_NOWAIT |  __GFP_NOFAIL) as the gfp_mask.
>
> If page_counter_try_charge() failes at the first time, -ENOMEM is returned.
> Then mem_cgroup_charge_skmem() will call try_charge() once more with
> __GFP_NOFAIL set, and this time if If page_counter_try_charge() failes
> again the '
> force' label in  try_charge() will be executed and 0 is returned.
>
> No matter what, the 'current' will be used and touched, that is
> meaning mem_cgroup_charge_skmem() isn't hanling the interrupt context
> correctly.
>
> Pls. let me know if I miss something.
>
>

Maybe bellow change is better,
@@ -2123,6 +2123,9 @@ static int try_charge(struct mem_cgroup *memcg,
gfp_t gfp_mask,
goto retry;
}

+   if (!gfpflags_allow_blocking(gfp_mask))
+   goto nomem;
+
/*
 * Unlike in global OOM situations, memcg is not in a physical
 * memory shortage.  Allow dying and OOM-killed tasks to
@@ -2146,9 +2149,6 @@ static int try_charge(struct mem_cgroup *memcg,
gfp_t gfp_mask,
if (unlikely(task_in_memcg_oom(current)))
goto nomem;

-   if (!gfpflags_allow_blocking(gfp_mask))
-   goto nomem;

Thanks
Yafang

Re: [PATCH 1/1] f2fs: checkpoint disabling

[Jaegeuk Kim]

On 07/11, Daniel Rosenberg wrote:
> This adds a lightweight non-persistent snapshotting scheme to f2fs.
> 
> To use, mount with the option checkpoint=disable, and to return to
> normal operation, remount with checkpoint=enable. If the filesystem
> is shut down before remounting with checkpoint=enable, it will revert
> back to its apparent state when it was first mounted with
> checkpoint=disable. This is useful for situations where you wish to be
> able to roll back the state of the disk in case of some critical
> failure.
> 
> Signed-off-by: Daniel Rosenberg 
> ---
> 
> This probably needs some work in the mount/remount areas to ensure it
> plays nicely with all combinations of other options.
> I'm also unsure how it should interact with statfs.
> 
> It currently handles accounting for free space in checkpoint disabled
> mode by setting up addition tracking for free data blocks, node blocks,
> and segments. These are used in inc_valid_block_cnt and inc_valid_node_cnt
> to track what the state will be once the blocks are actually allocated.
> We choose new current segments in SSR mode first to avoid the edge case
> where the disk is not yet full, but we only have dirty segments remaining
> that happen to not be of the right type. We also agressively add segments
> to the dirty list instead of pre-free when it is possible to reuse them to
> allow us to continue without a checkpoint as long as possible.
> 
>  Documentation/filesystems/f2fs.txt |   5 ++
>  fs/f2fs/data.c |  21 ++
>  fs/f2fs/f2fs.h |  63 +++-
>  fs/f2fs/file.c |  18 +
>  fs/f2fs/gc.c   |   4 +
>  fs/f2fs/segment.c  |  96 +++-
>  fs/f2fs/segment.h  |  66 +
>  fs/f2fs/super.c| 115 +++--
>  8 files changed, 326 insertions(+), 62 deletions(-)
> 
> diff --git a/Documentation/filesystems/f2fs.txt 
> b/Documentation/filesystems/f2fs.txt
> index 69f8de9957397..a026b353a99d4 100644
> --- a/Documentation/filesystems/f2fs.txt
> +++ b/Documentation/filesystems/f2fs.txt
> @@ -193,6 +193,11 @@ fsync_mode=%s  Control the policy of fsync. 
> Currently supports "posix",
> non-atomic files likewise "nobarrier" mount option.
>  test_dummy_encryption  Enable dummy encryption, which provides a fake fscrypt
> context. The fake fscrypt context is used by xfstests.
> +checkpoint=%s  Set to "disable" to turn off checkpointing. Set to 
> "enable"
> +   to reenable checkpointing. Is enabled by default. 
> While
> +   disabled, any unmounting or unexpected shutdowns will 
> cause
> +   the filesystem contents to appear as they did when the
> +   filesystem was mounted with that option.
>  
>  
> 
>  DEBUGFS ENTRIES
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 83d4cff445f53..b3fa713fd42bf 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1654,9 +1654,20 @@ bool f2fs_should_update_inplace(struct inode *inode, 
> struct f2fs_io_info *fio)
>  bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info 
> *fio)
>  {
>   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> + struct seg_entry *se;
> + unsigned int segno, offset;
>  
>   if (test_opt(sbi, LFS))
>   return true;
> + if (test_opt(sbi, DISABLE_CHECKPOINT)) {

struct seg_entry *se;
unsigned int segno, offset;

> + if (fio->old_blkaddr == NULL_ADDR)
-
NEW_ADDR

> + return true;
> + segno = GET_SEGNO(sbi, fio->old_blkaddr);
> + se = get_seg_entry(sbi, segno);
> + offset = GET_BLKOFF_FROM_SEG0(sbi, fio->old_blkaddr);
> + if (f2fs_test_bit(offset, se->ckpt_valid_map))
> + return true;
> + }
>   if (S_ISDIR(inode->i_mode))
>   return true;
>   if (f2fs_is_atomic_file(inode))
> @@ -1684,9 +1695,12 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>  {
>   struct page *page = fio->page;
>   struct inode *inode = page->mapping->host;
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>   struct dnode_of_data dn;
>   struct extent_info ei = {0,0,0};
>   bool ipu_force = false;
> + bool need_tmp_grab = test_opt(sbi, DISABLE_CHECKPOINT);
> + blkcnt_t tmp_block = 1;
>   int err = 0;
>  
>   set_new_dnode(, inode, NULL, NULL, 0);
> @@ -1750,6 +1764,11 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>   if (err)
>   goto out_writepage;
>  
> + if (need_tmp_grab) {
> + err = inc_valid_block_count(sbi, dn.inode, _block);

Re: [PATCH] mm: avoid bothering interrupted task when charge memcg in softirq

[Yafang Shao]

On Sat, Jul 14, 2018 at 11:38 PM, Shakeel Butt  wrote:
> On Sat, Jul 14, 2018 at 1:32 AM Yafang Shao  wrote:
>>
>> try_charge maybe executed in packet receive path, which is in interrupt
>> context.
>> In this situation, the 'current' is the interrupted task, which may has
>> no relation to the rx softirq, So it is nonsense to use 'current'.
>>
>
> Have you actually seen this occurring?

Hi Shakeel,

I'm trying to produce this issue, but haven't find it occur yet.

> I am not very familiar with the
> network code but I can think of two ways try_charge() can be called
> from network code. Either through kmem charging or through
> mem_cgroup_charge_skmem() and both locations correctly handle
> interrupt context.
>

Why do you say that mem_cgroup_charge_skmem() correctly hanle
interrupt context ?

Let me show you why mem_cgroup_charge_skmem isn't hanling interrupt
context correctly.

mem_cgroup_charge_skmem() is calling  try_charge() twice.
The first one is with GFP_NOWAIT as the gfp_mask, and the second one
is with  (GFP_NOWAIT |  __GFP_NOFAIL) as the gfp_mask.

If page_counter_try_charge() failes at the first time, -ENOMEM is returned.
Then mem_cgroup_charge_skmem() will call try_charge() once more with
__GFP_NOFAIL set, and this time if If page_counter_try_charge() failes
again the '
force' label in  try_charge() will be executed and 0 is returned.

No matter what, the 'current' will be used and touched, that is
meaning mem_cgroup_charge_skmem() isn't hanling the interrupt context
correctly.

Pls. let me know if I miss something.


Thanks
Yafang

Re: [PATCH] mm: avoid bothering interrupted task when charge memcg in softirq

[Yafang Shao]

On Sat, Jul 14, 2018 at 11:38 PM, Shakeel Butt  wrote:
> On Sat, Jul 14, 2018 at 1:32 AM Yafang Shao  wrote:
>>
>> try_charge maybe executed in packet receive path, which is in interrupt
>> context.
>> In this situation, the 'current' is the interrupted task, which may has
>> no relation to the rx softirq, So it is nonsense to use 'current'.
>>
>
> Have you actually seen this occurring?

Hi Shakeel,

I'm trying to produce this issue, but haven't find it occur yet.

> I am not very familiar with the
> network code but I can think of two ways try_charge() can be called
> from network code. Either through kmem charging or through
> mem_cgroup_charge_skmem() and both locations correctly handle
> interrupt context.
>

Why do you say that mem_cgroup_charge_skmem() correctly hanle
interrupt context ?

Let me show you why mem_cgroup_charge_skmem isn't hanling interrupt
context correctly.

mem_cgroup_charge_skmem() is calling  try_charge() twice.
The first one is with GFP_NOWAIT as the gfp_mask, and the second one
is with  (GFP_NOWAIT |  __GFP_NOFAIL) as the gfp_mask.

If page_counter_try_charge() failes at the first time, -ENOMEM is returned.
Then mem_cgroup_charge_skmem() will call try_charge() once more with
__GFP_NOFAIL set, and this time if If page_counter_try_charge() failes
again the '
force' label in  try_charge() will be executed and 0 is returned.

No matter what, the 'current' will be used and touched, that is
meaning mem_cgroup_charge_skmem() isn't hanling the interrupt context
correctly.

Pls. let me know if I miss something.


Thanks
Yafang

Re: [PATCH v7] add param that allows bootline control of hardened usercopy

[Kees Cook]

On Wed, Jul 4, 2018 at 10:47 AM, Vlastimil Babka  wrote:
> On 07/04/2018 06:52 PM, Kees Cook wrote:
>> This produces less efficient code in the general case, and I'd like to
>> keep the general case (hardening enabled) as fast as possible.
>
> How specifically is the code less efficient? It should be always a
> static key check (no-op thanks to the code patching involved) and a
> function call in the "hardening enabled" case, just in different order.
> And in either case compiled out if it's a constant.

My understanding from reading the jump label comments[1] is that on
order produces:

NOP
do normal thing
label1:
do rest of function
RET
label2:
do exceptional thing
jump label1

where "NOP" is changed to "JMP label2" when toggled, and the other is:

JMP label1
do exceptional thing
JMP label2
label1:
do normal thing
label2:
do rest of function
RET

where "JMP label1" is changed to NOP when toggled. (i.e. does the
default do NOP, thing, function, or does the default to JMP, thing,
JMP, function)

-Kees

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/jump_label.h#n334

-- 
Kees Cook
Pixel Security

Re: [PATCH v7] add param that allows bootline control of hardened usercopy

[Kees Cook]

On Wed, Jul 4, 2018 at 10:47 AM, Vlastimil Babka  wrote:
> On 07/04/2018 06:52 PM, Kees Cook wrote:
>> This produces less efficient code in the general case, and I'd like to
>> keep the general case (hardening enabled) as fast as possible.
>
> How specifically is the code less efficient? It should be always a
> static key check (no-op thanks to the code patching involved) and a
> function call in the "hardening enabled" case, just in different order.
> And in either case compiled out if it's a constant.

My understanding from reading the jump label comments[1] is that on
order produces:

NOP
do normal thing
label1:
do rest of function
RET
label2:
do exceptional thing
jump label1

where "NOP" is changed to "JMP label2" when toggled, and the other is:

JMP label1
do exceptional thing
JMP label2
label1:
do normal thing
label2:
do rest of function
RET

where "JMP label1" is changed to NOP when toggled. (i.e. does the
default do NOP, thing, function, or does the default to JMP, thing,
JMP, function)

-Kees

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/jump_label.h#n334

-- 
Kees Cook
Pixel Security

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Linus Torvalds]

Oh,
 and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.

Linus

On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds
 wrote:
>
> Honestly, I'd like to just encourage people to get the sparse update
> from Luc Van Oostenryck instead.
>
> For a while there it looked like Chris Li would just pull from Luc,
> and we'd have timely releases, but that really doesn't seem to have
> ended up happening after all. So right now it's probably just best to
> get Luc's tree instead from
>
> https://github.com/lucvoo/sparse-dev
>
> which also ends up fixing a lot of other issues.
>
> Linus

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Linus Torvalds]

Oh,
 and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.

Linus

On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds
 wrote:
>
> Honestly, I'd like to just encourage people to get the sparse update
> from Luc Van Oostenryck instead.
>
> For a while there it looked like Chris Li would just pull from Luc,
> and we'd have timely releases, but that really doesn't seem to have
> ended up happening after all. So right now it's probably just best to
> get Luc's tree instead from
>
> https://github.com/lucvoo/sparse-dev
>
> which also ends up fixing a lot of other issues.
>
> Linus

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Linus Torvalds]

On Sat, Jul 14, 2018 at 6:49 PM Kees Cook  wrote:
>
> I'm fine with this; it'll only activate for sparse. I'd like to get
> Linus's eyes on it, though, since this macro caused us SO much pain
> that I'm nervous to change it without some greater level of review. :)

Honestly, I'd like to just encourage people to get the sparse update
from Luc Van Oostenryck instead.

For a while there it looked like Chris Li would just pull from Luc,
and we'd have timely releases, but that really doesn't seem to have
ended up happening after all. So right now it's probably just best to
get Luc's tree instead from

https://github.com/lucvoo/sparse-dev

which also ends up fixing a lot of other issues.

Linus

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Linus Torvalds]

On Sat, Jul 14, 2018 at 6:49 PM Kees Cook  wrote:
>
> I'm fine with this; it'll only activate for sparse. I'd like to get
> Linus's eyes on it, though, since this macro caused us SO much pain
> that I'm nervous to change it without some greater level of review. :)

Honestly, I'd like to just encourage people to get the sparse update
from Luc Van Oostenryck instead.

For a while there it looked like Chris Li would just pull from Luc,
and we'd have timely releases, but that really doesn't seem to have
ended up happening after all. So right now it's probably just best to
get Luc's tree instead from

https://github.com/lucvoo/sparse-dev

which also ends up fixing a lot of other issues.

Linus

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Kees Cook]

On Thu, Jul 5, 2018 at 9:17 AM, Bart Van Assche  wrote:
> The macro __is_constexpr() causes sparse to report the following:
>
> warning: expression using sizeof(void)
>
> Avoid this by using __builtin_constant_p() instead.
>
> Fixes: 3c8ba0d61d04 ("kernel.h: Retain constant expression output for 
> max()/min()")
> Signed-off-by: Bart Van Assche 
> Cc: Linus Torvalds 
> Cc: Martin Uecker 
> Cc: Kees Cook 
> Cc: Ingo Molnar 
> Cc: Miguel Ojeda 
> Cc: 
> ---
>  include/linux/kernel.h | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index d23123238534..a9f0d0d48971 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -811,13 +811,19 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
> oops_dump_mode) { }
>  #define __typecheck(x, y) \
> (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>
> +#ifndef __CHECKER__
>  /*
>   * This returns a constant expression while determining if an argument is
>   * a constant expression, most importantly without evaluating the argument.
> - * Glory to Martin Uecker 
> + * Glory to Martin Uecker . However, 
> this
> + * macro causes sparse to report the warning "expression using sizeof(void)".
> + * Hence use __builtin_constant_p() instead when using sparse.
>   */
>  #define __is_constexpr(x) \
> (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
> +#else
> +#define __is_constexpr(x) __builtin_constant_p((x))
> +#endif
>
>  #define __no_side_effects(x, y) \
> (__is_constexpr(x) && __is_constexpr(y))

I'm fine with this; it'll only activate for sparse. I'd like to get
Linus's eyes on it, though, since this macro caused us SO much pain
that I'm nervous to change it without some greater level of review. :)

Acked-by: Kees Cook 

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Kees Cook]

On Thu, Jul 5, 2018 at 9:17 AM, Bart Van Assche  wrote:
> The macro __is_constexpr() causes sparse to report the following:
>
> warning: expression using sizeof(void)
>
> Avoid this by using __builtin_constant_p() instead.
>
> Fixes: 3c8ba0d61d04 ("kernel.h: Retain constant expression output for 
> max()/min()")
> Signed-off-by: Bart Van Assche 
> Cc: Linus Torvalds 
> Cc: Martin Uecker 
> Cc: Kees Cook 
> Cc: Ingo Molnar 
> Cc: Miguel Ojeda 
> Cc: 
> ---
>  include/linux/kernel.h | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index d23123238534..a9f0d0d48971 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -811,13 +811,19 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
> oops_dump_mode) { }
>  #define __typecheck(x, y) \
> (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>
> +#ifndef __CHECKER__
>  /*
>   * This returns a constant expression while determining if an argument is
>   * a constant expression, most importantly without evaluating the argument.
> - * Glory to Martin Uecker 
> + * Glory to Martin Uecker . However, 
> this
> + * macro causes sparse to report the warning "expression using sizeof(void)".
> + * Hence use __builtin_constant_p() instead when using sparse.
>   */
>  #define __is_constexpr(x) \
> (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
> +#else
> +#define __is_constexpr(x) __builtin_constant_p((x))
> +#endif
>
>  #define __no_side_effects(x, y) \
> (__is_constexpr(x) && __is_constexpr(y))

I'm fine with this; it'll only activate for sparse. I'd like to get
Linus's eyes on it, though, since this macro caused us SO much pain
that I'm nervous to change it without some greater level of review. :)

Acked-by: Kees Cook 

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v13 2/2] Add oom victim's memcg to the oom context information

[禹舟键]

Hi David
Could I use use plain old %d? Just like this,
pr_cont(",task=%s,pid=%d,uid=%d\n", p->comm, p->pid,
from_kuid(_user_ns, task_uid(p)));

Thanks

Re: [PATCH v13 2/2] Add oom victim's memcg to the oom context information

[禹舟键]

Hi David
Could I use use plain old %d? Just like this,
pr_cont(",task=%s,pid=%d,uid=%d\n", p->comm, p->pid,
from_kuid(_user_ns, task_uid(p)));

Thanks

[PATCH] RISC-V: Don't increment sepc after breakpoint.

[Jim Wilson]

Adding 4 to sepc is pointless, and is wrong if we executed a 2-byte
compressed breakpoint.  This plus a corresponding gdb patch allows
compressed breakpoints to work in gdb.  Gdb maintainers have already
agreed that this is the right approach.

Signed-off-by: Jim Wilson 
---
 arch/riscv/kernel/traps.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 14fcec5bdd24..ae323071c786 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -150,7 +150,6 @@ asmlinkage void do_trap_break(struct pt_regs *regs)
 #endif /* CONFIG_GENERIC_BUG */
 
do_trap_siginfo(SIGTRAP, TRAP_BRKPT, regs->sepc, current);
-   regs->sepc += 0x4;
 }
 
 #ifdef CONFIG_GENERIC_BUG
-- 
2.17.1

[PATCH] RISC-V: Don't increment sepc after breakpoint.

[Jim Wilson]

Adding 4 to sepc is pointless, and is wrong if we executed a 2-byte
compressed breakpoint.  This plus a corresponding gdb patch allows
compressed breakpoints to work in gdb.  Gdb maintainers have already
agreed that this is the right approach.

Signed-off-by: Jim Wilson 
---
 arch/riscv/kernel/traps.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 14fcec5bdd24..ae323071c786 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -150,7 +150,6 @@ asmlinkage void do_trap_break(struct pt_regs *regs)
 #endif /* CONFIG_GENERIC_BUG */
 
do_trap_siginfo(SIGTRAP, TRAP_BRKPT, regs->sepc, current);
-   regs->sepc += 0x4;
 }
 
 #ifdef CONFIG_GENERIC_BUG
-- 
2.17.1

Applied "ASoC: allow soc-core to pick up name prefixes from component nodes" to the asoc tree

[Mark Brown]

The patch

   ASoC: allow soc-core to pick up name prefixes from component nodes

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From aefba45539bc4868c1fae336410aec907ee0882a Mon Sep 17 00:00:00 2001
From: Jerome Brunet 
Date: Fri, 13 Jul 2018 14:50:43 +0200
Subject: [PATCH] ASoC: allow soc-core to pick up name prefixes from component
 nodes

When the component does not match the configuration table provided
by the card, let soc-core check the component node for a name prefix

Signed-off-by: Jerome Brunet 
Signed-off-by: Mark Brown 
---
 sound/soc/soc-core.c | 28 +++-
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 00bd58d167dd..3be0310d5c81 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1193,15 +1193,27 @@ void snd_soc_remove_dai_link(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
 
+static void soc_set_of_name_prefix(struct snd_soc_component *component)
+{
+   struct device_node *component_of_node = component->dev->of_node;
+   const char *str;
+   int ret;
+
+   if (!component_of_node && component->dev->parent)
+   component_of_node = component->dev->parent->of_node;
+
+   ret = of_property_read_string(component_of_node, "sound-name-prefix",
+ );
+   if (!ret)
+   component->name_prefix = str;
+}
+
 static void soc_set_name_prefix(struct snd_soc_card *card,
struct snd_soc_component *component)
 {
int i;
 
-   if (card->codec_conf == NULL)
-   return;
-
-   for (i = 0; i < card->num_configs; i++) {
+   for (i = 0; i < card->num_configs && card->codec_conf; i++) {
struct snd_soc_codec_conf *map = >codec_conf[i];
struct device_node *component_of_node = component->dev->of_node;
 
@@ -1213,8 +1225,14 @@ static void soc_set_name_prefix(struct snd_soc_card 
*card,
if (map->dev_name && strcmp(component->name, map->dev_name))
continue;
component->name_prefix = map->name_prefix;
-   break;
+   return;
}
+
+   /*
+* If there is no configuration table or no match in the table,
+* check if a prefix is provided in the node
+*/
+   soc_set_of_name_prefix(component);
 }
 
 static int soc_probe_component(struct snd_soc_card *card,
-- 
2.18.0

Re: [PATCH] base: core: Remove WARN_ON from link dependencies check

[Mark Brown]

On Thu, Jul 12, 2018 at 10:06:23AM +0200, Benjamin Gaignard wrote:
> In some cases the link between between customer and supplier
> already exist. Do not warn about already existing dependencies
> because device_link_add() take care of this case.

Reviwed-by: Mark Brown 


signature.asc
Description: PGP signature

Re: [PATCH] base: core: Remove WARN_ON from link dependencies check

[Mark Brown]

On Thu, Jul 12, 2018 at 10:06:23AM +0200, Benjamin Gaignard wrote:
> In some cases the link between between customer and supplier
> already exist. Do not warn about already existing dependencies
> because device_link_add() take care of this case.

Reviwed-by: Mark Brown 


signature.asc
Description: PGP signature

Applied "ASoC: allow soc-core to pick up name prefixes from component nodes" to the asoc tree

[Mark Brown]

The patch

   ASoC: allow soc-core to pick up name prefixes from component nodes

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From aefba45539bc4868c1fae336410aec907ee0882a Mon Sep 17 00:00:00 2001
From: Jerome Brunet 
Date: Fri, 13 Jul 2018 14:50:43 +0200
Subject: [PATCH] ASoC: allow soc-core to pick up name prefixes from component
 nodes

When the component does not match the configuration table provided
by the card, let soc-core check the component node for a name prefix

Signed-off-by: Jerome Brunet 
Signed-off-by: Mark Brown 
---
 sound/soc/soc-core.c | 28 +++-
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 00bd58d167dd..3be0310d5c81 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1193,15 +1193,27 @@ void snd_soc_remove_dai_link(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
 
+static void soc_set_of_name_prefix(struct snd_soc_component *component)
+{
+   struct device_node *component_of_node = component->dev->of_node;
+   const char *str;
+   int ret;
+
+   if (!component_of_node && component->dev->parent)
+   component_of_node = component->dev->parent->of_node;
+
+   ret = of_property_read_string(component_of_node, "sound-name-prefix",
+ );
+   if (!ret)
+   component->name_prefix = str;
+}
+
 static void soc_set_name_prefix(struct snd_soc_card *card,
struct snd_soc_component *component)
 {
int i;
 
-   if (card->codec_conf == NULL)
-   return;
-
-   for (i = 0; i < card->num_configs; i++) {
+   for (i = 0; i < card->num_configs && card->codec_conf; i++) {
struct snd_soc_codec_conf *map = >codec_conf[i];
struct device_node *component_of_node = component->dev->of_node;
 
@@ -1213,8 +1225,14 @@ static void soc_set_name_prefix(struct snd_soc_card 
*card,
if (map->dev_name && strcmp(component->name, map->dev_name))
continue;
component->name_prefix = map->name_prefix;
-   break;
+   return;
}
+
+   /*
+* If there is no configuration table or no match in the table,
+* check if a prefix is provided in the node
+*/
+   soc_set_of_name_prefix(component);
 }
 
 static int soc_probe_component(struct snd_soc_card *card,
-- 
2.18.0

Applied "ASoC: trace: remove snd_soc_codec" to the asoc tree

[Mark Brown]

The patch

   ASoC: trace: remove snd_soc_codec

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 211b4415ecb254e167837206756dea123342689c Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto 
Date: Wed, 28 Mar 2018 02:18:13 +
Subject: [PATCH] ASoC: trace: remove snd_soc_codec

snd_soc_codec is replaced to snd_soc_component,
and it is not used in this file.
Let's remove it

Signed-off-by: Kuninori Morimoto 
Signed-off-by: Mark Brown 
---
 include/trace/events/asoc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/trace/events/asoc.h b/include/trace/events/asoc.h
index ccd1a3bdff46..40c300fe704d 100644
--- a/include/trace/events/asoc.h
+++ b/include/trace/events/asoc.h
@@ -12,7 +12,6 @@
 #define DAPM_ARROW(dir) (((dir) == SND_SOC_DAPM_DIR_OUT) ? "->" : "<-")
 
 struct snd_soc_jack;
-struct snd_soc_codec;
 struct snd_soc_card;
 struct snd_soc_dapm_widget;
 struct snd_soc_dapm_path;
-- 
2.18.0

Applied "ASoC: trace: remove snd_soc_codec" to the asoc tree

[Mark Brown]

The patch

   ASoC: trace: remove snd_soc_codec

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 211b4415ecb254e167837206756dea123342689c Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto 
Date: Wed, 28 Mar 2018 02:18:13 +
Subject: [PATCH] ASoC: trace: remove snd_soc_codec

snd_soc_codec is replaced to snd_soc_component,
and it is not used in this file.
Let's remove it

Signed-off-by: Kuninori Morimoto 
Signed-off-by: Mark Brown 
---
 include/trace/events/asoc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/trace/events/asoc.h b/include/trace/events/asoc.h
index ccd1a3bdff46..40c300fe704d 100644
--- a/include/trace/events/asoc.h
+++ b/include/trace/events/asoc.h
@@ -12,7 +12,6 @@
 #define DAPM_ARROW(dir) (((dir) == SND_SOC_DAPM_DIR_OUT) ? "->" : "<-")
 
 struct snd_soc_jack;
-struct snd_soc_codec;
 struct snd_soc_card;
 struct snd_soc_dapm_widget;
 struct snd_soc_dapm_path;
-- 
2.18.0

Applied "ASoC: add DT documentation for the sound-name-prefix property" to the asoc tree

[Mark Brown]

The patch

   ASoC: add DT documentation for the sound-name-prefix property

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d5418ae3f9443f911d4324c0cade988ced39cfbe Mon Sep 17 00:00:00 2001
From: Jerome Brunet 
Date: Fri, 13 Jul 2018 14:50:42 +0200
Subject: [PATCH] ASoC: add DT documentation for the sound-name-prefix property

Signed-off-by: Jerome Brunet 
Signed-off-by: Mark Brown 
---
 .../devicetree/bindings/sound/name-prefix.txt | 24 +++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/name-prefix.txt

diff --git a/Documentation/devicetree/bindings/sound/name-prefix.txt 
b/Documentation/devicetree/bindings/sound/name-prefix.txt
new file mode 100644
index ..645775908657
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/name-prefix.txt
@@ -0,0 +1,24 @@
+Name prefix:
+
+Card implementing the routing property define the connection between
+audio components as list of string pair. Component using the same
+sink/source names may use the name prefix property to prepend the
+name of their sinks/sources with the provided string.
+
+Optional name prefix property:
+- sound-name-prefix : string using as prefix for the sink/source names of
+ the component.
+
+Example: Two instances of the same component.
+
+amp0: analog-amplifier@0 {
+   compatible = "simple-audio-amplifier";
+   enable-gpios = < GPIOH_3 0>;
+   sound-name-prefix = "FRONT";
+};
+
+amp1: analog-amplifier@1 {
+   compatible = "simple-audio-amplifier";
+   enable-gpios = < GPIOH_4 0>;
+   sound-name-prefix = "BACK";
+};
-- 
2.18.0

Applied "ASoC: add DT documentation for the sound-name-prefix property" to the asoc tree

[Mark Brown]

The patch

   ASoC: add DT documentation for the sound-name-prefix property

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d5418ae3f9443f911d4324c0cade988ced39cfbe Mon Sep 17 00:00:00 2001
From: Jerome Brunet 
Date: Fri, 13 Jul 2018 14:50:42 +0200
Subject: [PATCH] ASoC: add DT documentation for the sound-name-prefix property

Signed-off-by: Jerome Brunet 
Signed-off-by: Mark Brown 
---
 .../devicetree/bindings/sound/name-prefix.txt | 24 +++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/name-prefix.txt

diff --git a/Documentation/devicetree/bindings/sound/name-prefix.txt 
b/Documentation/devicetree/bindings/sound/name-prefix.txt
new file mode 100644
index ..645775908657
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/name-prefix.txt
@@ -0,0 +1,24 @@
+Name prefix:
+
+Card implementing the routing property define the connection between
+audio components as list of string pair. Component using the same
+sink/source names may use the name prefix property to prepend the
+name of their sinks/sources with the provided string.
+
+Optional name prefix property:
+- sound-name-prefix : string using as prefix for the sink/source names of
+ the component.
+
+Example: Two instances of the same component.
+
+amp0: analog-amplifier@0 {
+   compatible = "simple-audio-amplifier";
+   enable-gpios = < GPIOH_3 0>;
+   sound-name-prefix = "FRONT";
+};
+
+amp1: analog-amplifier@1 {
+   compatible = "simple-audio-amplifier";
+   enable-gpios = < GPIOH_4 0>;
+   sound-name-prefix = "BACK";
+};
-- 
2.18.0

Re: [PATCH] regulator: core: fix _regulator_do_disable return value

[Mark Brown]

On Fri, Jul 13, 2018 at 05:48:54PM +0200, Marco Felsch wrote:
> On 18-07-13 14:07, Mark Brown wrote:

> > This is fine - consumers shouldn't expect that a disable will cause
> > anything to actually get powered off, constraints or other consumers
> > might mean that the disable doesn't actually happen.  It's just the same
> > as a consumer with an always on flag.

> Okay, I understand that the behaviour should be like the always-on
> contrain. But now the behaviour of the core is like my v1 of
> "Re-Enable support to disable switch regulators". It's like a 'simulated
> off', which wasn't a good solution for you. The difference is, that the
> 'simulated off' is now made in the core.

Right, there's a difference between what the core (which does actually
explicitly turn things on and off) sees and what the consumers (which
only increment and decrement reference counts which may happen to result
in something being turned off immediately but also might not) see.


signature.asc
Description: PGP signature

Re: [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding

[Mark Brown]

On Fri, Jul 13, 2018 at 02:50:01PM +0200, Marco Felsch wrote:

> +Optional properties:
> +- fsl,pfuze-support-disable: Boolean, if present disable all unused switch
> +  regulators to save power consumption. Attention, till 4.18 these regulators

The property name sounds like it affects all the regulators but really
it's just the sw ones - how about adding -sw at the end?  Bit of a
bikeshed but it does end up in an ABI.


signature.asc
Description: PGP signature

Re: [PATCH] regulator: core: fix _regulator_do_disable return value

[Mark Brown]

On Fri, Jul 13, 2018 at 05:48:54PM +0200, Marco Felsch wrote:
> On 18-07-13 14:07, Mark Brown wrote:

> > This is fine - consumers shouldn't expect that a disable will cause
> > anything to actually get powered off, constraints or other consumers
> > might mean that the disable doesn't actually happen.  It's just the same
> > as a consumer with an always on flag.

> Okay, I understand that the behaviour should be like the always-on
> contrain. But now the behaviour of the core is like my v1 of
> "Re-Enable support to disable switch regulators". It's like a 'simulated
> off', which wasn't a good solution for you. The difference is, that the
> 'simulated off' is now made in the core.

Right, there's a difference between what the core (which does actually
explicitly turn things on and off) sees and what the consumers (which
only increment and decrement reference counts which may happen to result
in something being turned off immediately but also might not) see.


signature.asc
Description: PGP signature

Re: [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding

[Mark Brown]

On Fri, Jul 13, 2018 at 02:50:01PM +0200, Marco Felsch wrote:

> +Optional properties:
> +- fsl,pfuze-support-disable: Boolean, if present disable all unused switch
> +  regulators to save power consumption. Attention, till 4.18 these regulators

The property name sounds like it affects all the regulators but really
it's just the sw ones - how about adding -sw at the end?  Bit of a
bikeshed but it does end up in an ABI.


signature.asc
Description: PGP signature

[PATCH] ACPI: battery: remove redundant old_present check on insertion

[Lucas Magasweran]

From: Lucas Rangit Magasweran 

On removal battery_present changes from 1 to 0 after calling
acpi_battery_get_statu() and battery->update_time is set to 0
before returning.

On insertion battery_present changes from 0 to 1 after calling
acpi_battery_get_status() and acpi_battery_get_info() is called
because battery->update_time is 0.

The old_present condition is therefore redundant.

This was added in the commit below when there was a path without
sysfs that would skip getting the newly inserted battery info.

commit 50b178512b7d ("Newly inserted battery might differ from one
just removed, so update of battery info fields is required.")

Signed-off-by: Lucas Rangit Magasweran 
---
 drivers/acpi/battery.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index f88bc9f..c280e8e 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -918,10 +918,11 @@ static void acpi_battery_quirks(struct acpi_battery 
*battery)
 
 static int acpi_battery_update(struct acpi_battery *battery, bool resume)
 {
-   int result, old_present = acpi_battery_present(battery);
-   result = acpi_battery_get_status(battery);
+   int result = acpi_battery_get_status(battery);
+
if (result)
return result;
+
if (!acpi_battery_present(battery)) {
sysfs_remove_battery(battery);
battery->update_time = 0;
@@ -931,8 +932,7 @@ static int acpi_battery_update(struct acpi_battery 
*battery, bool resume)
if (resume)
return 0;
 
-   if (!battery->update_time ||
-   old_present != acpi_battery_present(battery)) {
+   if (!battery->update_time) {
result = acpi_battery_get_info(battery);
if (result)
return result;
-- 
2.7.4

[PATCH] ACPI: battery: remove redundant old_present check on insertion

[Lucas Magasweran]

From: Lucas Rangit Magasweran 

On removal battery_present changes from 1 to 0 after calling
acpi_battery_get_statu() and battery->update_time is set to 0
before returning.

On insertion battery_present changes from 0 to 1 after calling
acpi_battery_get_status() and acpi_battery_get_info() is called
because battery->update_time is 0.

The old_present condition is therefore redundant.

This was added in the commit below when there was a path without
sysfs that would skip getting the newly inserted battery info.

commit 50b178512b7d ("Newly inserted battery might differ from one
just removed, so update of battery info fields is required.")

Signed-off-by: Lucas Rangit Magasweran 
---
 drivers/acpi/battery.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index f88bc9f..c280e8e 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -918,10 +918,11 @@ static void acpi_battery_quirks(struct acpi_battery 
*battery)
 
 static int acpi_battery_update(struct acpi_battery *battery, bool resume)
 {
-   int result, old_present = acpi_battery_present(battery);
-   result = acpi_battery_get_status(battery);
+   int result = acpi_battery_get_status(battery);
+
if (result)
return result;
+
if (!acpi_battery_present(battery)) {
sysfs_remove_battery(battery);
battery->update_time = 0;
@@ -931,8 +932,7 @@ static int acpi_battery_update(struct acpi_battery 
*battery, bool resume)
if (resume)
return 0;
 
-   if (!battery->update_time ||
-   old_present != acpi_battery_present(battery)) {
+   if (!battery->update_time) {
result = acpi_battery_get_info(battery);
if (result)
return result;
-- 
2.7.4

Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

[Pavel Machek]

On Sun 2018-07-15 00:29:25, Pavel Machek wrote:
> On Sun 2018-07-15 00:02:57, Jacek Anaszewski wrote:
> > Hi Pavel,
> > 
> > On 07/14/2018 11:20 PM, Pavel Machek wrote:
> > >Hi!
> > >
> > >>>It also drew my attention to the issue of desired pattern sysfs
> > >>>interface semantics on uninitialized pattern. In your implementation
> > >>>user seems to be unable to determine if the pattern is activated
> > >>>or not. We should define the semantics for this use case and
> > >>>describe it in the documentation. Possibly pattern could
> > >>>return alone new line character then.
> > >
> > >Let me take a step back: we have triggers.. like LED blinking.
> > >
> > >How is that going to interact with patterns? We probably want the
> > >patterns to be ignored in that case...?
> > >
> > >Which suggest to me that we should treat patterns as a trigger. I
> > >believe we do something similar with blinking already.
> > >
> > >Then it is easy to determine if pattern is active, and pattern
> > >vs. trigger issue is solved automatically.
> > 
> > I'm all for it. I proposed this approach during the previous
> > discussions related to possible pattern interface implementations,
> > but you seemed not to be so enthusiastic in [0].
> > 
> > [0] https://lkml.org/lkml/2017/4/7/350
> 
> Hmm. Reading my own email now, I can't decipher it.
> 
> I believe I meant "changing patterns from kernel in response to events
> is probably overkill"... or something like that.

Anyway -- to clean up the confusion -- I'd like to see

echo pattern > trigger
echo "1 2 3 4 5 6 7 8" > somewhere

to activate pattern, and

echo none > trigger

to stop it.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH] ACPI: battery: use cache_time as cache "enabled"

[Lucas Magasweran]

From: Lucas Rangit Magasweran 

When battery state is not cached the module parameter cache_time is 0
and battery->update_time starts at 0. However, it set to jiffies in
each call to acpi_battery_get_state() and should not be used to
determine if a cache time is used.

Using battery->update_time causes the evaluation of time_before()
even though cache_time is 0.

This is a minor issue as the behavior is still as expected.
Even if kernel HZ was very slow and jiffies remained equal, the
expected branch (false) would be taken because time_before() is
used instead of time_before_eq().

Signed-off-by: Lucas Rangit Magasweran 
---
 drivers/acpi/battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index d79ad84..f88bc9f 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -549,7 +549,7 @@ static int acpi_battery_get_state(struct acpi_battery 
*battery)
if (!acpi_battery_present(battery))
return 0;
 
-   if (battery->update_time &&
+   if (cache_time &&
time_before(jiffies, battery->update_time +
msecs_to_jiffies(cache_time)))
return 0;
-- 
2.7.4

Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

[Pavel Machek]

On Sun 2018-07-15 00:29:25, Pavel Machek wrote:
> On Sun 2018-07-15 00:02:57, Jacek Anaszewski wrote:
> > Hi Pavel,
> > 
> > On 07/14/2018 11:20 PM, Pavel Machek wrote:
> > >Hi!
> > >
> > >>>It also drew my attention to the issue of desired pattern sysfs
> > >>>interface semantics on uninitialized pattern. In your implementation
> > >>>user seems to be unable to determine if the pattern is activated
> > >>>or not. We should define the semantics for this use case and
> > >>>describe it in the documentation. Possibly pattern could
> > >>>return alone new line character then.
> > >
> > >Let me take a step back: we have triggers.. like LED blinking.
> > >
> > >How is that going to interact with patterns? We probably want the
> > >patterns to be ignored in that case...?
> > >
> > >Which suggest to me that we should treat patterns as a trigger. I
> > >believe we do something similar with blinking already.
> > >
> > >Then it is easy to determine if pattern is active, and pattern
> > >vs. trigger issue is solved automatically.
> > 
> > I'm all for it. I proposed this approach during the previous
> > discussions related to possible pattern interface implementations,
> > but you seemed not to be so enthusiastic in [0].
> > 
> > [0] https://lkml.org/lkml/2017/4/7/350
> 
> Hmm. Reading my own email now, I can't decipher it.
> 
> I believe I meant "changing patterns from kernel in response to events
> is probably overkill"... or something like that.

Anyway -- to clean up the confusion -- I'd like to see

echo pattern > trigger
echo "1 2 3 4 5 6 7 8" > somewhere

to activate pattern, and

echo none > trigger

to stop it.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH] ACPI: battery: use cache_time as cache "enabled"

[Lucas Magasweran]

From: Lucas Rangit Magasweran 

When battery state is not cached the module parameter cache_time is 0
and battery->update_time starts at 0. However, it set to jiffies in
each call to acpi_battery_get_state() and should not be used to
determine if a cache time is used.

Using battery->update_time causes the evaluation of time_before()
even though cache_time is 0.

This is a minor issue as the behavior is still as expected.
Even if kernel HZ was very slow and jiffies remained equal, the
expected branch (false) would be taken because time_before() is
used instead of time_before_eq().

Signed-off-by: Lucas Rangit Magasweran 
---
 drivers/acpi/battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index d79ad84..f88bc9f 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -549,7 +549,7 @@ static int acpi_battery_get_state(struct acpi_battery 
*battery)
if (!acpi_battery_present(battery))
return 0;
 
-   if (battery->update_time &&
+   if (cache_time &&
time_before(jiffies, battery->update_time +
msecs_to_jiffies(cache_time)))
return 0;
-- 
2.7.4

[PATCH 2/2] staging: olpc_dcon: add missing identifier names

[Cristian Kubis]

Add missing function argument identifier names as suggested by
checkpatch.pl.

Signed-off-by: Cristian Kubis 
---
 drivers/staging/olpc_dcon/olpc_dcon.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.h 
b/drivers/staging/olpc_dcon/olpc_dcon.h
index fa89bb97c7b0..c987aaf894e7 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.h
+++ b/drivers/staging/olpc_dcon/olpc_dcon.h
@@ -91,10 +91,10 @@ struct dcon_priv {
 };
 
 struct dcon_platform_data {
-   int (*init)(struct dcon_priv *);
+   int (*init)(struct dcon_priv *dcon);
void (*bus_stabilize_wiggle)(void);
-   void (*set_dconload)(int);
-   int (*read_status)(u8 *);
+   void (*set_dconload)(int load);
+   int (*read_status)(u8 *status);
 };
 
 #include 
-- 
2.18.0

[PATCH 1/2] staging: olpc_dcon: prefer 'help' in KConfig

[Cristian Kubis]

Fix for a style warning reported by checkpatch.pl in KConfig
suggesting to use 'help' instead of '---help---'.

Signed-off-by: Cristian Kubis 
---
 drivers/staging/olpc_dcon/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/olpc_dcon/Kconfig 
b/drivers/staging/olpc_dcon/Kconfig
index d277f048789e..c91a56f77bcb 100644
--- a/drivers/staging/olpc_dcon/Kconfig
+++ b/drivers/staging/olpc_dcon/Kconfig
@@ -4,7 +4,7 @@ config FB_OLPC_DCON
depends on I2C
depends on (GPIO_CS5535 || GPIO_CS5535=n)
select BACKLIGHT_CLASS_DEVICE
-   ---help---
+   help
  In order to support very low power operation, the XO laptop uses a
  secondary Display CONtroller, or DCON.  This secondary controller
  is present in the video pipeline between the primary display
@@ -18,7 +18,7 @@ config FB_OLPC_DCON_1
bool "OLPC XO-1 DCON support"
depends on FB_OLPC_DCON && GPIO_CS5535
default y
-   ---help---
+   help
  Enable support for the DCON in XO-1 model laptops.  The kernel
  communicates with the DCON using model-specific code.  If you
  have an XO-1 (or if you're unsure what model you have), you should
@@ -28,7 +28,7 @@ config FB_OLPC_DCON_1_5
bool "OLPC XO-1.5 DCON support"
depends on FB_OLPC_DCON && ACPI
default y
-   ---help---
+   help
  Enable support for the DCON in XO-1.5 model laptops.  The kernel
  communicates with the DCON using model-specific code.  If you
  have an XO-1.5 (or if you're unsure what model you have), you
-- 
2.18.0

[PATCH 0/2] staging: olpc_dcon: fix style warnings

[Cristian Kubis]

Series of patches fixing two different kinds of style warnings in olpc_dcon
as reported by checkpatch.pl.

Cristian Kubis (2):
  staging: olpc_dcon: prefer 'help' in KConfig
  staging: olpc_dcon: add missing identifier names

 drivers/staging/olpc_dcon/Kconfig | 6 +++---
 drivers/staging/olpc_dcon/olpc_dcon.h | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.18.0

[PATCH 2/2] staging: olpc_dcon: add missing identifier names

[Cristian Kubis]

Add missing function argument identifier names as suggested by
checkpatch.pl.

Signed-off-by: Cristian Kubis 
---
 drivers/staging/olpc_dcon/olpc_dcon.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.h 
b/drivers/staging/olpc_dcon/olpc_dcon.h
index fa89bb97c7b0..c987aaf894e7 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.h
+++ b/drivers/staging/olpc_dcon/olpc_dcon.h
@@ -91,10 +91,10 @@ struct dcon_priv {
 };
 
 struct dcon_platform_data {
-   int (*init)(struct dcon_priv *);
+   int (*init)(struct dcon_priv *dcon);
void (*bus_stabilize_wiggle)(void);
-   void (*set_dconload)(int);
-   int (*read_status)(u8 *);
+   void (*set_dconload)(int load);
+   int (*read_status)(u8 *status);
 };
 
 #include 
-- 
2.18.0

[PATCH 1/2] staging: olpc_dcon: prefer 'help' in KConfig

[Cristian Kubis]

Fix for a style warning reported by checkpatch.pl in KConfig
suggesting to use 'help' instead of '---help---'.

Signed-off-by: Cristian Kubis 
---
 drivers/staging/olpc_dcon/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/olpc_dcon/Kconfig 
b/drivers/staging/olpc_dcon/Kconfig
index d277f048789e..c91a56f77bcb 100644
--- a/drivers/staging/olpc_dcon/Kconfig
+++ b/drivers/staging/olpc_dcon/Kconfig
@@ -4,7 +4,7 @@ config FB_OLPC_DCON
depends on I2C
depends on (GPIO_CS5535 || GPIO_CS5535=n)
select BACKLIGHT_CLASS_DEVICE
-   ---help---
+   help
  In order to support very low power operation, the XO laptop uses a
  secondary Display CONtroller, or DCON.  This secondary controller
  is present in the video pipeline between the primary display
@@ -18,7 +18,7 @@ config FB_OLPC_DCON_1
bool "OLPC XO-1 DCON support"
depends on FB_OLPC_DCON && GPIO_CS5535
default y
-   ---help---
+   help
  Enable support for the DCON in XO-1 model laptops.  The kernel
  communicates with the DCON using model-specific code.  If you
  have an XO-1 (or if you're unsure what model you have), you should
@@ -28,7 +28,7 @@ config FB_OLPC_DCON_1_5
bool "OLPC XO-1.5 DCON support"
depends on FB_OLPC_DCON && ACPI
default y
-   ---help---
+   help
  Enable support for the DCON in XO-1.5 model laptops.  The kernel
  communicates with the DCON using model-specific code.  If you
  have an XO-1.5 (or if you're unsure what model you have), you
-- 
2.18.0

[PATCH 0/2] staging: olpc_dcon: fix style warnings

[Cristian Kubis]

Series of patches fixing two different kinds of style warnings in olpc_dcon
as reported by checkpatch.pl.

Cristian Kubis (2):
  staging: olpc_dcon: prefer 'help' in KConfig
  staging: olpc_dcon: add missing identifier names

 drivers/staging/olpc_dcon/Kconfig | 6 +++---
 drivers/staging/olpc_dcon/olpc_dcon.h | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.18.0

Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

[Pavel Machek]

On Sun 2018-07-15 00:02:57, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 07/14/2018 11:20 PM, Pavel Machek wrote:
> >Hi!
> >
> >>>It also drew my attention to the issue of desired pattern sysfs
> >>>interface semantics on uninitialized pattern. In your implementation
> >>>user seems to be unable to determine if the pattern is activated
> >>>or not. We should define the semantics for this use case and
> >>>describe it in the documentation. Possibly pattern could
> >>>return alone new line character then.
> >
> >Let me take a step back: we have triggers.. like LED blinking.
> >
> >How is that going to interact with patterns? We probably want the
> >patterns to be ignored in that case...?
> >
> >Which suggest to me that we should treat patterns as a trigger. I
> >believe we do something similar with blinking already.
> >
> >Then it is easy to determine if pattern is active, and pattern
> >vs. trigger issue is solved automatically.
> 
> I'm all for it. I proposed this approach during the previous
> discussions related to possible pattern interface implementations,
> but you seemed not to be so enthusiastic in [0].
> 
> [0] https://lkml.org/lkml/2017/4/7/350

Hmm. Reading my own email now, I can't decipher it.

I believe I meant "changing patterns from kernel in response to events
is probably overkill"... or something like that.

Sorry about confusion,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

[Pavel Machek]

On Sun 2018-07-15 00:02:57, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 07/14/2018 11:20 PM, Pavel Machek wrote:
> >Hi!
> >
> >>>It also drew my attention to the issue of desired pattern sysfs
> >>>interface semantics on uninitialized pattern. In your implementation
> >>>user seems to be unable to determine if the pattern is activated
> >>>or not. We should define the semantics for this use case and
> >>>describe it in the documentation. Possibly pattern could
> >>>return alone new line character then.
> >
> >Let me take a step back: we have triggers.. like LED blinking.
> >
> >How is that going to interact with patterns? We probably want the
> >patterns to be ignored in that case...?
> >
> >Which suggest to me that we should treat patterns as a trigger. I
> >believe we do something similar with blinking already.
> >
> >Then it is easy to determine if pattern is active, and pattern
> >vs. trigger issue is solved automatically.
> 
> I'm all for it. I proposed this approach during the previous
> discussions related to possible pattern interface implementations,
> but you seemed not to be so enthusiastic in [0].
> 
> [0] https://lkml.org/lkml/2017/4/7/350

Hmm. Reading my own email now, I can't decipher it.

I believe I meant "changing patterns from kernel in response to events
is probably overkill"... or something like that.

Sorry about confusion,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v9 1/5] clocksource/drivers/timer-mediatek: Add system timer bindings

[Daniel Lezcano]

On 06/07/2018 01:11, Stanley Chu wrote:
> This patch adds bindings of new "System Timer" on Mediatek SoCs.
> 
> Remove RTC clock in the same time because it is not used by
> both "General Purpose Timer" and "System Timer" now.
> 
> Signed-off-by: Stanley Chu 
> ---

I have applied the series for 4.19.

Thanks!

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v9 1/5] clocksource/drivers/timer-mediatek: Add system timer bindings

[Daniel Lezcano]

On 06/07/2018 01:11, Stanley Chu wrote:
> This patch adds bindings of new "System Timer" on Mediatek SoCs.
> 
> Remove RTC clock in the same time because it is not used by
> both "General Purpose Timer" and "System Timer" now.
> 
> Signed-off-by: Stanley Chu 
> ---

I have applied the series for 4.19.

Thanks!

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [GIT PULL 3/3] ARM: defconfig: Pull for v4.19

[Olof Johansson]

On Thu, Jul 12, 2018 at 06:46:39PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> Rebased on v4.18-rc2.
> 
> Best regards,
> Krzysztof
> 
> 
> The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
> 
>   Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git 
> tags/samsung-defconfig-4.19
> 
> for you to fetch changes up to 24ede29d026a9f00aa3f4e7a5531803af4810734:
> 
>   ARM: s5pv210_defconfig: Enable options needed to boot typical Linux distro 
> (2018-07-07 10:49:16 +0200)
> 
> 
> Samsung defconfig changes for v4.19
> 
> Enable options needed for booting full system on S5Pv210-based Samsung
> Galaxy S mobile phones.

Thanks!


-Olof

Re: [GIT PULL 3/3] ARM: defconfig: Pull for v4.19

[Olof Johansson]

On Thu, Jul 12, 2018 at 06:46:39PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> Rebased on v4.18-rc2.
> 
> Best regards,
> Krzysztof
> 
> 
> The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
> 
>   Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git 
> tags/samsung-defconfig-4.19
> 
> for you to fetch changes up to 24ede29d026a9f00aa3f4e7a5531803af4810734:
> 
>   ARM: s5pv210_defconfig: Enable options needed to boot typical Linux distro 
> (2018-07-07 10:49:16 +0200)
> 
> 
> Samsung defconfig changes for v4.19
> 
> Enable options needed for booting full system on S5Pv210-based Samsung
> Galaxy S mobile phones.

Thanks!


-Olof

Re: [GIT PULL 1/3] ARM: dts: exynos: Pull for v4.19

[Olof Johansson]

On Thu, Jul 12, 2018 at 06:46:40PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> Rebased on v4.18-rc2.
> 
> Best regards,
> Krzysztof
> 
> 
> The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
> 
>   Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git 
> tags/samsung-dt-4.19
> 
> for you to fetch changes up to 57f4e8bc1c3ebbd06a278107edeb6af95b53e5bf:
> 
>   dt-bindings: samsung: Document bindings for SGH-T959P board (2018-07-07 
> 12:28:07 +0200)

Merged, thanks!


-Olof

Re: [GIT PULL 2/3] arm64: dts: exynos: Pull for v4.19

[Olof Johansson]

On Thu, Jul 12, 2018 at 06:46:41PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> Rebased on v4.18-rc2.
> 
> Best regards,
> Krzysztof
> 
> 
> The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
> 
>   Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git 
> tags/samsung-dt64-4.19
> 
> for you to fetch changes up to 17aa1530f1ff89ca4e21765e5f4d9e6bcd464eb2:
> 
>   arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433 
> (2018-06-25 18:38:49 +0200)


Merged, thanks!


-Olof

Re: [GIT PULL 1/3] ARM: dts: exynos: Pull for v4.19

[Olof Johansson]

On Thu, Jul 12, 2018 at 06:46:40PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> Rebased on v4.18-rc2.
> 
> Best regards,
> Krzysztof
> 
> 
> The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
> 
>   Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git 
> tags/samsung-dt-4.19
> 
> for you to fetch changes up to 57f4e8bc1c3ebbd06a278107edeb6af95b53e5bf:
> 
>   dt-bindings: samsung: Document bindings for SGH-T959P board (2018-07-07 
> 12:28:07 +0200)

Merged, thanks!


-Olof

Re: [GIT PULL 2/3] arm64: dts: exynos: Pull for v4.19

[Olof Johansson]

On Thu, Jul 12, 2018 at 06:46:41PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> Rebased on v4.18-rc2.
> 
> Best regards,
> Krzysztof
> 
> 
> The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
> 
>   Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git 
> tags/samsung-dt64-4.19
> 
> for you to fetch changes up to 17aa1530f1ff89ca4e21765e5f4d9e6bcd464eb2:
> 
>   arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433 
> (2018-06-25 18:38:49 +0200)


Merged, thanks!


-Olof

Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

[Jacek Anaszewski]

Hi Pavel,

On 07/14/2018 11:20 PM, Pavel Machek wrote:

Hi!


It also drew my attention to the issue of desired pattern sysfs
interface semantics on uninitialized pattern. In your implementation
user seems to be unable to determine if the pattern is activated
or not. We should define the semantics for this use case and
describe it in the documentation. Possibly pattern could
return alone new line character then.


Let me take a step back: we have triggers.. like LED blinking.

How is that going to interact with patterns? We probably want the
patterns to be ignored in that case...?

Which suggest to me that we should treat patterns as a trigger. I
believe we do something similar with blinking already.

Then it is easy to determine if pattern is active, and pattern
vs. trigger issue is solved automatically.


I'm all for it. I proposed this approach during the previous
discussions related to possible pattern interface implementations,
but you seemed not to be so enthusiastic in [0].

[0] https://lkml.org/lkml/2017/4/7/350

--
Best regards,
Jacek Anaszewski

Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

[Jacek Anaszewski]

Hi Pavel,

On 07/14/2018 11:20 PM, Pavel Machek wrote:

Hi!


It also drew my attention to the issue of desired pattern sysfs
interface semantics on uninitialized pattern. In your implementation
user seems to be unable to determine if the pattern is activated
or not. We should define the semantics for this use case and
describe it in the documentation. Possibly pattern could
return alone new line character then.


Let me take a step back: we have triggers.. like LED blinking.

How is that going to interact with patterns? We probably want the
patterns to be ignored in that case...?

Which suggest to me that we should treat patterns as a trigger. I
believe we do something similar with blinking already.

Then it is easy to determine if pattern is active, and pattern
vs. trigger issue is solved automatically.


I'm all for it. I proposed this approach during the previous
discussions related to possible pattern interface implementations,
but you seemed not to be so enthusiastic in [0].

[0] https://lkml.org/lkml/2017/4/7/350

--
Best regards,
Jacek Anaszewski

Re: [PATCH] vme: ca91cx42: remove redundant variable i

[Martyn Welch]

On Sat, Jul 14, 2018 at 05:33:32PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Variable i is being assigned but is never used hence it is redundant
> and can be removed.
> 
> Cleans up clang warning:
> warning: variable 'i' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King 

Reviewed-by: Martyn Welch 

> ---
>  drivers/vme/bridges/vme_ca91cx42.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/vme/bridges/vme_ca91cx42.c 
> b/drivers/vme/bridges/vme_ca91cx42.c
> index 5dd284008630..53bdc256805f 100644
> --- a/drivers/vme/bridges/vme_ca91cx42.c
> +++ b/drivers/vme/bridges/vme_ca91cx42.c
> @@ -970,7 +970,6 @@ static unsigned int ca91cx42_master_rmw(struct 
> vme_master_resource *image,
>  {
>   u32 result;
>   uintptr_t pci_addr;
> - int i;
>   struct ca91cx42_driver *bridge;
>   struct device *dev;
>  
> @@ -978,7 +977,6 @@ static unsigned int ca91cx42_master_rmw(struct 
> vme_master_resource *image,
>   dev = image->parent->parent;
>  
>   /* Find the PCI address that maps to the desired VME address */
> - i = image->number;
>  
>   /* Locking as we can only do one of these at a time */
>   mutex_lock(>vme_rmw);
> -- 
> 2.17.1
> 

Re: [PATCH] vme: ca91cx42: remove redundant variable i

[Martyn Welch]

On Sat, Jul 14, 2018 at 05:33:32PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Variable i is being assigned but is never used hence it is redundant
> and can be removed.
> 
> Cleans up clang warning:
> warning: variable 'i' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King 

Reviewed-by: Martyn Welch 

> ---
>  drivers/vme/bridges/vme_ca91cx42.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/vme/bridges/vme_ca91cx42.c 
> b/drivers/vme/bridges/vme_ca91cx42.c
> index 5dd284008630..53bdc256805f 100644
> --- a/drivers/vme/bridges/vme_ca91cx42.c
> +++ b/drivers/vme/bridges/vme_ca91cx42.c
> @@ -970,7 +970,6 @@ static unsigned int ca91cx42_master_rmw(struct 
> vme_master_resource *image,
>  {
>   u32 result;
>   uintptr_t pci_addr;
> - int i;
>   struct ca91cx42_driver *bridge;
>   struct device *dev;
>  
> @@ -978,7 +977,6 @@ static unsigned int ca91cx42_master_rmw(struct 
> vme_master_resource *image,
>   dev = image->parent->parent;
>  
>   /* Find the PCI address that maps to the desired VME address */
> - i = image->number;
>  
>   /* Locking as we can only do one of these at a time */
>   mutex_lock(>vme_rmw);
> -- 
> 2.17.1
> 

Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

[Pavel Machek]

Hi!

> > It also drew my attention to the issue of desired pattern sysfs
> > interface semantics on uninitialized pattern. In your implementation
> > user seems to be unable to determine if the pattern is activated
> > or not. We should define the semantics for this use case and
> > describe it in the documentation. Possibly pattern could
> > return alone new line character then.

Let me take a step back: we have triggers.. like LED blinking.

How is that going to interact with patterns? We probably want the
patterns to be ignored in that case...?

Which suggest to me that we should treat patterns as a trigger. I
believe we do something similar with blinking already.

Then it is easy to determine if pattern is active, and pattern
vs. trigger issue is solved automatically.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

[Pavel Machek]

Hi!

> > It also drew my attention to the issue of desired pattern sysfs
> > interface semantics on uninitialized pattern. In your implementation
> > user seems to be unable to determine if the pattern is activated
> > or not. We should define the semantics for this use case and
> > describe it in the documentation. Possibly pattern could
> > return alone new line character then.

Let me take a step back: we have triggers.. like LED blinking.

How is that going to interact with patterns? We probably want the
patterns to be ignored in that case...?

Which suggest to me that we should treat patterns as a trigger. I
believe we do something similar with blinking already.

Then it is easy to determine if pattern is active, and pattern
vs. trigger issue is solved automatically.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Ksummit-discuss] bug-introducing patches

[Pavel Machek]

Hi!

> >Well, 0day, kernelci etc... is nice... until the change is in the
> >driver. Most of the kernel are drivers, remember?
> >
> >I don't know. I'd say that if patch is important enough for -stable,
> >there should be someone testing it. For core kernel changes, that can
> >be 0day bot, but for drivers...
> >
> 
> For my part I am just glad that we were able to pick up a fix in xhci code
> just last week, tested or not, from -stable, instead of having to track it
> down ourselves. Similar for many other driver patches which _do_ affect us
> (like the flurry of ext4 patches this week). Granted, there are lots of
> patches which we don't use/need, but even there it is surprising how many
> problems are found with existing testing.
> 
> For anyone interested in making sure that obscure (whatever that means)
> drivers are tested for stable releases, but does not want to spend time on it,
> all I can recommend is to implement qemu support for it and let me know,
> and I'll be happy to add a respective test to my test farm.

Umm. Yes, qemu support for every driver would be nice, but will not happen.

> However, ultimately, stable release candidates are public. Everyone is
> invited to test them. Anyone interested in a specific release and
> driver

Yes, they are public. SubmittingPatches says every patch should be
tested, and that's clearly not happening for -stable. And I'd like
those patch marked such.

> >And problem exists on mainline, too.
> >
> >Hmm. Patch for obscure driver. Wow, nice, is WellKnownName actually
> >using that driver? Aha, no, he is not; he is doing global
> >search, and did not test the patch...
> >
> 
> Ah, like me with the strncpy(x, y, strlen(y)) -> memcpy() replacements
> I did a week or so ago ? You are right, I only compile tested those and
> otherwise trusted my ability to understand C code. If that caused any
> problems, please let me know, and hopefully I'll be able to learn something
> from it.

Yes, such stuff. No, I was not talking about you. I did not want to
give concrete example, but...

# > get_monotonic_boottime() is deprecated, so let's convert this to
# > the simpler ktime_get_boot_ns().
# >
# > Signed-off-by: 
# 
# Have you tested it?
#
...
#   > -curr_boot = timespec_to_ns(_time) * cpus;
# 
# Original code is pretty weird (notice the * cpus), so I'm
# double-checking.

Yes, often you can guess that patch was probably not tested, but it
would be nice to have

Tested: compile

annotation to take away the guesswork. It took me a while an some head
scratching in this concrete example, and it is not first time this
happened.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Ksummit-discuss] bug-introducing patches

[Pavel Machek]

Hi!

> >Well, 0day, kernelci etc... is nice... until the change is in the
> >driver. Most of the kernel are drivers, remember?
> >
> >I don't know. I'd say that if patch is important enough for -stable,
> >there should be someone testing it. For core kernel changes, that can
> >be 0day bot, but for drivers...
> >
> 
> For my part I am just glad that we were able to pick up a fix in xhci code
> just last week, tested or not, from -stable, instead of having to track it
> down ourselves. Similar for many other driver patches which _do_ affect us
> (like the flurry of ext4 patches this week). Granted, there are lots of
> patches which we don't use/need, but even there it is surprising how many
> problems are found with existing testing.
> 
> For anyone interested in making sure that obscure (whatever that means)
> drivers are tested for stable releases, but does not want to spend time on it,
> all I can recommend is to implement qemu support for it and let me know,
> and I'll be happy to add a respective test to my test farm.

Umm. Yes, qemu support for every driver would be nice, but will not happen.

> However, ultimately, stable release candidates are public. Everyone is
> invited to test them. Anyone interested in a specific release and
> driver

Yes, they are public. SubmittingPatches says every patch should be
tested, and that's clearly not happening for -stable. And I'd like
those patch marked such.

> >And problem exists on mainline, too.
> >
> >Hmm. Patch for obscure driver. Wow, nice, is WellKnownName actually
> >using that driver? Aha, no, he is not; he is doing global
> >search, and did not test the patch...
> >
> 
> Ah, like me with the strncpy(x, y, strlen(y)) -> memcpy() replacements
> I did a week or so ago ? You are right, I only compile tested those and
> otherwise trusted my ability to understand C code. If that caused any
> problems, please let me know, and hopefully I'll be able to learn something
> from it.

Yes, such stuff. No, I was not talking about you. I did not want to
give concrete example, but...

# > get_monotonic_boottime() is deprecated, so let's convert this to
# > the simpler ktime_get_boot_ns().
# >
# > Signed-off-by: 
# 
# Have you tested it?
#
...
#   > -curr_boot = timespec_to_ns(_time) * cpus;
# 
# Original code is pretty weird (notice the * cpus), so I'm
# double-checking.

Yes, often you can guess that patch was probably not tested, but it
would be nice to have

Tested: compile

annotation to take away the guesswork. It took me a while an some head
scratching in this concrete example, and it is not first time this
happened.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[GIT PULL] RTC fixes for 4.18

[Alexandre Belloni]

Hi Linus,

Here are two fixes for 4.18.

The following changes since commit ce397d215ccd07b8ae3f71db689aedb85d56ab40:

  Linux 4.18-rc1 (2018-06-17 08:04:49 +0900)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git 
tags/rtc-4.18-2

for you to fetch changes up to fd6792bb022e43faa0c4a45b6f25285e21206f9d:

  rtc: fix alarm read and set offset (2018-07-13 10:37:54 +0200)


RTC fixes for 4.18

 - an important core fix for RTCs using the core offsetting only one driver is
affected.
 - a fix for the error path of mrst


Alexandre Belloni (1):
  rtc: fix alarm read and set offset

Dan Carpenter (1):
  rtc: mrst: fix error code in probe()

 drivers/rtc/interface.c | 8 +---
 drivers/rtc/rtc-mrst.c  | 4 +---
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

[GIT PULL] RTC fixes for 4.18

[Alexandre Belloni]

Hi Linus,

Here are two fixes for 4.18.

The following changes since commit ce397d215ccd07b8ae3f71db689aedb85d56ab40:

  Linux 4.18-rc1 (2018-06-17 08:04:49 +0900)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git 
tags/rtc-4.18-2

for you to fetch changes up to fd6792bb022e43faa0c4a45b6f25285e21206f9d:

  rtc: fix alarm read and set offset (2018-07-13 10:37:54 +0200)


RTC fixes for 4.18

 - an important core fix for RTCs using the core offsetting only one driver is
affected.
 - a fix for the error path of mrst


Alexandre Belloni (1):
  rtc: fix alarm read and set offset

Dan Carpenter (1):
  rtc: mrst: fix error code in probe()

 drivers/rtc/interface.c | 8 +---
 drivers/rtc/rtc-mrst.c  | 4 +---
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

Re: [Ksummit-discuss] bug-introducing patches

[Guenter Roeck]

On 07/14/2018 12:47 PM, Pavel Machek wrote:

Hi!


The way I see it, if a commit can get one or two tested-by, it's a good
alternative to a week in -next.


Agreed. Even their own actually. And I'm not kidding. Those who run large
amounts of tests on certain patches could really mention is in tested-by,
as opposed to the most common cases where the code was just regularly
tested.


Actually, it would be cool to get "Tested: no" and "Tested: compile"
tags in the commit mesages. Sometimes it is clear from the context
that patch was not tested (treewide update of time to 64bit), but
sometime it is not.

This is especially problem for -stable, as it seems that lately
patches are backported from new version without any testing.



When I started my own testing some five years ago, most architectures
did not even build in stable releases. At that time, the only tests being
done on stable release candidates were a number of build tests, and most
of the results were ignored.

Today, we have 0day, kernelci, kerneltests, Linaro's LKFT, and more, plus
several merge and boot tests done by individuals. Stable release candidates
are build tested on all supported architectures, with hundreds of

...

Sure, testing is still far from perfect and needs to be improved. However,
requiring that every patch applied to stable releases be tested individually
(where ? on all affected architectures ?) would be the wrong
direction.


Well, 0day, kernelci etc... is nice... until the change is in the
driver. Most of the kernel are drivers, remember?

I don't know. I'd say that if patch is important enough for -stable,
there should be someone testing it. For core kernel changes, that can
be 0day bot, but for drivers...



For my part I am just glad that we were able to pick up a fix in xhci code
just last week, tested or not, from -stable, instead of having to track it
down ourselves. Similar for many other driver patches which _do_ affect us
(like the flurry of ext4 patches this week). Granted, there are lots of
patches which we don't use/need, but even there it is surprising how many
problems are found with existing testing.

For anyone interested in making sure that obscure (whatever that means)
drivers are tested for stable releases, but does not want to spend time on it,
all I can recommend is to implement qemu support for it and let me know,
and I'll be happy to add a respective test to my test farm.

However, ultimately, stable release candidates are public. Everyone is
invited to test them. Anyone interested in a specific release and driver
is invited take stable release candidates and do the necessary testing,
just like I and several others do.


And problem exists on mainline, too.

Hmm. Patch for obscure driver. Wow, nice, is WellKnownName actually
using that driver? Aha, no, he is not; he is doing global
search, and did not test the patch...



Ah, like me with the strncpy(x, y, strlen(y)) -> memcpy() replacements
I did a week or so ago ? You are right, I only compile tested those and
otherwise trusted my ability to understand C code. If that caused any
problems, please let me know, and hopefully I'll be able to learn something
from it.

Really, there are infrastructure changes all the time. Sometimes I am asked
to run a complete test sequence with those, but most of the time they are
applied to -next and people wait for the fallout. That may not be perfect but,
seriously, the only alternative would be to declare that in-kernel APIs
shall not be changed anymore. I don't think that would be feasible.

Thanks,
Guenter

Re: [Ksummit-discuss] bug-introducing patches

[Guenter Roeck]

On 07/14/2018 12:47 PM, Pavel Machek wrote:

Hi!


The way I see it, if a commit can get one or two tested-by, it's a good
alternative to a week in -next.


Agreed. Even their own actually. And I'm not kidding. Those who run large
amounts of tests on certain patches could really mention is in tested-by,
as opposed to the most common cases where the code was just regularly
tested.


Actually, it would be cool to get "Tested: no" and "Tested: compile"
tags in the commit mesages. Sometimes it is clear from the context
that patch was not tested (treewide update of time to 64bit), but
sometime it is not.

This is especially problem for -stable, as it seems that lately
patches are backported from new version without any testing.



When I started my own testing some five years ago, most architectures
did not even build in stable releases. At that time, the only tests being
done on stable release candidates were a number of build tests, and most
of the results were ignored.

Today, we have 0day, kernelci, kerneltests, Linaro's LKFT, and more, plus
several merge and boot tests done by individuals. Stable release candidates
are build tested on all supported architectures, with hundreds of

...

Sure, testing is still far from perfect and needs to be improved. However,
requiring that every patch applied to stable releases be tested individually
(where ? on all affected architectures ?) would be the wrong
direction.


Well, 0day, kernelci etc... is nice... until the change is in the
driver. Most of the kernel are drivers, remember?

I don't know. I'd say that if patch is important enough for -stable,
there should be someone testing it. For core kernel changes, that can
be 0day bot, but for drivers...



For my part I am just glad that we were able to pick up a fix in xhci code
just last week, tested or not, from -stable, instead of having to track it
down ourselves. Similar for many other driver patches which _do_ affect us
(like the flurry of ext4 patches this week). Granted, there are lots of
patches which we don't use/need, but even there it is surprising how many
problems are found with existing testing.

For anyone interested in making sure that obscure (whatever that means)
drivers are tested for stable releases, but does not want to spend time on it,
all I can recommend is to implement qemu support for it and let me know,
and I'll be happy to add a respective test to my test farm.

However, ultimately, stable release candidates are public. Everyone is
invited to test them. Anyone interested in a specific release and driver
is invited take stable release candidates and do the necessary testing,
just like I and several others do.


And problem exists on mainline, too.

Hmm. Patch for obscure driver. Wow, nice, is WellKnownName actually
using that driver? Aha, no, he is not; he is doing global
search, and did not test the patch...



Ah, like me with the strncpy(x, y, strlen(y)) -> memcpy() replacements
I did a week or so ago ? You are right, I only compile tested those and
otherwise trusted my ability to understand C code. If that caused any
problems, please let me know, and hopefully I'll be able to learn something
from it.

Really, there are infrastructure changes all the time. Sometimes I am asked
to run a complete test sequence with those, but most of the time they are
applied to -next and people wait for the fallout. That may not be perfect but,
seriously, the only alternative would be to declare that in-kernel APIs
shall not be changed anymore. I don't think that would be feasible.

Thanks,
Guenter

Re: [PATCH] kconfig: remove EXPERT from CHECKPOINT_RESTORE

[Cyrill Gorcunov]

On Sat, Jul 14, 2018 at 02:39:24PM -0500, Eric W. Biederman wrote:
> Josh Triplett  writes:
> 
> > On Sat, Jul 14, 2018 at 02:04:46PM -0500, Eric W. Biederman wrote:
> >> For a config option that no one has come forward with an actual real
> >> world use case for disabling, that cost seems much too high.
> >
> > The real-world use case is precisely as stated: code size, both storage
> > and RAM.
> 
> That is theoretical.  Which platform will break or feel distressed if we
> make it unconditional.  That is real world.
> 
> > I regularly encounter systems I'd *like* to put Linux in that have
> > around 1MB of storage and 1MB of RAM, or even less.
> 
> Yes.  There is so little code behind CONFIG_CHECKPOINT_RESTART that it
> won't help with that.
> 
> But if minification is the actual requirement for disabling
> CONFIG_CHECKPOINT_RESTART than CONFIG_CHECKPIONT_RESTART is properly
> behind expert and it needs to be default y instead of default n.

I happened to miss this thread, sorry. So as far as I remember it
was me who introduced this option in first place, and initially
I placed it under expert so it won't be enabled by default. Lately
we found that some of functionality introduced for criu sake actually
pretty convenient for other tools (for example vmflags reported in
procfs) so we dropped CONFIG_ option out for such blocks and merged
them into kernel directly. I won't mind if left is merged into the
kernel, there should not be that many places.

Re: [PATCH] kconfig: remove EXPERT from CHECKPOINT_RESTORE

[Cyrill Gorcunov]

On Sat, Jul 14, 2018 at 02:39:24PM -0500, Eric W. Biederman wrote:
> Josh Triplett  writes:
> 
> > On Sat, Jul 14, 2018 at 02:04:46PM -0500, Eric W. Biederman wrote:
> >> For a config option that no one has come forward with an actual real
> >> world use case for disabling, that cost seems much too high.
> >
> > The real-world use case is precisely as stated: code size, both storage
> > and RAM.
> 
> That is theoretical.  Which platform will break or feel distressed if we
> make it unconditional.  That is real world.
> 
> > I regularly encounter systems I'd *like* to put Linux in that have
> > around 1MB of storage and 1MB of RAM, or even less.
> 
> Yes.  There is so little code behind CONFIG_CHECKPOINT_RESTART that it
> won't help with that.
> 
> But if minification is the actual requirement for disabling
> CONFIG_CHECKPOINT_RESTART than CONFIG_CHECKPIONT_RESTART is properly
> behind expert and it needs to be default y instead of default n.

I happened to miss this thread, sorry. So as far as I remember it
was me who introduced this option in first place, and initially
I placed it under expert so it won't be enabled by default. Lately
we found that some of functionality introduced for criu sake actually
pretty convenient for other tools (for example vmflags reported in
procfs) so we dropped CONFIG_ option out for such blocks and merged
them into kernel directly. I won't mind if left is merged into the
kernel, there should not be that many places.

Re: [PATCH 2/2] ARM: dts: pxa: add mioa701 board description

[Robert Jarzmik]

Rob Herring  writes:

Hi Rob,

>> +   /* compatible = "mitac,mioa701"; */
>> +   compatible = "marvell,pxa270";
>
> Why the comment?
Leftover, I'll remove it.

>> +   pstore_region:region@0xa200 {
>
> Drop the 0x
Done.

>> +   compatible = 
>> "linux,contiguous-memory-region";
>> +   reg = <0xa200 1048576>;
>
> Use hex for the size.
Ok.

>> +   ffuart: uart@4010 {
>
> Should be "serial@...". You'll have to fix in the base dtsi.
Ok for all of these.
>> +   pxai2c1: i2c@40301680 {
>> +   mrvl,i2c-fast-mode;
>> +   pinctrl-names = "default";
>> +   pinctrl-0 = <_i2c_default>;
>> +   status = "okay";
>> +
>> +   mt9m111: camera@5d {
>> +   compatible = "micron,mt9m111";
>> +   reg = <0x5d>;
>> +   gpios = < 56 GPIO_ACTIVE_HIGH>;
>> +
>> +   remote = <_camera>;
>
> Not needed with the graph:
I don't understand that, do you mean the port node is not needed ?

>
>> +   port {
>> +   mt9m111_1: endpoint {
>> +   bus-width = <8>;
>> +   remote-endpoint = 
>> <_camera>;
>> +   };
>> +   };
>> +   };
>> +   };
>> +   gpio-keys {
>> +   compatible = "gpio-keys";
>> +   #address-cells = <1>;
>> +   #size-cells = <0>;
>> +   autorepeat;
>> +   pinctrl-names = "default";
>> +   pinctrl-0 = <_gpiokeys_default>;
>> +   status = "okay";
>
> Don't need status here? It shouldn't be defined in the base dtsi.
Heuh what ? The status is just above, and gpio-keys is not defined in any dtsi.

>> +
>> +   button@0 {
>
> If you have unit-address there should also be a reg property. Just
> drop and name the node 'power' or 'power-button'. And similarly for
> the rest.
Ok.

>> +
>> +   regulators {
>> +   compatible = "simple-bus";
>
> Drop this and move children to the top level. This is not an mmio bus.
Ok.

>> +   docg3: flash@0 {
>
> This probably should be a child of the bus controller.
Most certainly yes.

>
>> +   compatible = "m-systems,diskonchip-g3";
>> +   reg = <0x0 0x2000>;
>> +   };
>> +
>> +   panel {
>> +   compatible = "toshiba,ltm0305a776";
>> +   lcd-type = "color-tft";
>
> This should be implied by the compatible.
Mmm no. Actually, this is used by the framebuffer controller, ie. pxafb to set
it up. And I don't think adding a list of panel compatibles in pxafb is a good
idea.
The corresponding code is in : drivers/video/fbdev/pxafb.c, see
"static const char * const lcd_types[]".

>> +   display-timings {
>> +   native-mode = <>;
>
> We generally imply this from the compatible (i.e. use simple-panel).
But you suppose there is an actual driver for the panel, right ? But there is
none.

> Could use another level of indentation.
Sure.

>> +   ac_charger: charger@1 {
>
> Drop the unit-address.
Ok.

Thanks for the review.

-- 
Robert

Re: [PATCH] kconfig: remove EXPERT from CHECKPOINT_RESTORE

[Josh Triplett]

On Sat, Jul 14, 2018 at 02:39:24PM -0500, Eric W. Biederman wrote:
> Josh Triplett  writes:
> 
> > On Sat, Jul 14, 2018 at 02:04:46PM -0500, Eric W. Biederman wrote:
> >> For a config option that no one has come forward with an actual real
> >> world use case for disabling, that cost seems much too high.
> >
> > The real-world use case is precisely as stated: code size, both storage
> > and RAM.
> 
> That is theoretical.

No, it isn't. I've *watched* the kernel's size trend steadily upward
over time. And it largely happens in individual features that don't
think *their* contribution to size is all that large.

> > I regularly encounter systems I'd *like* to put Linux in that have
> > around 1MB of storage and 1MB of RAM, or even less.
> 
> Yes.  There is so little code behind CONFIG_CHECKPOINT_RESTART that it
> won't help with that.

It adds up; there are hundreds more small features like it.

> But if minification is the actual requirement for disabling
> CONFIG_CHECKPOINT_RESTART than CONFIG_CHECKPIONT_RESTART is properly
> behind expert and it needs to be default y instead of default n.

I don't have any objection to *that*, as long as the option remains.

Re: [PATCH 2/2] ARM: dts: pxa: add mioa701 board description

[Robert Jarzmik]

Rob Herring  writes:

Hi Rob,

>> +   /* compatible = "mitac,mioa701"; */
>> +   compatible = "marvell,pxa270";
>
> Why the comment?
Leftover, I'll remove it.

>> +   pstore_region:region@0xa200 {
>
> Drop the 0x
Done.

>> +   compatible = 
>> "linux,contiguous-memory-region";
>> +   reg = <0xa200 1048576>;
>
> Use hex for the size.
Ok.

>> +   ffuart: uart@4010 {
>
> Should be "serial@...". You'll have to fix in the base dtsi.
Ok for all of these.
>> +   pxai2c1: i2c@40301680 {
>> +   mrvl,i2c-fast-mode;
>> +   pinctrl-names = "default";
>> +   pinctrl-0 = <_i2c_default>;
>> +   status = "okay";
>> +
>> +   mt9m111: camera@5d {
>> +   compatible = "micron,mt9m111";
>> +   reg = <0x5d>;
>> +   gpios = < 56 GPIO_ACTIVE_HIGH>;
>> +
>> +   remote = <_camera>;
>
> Not needed with the graph:
I don't understand that, do you mean the port node is not needed ?

>
>> +   port {
>> +   mt9m111_1: endpoint {
>> +   bus-width = <8>;
>> +   remote-endpoint = 
>> <_camera>;
>> +   };
>> +   };
>> +   };
>> +   };
>> +   gpio-keys {
>> +   compatible = "gpio-keys";
>> +   #address-cells = <1>;
>> +   #size-cells = <0>;
>> +   autorepeat;
>> +   pinctrl-names = "default";
>> +   pinctrl-0 = <_gpiokeys_default>;
>> +   status = "okay";
>
> Don't need status here? It shouldn't be defined in the base dtsi.
Heuh what ? The status is just above, and gpio-keys is not defined in any dtsi.

>> +
>> +   button@0 {
>
> If you have unit-address there should also be a reg property. Just
> drop and name the node 'power' or 'power-button'. And similarly for
> the rest.
Ok.

>> +
>> +   regulators {
>> +   compatible = "simple-bus";
>
> Drop this and move children to the top level. This is not an mmio bus.
Ok.

>> +   docg3: flash@0 {
>
> This probably should be a child of the bus controller.
Most certainly yes.

>
>> +   compatible = "m-systems,diskonchip-g3";
>> +   reg = <0x0 0x2000>;
>> +   };
>> +
>> +   panel {
>> +   compatible = "toshiba,ltm0305a776";
>> +   lcd-type = "color-tft";
>
> This should be implied by the compatible.
Mmm no. Actually, this is used by the framebuffer controller, ie. pxafb to set
it up. And I don't think adding a list of panel compatibles in pxafb is a good
idea.
The corresponding code is in : drivers/video/fbdev/pxafb.c, see
"static const char * const lcd_types[]".

>> +   display-timings {
>> +   native-mode = <>;
>
> We generally imply this from the compatible (i.e. use simple-panel).
But you suppose there is an actual driver for the panel, right ? But there is
none.

> Could use another level of indentation.
Sure.

>> +   ac_charger: charger@1 {
>
> Drop the unit-address.
Ok.

Thanks for the review.

-- 
Robert

Re: [PATCH] kconfig: remove EXPERT from CHECKPOINT_RESTORE

[Josh Triplett]

On Sat, Jul 14, 2018 at 02:39:24PM -0500, Eric W. Biederman wrote:
> Josh Triplett  writes:
> 
> > On Sat, Jul 14, 2018 at 02:04:46PM -0500, Eric W. Biederman wrote:
> >> For a config option that no one has come forward with an actual real
> >> world use case for disabling, that cost seems much too high.
> >
> > The real-world use case is precisely as stated: code size, both storage
> > and RAM.
> 
> That is theoretical.

No, it isn't. I've *watched* the kernel's size trend steadily upward
over time. And it largely happens in individual features that don't
think *their* contribution to size is all that large.

> > I regularly encounter systems I'd *like* to put Linux in that have
> > around 1MB of storage and 1MB of RAM, or even less.
> 
> Yes.  There is so little code behind CONFIG_CHECKPOINT_RESTART that it
> won't help with that.

It adds up; there are hundreds more small features like it.

> But if minification is the actual requirement for disabling
> CONFIG_CHECKPOINT_RESTART than CONFIG_CHECKPIONT_RESTART is properly
> behind expert and it needs to be default y instead of default n.

I don't have any objection to *that*, as long as the option remains.

[PATCH] x86/events/intel/ds: Fix bts_interrupt_threshold alignment

[Hugh Dickins]

Markus reported that BTS is sporadically missing the tail of the trace
in the perf_event data buffer: [decode error (1): instruction overflow]
shown in GDB; and bisected it to the conversion of debug_store to PTI.

A little "optimization" crept into alloc_bts_buffer(), which mistakenly
placed bts_interrupt_threshold away from the 24-byte record boundary.
Intel SDM Vol 3B 17.4.9 says "This address must point to an offset from
the BTS buffer base that is a multiple of the BTS record size."

Revert "max" from a byte count to a record count, to calculate the
bts_interrupt_threshold correctly: which turns out to fix problem seen.

Fixes: c1961a4631da ("x86/events/intel/ds: Map debug buffers in cpu_entry_area")
Signed-off-by: Hugh Dickins 
Reported-and-tested-by: Markus T Metzger 
Cc:  # v4.14+
---
Sorry for the spam: I missed out [PATCH] and x...@kernel.org yesterday.

 arch/x86/events/intel/ds.c |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- 4.18-rc4/arch/x86/events/intel/ds.c 2018-06-03 14:15:21.0 -0700
+++ linux/arch/x86/events/intel/ds.c2018-07-12 17:38:28.471378616 -0700
@@ -408,9 +408,11 @@ static int alloc_bts_buffer(int cpu)
ds->bts_buffer_base = (unsigned long) cea;
ds_update_cea(cea, buffer, BTS_BUFFER_SIZE, PAGE_KERNEL);
ds->bts_index = ds->bts_buffer_base;
-   max = BTS_RECORD_SIZE * (BTS_BUFFER_SIZE / BTS_RECORD_SIZE);
-   ds->bts_absolute_maximum = ds->bts_buffer_base + max;
-   ds->bts_interrupt_threshold = ds->bts_absolute_maximum - (max / 16);
+   max = BTS_BUFFER_SIZE / BTS_RECORD_SIZE;
+   ds->bts_absolute_maximum = ds->bts_buffer_base +
+   max * BTS_RECORD_SIZE;
+   ds->bts_interrupt_threshold = ds->bts_absolute_maximum -
+   (max / 16) * BTS_RECORD_SIZE;
return 0;
 }
 

[PATCH] x86/events/intel/ds: Fix bts_interrupt_threshold alignment

[Hugh Dickins]

Markus reported that BTS is sporadically missing the tail of the trace
in the perf_event data buffer: [decode error (1): instruction overflow]
shown in GDB; and bisected it to the conversion of debug_store to PTI.

A little "optimization" crept into alloc_bts_buffer(), which mistakenly
placed bts_interrupt_threshold away from the 24-byte record boundary.
Intel SDM Vol 3B 17.4.9 says "This address must point to an offset from
the BTS buffer base that is a multiple of the BTS record size."

Revert "max" from a byte count to a record count, to calculate the
bts_interrupt_threshold correctly: which turns out to fix problem seen.

Fixes: c1961a4631da ("x86/events/intel/ds: Map debug buffers in cpu_entry_area")
Signed-off-by: Hugh Dickins 
Reported-and-tested-by: Markus T Metzger 
Cc:  # v4.14+
---
Sorry for the spam: I missed out [PATCH] and x...@kernel.org yesterday.

 arch/x86/events/intel/ds.c |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- 4.18-rc4/arch/x86/events/intel/ds.c 2018-06-03 14:15:21.0 -0700
+++ linux/arch/x86/events/intel/ds.c2018-07-12 17:38:28.471378616 -0700
@@ -408,9 +408,11 @@ static int alloc_bts_buffer(int cpu)
ds->bts_buffer_base = (unsigned long) cea;
ds_update_cea(cea, buffer, BTS_BUFFER_SIZE, PAGE_KERNEL);
ds->bts_index = ds->bts_buffer_base;
-   max = BTS_RECORD_SIZE * (BTS_BUFFER_SIZE / BTS_RECORD_SIZE);
-   ds->bts_absolute_maximum = ds->bts_buffer_base + max;
-   ds->bts_interrupt_threshold = ds->bts_absolute_maximum - (max / 16);
+   max = BTS_BUFFER_SIZE / BTS_RECORD_SIZE;
+   ds->bts_absolute_maximum = ds->bts_buffer_base +
+   max * BTS_RECORD_SIZE;
+   ds->bts_interrupt_threshold = ds->bts_absolute_maximum -
+   (max / 16) * BTS_RECORD_SIZE;
return 0;
 }
 

Re: [PATCH 1/2] mux:adgs1408/1409: New driver for Analog Devices ADGS1408/1409 mux …

[Peter Rosin]

On 2018-07-14 16:02, Lars-Peter Clausen wrote:
> On 07/14/2018 02:04 PM, Peter Rosin wrote:
> [...]
>>> +static int adgs140x_spi_reg_write(struct spi_device *spi,
>>> +   u8 reg_addr, u8 reg_data)
>>> +{
>>> +   u8 tx_buf[2];
>>> +
>>> +   tx_buf[0] = reg_addr;
>>> +   tx_buf[1] = reg_data;
>>> +
>>> +   return spi_write_then_read(spi, tx_buf, sizeof(tx_buf), NULL, 0);
>>
>>  return spi_write(spi, tx_buf, sizeof(tx_buf));
> 
> Be aware spi_write_then_read() can handle on stack buffers, spi_write() can't.
> 

How obvious...

But ok, I guess the ugliness is warranted.

Cheers,
Peter

Re: [PATCH 1/2] mux:adgs1408/1409: New driver for Analog Devices ADGS1408/1409 mux …

[Peter Rosin]

On 2018-07-14 16:02, Lars-Peter Clausen wrote:
> On 07/14/2018 02:04 PM, Peter Rosin wrote:
> [...]
>>> +static int adgs140x_spi_reg_write(struct spi_device *spi,
>>> +   u8 reg_addr, u8 reg_data)
>>> +{
>>> +   u8 tx_buf[2];
>>> +
>>> +   tx_buf[0] = reg_addr;
>>> +   tx_buf[1] = reg_data;
>>> +
>>> +   return spi_write_then_read(spi, tx_buf, sizeof(tx_buf), NULL, 0);
>>
>>  return spi_write(spi, tx_buf, sizeof(tx_buf));
> 
> Be aware spi_write_then_read() can handle on stack buffers, spi_write() can't.
> 

How obvious...

But ok, I guess the ugliness is warranted.

Cheers,
Peter

Re: [PATCH] kconfig: remove EXPERT from CHECKPOINT_RESTORE

[Josh Triplett]

On Sat, Jul 14, 2018 at 02:04:46PM -0500, Eric W. Biederman wrote:
> For a config option that no one has come forward with an actual real
> world use case for disabling, that cost seems much too high.

The real-world use case is precisely as stated: code size, both storage
and RAM.

I regularly encounter systems I'd *like* to put Linux in that have
around 1MB of storage and 1MB of RAM, or even less.

Re: [PATCH] kconfig: remove EXPERT from CHECKPOINT_RESTORE

[Josh Triplett]

On Sat, Jul 14, 2018 at 02:04:46PM -0500, Eric W. Biederman wrote:
> For a config option that no one has come forward with an actual real
> world use case for disabling, that cost seems much too high.

The real-world use case is precisely as stated: code size, both storage
and RAM.

I regularly encounter systems I'd *like* to put Linux in that have
around 1MB of storage and 1MB of RAM, or even less.

Re: [Ksummit-discuss] bug-introducing patches

[Pavel Machek]

Hi!

> >>>The way I see it, if a commit can get one or two tested-by, it's a good
> >>>alternative to a week in -next.
> >>
> >>Agreed. Even their own actually. And I'm not kidding. Those who run large
> >>amounts of tests on certain patches could really mention is in tested-by,
> >>as opposed to the most common cases where the code was just regularly
> >>tested.
> >
> >Actually, it would be cool to get "Tested: no" and "Tested: compile"
> >tags in the commit mesages. Sometimes it is clear from the context
> >that patch was not tested (treewide update of time to 64bit), but
> >sometime it is not.
> >
> >This is especially problem for -stable, as it seems that lately
> >patches are backported from new version without any testing.
> 
> 
> When I started my own testing some five years ago, most architectures
> did not even build in stable releases. At that time, the only tests being
> done on stable release candidates were a number of build tests, and most
> of the results were ignored.
> 
> Today, we have 0day, kernelci, kerneltests, Linaro's LKFT, and more, plus
> several merge and boot tests done by individuals. Stable release candidates
> are build tested on all supported architectures, with hundreds of
...
> Sure, testing is still far from perfect and needs to be improved. However,
> requiring that every patch applied to stable releases be tested individually
> (where ? on all affected architectures ?) would be the wrong
>direction.

Well, 0day, kernelci etc... is nice... until the change is in the
driver. Most of the kernel are drivers, remember?

I don't know. I'd say that if patch is important enough for -stable,
there should be someone testing it. For core kernel changes, that can
be 0day bot, but for drivers...

And problem exists on mainline, too.

Hmm. Patch for obscure driver. Wow, nice, is WellKnownName actually
using that driver? Aha, no, he is not; he is doing global
search, and did not test the patch...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Ksummit-discuss] bug-introducing patches

[Pavel Machek]

Hi!

> >>>The way I see it, if a commit can get one or two tested-by, it's a good
> >>>alternative to a week in -next.
> >>
> >>Agreed. Even their own actually. And I'm not kidding. Those who run large
> >>amounts of tests on certain patches could really mention is in tested-by,
> >>as opposed to the most common cases where the code was just regularly
> >>tested.
> >
> >Actually, it would be cool to get "Tested: no" and "Tested: compile"
> >tags in the commit mesages. Sometimes it is clear from the context
> >that patch was not tested (treewide update of time to 64bit), but
> >sometime it is not.
> >
> >This is especially problem for -stable, as it seems that lately
> >patches are backported from new version without any testing.
> 
> 
> When I started my own testing some five years ago, most architectures
> did not even build in stable releases. At that time, the only tests being
> done on stable release candidates were a number of build tests, and most
> of the results were ignored.
> 
> Today, we have 0day, kernelci, kerneltests, Linaro's LKFT, and more, plus
> several merge and boot tests done by individuals. Stable release candidates
> are build tested on all supported architectures, with hundreds of
...
> Sure, testing is still far from perfect and needs to be improved. However,
> requiring that every patch applied to stable releases be tested individually
> (where ? on all affected architectures ?) would be the wrong
>direction.

Well, 0day, kernelci etc... is nice... until the change is in the
driver. Most of the kernel are drivers, remember?

I don't know. I'd say that if patch is important enough for -stable,
there should be someone testing it. For core kernel changes, that can
be 0day bot, but for drivers...

And problem exists on mainline, too.

Hmm. Patch for obscure driver. Wow, nice, is WellKnownName actually
using that driver? Aha, no, he is not; he is doing global
search, and did not test the patch...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH] kconfig: remove EXPERT from CHECKPOINT_RESTORE

[Eric W. Biederman]

Josh Triplett  writes:

> On Sat, Jul 14, 2018 at 02:04:46PM -0500, Eric W. Biederman wrote:
>> For a config option that no one has come forward with an actual real
>> world use case for disabling, that cost seems much too high.
>
> The real-world use case is precisely as stated: code size, both storage
> and RAM.

That is theoretical.  Which platform will break or feel distressed if we
make it unconditional.  That is real world.

> I regularly encounter systems I'd *like* to put Linux in that have
> around 1MB of storage and 1MB of RAM, or even less.

Yes.  There is so little code behind CONFIG_CHECKPOINT_RESTART that it
won't help with that.

But if minification is the actual requirement for disabling
CONFIG_CHECKPOINT_RESTART than CONFIG_CHECKPIONT_RESTART is properly
behind expert and it needs to be default y instead of default n.

Eric

Re: [PATCH] kconfig: remove EXPERT from CHECKPOINT_RESTORE

[Eric W. Biederman]

Josh Triplett  writes:

> On Sat, Jul 14, 2018 at 02:04:46PM -0500, Eric W. Biederman wrote:
>> For a config option that no one has come forward with an actual real
>> world use case for disabling, that cost seems much too high.
>
> The real-world use case is precisely as stated: code size, both storage
> and RAM.

That is theoretical.  Which platform will break or feel distressed if we
make it unconditional.  That is real world.

> I regularly encounter systems I'd *like* to put Linux in that have
> around 1MB of storage and 1MB of RAM, or even less.

Yes.  There is so little code behind CONFIG_CHECKPOINT_RESTART that it
won't help with that.

But if minification is the actual requirement for disabling
CONFIG_CHECKPOINT_RESTART than CONFIG_CHECKPIONT_RESTART is properly
behind expert and it needs to be default y instead of default n.

Eric

Re: [PATCH 12/18] staging: gasket: annotate ioctl arg with __user

[Todd Poynor]

>> I think I'm following 
>> http://www.kroah.com/log/linux/linux-staging-update.html,
>> but if I'm off in the weeds do clue me in, thanks.
>
> That's something I wrote 9 years ago :)

Still the top search result for how to work in staging, suggest an update. :)

Re: [PATCH 12/18] staging: gasket: annotate ioctl arg with __user

[Todd Poynor]

>> I think I'm following 
>> http://www.kroah.com/log/linux/linux-staging-update.html,
>> but if I'm off in the weeds do clue me in, thanks.
>
> That's something I wrote 9 years ago :)

Still the top search result for how to work in staging, suggest an update. :)