Re: Packet gets stuck in NOLOCK pfifo_fast qdisc
Hi Hillf, > Let’s see if TCQ_F_NOLOC is making fq_codel different in your testing. I assume you meant disabling NOLOCK for pfifo_fast. Here is the modification, --- ./net/sched/sch_generic.c.orig 2020-08-24 22:02:04.589830751 +0800 +++ ./net/sched/sch_generic.c 2020-08-27 10:17:10.148977195 +0800 @@ -792,7 +792,7 @@ .dump = pfifo_fast_dump, .change_tx_queue_len = pfifo_fast_change_tx_queue_len, .owner = THIS_MODULE, - .static_flags = TCQ_F_NOLOCK | TCQ_F_CPUSTATS, + .static_flags = TCQ_F_CPUSTATS, The issue never happen again with it for over 3 hours stressing. And I restarted the test for two times. No any surprising. Quite stable... Hillf Danton 于2020年8月26日周三 下午2:37写道: > > Hi Feng, > > > > On Wed, 26 Aug 2020 11:12:38 +0800 Fengkehuan Feng wrote: > > >Hi Hillf, > > > >I just gave more tries on the patch, and it seems not that good as what I > >told in last email. > > >I could see more packets getting stuck now... > > We have more to learn here:P > > > > >Let me explain what I am facing in detail in case we are not aligning to fix > >the same problem. > > > >Our application is in deep learning scenario and it's based on NVIDIA NCCL > >to do > > >collective communication intra-node or inter-node (to be more specific, it's > >data > > > all-reduce on two servers witch 8 GPU nodes each). > >NCCL can support data transmission through TCP/RDMA/GDR. In normal, it takes > > > about 1000 us for TCP or less for RDMA/GDR to transmit 512KB packet, but > > > sometimes it tooks hundreds of millisecond or several seconds to get > > completed. > > > > >When we change the default qdisc from pfifo_fast to fq_codel, the issue never > > > happen, so we suspect it's something wrong within the networking stack (but > > > it's a bit strange that RDMA or GDR has the same problem) > > Let’s see if TCQ_F_NOLOC is making fq_codel different in your testing. > > > > --- a/net/sched/sch_generic.c > > +++ b/net/sched/sch_generic.c > > @@ -791,7 +791,7 @@ struct Qdisc_ops pfifo_fast_ops __read_m > > .dump =pfifo_fast_dump, > > .change_tx_queue_len = pfifo_fast_change_tx_queue_len, > > .owner =THIS_MODULE, > > - .static_flags =TCQ_F_NOLOCK | TCQ_F_CPUSTATS, > > +.static_flags =TCQ_F_CPUSTATS, > > }; > > EXPORT_SYMBOL(pfifo_fast_ops); > > -- > > > > > > >Here is the log print from our test application, > > > >size: 512KB, use_time: 1118us, speed: 0.436745GB/s > >size: 512KB, use_time: 912us, speed: 0.535396GB/s > >size: 512KB, use_time: 1023us, speed: 0.477303GB/s > >size: 512KB, use_time: 919us, speed: 0.531318GB/s > >size: 512KB, use_time: 1129us, speed: 0.432490GB/s > >size: 512KB, use_time: 2098748us, speed: 0.000233GB/s > >size: 512KB, use_time: 1018us, speed: 0.479648GB/s > >size: 512KB, use_time: 1120us, speed: 0.435965GB/s > >size: 512KB, use_time: 1071us, speed: 0.455912GB/ > > > > > > JFYI I failed to find this message at lore.kernel.org perhaps > > because of pure text mail. > > > > Thanks > > Hillf
Re: [PATCH v4 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support
On Wed, 26 Aug 2020, Mark Brown wrote: > On Wed, 19 Aug 2020 15:34:55 +0800, Xu Yilun wrote: > > This patchset adds the regmap-spi-avmm to support the Intel SPI Slave to > > AVMM Bus Bridge (spi-avmm) IP block. It also implements the usercase - the > > driver of Intel Max10 BMC chip which integrates this IP block. > > > > Patch #1 implements the regmap-spi-avmm. > > Patch #2 implements the mfd driver of Intel Max10 BMC chip. > > > > [...] > > Applied to > >https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next > > Thanks! > > [1/2] regmap: add Intel SPI Slave to AVMM Bus Bridge support > commit: 7f9fb67358a2bcaacbdfeee12e0f19e98c8bdf55 > [2/2] mfd: intel-m10-bmc: add Max10 BMC chip support for Intel FPGA PAC > commit: 53be8bbc2f4058d4a6bfff3dadf34164bcaafa87 Que? This is yet to be reviewed. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] iio: adc: exynos_adc: Replace indio_dev->mlock with own device lock
On Wed, Aug 26, 2020 at 04:22:03PM +0300, Alexandru Ardelean wrote: > From: Sergiu Cuciurean > > As part of the general cleanup of indio_dev->mlock, this change replaces > it with a local lock, to protect potential concurrent access to the > completion callback during a conversion. I don't know the bigger picture (and no links here for general cleanup) but I assume it is part of wider work and that mlock is unwanted. In such case: Reviewed-by: Krzysztof Kozlowski If it is part of some bigger work, please put a link to lore.kernel.org under separators ---, so everyone can get the context. Best regards, Krzysztof > > Signed-off-by: Sergiu Cuciurean > Signed-off-by: Alexandru Ardelean > --- > drivers/iio/adc/exynos_adc.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) >
[PATCH] net: usb: Fix uninit-was-stored issue in asix_read_phy_addr()
The buffer size is 2 Bytes and we expect to receive the same amount of data. But sometimes we receive less data and run into uninit-was-stored issue upon read. Hence modify the error check on the return value to match with the buffer size as a prevention. Reported-and-tested by: syzbot+a7e220df5a81d1ab4...@syzkaller.appspotmail.com Signed-off-by: Himadri Pandya --- drivers/net/usb/asix_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c index e39f41efda3e..7bc6e8f856fe 100644 --- a/drivers/net/usb/asix_common.c +++ b/drivers/net/usb/asix_common.c @@ -296,7 +296,7 @@ int asix_read_phy_addr(struct usbnet *dev, int internal) netdev_dbg(dev->net, "asix_get_phy_addr()\n"); - if (ret < 0) { + if (ret < 2) { netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret); goto out; } -- 2.17.1
Re: [PATCH] usb: host: ohci-exynos: Fix error handling in exynos_ohci_probe()
On Wed, Aug 26, 2020 at 10:49:31PM +0800, Tang Bin wrote: > If the function platform_get_irq() failed, the negative value > returned will not be detected here. So fix error handling in > exynos_ohci_probe(). And when get irq failed, the function > platform_get_irq() logs an error message, so remove redundant > message here. > > Fixes: 62194244cf87 ("USB: Add Samsung Exynos OHCI diver") > Signed-off-by: Zhang Shengju > Signed-off-by: Tang Bin > --- > drivers/usb/host/ohci-exynos.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
[PATCH v3 1/1] extcon: ptn5150: Set the VBUS and POLARITY property capability
From: Ramuthevar Vadivel Murugan Set the capability value of property for VBUS and POLARITY. Signed-off-by: Ramuthevar Vadivel Murugan --- drivers/extcon/extcon-ptn5150.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c index 841c9fe211f1..20d49a54c36e 100644 --- a/drivers/extcon/extcon-ptn5150.c +++ b/drivers/extcon/extcon-ptn5150.c @@ -275,6 +275,13 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c) return ret; } + extcon_set_property_capability(info->edev, EXTCON_USB, + EXTCON_PROP_USB_VBUS); + extcon_set_property_capability(info->edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_VBUS); + extcon_set_property_capability(info->edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_TYPEC_POLARITY); + /* Initialize PTN5150 device and print vendor id and version id */ ret = ptn5150_init_dev_type(info); if (ret) -- 2.11.0
[PATCH v3 0/1]extcon: ptn5150: Add usb-typec support for Intel LGM SoC
Add usb-typec detection support for the Intel LGM SoC based boards. Original driver is not supporting usb detection on Intel LGM SoC based boards then we debugged and fixed the issue, but before sending our patches Mr.Krzyszto has sent the same kind of patches, so I have rebased over his latest patches which is present in maintainer tree. Built and tested it's working fine, overthat created the new patch. Thanks to Chanwoo Choi for the review comments and suggestions --- v3: - Chanwoo Choi review comments update - replace 'capabiliy' to 'state' in commit message - add blank line v2: - Krzyszto review comments update - squash my previous patches 1 to 5 as single patch - add extcon_set_property_capability for EXTCON_USB and EXTCON_PROP_USB_TYPEC_POLARITY Ramuthevar Vadivel Murugan (1): extcon: ptn5150: Set the VBUS and POLARITY property capability drivers/extcon/extcon-ptn5150.c | 7 +++ 1 file changed, 7 insertions(+) -- 2.11.0
[PATCH 7/7 net-next] vxlan: fix vxlan_find_sock() documentation for l3mdev
Since commit aab8cc3630e32 ("vxlan: add support for underlay in non-default VRF") vxlan_find_sock() also checks if socket is assigned to the right level 3 master device when lower device is not in the default VRF. Signed-off-by: Fabian Frederick --- drivers/net/vxlan.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 1501a5633a97e..2c6189e988ba3 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -190,8 +190,9 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb) return list_first_entry(&fdb->remotes, struct vxlan_rdst, list); } -/* Find VXLAN socket based on network namespace, address family and UDP port - * and enabled unshareable flags. +/* Find VXLAN socket based on network namespace, address family, UDP port, + * enabled unshareable flags and socket device binding (see l3mdev with + * non-default VRF). */ static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family, __be16 port, u32 flags, int ifindex) -- 2.27.0
[PATCH 6/7 net-next] vxlan: merge VXLAN_NL2FLAG use in vxlan_nl2conf()
Sort flag assignment to add readability. Signed-off-by: Fabian Frederick --- drivers/net/vxlan.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index e9b561b9d23e1..1501a5633a97e 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -4035,6 +4035,18 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]); VXLAN_NL2FLAG(IFLA_VXLAN_TTL_INHERIT, VXLAN_F_TTL_INHERIT, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_PROXY, VXLAN_F_PROXY, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_RSC, VXLAN_F_RSC, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_L2MISS, VXLAN_F_L2MISS, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_L3MISS, VXLAN_F_L3MISS, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_COLLECT_METADATA, VXLAN_F_COLLECT_METADATA, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_UDP_ZERO_CSUM6_TX, VXLAN_F_UDP_ZERO_CSUM6_TX, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_UDP_ZERO_CSUM6_RX, VXLAN_F_UDP_ZERO_CSUM6_RX, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_TX, IFLA_VXLAN_REMCSUM_TX, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_RX, VXLAN_F_REMCSUM_RX, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_GBP, VXLAN_F_GBP, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_GPE, VXLAN_F_GPE, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_NOPARTIAL, VXLAN_F_REMCSUM_NOPARTIAL, changelink, false); if (data[IFLA_VXLAN_LABEL]) conf->label = nla_get_be32(data[IFLA_VXLAN_LABEL]) & @@ -4054,11 +4066,6 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], if (data[IFLA_VXLAN_AGEING]) conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]); - VXLAN_NL2FLAG(IFLA_VXLAN_PROXY, VXLAN_F_PROXY, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_RSC, VXLAN_F_RSC, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_L2MISS, VXLAN_F_L2MISS, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_L3MISS, VXLAN_F_L3MISS, changelink, false); - if (data[IFLA_VXLAN_LIMIT]) { if (changelink) { NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LIMIT], @@ -4068,8 +4075,6 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]); } - VXLAN_NL2FLAG(IFLA_VXLAN_COLLECT_METADATA, VXLAN_F_COLLECT_METADATA, changelink, false); - if (data[IFLA_VXLAN_PORT_RANGE]) { if (!changelink) { const struct ifla_vxlan_port_range *p @@ -4102,14 +4107,6 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX; } - VXLAN_NL2FLAG(IFLA_VXLAN_UDP_ZERO_CSUM6_TX, VXLAN_F_UDP_ZERO_CSUM6_TX, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_UDP_ZERO_CSUM6_RX, VXLAN_F_UDP_ZERO_CSUM6_RX, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_TX, IFLA_VXLAN_REMCSUM_TX, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_RX, VXLAN_F_REMCSUM_RX, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_GBP, VXLAN_F_GBP, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_GPE, VXLAN_F_GPE, changelink, false); - VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_NOPARTIAL, VXLAN_F_REMCSUM_NOPARTIAL, changelink, false); - if (tb[IFLA_MTU]) { if (changelink) { NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU], -- 2.27.0
[PATCH 5/7 net-next] vxlan: add VXLAN_NL2FLAG macro
Replace common flag assignment with a macro. This could yet be simplified with changelink/supported but it would remove clarity Signed-off-by: Fabian Frederick --- drivers/net/vxlan.c | 113 +--- include/net/vxlan.h | 10 2 files changed, 23 insertions(+), 100 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 1e9ab1002281c..e9b561b9d23e1 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -4034,14 +4034,7 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], if (data[IFLA_VXLAN_TTL]) conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]); - if (data[IFLA_VXLAN_TTL_INHERIT]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT, - VXLAN_F_TTL_INHERIT, changelink, false, - extack); - if (err) - return err; - - } + VXLAN_NL2FLAG(IFLA_VXLAN_TTL_INHERIT, VXLAN_F_TTL_INHERIT, changelink, false); if (data[IFLA_VXLAN_LABEL]) conf->label = nla_get_be32(data[IFLA_VXLAN_LABEL]) & @@ -4061,37 +4054,10 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], if (data[IFLA_VXLAN_AGEING]) conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]); - if (data[IFLA_VXLAN_PROXY]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_PROXY, - VXLAN_F_PROXY, changelink, false, - extack); - if (err) - return err; - } - - if (data[IFLA_VXLAN_RSC]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_RSC, - VXLAN_F_RSC, changelink, false, - extack); - if (err) - return err; - } - - if (data[IFLA_VXLAN_L2MISS]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L2MISS, - VXLAN_F_L2MISS, changelink, false, - extack); - if (err) - return err; - } - - if (data[IFLA_VXLAN_L3MISS]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L3MISS, - VXLAN_F_L3MISS, changelink, false, - extack); - if (err) - return err; - } + VXLAN_NL2FLAG(IFLA_VXLAN_PROXY, VXLAN_F_PROXY, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_RSC, VXLAN_F_RSC, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_L2MISS, VXLAN_F_L2MISS, changelink, false); + VXLAN_NL2FLAG(IFLA_VXLAN_L3MISS, VXLAN_F_L3MISS, changelink, false); if (data[IFLA_VXLAN_LIMIT]) { if (changelink) { @@ -4102,13 +4068,7 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]); } - if (data[IFLA_VXLAN_COLLECT_METADATA]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_COLLECT_METADATA, - VXLAN_F_COLLECT_METADATA, changelink, false, - extack); - if (err) - return err; - } + VXLAN_NL2FLAG(IFLA_VXLAN_COLLECT_METADATA, VXLAN_F_COLLECT_METADATA, changelink, false); if (data[IFLA_VXLAN_PORT_RANGE]) { if (!changelink) { @@ -4142,60 +4102,13 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX; } - if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_TX, - VXLAN_F_UDP_ZERO_CSUM6_TX, changelink, - false, extack); - if (err) - return err; - } - - if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_RX, - VXLAN_F_UDP_ZERO_CSUM6_RX, changelink, - false, extack); - if (err) - return err; - } - - if (data[IFLA_VXLAN_REMCSUM_TX]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_TX, - VXLAN_F_REMCSUM_TX, changelink, false, - extack); - if (err) - return err; - } - - if (data[IFLA_VXLAN_REMCSUM_RX]) { - err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_RX, - VXLAN_F_REMCSUM_RX, changelink, false, - extack); - if (err) -
[PATCH 4/7 net-next] vxlan: check rtnl_configure_link return code correctly
rtnl_configure_link is always checked if < 0 for error code. Signed-off-by: Fabian Frederick --- drivers/net/vxlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 14f903d09c010..1e9ab1002281c 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -3890,7 +3890,7 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev, } err = rtnl_configure_link(dev, NULL); - if (err) + if (err < 0) goto unlink; if (f) { -- 2.27.0
[PATCH 3/7 net-next] vxlan: move encapsulation warning
vxlan_xmit_one() was only called from vxlan_xmit() without rdst and info was already tested. Emit warning in that function instead Signed-off-by: Fabian Frederick --- drivers/net/vxlan.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index cc904f003f158..14f903d09c010 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -2650,11 +2650,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev, udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM6_TX); label = vxlan->cfg.label; } else { - if (!info) { - WARN_ONCE(1, "%s: Missing encapsulation instructions\n", - dev->name); - goto drop; - } remote_ip.sa.sa_family = ip_tunnel_info_af(info); if (remote_ip.sa.sa_family == AF_INET) { remote_ip.sin.sin_addr.s_addr = info->key.u.ipv4.dst; @@ -2889,6 +2884,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) info->mode & IP_TUNNEL_INFO_TX) { vni = tunnel_id_to_key32(info->key.tun_id); } else { + if (!info) + WARN_ONCE(1, "%s: Missing encapsulation instructions\n", + dev->name); + if (info && info->mode & IP_TUNNEL_INFO_TX) vxlan_xmit_one(skb, dev, vni, NULL, false); else -- 2.27.0
[PATCH 2/7 net-next] vxlan: add unlikely to vxlan_remcsum check
small optimization around checking as it's being done in all receptions Signed-off-by: Fabian Frederick --- drivers/net/vxlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 47c762f7f5b11..cc904f003f158 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -1876,7 +1876,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) goto drop; if (vs->flags & VXLAN_F_REMCSUM_RX) - if (!vxlan_remcsum(&unparsed, skb, vs->flags)) + if (unlikely(!vxlan_remcsum(&unparsed, skb, vs->flags))) goto drop; if (vxlan_collect_metadata(vs)) { -- 2.27.0
[PATCH 1/7 net-next] vxlan: don't collect metadata if remote checksum is wrong
call vxlan_remcsum() before md filling in vxlan_rcv() Signed-off-by: Fabian Frederick --- drivers/net/vxlan.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index b9fefe27e3e89..47c762f7f5b11 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -1875,6 +1875,10 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) !net_eq(vxlan->net, dev_net(vxlan->dev goto drop; + if (vs->flags & VXLAN_F_REMCSUM_RX) + if (!vxlan_remcsum(&unparsed, skb, vs->flags)) + goto drop; + if (vxlan_collect_metadata(vs)) { struct metadata_dst *tun_dst; @@ -1891,9 +1895,6 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) memset(md, 0, sizeof(*md)); } - if (vs->flags & VXLAN_F_REMCSUM_RX) - if (!vxlan_remcsum(&unparsed, skb, vs->flags)) - goto drop; if (vs->flags & VXLAN_F_GBP) vxlan_parse_gbp_hdr(&unparsed, skb, vs->flags, md); /* Note that GBP and GPE can never be active together. This is -- 2.27.0
[PATCH] perf/core: Fix hung issue on perf stat command during cpu hotplug
Commit 2ed6edd33a21 ("perf: Add cond_resched() to task_function_call()") added assignment of ret value as -EAGAIN in case function call to 'smp_call_function_single' fails. For non-zero ret value, it did 'ret = !ret ? data.ret : -EAGAIN;', which always assign -EAGAIN to ret and make second if condition useless. In scenarios like when executing a perf stat with --per-thread option, and if any of the monitoring cpu goes offline, the 'smp_call_function_single' function could return -ENXIO, and with the above check, task_function_call hung and increases CPU usage (because of repeated 'smp_call_function_single()') Recration scenario: # perf stat -a --per-thread && (offline a CPU ) Patch here removes the tertiary condition added as part of that commit and added a check for NULL and -EAGAIN. Fixes: 2ed6edd33a21("perf: Add cond_resched() to task_function_call()") Signed-off-by: Kajol Jain Reported-by: Srikar Dronamraju Reviewed-by: Barret Rhoden Tested-by: Srikar Dronamraju --- kernel/events/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) Changelog: - Remove RFC tag - Resolve some nits issues like space after if and added -ENXIO in comment msg of function 'task_function_call' as suggested by Barret Rhoden. Link to the RFC: https://lkml.org/lkml/2020/8/26/896 diff --git a/kernel/events/core.c b/kernel/events/core.c index 5bfe8e3c6e44..cef646084198 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -99,7 +99,7 @@ static void remote_function(void *data) * retry due to any failures in smp_call_function_single(), such as if the * task_cpu() goes offline concurrently. * - * returns @func return value or -ESRCH when the process isn't running + * returns @func return value or -ESRCH or -ENXIO when the process isn't running */ static int task_function_call(struct task_struct *p, remote_function_f func, void *info) @@ -115,7 +115,8 @@ task_function_call(struct task_struct *p, remote_function_f func, void *info) for (;;) { ret = smp_call_function_single(task_cpu(p), remote_function, &data, 1); - ret = !ret ? data.ret : -EAGAIN; + if (!ret) + ret = data.ret; if (ret != -EAGAIN) break; -- 2.26.2
Re: [PATCH v1 4/9] powerpc/vdso: Remove unnecessary ifdefs in vdso_pagelist initialization
On 08/26/2020 02:58 PM, Michael Ellerman wrote: Christophe Leroy writes: diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index daef14a284a3..bbb69832fd46 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -718,16 +710,14 @@ static int __init vdso_init(void) ... - -#ifdef CONFIG_VDSO32 vdso32_kbase = &vdso32_start; /* @@ -735,8 +725,6 @@ static int __init vdso_init(void) */ vdso32_pages = (&vdso32_end - &vdso32_start) >> PAGE_SHIFT; DBG("vdso32_kbase: %p, 0x%x pages\n", vdso32_kbase, vdso32_pages); -#endif This didn't build for ppc64le: /opt/cross/gcc-8.20_binutils-2.32/powerpc64-unknown-linux-gnu/bin/powerpc64-unknown-linux-gnu-ld: arch/powerpc/kernel/vdso.o:(.toc+0x0): undefined reference to `vdso32_end' /opt/cross/gcc-8.20_binutils-2.32/powerpc64-unknown-linux-gnu/bin/powerpc64-unknown-linux-gnu-ld: arch/powerpc/kernel/vdso.o:(.toc+0x8): undefined reference to `vdso32_start' make[1]: *** [/scratch/michael/build/maint/Makefile:1166: vmlinux] Error 1 make: *** [Makefile:185: __sub-make] Error 2 So I just put that ifdef back. The problem is because is_32bit() can still return true even when CONFIG_VDSO32 is not set. The change below fixes the problem: diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index bbb69832fd46..38abff60cbe2 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -132,11 +132,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp) if (!vdso_ready) return 0; - if (is_32bit_task()) { - vdso_pagelist = vdso32_pagelist; - vdso_pages = vdso32_pages; - vdso_base = VDSO32_MBASE; - } else { + if (!is_32bit_task()) { vdso_pagelist = vdso64_pagelist; vdso_pages = vdso64_pages; /* @@ -145,6 +141,12 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp) * and most likely share a SLB entry. */ vdso_base = 0; + } else if (IS_ENABLED(CONFIG_VDSO32)) { + vdso_pagelist = vdso32_pagelist; + vdso_pages = vdso32_pages; + vdso_base = VDSO32_MBASE; + } else { + vdso_pages = 0; } current->mm->context.vdso_base = 0; With this change all vdso32 static objects (functions and vars) go away as expected. We get a simple conflict with the following patch. Do you prefer an updated series or a follow up patch, or you take the above change yourself ? Christophe
[PATCH v2 4/4] crypto: sun8i-ss - use kfree_sensitive()
Use kfree_sensitive() instead of open-coding it. Signed-off-by: Denis Efremov --- .../crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c | 15 +++ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c index 7b39b4495571..deb8b39a86db 100644 --- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c +++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c @@ -368,10 +368,7 @@ void sun8i_ss_cipher_exit(struct crypto_tfm *tfm) { struct sun8i_cipher_tfm_ctx *op = crypto_tfm_ctx(tfm); - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); crypto_free_skcipher(op->fallback_tfm); pm_runtime_put_sync(op->ss->dev); } @@ -393,10 +390,7 @@ int sun8i_ss_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, dev_dbg(ss->dev, "ERROR: Invalid keylen %u\n", keylen); return -EINVAL; } - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); op->keylen = keylen; op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA); if (!op->key) @@ -419,10 +413,7 @@ int sun8i_ss_des3_setkey(struct crypto_skcipher *tfm, const u8 *key, return -EINVAL; } - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); op->keylen = keylen; op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA); if (!op->key) -- 2.26.2
Re: Broadcom WiFi SDIO performance regression after commit "mmc: sdhci: Remove finish_tasklet"
On 27/08/20 9:07 am, Dmitry Osipenko wrote: > Hello! > > I was debugging WiFi performance problems on Acer A500 tablet device > that has BCM4329 WiFi chip which is connected to NVIDIA Terga20 SoC via > SDIO and found that the following commit causes a solid 5-10 Mbit/s of > WiFi throughput regression after 5.2 kernel: What is that in percentage terms? > > commit c07a48c2651965e84d35cf193dfc0e5f7892d612 > Author: Adrian Hunter > Date: Fri Apr 5 15:40:20 2019 +0300 > > mmc: sdhci: Remove finish_tasklet > > Remove finish_tasklet. Requests that require DMA-unmapping or > sdhci_reset > are completed either in the IRQ thread or a workqueue if the > completion is > not initiated by the IRQ. > > Reverting the offending commit on top of recent linux-next resolves the > problem. > > Ulf / Adrian, do you have any ideas what could be done in regards to > restoring the SDIO performance? Should we just revert the offending commit? > Unfortunately I think we are past the point of returning to the tasklet. sdhci can complete requests in the irq handler but only if ->pre_req() and ->post_req() are used, which is not supported by SDIO at present. pre_req and post_req were introduced to reduce latency for the block driver, so it seems reasonable perhaps to look at using them in SDIO as well.
[PATCH v2 0/4] crypto: use kfree_sensitive()
kfree_sensitive() is introduced in commit 453431a54934 ("mm, treewide: rename kzfree() to kfree_sensitive()") and uses memzero_explicit() internally. Thus, we can switch to this API instead of open-coding memzero_explicit() && kfree(). Changes in v2: - if (op->len) check removed Denis Efremov (4): crypto: inside-secure - use kfree_sensitive() crypto: amlogic - use kfree_sensitive() crypto: sun8i-ce - use kfree_sensitive() crypto: sun8i-ss - use kfree_sensitive() .../crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c | 15 +++ .../crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c | 15 +++ drivers/crypto/amlogic/amlogic-gxl-cipher.c | 10 ++ drivers/crypto/inside-secure/safexcel_hash.c | 3 +-- 4 files changed, 9 insertions(+), 34 deletions(-) -- 2.26.2
Re: [PATCH] padata: add a reviewer
On Wed, Aug 26, 2020 at 10:59:23AM -0400, Daniel Jordan wrote: > I volunteer to review padata changes for the foreseeable future. > > Signed-off-by: Daniel Jordan > Cc: Herbert Xu > Cc: Steffen Klassert > Cc: linux-cry...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3b186ade3597..1481d47cfd75 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13024,6 +13024,7 @@ F:lib/packing.c > > PADATA PARALLEL EXECUTION MECHANISM > M: Steffen Klassert > +R: Daniel Jordan Please also consider to add yourself as one of the maintainers. Otherwise: Acked-by: Steffen Klassert Thanks!
[PATCH v2 3/4] crypto: sun8i-ce - use kfree_sensitive()
Use kfree_sensitive() instead of open-coding it. Signed-off-by: Denis Efremov --- .../crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c | 15 +++ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c index b4d5fea27d20..f996dc3d7dcc 100644 --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c @@ -366,10 +366,7 @@ void sun8i_ce_cipher_exit(struct crypto_tfm *tfm) { struct sun8i_cipher_tfm_ctx *op = crypto_tfm_ctx(tfm); - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); crypto_free_skcipher(op->fallback_tfm); pm_runtime_put_sync_suspend(op->ce->dev); } @@ -391,10 +388,7 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen); return -EINVAL; } - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); op->keylen = keylen; op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA); if (!op->key) @@ -416,10 +410,7 @@ int sun8i_ce_des3_setkey(struct crypto_skcipher *tfm, const u8 *key, if (err) return err; - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); op->keylen = keylen; op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA); if (!op->key) -- 2.26.2
[PATCH v2 2/4] crypto: amlogic - use kfree_sensitive()
Use kfree_sensitive() instead of open-coding it. Signed-off-by: Denis Efremov --- drivers/crypto/amlogic/amlogic-gxl-cipher.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/crypto/amlogic/amlogic-gxl-cipher.c b/drivers/crypto/amlogic/amlogic-gxl-cipher.c index d93210726697..ee5998af2fe8 100644 --- a/drivers/crypto/amlogic/amlogic-gxl-cipher.c +++ b/drivers/crypto/amlogic/amlogic-gxl-cipher.c @@ -340,10 +340,7 @@ void meson_cipher_exit(struct crypto_tfm *tfm) { struct meson_cipher_tfm_ctx *op = crypto_tfm_ctx(tfm); - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); crypto_free_skcipher(op->fallback_tfm); } @@ -367,10 +364,7 @@ int meson_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, dev_dbg(mc->dev, "ERROR: Invalid keylen %u\n", keylen); return -EINVAL; } - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); op->keylen = keylen; op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA); if (!op->key) -- 2.26.2
Re: [PATCH 4/4] arm: dts: owl-s500: Add RoseapplePi
> "Cristian" == Cristian Ciocaltea writes: > Add a Device Tree for the RoseapplePi SBC. > Signed-off-by: Cristian Ciocaltea Reviewed-by: Peter Korsgaard On a related note: There is now an owl-mmc driver for the s900. From a quick look at the datasheet it looks compatible with the controller on the s500. Did you have a look at hooking that up? -- Bye, Peter Korsgaard
[PATCH v2 1/4] crypto: inside-secure - use kfree_sensitive()
Use kfree_sensitive() instead of open-coding it. Signed-off-by: Denis Efremov --- drivers/crypto/inside-secure/safexcel_hash.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c index 16a467969d8e..5ffdc1cd5847 100644 --- a/drivers/crypto/inside-secure/safexcel_hash.c +++ b/drivers/crypto/inside-secure/safexcel_hash.c @@ -1082,8 +1082,7 @@ static int safexcel_hmac_init_pad(struct ahash_request *areq, } /* Avoid leaking */ - memzero_explicit(keydup, keylen); - kfree(keydup); + kfree_sensitive(keydup); if (ret) return ret; -- 2.26.2
Re: [PATCH] usb: atm: don't use snprintf() for sysfs attrs
On 25/08/2020 00.23, Alex Dewar wrote: > kernel/cpu.c: don't use snprintf() for sysfs attrs > > As per the documentation (Documentation/filesystems/sysfs.rst), > snprintf() should not be used for formatting values returned by sysfs. > Sure. But then the security guys come along and send a patch saying "sprintf is evil, always pass a buffer bound". Perhaps with a side of "this code could get copy-pasted, better not promote the use of sprintf more than strictly necessary". Can we have a sysfs_sprintf() (could just be a macro that does sprintf) to make it clear to the next reader that we know we're in a sysfs show method? It would make auditing uses of sprintf() much easier. > static ssize_t cxacru_sysfs_showattr_LINE(u32 value, char *buf) > @@ -275,8 +275,8 @@ static ssize_t cxacru_sysfs_showattr_LINE(u32 value, char > *buf) > "waiting", "initialising" > }; > if (unlikely(value >= ARRAY_SIZE(str))) > - return snprintf(buf, PAGE_SIZE, "%u\n", value); > - return snprintf(buf, PAGE_SIZE, "%s\n", str[value]); > + return sprintf(buf, "%u\n", value); > + return sprintf(buf, "%s\n", str[value]); > } Not this patch in particular, but in some cases the string being printed is not immediately adjacent to the sprintf call, making it rather hard to subsequently verify that yes, that string is certainly reasonably bounded. If you really want to save the few bytes of code that is the practical effect of eliding the PAGE_SIZE argument, how about a sysfs_print_string(buf, str) helper that prints the string and appends a newline; that's another argument saved. And again it would make it obvious to a reader that that particular helper is only to be used in sysfs show methods. Rasmus
Re: [PATCH 3/4] libnvdimm: eliminate two unnecessary zero initializations in badrange.c
I will drop this patch, because badrange_add() is unlikely to be called. There's no need to care about trivial performance improvements. On 2020/8/20 22:30, Zhen Lei wrote: > Currently, the "struct badrange_entry" has three members: start, length, > list. In append_badrange_entry(), "start" and "length" will be assigned > later, and "list" does not need to be initialized before calling > list_add_tail(). That means, the kzalloc() in badrange_add() or > alloc_and_append_badrange_entry() can be replaced with kmalloc(), because > the zero initialization is not required. > > Signed-off-by: Zhen Lei > --- > drivers/nvdimm/badrange.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c > index 7f78b659057902d..13145001c52ff39 100644 > --- a/drivers/nvdimm/badrange.c > +++ b/drivers/nvdimm/badrange.c > @@ -37,7 +37,7 @@ static int alloc_and_append_badrange_entry(struct badrange > *badrange, > { > struct badrange_entry *bre; > > - bre = kzalloc(sizeof(*bre), flags); > + bre = kmalloc(sizeof(*bre), flags); > if (!bre) > return -ENOMEM; > > @@ -49,7 +49,7 @@ int badrange_add(struct badrange *badrange, u64 addr, u64 > length) > { > struct badrange_entry *bre, *bre_new; > > - bre_new = kzalloc(sizeof(*bre_new), GFP_KERNEL); > + bre_new = kmalloc(sizeof(*bre_new), GFP_KERNEL); > > spin_lock(&badrange->lock); > >
Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise
On Thu, 27 Aug 2020 at 00:19, Herbert Xu wrote: > > On Wed, Aug 26, 2020 at 05:42:27PM +0200, Ard Biesheuvel wrote: > > > > I still get a failure in aes_siv_encrypt(), which does not occur with > > the kernel side fix applied. > > Where is this test from? I can't find it in the ell git tree. > It is part of iwd - just build that and run 'make check' With your patch applied, the occurrence of sendmsg() in operate_cipher() triggers the warn_once(), but if I add MSG_MORE there, the test hangs.
Re: [PATCH] thermal: sysfs: fall back to vzalloc for cooling device's statistics
On Thu, 27 Aug 2020 11:56:46 +0530 Viresh Kumar wrote: > On 27-08-20, 14:20, Yue Hu wrote: > > Currenly, drivers/video/backlight does not call > > thermal_of_cooling_device_register() > > to register thermal cooling device. The issue happened in msm-4.19 kernel > > for > > QCOM/Android platform. Backlight in msm-4.19 kernel will register thermal > > cooling > > device as below: > > > > +static int bd_cdev_get_max_brightness(struct thermal_cooling_device *cdev, > > + unsigned long *state) > > +{ > > + struct backlight_device *bd = (struct backlight_device *)cdev->devdata; > > + > > + *state = bd->props.max_brightness; > > + > > + return 0; > > +} > > > > > > +static struct thermal_cooling_device_ops bd_cdev_ops = { > > + .get_max_state = bd_cdev_get_max_brightness, > > > > +static void backlight_cdev_register(struct device *parent, > > + struct backlight_device *bd) > > +{ > > + if (of_find_property(parent->of_node, "#cooling-cells", NULL)) { > > + bd->cdev = thermal_of_cooling_device_register(parent->of_node, > > + (char *)dev_name(&bd->dev), bd, &bd_cdev_ops); > > > > And the bd->props.max_brightness is getting from > > video/backlight/qcom-wled.c. Maybe > > the driver should not assign 1024 to states/max_brightness. I'm not sure > > about it. > > So i consider to change memory allocation methord. That's the origin of the > > patch. > > Thanks for the details. So this is not about upstream tree, as a rule > we aren't going to make changes here for any downstream tree. I at first thought it's a possible issue in upstream tree. > > Now coming back to the downstream driver, I also don't see a point in > returning bd->props.max_brightness as the max number of states there. > Maybe have 10 states, each occupying bd->props.max_brightness/10 > brightness and so you will end up with 10 states only. But yeah, > whatever downstream decides on that. > Got it. Thx.
Re: [PATCH] fs: Kill DCACHE_DONTCACHE dentry even if DCACHE_REFERENCED is set
On Fri, Aug 21, 2020 at 09:59:53AM +0800, Hao Li wrote: > Currently, DCACHE_REFERENCED prevents the dentry with DCACHE_DONTCACHE > set from being killed, so the corresponding inode can't be evicted. If > the DAX policy of an inode is changed, we can't make policy changing > take effects unless dropping caches manually. > > This patch fixes this problem and flushes the inode to disk to prepare > for evicting it. > > Signed-off-by: Hao Li > --- > fs/dcache.c | 3 ++- > fs/inode.c | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/dcache.c b/fs/dcache.c > index ea0485861d93..486c7409dc82 100644 > --- a/fs/dcache.c > +++ b/fs/dcache.c > @@ -796,7 +796,8 @@ static inline bool fast_dput(struct dentry *dentry) >*/ > smp_rmb(); > d_flags = READ_ONCE(dentry->d_flags); > - d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | DCACHE_DISCONNECTED; > + d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | DCACHE_DISCONNECTED > + | DCACHE_DONTCACHE; Seems reasonable, but you need to update the comment above as to how this flag fits into this code > /* Nothing to do? Dropping the reference was all we needed? */ > if (d_flags == (DCACHE_REFERENCED | DCACHE_LRU_LIST) && > !d_unhashed(dentry)) > diff --git a/fs/inode.c b/fs/inode.c > index 72c4c347afb7..5218a8aebd7f 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -1632,7 +1632,7 @@ static void iput_final(struct inode *inode) > } > > state = inode->i_state; > - if (!drop) { > + if (!drop || (drop && (inode->i_state & I_DONTCACHE))) { > WRITE_ONCE(inode->i_state, state | I_WILL_FREE); > spin_unlock(&inode->i_lock); What's this supposed to do? We'll only get here with drop set if the filesystem is mounting or unmounting. In either case, why does having I_DONTCACHE set require the inode to be written back here before it is evicted from the cache? Cheers, Dave. -- Dave Chinner da...@fromorbit.com
Re: [PATCH 12/30] wireless: ath: wil6210: wmi: Correct misnamed function parameter 'ptr_'
On Wed, 26 Aug 2020, Kalle Valo wrote: > Lee Jones wrote: > > > Fixes the following W=1 kernel build warning(s): > > > > drivers/net/wireless/ath/wil6210/wmi.c:279: warning: Function parameter or > > member 'ptr_' not described in 'wmi_buffer_block' > > drivers/net/wireless/ath/wil6210/wmi.c:279: warning: Excess function > > parameter 'ptr' description in 'wmi_buffer_block' > > > > Cc: Maya Erez > > Cc: Kalle Valo > > Cc: "David S. Miller" > > Cc: Jakub Kicinski > > Cc: linux-wirel...@vger.kernel.org > > Cc: wil6...@qti.qualcomm.com > > Cc: net...@vger.kernel.org > > Signed-off-by: Lee Jones > > Failed to apply: > > error: patch failed: drivers/net/wireless/ath/wil6210/wmi.c:266 > error: drivers/net/wireless/ath/wil6210/wmi.c: patch does not apply > stg import: Diff does not apply cleanly > > Patch set to Changes Requested. Are you applying them in order? It may be affected by: wireless: ath: wil6210: wmi: Fix formatting and demote non-conforming function headers I'll also rebase onto the latest -next and resubmit. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
[PATCH v8 3/3] Wire UFFD up to SELinux
From: Daniel Colascione This change gives userfaultfd file descriptors a real security context, allowing policy to act on them. Signed-off-by: Daniel Colascione [Remove owner inode from userfaultfd_ctx] [Use anon_inode_getfd_secure() instead of anon_inode_getfile_secure() in userfaultfd syscall] [Use inode of file in userfaultfd_read() in resolve_userfault_fork()] Signed-off-by: Lokesh Gidra --- fs/userfaultfd.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 0e4a3837da52..918535b49475 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -978,14 +978,14 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait) static const struct file_operations userfaultfd_fops; -static int resolve_userfault_fork(struct userfaultfd_ctx *ctx, - struct userfaultfd_ctx *new, +static int resolve_userfault_fork(struct userfaultfd_ctx *new, + struct inode *inode, struct uffd_msg *msg) { int fd; - fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, new, - O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS)); + fd = anon_inode_getfd_secure("[userfaultfd]", &userfaultfd_fops, new, + O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS), inode); if (fd < 0) return fd; @@ -995,7 +995,7 @@ static int resolve_userfault_fork(struct userfaultfd_ctx *ctx, } static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait, - struct uffd_msg *msg) + struct uffd_msg *msg, struct inode *inode) { ssize_t ret; DECLARE_WAITQUEUE(wait, current); @@ -1106,7 +1106,7 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait, spin_unlock_irq(&ctx->fd_wqh.lock); if (!ret && msg->event == UFFD_EVENT_FORK) { - ret = resolve_userfault_fork(ctx, fork_nctx, msg); + ret = resolve_userfault_fork(fork_nctx, inode, msg); spin_lock_irq(&ctx->event_wqh.lock); if (!list_empty(&fork_event)) { /* @@ -1166,6 +1166,7 @@ static ssize_t userfaultfd_read(struct file *file, char __user *buf, ssize_t _ret, ret = 0; struct uffd_msg msg; int no_wait = file->f_flags & O_NONBLOCK; + struct inode *inode = file_inode(file); if (ctx->state == UFFD_STATE_WAIT_API) return -EINVAL; @@ -1173,7 +1174,7 @@ static ssize_t userfaultfd_read(struct file *file, char __user *buf, for (;;) { if (count < sizeof(msg)) return ret ? ret : -EINVAL; - _ret = userfaultfd_ctx_read(ctx, no_wait, &msg); + _ret = userfaultfd_ctx_read(ctx, no_wait, &msg, inode); if (_ret < 0) return ret ? ret : _ret; if (copy_to_user((__u64 __user *) buf, &msg, sizeof(msg))) @@ -1995,8 +1996,8 @@ SYSCALL_DEFINE1(userfaultfd, int, flags) /* prevent the mm struct to be freed */ mmgrab(ctx->mm); - fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx, - O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS)); + fd = anon_inode_getfd_secure("[userfaultfd]", &userfaultfd_fops, ctx, + O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS), NULL); if (fd < 0) { mmdrop(ctx->mm); kmem_cache_free(userfaultfd_ctx_cachep, ctx); -- 2.28.0.297.g1956fa8f8d-goog
Re: [PATCH v3 0/7] bugfix and optimize for drivers/nvdimm
Hi all: Any comment? I want to merge patches 1 and 2 into one, then send other patches separately. On 2020/8/20 10:16, Zhen Lei wrote: > v2 --> v3: > 1. Fix spelling error of patch 1 subject: memmory --> memory > 2. Add "Reviewed-by: Oliver O'Halloran " into patch 1 > 3. Rewrite patch descriptions of Patch 1, 3, 4 > 4. Add 3 new trivial patches 5-7, I just found that yesterday. > 5. Unify all "subsystem" names to "libnvdimm:" > > v1 --> v2: > 1. Add Fixes for Patch 1-2 > 2. Slightly change the subject and description of Patch 1 > 3. Add a new trivial Patch 4, I just found that yesterday. > > v1: > I found a memleak when I learned the drivers/nvdimm code today. And I also > added a sanity check for priv->bus_desc.provider_name, because strdup() > maybe failed. Patch 3 is a trivial source code optimization. > > > Zhen Lei (7): > libnvdimm: fix memory leaks in of_pmem.c > libnvdimm: add sanity check for provider_name in > of_pmem_region_probe() > libnvdimm: simplify walk_to_nvdimm_bus() > libnvdimm: reduce an unnecessary if branch in nd_region_create() > libnvdimm: reduce an unnecessary if branch in nd_region_activate() > libnvdimm: make sure EXPORT_SYMBOL_GPL(nvdimm_flush) close to its > function > libnvdimm: slightly simplify available_slots_show() > > drivers/nvdimm/bus.c | 7 +++ > drivers/nvdimm/dimm_devs.c | 5 ++--- > drivers/nvdimm/of_pmem.c | 7 +++ > drivers/nvdimm/region_devs.c | 13 - > 4 files changed, 16 insertions(+), 16 deletions(-) >
[PATCH v8 0/3] SELinux support for anonymous inodes and UFFD
Userfaultfd in unprivileged contexts could be potentially very useful. We'd like to harden userfaultfd to make such unprivileged use less risky. This patch series allows SELinux to manage userfaultfd file descriptors and in the future, other kinds of anonymous-inode-based file descriptor. SELinux policy authors can apply policy types to anonymous inodes by providing name-based transition rules keyed off the anonymous inode internal name ( "[userfaultfd]" in the case of userfaultfd(2) file descriptors) and applying policy to the new SIDs thus produced. With SELinux managed userfaultfd, an admin can control creation and movement of the file descriptors. In particular, handling of a userfaultfd descriptor by a different process is essentially a ptrace access into the process, without any of the corresponding security_ptrace_access_check() checks. For privacy, the admin may want to deny such accesses, which is possible with SELinux support. Inside the kernel, a new anon_inode interface, anon_inode_getfd_secure, allows callers to opt into this SELinux management. In this new "secure" mode, anon_inodes create new ephemeral inodes for anonymous file objects instead of reusing the normal anon_inodes singleton dummy inode. A new LSM hook gives security modules an opportunity to configure and veto these ephemeral inodes. This patch series is one of two fork of [1] and is an alternative to [2]. The primary difference between the two patch series is that this partch series creates a unique inode for each "secure" anonymous inode, while the other patch series ([2]) continues using the singleton dummy anonymous inode and adds a way to attach SELinux security information directly to file objects. I prefer the approach in this patch series because 1) it's a smaller patch than [2], and 2) it produces a more regular security architecture: in this patch series, secure anonymous inodes aren't S_PRIVATE and they maintain the SELinux property that the label for a file is in its inode. We do need an additional inode per anonymous file, but per-struct-file inode creation doesn't seem to be a problem for pipes and sockets. The previous version of this feature ([1]) created a new SELinux security class for userfaultfd file descriptors. This version adopts the generic transition-based approach of [2]. This patch series also differs from [2] in that it doesn't affect all anonymous inodes right away --- instead requiring anon_inodes callers to opt in --- but this difference isn't one of basic approach. The important question to resolve is whether we should be creating new inodes or enhancing per-file data. Changes from the first version of the patch: - Removed some error checks - Defined a new anon_inode SELinux class to resolve the ambiguity in [3] - Inherit sclass as well as descriptor from context inode Changes from the second version of the patch: - Fixed example policy in the commit message to reflect the use of the new anon_inode class. Changes from the third version of the patch: - Dropped the fops parameter to the LSM hook - Documented hook parameters - Fixed incorrect class used for SELinux transition - Removed stray UFFD changed early in the series - Removed a redundant ERR_PTR(PTR_ERR()) Changes from the fourth version of the patch: - Removed an unused parameter from an internal function - Fixed function documentation Changes from the fifth version of the patch: - Fixed function documentation in fs/anon_inodes.c and include/linux/lsm_hooks.h - Used anon_inode_getfd_secure() in userfaultfd() syscall and removed owner from userfaultfd_ctx. Changed from the sixth version of the patch: - Removed definition of anon_inode_getfile_secure() as there are no callers. - Simplified function description of anon_inode_getfd_secure(). - Elaborated more on the purpose of 'context_inode' in commit message. Changed from the seventh version of the patch: - Fixed error handling in _anon_inode_getfile(). - Fixed minor comment and indentation related issues. [1] https://lore.kernel.org/lkml/20200211225547.235083-1-dan...@google.com/ [2] https://lore.kernel.org/linux-fsdevel/20200213194157.5877-1-...@tycho.nsa.gov/ [3] https://lore.kernel.org/lkml/23f725ca-5b5a-5938-fcc8-5bbbfc9ba...@tycho.nsa.gov/ Daniel Colascione (3): Add a new LSM-supporting anonymous inode interface Teach SELinux about anonymous inodes Wire UFFD up to SELinux fs/anon_inodes.c| 147 fs/userfaultfd.c| 19 ++-- include/linux/anon_inodes.h | 8 ++ include/linux/lsm_hook_defs.h | 2 + include/linux/lsm_hooks.h | 9 ++ include/linux/security.h| 10 ++ security/security.c | 8 ++ security/selinux/hooks.c| 53 ++ security/selinux/include/classmap.h | 2 + 9 files changed, 209 insertions(+), 49 deletions(-) -- 2.28.0.297.g1956fa8f8d-goog
[PATCH v8 1/3] Add a new LSM-supporting anonymous inode interface
From: Daniel Colascione This change adds a new function, anon_inode_getfd_secure, that creates anonymous-node file with individual non-S_PRIVATE inode to which security modules can apply policy. Existing callers continue using the original singleton-inode kind of anonymous-inode file. We can transition anonymous inode users to the new kind of anonymous inode in individual patches for the sake of bisection and review. The new function accepts an optional context_inode parameter that callers can use to provide additional contextual information to security modules for granting/denying permission to create an anon inode of the same type. For example, in case of userfaultfd, the created inode is a 'logical child' of the context_inode (userfaultfd inode of the parent process) in the sense that it provides the security context required during creation of the child process' userfaultfd inode. Signed-off-by: Daniel Colascione [Fix comment documenting return values of inode_init_security_anon()] [Add context_inode description in comments to anon_inode_getfd_secure()] [Remove definition of anon_inode_getfile_secure() as there are no callers] [Make _anon_inode_getfile() static] [Use correct error cast in _anon_inode_getfile()] [Fix error handling in _anon_inode_getfile()] Signed-off-by: Lokesh Gidra --- fs/anon_inodes.c | 147 +- include/linux/anon_inodes.h | 8 ++ include/linux/lsm_hook_defs.h | 2 + include/linux/lsm_hooks.h | 9 +++ include/linux/security.h | 10 +++ security/security.c | 8 ++ 6 files changed, 144 insertions(+), 40 deletions(-) diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c index 89714308c25b..c3f16deda211 100644 --- a/fs/anon_inodes.c +++ b/fs/anon_inodes.c @@ -55,61 +55,79 @@ static struct file_system_type anon_inode_fs_type = { .kill_sb= kill_anon_super, }; -/** - * anon_inode_getfile - creates a new file instance by hooking it up to an - * anonymous inode, and a dentry that describe the "class" - * of the file - * - * @name:[in]name of the "class" of the new file - * @fops:[in]file operations for the new file - * @priv:[in]private data for the new file (will be file's private_data) - * @flags: [in]flags - * - * Creates a new file by hooking it on a single inode. This is useful for files - * that do not need to have a full-fledged inode in order to operate correctly. - * All the files created with anon_inode_getfile() will share a single inode, - * hence saving memory and avoiding code duplication for the file/inode/dentry - * setup. Returns the newly created file* or an error pointer. - */ -struct file *anon_inode_getfile(const char *name, - const struct file_operations *fops, - void *priv, int flags) +static struct inode *anon_inode_make_secure_inode( + const char *name, + const struct inode *context_inode) { - struct file *file; + struct inode *inode; + const struct qstr qname = QSTR_INIT(name, strlen(name)); + int error; + + inode = alloc_anon_inode(anon_inode_mnt->mnt_sb); + if (IS_ERR(inode)) + return inode; + inode->i_flags &= ~S_PRIVATE; + error = security_inode_init_security_anon(inode, &qname, context_inode); + if (error) { + iput(inode); + return ERR_PTR(error); + } + return inode; +} - if (IS_ERR(anon_inode_inode)) - return ERR_PTR(-ENODEV); +static struct file *_anon_inode_getfile(const char *name, + const struct file_operations *fops, + void *priv, int flags, + const struct inode *context_inode, + bool secure) +{ + struct inode *inode; + struct file *file; if (fops->owner && !try_module_get(fops->owner)) return ERR_PTR(-ENOENT); - /* -* We know the anon_inode inode count is always greater than zero, -* so ihold() is safe. -*/ - ihold(anon_inode_inode); - file = alloc_file_pseudo(anon_inode_inode, anon_inode_mnt, name, + if (secure) { + inode = anon_inode_make_secure_inode(name, context_inode); + if (IS_ERR(inode)) { + file = ERR_CAST(inode); + goto err; + } + } else { + inode = anon_inode_inode; + if (IS_ERR(inode)) { + file = ERR_PTR(-ENODEV); + goto err; + } + /* +* We know the anon_inode inode count is always +* greater than zero, so ihold() is safe. +*/ + ihold(inode); + } + + file = alloc_file_pseudo(in
[PATCH v8 2/3] Teach SELinux about anonymous inodes
From: Daniel Colascione This change uses the anon_inodes and LSM infrastructure introduced in the previous patch to give SELinux the ability to control anonymous-inode files that are created using the new anon_inode_getfd_secure() function. A SELinux policy author detects and controls these anonymous inodes by adding a name-based type_transition rule that assigns a new security type to anonymous-inode files created in some domain. The name used for the name-based transition is the name associated with the anonymous inode for file listings --- e.g., "[userfaultfd]" or "[perf_event]". Example: type uffd_t; type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]"; allow sysadm_t uffd_t:anon_inode { create }; (The next patch in this series is necessary for making userfaultfd support this new interface. The example above is just for exposition.) Signed-off-by: Daniel Colascione Acked-by: Casey Schaufler Acked-by: Stephen Smalley Cc: Al Viro Cc: Andrew Morton --- security/selinux/hooks.c| 53 + security/selinux/include/classmap.h | 2 ++ 2 files changed, 55 insertions(+) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a340986aa92e..b83f56e5ef40 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2926,6 +2926,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, return 0; } +static int selinux_inode_init_security_anon(struct inode *inode, + const struct qstr *name, + const struct inode *context_inode) +{ + const struct task_security_struct *tsec = selinux_cred(current_cred()); + struct common_audit_data ad; + struct inode_security_struct *isec; + int rc; + + if (unlikely(!selinux_state.initialized)) + return 0; + + isec = selinux_inode(inode); + + /* +* We only get here once per ephemeral inode. The inode has +* been initialized via inode_alloc_security but is otherwise +* untouched. +*/ + + if (context_inode) { + struct inode_security_struct *context_isec = + selinux_inode(context_inode); + isec->sclass = context_isec->sclass; + isec->sid = context_isec->sid; + } else { + isec->sclass = SECCLASS_ANON_INODE; + rc = security_transition_sid( + &selinux_state, tsec->sid, tsec->sid, + isec->sclass, name, &isec->sid); + if (rc) + return rc; + } + + isec->initialized = LABEL_INITIALIZED; + + /* +* Now that we've initialized security, check whether we're +* allowed to actually create this type of anonymous inode. +*/ + + ad.type = LSM_AUDIT_DATA_INODE; + ad.u.inode = inode; + + return avc_has_perm(&selinux_state, + tsec->sid, + isec->sid, + isec->sclass, + FILE__CREATE, + &ad); +} + static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode) { return may_create(dir, dentry, SECCLASS_FILE); @@ -6987,6 +7039,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security), LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security), + LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon), LSM_HOOK_INIT(inode_create, selinux_inode_create), LSM_HOOK_INIT(inode_link, selinux_inode_link), LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink), diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 40cebde62856..ba2e01a6955c 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -249,6 +249,8 @@ struct security_class_mapping secclass_map[] = { {"open", "cpu", "kernel", "tracepoint", "read", "write"} }, { "lockdown", { "integrity", "confidentiality", NULL } }, + { "anon_inode", + { COMMON_FILE_PERMS, NULL } }, { NULL } }; -- 2.28.0.297.g1956fa8f8d-goog
Re: [PATCH] PCI/ASPM: Enable ASPM for links under VMD domain
On Wed, Aug 26, 2020 at 09:43:27PM +, Derrick, Jonathan wrote: > Feel free to review my set to disable the MSI remapping which will make > it perform as well as direct-attached: > > https://patchwork.kernel.org/project/linux-pci/list/?series=325681 So that then we have to deal with your schemes to make individual device direct assignment work in a convoluted way? Please just give us a disable nob for VMD, which solves _all_ these problems without adding any.
Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote: > Hi, > > On 8/24/2020 12:30 PM, Jim Quinlan wrote: >> >> Patchset Summary: >>Enhance a PCIe host controller driver. Because of its unusual design >>we are foced to change dev->dma_pfn_offset into a more general role >>allowing multiple offsets. See the 'v1' notes below for more info. > > We are version 11 and counting, and it is not clear to me whether there is > any chance of getting these patches reviewed and hopefully merged for the > 5.10 merge window. > > There are a lot of different files being touched, so what would be the > ideal way of routing those changes towards inclusion? FYI, I offered to take the dma-mapping bits through the dma-mapping tree. I have a bit of a backlog, but plan to review and if Jim is ok with that apply the current version.
Re: [PATCH 25/32] wireless: ath: wil6210: wmi: Fix formatting and demote non-conforming function headers
On Wed, 26 Aug 2020, Kalle Valo wrote: > Lee Jones wrote: > > > Fixes the following W=1 kernel build warning(s): > > > > drivers/net/wireless/ath/wil6210/wmi.c:52: warning: Incorrect use of > > kernel-doc format: * Addressing - theory of operations > > drivers/net/wireless/ath/wil6210/wmi.c:70: warning: Incorrect use of > > kernel-doc format: * @sparrow_fw_mapping provides memory remapping table > > for sparrow > > drivers/net/wireless/ath/wil6210/wmi.c:80: warning: cannot understand > > function prototype: 'const struct fw_map sparrow_fw_mapping[] = ' > > drivers/net/wireless/ath/wil6210/wmi.c:107: warning: Cannot understand * > > @sparrow_d0_mac_rgf_ext - mac_rgf_ext section for Sparrow D0 > > drivers/net/wireless/ath/wil6210/wmi.c:115: warning: Cannot understand * > > @talyn_fw_mapping provides memory remapping table for Talyn > > drivers/net/wireless/ath/wil6210/wmi.c:158: warning: Cannot understand * > > @talyn_mb_fw_mapping provides memory remapping table for Talyn-MB > > drivers/net/wireless/ath/wil6210/wmi.c:236: warning: Function parameter or > > member 'x' not described in 'wmi_addr_remap' > > drivers/net/wireless/ath/wil6210/wmi.c:255: warning: Function parameter or > > member 'section' not described in 'wil_find_fw_mapping' > > drivers/net/wireless/ath/wil6210/wmi.c:278: warning: Function parameter or > > member 'wil' not described in 'wmi_buffer_block' > > drivers/net/wireless/ath/wil6210/wmi.c:278: warning: Function parameter or > > member 'ptr_' not described in 'wmi_buffer_block' > > drivers/net/wireless/ath/wil6210/wmi.c:278: warning: Function parameter or > > member 'size' not described in 'wmi_buffer_block' > > drivers/net/wireless/ath/wil6210/wmi.c:307: warning: Function parameter or > > member 'wil' not described in 'wmi_addr' > > drivers/net/wireless/ath/wil6210/wmi.c:307: warning: Function parameter or > > member 'ptr' not described in 'wmi_addr' > > drivers/net/wireless/ath/wil6210/wmi.c:1589: warning: Function parameter > > or member 'wil' not described in 'wil_find_cid_ringid_sta' > > drivers/net/wireless/ath/wil6210/wmi.c:1589: warning: Function parameter > > or member 'vif' not described in 'wil_find_cid_ringid_sta' > > drivers/net/wireless/ath/wil6210/wmi.c:1589: warning: Function parameter > > or member 'cid' not described in 'wil_find_cid_ringid_sta' > > drivers/net/wireless/ath/wil6210/wmi.c:1589: warning: Function parameter > > or member 'ringid' not described in 'wil_find_cid_ringid_sta' > > drivers/net/wireless/ath/wil6210/wmi.c:1876: warning: Function parameter > > or member 'vif' not described in 'wmi_evt_ignore' > > drivers/net/wireless/ath/wil6210/wmi.c:1876: warning: Function parameter > > or member 'id' not described in 'wmi_evt_ignore' > > drivers/net/wireless/ath/wil6210/wmi.c:1876: warning: Function parameter > > or member 'd' not described in 'wmi_evt_ignore' > > drivers/net/wireless/ath/wil6210/wmi.c:1876: warning: Function parameter > > or member 'len' not described in 'wmi_evt_ignore' > > drivers/net/wireless/ath/wil6210/wmi.c:2588: warning: Function parameter > > or member 'wil' not described in 'wmi_rxon' > > > > Cc: Maya Erez > > Cc: Kalle Valo > > Cc: "David S. Miller" > > Cc: Jakub Kicinski > > Cc: linux-wirel...@vger.kernel.org > > Cc: wil6...@qti.qualcomm.com > > Cc: net...@vger.kernel.org > > Signed-off-by: Lee Jones > > So what's the plan, should I drop the six patches for wil6210 in this > patchset? I'll fix them and submit the v2s in reply-to the v1s. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 2/2] usb: dwc3: Add driver for Xilinx platforms
Manish Narani writes: > Add a new driver for supporting Xilinx platforms. This driver handles > the USB 3.0 PHY initialization and PIPE control & reset operations for PHY initialization should be done as part of a drivers/phy driver. > ZynqMP platforms. This also handles the USB 2.0 PHY initialization and > reset operations for Versal platforms. similarly for USB2 PHYs > diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c > new file mode 100644 > index ..272906797a7a > --- /dev/null > +++ b/drivers/usb/dwc3/dwc3-xilinx.c > @@ -0,0 +1,416 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/** > + * dwc3-xilinx.c - Xilinx DWC3 controller specific glue driver > + * > + * Authors: Anurag Kumar Vulisha > + * Manish Narani > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +/* USB phy reset mask register */ > +#define XLNX_USB_PHY_RST 0x001C > +#define XLNX_PHY_RST_MASK0x1 > + > +/* Xilinx USB 3.0 IP Register */ > +#define XLNX_USB_COHERENCY 0x005C > +#define XLNX_USB_COHERENCY_ENABLE0x1 > + > +/* Versal USB Reset ID */ > +#define VERSAL_USB_RESET_ID 0xC104036 > + > +#define PIPE_CLK_OFFSET 0x7c > +#define PIPE_CLK_ON 1 > +#define PIPE_CLK_OFF 0 > +#define PIPE_POWER_OFFSET0x80 > +#define PIPE_POWER_ON1 > +#define PIPE_POWER_OFF 0 > + > +#define RST_TIMEOUT 1000 > + > +struct dwc3_xlnx { > + int num_clocks; > + struct clk_bulk_data*clks; > + struct device *dev; > + void __iomem*regs; > + struct dwc3 *dwc; > + struct phy *phy; > + struct phy *usb3_phy; > + struct reset_control*crst; > + struct reset_control*hibrst; > + struct reset_control*apbrst; > +}; > + > +static void dwc3_xlnx_mask_phy_rst(struct dwc3_xlnx *priv_data, bool mask) > +{ > + u32 reg; > + > + reg = readl(priv_data->regs + XLNX_USB_PHY_RST); > + > + if (mask) > + /* > + * Mask the phy reset signal from comtroller s/comtroller/controller But really, the comments don't bring any extra information. I'd say remove the comments as the code speaks for itself very clearly in this case. > +static int dwc3_xlnx_rst_assert(struct reset_control *rstc) this looks like it should be an actual reset controller driver. > +static int dwc3_xlnx_init_versal(struct dwc3_xlnx *priv_data) > +{ > + struct device *dev = priv_data->dev; > + int ret; > + > + dwc3_xlnx_mask_phy_rst(priv_data, false); > + > + /* Assert and De-assert reset */ > + ret = zynqmp_pm_reset_assert(VERSAL_USB_RESET_ID, > + PM_RESET_ACTION_ASSERT); > + if (ret < 0) { > + dev_err(dev, "failed to assert Reset\n"); > + return ret; > + } > + > + ret = zynqmp_pm_reset_assert(VERSAL_USB_RESET_ID, > + PM_RESET_ACTION_RELEASE); > + if (ret < 0) { > + dev_err(dev, "failed to De-assert Reset\n"); > + return ret; > + } > + > + dwc3_xlnx_mask_phy_rst(priv_data, true); > + > + return 0; > +} > + > +static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data) > +{ > + struct device *dev = priv_data->dev; > + int ret; > + u32 reg; > + > + priv_data->crst = devm_reset_control_get(dev, "usb_crst"); > + if (IS_ERR(priv_data->crst)) { > + dev_err(dev, "failed to get %s reset signal\n", "usb_crst"); > + ret = PTR_ERR(priv_data->crst); > + goto err; > + } > + > + priv_data->hibrst = devm_reset_control_get(dev, "usb_hibrst"); > + if (IS_ERR(priv_data->hibrst)) { > + dev_err(dev, "failed to get %s reset signal\n", "usb_hibrst"); > + ret = PTR_ERR(priv_data->hibrst); > + goto err; > + } > + > + priv_data->apbrst = devm_reset_control_get(dev, "usb_apbrst"); > + if (IS_ERR(priv_data->apbrst)) { > + dev_err(dev, "failed to get %s reset signal\n", "usb_apbrst"); > + ret = PTR_ERR(priv_data->apbrst); > + goto err; > + } > + > + priv_data->usb3_phy = devm_phy_get(dev, "usb3-phy"); > + > + ret = dwc3_xlnx_rst_assert(priv_data->crst); > + if (ret < 0) { > + dev_err(dev, "%s: %d: Failed to assert reset\n", > + __func__, __LINE__); you don't need the function name or the line number here. Just improve your error message a bit: dev_err(dev, "Failed to assert usb3-phy reset\n"); > + goto err; > + } > + > +
[PATCH] ext4: Fix memleak in add_new_gdb
When ext4_journal_get_write_access() fails, we should release n_group_desc, iloc.bh, dind and gdb_bh to prevent memleak. It's the same when ext4_handle_dirty_super() fails, but we don't need to release dind here because it has been released before. Signed-off-by: Dinghao Liu --- fs/ext4/resize.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c index a50b51270ea9..efc0a022ca8e 100644 --- a/fs/ext4/resize.c +++ b/fs/ext4/resize.c @@ -843,8 +843,10 @@ static int add_new_gdb(handle_t *handle, struct inode *inode, BUFFER_TRACE(dind, "get_write_access"); err = ext4_journal_get_write_access(handle, dind); - if (unlikely(err)) + if (unlikely(err)) { ext4_std_error(sb, err); + goto errout; + } /* ext4_reserve_inode_write() gets a reference on the iloc */ err = ext4_reserve_inode_write(handle, inode, &iloc); @@ -899,13 +901,17 @@ static int add_new_gdb(handle_t *handle, struct inode *inode, le16_add_cpu(&es->s_reserved_gdt_blocks, -1); err = ext4_handle_dirty_super(handle, sb); - if (err) + if (err) { ext4_std_error(sb, err); + goto errsuper; + } + return err; errout: + brelse(dind); +errsuper: kvfree(n_group_desc); brelse(iloc.bh); - brelse(dind); brelse(gdb_bh); ext4_debug("leaving with error %d\n", err); -- 2.17.1
Re: [PATCH 1/4] arm: dts: owl-s500: Fix incorrect PPI interrupt specifiers
> "Cristian" == Cristian Ciocaltea writes: > The PPI interrupts for cortex-a9 were incorrectly specified, fix them. > Fixes: fdfe7f4f9d85 ("ARM: dts: Add Actions Semi S500 and LeMaker Guitar") > Signed-off-by: Cristian Ciocaltea Reviewed-by: Peter Korsgaard -- Bye, Peter Korsgaard
[PATCH v3 2/2] Input: gpio_keys - Simplify with dev_err_probe()
Common pattern of handling deferred probe can be simplified with dev_err_probe() and devm_fwnode_gpiod_get_optional(). Less code and the error value gets printed. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Hans de Goede --- Changes since v2: 1. Preserve comment, 2. Include to fix warning on clang (reported by kbuild), 3. Fix use of uninitialized "error" variable. Changes since v1: 1. Use devm_fwnode_gpiod_get_optional --- drivers/input/keyboard/gpio_keys.c | 25 - 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index f2d4e4daa818..160d94b1c2c0 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -494,23 +495,13 @@ static int gpio_keys_setup_key(struct platform_device *pdev, spin_lock_init(&bdata->lock); if (child) { - bdata->gpiod = devm_fwnode_gpiod_get(dev, child, -NULL, GPIOD_IN, desc); - if (IS_ERR(bdata->gpiod)) { - error = PTR_ERR(bdata->gpiod); - if (error == -ENOENT) { - /* -* GPIO is optional, we may be dealing with -* purely interrupt-driven setup. -*/ - bdata->gpiod = NULL; - } else { - if (error != -EPROBE_DEFER) - dev_err(dev, "failed to get gpio: %d\n", - error); - return error; - } - } + /* +* GPIO is optional, we may be dealing with purely +* interrupt-driven setup. +*/ + bdata->gpiod = devm_fwnode_gpiod_get_optional(dev, child, NULL, GPIOD_IN, desc); + if (IS_ERR(bdata->gpiod)) + return dev_err_probe(dev, PTR_ERR(bdata->gpiod), "failed to get gpio\n"); } else if (gpio_is_valid(button->gpio)) { /* * Legacy GPIO number, so request the GPIO here and -- 2.17.1
[PATCH v3 1/2] gpio: Add devm_fwnode_gpiod_get_optional() helpers
Add devm_fwnode_gpiod_get_optional() and devm_fwnode_gpiod_get_index_optional() helpers, similar to regular devm_gpiod optional versions. Drivers getting GPIOs from a firmware node might use it to remove some boilerplate code. Signed-off-by: Krzysztof Kozlowski --- Changes since v2: 1. Return NULL Changes since v1: 1. New patch --- drivers/gpio/gpiolib-devres.c | 71 +++ include/linux/gpio/consumer.h | 30 +++ 2 files changed, 101 insertions(+) diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.c index 7dbce4c4ebdf..f8476f6a65cc 100644 --- a/drivers/gpio/gpiolib-devres.c +++ b/drivers/gpio/gpiolib-devres.c @@ -184,6 +184,37 @@ struct gpio_desc *devm_gpiod_get_from_of_node(struct device *dev, } EXPORT_SYMBOL_GPL(devm_gpiod_get_from_of_node); +/** + * devm_fwnode_gpiod_get_optional - Resource-managed fwnode_gpiod_get_index() + * for optional GPIO + * @dev: GPIO consumer + * @fwnode:firmware node containing GPIO reference + * @con_id:function within the GPIO consumer + * @flags: GPIO initialization flags + * @label: label to attach to the requested GPIO + * + * GPIO descriptors returned from this function are automatically disposed on + * driver detach. + * + * This is equivalent to devm_fwnode_gpiod_get_index(), except that when no + * GPIO with the specified index was assigned to the requested function it will + * return NULL. This is convenient for drivers that need to handle optional + * GPIOs. + * + * On successful request the GPIO pin is configured in accordance with + * provided @flags. + */ +struct gpio_desc *devm_fwnode_gpiod_get_optional(struct device *dev, +struct fwnode_handle *fwnode, +const char *con_id, +enum gpiod_flags flags, +const char *label) +{ + return devm_fwnode_gpiod_get_index_optional(dev, fwnode, con_id, 0, + flags, label); +} +EXPORT_SYMBOL_GPL(devm_fwnode_gpiod_get_optional); + /** * devm_fwnode_gpiod_get_index - get a GPIO descriptor from a given node * @dev: GPIO consumer @@ -226,6 +257,46 @@ struct gpio_desc *devm_fwnode_gpiod_get_index(struct device *dev, } EXPORT_SYMBOL_GPL(devm_fwnode_gpiod_get_index); +/** + * devm_fwnode_gpiod_get_index_optional - Resource-managed fwnode_gpiod_get_index() + *for optional GPIO + * @dev: GPIO consumer + * @fwnode:firmware node containing GPIO reference + * @con_id:function within the GPIO consumer + * @index: index of the GPIO to obtain in the consumer + * @flags: GPIO initialization flags + * @label: label to attach to the requested GPIO + * + * GPIO descriptors returned from this function are automatically disposed on + * driver detach. + * + * This is equivalent to devm_fwnode_gpiod_get_index(), except that when no + * GPIO with the specified index was assigned to the requested function it will + * return NULL. This is convenient for drivers that need to handle optional + * GPIOs. + * + * On successful request the GPIO pin is configured in accordance with + * provided @flags. + */ +struct gpio_desc *devm_fwnode_gpiod_get_index_optional(struct device *dev, + struct fwnode_handle *fwnode, + const char *con_id, int index, + enum gpiod_flags flags, + const char *label) +{ + struct gpio_desc *desc; + + desc = devm_fwnode_gpiod_get_index(dev, fwnode, con_id, index, flags, + label); + if (IS_ERR(desc)) { + if (PTR_ERR(desc) == -ENOENT) + return NULL; + } + + return desc; +} +EXPORT_SYMBOL_GPL(devm_fwnode_gpiod_get_index_optional); + /** * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional() * @dev: GPIO consumer diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index 901aab89d025..7854d80b1e9a 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -183,11 +183,21 @@ struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode, const char *con_id, int index, enum gpiod_flags flags, const char *label); +struct gpio_desc *devm_fwnode_gpiod_get_optional(struct device *dev, +struct fwnode_handle *fwnode, +const char *con_id, +e
Re: [PATCH] ARM: aspeed: g5: Do not set sirq polarity
Hi Joel, > A feature was added to the aspeed vuart driver to configure the vuart > interrupt (sirq) polarity according to the LPC/eSPI strapping register. > > Systems that depend on a active low behaviour (sirq_polarity set to 0) > such as OpenPower boxes also use LPC, so this relationship does not > hold. > > The property was added for a Tyan S7106 system which is not supported > in the kernel tree. Should this or other systems wish to use this > feature of the driver they should add it to the machine specific device > tree. > > Fixes: c791fc76bc72 ("arm: dts: aspeed: Add vuart > aspeed,sirq-polarity-sense...") > Cc: sta...@vger.kernel.org > Signed-off-by: Joel Stanley LGTM. I've tested this on the s2600st, which is strapped for eSPI. All good there too, as expected. Tested-by: Jeremy Kerr and/or: Reviewed-by: Jeremy Kerr Cheers, Jeremy
Re: [PATCH] thermal: sysfs: fall back to vzalloc for cooling device's statistics
On 27-08-20, 14:20, Yue Hu wrote: > Currenly, drivers/video/backlight does not call > thermal_of_cooling_device_register() > to register thermal cooling device. The issue happened in msm-4.19 kernel for > QCOM/Android platform. Backlight in msm-4.19 kernel will register thermal > cooling > device as below: > > +static int bd_cdev_get_max_brightness(struct thermal_cooling_device *cdev, > + unsigned long *state) > +{ > + struct backlight_device *bd = (struct backlight_device *)cdev->devdata; > + > + *state = bd->props.max_brightness; > + > + return 0; > +} > > > +static struct thermal_cooling_device_ops bd_cdev_ops = { > + .get_max_state = bd_cdev_get_max_brightness, > > +static void backlight_cdev_register(struct device *parent, > + struct backlight_device *bd) > +{ > + if (of_find_property(parent->of_node, "#cooling-cells", NULL)) { > + bd->cdev = thermal_of_cooling_device_register(parent->of_node, > + (char *)dev_name(&bd->dev), bd, &bd_cdev_ops); > > And the bd->props.max_brightness is getting from video/backlight/qcom-wled.c. > Maybe > the driver should not assign 1024 to states/max_brightness. I'm not sure > about it. > So i consider to change memory allocation methord. That's the origin of the > patch. Thanks for the details. So this is not about upstream tree, as a rule we aren't going to make changes here for any downstream tree. Now coming back to the downstream driver, I also don't see a point in returning bd->props.max_brightness as the max number of states there. Maybe have 10 states, each occupying bd->props.max_brightness/10 brightness and so you will end up with 10 states only. But yeah, whatever downstream decides on that. -- viresh
[PATCH 1/1] watchdog: remove unneeded inclusion of
There has been no reference to "struct sched_param" since commit 94beddacb53c ("sched,watchdog: Convert to sched_set_fifo()"), so there's no need to include any more, delete it. Signed-off-by: Zhen Lei --- drivers/watchdog/watchdog_dev.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 6798addabd5a067..0f18fa2433310b0 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -43,8 +43,6 @@ #include /* For watchdog specific items */ #include /* For copy_to_user/put_user/... */ -#include /* For struct sched_param */ - #include "watchdog_core.h" #include "watchdog_pretimeout.h" -- 1.8.3
Re: [PATCH net backport 5.6.14-5.8.3 v1] net: openvswitch: introduce common code for flushing flows
On Wed, Aug 26, 2020 at 6:31 PM Greg KH wrote: > > On Tue, Aug 25, 2020 at 01:25:32PM +0800, xiangxia.m@gmail.com wrote: > > From: Tonghao Zhang > > > > [ Upstream commit 77b981c82c1df7c7ad32a046f17f007450b46954 ] > > That is not what this commit is :( > > Please fix up and resend with the correct commit. Sorry for that. v2 is sent, please review. http://patchwork.ozlabs.org/project/netdev/patch/20200827061952.5789-1-xiangxia.m@gmail.com/ > thanks, > > greg k-h -- Best regards, Tonghao
[PATCH net backport to 5.5 - 5.8.3 v2] net: openvswitch: introduce common code for flushing flows
From: Tonghao Zhang [ Upstream commit 1f3a090b9033f69de380c03db3ea1a1015c850cf ] Backport this commit to 5.5 - 5.8.3. To avoid some issues, for example RCU usage warning and double free, we should flush the flows under ovs_lock. This patch refactors table_instance_destroy and introduces table_instance_flow_flush which can be invoked by __dp_destroy or ovs_flow_tbl_flush. Fixes: 50b0e61b32ee ("net: openvswitch: fix possible memleak on destroy flow-table") Reported-by: Johan Knöös Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-August/050489.html Signed-off-by: Tonghao Zhang Reviewed-by: Cong Wang Signed-off-by: David S. Miller --- net/openvswitch/datapath.c | 10 +- net/openvswitch/flow_table.c | 35 +++ net/openvswitch/flow_table.h | 3 +++ 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index e3a37d22539c..15615cae4c89 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -1737,6 +1737,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info) /* Called with ovs_mutex. */ static void __dp_destroy(struct datapath *dp) { + struct flow_table *table = &dp->table; int i; for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++) { @@ -1755,7 +1756,14 @@ static void __dp_destroy(struct datapath *dp) */ ovs_dp_detach_port(ovs_vport_ovsl(dp, OVSP_LOCAL)); - /* RCU destroy the flow table */ + /* Flush sw_flow in the tables. RCU cb only releases resource +* such as dp, ports and tables. That may avoid some issues +* such as RCU usage warning. +*/ + table_instance_flow_flush(table, ovsl_dereference(table->ti), + ovsl_dereference(table->ufid_ti)); + + /* RCU destroy the ports, meters and flow tables. */ call_rcu(&dp->rcu, destroy_dp_rcu); } diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c index 5904e93e5765..1c4fbac53fbd 100644 --- a/net/openvswitch/flow_table.c +++ b/net/openvswitch/flow_table.c @@ -345,19 +345,15 @@ static void table_instance_flow_free(struct flow_table *table, flow_mask_remove(table, flow->mask); } -static void table_instance_destroy(struct flow_table *table, - struct table_instance *ti, - struct table_instance *ufid_ti, - bool deferred) +/* Must be called with OVS mutex held. */ +void table_instance_flow_flush(struct flow_table *table, + struct table_instance *ti, + struct table_instance *ufid_ti) { int i; - if (!ti) - return; - - BUG_ON(!ufid_ti); if (ti->keep_flows) - goto skip_flows; + return; for (i = 0; i < ti->n_buckets; i++) { struct sw_flow *flow; @@ -369,18 +365,16 @@ static void table_instance_destroy(struct flow_table *table, table_instance_flow_free(table, ti, ufid_ti, flow, false); - ovs_flow_free(flow, deferred); + ovs_flow_free(flow, true); } } +} -skip_flows: - if (deferred) { - call_rcu(&ti->rcu, flow_tbl_destroy_rcu_cb); - call_rcu(&ufid_ti->rcu, flow_tbl_destroy_rcu_cb); - } else { - __table_instance_destroy(ti); - __table_instance_destroy(ufid_ti); - } +static void table_instance_destroy(struct table_instance *ti, + struct table_instance *ufid_ti) +{ + call_rcu(&ti->rcu, flow_tbl_destroy_rcu_cb); + call_rcu(&ufid_ti->rcu, flow_tbl_destroy_rcu_cb); } /* No need for locking this function is called from RCU callback or @@ -393,7 +387,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table) free_percpu(table->mask_cache); kfree_rcu(rcu_dereference_raw(table->mask_array), rcu); - table_instance_destroy(table, ti, ufid_ti, false); + table_instance_destroy(ti, ufid_ti); } struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti, @@ -509,7 +503,8 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table) flow_table->count = 0; flow_table->ufid_count = 0; - table_instance_destroy(flow_table, old_ti, old_ufid_ti, true); + table_instance_flow_flush(flow_table, old_ti, old_ufid_ti); + table_instance_destroy(old_ti, old_ufid_ti); return 0; err_free_ti: diff --git a/net/openvswitch/flow_table.h b/net/openvswitch/flow_table.h index 8a5cea6ae111..8ea8fc957377 100644 --- a/net/openvswitch/flow_table.h +++ b/net/openvswitch/flow_table.h @@ -86,4 +86,7 @@ bool ovs_flow_cmp(const struct sw_flow *, const struct sw_flow_match *); void ovs_flow_mask_key(s
Re: [PATCH v2 1/2] drm/amdkfd: Move the ignore_crat check before the CRAT table get
On 2020/8/27 12:28, Felix Kuehling wrote: Am 2020-08-26 um 4:29 a.m. schrieb Hanjun Guo: If the ignore_crat is set to non-zero value, it's no point getting the CRAT table, so just move the ignore_crat check before we get the CRAT table. Signed-off-by: Hanjun Guo --- drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c index 6a250f8..ed77b109 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c @@ -764,6 +764,11 @@ int kfd_create_crat_image_acpi(void **crat_image, size_t *size) *crat_image = NULL; + if (ignore_crat) { A conflicting change in this area was just submitted on Monday to amd-staging-drm-next. You'll need to rebase your patch. It should be straight-forward. ignore_crat was replaced with a function call kfd_ignore_crat(). Other than that, your patch series looks good to me. If I don't see an update from you in a day or two, I'll fix it up myself and add my R-b. Please fix it up by yourself, thanks a lot for the help! Regards Hanjun
Re: [PATCH v2 2/4] brcmfmac: drop unnecessary "fallthrough" comments
27.08.2020 09:23, Gustavo A. R. Silva пишет: > Hi, > > There is a patch that address this, already: > > https://lore.kernel.org/lkml/20200821063758.GA17783@embeddedor/ > > Thanks Okay, then my patch is unnecessary. Thank you!
arch/parisc/kernel/module.c:965:8: warning: Uninitialized variable: err
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 15bc20c6af4ceee97a1f90b43c0e386643c071b4 commit: a1326b17ac03a9012cb3d01e434aacb4d67a416c module/ftrace: handle patchable-function-entry date: 10 months ago compiler: hppa-linux-gcc (GCC) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot cppcheck warnings: (new ones prefixed by >>) >> arch/parisc/kernel/module.c:965:8: warning: Uninitialized variable: err >> [uninitvar] if (err) ^ # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a1326b17ac03a9012cb3d01e434aacb4d67a416c git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout a1326b17ac03a9012cb3d01e434aacb4d67a416c vim +965 arch/parisc/kernel/module.c ^1da177e4c3f41 Linus Torvalds 2005-04-16 884 ^1da177e4c3f41 Linus Torvalds 2005-04-16 885 register_unwind_table(me, sechdrs); ^1da177e4c3f41 Linus Torvalds 2005-04-16 886 ^1da177e4c3f41 Linus Torvalds 2005-04-16 887 /* haven't filled in me->symtab yet, so have to find it ^1da177e4c3f41 Linus Torvalds 2005-04-16 888* ourselves */ ^1da177e4c3f41 Linus Torvalds 2005-04-16 889 for (i = 1; i < hdr->e_shnum; i++) { ^1da177e4c3f41 Linus Torvalds 2005-04-16 890 if(sechdrs[i].sh_type == SHT_SYMTAB fe579c69c6d437 Julia Lawall 2009-08-04 891 && (sechdrs[i].sh_flags & SHF_ALLOC)) { ^1da177e4c3f41 Linus Torvalds 2005-04-16 892 int strindex = sechdrs[i].sh_link; 6ca6366220ed28 Sven Schnelle 2019-06-05 893 symindex = i; ^1da177e4c3f41 Linus Torvalds 2005-04-16 894 /* FIXME: AWFUL HACK ^1da177e4c3f41 Linus Torvalds 2005-04-16 895* The cast is to drop the const from ^1da177e4c3f41 Linus Torvalds 2005-04-16 896* the sechdrs pointer */ ^1da177e4c3f41 Linus Torvalds 2005-04-16 897 symhdr = (Elf_Shdr *)&sechdrs[i]; ^1da177e4c3f41 Linus Torvalds 2005-04-16 898 strtab = (char *)sechdrs[strindex].sh_addr; ^1da177e4c3f41 Linus Torvalds 2005-04-16 899 break; ^1da177e4c3f41 Linus Torvalds 2005-04-16 900 } ^1da177e4c3f41 Linus Torvalds 2005-04-16 901 } ^1da177e4c3f41 Linus Torvalds 2005-04-16 902 6183d68b8b01e3 Sven Schnelle 2019-06-05 903 pr_debug("module %s: strtab %p, symhdr %p\n", ^1da177e4c3f41 Linus Torvalds 2005-04-16 904 me->name, strtab, symhdr); ^1da177e4c3f41 Linus Torvalds 2005-04-16 905 ^1da177e4c3f41 Linus Torvalds 2005-04-16 906 if(me->arch.got_count > MAX_GOTS) { f8fc18a1323c3f Helge Deller 2006-10-18 907 printk(KERN_ERR "%s: Global Offset Table overflow (used %ld, allowed %d)\n", f8fc18a1323c3f Helge Deller 2006-10-18 908 me->name, me->arch.got_count, MAX_GOTS); ^1da177e4c3f41 Linus Torvalds 2005-04-16 909 return -EINVAL; ^1da177e4c3f41 Linus Torvalds 2005-04-16 910 } ^1da177e4c3f41 Linus Torvalds 2005-04-16 911 c298be74492bec Helge Deller 2009-01-01 912 kfree(me->arch.section); c298be74492bec Helge Deller 2009-01-01 913 me->arch.section = NULL; c298be74492bec Helge Deller 2009-01-01 914 ^1da177e4c3f41 Linus Torvalds 2005-04-16 915 /* no symbol table */ ^1da177e4c3f41 Linus Torvalds 2005-04-16 916 if(symhdr == NULL) ^1da177e4c3f41 Linus Torvalds 2005-04-16 917 return 0; ^1da177e4c3f41 Linus Torvalds 2005-04-16 918 ^1da177e4c3f41 Linus Torvalds 2005-04-16 919 oldptr = (void *)symhdr->sh_addr; ^1da177e4c3f41 Linus Torvalds 2005-04-16 920 newptr = oldptr + 1;/* we start counting at 1 */ ^1da177e4c3f41 Linus Torvalds 2005-04-16 921 nsyms = symhdr->sh_size / sizeof(Elf_Sym); 6183d68b8b01e3 Sven Schnelle 2019-06-05 922 pr_debug("OLD num_symtab %lu\n", nsyms); ^1da177e4c3f41 Linus Torvalds 2005-04-16 923 ^1da177e4c3f41 Linus Torvalds 2005-04-16 924 for (i = 1; i < nsyms; i++) { ^1da177e4c3f41 Linus Torvalds 2005-04-16 925 oldptr++; /* note, count starts at 1 so preincrement */ ^1da177e4c3f41 Linus Torvalds 2005-04-16 926 if(strncmp(strtab + oldptr->st_name, ^1da177e4c3f41 Linus Torvalds 2005-04-16 927 ".L", 2) == 0) ^1da177e4c3f41 Linus Torvalds 2005-04-16 928 continue; ^1da177e4c3f41 Linus Torvalds 2005-04-16 929 ^1da177e4c3f41 Linus Torvalds 2005-04-16 930 if(newptr != oldptr) ^1da177e4c3f41 Linus Torvalds 2005-04-16 931 *newptr++ = *oldptr; ^1da177e4c3f41 Linus Torvalds 2005-04-16 932 else ^1da177e4c3f41 Linus Torvalds 2005-04-16 933 newptr++; ^1da177e4c3f41 Linus Torvalds 2005-04-16 934 ^1da177e4c3f41 Linus Torvalds 2005-04-16 935 } ^1da177e4c3f41 Linus Torvalds 2005-04-16 936 nsyms = newptr - (Elf_Sym *)symhdr->sh_addr; 6183d68b8b01e
Re: [PATCH] thermal: sysfs: fall back to vzalloc for cooling device's statistics
On Thu, 27 Aug 2020 10:44:01 +0530 Viresh Kumar wrote: > On 27-08-20, 12:03, Yue Hu wrote: > > Hi Daniel, > > > > Now, i'm just focus on removing the kernel warning based on current code > > logic. > > Commit 8ea229511e06 (thermal: Add cooling device's statistics in sysfs) > > added > > the thermal statistics by viresh and viresh gived the patch an > > acknowledgement > > in anther mail thread. > > > > Hi viresh, > > > > Could you review the patch again about the question above? > > Yeah, I Acked it but the questions raised by Daniel are very valid and must be > answered. Yes, sure. > > I understand that you only cared about fixing the warning, but maybe we need > to > fix the driver and the warning will go away by itself. If you don't want to do > it, then someone who is responsible for the driver should do it. Yes, maybe the patch is not totally correct. maybe the driver has issue. Let's check the driver firstly. > > Was it the acpi_video.c driver that you got the warning from ? I have added > Rafael to the email in case that driver needs getting fixed. > Currenly, drivers/video/backlight does not call thermal_of_cooling_device_register() to register thermal cooling device. The issue happened in msm-4.19 kernel for QCOM/Android platform. Backlight in msm-4.19 kernel will register thermal cooling device as below: +static int bd_cdev_get_max_brightness(struct thermal_cooling_device *cdev, + unsigned long *state) +{ + struct backlight_device *bd = (struct backlight_device *)cdev->devdata; + + *state = bd->props.max_brightness; + + return 0; +} +static struct thermal_cooling_device_ops bd_cdev_ops = { + .get_max_state = bd_cdev_get_max_brightness, +static void backlight_cdev_register(struct device *parent, + struct backlight_device *bd) +{ + if (of_find_property(parent->of_node, "#cooling-cells", NULL)) { + bd->cdev = thermal_of_cooling_device_register(parent->of_node, + (char *)dev_name(&bd->dev), bd, &bd_cdev_ops); And the bd->props.max_brightness is getting from video/backlight/qcom-wled.c. Maybe the driver should not assign 1024 to states/max_brightness. I'm not sure about it. So i consider to change memory allocation methord. That's the origin of the patch. Thank you.
Re: [PATCH v2 2/2] Input: gpio_keys - Simplify with dev_err_probe()
On Thu, Aug 27, 2020 at 12:05:51AM +0300, Andy Shevchenko wrote: > On Wednesday, August 26, 2020, Krzysztof Kozlowski wrote: > > > Common pattern of handling deferred probe can be simplified with > > dev_err_probe() and devm_fwnode_gpiod_get_optional(). Less code and > > the error value gets printed. > > > > Signed-off-by: Krzysztof Kozlowski > > Reviewed-by: Hans de Goede > > > > --- > > > > Changes since v1: > > 1. Use devm_fwnode_gpiod_get_optional > > --- > > drivers/input/keyboard/gpio_keys.c | 21 - > > 1 file changed, 4 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/input/keyboard/gpio_keys.c > > b/drivers/input/keyboard/gpio_keys.c > > index f2d4e4daa818..a07ac6fa25ed 100644 > > --- a/drivers/input/keyboard/gpio_keys.c > > +++ b/drivers/input/keyboard/gpio_keys.c > > @@ -494,23 +494,10 @@ static int gpio_keys_setup_key(struct > > platform_device *pdev, > > spin_lock_init(&bdata->lock); > > > > if (child) { > > - bdata->gpiod = devm_fwnode_gpiod_get(dev, child, > > -NULL, GPIOD_IN, desc); > > - if (IS_ERR(bdata->gpiod)) { > > - error = PTR_ERR(bdata->gpiod); > > - if (error == -ENOENT) { > > - /* > > -* GPIO is optional, we may be dealing with > > -* purely interrupt-driven setup. > > -*/ > > > Can we preserve this comment? Sure. Best regards, Krzysztof
Re: [PATCH v2 2/4] brcmfmac: drop unnecessary "fallthrough" comments
Hi, There is a patch that address this, already: https://lore.kernel.org/lkml/20200821063758.GA17783@embeddedor/ Thanks -- Gustavo On 8/27/20 01:04, Dmitry Osipenko wrote: > There is no need to insert the "fallthrough" comment if there is nothing > in-between of case switches. Hence let's remove the unnecessary comments > in order to make code cleaner a tad. > > Signed-off-by: Dmitry Osipenko > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 2 -- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 -- > 2 files changed, 4 deletions(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > index 1a7ab49295aa..0dc4de2fa9f6 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > @@ -916,9 +916,7 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev) > f2_blksz = SDIO_4373_FUNC2_BLOCKSIZE; > break; > case SDIO_DEVICE_ID_BROADCOM_4359: > - /* fallthrough */ > case SDIO_DEVICE_ID_BROADCOM_4354: > - /* fallthrough */ > case SDIO_DEVICE_ID_BROADCOM_4356: > f2_blksz = SDIO_435X_FUNC2_BLOCKSIZE; > break; > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > index ac3ee93a2378..b16944a898f9 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > @@ -4306,9 +4306,7 @@ static void brcmf_sdio_firmware_callback(struct device > *dev, int err, > CY_43455_MESBUSYCTRL, &err); > break; > case SDIO_DEVICE_ID_BROADCOM_4359: > - /* fallthrough */ > case SDIO_DEVICE_ID_BROADCOM_4354: > - /* fallthrough */ > case SDIO_DEVICE_ID_BROADCOM_4356: > brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes\n", > CY_435X_F2_WATERMARK); >
Re: [PATCH v2 1/2] gpio: Add devm_fwnode_gpiod_get_optional() helpers
On Thu, Aug 27, 2020 at 12:04:13AM +0300, Andy Shevchenko wrote: > On Wednesday, August 26, 2020, Krzysztof Kozlowski wrote: > > > Add devm_fwnode_gpiod_get_optional() and > > devm_fwnode_gpiod_get_index_optional() helpers, similar to regular > > devm_gpiod optional versions. Drivers getting GPIOs from a firmware > > node might use it to remove some boilerplate code. > > > > > Shouldn't it return NULL for !GPIO case? Indeed, good point. Best regards, Krzysztof
Re: [PATCH 1/1] block: Set same_page to false in __bio_try_merge_page if ret is false
Hello Jens, On 8/21/20 11:41 PM, Ritesh Harjani wrote: If we hit the UINT_MAX limit of bio->bi_iter.bi_size and so we are anyway not merging this page in this bio, then it make sense to make same_page also as false before returning. Without this patch, we hit below WARNING in iomap. This mostly happens with very large memory system and / or after tweaking vm dirty threshold params to delay writeback of dirty data. WARNING: CPU: 18 PID: 5130 at fs/iomap/buffered-io.c:74 iomap_page_release+0x120/0x150 CPU: 18 PID: 5130 Comm: fio Kdump: loaded Tainted: GW 5.8.0-rc3 #6 Call Trace: __remove_mapping+0x154/0x320 (unreliable) iomap_releasepage+0x80/0x180 try_to_release_page+0x94/0xe0 invalidate_inode_page+0xc8/0x110 invalidate_mapping_pages+0x1dc/0x540 generic_fadvise+0x3c8/0x450 xfs_file_fadvise+0x2c/0xe0 [xfs] vfs_fadvise+0x3c/0x60 ksys_fadvise64_64+0x68/0xe0 sys_fadvise64+0x28/0x40 system_call_exception+0xf8/0x1c0 system_call_common+0xf0/0x278 Suggested-by: Christoph Hellwig Reported-by: Shivaprasad G Bhat Signed-off-by: Anju T Sudhakar Signed-off-by: Ritesh Harjani Sorry, I forgot to add the fixes tag. I think below commit have introduced this. Let me know if you want me to re-send this patch with below fixes tag. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cc90bc68422318eb8e75b15cd74bc8d538a7df29 -ritesh
Re: [PATCH v2 1/4] brcmfmac: increase F2 watermark for BCM4329
27.08.2020 09:04, Dmitry Osipenko пишет: > This patch fixes SDHCI CRC errors during of RX throughput testing on > BCM4329 chip if SDIO BUS is clocked above 25MHz. In particular the > checksum problem is observed on NVIDIA Tegra20 SoCs. The good watermark > value is borrowed from downstream BCMDHD driver and it's matching to the > value that is already used for the BCM4339 chip, hence let's re-use it > for BCM4329. > > Signed-off-by: Dmitry Osipenko > --- I accidentally missed to add the r-b from Arend that he gave to the v1: Reviewed-by: Arend van Spriel
Re: [PATCH v2 2/2] extcon: ptn5150: Set the VBUS and POLARITY property state
Hi, On 27/8/2020 1:35 pm, Chanwoo Choi wrote: Hi, On 8/27/20 2:17 PM, Ramuthevar, Vadivel MuruganX wrote: Hi, On 27/8/2020 12:51 pm, Chanwoo Choi wrote: Hi, You better to change the 'state' word to 'capability'. Actually, this patch doesn't change the value of property. It set the capability value of property. "Set the VBUS and POLARITY property capability" Thank you for the review comments, sure will update. On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote: From: Ramuthevar Vadivel Murugan Set the VBUS and POLARITY property state. ditto. Need to change the work from 'state' and 'capability'. Noted. Signed-off-by: Ramuthevar Vadivel Murugan --- drivers/extcon/extcon-ptn5150.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c index 8b930050a3f1..b5217a61615c 100644 --- a/drivers/extcon/extcon-ptn5150.c +++ b/drivers/extcon/extcon-ptn5150.c @@ -279,6 +279,12 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c) return ret; } + extcon_set_property_capability(info->edev, EXTCON_USB, + EXTCON_PROP_USB_VBUS); + extcon_set_property_capability(info->edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_VBUS); + extcon_set_property_capability(info->edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_TYPEC_POLARITY); Need to add blank line. Noted. I understood that you set the property capability because of get_flipped() function of your patch[1]. But, I think that you need to change the value of EXTCON_PROP_USB_TYPEC_POLARITY when changing the state of EXTCON_USB_HOST. The polarity property value is always zero regardless of EXTCON_USB_HOST state as following: The get_flipped()[1] returns always the same *flipped value. EXTCON_USB_HOST is 1, EXTCON_PROP_USB_TYPEC_POLARITY is 0 EXTCON_USB_HOST is 0, EXTCON_PROP_USB_TYPEC_POLARITY is 0 by default EXTCON_PROP_USB_TYPEC_POLARITY is 1 If you don't touch the value of EXTCON_PROP_USB_TYPEC_POLARITY property, EXTCON_PROP_USB_TYPEC_POLARITY has default value (0). Ok, thanks! will update the patch, send it. Regards Vadivel If EXTCON_PROP_USB_TYPEC_POLARITY value is not related to any behavior, you don't need to get the property value from extcon consumer driver like drivers/phy/phy-lgm-usb.c. Actually, I don't understand why you don't handle the value of EXTCON_PROP_USB_TYPEC_POLARITY. Or, are there any case of what drivers/phy/phy-lgm-usb.c uses the different extcon device with EXTCON_PROP_USB_TYPEC_POLARITY property in the future? Yes, you're right, user connect the different USB cable then we check polarity, accordingly driver proceeds, thanks! OK. So, do you set the EXTCON_PROP_USB_TYPEC_POLARITY capability for the extensibility in order to use other extcon device on later? yes, that might be the case as well. OK. [1] https://protect2.fireeye.com/v1/url?k=1fb29698-422c0d72-1fb31dd7-0cc47a6cba04-3009aa7184024984&q=1&e=566e4565-e7db-4a90-b036-fc28dbdb742f&u=https%3A%2F%2Fwww.spinics.net%2Flists%2Fdevicetree%2Fmsg371828.html +static int get_flipped(struct tca_apb *ta, bool *flipped) +{ + union extcon_property_value property; + int ret; + + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_TYPEC_POLARITY, &property); + if (ret) { + dev_err(ta->phy.dev, "no polarity property from extcon\n"); + return ret; + } + + *flipped = property.intval; + + return ret; +} Thank you for the gone through my usb-phy patch as well Regards Vadivel /* Initialize PTN5150 device and print vendor id and version id */ ret = ptn5150_init_dev_type(info); if (ret)
Re: [PATCH v3 1/5] arch_topology: validate input frequencies to arch_set_freq_scale()
On 25-08-20, 12:31, Ionela Voinescu wrote: > On Tuesday 25 Aug 2020 at 11:26:18 (+0530), Viresh Kumar wrote: > > On 24-08-20, 22:02, Ionela Voinescu wrote: > > > The current frequency passed to arch_set_freq_scale() could end up > > > being 0, signaling an error in setting a new frequency. Also, if the > > > maximum frequency in 0, this will result in a division by 0 error. > > > > > > Therefore, validate these input values before using them for the > > > setting of the frequency scale factor. > > > > > > Signed-off-by: Ionela Voinescu > > > Cc: Sudeep Holla > > > Cc: Rafael J. Wysocki > > > --- > > > drivers/base/arch_topology.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > > > index 75f72d684294..1aca82fcceb8 100644 > > > --- a/drivers/base/arch_topology.c > > > +++ b/drivers/base/arch_topology.c > > > @@ -33,6 +33,9 @@ void arch_set_freq_scale(struct cpumask *cpus, unsigned > > > long cur_freq, > > > unsigned long scale; > > > int i; > > > > > > + if (!cur_freq || !max_freq) > > > > We should probably use unlikely() here. > > > > Rafael: Shouldn't this have a WARN_ON_ONCE() as well ? > > > > I'll add the unlikely() as it's definitely useful. > > I'm somewhat on the fence about WARN_ON_ONCE() here. Wouldn't it work > better in cpufreq_driver_fast_switch()? It would cover scenarios where > the default arch_set_freq_scale() is used and flag potential hardware > issues with setting frequency that are currently just ignored both here > and in sugov_fast_switch(). I think validation and the WARN (if required) must all happen at the same place. Considering that there can be many callers of a routine, like this one, it is better to put all that in the end function only. Maybe we can add the same in the dummy arch_set_freq_scale() if required. -- viresh
[PATCH v4] workqueue: Warn when work flush own workqueue
From: Qianli Zhao If a workqueue flushes itself then that will lead to a deadlock. Print a warning and a stack trace when this happens. crash> ps 10856 PIDPPID CPU TASK ST COMM 108562 2 ffc873428080 UN [kworker/u16:15] crash> bt 10856 PID: 10856 TASK: ffc873428080 CPU: 2 COMMAND: "kworker/u16:15" #0 [ff80270cb9a0] __switch_to at ff99bba8533c #1 [ff80270cba30] __schedule at ff99bcda18dc #2 [ff80270cba50] schedule at ff99bcda1cdc #3 [ff80270cbaf0] schedule_timeout at ff99bcda6674 #4 [ff80270cbb70] wait_for_common at ff99bcda2c68 #5 [ff80270cbb80] wait_for_completion at ff99bcda2b60 #6 [ff80270cbc30] flush_workqueue at ff99bbad7a60 #7 [ff80270cbc90] drain_workqueue at ff99bbad80fc #8 [ff80270cbcb0] destroy_workqueue at ff99bbad92f8 #9 [ff80270cbda0] dfc_svc_init at ff99bbfbfb6c #10 [ff80270cbdf0] process_one_work at ff99bbadc478 #11 [ff80270cbe50] worker_thread at ff99bbadc9dc #12 [ff80270cbeb0] kthread at ff99bbae1f84 Reported-by: kernel test robot Signed-off-by: Qianli Zhao --- Changes in V4: - Fix an error that when different work item used same workqueue in flush_work, patch will trigger WARNING incorrectly, reported-by "kernel test robot " Changes in V3: - update changelog - update comment Changes in V2: - update changelog - update comment kernel/workqueue.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index c41c3c1..6a8a4e0 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -2585,6 +2585,7 @@ static int rescuer_thread(void *__rescuer) * @target_work: work item being flushed (NULL for workqueue flushes) * * %current is trying to flush the whole @target_wq or @target_work on it. + * If a work queue flushes itself, that will lead to a deadlock. * If @target_wq doesn't have %WQ_MEM_RECLAIM, verify that %current is not * reclaiming memory or running on a workqueue which doesn't have * %WQ_MEM_RECLAIM as that can break forward-progress guarantee leading to @@ -2594,13 +2595,17 @@ static void check_flush_dependency(struct workqueue_struct *target_wq, struct work_struct *target_work) { work_func_t target_func = target_work ? target_work->func : NULL; - struct worker *worker; + struct worker *worker = current_wq_worker(); + + WARN_ONCE(worker && worker->current_pwq->wq == target_wq && + worker->task == current && + (target_work ? worker->current_work == target_work : true), + "workqueue: current work function:%ps is flushing own workqueue:%s", + worker->current_func, target_wq->name); if (target_wq->flags & WQ_MEM_RECLAIM) return; - worker = current_wq_worker(); - WARN_ONCE(current->flags & PF_MEMALLOC, "workqueue: PF_MEMALLOC task %d(%s) is flushing !WQ_MEM_RECLAIM %s:%ps", current->pid, current->comm, target_wq->name, target_func); -- 2.7.4
Re: [PATCH] Revert "powerpc/powernv/idle: Replace CPU feature check with PVR check"
Pratik Sampat writes: > On 26/08/20 2:07 pm, Christophe Leroy wrote: >> Le 26/08/2020 à 10:29, Pratik Rajesh Sampat a écrit : >>> Cpuidle stop state implementation has minor optimizations for P10 >>> where hardware preserves more SPR registers compared to P9. >>> The current P9 driver works for P10, although does few extra >>> save-restores. P9 driver can provide the required power management >>> features like SMT thread folding and core level power savings >>> on a P10 platform. >>> >>> Until the P10 stop driver is available, revert the commit which >>> allows for only P9 systems to utilize cpuidle and blocks all >>> idle stop states for P10. >>> Cpu idle states are enabled and tested on the P10 platform >>> with this fix. >>> >>> This reverts commit 8747bf36f312356f8a295a0c39ff092d65ce75ae. >>> >>> Fixes: 8747bf36f312 ("powerpc/powernv/idle: Replace CPU feature check >>> with PVR check") >>> Signed-off-by: Pratik Rajesh Sampat >>> --- >>> @mpe: This revert would resolve a staging issue wherein the P10 stop >>> driver is not yet ready while cpuidle stop states need not be blocked >>> on 5.9 for Power10 systems which could cause SMT folding related >>> performance issues. >>> >>> The P10 stop driver is in the works here: >>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html >>> >>> arch/powerpc/platforms/powernv/idle.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/powerpc/platforms/powernv/idle.c >>> b/arch/powerpc/platforms/powernv/idle.c >>> index 77513a80cef9..345ab062b21a 100644 >>> --- a/arch/powerpc/platforms/powernv/idle.c >>> +++ b/arch/powerpc/platforms/powernv/idle.c >>> @@ -1223,7 +1223,7 @@ static void __init pnv_probe_idle_states(void) >>> return; >>> } >>> - if (pvr_version_is(PVR_POWER9)) >>> + if (cpu_has_feature(CPU_FTR_ARCH_300)) >> >> Why not something like: >> >> if (pvr_version_is(PVR_POWER9) || pvr_version_is(PVR_POWER10)) >> pnv_power9_idle_init(); > > In order to use PVR_POWER10 I would need to define it under > arch/powerpc/include/asm/reg.h, which is not present in 5.9 yet. > > However, if it okay with @mpe I could split out Nick's P10 stop driver > (https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html) > into two parts: I'll just take this for now, it's the simplest option. cheers
Broadcom WiFi SDIO performance regression after commit "mmc: sdhci: Remove finish_tasklet"
Hello! I was debugging WiFi performance problems on Acer A500 tablet device that has BCM4329 WiFi chip which is connected to NVIDIA Terga20 SoC via SDIO and found that the following commit causes a solid 5-10 Mbit/s of WiFi throughput regression after 5.2 kernel: commit c07a48c2651965e84d35cf193dfc0e5f7892d612 Author: Adrian Hunter Date: Fri Apr 5 15:40:20 2019 +0300 mmc: sdhci: Remove finish_tasklet Remove finish_tasklet. Requests that require DMA-unmapping or sdhci_reset are completed either in the IRQ thread or a workqueue if the completion is not initiated by the IRQ. Reverting the offending commit on top of recent linux-next resolves the problem. Ulf / Adrian, do you have any ideas what could be done in regards to restoring the SDIO performance? Should we just revert the offending commit?
[PATCH v2 1/4] brcmfmac: increase F2 watermark for BCM4329
This patch fixes SDHCI CRC errors during of RX throughput testing on BCM4329 chip if SDIO BUS is clocked above 25MHz. In particular the checksum problem is observed on NVIDIA Tegra20 SoCs. The good watermark value is borrowed from downstream BCMDHD driver and it's matching to the value that is already used for the BCM4339 chip, hence let's re-use it for BCM4329. Signed-off-by: Dmitry Osipenko --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 3c07d1bbe1c6..ac3ee93a2378 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -4278,6 +4278,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err, brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_MESBUSYCTRL, CY_43012_MESBUSYCTRL, &err); break; + case SDIO_DEVICE_ID_BROADCOM_4329: case SDIO_DEVICE_ID_BROADCOM_4339: brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes for 4339\n", CY_4339_F2_WATERMARK); -- 2.27.0
[PATCH v2 3/4] brcmfmac: drop chip id from debug messages
The chip ID was already printed out at the time when debug message about the changed F2 watermark is printed, hence let's drop the unnecessary part of the debug messages. This cleans code a tad and also allows to re-use the F2 watermark debug messages by multiple chips. Suggested-by: Arend van Spriel Signed-off-by: Dmitry Osipenko --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index b16944a898f9..d4989e0cd7be 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -4280,7 +4280,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err, break; case SDIO_DEVICE_ID_BROADCOM_4329: case SDIO_DEVICE_ID_BROADCOM_4339: - brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes for 4339\n", + brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes\n", CY_4339_F2_WATERMARK); brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK, CY_4339_F2_WATERMARK, &err); @@ -4293,7 +4293,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err, CY_4339_MESBUSYCTRL, &err); break; case SDIO_DEVICE_ID_BROADCOM_43455: - brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes for 43455\n", + brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes\n", CY_43455_F2_WATERMARK); brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK, CY_43455_F2_WATERMARK, &err); -- 2.27.0
[PATCH v2 2/4] brcmfmac: drop unnecessary "fallthrough" comments
There is no need to insert the "fallthrough" comment if there is nothing in-between of case switches. Hence let's remove the unnecessary comments in order to make code cleaner a tad. Signed-off-by: Dmitry Osipenko --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 2 -- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 -- 2 files changed, 4 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index 1a7ab49295aa..0dc4de2fa9f6 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -916,9 +916,7 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev) f2_blksz = SDIO_4373_FUNC2_BLOCKSIZE; break; case SDIO_DEVICE_ID_BROADCOM_4359: - /* fallthrough */ case SDIO_DEVICE_ID_BROADCOM_4354: - /* fallthrough */ case SDIO_DEVICE_ID_BROADCOM_4356: f2_blksz = SDIO_435X_FUNC2_BLOCKSIZE; break; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index ac3ee93a2378..b16944a898f9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -4306,9 +4306,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err, CY_43455_MESBUSYCTRL, &err); break; case SDIO_DEVICE_ID_BROADCOM_4359: - /* fallthrough */ case SDIO_DEVICE_ID_BROADCOM_4354: - /* fallthrough */ case SDIO_DEVICE_ID_BROADCOM_4356: brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes\n", CY_435X_F2_WATERMARK); -- 2.27.0
[PATCH v2 0/4] Fixes and improvements for brcmfmac driver
Hello! Recently I was debugging WiFi performance problems on Acer A500 tablet device that got upstreamed recently. This is an older Android device from 2011-2012 that is powered by NVIDIA Tegra20 SoC and it has BCM4329 chip that provides WiFi (SDIO) and Bluetooth (UART). I noticed that WiFi throughput on a recent Linux kernel wasn't as great as it could be in comparison to older 3.18 kernel that is uses downstream BCMDHD driver and this series fixes a major part of the problems that I found. Found problems: 1. The WiFi SDIO pinmux configuration had a bug in Acer A500 device-tree and MMC refused to work if it was clocked above 25MHz and legacy signaling mode was used. After fixing the bug, WiFi SDIO works perfectly well at 50MHz and this increases TX throughput by 5-10 Mbit/s. I already sent out patches that fix this bug to the Tegra ML. 2. There are occasional SDHCI CRC errors if SDIO is clocked above 25Mhz. The "increase F2 watermark" patch fixes this problem. 3. WiFi TX throughput is lower by 10 Mbit/s than it should be using 512B for maximum F2 SDIO block size. Reducing block size to 128B fixes this problem. The "set F2 SDIO block size to 128 bytes" patch addresses this issue. The exact reason why 128B is more efficient than 512B is unknown, this optimization is borrowed from the BCMDHD driver. 4. While I was bisecting upstream kernel, I found that WiFi RX/TX throughput dropped by 5-10 Mbit/s after 5.2 kernel and reverting the following commit from linux-next resolves the problem: commit c07a48c2651965e84d35cf193dfc0e5f7892d612 Author: Adrian Hunter Date: Fri Apr 5 15:40:20 2019 +0300 mmc: sdhci: Remove finish_tasklet I'll send a separate email for discussing this problem. After fixing all the above problems, I'm now getting a solid 40 Mbit/s up/down on Acer A500 on a recent linux-next in comparison to 15 Mbit/s that I was getting before the fixes. Big thanks to Wright Feng who helped me to find and fix some of the problems! Changelog: v2: - Added "drop chip id from debug messages" as was requested by Arend Van Spriel in the review comment to v1 of the "increase F2 watermark" patch. - Added patches that remove unnecessary "fallthrough" comments and change F2 SDIO block size to 128 bytes for BCM4329. Dmitry Osipenko (4): brcmfmac: increase F2 watermark for BCM4329 brcmfmac: drop unnecessary "fallthrough" comments brcmfmac: drop chip id from debug messages brcmfmac: set F2 SDIO block size to 128 bytes for BCM4329 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 6 -- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 +++ 2 files changed, 7 insertions(+), 6 deletions(-) -- 2.27.0
[PATCH v2 4/4] brcmfmac: set F2 SDIO block size to 128 bytes for BCM4329
Setting F2 block size to 128 bytes for BCM4329 allows to significantly improve RX throughput on NVIDIA Tegra20. Before this change the throughput was capped to 30 Mbit/s on Tegra, now throughput is at 40 Mbit/s, which is a maximum throughput for the BCM4329 chip. The F2 block size is borrowed from the downstream BCMDHD driver. The comment in the BCMDHD driver says that 128B improves throughput and turns out that it works for the brcmfmac as well. Signed-off-by: Dmitry Osipenko --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index 0dc4de2fa9f6..318bd00bf94f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -45,6 +45,7 @@ #define SDIO_FUNC2_BLOCKSIZE 512 #define SDIO_4373_FUNC2_BLOCKSIZE 256 #define SDIO_435X_FUNC2_BLOCKSIZE 256 +#define SDIO_4329_FUNC2_BLOCKSIZE 128 /* Maximum milliseconds to wait for F2 to come up */ #define SDIO_WAIT_F2RDY3000 @@ -920,6 +921,9 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev) case SDIO_DEVICE_ID_BROADCOM_4356: f2_blksz = SDIO_435X_FUNC2_BLOCKSIZE; break; + case SDIO_DEVICE_ID_BROADCOM_4329: + f2_blksz = SDIO_4329_FUNC2_BLOCKSIZE; + break; default: break; } -- 2.27.0
Re: [PATCH bpf-next v3 2/4] selftests/bpf: add test for freplace program with write access
On Tue, Aug 25, 2020 at 4:21 PM Udip Pant wrote: > > This adds a selftest that tests the behavior when a freplace target program > attempts to make a write access on a packet. The expectation is that the read > or write > access is granted based on the program type of the linked program and > not itself (which is of type, for e.g., BPF_PROG_TYPE_EXT). > > This test fails without the associated patch on the verifier. > > Signed-off-by: Udip Pant > --- > .../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 1 + > .../selftests/bpf/progs/fexit_bpf2bpf.c | 27 +++ > .../selftests/bpf/progs/test_pkt_access.c | 20 ++ > 3 files changed, 48 insertions(+) > [...] > +__attribute__ ((noinline)) > +int test_pkt_write_access_subprog(struct __sk_buff *skb, __u32 off) > +{ > + void *data = (void *)(long)skb->data; > + void *data_end = (void *)(long)skb->data_end; > + struct tcphdr *tcp = NULL; > + > + if (off > sizeof(struct ethhdr) + sizeof(struct ipv6hdr)) > + return -1; > + > + tcp = data + off; > + if (tcp + 1 > data_end) > + return -1; > + /* make modification to the packet data */ > + tcp->check++; Just FYI for all BPF contributors. This change makes test_pkt_access BPF program to fail on kernel 5.5, which (the kernel) we use as part libbpf CI testing. test_pkt_access.o in turn makes few different selftests (see [0] for details) to fail on 5.5 (because test_pkt_access is used as one of BPF objects loaded as part of those selftests). This is ok, I'm blacklisting (at least temporarily) those tests, but I wanted to bring up this issue, as it did happen before and will keep happening in the future and will constantly decrease test coverage for older kernels that libbpf CI performs. I propose that when we introduce new features (like new fields in a BPF program's context or something along those lines) and want to test them, we should lean towards creating new tests, not modify existing ones. This will allow all already working selftests to keep working for older kernels. Does this sound reasonable as an approach? As for this particular breakage, I'd appreciate someone taking a look at the problem and checking if it's some new feature that's not present in 5.5 or just Clang/verifier interactions (32-bit pointer arithmetic, is this a new issue?). If it's something fixable, it would be nice to fix and restore 5.5 tests. Thanks! [0] https://travis-ci.com/github/libbpf/libbpf/jobs/378226438 Verifier complains about: ; if (test_pkt_write_access_subprog(skb, (void *)tcp - data)) 57: (79) r1 = *(u64 *)(r10 -8) 58: (bc) w2 = w1 59: (1c) w2 -= w9 R2 32-bit pointer arithmetic prohibited processed 198 insns (limit 100) max_states_per_insn 1 total_states 8 peak_states 8 mark_read 7 > + return 0; > +} > + > SEC("classifier/test_pkt_access") > int test_pkt_access(struct __sk_buff *skb) > { > @@ -117,6 +135,8 @@ int test_pkt_access(struct __sk_buff *skb) > if (test_pkt_access_subprog3(3, skb) != skb->len * 3 * skb->ifindex) > return TC_ACT_SHOT; > if (tcp) { > + if (test_pkt_write_access_subprog(skb, (void *)tcp - data)) > + return TC_ACT_SHOT; > if (((void *)(tcp) + 20) > data_end || proto != 6) > return TC_ACT_SHOT; > barrier(); /* to force ordering of checks */ > -- > 2.24.1 >
[PATCH 1/1] iommu/vt-d: Use device numa domain if RHSA is missing
If there are multiple NUMA domains but the RHSA is missing in ACPI/DMAR table, we could default to the device NUMA domain as fall back. This also benefits the vIOMMU use case where only a single vIOMMU is exposed, hence no RHSA will be present but device numa domain can be correct. Cc: Jacob Pan Cc: Kevin Tian Cc: Ashok Raj Signed-off-by: Lu Baolu --- drivers/iommu/intel/iommu.c | 31 +-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index e0516d64d7a3..bce158468abf 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -700,12 +700,41 @@ static int domain_update_iommu_superpage(struct dmar_domain *domain, return fls(mask); } +static int domain_update_device_node(struct dmar_domain *domain) +{ + struct device_domain_info *info; + int nid = NUMA_NO_NODE; + + assert_spin_locked(&device_domain_lock); + + if (list_empty(&domain->devices)) + return NUMA_NO_NODE; + + list_for_each_entry(info, &domain->devices, link) { + if (!info->dev) + continue; + + nid = dev_to_node(info->dev); + if (nid != NUMA_NO_NODE) + break; + } + + return nid; +} + /* Some capabilities may be different across iommus */ static void domain_update_iommu_cap(struct dmar_domain *domain) { domain_update_iommu_coherency(domain); domain->iommu_snooping = domain_update_iommu_snooping(NULL); domain->iommu_superpage = domain_update_iommu_superpage(domain, NULL); + + /* +* If RHSA is missing, we should default to the device numa domain +* as fall back. +*/ + if (domain->nid == NUMA_NO_NODE) + domain->nid = domain_update_device_node(domain); } struct context_entry *iommu_context_addr(struct intel_iommu *iommu, u8 bus, @@ -5086,8 +5115,6 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type) if (type == IOMMU_DOMAIN_DMA) intel_init_iova_domain(dmar_domain); - domain_update_iommu_cap(dmar_domain); - domain = &dmar_domain->domain; domain->geometry.aperture_start = 0; domain->geometry.aperture_end = -- 2.17.1
Re: [Resend PATCH 1/6] mm/memcg: warning on !memcg after readahead page charged
On Mon, 24 Aug 2020, Hugh Dickins wrote: > On Tue, 25 Aug 2020, Alex Shi wrote: > > reproduce using our linux-mm random bug collection on NUMA systems. > > >> > > >> OK, I must have missed that this was on ppc. The order makes more sense > > >> now. I will have a look at this next week. > > > > > > OK, so I've had a look and I know what's going on there. The > > > move_pages12 is migrating hugetlb pages. Those are not charged to any > > > memcg. We have completely missed this case. There are two ways going > > > around that. Drop the warning and update the comment so that we do not > > > forget about that or special case hugetlb pages. > > > > > > I think the first option is better. > > > > > > > > > Hi Michal, > > > > Compare to ignore the warning which is designed to give, seems addressing > > the hugetlb out of charge issue is a better solution, otherwise the memcg > > memory usage is out of control on hugetlb, is that right? I agree: it seems that hugetlb is not participating in memcg and lrus, so it should not even be calling mem_cgroup_migrate(). That happens because hugetlb finds the rest of migrate_page_states() useful, but maybe there just needs to be an "if (!PageHuge(page))" or "if (!PageHuge(newpage))" before its call to mem_cgroup_migrate() - but I have not yet checked whether either of those actually works. The same could be done inside mem_cgroup_migrate() instead, but it just seems wrong for hugetlb to be getting that far, if it has no other reason to enter mm/memcontrol.c. > > Please don't suppose that this is peculiar to hugetlb: I'm not > testing hugetlb at all (sorry), but I see the VM_WARN_ON_ONCE from > mem_cgroup_page_lruvec(), and from mem_cgroup_migrate(), and from > mem_cgroup_swapout(). > > In all cases seen on a PageAnon page (well, in one case PageKsm). > And not related to THP either: seen also on machine incapable of THP. > > Maybe there's an independent change in 5.9-rc that's defeating > expectations here, or maybe they were never valid. Worth > investigating, even though the patch is currently removed, > to find out why expectations were wrong. It was very well worth investigating. And at the time of writing the above, I thought it was coming up very quickly on all machines, but in fact it only came up quickly on the one exercising KSM; on the other machines it took about an hour to appear, so no wonder that you and others had not already seen it. While I'd prefer to spring the answer on you all in the patch that fixes it, there's something more there that I don't fully understand yet, and want to sort out before posting; so I'd better not keep you in suspense... we broke the memcg charging of ksm_might_need_to_copy() pages a couple of releases ago, and not noticed until your warning. What's surprising is that the same bug can affect PageAnon pages too, even when there's been no KSM involved whatsoever. I put in the KSM fix, set all the machines running, expecting to get more info on the PageAnon instances, but all of them turned out to be fixed. > > You'll ask me for more info, stacktraces etc, and I'll say sorry, > no time today. Please try the swapping tests I sent before. > > And may I say, the comment > /* Readahead page is charged too, to see if other page uncharged */ > is nonsensical to me, and much better deleted: maybe it would make > some sense if the reader could see the comment it replaces - as > they can in the patch - but not in the resulting source file. I stand by that remark; but otherwise, I think this was a helpful commit that helped to identify a bug, just as it was intended to do. (I say "helped to" because its warnings alerted, but did not point to the culprit: I had to add another in lru_cache_add() to find it.) Hugh
Re: [RFC v2] perf/core: Fixes hung issue on perf stat command during cpu hotplug
* Kajol Jain [2020-08-26 20:24:11]: > Commit 2ed6edd33a21 ("perf: Add cond_resched() to task_function_call()") > added assignment of ret value as -EAGAIN in case function > call to 'smp_call_function_single' fails. > For non-zero ret value, it did > 'ret = !ret ? data.ret : -EAGAIN;', which always > assign -EAGAIN to ret and make second if condition useless. > > In scenarios like when executing a perf stat with --per-thread option, and > if any of the monitoring cpu goes offline, the 'smp_call_function_single' > function could return -ENXIO, and with the above check, > task_function_call hung and increases CPU > usage (because of repeated 'smp_call_function_single()') > > Recration scenario: > # perf stat -a --per-thread && (offline a CPU ) > > Patch here removes the tertiary condition added as part of that > commit and added a check for NULL and -EAGAIN. > > Fixes: 2ed6edd33a21("perf: Add cond_resched() to task_function_call()") > Signed-off-by: Kajol Jain > Reported-by: Srikar Dronamraju Tested-by: Srikar Dronamraju -- Thanks and Regards Srikar Dronamraju
BUG: stack guard page was hit in __zone_watermark_ok
Hello, syzbot found the following issue on: HEAD commit:85eb5bc3 net: atheros: switch from 'pci_' to 'dma_' API git tree: net-next console output: https://syzkaller.appspot.com/x/log.txt?x=14e3213990 kernel config: https://syzkaller.appspot.com/x/.config?x=a0437fdd630bee11 dashboard link: https://syzkaller.appspot.com/bug?extid=144d1f35995353c5779c compiler: gcc (GCC) 10.1.0-syz 20200507 Unfortunately, I don't have any reproducer for this issue yet. IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+144d1f35995353c57...@syzkaller.appspotmail.com BUG: stack guard page was hit at b2f481cf (stack is 9cbf9546..99742d14) kernel stack overflow (double-fault): [#1] PREEMPT SMP KASAN CPU: 1 PID: 29993 Comm: syz-executor.4 Not tainted 5.9.0-rc1-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:instrument_atomic_read include/linux/instrumented.h:56 [inline] RIP: 0010:atomic64_read include/asm-generic/atomic-instrumented.h:837 [inline] RIP: 0010:atomic_long_read include/asm-generic/atomic-long.h:29 [inline] RIP: 0010:zone_page_state include/linux/vmstat.h:217 [inline] RIP: 0010:__zone_watermark_unusable_free mm/page_alloc.c:3508 [inline] RIP: 0010:__zone_watermark_ok+0x23f/0x3f0 mm/page_alloc.c:3529 Code: c4 28 31 c0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8d bc 24 98 06 00 00 be 08 00 00 00 4c 89 4c 24 20 4c 89 54 24 18 89 54 24 10 <4c> 89 44 24 08 48 89 3c 24 e8 33 78 08 00 48 8b 3c 24 48 b8 00 00 RSP: 0018:c900169f7ff0 EFLAGS: 00010046 RAX: dc00 RBX: 0001 RCX: 0001 RDX: RSI: 0008 RDI: 88812fffc498 RBP: 0001 R08: R09: R10: R11: 0001 R12: 88812fffbe00 R13: 0040 R14: 0002 R15: FS: 7f98bc4db700() GS:8880ae70() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: c900169f7fe8 CR3: 0002063a9000 CR4: 001506e0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: zone_watermark_fast mm/page_alloc.c:3612 [inline] get_page_from_freelist+0x102f/0x37f0 mm/page_alloc.c:3785 __alloc_pages_slowpath.constprop.0+0x322/0x2860 mm/page_alloc.c:4592 __alloc_pages_nodemask+0x62c/0x790 mm/page_alloc.c:4901 __alloc_pages include/linux/gfp.h:509 [inline] __alloc_pages_node include/linux/gfp.h:522 [inline] kmem_getpages mm/slab.c:1376 [inline] cache_grow_begin+0x71/0x430 mm/slab.c:2590 cache_alloc_refill+0x27b/0x340 mm/slab.c:2962 cache_alloc mm/slab.c:3045 [inline] cache_alloc mm/slab.c:3028 [inline] slab_alloc_node mm/slab.c:3241 [inline] kmem_cache_alloc_node_trace+0x3de/0x400 mm/slab.c:3592 __do_kmalloc_node mm/slab.c:3614 [inline] __kmalloc_node_track_caller+0x38/0x60 mm/slab.c:3629 __kmalloc_reserve net/core/skbuff.c:142 [inline] __alloc_skb+0xae/0x550 net/core/skbuff.c:210 alloc_skb include/linux/skbuff.h:1085 [inline] nlmsg_new include/net/netlink.h:944 [inline] rtmsg_ifinfo_build_skb+0x72/0x1a0 net/core/rtnetlink.c:3804 rtmsg_ifinfo_event net/core/rtnetlink.c:3840 [inline] rtmsg_ifinfo_event net/core/rtnetlink.c:3831 [inline] rtnetlink_event+0x123/0x1d0 net/core/rtnetlink.c:5614 notifier_call_chain+0xb5/0x200 kernel/notifier.c:83 call_netdevice_notifiers_info+0xb5/0x130 net/core/dev.c:2033 call_netdevice_notifiers_extack net/core/dev.c:2045 [inline] call_netdevice_notifiers net/core/dev.c:2059 [inline] netdev_features_change net/core/dev.c:1444 [inline] netdev_sync_lower_features net/core/dev.c:9371 [inline] __netdev_update_features+0x88d/0x1360 net/core/dev.c:9502 netdev_change_features+0x61/0xb0 net/core/dev.c:9574 bond_compute_features+0x562/0xa80 drivers/net/bonding/bond_main.c:1308 bond_slave_netdev_event drivers/net/bonding/bond_main.c:3375 [inline] bond_netdev_event+0x871/0xb80 drivers/net/bonding/bond_main.c:3415 notifier_call_chain+0xb5/0x200 kernel/notifier.c:83 call_netdevice_notifiers_info+0xb5/0x130 net/core/dev.c:2033 call_netdevice_notifiers_extack net/core/dev.c:2045 [inline] call_netdevice_notifiers net/core/dev.c:2059 [inline] netdev_features_change net/core/dev.c:1444 [inline] netdev_sync_lower_features net/core/dev.c:9371 [inline] __netdev_update_features+0x88d/0x1360 net/core/dev.c:9502 netdev_change_features+0x61/0xb0 net/core/dev.c:9574 bond_compute_features+0x562/0xa80 drivers/net/bonding/bond_main.c:1308 bond_slave_netdev_event drivers/net/bonding/bond_main.c:3375 [inline] bond_netdev_event+0x871/0xb80 drivers/net/bonding/bond_main.c:3415 notifier_call_chain+0xb5/0x200 kernel/notifier.c:83 call_netdevice_notifiers_info+0xb5/0x130 net/core/dev.c:2033 call_netdevice_notifiers_extack net/core/dev.c:2045 [inline] call_netdevice_notifiers net/core/dev.c:20
aarch64-linux-ld: sound/soc/fsl/fsl_asrc_dma.c:211: undefined reference to `dma_request_slave_channel'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 15bc20c6af4ceee97a1f90b43c0e386643c071b4 commit: 41206a073ceebc517245677a19f52ba6379b33a9 Merge v5.8-rc6 into drm-next date: 5 weeks ago config: arm64-randconfig-r013-20200826 (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 41206a073ceebc517245677a19f52ba6379b33a9 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): drivers/spi/spi-dw-dma.c:45: undefined reference to `dma_get_slave_caps' aarch64-linux-ld: drivers/spi/spi-dw-dma.c:53: undefined reference to `dma_get_slave_caps' aarch64-linux-ld: drivers/spi/spi-dw-dma.o: in function `dw_spi_dma_init_generic': drivers/spi/spi-dw-dma.c:110: undefined reference to `dma_request_slave_channel' aarch64-linux-ld: drivers/spi/spi-dw-dma.c:114: undefined reference to `dma_request_slave_channel' aarch64-linux-ld: drivers/spi/spi-dw-dma.c:116: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-dw-dma.o: in function `dw_spi_dma_init_mfld': drivers/spi/spi-dw-dma.c:82: undefined reference to `__dma_request_channel' aarch64-linux-ld: drivers/spi/spi-dw-dma.c:88: undefined reference to `__dma_request_channel' aarch64-linux-ld: drivers/spi/spi-dw-dma.c:102: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-dw-dma.o: in function `dw_spi_dma_exit': drivers/spi/spi-dw-dma.c:135: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-dw-dma.c:140: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-omap2-mcspi.o: in function `omap2_mcspi_release_dma': drivers/spi/spi-omap2-mcspi.c:1017: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-omap2-mcspi.c:1021: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-omap2-mcspi.o: in function `omap2_mcspi_request_dma': drivers/spi/spi-omap2-mcspi.c:991: undefined reference to `dma_request_chan' aarch64-linux-ld: drivers/spi/spi-omap2-mcspi.c:983: undefined reference to `dma_request_chan' aarch64-linux-ld: drivers/spi/spi-omap2-mcspi.c:996: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pl022.o: in function `pl022_dma_remove': drivers/spi/spi-pl022.c:1211: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pl022.c:1213: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pl022.o: in function `pl022_dma_autoprobe': drivers/spi/spi-pl022.c:1161: undefined reference to `dma_request_chan' aarch64-linux-ld: drivers/spi/spi-pl022.c:1169: undefined reference to `dma_request_chan' aarch64-linux-ld: drivers/spi/spi-pl022.c:1186: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pl022.c:1189: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pl022.o: in function `pl022_dma_probe': drivers/spi/spi-pl022.c:1117: undefined reference to `__dma_request_channel' aarch64-linux-ld: drivers/spi/spi-pl022.c:1125: undefined reference to `__dma_request_channel' aarch64-linux-ld: drivers/spi/spi-pl022.c:1144: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pl022.c:1146: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pxa2xx-dma.o: in function `dma_request_slave_channel_compat': include/linux/dmaengine.h:1569: undefined reference to `dma_request_slave_channel' aarch64-linux-ld: include/linux/dmaengine.h:1569: undefined reference to `dma_request_slave_channel' aarch64-linux-ld: include/linux/dmaengine.h:1576: undefined reference to `__dma_request_channel' aarch64-linux-ld: include/linux/dmaengine.h:1576: undefined reference to `__dma_request_channel' aarch64-linux-ld: drivers/spi/spi-pxa2xx-dma.o: in function `pxa2xx_spi_dma_setup': drivers/spi/spi-pxa2xx-dma.c:209: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pxa2xx-dma.o: in function `pxa2xx_spi_dma_release': drivers/spi/spi-pxa2xx-dma.c:223: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-pxa2xx-dma.c:228: undefined reference to `dma_release_channel' aarch64-linux-ld: drivers/spi/spi-unip
[PATCH 1/2] hwmon: (k10temp) Create common functions and macros for Zen CPU families
Many SMN thermal registers in Zen CPU families are common across different generations. For long-term code maintenance, it is better to rename these macro and function names to Zen. Signed-off-by: Wei Huang --- drivers/hwmon/k10temp.c | 56 + 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index 8f12995ec133..f3addb97b021 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -73,22 +73,24 @@ static DEFINE_MUTEX(nb_smu_ind_mutex); #define F15H_M60H_HARDWARE_TEMP_CTRL_OFFSET0xd8200c64 #define F15H_M60H_REPORTED_TEMP_CTRL_OFFSET0xd8200ca4 -/* F17h M01h Access througn SMN */ -#define F17H_M01H_REPORTED_TEMP_CTRL_OFFSET0x00059800 +/* Common for Zen CPU families (Family 17h and 18h) */ +#define ZEN_REPORTED_TEMP_CTRL_OFFSET 0x00059800 -#define F17H_M70H_CCD_TEMP(x) (0x00059954 + ((x) * 4)) -#define F17H_M70H_CCD_TEMP_VALID BIT(11) -#define F17H_M70H_CCD_TEMP_MASKGENMASK(10, 0) +#define ZEN_CCD_TEMP(x)(0x00059954 + ((x) * 4)) +#define ZEN_CCD_TEMP_VALID BIT(11) +#define ZEN_CCD_TEMP_MASK GENMASK(10, 0) -#define F17H_M01H_SVI 0x0005A000 -#define F17H_M01H_SVI_TEL_PLANE0 (F17H_M01H_SVI + 0xc) -#define F17H_M01H_SVI_TEL_PLANE1 (F17H_M01H_SVI + 0x10) +#define ZEN_CUR_TEMP_SHIFT 21 +#define ZEN_CUR_TEMP_RANGE_SEL_MASKBIT(19) -#define CUR_TEMP_SHIFT 21 -#define CUR_TEMP_RANGE_SEL_MASKBIT(19) +#define ZEN_SVI_BASE 0x0005A000 -#define CFACTOR_ICORE 100 /* 1A / LSB */ -#define CFACTOR_ISOC 25 /* 0.25A / LSB */ +/* F17h thermal registers through SMN */ +#define F17H_M01H_SVI_TEL_PLANE0 (ZEN_SVI_BASE + 0xc) +#define F17H_M01H_SVI_TEL_PLANE1 (ZEN_SVI_BASE + 0x10) + +#define F17H_CFACTOR_ICORE 100 /* 1A / LSB */ +#define F17H_CFACTOR_ISOC 25 /* 0.25A / LSB */ struct k10temp_data { struct pci_dev *pdev; @@ -168,10 +170,10 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 *regval) F15H_M60H_REPORTED_TEMP_CTRL_OFFSET, regval); } -static void read_tempreg_nb_f17(struct pci_dev *pdev, u32 *regval) +static void read_tempreg_nb_zen(struct pci_dev *pdev, u32 *regval) { amd_smn_read(amd_pci_dev_to_node_id(pdev), -F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval); +ZEN_REPORTED_TEMP_CTRL_OFFSET, regval); } static long get_raw_temp(struct k10temp_data *data) @@ -180,7 +182,7 @@ static long get_raw_temp(struct k10temp_data *data) long temp; data->read_tempreg(data->pdev, ®val); - temp = (regval >> CUR_TEMP_SHIFT) * 125; + temp = (regval >> ZEN_CUR_TEMP_SHIFT) * 125; if (regval & data->temp_adjust_mask) temp -= 49000; return temp; @@ -288,8 +290,8 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel, break; case 2 ... 9: /* Tccd{1-8} */ amd_smn_read(amd_pci_dev_to_node_id(data->pdev), -F17H_M70H_CCD_TEMP(channel - 2), ®val); - *val = (regval & F17H_M70H_CCD_TEMP_MASK) * 125 - 49000; +ZEN_CCD_TEMP(channel - 2), ®val); + *val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000; break; default: return -EOPNOTSUPP; @@ -438,7 +440,7 @@ static int svi_show(struct seq_file *s, void *unused) { struct k10temp_data *data = s->private; - k10temp_smn_regs_show(s, data->pdev, F17H_M01H_SVI, 32); + k10temp_smn_regs_show(s, data->pdev, ZEN_SVI_BASE, 32); return 0; } DEFINE_SHOW_ATTRIBUTE(svi); @@ -448,7 +450,7 @@ static int thm_show(struct seq_file *s, void *unused) struct k10temp_data *data = s->private; k10temp_smn_regs_show(s, data->pdev, - F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, 256); + ZEN_REPORTED_TEMP_CTRL_OFFSET, 256); return 0; } DEFINE_SHOW_ATTRIBUTE(thm); @@ -528,8 +530,8 @@ static void k10temp_get_ccd_support(struct pci_dev *pdev, for (i = 0; i < limit; i++) { amd_smn_read(amd_pci_dev_to_node_id(pdev), -F17H_M70H_CCD_TEMP(i), ®val); - if (regval & F17H_M70H_CCD_TEMP_VALID) +ZEN_CCD_TEMP(i), ®val); + if (regval & ZEN_CCD_TEMP_VALID) data->show_temp |= BIT(TCCD_BIT(i)); } } @@ -565,8 +56
[PATCH 2/2] hwmon: (k10temp) Define SVI telemetry and current factors for Zen2 CPUs
The voltage telemetry registers for Zen2 are different from Zen1. Also the factors of CPU current values are changed on Zen2. Add new definitions for these register. Signed-off-by: Wei Huang --- drivers/hwmon/k10temp.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index f3addb97b021..de9f68570a4f 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -88,9 +88,13 @@ static DEFINE_MUTEX(nb_smu_ind_mutex); /* F17h thermal registers through SMN */ #define F17H_M01H_SVI_TEL_PLANE0 (ZEN_SVI_BASE + 0xc) #define F17H_M01H_SVI_TEL_PLANE1 (ZEN_SVI_BASE + 0x10) +#define F17H_M31H_SVI_TEL_PLANE0 (ZEN_SVI_BASE + 0x14) +#define F17H_M31H_SVI_TEL_PLANE1 (ZEN_SVI_BASE + 0x10) -#define F17H_CFACTOR_ICORE 100 /* 1A / LSB */ -#define F17H_CFACTOR_ISOC 25 /* 0.25A / LSB */ +#define F17H_M01H_CFACTOR_ICORE100 /* 1A / LSB */ +#define F17H_M01H_CFACTOR_ISOC 25 /* 0.25A / LSB */ +#define F17H_M31H_CFACTOR_ICORE100 /* 1A / LSB */ +#define F17H_M31H_CFACTOR_ISOC 31 /* 0.31A / LSB */ struct k10temp_data { struct pci_dev *pdev; @@ -580,17 +584,17 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id) data->show_current = !is_threadripper() && !is_epyc(); data->svi_addr[0] = F17H_M01H_SVI_TEL_PLANE0; data->svi_addr[1] = F17H_M01H_SVI_TEL_PLANE1; - data->cfactor[0] = F17H_CFACTOR_ICORE; - data->cfactor[1] = F17H_CFACTOR_ISOC; + data->cfactor[0] = F17H_M01H_CFACTOR_ICORE; + data->cfactor[1] = F17H_M01H_CFACTOR_ISOC; k10temp_get_ccd_support(pdev, data, 4); break; case 0x31: /* Zen2 Threadripper */ case 0x71: /* Zen2 */ data->show_current = !is_threadripper() && !is_epyc(); - data->cfactor[0] = F17H_CFACTOR_ICORE; - data->cfactor[1] = F17H_CFACTOR_ISOC; - data->svi_addr[0] = F17H_M01H_SVI_TEL_PLANE1; - data->svi_addr[1] = F17H_M01H_SVI_TEL_PLANE0; + data->cfactor[0] = F17H_M31H_CFACTOR_ICORE; + data->cfactor[1] = F17H_M31H_CFACTOR_ISOC; + data->svi_addr[0] = F17H_M31H_SVI_TEL_PLANE0; + data->svi_addr[1] = F17H_M31H_SVI_TEL_PLANE1; k10temp_get_ccd_support(pdev, data, 8); break; } -- 2.25.2
RE: [PATCH v2 1/1] iommu/vt-d: Serialize IOMMU GCMD register modifications
> From: Lu Baolu > Sent: Thursday, August 27, 2020 12:25 PM > > The VT-d spec requires (10.4.4 Global Command Register, GCMD_REG > General > Description) that: > > If multiple control fields in this register need to be modified, software > must serialize the modifications through multiple writes to this register. > > However, in irq_remapping.c, modifications of IRE and CFI are done in one > write. We need to do two separate writes with STS checking after each. > > Fixes: af8d102f999a4 ("x86/intel/irq_remapping: Clean up x2apic opt-out > security warning mess") > Cc: Andy Lutomirski > Cc: Jacob Pan > Cc: Kevin Tian > Cc: Ashok Raj > Signed-off-by: Lu Baolu > --- > drivers/iommu/intel/irq_remapping.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > Change log: > v1->v2: > - v1 posted here > https://lore.kernel.org/linux-iommu/20200826025825.2322-1- > baolu...@linux.intel.com/; > - Add status check before disabling CFI. (Kevin) > > diff --git a/drivers/iommu/intel/irq_remapping.c > b/drivers/iommu/intel/irq_remapping.c > index 9564d23d094f..7552bb7e92c8 100644 > --- a/drivers/iommu/intel/irq_remapping.c > +++ b/drivers/iommu/intel/irq_remapping.c > @@ -507,12 +507,19 @@ static void iommu_enable_irq_remapping(struct > intel_iommu *iommu) > > /* Enable interrupt-remapping */ > iommu->gcmd |= DMA_GCMD_IRE; > - iommu->gcmd &= ~DMA_GCMD_CFI; /* Block compatibility-format > MSIs */ > writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG); > - > IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, > readl, (sts & DMA_GSTS_IRES), sts); > > + /* Block compatibility-format MSIs */ > + sts = readl(iommu->reg + DMAR_GSTS_REG); no need of this readl as the status is already three in IOMMU_WAIT_OP. > + if (sts & DMA_GSTS_CFIS) { > + iommu->gcmd &= ~DMA_GCMD_CFI; > + writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG); > + IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, > + readl, !(sts & DMA_GSTS_CFIS), sts); > + } > + > /* >* With CFI clear in the Global Command register, we should be >* protected from dangerous (i.e. compatibility) interrupts > -- > 2.17.1
Re: [PATCH] perf stat: update POWER9 metrics to utilize other metrics
On Wed, Aug 26, 2020 at 7:06 PM Paul A. Clarke wrote: > > On Wed, Aug 26, 2020 at 09:26:40AM -0700, Ian Rogers wrote: > > On Fri, Aug 14, 2020 at 5:43 AM Arnaldo Carvalho de Melo > > wrote: > > > Em Fri, Aug 14, 2020 at 11:20:42AM +0530, kajoljain escreveu: > > > > On 8/14/20 9:13 AM, Ian Rogers wrote: > > > > > On Thu, Aug 13, 2020 at 3:21 PM Paul A. Clarke > > > > > wrote: > > > > >> These changes take advantage of the new capability added in > > > > >> merge commit 00e4db51259a5f936fec1424b884f029479d3981 > > > > >> "Allow using computed metrics in calculating other metrics". > > > > >> > > > > >> The net is a simplification of the expressions for a handful > > > > >> of metrics, but no functional change. > > > > >> > > > > >> Signed-off-by: Paul A. Clarke > > > > > > > > The patch looks good to me. > > > > > > > > Reviewed-by: Kajol Jain > > > > > > Thanks, applied. Added Ian's Acked-by as well. > > > > I've synced perf and testing on a remote machine (not easy for me to > > log into) I see failures in perf test "10.3: Parsing of PMU event > > table metrics" like: > > ... > > parsing metric: dfu_stall_cpi - dflong_stall_cpi > > Parse event failed metric 'dfu_other_stall_cpi' id 'dflong_stall_cpi' > > expr 'dfu_stall_cpi - dflong_stall_cpi' > > Error string 'parser error' help '(null)' > > Parse event failed metric 'dfu_other_stall_cpi' id 'dfu_stall_cpi' > > expr 'dfu_stall_cpi - dflong_stall_cpi' > > Error string 'parser error' help '(null)' > > ... > > > > This may be that the test doesn't handle the metric in terms of metric > > addition and so I'll look for a fix. I thought I'd send a heads up in > > case you had already seen/addressed this. Is perf test on PowerPC > > clean for you at the moment? > > I see these errors as well (on 5.9-rc2). Each error seems to be for the > newer metrics that take advantage of the newer functionality, including > the metrics I changed recently, and Kajol's 24x7 and nest metrics. > > Thanks for the heads up! I confess I had not seen the errors only because > I wasn't looking. :-/ No worries, if we create a similar Intel metric it will likely exhibit a similar issue in the test. Arnaldo and I have wondered about having an all architectures mode for jevents to make it easier to test cases like this. As my PowerPC set up is a bit special it is great that you've confirmed this isn't at fault :-) I'll try to get time to dig a little further. Thanks, Ian > PC
Re: [RESEND PATCH v4] iommu/mediatek: check 4GB mode by reading infracfg
On Wed, 2020-08-26 at 16:56 +0800, Miles Chen wrote: > In previous discussion [1] and [2], we found that it is risky to > use max_pfn or totalram_pages to tell if 4GB mode is enabled. > > Check 4GB mode by reading infracfg register, remove the usage > of the un-exported symbol max_pfn. > > This is a step towards building mtk_iommu as a kernel module. > > [1] > https://lore.kernel.org/lkml/20200603161132.2441-1-miles.c...@mediatek.com/ > [2] > https://lore.kernel.org/lkml/20200604080120.2628-1-miles.c...@mediatek.com/ > [3] https://lore.kernel.org/lkml/20200715205120.GA778876@bogus/ > > Cc: Mike Rapoport > Cc: David Hildenbrand > Cc: Yong Wu > Cc: Yingjoe Chen > Cc: Christoph Hellwig > Cc: Rob Herring > Cc: Matthias Brugger > Signed-off-by: Miles Chen > > --- > > Change since v3 > - use lore.kernel.org links > - move "change since..." after "---" > > Change since v2: > - determine compatible string by m4u_plat > - rebase to next-20200720 > - add "---" > > Change since v1: > - remove the phandle usage, search for infracfg instead [3] > - use infracfg instead of infracfg_regmap > - move infracfg definitaions to linux/soc/mediatek/infracfg.h > - update enable_4GB only when has_4gb_mode > --- > drivers/iommu/mtk_iommu.c | 34 +++ > include/linux/soc/mediatek/infracfg.h | 3 +++ > 2 files changed, 32 insertions(+), 5 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 785b228d39a6..adc350150492 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -3,7 +3,6 @@ > * Copyright (c) 2015-2016 MediaTek Inc. > * Author: Yong Wu > */ > -#include > #include > #include > #include > @@ -15,13 +14,16 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > #include > +#include > #include > #include > +#include > #include > #include > > @@ -640,8 +642,11 @@ static int mtk_iommu_probe(struct platform_device *pdev) > struct resource *res; > resource_size_t ioaddr; > struct component_match *match = NULL; > + struct regmap *infracfg; > void*protect; > int i, larb_nr, ret; > + u32 val; > + char*p; > > data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > if (!data) > @@ -655,10 +660,29 @@ static int mtk_iommu_probe(struct platform_device *pdev) > return -ENOMEM; > data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN); > > - /* Whether the current dram is over 4GB */ > - data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT)); > - if (!MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) > - data->enable_4GB = false; > + data->enable_4GB = false; > + if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) { > + switch (data->plat_data->m4u_plat) { > + case M4U_MT2712: > + p = "mediatek,mt2712-infracfg"; > + break; > + case M4U_MT8173: > + p = "mediatek,mt8173-infracfg"; > + break; > + default: > + p = NULL; > + } > + This can be simplified: if (data->plat_data->m4u_plat == M4U_MT2712) p = "mediatek,mt2712-infracfg"; else if(data->plat_data->m4u_plat == M4U_MT8173) p = "mediatek,mt8173-infracfg"; else return -EINVAL; Then, Reviewed-by: Yong Wu > + infracfg = syscon_regmap_lookup_by_compatible(p); > + > + if (IS_ERR(infracfg)) > + return PTR_ERR(infracfg); > + > + ret = regmap_read(infracfg, REG_INFRA_MISC, &val); > + if (ret) > + return ret; > + data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN); > + } > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > data->base = devm_ioremap_resource(dev, res); > diff --git a/include/linux/soc/mediatek/infracfg.h > b/include/linux/soc/mediatek/infracfg.h > index fd25f0148566..233463d789c6 100644 > --- a/include/linux/soc/mediatek/infracfg.h > +++ b/include/linux/soc/mediatek/infracfg.h > @@ -32,6 +32,9 @@ > #define MT7622_TOP_AXI_PROT_EN_WB(BIT(2) | BIT(6) | \ >BIT(7) | BIT(8)) > > +#define REG_INFRA_MISC 0xf00 > +#define F_DDR_4GB_SUPPORT_EN BIT(13) > + > int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask, > bool reg_update); > int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
[PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and get the slower speed even the mst hub can run in the faster speed. As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT. If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV to get the faster capability. If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV. Signed-off-by: Koba Ko --- drivers/gpu/drm/drm_dp_mst_topology.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 67dd72ea200e..3b84c6801281 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count) int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool mst_state) { int ret = 0; + u8 dpcd_ext = 0; + unsigned int dpcd_offset = 0; struct drm_dp_mst_branch *mstb = NULL; mutex_lock(&mgr->payload_lock); @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms struct drm_dp_payload reset_pay; WARN_ON(mgr->mst_primary); + drm_dp_dpcd_read(mgr->aux, +DP_TRAINING_AUX_RD_INTERVAL, +&dpcd_ext, sizeof(dpcd_ext)); + + dpcd_offset = + ((dpcd_ext & DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV : DP_DPCD_REV); /* get dpcd info */ - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd, DP_RECEIVER_CAP_SIZE); + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd, DP_RECEIVER_CAP_SIZE); if (ret != DP_RECEIVER_CAP_SIZE) { DRM_DEBUG_KMS("failed to read DPCD\n"); goto out_unlock; -- 2.25.1
[PATCH 2/2] cpufreq: Do WARN() for invalid relation
The relation can't be invalid here, if it is just WARN() and return 0. Signed-off-by: Viresh Kumar --- include/linux/cpufreq.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 8f141d4c859c..a911e5d06845 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -956,8 +956,8 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy, case CPUFREQ_RELATION_C: return cpufreq_table_find_index_c(policy, target_freq); default: - pr_err("%s: Invalid relation: %d\n", __func__, relation); - return -EINVAL; + WARN_ON_ONCE(1); + return 0; } } -- 2.25.0.rc1.19.g042ed3e048af
[PATCH 1/2] cpufreq: No need to verify cpufreq_driver in show_scaling_cur_freq()
"cpufreq_driver" is guaranteed to be valid here, no need to check it here. Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 02ab56b2a0d8..47aa90f9a7c2 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -703,8 +703,7 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf) freq = arch_freq_get_on_cpu(policy->cpu); if (freq) ret = sprintf(buf, "%u\n", freq); - else if (cpufreq_driver && cpufreq_driver->setpolicy && - cpufreq_driver->get) + else if (cpufreq_driver->setpolicy && cpufreq_driver->get) ret = sprintf(buf, "%u\n", cpufreq_driver->get(policy->cpu)); else ret = sprintf(buf, "%u\n", policy->cur); -- 2.25.0.rc1.19.g042ed3e048af
Re: [PATCH v2 2/2] extcon: ptn5150: Set the VBUS and POLARITY property state
Hi, On 8/27/20 2:17 PM, Ramuthevar, Vadivel MuruganX wrote: > Hi, > > On 27/8/2020 12:51 pm, Chanwoo Choi wrote: >> Hi, >> >> You better to change the 'state' word to 'capability'. >> Actually, this patch doesn't change the value of property. >> It set the capability value of property. >> >> "Set the VBUS and POLARITY property capability" > Thank you for the review comments, sure will update. >> >> On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote: >>> From: Ramuthevar Vadivel Murugan >>> >>> >>> Set the VBUS and POLARITY property state. >> >> ditto. Need to change the work from 'state' and 'capability'. > Noted. >> >>> >>> Signed-off-by: Ramuthevar Vadivel Murugan >>> >>> --- >>> drivers/extcon/extcon-ptn5150.c | 6 ++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/extcon/extcon-ptn5150.c >>> b/drivers/extcon/extcon-ptn5150.c >>> index 8b930050a3f1..b5217a61615c 100644 >>> --- a/drivers/extcon/extcon-ptn5150.c >>> +++ b/drivers/extcon/extcon-ptn5150.c >>> @@ -279,6 +279,12 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c) >>> return ret; >>> } >>> + extcon_set_property_capability(info->edev, EXTCON_USB, >>> + EXTCON_PROP_USB_VBUS); >>> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST, >>> + EXTCON_PROP_USB_VBUS); >>> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST, >>> + EXTCON_PROP_USB_TYPEC_POLARITY); >> >> Need to add blank line. > Noted. >> >> I understood that you set the property capability >> because of get_flipped() function of your patch[1]. >> >> But, I think that you need to change the value of >> EXTCON_PROP_USB_TYPEC_POLARITY >> when changing the state of EXTCON_USB_HOST. The polarity property value is >> always >> zero regardless of EXTCON_USB_HOST state as following: The get_flipped()[1] >> returns >> always the same *flipped value. >> >> EXTCON_USB_HOST is 1, EXTCON_PROP_USB_TYPEC_POLARITY is 0 >> EXTCON_USB_HOST is 0, EXTCON_PROP_USB_TYPEC_POLARITY is 0 > by default EXTCON_PROP_USB_TYPEC_POLARITY is 1 If you don't touch the value of EXTCON_PROP_USB_TYPEC_POLARITY property, EXTCON_PROP_USB_TYPEC_POLARITY has default value (0). >> >> If EXTCON_PROP_USB_TYPEC_POLARITY value is not related to any behavior, >> you don't need to get the property value from extcon consumer driver >> like drivers/phy/phy-lgm-usb.c. >> >> Actually, I don't understand why you don't handle the value >> of EXTCON_PROP_USB_TYPEC_POLARITY. >> >> Or, are there any case of what drivers/phy/phy-lgm-usb.c >> uses the different extcon device with EXTCON_PROP_USB_TYPEC_POLARITY property >> in the future? > Yes, you're right, user connect the different USB cable then we check > polarity, accordingly driver proceeds, thanks! OK. >> >> So, do you set the EXTCON_PROP_USB_TYPEC_POLARITY capability >> for the extensibility in order to use other extcon device on later? > yes, that might be the case as well. OK. >> >> >> [1] >> https://protect2.fireeye.com/v1/url?k=1fb29698-422c0d72-1fb31dd7-0cc47a6cba04-3009aa7184024984&q=1&e=566e4565-e7db-4a90-b036-fc28dbdb742f&u=https%3A%2F%2Fwww.spinics.net%2Flists%2Fdevicetree%2Fmsg371828.html >> +static int get_flipped(struct tca_apb *ta, bool *flipped) >> +{ >> + union extcon_property_value property; >> + int ret; >> + >> + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST, >> + EXTCON_PROP_USB_TYPEC_POLARITY, &property); >> + if (ret) { >> + dev_err(ta->phy.dev, "no polarity property from extcon\n"); >> + return ret; >> + } >> + >> + *flipped = property.intval; >> + >> + return ret; >> +} > Thank you for the gone through my usb-phy patch as well > > Regards > Vadivel >> >> >>> /* Initialize PTN5150 device and print vendor id and version id */ >>> ret = ptn5150_init_dev_type(info); >>> if (ret) >>> >> >> > > -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v2 1/2] extcon: ptn5150: Switch to GENMASK() and BIT() macros
Hi, On 27/8/2020 1:26 pm, Chanwoo Choi wrote: - + vendor_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VENDOR, reg_data); + version_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VERSION, reg_data); dev_dbg(info->dev, "Device type: version: 0x%x, vendor: 0x%x\n", version_id, vendor_id); Applied it. Thanks. Thank you! Regards Vadivel
Re: Lockdep warning caused by "driver core: Fix sleeping in invalid context during device link deletion"
On Thu, Aug 20, 2020 at 8:50 PM Dong Aisheng wrote: > > Hi ALL, > > We met the below WARNING during system suspend on an iMX6Q SDB board > with the latest linus/master branch (v5.9-rc1+) and next-20200820. > v5.8 kernel is ok. So i did bisect and finally found it's caused by > the patch below. > Reverting it can get rid of the warning, but I wonder if there may be > other potential issues. > Any ideas? > > Defconfig used is: imx_v6_v7_defconfig > - 8< - Snipped text that was a bit misleading > > Error log: > # echo mem > /sys/power/state > [ 39.111865] PM: suspend entry (deep) > [ 39.148650] Filesystems sync: 0.032 seconds > [ 39.154034] > [ 39.155537] == > [ 39.161723] WARNING: possible circular locking dependency detected > [ 39.167911] 5.9.0-rc1-00103-g7eac66d0456f #37 Not tainted > [ 39.173315] -- > [ 39.179500] sh/647 is trying to acquire lock: > [ 39.183862] c15a310c (dpm_list_mtx){+.+.}-{3:3}, at: > dpm_for_each_dev+0x20/0x5c > [ 39.191200] > [ 39.191200] but task is already holding lock: > [ 39.197036] c15a37e4 (fw_lock){+.+.}-{3:3}, at: fw_pm_notify+0x90/0xd4 > [ 39.203582] > [ 39.203582] which lock already depends on the new lock. > [ 39.203582] > [ 39.211763] > [ 39.211763] the existing dependency chain (in reverse order) is: > [ 39.219249] > [ 39.219249] -> #2 (fw_lock){+.+.}-{3:3}: > [ 39.224673]mutex_lock_nested+0x1c/0x24 > [ 39.229126]firmware_uevent+0x18/0xa0 > [ 39.233411]dev_uevent+0xc4/0x1f8 > [ 39.237343]uevent_show+0x98/0x114 > [ 39.241362]dev_attr_show+0x18/0x48 > [ 39.245472]sysfs_kf_seq_show+0x84/0xec > [ 39.249927]seq_read+0x138/0x550 > [ 39.253774]vfs_read+0x94/0x164 > [ 39.257529]ksys_read+0x60/0xe8 > [ 39.261288]ret_fast_syscall+0x0/0x28 > [ 39.265564]0xbed7c808 > [ 39.268538] > [ 39.268538] -> #1 (kn->active#3){}-{0:0}: > [ 39.274391]kernfs_remove_by_name_ns+0x40/0x94 > [ 39.279450]device_del+0x144/0x3fc Rafael/Greg, I'm not very familiar with the #0 and #2 calls stacks. But poking around a bit, they are NOT due to the device-link-device. But the new stuff is the above two lines that are deleting the device-link-device (that's used to expose device link details in sysfs) when the device link is deleted. Kicking off a workqueue to break this cycle is easy, but the problem is that if I queue a work to delete the device, then the sysfs folder won't get removed immediately. And if the same link is created again before the work is completed, then there'll be a sysfs name collision and warning. So, I'm kinda stuck here. Open to suggestions. Hoping you'll have better ideas for breaking the cycle. Or point out how I'm misunderstanding the cycle here. -Saravana > [ 39.283467]__device_link_del+0x4c/0x70 > [ 39.287919]device_link_remove+0x5c/0x8c > [ 39.292464]_regulator_put.part.0+0x104/0x1dc > [ 39.297436]regulator_put+0x2c/0x3c > [ 39.299731] regulator regulator.5: Failed to increase supply voltage: -110 > [ 39.301544]release_nodes+0x1b4/0x204 > [ 39.301553]really_probe+0x104/0x3b4 > [ 39.316881]driver_probe_device+0x58/0xb4 > [ 39.321506]device_driver_attach+0x58/0x60 > [ 39.326217]__driver_attach+0x58/0xd0 > [ 39.330499]bus_for_each_dev+0x74/0xbc > [ 39.334863]bus_add_driver+0x150/0x1dc > [ 39.339227]driver_register+0x74/0x108 > [ 39.343599]i2c_register_driver+0x38/0x8c > [ 39.348227]do_one_initcall+0x84/0x3b4 > [ 39.352598]kernel_init_freeable+0x154/0x1e4 > [ 39.357485]kernel_init+0x8/0x118 > [ 39.361415]ret_from_fork+0x14/0x20 > [ 39.365518]0x0 > [ 39.367883] > [ 39.367883] -> #0 (dpm_list_mtx){+.+.}-{3:3}: > [ 39.373740]lock_acquire+0xe0/0x4ec > [ 39.377848]__mutex_lock+0x94/0x9d0 > [ 39.381952]mutex_lock_nested+0x1c/0x24 > [ 39.386405]dpm_for_each_dev+0x20/0x5c > [ 39.390769]fw_pm_notify+0xa4/0xd4 > [ 39.394795]notifier_call_chain+0x48/0x80 > [ 39.399420]__blocking_notifier_call_chain+0x48/0x64 > [ 39.405003]__pm_notifier_call_chain+0x20/0x3c > [ 39.410063]pm_suspend+0x1ac/0x438 > [ 39.414080]state_store+0x68/0xc8 > [ 39.418013]kernfs_fop_write+0x10c/0x22c > [ 39.419741] cpu cpu0: failed to scale vddpu up: -110 > [ 39.422551]vfs_write+0xbc/0x1d8 > [ 39.422559]ksys_write+0x60/0xe8 > [ 39.427529] cpufreq: __target_index: Failed to change cpu frequency: -110 > [ 39.431362]ret_fast_syscall+0x0/0x28 > [ 39.431368]0xbeec8958 > [ 39.431372] > [ 39.431372] other info that might help us debug this: > [ 39.431372] > [ 39.431375] Chain exists of: > [ 39.43
Re: [PATCH v2 2/2] extcon: ptn5150: Set the VBUS and POLARITY property state
Hi, On 27/8/2020 12:51 pm, Chanwoo Choi wrote: Hi, You better to change the 'state' word to 'capability'. Actually, this patch doesn't change the value of property. It set the capability value of property. "Set the VBUS and POLARITY property capability" Thank you for the review comments, sure will update. On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote: From: Ramuthevar Vadivel Murugan Set the VBUS and POLARITY property state. ditto. Need to change the work from 'state' and 'capability'. Noted. Signed-off-by: Ramuthevar Vadivel Murugan --- drivers/extcon/extcon-ptn5150.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c index 8b930050a3f1..b5217a61615c 100644 --- a/drivers/extcon/extcon-ptn5150.c +++ b/drivers/extcon/extcon-ptn5150.c @@ -279,6 +279,12 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c) return ret; } + extcon_set_property_capability(info->edev, EXTCON_USB, + EXTCON_PROP_USB_VBUS); + extcon_set_property_capability(info->edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_VBUS); + extcon_set_property_capability(info->edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_TYPEC_POLARITY); Need to add blank line. Noted. I understood that you set the property capability because of get_flipped() function of your patch[1]. But, I think that you need to change the value of EXTCON_PROP_USB_TYPEC_POLARITY when changing the state of EXTCON_USB_HOST. The polarity property value is always zero regardless of EXTCON_USB_HOST state as following: The get_flipped()[1] returns always the same *flipped value. EXTCON_USB_HOST is 1, EXTCON_PROP_USB_TYPEC_POLARITY is 0 EXTCON_USB_HOST is 0, EXTCON_PROP_USB_TYPEC_POLARITY is 0 by default EXTCON_PROP_USB_TYPEC_POLARITY is 1 If EXTCON_PROP_USB_TYPEC_POLARITY value is not related to any behavior, you don't need to get the property value from extcon consumer driver like drivers/phy/phy-lgm-usb.c. Actually, I don't understand why you don't handle the value of EXTCON_PROP_USB_TYPEC_POLARITY. Or, are there any case of what drivers/phy/phy-lgm-usb.c uses the different extcon device with EXTCON_PROP_USB_TYPEC_POLARITY property in the future? Yes, you're right, user connect the different USB cable then we check polarity, accordingly driver proceeds, thanks! So, do you set the EXTCON_PROP_USB_TYPEC_POLARITY capability for the extensibility in order to use other extcon device on later? yes, that might be the case as well. [1] https://www.spinics.net/lists/devicetree/msg371828.html +static int get_flipped(struct tca_apb *ta, bool *flipped) +{ + union extcon_property_value property; + int ret; + + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_TYPEC_POLARITY, &property); + if (ret) { + dev_err(ta->phy.dev, "no polarity property from extcon\n"); + return ret; + } + + *flipped = property.intval; + + return ret; +} Thank you for the gone through my usb-phy patch as well Regards Vadivel /* Initialize PTN5150 device and print vendor id and version id */ ret = ptn5150_init_dev_type(info); if (ret)
Re: [PATCH v2 1/2] extcon: ptn5150: Switch to GENMASK() and BIT() macros
On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote: > From: Ramuthevar Vadivel Murugan > > Switch to GENMASK() and BIT() macros. > > Signed-off-by: Ramuthevar Vadivel Murugan > > Reviewed-by: Krzysztof Kozlowski > --- > drivers/extcon/extcon-ptn5150.c | 43 > +++-- > 1 file changed, 11 insertions(+), 32 deletions(-) > > diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c > index 8ba706fad887..8b930050a3f1 100644 > --- a/drivers/extcon/extcon-ptn5150.c > +++ b/drivers/extcon/extcon-ptn5150.c > @@ -7,6 +7,7 @@ > // Author: Vijai Kumar K > // Copyright (c) 2020 Krzysztof Kozlowski > > +#include > #include > #include > #include > @@ -35,29 +36,13 @@ enum ptn5150_reg { > #define PTN5150_UFP_ATTACHED 0x2 > > /* Define PTN5150 MASK/SHIFT constant */ > -#define PTN5150_REG_DEVICE_ID_VENDOR_SHIFT 0 > -#define PTN5150_REG_DEVICE_ID_VENDOR_MASK\ > - (0x3 << PTN5150_REG_DEVICE_ID_VENDOR_SHIFT) > +#define PTN5150_REG_DEVICE_ID_VERSIONGENMASK(7, 3) > +#define PTN5150_REG_DEVICE_ID_VENDOR GENMASK(2, 0) > > -#define PTN5150_REG_DEVICE_ID_VERSION_SHIFT 3 > -#define PTN5150_REG_DEVICE_ID_VERSION_MASK \ > - (0x1f << PTN5150_REG_DEVICE_ID_VERSION_SHIFT) > - > -#define PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT 2 > -#define PTN5150_REG_CC_PORT_ATTACHMENT_MASK \ > - (0x7 << PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT) > - > -#define PTN5150_REG_CC_VBUS_DETECTION_SHIFT 7 > -#define PTN5150_REG_CC_VBUS_DETECTION_MASK \ > - (0x1 << PTN5150_REG_CC_VBUS_DETECTION_SHIFT) > - > -#define PTN5150_REG_INT_CABLE_ATTACH_SHIFT 0 > -#define PTN5150_REG_INT_CABLE_ATTACH_MASK\ > - (0x1 << PTN5150_REG_INT_CABLE_ATTACH_SHIFT) > - > -#define PTN5150_REG_INT_CABLE_DETACH_SHIFT 1 > -#define PTN5150_REG_INT_CABLE_DETACH_MASK\ > - (0x1 << PTN5150_REG_CC_CABLE_DETACH_SHIFT) > +#define PTN5150_REG_CC_PORT_ATTACHMENT GENMASK(4, 2) > +#define PTN5150_REG_CC_VBUS_DETECTIONBIT(7) > +#define PTN5150_REG_INT_CABLE_ATTACH_MASKBIT(0) > +#define PTN5150_REG_INT_CABLE_DETACH_MASKBIT(1) > > struct ptn5150_info { > struct device *dev; > @@ -95,9 +80,7 @@ static void ptn5150_check_state(struct ptn5150_info *info) > return; > } > > - port_status = ((reg_data & > - PTN5150_REG_CC_PORT_ATTACHMENT_MASK) >> > - PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT); > + port_status = FIELD_GET(PTN5150_REG_CC_PORT_ATTACHMENT, reg_data); > > switch (port_status) { > case PTN5150_DFP_ATTACHED: > @@ -107,8 +90,7 @@ static void ptn5150_check_state(struct ptn5150_info *info) > break; > case PTN5150_UFP_ATTACHED: > extcon_set_state_sync(info->edev, EXTCON_USB, false); > - vbus = ((reg_data & PTN5150_REG_CC_VBUS_DETECTION_MASK) >> > - PTN5150_REG_CC_VBUS_DETECTION_SHIFT); > + vbus = FIELD_GET(PTN5150_REG_CC_VBUS_DETECTION, reg_data); > if (vbus) > gpiod_set_value_cansleep(info->vbus_gpiod, 0); > else > @@ -191,11 +173,8 @@ static int ptn5150_init_dev_type(struct ptn5150_info > *info) > return -EINVAL; > } > > - vendor_id = ((reg_data & PTN5150_REG_DEVICE_ID_VENDOR_MASK) >> > - PTN5150_REG_DEVICE_ID_VENDOR_SHIFT); > - version_id = ((reg_data & PTN5150_REG_DEVICE_ID_VERSION_MASK) >> > - PTN5150_REG_DEVICE_ID_VERSION_SHIFT); > - > + vendor_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VENDOR, reg_data); > + version_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VERSION, reg_data); > dev_dbg(info->dev, "Device type: version: 0x%x, vendor: 0x%x\n", > version_id, vendor_id); > > Applied it. Thanks. -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH] thermal: sysfs: fall back to vzalloc for cooling device's statistics
On 27-08-20, 12:03, Yue Hu wrote: > Hi Daniel, > > Now, i'm just focus on removing the kernel warning based on current code > logic. > Commit 8ea229511e06 (thermal: Add cooling device's statistics in sysfs) added > the thermal statistics by viresh and viresh gived the patch an acknowledgement > in anther mail thread. > > Hi viresh, > > Could you review the patch again about the question above? Yeah, I Acked it but the questions raised by Daniel are very valid and must be answered. I understand that you only cared about fixing the warning, but maybe we need to fix the driver and the warning will go away by itself. If you don't want to do it, then someone who is responsible for the driver should do it. Was it the acpi_video.c driver that you got the warning from ? I have added Rafael to the email in case that driver needs getting fixed. -- viresh
linux-next: Tree for Aug 27
Hi all, News: There will be no linux-next releases next Monday or Tuesday. Changes since 20200826: The net-next tree gained a conflict against the net tree. Non-merge commits (relative to Linus' tree): 2901 3429 files changed, 100496 insertions(+), 37081 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig and htmldocs. And finally, a simple boot test of the powerpc pseries_le_defconfig kernel in qemu (with and without kvm enabled). Below is a summary of the state of the merge. I am currently merging 328 trees (counting Linus' and 86 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (15bc20c6af4c Merge tag 'tty-5.9-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty) Merging fixes/fixes (9123e3a74ec7 Linux 5.9-rc1) Merging kbuild-current/fixes (d012a7190fc1 Linux 5.9-rc2) Merging arc-current/for-curr (37016ab49214 irqchip/eznps: Fix build error for !ARC700 builds) Merging arm-current/fixes (5c6360ee4a0e ARM: 8988/1: mmu: fix crash in EFI calls due to p4d typo in create_mapping_late()) Merging arm64-fixes/for-next/fixes (8d75785a8142 ARM64: vdso32: Install vdso32 from vdso_install) Merging arm-soc-fixes/arm/fixes (9c8b0a9c37b7 Merge tag 'imx-fixes-5.9' of git://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux into arm/fixes) Merging uniphier-fixes/fixes (48778464bb7d Linux 5.8-rc2) Merging drivers-memory-fixes/fixes (7ff3a2a626f7 memory: jz4780_nemc: Fix an error pointer vs NULL check in probe()) Merging m68k-current/for-linus (382f429bb559 m68k: defconfig: Update defconfigs for v5.8-rc3) Merging powerpc-fixes/fixes (64ef8f2c4791 powerpc/perf/hv-24x7: Move cpumask file to top folder of hv-24x7 driver) Merging s390-fixes/fixes (bffc2f7aa963 s390/vmem: fix vmem_add_range for 4-level paging) Merging sparc/master (0a95a6d1a4cd sparc: use for_each_child_of_node() macro) Merging fscrypt-current/for-stable (2b4eae95c736 fscrypt: don't evict dirty inodes after removing key) Merging net/master (5fd99b5d9950 net: cdc_ncm: Fix build error) Merging bpf/master (7787b6fc938e bpf, sysctl: Let bpf_stats_handler take a kernel pointer buffer) Merging ipsec/master (4eb2e1341575 espintcp: restore IP CB before handing the packet to xfrm) Merging netfilter/master (3622adb02623 ipv6: ndisc: adjust ndisc_ifinfo_sysctl_change prototype) Merging ipvs/master (7c7ab580db49 net: Convert to use the fallthrough macro) Merging wireless-drivers/master (9a2a0862d973 brcmfmac: reserve tx credit only when txctl is ready to send) Merging mac80211/master (fce2ff728f95 nl80211: fix NL80211_ATTR_HE_6GHZ_CAPABILITY usage) Merging rdma-fixes/for-rc (60b1af64eb35 RDMA/rxe: Fix the parent sysfs read when the interface has 15 chars) Merging sound-current/for-linus (858e0ad9301d ALSA: hda/hdmi: always check pin power status in i915 pin fixup) Merging sound-asoc-fixes/for-linus (d563b6c834ae Merge series "ASoC: Fix return check for devm_regmap_init_sdw()" from Vinod Koul :) Merging regmap-fixes/for-linus (d012a7190fc1 Linux 5.9-rc2) Merging regulator-fixes/for-linus (3bec5b6aae83 Merge tag 'v5.9-rc2' into regulator-5.9) Merging spi-fixes/for-linus (3812e0343b42 Merge remote-tracking branch 'spi/for-5.9' into spi-linus) Merging pci-current/for-linus (7c2308f79fc8 PCI/P2PDMA: Fix build without DMA ops) Merging driver-core.current/driver-core-linus (d012a7190fc1 Linux 5.9-rc2) Merging tty.current/tty-linus (ea1f
Re: [PATCH 1/2] power: supply: Add support for RN5T618/RC5T619 charger and fuel gauge
Hi, On Wed, 26 Aug 2020 23:59:50 +0200 Sebastian Reichel wrote: > Hi, > > On Wed, Aug 26, 2020 at 08:28:34PM +0200, Andreas Kemnade wrote: > > On Wed, 26 Aug 2020 19:48:17 +0200 > > Sebastian Reichel wrote: > > > On Sat, Aug 15, 2020 at 06:56:09PM +0200, Andreas Kemnade wrote: > > > > [...] > > > > +static int rn5t618_battery_current_now(struct rn5t618_power_info *info, > > > > + union power_supply_propval *val) > > > > +{ > > > > + u16 res; > > > > + int ret; > > > > + > > > > + ret = rn5t618_battery_read_doublereg(info, RN5T618_CC_AVEREG1, > > > > &res); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + val->intval = res; > > > > + /* 2's complement */ > > > > + if (val->intval & (1 << 13)) > > > > + val->intval = val->intval - (1 << 14); > > Btw. I think sign_extend32() can be used here? > > > > > + /* negate current to be positive when discharging */ > > > > + val->intval *= -1000; > > > > > > mh, the sign is not documented (which should be fixed). At least > > > sbs-battery does it the other way around (negative current when > > > discharging, positive otherwise). Some drivers do not support > > > signed current and always report positive values (e.g. ACPI driver). > > > > > > What did you use as reference for swapping the sign? > > > > > Well, I have searched for documentation, found nothing and used the > > bq27xxx driver as reference which I am used to from the GTA04/GTA02, > > so things behave equal. That are the devices where a was most > > intensively looking at those values. > > I thought that there would be some unwritten rule about that. > > The mess is mostly due to lacking reviewing from my side > (and possibly my predecessors). I just went through a dozen of > drivers and it looks like most either do not support signed current > (and use negative values as error code :() or use negative current > for discharge. I could not find any other driver using negative > numbers for charging. I think it's best to negative = discharge as > official correct value and will send an update patch for the > documentation later. > ok, I will remove that sign-change and keep/update the comment so that you know that the driver does it the official way and send a v2 with the other things you mentioned. Regards, Andreas pgp5ljy2PRtrW.pgp Description: OpenPGP digital signature
[PATCH v2] debugobjects: install cpu hotplug callback
From: Zqiang Due to cpu hotplug, it may never be online after it's offline, some objects in percpu pool is never free, in order to avoid this happening, install cpu hotplug callback, call this callback func to free objects in percpu pool when cpu going offline. Signed-off-by: Zqiang --- v1->v2: Modify submission information. include/linux/cpuhotplug.h | 1 + lib/debugobjects.c | 23 +++ 2 files changed, 24 insertions(+) diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h index a2710e654b64..2e77db655cfa 100644 --- a/include/linux/cpuhotplug.h +++ b/include/linux/cpuhotplug.h @@ -36,6 +36,7 @@ enum cpuhp_state { CPUHP_X86_MCE_DEAD, CPUHP_VIRT_NET_DEAD, CPUHP_SLUB_DEAD, + CPUHP_DEBUG_OBJ_DEAD, CPUHP_MM_WRITEBACK_DEAD, CPUHP_MM_VMSTAT_DEAD, CPUHP_SOFTIRQ_DEAD, diff --git a/lib/debugobjects.c b/lib/debugobjects.c index fe4557955d97..50e21ed0519e 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -19,6 +19,7 @@ #include #include #include +#include #define ODEBUG_HASH_BITS 14 #define ODEBUG_HASH_SIZE (1 << ODEBUG_HASH_BITS) @@ -433,6 +434,23 @@ static void free_object(struct debug_obj *obj) } } +#if defined(CONFIG_HOTPLUG_CPU) +static int object_cpu_offline(unsigned int cpu) +{ + struct debug_percpu_free *percpu_pool; + struct hlist_node *tmp; + struct debug_obj *obj; + + percpu_pool = per_cpu_ptr(&percpu_obj_pool, cpu); + hlist_for_each_entry_safe(obj, tmp, &percpu_pool->free_objs, node) { + hlist_del(&obj->node); + kmem_cache_free(obj_cache, obj); + } + + return 0; +} +#endif + /* * We run out of memory. That means we probably have tons of objects * allocated. @@ -1367,6 +1385,11 @@ void __init debug_objects_mem_init(void) } else debug_objects_selftest(); +#if defined(CONFIG_HOTPLUG_CPU) + cpuhp_setup_state_nocalls(CPUHP_DEBUG_OBJ_DEAD, "object:offline", NULL, + object_cpu_offline); +#endif + /* * Increase the thresholds for allocating and freeing objects * according to the number of possible CPUs available in the system. -- 2.17.1
Re: [PATCH] i2c: do not acpi/of match device in i2c_device_probe()
On (20/08/26 18:16), Andy Shevchenko wrote: > On Wed, Aug 26, 2020 at 11:49:20PM +0900, Sergey Senozhatsky wrote: > > i2c, apparently, can match the same device twice - the first > > time in ->match bus hook (i2c_device_match()), and the second > > one in ->probe (i2c_device_probe()) bus hook. > > > > To make things more complicated, the second matching does not > > do exactly same checks as the first one. Namely, i2c_device_match() > > calls acpi_driver_match_device() which considers devices that > > provide of_match_table and performs of_compatible() matching for > > such devices. One important thing to note here is that ACPI > > of_compatible() matching (acpi_of_match_device()) is part of ACPI > > and does not depend on CONFIG_OF. > > > > i2c_device_probe(), on the other hand, calls acpi_match_device() > > which does not perform of_compatible() matching, but instead > > i2c_device_probe() relies on CONFIG_OF API to perform of_match_table > > matching, IOW ->probe matching, unlike ->match matching, depends on > > CONFIG_OF. This can break i2c device probing on !CONFIG_OF systems > > if the device does not provide .id_table. > > > > i2c_device_probe() > > ... > >if (!driver->id_table && > >!i2c_acpi_match_device(dev->driver->acpi_match_table, client) && > >!i2c_of_match_device(dev->driver->of_match_table, client)) { > >status = -ENODEV; > >goto put_sync_adapter; > >} > > > > i2c_of_match_device() on !CONFIG_OF systems is always false, so we never > > perform of_match_table matching. i2c_acpi_match_device() does ACPI match > > only, no of_compatible() matching takes place, even though the device > > provides .of_match_table and ACPI is capable of matching such device. > > > > It is not entirely clear why the device is matched again in bus > > ->probe after successful and proper matching in bus ->match. Let's > > remove ->probe matching. > > Reviewed-by: Andy Shevchenko > (assuming it's okay to go) Thanks. I tested the patch on x86_64 (a mix of i2c devices with and without .id_table) and arm64 boards - didn't notice any difference, module probing wise. -ss
Re: [PATCH] drm/i915/lspcon: Limits to 8 bpc for RGB/YCbCr444
Hi Ville, > On Aug 27, 2020, at 12:24 AM, Ville Syrjälä > wrote: > > On Wed, Aug 26, 2020 at 01:21:15PM +0800, Kai-Heng Feng wrote: >> LSPCON only supports 8 bpc for RGB/YCbCr444. >> >> Set the correct bpp otherwise it renders blank screen. > > Hmm. Does > git://github.com/vsyrjala/linux.git dp_downstream_ports_5 > work? > > Actually better make that dp_downstream_ports_5^^^ aka. > 54d846ce62a2 ("drm/i915: Do YCbCr 444->420 conversion via DP protocol > converters") to avoid the experiments and hacks I have sitting on top. Can you please rebase it to mainline master or drm-tip? I am getting errors on the branch: DESCEND objtool CALLscripts/atomic/check-atomics.sh CALLscripts/checksyscalls.sh CHK include/generated/compile.h Building modules, stage 2. MODPOST 166 modules LD arch/x86/boot/compressed/vmlinux ld: arch/x86/boot/compressed/pgtable_64.o:(.bss+0x0): multiple definition of `__force_order'; arch/x86/boot/compressed/kaslr_64.o:(.bss+0x0): first defined here ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text' ld: warning: creating DT_TEXTREL in a PIE make[2]: *** [arch/x86/boot/compressed/Makefile:119: arch/x86/boot/compressed/vmlinux] Error 1 make[1]: *** [arch/x86/boot/Makefile:113: arch/x86/boot/compressed/vmlinux] Error 2 make: *** [arch/x86/Makefile:284: bzImage] Error 2 make: *** Waiting for unfinished jobs Kai-Heng > >> >> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2195 >> Signed-off-by: Kai-Heng Feng >> --- >> drivers/gpu/drm/i915/display/intel_lspcon.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c >> b/drivers/gpu/drm/i915/display/intel_lspcon.c >> index b781bf469644..c7a44fcaade8 100644 >> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c >> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c >> @@ -196,7 +196,8 @@ void lspcon_ycbcr420_config(struct drm_connector >> *connector, >> crtc_state->port_clock /= 2; >> crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444; >> crtc_state->lspcon_downsampling = true; >> -} >> +} else >> +crtc_state->pipe_bpp = 24; >> } >> >> static bool lspcon_probe(struct intel_lspcon *lspcon) >> -- >> 2.17.1 > > -- > Ville Syrjälä > Intel
Re: [PATCH v2 3/3] usb typec: mt6360: Prevent the race condition during module remove
Guenter Roeck 於 2020年8月27日 週四 上午11:34寫道: > > On 8/26/20 8:25 PM, cy_huang wrote: > > From: ChiYuan Huang > > > > Prevent the race condition from interrupt and tcpci port unregister > > during module remove. > > > > Signed-off-by: ChiYuan Huang > > Please squash with the first patch of the series. Done, squash into the first one and resend the patch. Thx. > > Thanks, > Guenter > > > --- > > drivers/usb/typec/tcpm/tcpci_mt6360.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c > > b/drivers/usb/typec/tcpm/tcpci_mt6360.c > > index 6a28193..a381b5d 100644 > > --- a/drivers/usb/typec/tcpm/tcpci_mt6360.c > > +++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c > > @@ -164,6 +164,7 @@ static int mt6360_tcpc_remove(struct platform_device > > *pdev) > > { > > struct mt6360_tcpc_info *mti = platform_get_drvdata(pdev); > > > > + disable_irq(mti->irq); > > tcpci_unregister_port(mti->tcpci); > > return 0; > > } > > >
[PATCH v2 1/2] usb typec: mt6360: Add support for mt6360 Type-C driver
From: ChiYuan Huang Mediatek MT6360 is a multi-functional IC that includes USB Type-C. It works with Type-C Port Controller Manager to provide USB PD and USB Type-C functionalities. Add fix to Prevent the race condition from interrupt and tcpci port unregister during module remove. Signed-off-by: ChiYuan Huang --- drivers/usb/typec/tcpm/Kconfig| 8 ++ drivers/usb/typec/tcpm/Makefile | 1 + drivers/usb/typec/tcpm/tcpci_mt6360.c | 213 ++ 3 files changed, 222 insertions(+) create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig index fa3f393..58a64e1 100644 --- a/drivers/usb/typec/tcpm/Kconfig +++ b/drivers/usb/typec/tcpm/Kconfig @@ -27,6 +27,14 @@ config TYPEC_RT1711H Type-C Port Controller Manager to provide USB PD and USB Type-C functionalities. +config TYPEC_MT6360 + tristate "Mediatek MT6360 Type-C driver" + depends on MFD_MT6360 + help + Mediatek MT6360 is a multi-functional IC that includes + USB Type-C. It works with Type-C Port Controller Manager + to provide USB PD and USB Type-C functionalities. + endif # TYPEC_TCPCI config TYPEC_FUSB302 diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile index a5ff6c8..7592ccb 100644 --- a/drivers/usb/typec/tcpm/Makefile +++ b/drivers/usb/typec/tcpm/Makefile @@ -5,3 +5,4 @@ obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o typec_wcove-y := wcove.o obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o +obj-$(CONFIG_TYPEC_MT6360) += tcpci_mt6360.o diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c b/drivers/usb/typec/tcpm/tcpci_mt6360.c new file mode 100644 index ..a381b5d --- /dev/null +++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c @@ -0,0 +1,213 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// Copyright (C) 2020 MediaTek Inc. +// +// Author: ChiYuan Huang + +#include +#include +#include +#include +#include +#include +#include + +#include "tcpci.h" + +#define MT6360_REG_VCONNCTRL1 0x8C +#define MT6360_REG_MODECTRL2 0x8F +#define MT6360_REG_SWRESET 0xA0 +#define MT6360_REG_DEBCTRL10xA1 +#define MT6360_REG_DRPCTRL10xA2 +#define MT6360_REG_DRPCTRL20xA3 +#define MT6360_REG_I2CTORST0xBF +#define MT6360_REG_RXCTRL2 0xCF +#define MT6360_REG_CTDCTRL20xEC + +/* MT6360_REG_VCONNCTRL1 */ +#define MT6360_VCONNCL_ENABLE BIT(0) +/* MT6360_REG_RXCTRL2 */ +#define MT6360_OPEN40M_ENABLE BIT(7) +/* MT6360_REG_CTDCTRL2 */ +#define MT6360_RPONESHOT_ENABLEBIT(6) + +struct mt6360_tcpc_info { + struct tcpci_data tdata; + struct tcpci *tcpci; + struct device *dev; + int irq; +}; + +static inline int mt6360_tcpc_read16(struct regmap *regmap, +unsigned int reg, u16 *val) +{ + return regmap_raw_read(regmap, reg, val, sizeof(u16)); +} + +static inline int mt6360_tcpc_write16(struct regmap *regmap, + unsigned int reg, u16 val) +{ + return regmap_raw_write(regmap, reg, &val, sizeof(u16)); +} + +static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata) +{ + struct regmap *regmap = tdata->regmap; + int ret; + + ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01); + if (ret) + return ret; + + /* after reset command, wait 1~2ms to wait IC action */ + usleep_range(1000, 2000); + + /* write all alert to masked */ + ret = mt6360_tcpc_write16(regmap, TCPC_ALERT_MASK, 0); + if (ret) + return ret; + + /* config I2C timeout reset enable , and timeout to 200ms */ + ret = regmap_write(regmap, MT6360_REG_I2CTORST, 0x8F); + if (ret) + return ret; + + /* config CC Detect Debounce : 26.7*val us */ + ret = regmap_write(regmap, MT6360_REG_DEBCTRL1, 0x10); + if (ret) + return ret; + + /* DRP Toggle Cycle : 51.2 + 6.4*val ms */ + ret = regmap_write(regmap, MT6360_REG_DRPCTRL1, 4); + if (ret) + return ret; + + /* DRP Duyt Ctrl : dcSRC: /1024 */ + ret = mt6360_tcpc_write16(regmap, MT6360_REG_DRPCTRL2, 330); + if (ret) + return ret; + + /* Enable VCONN Current Limit function */ + ret = regmap_update_bits(regmap, MT6360_REG_VCONNCTRL1, MT6360_VCONNCL_ENABLE, +MT6360_VCONNCL_ENABLE); + if (ret) + return ret; + + /* Enable cc open 40ms when pmic send vsysuv signal */ + ret = regmap_update_bits(regmap, MT6360_REG_RXCTRL2, MT6360_OPEN40M_ENABLE, +MT6360_OPEN40M_ENABLE); + if (ret) + return ret; + + /* Enable Rpdet oneshot detection */ + ret = regmap_update_bits(regmap, MT6360_REG_CTDCTRL2, MT6360_RPONESHOT_E
Re: [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding
On 26-08-20, 08:35, Rob Herring wrote: > On Wed, Aug 26, 2020 at 12:32 AM Vinod Koul wrote: > > > > On 25-08-20, 20:21, Vinod Koul wrote: > > > Hey Rob, > > > > > > On 24-08-20, 11:40, Rob Herring wrote: > > > > On Mon, 24 Aug 2020 14:17:10 +0530, Vinod Koul wrote: > > > > > Add devicetree binding documentation for GPI DMA controller > > > > > implemented on Qualcomm SoCs > > > > > > > > > > Signed-off-by: Vinod Koul > > > > > --- > > > > > .../devicetree/bindings/dma/qcom-gpi.yaml | 87 > > > > > +++ > > > > > 1 file changed, 87 insertions(+) > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/dma/qcom-gpi.yaml > > > > > > > > > > > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > > > > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: > > > > properties:qcom,ev-factor: {'description': 'Event ring transfer size > > > > compare to channel transfer ring. Event ring length = ev-factor * > > > > transfer ring size', 'maxItems': 1} is not valid under any of the given > > > > schemas (Possible causes of the failure): > > > > > > > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: > > > > properties:qcom,ev-factor: 'not' is a required property > > > > > > Okay updating dt-schema I do see this, now the question is what is this > > > and what does it mean ;-) I am not sure I comprehend the error message. > > > I see this for all the new properties I added as required for this > > > device node > > > > Okay I think I have figured it out, I need to provide ref to > > /schemas/types.yaml#definitions/uint32 for this to work, which does > > makes sense to me. > > > > qcom,max-num-gpii: > > $ref: /schemas/types.yaml#definitions/uint32 > > maxItems: 1 > > uint32 is always 1 item, so drop. Is there a max value you can define? Sorry not sure I follow, to clarify you mean drop uint32, if so which type to use u8? I can use u8 as max wont be beyond 255. Yes I will define min as well max values too. > Otherwise, up to 2^32 - 1 is valid. I see one more warning given by your bot which I am able to reproduce as well: Documentation/devicetree/bindings/dma/qcom,gpi.example.dt.yaml: example-0: dma-controller@80:reg:0: [0, 8388608, 0, 393216] is too long So to fix this I added the #address-cells and #size-cells #address-cells = <2>; #size-cells = <2>; reg = <0x0 0x0080 0x0 0x6>; But I am getting the warning, what am I doing incorrect -- ~Vinod
linux-next: build failure after upgrading sphinx
Hi all, Today I upgraded ot sphinx v3.2.1 and got the following error from "make htmldocs": Running Sphinx v3.2.1 enabling CJK for LaTeX builder Extension error: Could not import extension cdomain (exception: cannot import name 'c_funcptr_sig_re' from 'sphinx.domains.c' (/usr/lib/python3/dist-packages/sphinx/domains/c.py)) I have downgraded to version 2.4.3 and await suggestions/patches :-) -- Cheers, Stephen Rothwell pgpOxRaqIun_C.pgp Description: OpenPGP digital signature
[PATCH v2 2/2] usb typec: mt6360: Add MT6360 Type-C DT binding documentation
From: ChiYuan Huang Add a devicetree binding documentation for the MT6360 Type-C driver. Signed-off-by: ChiYuan Huang --- .../bindings/usb/mediatek,mt6360-tcpc.yaml | 73 ++ 1 file changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/mediatek,mt6360-tcpc.yaml diff --git a/Documentation/devicetree/bindings/usb/mediatek,mt6360-tcpc.yaml b/Documentation/devicetree/bindings/usb/mediatek,mt6360-tcpc.yaml new file mode 100644 index ..9e8ab0d --- /dev/null +++ b/Documentation/devicetree/bindings/usb/mediatek,mt6360-tcpc.yaml @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/usb/mediatek,mt6360-tcpc.yaml#"; +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; + +title: Mediatek MT6360 Type-C Port Switch and Power Delivery controller DT bindings + +maintainers: + - ChiYuan Huang + +description: | + Mediatek MT6360 is a multi-functional device. It integrates charger, ADC, flash, RGB indicators, + regulators (BUCKs/LDOs), and TypeC Port Switch with Power Delivery controller. + This document only describes MT6360 Type-C Port Switch and Power Delivery controller. + +properties: + compatible: +enum: + - mediatek,mt6360-tcpc + + interrupts-extended: +maxItems: 1 + + interrupt-names: +items: + - const: PD_IRQB + +patternProperties: + "connector": +type: object +$ref: ../connector/usb-connector.yaml# +description: + Properties for usb c connector. + +additionalProperties: false + +required: + - compatible + - interrupts-extended + - interrupt-names + +examples: + - | +#include +#include +i2c0 { +#address-cells = <1>; +#size-cells = <0>; + +mt6360@34 { +compatible = "mediatek,mt6360"; +reg = <0x34>; + +tcpc { +compatible = "mediatek,mt6360-tcpc"; +interrupts-extended = <&gpio26 3 IRQ_TYPE_LEVEL_LOW>; +interrupt-names = "PD_IRQB"; + +connector { +compatible = "usb-c-connector"; +label = "USB-C"; +data-role = "dual"; +power-role = "dual"; +try-power-role = "sink"; +source-pdos = ; +sink-pdos = ; +op-sink-microwatt = <1000>; +}; +}; +}; +}; +... -- 2.7.4
RE: [EXT] Re: [PATCH v2] mwifiex: don't call del_timer_sync() on uninitialized timer
Hi Tetsuo, > > "grep -FrB1 'del_timer' drivers/ | grep -FA1 '.function)'" says that > > currently there are 28 locations which call del_timer[_sync]() only if > > that timer's function field was initialized (because timer_setup() > > sets that timer's function field). Therefore, let's use same approach here. Thanks for the change, it look cleaner than my re-work; Acked-by: Ganapathi Bhat Regards, Ganapathi
[tip:ras/core] BUILD SUCCESS 1e36d9c6886849c6f3d3c836370563e6bc1a6ddd
allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc defconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a002-20200826 i386 randconfig-a004-20200826 i386 randconfig-a003-20200826 i386 randconfig-a005-20200826 i386 randconfig-a006-20200826 i386 randconfig-a001-20200826 x86_64 randconfig-a015-20200826 x86_64 randconfig-a016-20200826 x86_64 randconfig-a012-20200826 x86_64 randconfig-a014-20200826 x86_64 randconfig-a011-20200826 x86_64 randconfig-a013-20200826 i386 randconfig-a013-20200826 i386 randconfig-a012-20200826 i386 randconfig-a011-20200826 i386 randconfig-a016-20200826 i386 randconfig-a015-20200826 i386 randconfig-a014-20200826 riscvallyesconfig riscv allnoconfig riscv defconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 rhel-8.3 x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH v36 22/24] selftests/x86: Add a selftest for SGX
On Thu, Jul 16, 2020 at 9:58 AM Jarkko Sakkinen wrote: > > Add a selftest for SGX. It is a trivial test where a simple enclave > copies one 64-bit word of memory between two memory locations. > > Cc: linux-kselft...@vger.kernel.org > Signed-off-by: Jarkko Sakkinen > --- > tools/testing/selftests/Makefile | 1 + > tools/testing/selftests/sgx/.gitignore| 2 + > tools/testing/selftests/sgx/Makefile | 53 +++ > tools/testing/selftests/sgx/call.S| 54 +++ > tools/testing/selftests/sgx/defines.h | 21 + > tools/testing/selftests/sgx/load.c| 282 + > tools/testing/selftests/sgx/main.c| 199 + > tools/testing/selftests/sgx/main.h| 38 ++ > tools/testing/selftests/sgx/sigstruct.c | 395 ++ > tools/testing/selftests/sgx/test_encl.c | 20 + > tools/testing/selftests/sgx/test_encl.lds | 40 ++ > .../selftests/sgx/test_encl_bootstrap.S | 89 > 12 files changed, 1194 insertions(+) > create mode 100644 tools/testing/selftests/sgx/.gitignore > create mode 100644 tools/testing/selftests/sgx/Makefile > create mode 100644 tools/testing/selftests/sgx/call.S > create mode 100644 tools/testing/selftests/sgx/defines.h > create mode 100644 tools/testing/selftests/sgx/load.c > create mode 100644 tools/testing/selftests/sgx/main.c > create mode 100644 tools/testing/selftests/sgx/main.h > create mode 100644 tools/testing/selftests/sgx/sigstruct.c > create mode 100644 tools/testing/selftests/sgx/test_encl.c > create mode 100644 tools/testing/selftests/sgx/test_encl.lds > create mode 100644 tools/testing/selftests/sgx/test_encl_bootstrap.S > > diff --git a/tools/testing/selftests/Makefile > b/tools/testing/selftests/Makefile > index 1195bd85af38..ec7be6d5a10d 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -64,6 +64,7 @@ TARGETS += user > TARGETS += vm > TARGETS += x86 > TARGETS += zram > +TARGETS += sgx > #Please keep the TARGETS list alphabetically sorted > # Run "make quicktest=1 run_tests" or > # "make quicktest=1 kselftest" from top level Makefile > diff --git a/tools/testing/selftests/sgx/.gitignore > b/tools/testing/selftests/sgx/.gitignore > new file mode 100644 > index ..fbaf0bda9a92 > --- /dev/null > +++ b/tools/testing/selftests/sgx/.gitignore > @@ -0,0 +1,2 @@ > +test_sgx > +test_encl.elf > diff --git a/tools/testing/selftests/sgx/Makefile > b/tools/testing/selftests/sgx/Makefile > new file mode 100644 > index ..95e5c4df8014 > --- /dev/null > +++ b/tools/testing/selftests/sgx/Makefile > @@ -0,0 +1,53 @@ > +top_srcdir = ../../../.. > + > +include ../lib.mk > + > +.PHONY: all clean > + > +CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh $(CC) \ > + ../x86/trivial_64bit_program.c) > + > +ifndef OBJCOPY > +OBJCOPY := $(CROSS_COMPILE)objcopy > +endif > + > +INCLUDES := -I$(top_srcdir)/tools/include > +HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack > +ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \ > + -fno-stack-protector -mrdrnd $(INCLUDES) > + > +TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx $(OUTPUT)/test_encl.elf > + > +ifeq ($(CAN_BUILD_X86_64), 1) > +all: $(TEST_CUSTOM_PROGS) > +endif > + > +$(OUTPUT)/test_sgx: $(OUTPUT)/main.o \ > + $(OUTPUT)/load.o \ > + $(OUTPUT)/sigstruct.o \ > + $(OUTPUT)/call.o > + $(CC) $(HOST_CFLAGS) -o $@ $^ -lcrypto > + > +$(OUTPUT)/main.o: main.c > + $(CC) $(HOST_CFLAGS) -c $< -o $@ > + > +$(OUTPUT)/load.o: load.c > + $(CC) $(HOST_CFLAGS) -c $< -o $@ > + > +$(OUTPUT)/sigstruct.o: sigstruct.c > + $(CC) $(HOST_CFLAGS) -c $< -o $@ > + > +$(OUTPUT)/call.o: call.S > + $(CC) $(HOST_CFLAGS) -c $< -o $@ > + > +$(OUTPUT)/test_encl.elf: test_encl.lds test_encl.c test_encl_bootstrap.S > + $(CC) $(ENCL_CFLAGS) -T $^ -o $@ > + > +EXTRA_CLEAN := \ > + $(OUTPUT)/test_encl.elf \ > + $(OUTPUT)/load.o \ > + $(OUTPUT)/call.o \ > + $(OUTPUT)/main.o \ > + $(OUTPUT)/sigstruct.o \ > + $(OUTPUT)/test_sgx \ > + $(OUTPUT)/test_sgx.o \ > diff --git a/tools/testing/selftests/sgx/call.S > b/tools/testing/selftests/sgx/call.S > new file mode 100644 > index ..77131e83db42 > --- /dev/null > +++ b/tools/testing/selftests/sgx/call.S > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ > +/** > +* Copyright(c) 2016-18 Intel Corporation. > +*/ > + > + .text > + > + .macro ENCLU > + .byte 0x0f, 0x01, 0xd7 > + .endm > + > + .text > + > + .global sgx_call_vdso > +sgx_call_vdso: > + .cfi_startproc > + push%r15 > + .cfi_adjust_cfa_offset 8 > + .cfi_rel_offset %r15, 0 > + push%r14 > + .cfi_adjust_cfa_offset 8 > + .cfi_rel_offset %r14, 0 > + pu