soundwire: Fix duplicate stream state assignment
Subsystems (3)
Sasha Levin
maintainerPATCH 1/58 soundwire: Fix duplicate stream state assignment
From: Shreyas NC <[email protected]>
[ Upstream commit 0aebe40bae6cf5652fdc3d05ecee15fbf5748194 ]
For a SoundWire stream it is expected that a Slave is added to the stream before Master is added.
So, move the stream state to CONFIGURED after the first Slave is added and remove the stream state assignment for Master add. Along with these changes, add additional comments to explain the same.
--- drivers/soundwire/stream.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
drivers/soundwire/stream.c +10 -2
@@ -1123,8 +1123,6 @@ int sdw_stream_add_master(struct sdw_bus *bus,if (ret) goto stream_error; - stream->state =SDW_STREAM_CONFIGURED ; - stream_error:sdw_release_master_stream (stream); error:
@@ -1141,6 +1139,10 @@ EXPORT_SYMBOL(sdw_stream_add_master);* @stream: SoundWire stream * @port_config: Port configuration for audio stream * @num_ports: Number of ports +* +* It is expected that Slave is added before adding Master +* to the Stream. +* */ int sdw_stream_add_slave(struct sdw_slave *slave, struct sdw_stream_config *stream_config,
@@ -1186,6 +1188,12 @@ int sdw_stream_add_slave(struct sdw_slave *slave,if (ret) goto stream_error; +/* +* Change stream state to CONFIGURED on first Slave add. +* Bus is not aware of number of Slave(s) in a stream at this +* point so cannot depend on all Slave(s) to be added in order to +* change stream state to CONFIGURED. +*/ stream->state =SDW_STREAM_CONFIGURED ; goto error;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 2/58 soundwire: Fix incorrect exit after configuring stream
From: Shreyas NC <[email protected]>
[ Upstream commit 3fef1a2259c556cce34df2791688cb3001f81c92 ]
In sdw_stream_add_master() after the Master ports are configured, the stream is released incorrectly.
So, fix it by avoiding stream release after configuring the Master for the stream.
While at it, rename the label appropriately.
--- drivers/soundwire/stream.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/soundwire/stream.c +4 -2
@@ -1112,7 +1112,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,"Master runtime config failed for stream:%s" , stream->name); ret = -ENOMEM ; - goto error; + goto unlock; } ret =sdw_config_stream (bus->dev, stream, stream_config, false);
@@ -1123,9 +1123,11 @@ int sdw_stream_add_master(struct sdw_bus *bus,if (ret) goto stream_error; + goto unlock; + stream_error:sdw_release_master_stream (stream); -error: +unlock:mutex_unlock (&bus->bus_lock);return ret; }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 3/58 soundwire: Fix acquiring bus lock twice during master release
In reply to Sasha LevinFrom: Sanyog Kale <[email protected]>
[ Upstream commit 8d6ccf5cebbc7ed1dee9986e36853a78dfb64084 ]
As part of sdw_stream_remove_master(), sdw_stream_remove_slave() is called which results in bus lock being acquired twice.
So, fix it by performing specific Slave remove operations in sdw_release_master_stream() instead of calling sdw_stream_remove_slave().
--- drivers/soundwire/stream.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/soundwire/stream.c +4 -3
@@ -899,9 +899,10 @@ static void sdw_release_master_stream(struct sdw_stream_runtime *stream)struct sdw_master_runtime *m_rt = stream->m_rt;struct sdw_slave_runtime *s_rt, *_s_rt; -list_for_each_entry_safe (s_rt, _s_rt, - &m_rt->slave_rt_list, m_rt_node) -sdw_stream_remove_slave (s_rt->slave, stream); +list_for_each_entry_safe (s_rt, _s_rt, &m_rt->slave_rt_list, m_rt_node) { +sdw_slave_port_release (s_rt->slave->bus, s_rt->slave, stream); +sdw_release_slave_stream (s_rt->slave, stream); + }list_del (&m_rt->bus_node); }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 4/58 media: af9035: prevent buffer overflow on write
In reply to Sasha LevinFrom: Jozef Balga <[email protected]>
[ Upstream commit 312f73b648626a0526a3aceebb0a3192aaba05ce ]
When less than 3 bytes are written to the device, memcpy is called with negative array size which leads to buffer overflow and kernel panic. This patch adds a condition and returns -EOPNOTSUPP instead.
Fixes bugzilla issue 64871
[[email protected]: fix a merge conflict and changed the
condition to match the patch's comment, e. g. len == 3 could also be valid]
--- drivers/media/usb/dvb-usb-v2/af9035.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/media/usb/dvb-usb-v2/af9035.c +4 -2
@@ -402,8 +402,10 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap,if (msg[0 ].addr == state->af9033_i2c_addr[1 ]) reg |=0x100000 ; - ret =af9035_wr_regs (d, reg, &msg[0 ].buf[3 ], - msg[0 ].len -3 ); + ret = (msg[0 ].len >=3 ) ?af9035_wr_regs (d, reg, + &msg[0 ].buf[3 ], + msg[0 ].len -3 ) + : -EOPNOTSUPP ; }else {/* I2C write */ u8 buf[MAX_XFER_SIZE ];
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 5/58 spi: gpio: Fix copy-and-paste error
In reply to Sasha LevinFrom: Linus Walleij <[email protected]>
[ Upstream commit 1723c3155f117ee6e00f28fadf6e9eda4fc85806 ]
This fixes an embarrassing copy-and-paste error in the errorpath of spi_gpio_request(): we were checking the wrong struct member for error code right after retrieveing the sck GPIO.
--- drivers/spi/spi-gpio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/spi/spi-gpio.c +2 -2
@@ -287,8 +287,8 @@ static int spi_gpio_request(struct device *dev, *mflags |=SPI_MASTER_NO_RX ; spi_gpio->sck =devm_gpiod_get (dev,"sck" ,GPIOD_OUT_LOW ); -if (IS_ERR (spi_gpio->mosi)) -return PTR_ERR (spi_gpio->mosi); +if (IS_ERR (spi_gpio->sck)) +return PTR_ERR (spi_gpio->sck);for (i =0 ; i < num_chipselects; i++) { spi_gpio->cs_gpios[i] = devm_gpiod_get_index(dev, "cs",
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 6/58 batman-adv: Avoid probe ELP information leak
In reply to Sasha LevinFrom: Sven Eckelmann <[email protected]>
[ Upstream commit 88d0895d0ea9d4431507d576c963f2ff9918144d ]
The probe ELPs for WiFi interfaces are expanded to contain at least BATADV_ELP_MIN_PROBE_SIZE bytes. This is usually a lot more than the number of bytes which the template ELP packet requires.
These extra padding bytes were not initialized and thus could contain data which were previously stored at the same location. It is therefore required to set it to some predefined or random values to avoid leaking private information from the system transmitting these kind of packets.
--- net/batman-adv/bat_v_elp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/batman-adv/bat_v_elp.c +1 -1
@@ -241,7 +241,7 @@ batadv_v_elp_wifi_neigh_probe(struct batadv_hardif_neigh_node *neigh)* the packet to be exactly of that size to make the link * throughput estimation effective. */ -skb_put (skb, probe_len - hard_iface->bat_v.elp_skb->len); +skb_put_zero (skb, probe_len - hard_iface->bat_v.elp_skb->len); batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Sending unicast (probe) ELP packet on interface %s to %pM\n",
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 7/58 batman-adv: Fix segfault when writing to throughput_override
In reply to Sasha LevinFrom: Sven Eckelmann <[email protected]>
[ Upstream commit b9fd14c20871e6189f635e49b32d7789e430b3c8 ]
The per hardif sysfs file "batman_adv/throughput_override" prints the resulting change as info text when the users writes to this file. It uses the helper function batadv_info to add it at the same time to the kernel ring buffer and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG is enabled).
The function batadv_info requires as first parameter the batman-adv softif net_device. This parameter is then used to find the private buffer which contains the debug log for this batman-adv interface. But batadv_store_throughput_override used as first argument the slave net_device. This slave device doesn't have the batadv_priv private data which is access by batadv_info.
Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead to a segfault or to memory corruption.
--- net/batman-adv/sysfs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
net/batman-adv/sysfs.c +3 -2
@@ -1090,8 +1090,9 @@ static ssize_t batadv_store_throughput_override(struct kobject *kobj,if (old_tp_override == tp_override) goto out; -batadv_info (net_dev,"%s: Changing from: %u.%u MBit to: %u.%u MBit\n" , -"throughput_override" , +batadv_info (hard_iface->soft_iface, +"%s: %s: Changing from: %u.%u MBit to: %u.%u MBit\n" , +"throughput_override" , net_dev->name, old_tp_override /10 , old_tp_override %10 , tp_override /10 , tp_override %10 );
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 8/58 batman-adv: Fix segfault when writing to sysfs elp_interval
In reply to Sasha LevinFrom: Sven Eckelmann <[email protected]>
[ Upstream commit a25bab9d723a08bd0bdafb1529faf9094c690b70 ]
The per hardif sysfs file "batman_adv/elp_interval" is using the generic functions to store/show uint values. The helper __batadv_store_uint_attr requires the softif net_device as parameter to print the resulting change as info text when the users writes to this file. It uses the helper function batadv_info to add it at the same time to the kernel ring buffer and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG is enabled).
The function batadv_info requires as first parameter the batman-adv softif net_device. This parameter is then used to find the private buffer which contains the debug log for this batman-adv interface. But batadv_store_throughput_override used as first argument the slave net_device. This slave device doesn't have the batadv_priv private data which is access by batadv_info.
Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead to a segfault or to memory corruption.
--- net/batman-adv/sysfs.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)
net/batman-adv/sysfs.c +17 -8
@@ -188,7 +188,8 @@ ssize_t batadv_store_##_name(struct kobject *kobj, \ \return __batadv_store_uint_attr (buff, count, _min, _max, \ _post_func, attr, \ - &bat_priv->_var, net_dev); \ + &bat_priv->_var, net_dev, \ +NULL ); \ }#define BATADV_ATTR_SIF_SHOW_UINT (_name, _var) \
@@ -262,7 +263,9 @@ ssize_t batadv_store_##_name(struct kobject *kobj, \ \ length =__batadv_store_uint_attr (buff, count, _min, _max, \ _post_func, attr, \ - &hard_iface->_var, net_dev); \ + &hard_iface->_var, \ + hard_iface->soft_iface, \ + net_dev); \ \batadv_hardif_put (hard_iface); \return length; \
@@ -356,10 +359,12 @@ __batadv_store_bool_attr(char *buff, size_t count,static int batadv_store_uint_attr (const char *buff,size_t count,struct net_device *net_dev, +struct net_device *slave_dev,const char *attr_name,unsigned int min,unsigned int max,atomic_t *attr) { +char ifname[IFNAMSIZ +3 ] ="" ;unsigned long uint_val;int ret;
@@ -385,8 +390,11 @@ static int batadv_store_uint_attr(const char *buff, size_t count,if (atomic_read (attr) == uint_val)return count; -batadv_info (net_dev,"%s: Changing from: %i to: %lu\n" , - attr_name,atomic_read (attr), uint_val); +if (slave_dev) +snprintf (ifname,sizeof (ifname),"%s: " , slave_dev->name); + +batadv_info (net_dev,"%s: %sChanging from: %i to: %lu\n" , + attr_name, ifname,atomic_read (attr), uint_val);atomic_set (attr, uint_val);return count;
@@ -397,12 +405,13 @@ static ssize_t __batadv_store_uint_attr(const char *buff, size_t count,void (*post_func)(struct net_device *), const struct attribute *attr, atomic_t *attr_store, - struct net_device *net_dev) + struct net_device *net_dev, + struct net_device *slave_dev) { int ret; - ret =batadv_store_uint_attr (buff, count, net_dev, attr->name, min, max, - attr_store); + ret =batadv_store_uint_attr (buff, count, net_dev, slave_dev, + attr->name, min, max, attr_store);if (post_func && ret)post_func (net_dev);
@@ -571,7 +580,7 @@ static ssize_t batadv_store_gw_sel_class(struct kobject *kobj,return __batadv_store_uint_attr (buff, count,1 ,BATADV_TQ_MAX_VALUE , batadv_post_gw_reselect, attr, &bat_priv->gw.sel_class, - bat_priv->soft_iface); + bat_priv->soft_iface,NULL ); } static ssize_t batadv_show_gw_bwidth(struct kobject *kobj,
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 9/58 batman-adv: Prevent duplicated gateway_node entry
In reply to Sasha LevinFrom: Sven Eckelmann <[email protected]>
[ Upstream commit dff9bc42ab0b2d38c5e90ddd79b238fed5b4c7ad ]
The function batadv_gw_node_add is responsible for adding new gw_node to the gateway_list. It is expecting that the caller already checked that there is not already an entry with the same key or not.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
--- net/batman-adv/gateway_client.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
net/batman-adv/gateway_client.c +9 -2
@@ -32,6 +32,7 @@#include <linux/kernel.h> #include <linux/kref.h> #include <linux/list.h> +#include <linux/lockdep.h> #include <linux/netdevice.h> #include <linux/netlink.h> #include <linux/rculist.h>
@@ -348,6 +349,9 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv,* @bat_priv: the bat priv with all the soft interface information * @orig_node: originator announcing gateway capabilities * @gateway: announced bandwidth information +* +* Has to be called with the appropriate locks being acquired +* (gw.list_lock). */ static void batadv_gw_node_add(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node,
@@ -355,6 +359,8 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv, {struct batadv_gw_node *gw_node; +lockdep_assert_held (&bat_priv->gw.list_lock); +if (gateway->bandwidth_down ==0 )return ;
@@ -369,10 +375,8 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv, gw_node->bandwidth_down =ntohl (gateway->bandwidth_down); gw_node->bandwidth_up =ntohl (gateway->bandwidth_up); -spin_lock_bh (&bat_priv->gw.list_lock);kref_get (&gw_node->refcount);hlist_add_head_rcu (&gw_node->list, &bat_priv->gw.gateway_list); -spin_unlock_bh (&bat_priv->gw.list_lock); batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Found new gateway %pM -> gw bandwidth: %u.%u/%u.%u MBit\n",
@@ -428,11 +432,14 @@ void batadv_gw_node_update(struct batadv_priv *bat_priv, {struct batadv_gw_node *gw_node, *curr_gw =NULL ; +spin_lock_bh (&bat_priv->gw.list_lock); gw_node =batadv_gw_node_get (bat_priv, orig_node);if (!gw_node) {batadv_gw_node_add (bat_priv, orig_node, gateway); +spin_unlock_bh (&bat_priv->gw.list_lock); goto out; } +spin_unlock_bh (&bat_priv->gw.list_lock);if (gw_node->bandwidth_down ==ntohl (gateway->bandwidth_down) && gw_node->bandwidth_up ==ntohl (gateway->bandwidth_up))
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 10/58 batman-adv: Prevent duplicated nc_node entry
In reply to Sasha LevinFrom: Sven Eckelmann <[email protected]>
[ Upstream commit fa122fec8640eb7186ce5a41b83a4c1744ceef8f ]
The function batadv_nc_get_nc_node is responsible for adding new nc_nodes to the in_coding_list and out_coding_list. It first checks whether the entry already is in the list or not. If it is, then the creation of a new entry is aborted.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
--- net/batman-adv/network-coding.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)
net/batman-adv/network-coding.c +15 -12
@@ -854,16 +854,27 @@ batadv_nc_get_nc_node(struct batadv_priv *bat_priv,spinlock_t *lock;/* Used to lock list selected by "int in_coding" */ struct list_head *list; +/* Select ingoing or outgoing coding node */ +if (in_coding) { + lock = &orig_neigh_node->in_coding_list_lock; + list = &orig_neigh_node->in_coding_list; + }else { + lock = &orig_neigh_node->out_coding_list_lock; + list = &orig_neigh_node->out_coding_list; + } + +spin_lock_bh (lock); +/* Check if nc_node is already added */ nc_node =batadv_nc_find_nc_node (orig_node, orig_neigh_node, in_coding);/* Node found */ if (nc_node) -return nc_node; + goto unlock; nc_node =kzalloc (sizeof (*nc_node),GFP_ATOMIC );if (!nc_node) -return NULL ; + goto unlock;/* Initialize nc_node */ INIT_LIST_HEAD (&nc_node->list);
@@ -872,22 +883,14 @@ batadv_nc_get_nc_node(struct batadv_priv *bat_priv,kref_get (&orig_neigh_node->refcount); nc_node->orig_node = orig_neigh_node; -/* Select ingoing or outgoing coding node */ -if (in_coding) { - lock = &orig_neigh_node->in_coding_list_lock; - list = &orig_neigh_node->in_coding_list; - }else { - lock = &orig_neigh_node->out_coding_list_lock; - list = &orig_neigh_node->out_coding_list; - } -batadv_dbg (BATADV_DBG_NC , bat_priv,"Adding nc_node %pM -> %pM\n" , nc_node->addr, nc_node->orig_node->orig);/* Add nc_node to orig_node */ -spin_lock_bh (lock);kref_get (&nc_node->refcount);list_add_tail_rcu (&nc_node->list, list); + +unlock:spin_unlock_bh (lock);return nc_node;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 11/58 batman-adv: Prevent duplicated softif_vlan entry
In reply to Sasha LevinFrom: Sven Eckelmann <[email protected]>
[ Upstream commit 94cb82f594ed86be303398d6dfc7640a6f1d45d4 ]
The function batadv_softif_vlan_get is responsible for adding new softif_vlan to the softif_vlan_list. It first checks whether the entry already is in the list or not. If it is, then the creation of a new entry is aborted.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
--- net/batman-adv/soft-interface.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)
net/batman-adv/soft-interface.c +18 -7
@@ -574,15 +574,20 @@ int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)struct batadv_softif_vlan *vlan;int err; +spin_lock_bh (&bat_priv->softif_vlan_list_lock); + vlan =batadv_softif_vlan_get (bat_priv, vid);if (vlan) {batadv_softif_vlan_put (vlan); +spin_unlock_bh (&bat_priv->softif_vlan_list_lock);return -EEXIST ; } vlan =kzalloc (sizeof (*vlan),GFP_ATOMIC ); -if (!vlan) +if (!vlan) { +spin_unlock_bh (&bat_priv->softif_vlan_list_lock);return -ENOMEM ; + } vlan->bat_priv = bat_priv; vlan->vid = vid;
@@ -590,17 +595,23 @@ int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)atomic_set (&vlan->ap_isolation,0 ); +kref_get (&vlan->refcount); +hlist_add_head_rcu (&vlan->list, &bat_priv->softif_vlan_list); +spin_unlock_bh (&bat_priv->softif_vlan_list_lock); + +/* batadv_sysfs_add_vlan cannot be in the spinlock section due to the +* sleeping behavior of the sysfs functions and the fs_reclaim lock +*/ err =batadv_sysfs_add_vlan (bat_priv->soft_iface, vlan);if (err) { -kfree (vlan); +/* ref for the function */ +batadv_softif_vlan_put (vlan); + +/* ref for the list */ +batadv_softif_vlan_put (vlan);return err; } -spin_lock_bh (&bat_priv->softif_vlan_list_lock); -kref_get (&vlan->refcount); -hlist_add_head_rcu (&vlan->list, &bat_priv->softif_vlan_list); -spin_unlock_bh (&bat_priv->softif_vlan_list_lock); -/* add a new TT local entry. This one will be marked with the NOPURGE * flag */
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 12/58 batman-adv: Prevent duplicated global TT entry
In reply to Sasha LevinFrom: Sven Eckelmann <[email protected]>
[ Upstream commit e7136e48ffdfb9f37b0820f619380485eb407361 ]
The function batadv_tt_global_orig_entry_add is responsible for adding new tt_orig_list_entry to the orig_list. It first checks whether the entry already is in the list or not. If it is, then the creation of a new entry is aborted.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
--- net/batman-adv/translation-table.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
net/batman-adv/translation-table.c +4 -2
@@ -1613,6 +1613,8 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, {struct batadv_tt_orig_list_entry *orig_entry; +spin_lock_bh (&tt_global->list_lock); + orig_entry =batadv_tt_global_orig_entry_find (tt_global, orig_node);if (orig_entry) {/* refresh the ttvn: the current value could be a bogus one that
@@ -1635,11 +1637,9 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, orig_entry->flags = flags;kref_init (&orig_entry->refcount); -spin_lock_bh (&tt_global->list_lock);kref_get (&orig_entry->refcount);hlist_add_head_rcu (&orig_entry->list, &tt_global->orig_list); -spin_unlock_bh (&tt_global->list_lock);atomic_inc (&tt_global->orig_list_count); sync_flags:
@@ -1647,6 +1647,8 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global, out:if (orig_entry)batadv_tt_orig_list_entry_put (orig_entry); + +spin_unlock_bh (&tt_global->list_lock); }/**
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 13/58 batman-adv: Prevent duplicated tvlv handler
In reply to Sasha LevinFrom: Sven Eckelmann <[email protected]>
[ Upstream commit ae3cdc97dc10c7a3b31f297dab429bfb774c9ccb ]
The function batadv_tvlv_handler_register is responsible for adding new tvlv_handler to the handler_list. It first checks whether the entry already is in the list or not. If it is, then the creation of a new entry is aborted.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
--- net/batman-adv/tvlv.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
net/batman-adv/tvlv.c +6 -2
@@ -529,15 +529,20 @@ void batadv_tvlv_handler_register(struct batadv_priv *bat_priv, {struct batadv_tvlv_handler *tvlv_handler; +spin_lock_bh (&bat_priv->tvlv.handler_list_lock); + tvlv_handler =batadv_tvlv_handler_get (bat_priv, type, version);if (tvlv_handler) { +spin_unlock_bh (&bat_priv->tvlv.handler_list_lock);batadv_tvlv_handler_put (tvlv_handler);return ; } tvlv_handler =kzalloc (sizeof (*tvlv_handler),GFP_ATOMIC ); -if (!tvlv_handler) +if (!tvlv_handler) { +spin_unlock_bh (&bat_priv->tvlv.handler_list_lock);return ; + } tvlv_handler->ogm_handler = optr; tvlv_handler->unicast_handler = uptr;
@@ -547,7 +552,6 @@ void batadv_tvlv_handler_register(struct batadv_priv *bat_priv,kref_init (&tvlv_handler->refcount);INIT_HLIST_NODE (&tvlv_handler->list); -spin_lock_bh (&bat_priv->tvlv.handler_list_lock);kref_get (&tvlv_handler->refcount);hlist_add_head_rcu (&tvlv_handler->list, &bat_priv->tvlv.handler_list);spin_unlock_bh (&bat_priv->tvlv.handler_list_lock);
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 14/58 batman-adv: fix backbone_gw refcount on queue_work() failure
In reply to Sasha LevinFrom: Marek Lindner <[email protected]>
[ Upstream commit 5af96b9c59c72fb2af2d19c5cc2f3cdcee391dff ]
The backbone_gw refcounter is to be decreased by the queued work and currently is never decreased if the queue_work() call fails. Fix by checking the queue_work() return value and decrease refcount if necessary.
--- net/batman-adv/bridge_loop_avoidance.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
net/batman-adv/bridge_loop_avoidance.c +8 -2
@@ -1772,6 +1772,7 @@ batadv_bla_loopdetect_check(struct batadv_priv *bat_priv, struct sk_buff *skb, {struct batadv_bla_backbone_gw *backbone_gw;struct ethhdr *ethhdr; +bool ret; ethhdr =eth_hdr (skb);
@@ -1795,8 +1796,13 @@ batadv_bla_loopdetect_check(struct batadv_priv *bat_priv, struct sk_buff *skb,if (unlikely (!backbone_gw))return true; -queue_work (batadv_event_workqueue, &backbone_gw->report_work); -/* backbone_gw is unreferenced in the report work function function */ + ret =queue_work (batadv_event_workqueue, &backbone_gw->report_work); + +/* backbone_gw is unreferenced in the report work function function +* if queue_work() call was successful +*/ +if (!ret) +batadv_backbone_gw_put (backbone_gw);return true; }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 15/58 batman-adv: fix hardif_neigh refcount on queue_work() failure
In reply to Sasha LevinFrom: Marek Lindner <[email protected]>
[ Upstream commit 4c4af6900844ab04c9434c972021d7b48610e06a ]
The hardif_neigh refcounter is to be decreased by the queued work and currently is never decreased if the queue_work() call fails. Fix by checking the queue_work() return value and decrease refcount if necessary.
--- net/batman-adv/bat_v_elp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
net/batman-adv/bat_v_elp.c +6 -2
@@ -268,6 +268,7 @@ static void batadv_v_elp_periodic_work(struct work_struct *work)struct batadv_priv *bat_priv;struct sk_buff *skb;u32 elp_interval; +bool ret; bat_v = container_of(work, struct batadv_hard_iface_bat_v, elp_wq.work); hard_iface = container_of(bat_v, struct batadv_hard_iface, bat_v);
@@ -329,8 +330,11 @@ static void batadv_v_elp_periodic_work(struct work_struct *work)* may sleep and that is not allowed in an rcu protected * context. Therefore schedule a task for that. */ -queue_work (batadv_event_workqueue, - &hardif_neigh->bat_v.metric_work); + ret =queue_work (batadv_event_workqueue, + &hardif_neigh->bat_v.metric_work); + +if (!ret) +batadv_hardif_neigh_put (hardif_neigh); }rcu_read_unlock ();
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 16/58 cxgb4: fix abort_req_rss6 struct
In reply to Sasha LevinFrom: Steve Wise <[email protected]>
[ Upstream commit 9f34519a82356f6cf0ccb8480ee0ed99b3d0af75 ]
Remove the incorrect WR_HDR field which can cause a misinterpretation of ABORT CPL by ULDs, such as iw_cxgb4.
--- drivers/net/ethernet/chelsio/cxgb4/t4_msg.h | 1 - 1 file changed, 1 deletion(-)
drivers/net/ethernet/chelsio/cxgb4/t4_msg.h +0 -1
@@ -753,7 +753,6 @@ struct cpl_abort_req_rss { };struct cpl_abort_req_rss6 { -WR_HDR ;union opcode_tid ot;__u32 srqidx_status; };
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 17/58 clocksource/drivers/ti-32k: Add CLOCK_SOURCE_SUSPEND_NONSTOP flag for non-am43 SoCs
In reply to Sasha LevinFrom: Keerthy <[email protected]>
[ Upstream commit 3b7d96a0dbb6b630878597a1838fc39f808b761b ]
The 32k clocksource is NONSTOP for non-am43 SoCs. Hence add the flag for all the other SoCs.
--- drivers/clocksource/timer-ti-32k.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/clocksource/timer-ti-32k.c +3 -0
@@ -98,6 +98,9 @@ static int __init ti_32k_timer_init(struct device_node *np)return -ENXIO ; } +if (!of_machine_is_compatible ("ti,am43" )) + ti_32k_timer.cs.flags |=CLOCK_SOURCE_SUSPEND_NONSTOP ; + ti_32k_timer.counter = ti_32k_timer.base;/*
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 18/58 ucma: fix a use-after-free in ucma_resolve_ip()
In reply to Sasha LevinFrom: Cong Wang <[email protected]>
[ Upstream commit 5fe23f262e0548ca7f19fb79f89059a60d087d22 ]
There is a race condition between ucma_close() and ucma_resolve_ip():
CPU0 CPU1ucma_resolve_ip(): ucma_close():
ctx = ucma_get_ctx(file, cmd.id);
list_for_each_entry_safe(ctx, tmp, &file->ctx_list, list) {
mutex_lock(&mut);
idr_remove(&ctx_idr, ctx->id);
mutex_unlock(&mut);
...
mutex_lock(&mut);
if (!ctx->closing) {
mutex_unlock(&mut);
rdma_destroy_id(ctx->cm_id);
...
ucma_free_ctx(ctx);ret = rdma_resolve_addr();ucma_put_ctx(ctx);
Before idr_remove(), ucma_get_ctx() could still find the ctx and after rdma_destroy_id(), rdma_resolve_addr() may still access id_priv pointer. Also, ucma_put_ctx() may use ctx after ucma_free_ctx() too.
ucma_close() should call ucma_put_ctx() too which tests the refcnt and waits for the last one releasing it. The similar pattern is already used by ucma_destroy_id().
--- drivers/infiniband/core/ucma.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/infiniband/core/ucma.c +2 -0
@@ -1753,6 +1753,8 @@ static int ucma_close(struct inode *inode, struct file *filp)mutex_lock (&mut);if (!ctx->closing) {mutex_unlock (&mut); +ucma_put_ctx (ctx); +wait_for_completion (&ctx->comp);/* rdma_destroy_id ensures that no event handlers are * inflight for that id before releasing it. */
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 19/58 scsi: ibmvscsis: Fix a stringop-overflow warning
In reply to Sasha LevinFrom: Laura Abbott <[email protected]>
[ Upstream commit d792d4c4fc866ae224b0b0ca2aabd87d23b4d6cc ]
There's currently a warning about string overflow with strncat:
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c: In function 'ibmvscsis_probe':drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:3479:2: error: 'strncat' specified bound 64 equals destination size [-Werror=stringop-overflow=]
strncat(vscsi->eye, vdev->name, MAX_EYE); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Switch to a single snprintf instead of a strcpy + strcat to handle this cleanly.
--- drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c +1 -2
@@ -3475,8 +3475,7 @@ static int ibmvscsis_probe(struct vio_dev *vdev, vscsi->dds.window[LOCAL ].liobn, vscsi->dds.window[REMOTE].liobn); -strcpy (vscsi->eye,"VSCSI " ); -strncat (vscsi->eye, vdev->name,MAX_EYE ); +snprintf (vscsi->eye,sizeof (vscsi->eye),"VSCSI %s" , vdev->name); vscsi->dds.unit_id = vdev->unit_address; strncpy(vscsi->dds.partition_name, partition_name,
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 20/58 scsi: ibmvscsis: Ensure partition name is properly NUL terminated
In reply to Sasha LevinFrom: Laura Abbott <[email protected]>
[ Upstream commit adad633af7b970bfa5dd1b624a4afc83cac9b235 ]
While reviewing another part of the code, Kees noticed that the strncpy of the partition name might not always be NUL terminated. Switch to using strscpy which does this safely.
--- drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c +1 -1
@@ -3478,7 +3478,7 @@ static int ibmvscsis_probe(struct vio_dev *vdev,snprintf (vscsi->eye,sizeof (vscsi->eye),"VSCSI %s" , vdev->name); vscsi->dds.unit_id = vdev->unit_address; -strncpy (vscsi->dds.partition_name, partition_name, +strscpy (vscsi->dds.partition_name, partition_name,sizeof (vscsi->dds.partition_name)); vscsi->dds.partition_num = partition_number;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 21/58 net/mlx5: Check for SQ and not RQ state when modifying hairpin SQ
In reply to Sasha LevinFrom: Alaa Hleihel <[email protected]>
[ Upstream commit 6b359d5550a1ae7a1269c9dc1dd73dfdc4d6fe58 ]
When modifying hairpin SQ, instead of checking if the next state equals to MLX5_SQC_STATE_RDY, we compare it against the MLX5_RQC_STATE_RDY enum value.
The code worked since both of MLX5_RQC_STATE_RDY and MLX5_SQC_STATE_RDY have the same value today.
This patch fixes this issue.
--- drivers/net/ethernet/mellanox/mlx5/core/transobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlx5/core/transobj.c +1 -1
@@ -509,7 +509,7 @@ static int mlx5_hairpin_modify_sq(struct mlx5_core_dev *peer_mdev, u32 sqn, sqc =MLX5_ADDR_OF (modify_sq_in, in, ctx); -if (next_state ==MLX5_RQC_STATE_RDY ) { +if (next_state ==MLX5_SQC_STATE_RDY ) {MLX5_SET (sqc, sqc, hairpin_peer_rq, peer_rq);MLX5_SET (sqc, sqc, hairpin_peer_vhca, peer_vhca); }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 22/58 intel_th: pci: Add Ice Lake PCH support
In reply to Sasha LevinFrom: Alexander Shishkin <[email protected]>
[ Upstream commit 59d08d00d43c644ee2011d7ff1807bdd69f31fe0 ]
This adds Intel(R) Trace Hub PCI ID for Ice Lake PCH.
--- drivers/hwtracing/intel_th/pci.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/hwtracing/intel_th/pci.c +5 -0
@@ -160,6 +160,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x18e1), .driver_data = (kernel_ulong_t)&intel_th_2x, }, + { +/* Ice Lake PCH */ + PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x34a6), + .driver_data = (kernel_ulong_t)&intel_th_2x, + }, { 0 }, };
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 23/58 Input: atakbd - fix Atari keymap
In reply to Sasha LevinFrom: Andreas Schwab <[email protected]>
[ Upstream commit 9e62df51be993035c577371ffee5477697a56aad ]
Fix errors in Atari keymap (mostly in keypad, help and undo keys).
Patch provided on debian-68k ML by Andreas Schwab <[email protected]>, keymap array size and unhandled scancode limit adjusted to 0x73 by me.
--- drivers/input/keyboard/atakbd.c | 64 ++++++++++++++------------------- 1 file changed, 26 insertions(+), 38 deletions(-)
drivers/input/keyboard/atakbd.c +26 -38
@@ -79,8 +79,7 @@ MODULE_LICENSE("GPL");*/ -static unsigned char atakbd_keycode[0x72 ] = {/* American layout */ - [0 ] =KEY_GRAVE , +static unsigned char atakbd_keycode[0x73 ] = {/* American layout */ [1 ] =KEY_ESC , [2 ] =KEY_1 , [3 ] =KEY_2 ,
@@ -121,9 +120,9 @@ static unsigned char atakbd_keycode[0x72] = { /* American layout */ [38] = KEY_L, [39] = KEY_SEMICOLON, [40] = KEY_APOSTROPHE, - [41] = KEY_BACKSLASH, /* FIXME, '#' */ + [41] = KEY_GRAVE, [42] = KEY_LEFTSHIFT, - [43] = KEY_GRAVE, /* FIXME: '~' */ + [43] = KEY_BACKSLASH, [44] = KEY_Z, [45] = KEY_X, [46] = KEY_C,
@@ -149,45 +148,34 @@ static unsigned char atakbd_keycode[0x72] = { /* American layout */ [66] = KEY_F8, [67] = KEY_F9, [68] = KEY_F10, - [69] = KEY_ESC, - [70] = KEY_DELETE, - [71] = KEY_KP7, - [72] = KEY_KP8, - [73] = KEY_KP9, + [71] = KEY_HOME, + [72] = KEY_UP, [74] = KEY_KPMINUS, - [75] = KEY_KP4, - [76] = KEY_KP5, - [77] = KEY_KP6, + [75] = KEY_LEFT, + [77] = KEY_RIGHT, [78] = KEY_KPPLUS, - [79] = KEY_KP1, - [80] = KEY_KP2, - [81] = KEY_KP3, - [82] = KEY_KP0, - [83] = KEY_KPDOT, - [90] = KEY_KPLEFTPAREN, - [91] = KEY_KPRIGHTPAREN, - [92] = KEY_KPASTERISK, /* FIXME */ - [93] = KEY_KPASTERISK, - [94] = KEY_KPPLUS, - [95] = KEY_HELP, + [80] = KEY_DOWN, + [82] = KEY_INSERT, + [83] = KEY_DELETE, [96] = KEY_102ND, - [97] = KEY_KPASTERISK, /* FIXME */ - [98] = KEY_KPSLASH, + [97] = KEY_UNDO, + [98] = KEY_HELP, [99] = KEY_KPLEFTPAREN, [100] = KEY_KPRIGHTPAREN, [101] = KEY_KPSLASH, [102] = KEY_KPASTERISK, - [103] = KEY_UP, - [104] = KEY_KPASTERISK, /* FIXME */ - [105] = KEY_LEFT, - [106] = KEY_RIGHT, - [107] = KEY_KPASTERISK, /* FIXME */ - [108] = KEY_DOWN, - [109] = KEY_KPASTERISK, /* FIXME */ - [110] = KEY_KPASTERISK, /* FIXME */ - [111] = KEY_KPASTERISK, /* FIXME */ - [112] = KEY_KPASTERISK, /* FIXME */ - [113] = KEY_KPASTERISK /* FIXME */ + [103] = KEY_KP7, + [104] = KEY_KP8, + [105] = KEY_KP9, + [106] = KEY_KP4, + [107] = KEY_KP5, + [108] = KEY_KP6, + [109] = KEY_KP1, + [110] = KEY_KP2, + [111] = KEY_KP3, + [112] = KEY_KP0, + [113] = KEY_KPDOT, + [114] = KEY_KPENTER, };static struct input_dev *atakbd_dev;
@@ -195,7 +183,7 @@ static struct input_dev *atakbd_dev;static void atakbd_interrupt (unsigned char scancode,char down) { -if (scancode <0x72 ) {/* scancodes < 0xf2 are keys */ +if (scancode <0x73 ) {/* scancodes < 0xf3 are keys */ // report raw events here?
@@ -209,7 +197,7 @@ static void atakbd_interrupt(unsigned char scancode, char down)input_report_key (atakbd_dev, scancode, down);input_sync (atakbd_dev); } - }else /* scancodes >= 0xf2 are mouse data, most likely */ + }else /* scancodes >= 0xf3 are mouse data, most likely */ printk (KERN_INFO "atakbd: unhandled scancode %x\n" , scancode);return ;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 24/58 Input: atakbd - fix Atari CapsLock behaviour
In reply to Sasha LevinFrom: Michael Schmitz <[email protected]>
[ Upstream commit 52d2c7bf7c90217fbe875d2d76f310979c48eb83 ]
The CapsLock key on Atari keyboards is not a toggle, it does send the normal make and break scancodes.
Drop the CapsLock toggle handling code, which did cause the CapsLock key to merely act as a Shift key.
--- drivers/input/keyboard/atakbd.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/input/keyboard/atakbd.c +2 -8
@@ -189,14 +189,8 @@ static void atakbd_interrupt(unsigned char scancode, char down) scancode = atakbd_keycode[scancode]; -if (scancode ==KEY_CAPSLOCK ) {/* CapsLock is a toggle switch key on Amiga */ -input_report_key (atakbd_dev, scancode,1 ); -input_report_key (atakbd_dev, scancode,0 ); -input_sync (atakbd_dev); - }else { -input_report_key (atakbd_dev, scancode, down); -input_sync (atakbd_dev); - } +input_report_key (atakbd_dev, scancode, down); +input_sync (atakbd_dev); }else /* scancodes >= 0xf3 are mouse data, most likely */ printk (KERN_INFO "atakbd: unhandled scancode %x\n" , scancode);
Signature · 1 line
2.17.1
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 10 lines · Sasha Levin
From: Michael Schmitz <[email protected]>
[ Upstream commit
52d2c7bf7c90217fbe875d2d76f310979c48eb83]The CapsLock key on Atari keyboards is not a toggle, it does send the normal make and break scancodes.
Drop the CapsLock toggle handling code, which did cause the CapsLock key to merely act as a Shift key.
This has been broken for 10+ years. Does it really make sense to promote it to stable?
Quoted 34 lines
Tested-by Michael Schmitz <[email protected]>Signed-off-by Michael Schmitz <[email protected]>Signed-off-by Andreas Schwab <[email protected]>Signed-off-by Dmitry Torokhov <[email protected]>Signed-off-by Sasha Levin <[email protected]>--- drivers/input/keyboard/atakbd.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)drivers/input/keyboard/atakbd.c +2
-9@@ -189,14 +189,8 @@ static void atakbd_interrupt(unsigned char scancode, char down) scancode = atakbd_keycode[scancode]; -if (scancode ==KEY_CAPSLOCK ) {/* CapsLock is a toggle switch key on Amiga */ -input_report_key (atakbd_dev, scancode,1 ); -input_report_key (atakbd_dev, scancode,0 ); -input_sync (atakbd_dev); - }else { -input_report_key (atakbd_dev, scancode, down); -input_sync (atakbd_dev); - } +input_report_key (atakbd_dev, scancode, down); +input_sync (atakbd_dev); }else /* scancodes >= 0xf3 are mouse data, most likely */ printk (KERN_INFO "atakbd: unhandled scancode %x\n" , scancode); --2.17.1
Signature · 1 line
Dmitry
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Dmitry,
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
Debian will carry stable patches without explicit action on behalf of the maintainer. Unstable patches are a little harder to get accepted.
Cheers,
Michael
Quoted 47 lines · Dmitry Torokhov
Quoted 9 lines · Sasha Levin
From: Michael Schmitz <[email protected]>
[ Upstream commit
52d2c7bf7c90217fbe875d2d76f310979c48eb83]The CapsLock key on Atari keyboards is not a toggle, it does send the normal make and break scancodes.
Drop the CapsLock toggle handling code, which did cause the CapsLock key to merely act as a Shift key.
This has been broken for 10+ years. Does it really make sense to promote it to stable?
Quoted 33 lines
Tested-by Michael Schmitz <[email protected]>Signed-off-by Michael Schmitz <[email protected]>Signed-off-by Andreas Schwab <[email protected]>Signed-off-by Dmitry Torokhov <[email protected]>Signed-off-by Sasha Levin <[email protected]>--- drivers/input/keyboard/atakbd.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)drivers/input/keyboard/atakbd.c +2
-9@@ -189,14 +189,8 @@ static void atakbd_interrupt(unsigned char scancode, char down) scancode = atakbd_keycode[scancode]; -if (scancode ==KEY_CAPSLOCK ) {/* CapsLock is a toggle switch key on Amiga */ -input_report_key (atakbd_dev, scancode,1 ); -input_report_key (atakbd_dev, scancode,0 ); -input_sync (atakbd_dev); - }else { -input_report_key (atakbd_dev, scancode, down); -input_sync (atakbd_dev); - } +input_report_key (atakbd_dev, scancode, down); +input_sync (atakbd_dev); }else /* scancodes >= 0xf3 are mouse data, most likely */ printk (KERN_INFO "atakbd: unhandled scancode %x\n" , scancode); --2.17.1
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Hi Michael,
Quoted 9 lines · Michael Schmitz
Dmitry,
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
I have exactly the same concerns about the keymap patch. This all has not been working correctly for many many years (and it was not broken in a subtly way as far as I understand, but rather quite obvious). Thus I do not understand why this belongs to stable release. It is not a [recent] regression, nor secutiry bug, nor even enabling of new hardware, that is why I myself did not mark it as stable.
I still maintain that we pick up for stable too many patches for no clear benefit. This is similar to the patch for Atmel controllers that was picked to stable and I asked why, as it is not clear how many users might be affected (or if the problem the patch was solving was purely theoretical, or only affecting hardware that is not in circulation yet).
Quoted 4 lines
Debian will carry stable patches without explicit action on behalf of the maintainer. Unstable patches are a little harder to get accepted.
Quoted 55 lines
Cheers,
Michael
Quoted 47 lines · Dmitry Torokhov
Quoted 9 lines · Sasha Levin
From: Michael Schmitz <[email protected]>
[ Upstream commit
52d2c7bf7c90217fbe875d2d76f310979c48eb83]The CapsLock key on Atari keyboards is not a toggle, it does send the normal make and break scancodes.
Drop the CapsLock toggle handling code, which did cause the CapsLock key to merely act as a Shift key.
This has been broken for 10+ years. Does it really make sense to promote it to stable?
Quoted 33 lines
Tested-by Michael Schmitz <[email protected]>Signed-off-by Michael Schmitz <[email protected]>Signed-off-by Andreas Schwab <[email protected]>Signed-off-by Dmitry Torokhov <[email protected]>Signed-off-by Sasha Levin <[email protected]>--- drivers/input/keyboard/atakbd.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)drivers/input/keyboard/atakbd.c +2
-9@@ -189,14 +189,8 @@ static void atakbd_interrupt(unsigned char scancode, char down) scancode = atakbd_keycode[scancode]; -if (scancode ==KEY_CAPSLOCK ) {/* CapsLock is a toggle switch key on Amiga */ -input_report_key (atakbd_dev, scancode,1 ); -input_report_key (atakbd_dev, scancode,0 ); -input_sync (atakbd_dev); - }else { -input_report_key (atakbd_dev, scancode, down); -input_sync (atakbd_dev); - } +input_report_key (atakbd_dev, scancode, down); +input_sync (atakbd_dev); }else /* scancodes >= 0xf3 are mouse data, most likely */ printk (KERN_INFO "atakbd: unhandled scancode %x\n" , scancode); --2.17.1
Thanks.
Signature · 1 line
Dmitry
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Hi Dmitry,
I agree the bug is neither subtle nor recent, not security relevant and will affect only a handful of users at best.
If you're worried about weakening the rules around stable releases, by all means go ahead and veto the inclusion of these patches in the next stable release.
Cheers,
Michael
Quoted 83 lines · Dmitry Torokhov
Hi Michael,
Quoted 8 lines · Michael Schmitz
Dmitry,
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
I have exactly the same concerns about the keymap patch. This all has not been working correctly for many many years (and it was not broken in a subtly way as far as I understand, but rather quite obvious). Thus I do not understand why this belongs to stable release. It is not a [recent] regression, nor secutiry bug, nor even enabling of new hardware, that is why I myself did not mark it as stable.
I still maintain that we pick up for stable too many patches for no clear benefit. This is similar to the patch for Atmel controllers that was picked to stable and I asked why, as it is not clear how many users might be affected (or if the problem the patch was solving was purely theoretical, or only affecting hardware that is not in circulation yet).
Quoted 53 lines
Debian will carry stable patches without explicit action on behalf of the maintainer. Unstable patches are a little harder to get accepted.
Cheers,
Michael
Quoted 43 lines · Dmitry Torokhov
Quoted 9 lines · Sasha Levin
From: Michael Schmitz <[email protected]>
[ Upstream commit
52d2c7bf7c90217fbe875d2d76f310979c48eb83]The CapsLock key on Atari keyboards is not a toggle, it does send the normal make and break scancodes.
Drop the CapsLock toggle handling code, which did cause the CapsLock key to merely act as a Shift key.
This has been broken for 10+ years. Does it really make sense to promote it to stable?
Quoted 30 lines
Tested-by Michael Schmitz <[email protected]>Signed-off-by Michael Schmitz <[email protected]>Signed-off-by Andreas Schwab <[email protected]>Signed-off-by Dmitry Torokhov <[email protected]>Signed-off-by Sasha Levin <[email protected]>--- drivers/input/keyboard/atakbd.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)drivers/input/keyboard/atakbd.c +2
-8@@ -189,14 +189,8 @@ static void atakbd_interrupt(unsigned char scancode, char down) scancode = atakbd_keycode[scancode]; -if (scancode ==KEY_CAPSLOCK ) {/* CapsLock is a toggle switch key on Amiga */ -input_report_key (atakbd_dev, scancode,1 ); -input_report_key (atakbd_dev, scancode,0 ); -input_sync (atakbd_dev); - }else { -input_report_key (atakbd_dev, scancode, down); -input_sync (atakbd_dev); - } +input_report_key (atakbd_dev, scancode, down); +input_sync (atakbd_dev); }else /* scancodes >= 0xf3 are mouse data, most likely */ printk (KERN_INFO "atakbd: unhandled scancode %x\n" , scancode);Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
--
2.17.1Thanks.
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Hi Michael,
Quoted 6 lines · Michael Schmitz
I agree the bug is neither subtle nor recent, not security relevant and will affect only a handful of users at best.
If you're worried about weakening the rules around stable releases, by all means go ahead and veto the inclusion of these patches in the next stable release.
I believe the distro the issue was reported against (Debian ports) will not get the fix until the issue is fixed in the upstream stable release?
Quoted 56 lines · Dmitry Torokhov
Quoted 55 lines · Michael Schmitz
Quoted 6 lines
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
I have exactly the same concerns about the keymap patch. This all has not been working correctly for many many years (and it was not broken in a subtly way as far as I understand, but rather quite obvious). Thus I do not understand why this belongs to stable release. It is not a [recent] regression, nor secutiry bug, nor even enabling of new hardware, that is why I myself did not mark it as stable.
I still maintain that we pick up for stable too many patches for no clear benefit. This is similar to the patch for Atmel controllers that was picked to stable and I asked why, as it is not clear how many users might be affected (or if the problem the patch was solving was purely theoretical, or only affecting hardware that is not in circulation yet).
Quoted 34 lines
Debian will carry stable patches without explicit action on behalf of the maintainer. Unstable patches are a little harder to get accepted.
Quoted 30 lines · Dmitry Torokhov
Quoted 9 lines · Sasha Levin
From: Michael Schmitz <[email protected]>
[ Upstream commit
52d2c7bf7c90217fbe875d2d76f310979c48eb83]The CapsLock key on Atari keyboards is not a toggle, it does send the normal make and break scancodes.
Drop the CapsLock toggle handling code, which did cause the CapsLock key to merely act as a Shift key.
This has been broken for 10+ years. Does it really make sense to promote it to stable?
Quoted 17 lines
Tested-by Michael Schmitz <[email protected]>Signed-off-by Michael Schmitz <[email protected]>Signed-off-by Andreas Schwab <[email protected]>Signed-off-by Dmitry Torokhov <[email protected]>Signed-off-by Sasha Levin <[email protected]>--- drivers/input/keyboard/atakbd.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)drivers/input/keyboard/atakbd.c +0
-0@@ -189,14 +189,8 @@ static void atakbd_interrupt(unsigned char scancode, char down) scancode = atakbd_keycode[scancode];
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/input/keyboard/atakbd.c +0
-5-if (scancode ==KEY_CAPSLOCK ) {/* CapsLock is a toggle switch key on Amiga */ -input_report_key (atakbd_dev, scancode,1 ); -input_report_key (atakbd_dev, scancode,0 ); -input_sync (atakbd_dev); - }else {Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/input/keyboard/atakbd.c static void atakbd_interrupt(unsigned char scancode, char down) · line 196
Quoted 8 lines
drivers/input/keyboard/atakbd.c +2
-3-input_report_key (atakbd_dev, scancode, down); -input_sync (atakbd_dev); - } +input_report_key (atakbd_dev, scancode, down); +input_sync (atakbd_dev); }else /* scancodes >= 0xf3 are mouse data, most likely */ printk (KERN_INFO "atakbd: unhandled scancode %x\n" , scancode);
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Hi Geert,
Quoted 11 lines · Geert Uytterhoeven
Hi Michael,
Quoted 6 lines · Michael Schmitz
I agree the bug is neither subtle nor recent, not security relevant and will affect only a handful of users at best.
If you're worried about weakening the rules around stable releases, by all means go ahead and veto the inclusion of these patches in the next stable release.
I believe the distro the issue was reported against (Debian ports) will not get the fix until the issue is fixed in the upstream stable release?
That's correct. but there are ways around that. I could petition Ben to add it as a separate patch for now.
Boils down to the same question really - what's the impact of this bug? Will it prevent installation from distribution media f.e.?
Given that it's taken ten years for someone to even notice one aspect (missing key mappings), and then I just happened to see the discussion on the ML (which wasn't even about mapping so much as about keys stuck in autorepeat, which I haven't been able to reproduce), I think it's clear that this has minimal impact. m68k users tend to be reasonably computer savvy, and can install a custom built kernel until Debian catches up with current kernel versions.
Having reread Documentation/process/stable-kernel-rules.rst, it's hard for me to argue these patches belong in stable.
Cheers,
Michael
Quoted 74 lines
Quoted 57 lines · Dmitry Torokhov
Quoted 55 lines · Michael Schmitz
Quoted 6 lines
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
I have exactly the same concerns about the keymap patch. This all has not been working correctly for many many years (and it was not broken in a subtly way as far as I understand, but rather quite obvious). Thus I do not understand why this belongs to stable release. It is not a [recent] regression, nor secutiry bug, nor even enabling of new hardware, that is why I myself did not mark it as stable.
I still maintain that we pick up for stable too many patches for no clear benefit. This is similar to the patch for Atmel controllers that was picked to stable and I asked why, as it is not clear how many users might be affected (or if the problem the patch was solving was purely theoretical, or only affecting hardware that is not in circulation yet).
Quoted 34 lines
Debian will carry stable patches without explicit action on behalf of the maintainer. Unstable patches are a little harder to get accepted.
Quoted 30 lines · Dmitry Torokhov
Quoted 9 lines · Sasha Levin
From: Michael Schmitz <[email protected]>
[ Upstream commit
52d2c7bf7c90217fbe875d2d76f310979c48eb83]The CapsLock key on Atari keyboards is not a toggle, it does send the normal make and break scancodes.
Drop the CapsLock toggle handling code, which did cause the CapsLock key to merely act as a Shift key.
This has been broken for 10+ years. Does it really make sense to promote it to stable?
Quoted 17 lines
Tested-by Michael Schmitz <[email protected]>Signed-off-by Michael Schmitz <[email protected]>Signed-off-by Andreas Schwab <[email protected]>Signed-off-by Dmitry Torokhov <[email protected]>Signed-off-by Sasha Levin <[email protected]>--- drivers/input/keyboard/atakbd.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)drivers/input/keyboard/atakbd.c +0
-0@@ -189,14 +189,8 @@ static void atakbd_interrupt(unsigned char scancode, char down) scancode = atakbd_keycode[scancode];Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/input/keyboard/atakbd.c +0
-5-if (scancode ==KEY_CAPSLOCK ) {/* CapsLock is a toggle switch key on Amiga */ -input_report_key (atakbd_dev, scancode,1 ); -input_report_key (atakbd_dev, scancode,0 ); -input_sync (atakbd_dev); - }else {Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/input/keyboard/atakbd.c static void atakbd_interrupt(unsigned char scancode, char down) · line 196
Quoted 8 lines
drivers/input/keyboard/atakbd.c +2
-3-input_report_key (atakbd_dev, scancode, down); -input_sync (atakbd_dev); - } +input_report_key (atakbd_dev, scancode, down); +input_sync (atakbd_dev); }else /* scancodes >= 0xf3 are mouse data, most likely */ printk (KERN_INFO "atakbd: unhandled scancode %x\n" , scancode);Gr{oetje,eeting}s,
Geert
Sasha Levin
maintainerRe: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
In reply to Dmitry TorokhovQuoted 20 lines · Dmitry Torokhov
Hi Michael,
Quoted 9 lines · Michael Schmitz
Dmitry,
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
I have exactly the same concerns about the keymap patch. This all has not been working correctly for many many years (and it was not broken in a subtly way as far as I understand, but rather quite obvious). Thus I do not understand why this belongs to stable release. It is not a [recent] regression, nor secutiry bug, nor even enabling of new hardware, that is why I myself did not mark it as stable.
Quoted 6 lines
I still maintain that we pick up for stable too many patches for no clear benefit. This is similar to the patch for Atmel controllers that was picked to stable and I asked why, as it is not clear how many users might be affected (or if the problem the patch was solving was purely theoretical, or only affecting hardware that is not in circulation yet).
If you belive that a certain piece of code has no actual users, why do you keep it in the upstream kernel to begin with?
I don't think it makes sense to keep something upstream because it might have users, but not backport fixes because there might not have any users.
You haven't seen evidence of anyone using/caring about it for a few years? Great! Remove the code and if someone complains we can always revert. This is how all those orphaned archs got removed a few releases back. I'll even submit the patch if you'd like.
It doesn't make sense to have "second class citizens" like how you suggested.
--
Thanks,
Sasha
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 28 lines · Sasha Levin
Quoted 20 lines · Dmitry Torokhov
Hi Michael,
Quoted 9 lines · Michael Schmitz
Dmitry,
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
I have exactly the same concerns about the keymap patch. This all has not been working correctly for many many years (and it was not broken in a subtly way as far as I understand, but rather quite obvious). Thus I do not understand why this belongs to stable release. It is not a [recent] regression, nor secutiry bug, nor even enabling of new hardware, that is why I myself did not mark it as stable.
Quoted 6 lines
I still maintain that we pick up for stable too many patches for no clear benefit. This is similar to the patch for Atmel controllers that was picked to stable and I asked why, as it is not clear how many users might be affected (or if the problem the patch was solving was purely theoretical, or only affecting hardware that is not in circulation yet).
Quoted 2 lines
If you belive that a certain piece of code has no actual users, why do you keep it in the upstream kernel to begin with?
Because obviously there are users. Maybe 5 of them, maybe 10. They do not follow upstream very closely (otherwise these fixes would have appeared in mainline long time ago) but they still exist. And it is not hard to keep the driver in if there are not many changes.
Quoted 5 lines
I don't think it makes sense to keep something upstream because it might have users, but not backport fixes because there might not have any users.
Quoted 7 lines
You haven't seen evidence of anyone using/caring about it for a few years? Great! Remove the code and if someone complains we can always revert. This is how all those orphaned archs got removed a few releases back. I'll even submit the patch if you'd like.
It doesn't make sense to have "second class citizens" like how you suggested.
Sure does. Look, one of roles of a mantainer is basically a release manager. We need to decide what goes into new release, what goes into maintenance release, what stays until later milestone. Each release has different criteria. With maintenance release you want to minimize the disruption while alleviating most recent and critical/high visibility issues, IOW you want to get the most bang for your buck. And these particular fixes do not give you any bang, none at all.
Same goes for patches that deal with error handling in probe() functions that your AUTOSEL scripts like to pick. Yes, they are fixing bugs. But show me actually users affected by them? You encounter these issues with probe when you do initial device bringup, but once device is stabilized probes are expected to succeed. There won't be duplicate sysfs attributes, memory will be allocated, and so on. Fixes to remove() might be worthwhile if it is a hot-pluggable bus, but otherwise - no. Yes, the box may OOPS if someone manually unbind device through sysfs, but the solution is no to patch stable kernels, but simply tell user "dont to that [yet]".
When selecting a patch for stable ask yourself: "if I do not pick this for stable will a distribution be willing to patch this into their kernel on their own"? If the answer is "no" it should be pretty strong indicator whether a patch belongs to stable or not.
Thanks.
Signature · 1 line
Dmitry
Sasha Levin
maintainerRe: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 29 lines · Dmitry Torokhov
Quoted 28 lines · Sasha Levin
Quoted 26 lines · Dmitry Torokhov
Hi Michael,
Quoted 9 lines · Michael Schmitz
Dmitry,
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
I have exactly the same concerns about the keymap patch. This all has not been working correctly for many many years (and it was not broken in a subtly way as far as I understand, but rather quite obvious). Thus I do not understand why this belongs to stable release. It is not a [recent] regression, nor secutiry bug, nor even enabling of new hardware, that is why I myself did not mark it as stable.
I still maintain that we pick up for stable too many patches for no clear benefit. This is similar to the patch for Atmel controllers that was picked to stable and I asked why, as it is not clear how many users might be affected (or if the problem the patch was solving was purely theoretical, or only affecting hardware that is not in circulation yet).
Quoted 7 lines
Quoted 2 lines
If you belive that a certain piece of code has no actual users, why do you keep it in the upstream kernel to begin with?
Because obviously there are users. Maybe 5 of them, maybe 10. They do not follow upstream very closely (otherwise these fixes would have appeared in mainline long time ago) but they still exist. And it is not hard to keep the driver in if there are not many changes.
It, similarily, not hard to backport these changes to stable tree especially if they're in a self-contained driver such as this one.
Quoted 32 lines
Quoted 5 lines
I don't think it makes sense to keep something upstream because it might have users, but not backport fixes because there might not have any users.
Quoted 7 lines
You haven't seen evidence of anyone using/caring about it for a few years? Great! Remove the code and if someone complains we can always revert. This is how all those orphaned archs got removed a few releases back. I'll even submit the patch if you'd like.
It doesn't make sense to have "second class citizens" like how you suggested.
Sure does. Look, one of roles of a mantainer is basically a release manager. We need to decide what goes into new release, what goes into maintenance release, what stays until later milestone. Each release has different criteria. With maintenance release you want to minimize the disruption while alleviating most recent and critical/high visibility issues, IOW you want to get the most bang for your buck. And these particular fixes do not give you any bang, none at all.
Same goes for patches that deal with error handling in
probe()functions that your AUTOSEL scripts like to pick. Yes, they are fixing bugs. But show me actually users affected by them? You encounter these issues with probe when you do initial device bringup, but once device is stabilized probes are expected to succeed. There won't be duplicate sysfs attributes, memory will be allocated, and so on. Fixes toremove()might be worthwhile if it is a hot-pluggable bus, but otherwise - no. Yes, the box may OOPS if someone manually unbind device through sysfs, but the solution is no to patch stable kernels, but simply tell user "dont to that [yet]".
Quoted 4 lines
When selecting a patch for stable ask yourself: "if I do not pick this for stable will a distribution be willing to patch this into their kernel on their own"? If the answer is "no" it should be pretty strong indicator whether a patch belongs to stable or not.
There is a big difference between distros and the stable trees. Distros know who their users/customers are, OTOH we have no clue who the users of stable trees are.
I think Greg's last estimate was that about 1/3 of the kernels in the wild are custom based on a kernel.org stable kernel, which means that we have no visibility as to what they do with the kernel. If you don't know who your users are, how can you prioritize some subsystems over others?
I don't think we can do any of that because we don't know who uses the kernel and what bugs they hit, or don't hit. This is one of the reasons we ask everyone to pick everything, if they don't use whatever code we changed they won't be affected at all.
As a trivial example: we run a few kernels with custom configs in Microsoft. Can you tell which drivers we use and how many machines use them? Us not showing up in git log doesn't mean we don't use something, it just means that the stable process works for us.
--
Thanks,
Sasha
Greg KH
maintainerRe: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 4 lines · Sasha Levin
I think Greg's last estimate was that about 1/3 of the kernels in the wild are custom based on a kernel.org stable kernel, which means that we have no visibility as to what they do with the kernel. If you don't know who your users are, how can you prioritize some subsystems over others?
The numbers I had was 75% of the images a major cloud provider was using was either a kernel.org stable kernel release, or Debian. The remaining 25% was an "enterprise" kernel including CentOS.
Also note that all Android devices are now required to follow the stable kernel releases as well, so add a few more million to that number :)
That being said, for a well-maintained subsystem like Input, whose maintainer almost always marks patches for stable releases, having them picked up by the autobot is unusual. Dmitry, if you want your subsytem to be excluded, just let Sasha know, other subsystems have been excluded at the maintainer's request, and that's fine.
There are many other subsystems whose maintainers never mark stuff for stable, and that's a much bigger issue that the autobot is working to help solve. I doubt we need to worrya bout Atari keyboards :)
thanks,
greg k-h
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 10 lines · Greg KH
Quoted 4 lines · Sasha Levin
I think Greg's last estimate was that about 1/3 of the kernels in the wild are custom based on a kernel.org stable kernel, which means that we have no visibility as to what they do with the kernel. If you don't know who your users are, how can you prioritize some subsystems over others?
The numbers I had was 75% of the images a major cloud provider was using was either a kernel.org stable kernel release, or Debian. The remaining 25% was an "enterprise" kernel including CentOS.
Quoted 8 lines
Also note that all Android devices are now required to follow the stable kernel releases as well, so add a few more million to that number :)
That being said, for a well-maintained subsystem like Input, whose maintainer almost always marks patches for stable releases, having them picked up by the autobot is unusual. Dmitry, if you want your subsytem to be excluded, just let Sasha know, other subsystems have been excluded at the maintainer's request, and that's fine.
I am hesitant to request to exclude input from autosel just yet, on an account that I might miss something. But I would like autosel to acquire more smarts and be less trigger happy. I think it would be for the best for everyone.
Thanks.
Signature · 1 line
Dmitry
Sasha Levin
maintainerRe: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 20 lines · Dmitry Torokhov
Quoted 10 lines · Greg KH
Quoted 4 lines · Sasha Levin
I think Greg's last estimate was that about 1/3 of the kernels in the wild are custom based on a kernel.org stable kernel, which means that we have no visibility as to what they do with the kernel. If you don't know who your users are, how can you prioritize some subsystems over others?
The numbers I had was 75% of the images a major cloud provider was using was either a kernel.org stable kernel release, or Debian. The remaining 25% was an "enterprise" kernel including CentOS.
Quoted 8 lines
Also note that all Android devices are now required to follow the stable kernel releases as well, so add a few more million to that number :)
That being said, for a well-maintained subsystem like Input, whose maintainer almost always marks patches for stable releases, having them picked up by the autobot is unusual. Dmitry, if you want your subsytem to be excluded, just let Sasha know, other subsystems have been excluded at the maintainer's request, and that's fine.
Quoted 4 lines
I am hesitant to request to exclude input from autosel just yet, on an account that I might miss something. But I would like autosel to acquire more smarts and be less trigger happy. I think it would be for the best for everyone.
Keep in mind that it learns based on your choices, so maybe in a while it will be a perfect fit :)
Maybe to make it easier, just reply with NAK on the patch(es) you don't want in the tree and I'll drop them.
I was discussing more about the general case and stable tree policy rather than drivers/input and AUTOSEL, and didn't mean to try and dictate what we'll take or not. Sorry!
--
Thanks,
Sasha
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 26 lines · Sasha Levin
Quoted 20 lines · Dmitry Torokhov
Quoted 10 lines · Greg KH
Quoted 4 lines · Sasha Levin
I think Greg's last estimate was that about 1/3 of the kernels in the wild are custom based on a kernel.org stable kernel, which means that we have no visibility as to what they do with the kernel. If you don't know who your users are, how can you prioritize some subsystems over others?
The numbers I had was 75% of the images a major cloud provider was using was either a kernel.org stable kernel release, or Debian. The remaining 25% was an "enterprise" kernel including CentOS.
Quoted 8 lines
Also note that all Android devices are now required to follow the stable kernel releases as well, so add a few more million to that number :)
That being said, for a well-maintained subsystem like Input, whose maintainer almost always marks patches for stable releases, having them picked up by the autobot is unusual. Dmitry, if you want your subsytem to be excluded, just let Sasha know, other subsystems have been excluded at the maintainer's request, and that's fine.
Quoted 4 lines
I am hesitant to request to exclude input from autosel just yet, on an account that I might miss something. But I would like autosel to acquire more smarts and be less trigger happy. I think it would be for the best for everyone.
Quoted 5 lines
Keep in mind that it learns based on your choices, so maybe in a while it will be a perfect fit :)
Maybe to make it easier, just reply with NAK on the patch(es) you don't want in the tree and I'll drop them.
OK, I will be doing it then. Should it be a certain string so you can script or a free form?
Quoted 4 lines
I was discussing more about the general case and stable tree policy rather than
drivers/inputand AUTOSEL, and didn't mean to try and dictate what we'll take or not. Sorry!
Me too and I am not trying to be dismissive of your effort, I am just trying to have discussion on the criteria that we should use for stable candidates.
Thanks.
Signature · 1 line
Dmitry
Sasha Levin
maintainerRe: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 27 lines · Dmitry Torokhov
Quoted 26 lines · Sasha Levin
Quoted 24 lines · Dmitry Torokhov
Quoted 10 lines · Greg KH
Quoted 4 lines · Sasha Levin
I think Greg's last estimate was that about 1/3 of the kernels in the wild are custom based on a kernel.org stable kernel, which means that we have no visibility as to what they do with the kernel. If you don't know who your users are, how can you prioritize some subsystems over others?
The numbers I had was 75% of the images a major cloud provider was using was either a kernel.org stable kernel release, or Debian. The remaining 25% was an "enterprise" kernel including CentOS.
Quoted 8 lines
Also note that all Android devices are now required to follow the stable kernel releases as well, so add a few more million to that number :)
That being said, for a well-maintained subsystem like Input, whose maintainer almost always marks patches for stable releases, having them picked up by the autobot is unusual. Dmitry, if you want your subsytem to be excluded, just let Sasha know, other subsystems have been excluded at the maintainer's request, and that's fine.
I am hesitant to request to exclude input from autosel just yet, on an account that I might miss something. But I would like autosel to acquire more smarts and be less trigger happy. I think it would be for the best for everyone.
Quoted 8 lines
Quoted 5 lines
Keep in mind that it learns based on your choices, so maybe in a while it will be a perfect fit :)
Maybe to make it easier, just reply with NAK on the patch(es) you don't want in the tree and I'll drop them.
OK, I will be doing it then. Should it be a certain string so you can script or a free form?
Nah, I go through any comments I receive and am not planning on changing that.
Quoted 8 lines
Quoted 4 lines
I was discussing more about the general case and stable tree policy rather than
drivers/inputand AUTOSEL, and didn't mean to try and dictate what we'll take or not. Sorry!Me too and I am not trying to be dismissive of your effort, I am just trying to have discussion on the criteria that we should use for stable candidates.
This is great input for me as well. Thank you!
--
Thanks,
Sasha
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
In reply to Sasha LevinQuoted 33 lines · Sasha Levin
Quoted 32 lines · Dmitry Torokhov
Quoted 30 lines · Sasha Levin
Quoted 20 lines · Dmitry Torokhov
Hi Michael,
Quoted 9 lines · Michael Schmitz
Dmitry,
someone on debian-68k reported the bug, which (to me) indicates that the code is not just used by me.
Whether or not a functioning Capslock is essential to have? You be the judge of that. If you are OK with applying the keymap patch, why not this one?
I have exactly the same concerns about the keymap patch. This all has not been working correctly for many many years (and it was not broken in a subtly way as far as I understand, but rather quite obvious). Thus I do not understand why this belongs to stable release. It is not a [recent] regression, nor secutiry bug, nor even enabling of new hardware, that is why I myself did not mark it as stable.
Quoted 6 lines
I still maintain that we pick up for stable too many patches for no clear benefit. This is similar to the patch for Atmel controllers that was picked to stable and I asked why, as it is not clear how many users might be affected (or if the problem the patch was solving was purely theoretical, or only affecting hardware that is not in circulation yet).
If you belive that a certain piece of code has no actual users, why do you keep it in the upstream kernel to begin with?
Quoted 7 lines
Quoted 4 lines
Because obviously there are users. Maybe 5 of them, maybe 10. They do not follow upstream very closely (otherwise these fixes would have appeared in mainline long time ago) but they still exist. And it is not hard to keep the driver in if there are not many changes.
It, similarily, not hard to backport these changes to stable tree especially if they're in a self-contained driver such as this one.
It is easy to backport, but that should not be a criteria. If we backport each end every one patch landing into mainline into stable then "backporting" is trivial. It does mean that it should be done.
Quoted 38 lines
Quoted 33 lines
Quoted 6 lines
I don't think it makes sense to keep something upstream because it might have users, but not backport fixes because there might not have any users.
Quoted 7 lines
You haven't seen evidence of anyone using/caring about it for a few years? Great! Remove the code and if someone complains we can always revert. This is how all those orphaned archs got removed a few releases back. I'll even submit the patch if you'd like.
It doesn't make sense to have "second class citizens" like how you suggested.
Sure does. Look, one of roles of a mantainer is basically a release manager. We need to decide what goes into new release, what goes into maintenance release, what stays until later milestone. Each release has different criteria. With maintenance release you want to minimize the disruption while alleviating most recent and critical/high visibility issues, IOW you want to get the most bang for your buck. And these particular fixes do not give you any bang, none at all.
Same goes for patches that deal with error handling in
probe()functions that your AUTOSEL scripts like to pick. Yes, they are fixing bugs. But show me actually users affected by them? You encounter these issues with probe when you do initial device bringup, but once device is stabilized probes are expected to succeed. There won't be duplicate sysfs attributes, memory will be allocated, and so on. Fixes toremove()might be worthwhile if it is a hot-pluggable bus, but otherwise - no. Yes, the box may OOPS if someone manually unbind device through sysfs, but the solution is no to patch stable kernels, but simply tell user "dont to that [yet]".Quoted 4 lines
When selecting a patch for stable ask yourself: "if I do not pick this for stable will a distribution be willing to patch this into their kernel on their own"? If the answer is "no" it should be pretty strong indicator whether a patch belongs to stable or not.
Quoted 8 lines
There is a big difference between distros and the stable trees. Distros know who their users/customers are, OTOH we have no clue who the users of stable trees are.
I think Greg's last estimate was that about 1/3 of the kernels in the wild are custom based on a kernel.org stable kernel, which means that we have no visibility as to what they do with the kernel. If you don't know who your users are, how can you prioritize some subsystems over others?
You make a judgement call, based on the data you have. And I think if you want override maintainer decision you need a decent justification, better than "it is easy to backport" or "we do not know if someone might use it".
Quoted 3 lines
I don't think we can do any of that because we don't know who uses the kernel and what bugs they hit, or don't hit.
They should file bugs, report on mailing lists, and so on. If they hit bugs and do not bother to report them anywhere, it is not our problem. They need to be part of community.
Quoted 3 lines
This is one of the reasons
we ask everyone to pick everything, if they don't use whatever code we changed they won't be affected at all.
Or there is unexpected behavior change and they are affected. What do you do if users of atakbd adjusted their userspace for the broken kernel map? From my POV it is OK-ish to change in new kernel release, but not in a middle of "stable" series.
Quoted 5 lines
As a trivial example: we run a few kernels with custom configs in Microsoft. Can you tell which drivers we use and how many machines use them? Us not showing up in git log doesn't mean we don't use something, it just means that the stable process works for us.
I can tell you that you are not using atakbd ;)
Look, if we are talking about Microsoft, what are criteria for changes that are going into "patch tuesday" releases? Is it any random junk that might land on top of the development tree? Or is it just a bit more disciplined? Like Release managers looking at the incoming problem report rates and make a judgement call as to whether pull the fix in or wait for bigger maintenance release? In ChromeOS land it is the latter.
Thanks.
Signature · 1 line
Dmitry
Sasha Levin
maintainerRe: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Quoted 10 lines · Dmitry Torokhov
Quoted 8 lines · Sasha Levin
There is a big difference between distros and the stable trees. Distros know who their users/customers are, OTOH we have no clue who the users of stable trees are.
I think Greg's last estimate was that about 1/3 of the kernels in the wild are custom based on a kernel.org stable kernel, which means that we have no visibility as to what they do with the kernel. If you don't know who your users are, how can you prioritize some subsystems over others?
Quoted 4 lines
You make a judgement call, based on the data you have. And I think if you want override maintainer decision you need a decent justification, better than "it is easy to backport" or "we do not know if someone might use it".
I'll never override a maintainer with regards to whether a patch will go in stable or not. We're having this discussion only because you posed a question rather than NAKed it. If you want me to drop this or any other patch I'll happily do that.
Quoted 7 lines
Quoted 3 lines
I don't think we can do any of that because we don't know who uses the kernel and what bugs they hit, or don't hit.
They should file bugs, report on mailing lists, and so on. If they hit bugs and do not bother to report them anywhere, it is not our problem. They need to be part of community.
The tricky part I see here is being able to say "this is a kernel bug". Given an average user who's capslock key stops working, do you really expect them to diagnose this as a kernel issue? Most users don't even know what the kernel is.
Quoted 8 lines
Quoted 3 lines
This is one of the reasons
we ask everyone to pick everything, if they don't use whatever code we changed they won't be affected at all.Or there is unexpected behavior change and they are affected. What do you do if users of atakbd adjusted their userspace for the broken kernel map? From my POV it is OK-ish to change in new kernel release, but not in a middle of "stable" series.
Fair enough, we usually do that trade-off based on how complex a patch is rather than how many users a certain driver has.
Quoted 6 lines
Quoted 5 lines
As a trivial example: we run a few kernels with custom configs in Microsoft. Can you tell which drivers we use and how many machines use them? Us not showing up in git log doesn't mean we don't use something, it just means that the stable process works for us.
Quoted 8 lines
I can tell you that you are not using atakbd ;)
Look, if we are talking about Microsoft, what are criteria for changes that are going into "patch tuesday" releases? Is it any random junk that might land on top of the development tree? Or is it just a bit more disciplined? Like Release managers looking at the incoming problem report rates and make a judgement call as to whether pull the fix in or wait for bigger maintenance release? In ChromeOS land it is the latter.
Exactly! If we'd have a system to report bugs and issues where these are automatically sent back to the mothership like on Windows I'd agree with you. But for now we have shit for bugtracker and noisy LKML that no one reads. And this assumes that a user knows what this mystical "kernel" thing is.
As you might know, Microsoft is rather new in the Linux field, and we sometimes have a "what is the linux kernel?" discussions. This is inside a TECH company. I can't iamgine a lay person knows what a kernel is.
It's hard to expect those users to do that much footwork to report bugs.
--
Thanks,
Sasha
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
In reply to Dmitry TorokhovHi Dmitry,
Quoted 10 lines · Dmitry Torokhov
Same goes for patches that deal with error handling in
probe()functions that your AUTOSEL scripts like to pick. Yes, they are fixing bugs. But show me actually users affected by them? You encounter these issues with probe when you do initial device bringup, but once device is stabilized probes are expected to succeed. There won't be duplicate sysfs attributes, memory will be allocated, and so on. Fixes toremove()might be worthwhile if it is a hot-pluggable bus, but otherwise - no. Yes, the box may OOPS if someone manually unbind device through sysfs, but the solution is no to patch stable kernels, but simply tell user "dont to that [yet]".
In modern days with -EPROBE_DEFER, bugs in probe() are much more likely to cause damage than before.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH AUTOSEL 4.18 24/58] Input: atakbd - fix Atari CapsLock behaviour
Hi Geert,
Quoted 15 lines · Geert Uytterhoeven
Hi Dmitry,
Quoted 10 lines · Dmitry Torokhov
Same goes for patches that deal with error handling in
probe()functions that your AUTOSEL scripts like to pick. Yes, they are fixing bugs. But show me actually users affected by them? You encounter these issues with probe when you do initial device bringup, but once device is stabilized probes are expected to succeed. There won't be duplicate sysfs attributes, memory will be allocated, and so on. Fixes toremove()might be worthwhile if it is a hot-pluggable bus, but otherwise - no. Yes, the box may OOPS if someone manually unbind device through sysfs, but the solution is no to patch stable kernels, but simply tell user "dont to that [yet]".
Quoted 2 lines
In modern days with -EPROBE_DEFER, bugs in
probe()are much more likely to cause damage than before.
Yes, as usual, the real life is not quite as black-and-white as I painted it to be ;)
If the patch handles failures that could be result of signalling deferral I would consider it for stable. OTOH it is not very likely that older, previously working device, will start signalling deferral where it did not before, so if we see deferral it is likely we are dealing with newer platform, and it is unclear how far stable backport should go in this case. So again, choices, choices ;)
Thanks.
Signature · 1 line
Dmitry
Sasha Levin
maintainerPATCH 25/58 net: stmmac: Rework coalesce timer and fix multi-queue races
In reply to Sasha LevinFrom: Jose Abreu <[email protected]>
[ Upstream commit 8fce3331702316d4bcfeb0771c09ac75d2192bbc ]
This follows David Miller advice and tries to fix coalesce timer in multi-queue scenarios.
We are now using per-queue coalesce values and per-queue TX timer.
Coalesce timer default values was changed to 1ms and the coalesce frames to 25.
Tested in B2B setup between XGMAC2 and GMAC5.
--- drivers/net/ethernet/stmicro/stmmac/common.h | 4 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 14 +- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 233 ++++++++++-------- include/linux/stmmac.h | 1 + 4 files changed, 146 insertions(+), 106 deletions(-)
drivers/net/ethernet/stmicro/stmmac/common.h +2 -2
@@ -256,10 +256,10 @@ struct stmmac_safety_stats {#define MAX_DMA_RIWT 0xff#define MIN_DMA_RIWT 0x20/* Tx coalesce parameters */ -#define STMMAC_COAL_TX_TIMER 40000 +#define STMMAC_COAL_TX_TIMER 1000#define STMMAC_MAX_COAL_TX_TICK 100000#define STMMAC_TX_MAX_FRAMES 256 -#define STMMAC_TX_FRAMES 64 +#define STMMAC_TX_FRAMES 25/* Packets types */ enum packets_types {
drivers/net/ethernet/stmicro/stmmac/stmmac.h +12 -2
@@ -48,6 +48,8 @@ struct stmmac_tx_info {/* Frequently used values are kept adjacent for cache effect */ struct stmmac_tx_queue { +u32 tx_count_frames; +struct timer_list txtimer;u32 queue_index;struct stmmac_priv *priv_data;struct dma_extended_desc *dma_etx____cacheline_aligned_in_smp ;
@@ -73,7 +75,14 @@ struct stmmac_rx_queue {u32 rx_zeroc_thresh;dma_addr_t dma_rx_phy;u32 rx_tail_addr; +}; + +struct stmmac_channel {struct napi_struct napi____cacheline_aligned_in_smp ; +struct stmmac_priv *priv_data; +u32 index; +int has_rx; +int has_tx; };struct stmmac_tc_entry {
@@ -109,14 +118,12 @@ struct stmmac_pps_cfg {struct stmmac_priv {/* Frequently used values are kept adjacent for cache effect */ -u32 tx_count_frames;u32 tx_coal_frames;u32 tx_coal_timer;int tx_coalesce;int hwts_tx_en;bool tx_path_in_lpi_mode; -struct timer_list txtimer;bool tso;unsigned int dma_buf_sz;
@@ -137,6 +144,9 @@ struct stmmac_priv {/* TX Queue */ struct stmmac_tx_queue tx_queue[MTL_MAX_TX_QUEUES ]; +/* Generic channel for NAPI */ +struct stmmac_channel channel[STMMAC_CH_MAX ]; +bool oldlink;int speed;int oldduplex;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +131 -102
408 lines not on this page · raw message
include/linux/stmmac.h +1 -0
@@ -30,6 +30,7 @@#define MTL_MAX_RX_QUEUES 8#define MTL_MAX_TX_QUEUES 8 +#define STMMAC_CH_MAX 8#define STMMAC_RX_COE_NONE 0#define STMMAC_RX_COE_TYPE1 1
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 26/58 net: stmmac: Fixup the tail addr setting in xmit path
In reply to Sasha LevinFrom: Jose Abreu <[email protected]>
[ Upstream commit 0431100b3d82c509729ece1ab22ada2484e209c1 ]
Currently we are always setting the tail address of descriptor list to the end of the pre-allocated list.
According to databook this is not correct. Tail address should point to the last available descriptor + 1, which means we have to update the tail address everytime we call the xmit function.
This should make no impact in older versions of MAC but in newer versions there are some DMA features which allows the IP to fetch descriptors in advance and in a non sequential order so its critical that we set the tail address correctly.
--- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3 -2
@@ -2186,8 +2186,7 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)stmmac_init_tx_chan (priv, priv->ioaddr, priv->plat->dma_cfg, tx_q->dma_tx_phy, chan); - tx_q->tx_tail_addr = tx_q->dma_tx_phy + - (DMA_TX_SIZE *sizeof (struct dma_desc )); + tx_q->tx_tail_addr = tx_q->dma_tx_phy;stmmac_set_tx_tail_ptr (priv, priv->ioaddr, tx_q->tx_tail_addr, chan); }
@@ -2982,6 +2981,7 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)netdev_tx_sent_queue (netdev_get_tx_queue (dev, queue), skb->len); + tx_q->tx_tail_addr = tx_q->dma_tx_phy + (tx_q->cur_tx *sizeof (*desc));stmmac_set_tx_tail_ptr (priv, priv->ioaddr, tx_q->tx_tail_addr, queue);return NETDEV_TX_OK ;
@@ -3189,6 +3189,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)stmmac_enable_dma_transmission (priv, priv->ioaddr); + tx_q->tx_tail_addr = tx_q->dma_tx_phy + (tx_q->cur_tx *sizeof (*desc));stmmac_set_tx_tail_ptr (priv, priv->ioaddr, tx_q->tx_tail_addr, queue);return NETDEV_TX_OK ;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 27/58 selftests: pmtu: properly redirect stderr to /dev/null
In reply to Sasha LevinFrom: Sabrina Dubroca <[email protected]>
[ Upstream commit 0a286afee5a1e8dca86d824209dbd3200294f86f ]
The cleanup function uses "$CMD 2 > /dev/null", which doesn't actually send stderr to /dev/null, so when the netns doesn't exist, the error message is shown. Use "2> /dev/null" instead, so that those messages disappear, as was intended.
--- tools/testing/selftests/net/pmtu.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/testing/selftests/net/pmtu.sh +2 -2
@@ -175,8 +175,8 @@ setup() { cleanup() { [ ${cleanup_done} -eq 1 ] && return - ip netns del ${NS_A} 2 > /dev/null - ip netns del ${NS_B} 2 > /dev/null + ip netns del ${NS_A} 2> /dev/null + ip netns del ${NS_B} 2> /dev/null cleanup_done=1 }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 28/58 net: emac: fix fixed-link setup for the RTL8363SB switch
In reply to Sasha LevinFrom: Christian Lamparter <[email protected]>
[ Upstream commit 08e39982ef64f800fd1f9b9b92968d14d5fafa82 ]
On the Netgear WNDAP620, the emac ethernet isn't receiving nor xmitting any frames from/to the RTL8363SB (identifies itself as a RTL8367RB).
This is caused by the emac hardware not knowing the forced link parameters for speed, duplex, pause, etc.
This begs the question, how this was working on the original driver code, when it was necessary to set the phy_address and phy_map to 0xffffffff. But I guess without access to the old PPC405/440/460 hardware, it's not possible to know.
--- drivers/net/ethernet/ibm/emac/core.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/net/ethernet/ibm/emac/core.c +10 -5
@@ -2677,12 +2677,17 @@ static int emac_init_phy(struct emac_instance *dev)if (of_phy_is_fixed_link (np)) {int res =emac_dt_mdio_probe (dev); -if (!res) { - res =of_phy_register_fixed_link (np); -if (res) -mdiobus_unregister (dev->mii_bus); +if (res) +return res; + + res =of_phy_register_fixed_link (np); + dev->phy_dev =of_phy_find_device (np); +if (res || !dev->phy_dev) { +mdiobus_unregister (dev->mii_bus); +return res ? res : -EINVAL ; } -return res; +emac_adjust_link (dev->ndev); +put_device (&dev->phy_dev->mdio.dev); }return 0 ; }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 29/58 ravb: do not write 1 to reserved bits
In reply to Sasha LevinFrom: Kazuya Mizuguchi <[email protected]>
[ Upstream commit 2fe397a3959de8a472f165e6d152f64cb77fa2cc ]
EtherAVB hardware requires 0 to be written to status register bits in order to clear them, however, care must be taken not to:
1. Clear other bits, by writing zero to them
2. Write one to reserved bits
This patch corrects the ravb driver with respect to the second point above. This is done by defining reserved bit masks for the affected registers and, after auditing the code, ensure all sites that may write a one to a reserved bit use are suitably masked.
--- drivers/net/ethernet/renesas/ravb.h | 5 +++++ drivers/net/ethernet/renesas/ravb_main.c | 11 ++++++----- drivers/net/ethernet/renesas/ravb_ptp.c | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-)
drivers/net/ethernet/renesas/ravb.h +5 -0
@@ -431,6 +431,7 @@ enum EIS_BIT {EIS_CULF1 =0x00000080 ,EIS_TFFF =0x00000100 ,EIS_QFS =0x00010000 , + EIS_RESERVED = (GENMASK(31, 17) | GENMASK(15, 11)), };/* RIC0 */
@@ -475,6 +476,7 @@ enum RIS0_BIT {RIS0_FRF15 =0x00008000 ,RIS0_FRF16 =0x00010000 ,RIS0_FRF17 =0x00020000 , + RIS0_RESERVED = GENMASK(31, 18), };/* RIC1 */
@@ -531,6 +533,7 @@ enum RIS2_BIT {RIS2_QFF16 =0x00010000 ,RIS2_QFF17 =0x00020000 ,RIS2_RFFF =0x80000000 , + RIS2_RESERVED = GENMASK(30, 18), };/* TIC */
@@ -547,6 +550,7 @@ enum TIS_BIT {TIS_FTF1 =0x00000002 ,/* Undocumented? */ TIS_TFUF =0x00000100 ,TIS_TFWF =0x00000200 , + TIS_RESERVED = (GENMASK(31, 20) | GENMASK(15, 12) | GENMASK(7, 4)) };/* ISS */
@@ -620,6 +624,7 @@ enum GIC_BIT {enum GIS_BIT {GIS_PTCF =0x00000001 ,/* Undocumented? */ GIS_PTMF =0x00000004 , +GIS_RESERVED =GENMASK (15 ,10 ), };/* GIE (R-Car Gen3 only) */
drivers/net/ethernet/renesas/ravb_main.c +6 -5
@@ -742,10 +742,11 @@ static void ravb_error_interrupt(struct net_device *ndev)u32 eis, ris2; eis =ravb_read (ndev,EIS ); -ravb_write (ndev, ~EIS_QFS ,EIS ); +ravb_write (ndev, ~(EIS_QFS |EIS_RESERVED ),EIS );if (eis &EIS_QFS ) { ris2 =ravb_read (ndev,RIS2 ); -ravb_write (ndev, ~(RIS2_QFF0 |RIS2_RFFF ),RIS2 ); +ravb_write (ndev, ~(RIS2_QFF0 |RIS2_RFFF |RIS2_RESERVED ), +RIS2 );/* Receive Descriptor Empty int */ if (ris2 &RIS2_QFF0 )
@@ -798,7 +799,7 @@ static bool ravb_timestamp_interrupt(struct net_device *ndev)u32 tis =ravb_read (ndev,TIS );if (tis &TIS_TFUF ) { -ravb_write (ndev, ~TIS_TFUF ,TIS ); +ravb_write (ndev, ~(TIS_TFUF |TIS_RESERVED ),TIS );ravb_get_tx_tstamp (ndev);return true; }
@@ -933,7 +934,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)/* Processing RX Descriptor Ring */ if (ris0 & mask) {/* Clear RX interrupt */ -ravb_write (ndev, ~mask,RIS0 ); +ravb_write (ndev, ~(mask |RIS0_RESERVED ),RIS0 );if (ravb_rx (ndev, "a, q)) goto out; }
@@ -941,7 +942,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)if (tis & mask) {spin_lock_irqsave (&priv->lock, flags);/* Clear TX interrupt */ -ravb_write (ndev, ~mask,TIS ); +ravb_write (ndev, ~(mask |TIS_RESERVED ),TIS );ravb_tx_free (ndev, q, true);netif_wake_subqueue (ndev, q);mmiowb ();
drivers/net/ethernet/renesas/ravb_ptp.c +1 -1
@@ -319,7 +319,7 @@ void ravb_ptp_interrupt(struct net_device *ndev) } } -ravb_write (ndev, ~gis,GIS ); +ravb_write (ndev, ~(gis |GIS_RESERVED ),GIS ); }void ravb_ptp_init (struct net_device *ndev,struct platform_device *pdev)
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 30/58 net/smc: fix non-blocking connect problem
In reply to Sasha LevinFrom: Ursula Braun <[email protected]>
[ Upstream commit 648a5a7aed346c3b8fe7c32a835edfb0dfbf4451 ]
In state SMC_INIT smc_poll() delegates polling to the internal CLC socket. This means, once the connect worker has finished its kernel_connect() step, the poll wake-up may occur. This is not intended. The wake-up should occur from the wake up call in smc_connect_work() after __smc_connect() has finished. Thus in state SMC_INIT this patch now calls sock_poll_wait() on the main SMC socket.
--- net/smc/af_smc.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
net/smc/af_smc.c +5 -2
@@ -612,7 +612,10 @@ static void smc_connect_work(struct work_struct *work) smc->sk.sk_err = -rc; out: - smc->sk.sk_state_change (&smc->sk); +if (smc->sk.sk_err) + smc->sk.sk_state_change (&smc->sk); +else + smc->sk.sk_write_space (&smc->sk);kfree (smc->connect_info); smc->connect_info =NULL ;release_sock (&smc->sk);
@@ -1345,7 +1348,7 @@ static __poll_t smc_poll(struct file *file, struct socket *sock,return EPOLLNVAL ; smc =smc_sk (sock->sk); -if ((sk->sk_state ==SMC_INIT ) || smc->use_fallback) { +if (smc->use_fallback) {/* delegate to CLC child sock */ mask = smc->clcsock->ops->poll (file, smc->clcsock, wait); sk->sk_err = smc->clcsock->sk->sk_err;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 31/58 net/smc: fix sizeof to int comparison
In reply to Sasha LevinFrom: YueHaibing <[email protected]>
[ Upstream commit 381897798a94065ffcad0772eecdc6b04a7ff23d ]
Comparing an int to a size, which is unsigned, causes the int to become unsigned, giving the wrong result. kernel_sendmsg can return a negative error code.
--- net/smc/smc_clc.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
net/smc/smc_clc.c +6 -8
@@ -405,14 +405,12 @@ int smc_clc_send_proposal(struct smc_sock *smc, vec[i++].iov_len =sizeof (trl);/* due to the few bytes needed for clc-handshake this cannot block */ len =kernel_sendmsg (smc->clcsock, &msg, vec, i, plen); -if (len <sizeof (pclc)) { -if (len >=0 ) { - reason_code = -ENETUNREACH ; - smc->sk.sk_err = -reason_code; - }else { - smc->sk.sk_err = smc->clcsock->sk->sk_err; - reason_code = -smc->sk.sk_err; - } +if (len <0 ) { + smc->sk.sk_err = smc->clcsock->sk->sk_err; + reason_code = -smc->sk.sk_err; + }else if (len < (int )sizeof (pclc)) { + reason_code = -ENETUNREACH ; + smc->sk.sk_err = -reason_code; }return reason_code;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 32/58 net: mvpp2: fix a txq_done race condition
In reply to Sasha LevinFrom: Antoine Tenart <[email protected]>
[ Upstream commit 774268f3e51b53ed432a1ec516574fd5ba469398 ]
When no Tx IRQ is available, the txq_done() routine (called from tx_done()) shouldn't be called from the polling function, as in such case it is already called in the Tx path thanks to an hrtimer. This mostly occurred when using PPv2.1, as the engine then do not have Tx IRQs.
--- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +6 -4
@@ -3055,10 +3055,12 @@ static int mvpp2_poll(struct napi_struct *napi, int budget) cause_rx_tx & ~MVPP2_CAUSE_MISC_SUM_MASK); } - cause_tx = cause_rx_tx &MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_MASK ; -if (cause_tx) { - cause_tx >>=MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_OFFSET ; -mvpp2_tx_done (port, cause_tx, qv->sw_thread_id); +if (port->has_tx_irqs) { + cause_tx = cause_rx_tx &MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_MASK ; +if (cause_tx) { + cause_tx >>=MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_OFFSET ; +mvpp2_tx_done (port, cause_tx, qv->sw_thread_id); + } }/* Process RX packets */
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 33/58 sfp: fix oops with ethtool -m
In reply to Sasha LevinFrom: Russell King <[email protected]>
[ Upstream commit 126d6848ef13958e1cb959e96c21d19bc498ade9 ]
If a network interface is created prior to the SFP socket being available, ethtool can request module information. This unfortunately leads to an oops:
Unable to handle kernel NULL pointer dereference at virtual address 00000008 pgd = (ptrval) [00000008] *pgd=7c400831, *pte=00000000, *ppte=00000000 Internal error: Oops: 17 [#1] SMP ARM Modules linked in: CPU: 0 PID: 1480 Comm: ethtool Not tainted 4.19.0-rc3 #138 Hardware name: Broadcom Northstar Plus SoC PC is at sfp_get_module_info+0x8/0x10 LR is at dev_ethtool+0x218c/0x2afc
Fix this by not filling in the network device's SFP bus pointer until SFP is fully bound, thereby avoiding the core calling into the SFP bus code.
--- drivers/net/phy/sfp-bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/phy/sfp-bus.c +2 -2
@@ -349,6 +349,7 @@ static int sfp_register_bus(struct sfp_bus *bus) }if (bus->started) bus->socket_ops->start (bus->sfp); + bus->netdev->sfp_bus = bus; bus->registered = true;return 0 ; }
@@ -357,6 +358,7 @@ static void sfp_unregister_bus(struct sfp_bus *bus) {const struct sfp_upstream_ops *ops = bus->upstream_ops; + bus->netdev->sfp_bus =NULL ;if (bus->registered) {if (bus->started) bus->socket_ops->stop (bus->sfp);
@@ -438,7 +440,6 @@ static void sfp_upstream_clear(struct sfp_bus *bus) { bus->upstream_ops =NULL ; bus->upstream =NULL ; - bus->netdev->sfp_bus =NULL ; bus->netdev =NULL ; }
@@ -467,7 +468,6 @@ struct sfp_bus *sfp_register_upstream(struct fwnode_handle *fwnode, bus->upstream_ops = ops; bus->upstream = upstream; bus->netdev = ndev; - ndev->sfp_bus = bus;if (bus->sfp) { ret =sfp_register_bus (bus);
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 34/58 qed: Fix populating the invalid stag value in multi function mode.
In reply to Sasha LevinFrom: Sudarsana Reddy Kalluru <[email protected]>
[ Upstream commit 50fdf60181b01b7383b85d4b9acbb842263d96a2 ]
In multi-function mode, driver receives the stag value (outer vlan) for a PF from management FW (MFW). If the stag value is negotiated prior to the driver load, then the stag is not notified to the driver and hence driver will have the invalid stag value.
The fix is to request the MFW for STAG value during the driver load time.
--- drivers/net/ethernet/qlogic/qed/qed_dev.c | 15 ++++++++++++++- drivers/net/ethernet/qlogic/qed/qed_hsi.h | 4 ++++ 2 files changed, 18 insertions(+), 1 deletion(-)
drivers/net/ethernet/qlogic/qed/qed_dev.c +14 -1
@@ -1636,7 +1636,7 @@ static int qed_vf_start(struct qed_hwfn *p_hwfn,int qed_hw_init (struct qed_dev *cdev,struct qed_hw_init_params *p_params) {struct qed_load_req_params load_req_params; -u32 load_code, param, drv_mb_param; +u32 load_code, resp, param, drv_mb_param;bool b_default_mtu = true;struct qed_hwfn *p_hwfn;int rc =0 , mfw_rc, i;
@@ -1782,6 +1782,19 @@ int qed_hw_init(struct qed_dev *cdev, struct qed_hw_init_params *p_params)if (IS_PF (cdev)) { p_hwfn =QED_LEADING_HWFN (cdev); + +/* Get pre-negotiated values for stag, bandwidth etc. */ +DP_VERBOSE (p_hwfn, +QED_MSG_SPQ , +"Sending GET_OEM_UPDATES command to trigger stag/bandwidth attention handling\n" ); + drv_mb_param =1 <<DRV_MB_PARAM_DUMMY_OEM_UPDATES_OFFSET ; + rc =qed_mcp_cmd (p_hwfn, p_hwfn->p_main_ptt, +DRV_MSG_CODE_GET_OEM_UPDATES , + drv_mb_param, &resp, ¶m); +if (rc) +DP_NOTICE (p_hwfn, +"Failed to send GET_OEM_UPDATES attention request\n" ); + drv_mb_param =STORM_FW_VERSION ; rc = qed_mcp_cmd(p_hwfn, p_hwfn->p_main_ptt, DRV_MSG_CODE_OV_UPDATE_STORM_FW_VER,
drivers/net/ethernet/qlogic/qed/qed_hsi.h +4 -0
@@ -12414,6 +12414,7 @@ struct public_drv_mb {#define DRV_MSG_SET_RESOURCE_VALUE_MSG 0x35000000#define DRV_MSG_CODE_OV_UPDATE_WOL 0x38000000#define DRV_MSG_CODE_OV_UPDATE_ESWITCH_MODE 0x39000000 +#define DRV_MSG_CODE_GET_OEM_UPDATES 0x41000000#define DRV_MSG_CODE_BW_UPDATE_ACK 0x32000000#define DRV_MSG_CODE_NIG_DRAIN 0x30000000
@@ -12539,6 +12540,9 @@ struct public_drv_mb {#define DRV_MB_PARAM_ESWITCH_MODE_VEB 0x1#define DRV_MB_PARAM_ESWITCH_MODE_VEPA 0x2 +#define DRV_MB_PARAM_DUMMY_OEM_UPDATES_MASK 0x1 +#define DRV_MB_PARAM_DUMMY_OEM_UPDATES_OFFSET 0 +#define DRV_MB_PARAM_SET_LED_MODE_OPER 0x0#define DRV_MB_PARAM_SET_LED_MODE_ON 0x1#define DRV_MB_PARAM_SET_LED_MODE_OFF 0x2
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 35/58 qed: Do not add VLAN 0 tag to untagged frames in multi-function mode.
In reply to Sasha LevinFrom: Sudarsana Reddy Kalluru <[email protected]>
[ Upstream commit 0216da9413afa546627a1b0d319dfd17fef34050 ]
In certain multi-function switch dependent modes, firmware adds vlan tag 0 to the untagged frames. This leads to double tagging for the traffic if the dcbx is enabled, which is not the desired behavior. To avoid this, driver needs to set "dcb_dont_add_vlan0" flag.
--- drivers/net/ethernet/qlogic/qed/qed_dcbx.c | 9 ++++++++- drivers/net/ethernet/qlogic/qed/qed_dcbx.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
drivers/net/ethernet/qlogic/qed/qed_dcbx.c +8 -1
@@ -190,6 +190,7 @@ qed_dcbx_dp_protocol(struct qed_hwfn *p_hwfn, struct qed_dcbx_results *p_data) static void qed_dcbx_set_params(struct qed_dcbx_results *p_data, + struct qed_hwfn *p_hwfn, struct qed_hw_info *p_info, bool enable, u8 prio,
@@ -206,6 +207,11 @@ qed_dcbx_set_params(struct qed_dcbx_results *p_data,else p_data->arr[type].update =DONT_UPDATE_DCB_DSCP ; +/* Do not add vlan tag 0 when DCB is enabled and port in UFP/OV mode */ +if ((test_bit (QED_MF_8021Q_TAGGING , &p_hwfn->cdev->mf_bits) || +test_bit (QED_MF_8021AD_TAGGING , &p_hwfn->cdev->mf_bits))) + p_data->arr[type].dont_add_vlan0 = true; +/* QM reconf data */ if (p_info->personality == personality) p_info->offload_tc = tc;
@@ -233,7 +239,7 @@ qed_dcbx_update_app_info(struct qed_dcbx_results *p_data, personality = qed_dcbx_app_update[i].personality; name = qed_dcbx_app_update[i].name; -qed_dcbx_set_params (p_data, p_info, enable, +qed_dcbx_set_params (p_data, p_hwfn, p_info, enable, prio, tc, type, personality); } }
@@ -956,6 +962,7 @@ static void qed_dcbx_update_protocol_data(struct protocol_dcb_data *p_data, p_data->dcb_enable_flag = p_src->arr[type].enable; p_data->dcb_priority = p_src->arr[type].priority; p_data->dcb_tc = p_src->arr[type].tc; + p_data->dcb_dont_add_vlan0 = p_src->arr[type].dont_add_vlan0; }/* Set pf update ramrod command params */
drivers/net/ethernet/qlogic/qed/qed_dcbx.h +1 -0
@@ -55,6 +55,7 @@ struct qed_dcbx_app_data {u8 update;/* Update indication */ u8 priority;/* Priority */ u8 tc;/* Traffic Class */ +bool dont_add_vlan0;/* Do not insert a vlan tag with id 0 */ };#define QED_DCBX_VERSION_DISABLED 0
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 36/58 bnxt_en: don't try to offload VLAN 'modify' action
In reply to Sasha LevinFrom: Davide Caratti <[email protected]>
[ Upstream commit 8c6ec3613e7b0aade20a3196169c0bab32ed3e3f ]
bnxt offload code currently supports only 'push' and 'pop' operation: let .ndo_setup_tc() return -EOPNOTSUPP if VLAN 'modify' action is configured.
--- drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c +14 -6
@@ -75,17 +75,23 @@ static int bnxt_tc_parse_redir(struct bnxt *bp,return 0 ; } -static void bnxt_tc_parse_vlan (struct bnxt *bp, -struct bnxt_tc_actions *actions, -const struct tc_action *tc_act) +static int bnxt_tc_parse_vlan (struct bnxt *bp, +struct bnxt_tc_actions *actions, +const struct tc_action *tc_act) { -if (tcf_vlan_action (tc_act) ==TCA_VLAN_ACT_POP ) { +switch (tcf_vlan_action (tc_act)) { +case TCA_VLAN_ACT_POP : actions->flags |=BNXT_TC_ACTION_FLAG_POP_VLAN ; - }else if (tcf_vlan_action (tc_act) ==TCA_VLAN_ACT_PUSH ) { +break ; +case TCA_VLAN_ACT_PUSH : actions->flags |=BNXT_TC_ACTION_FLAG_PUSH_VLAN ; actions->push_vlan_tci =htons (tcf_vlan_push_vid (tc_act)); actions->push_vlan_tpid =tcf_vlan_push_proto (tc_act); +break ; +default : +return -EOPNOTSUPP ; } +return 0 ; } static int bnxt_tc_parse_tunnel_set(struct bnxt *bp,
@@ -136,7 +142,9 @@ static int bnxt_tc_parse_actions(struct bnxt *bp,/* Push/pop VLAN */ if (is_tcf_vlan (tc_act)) { -bnxt_tc_parse_vlan (bp, actions, tc_act); + rc =bnxt_tc_parse_vlan (bp, actions, tc_act); +if (rc) +return rc;continue ; }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 37/58 PCI: dwc: Fix scheduling while atomic issues
In reply to Sasha LevinFrom: Jisheng Zhang <[email protected]>
[ Upstream commit 9024143e700f89d74b8cdaf316a3499d74fc56fe ]
When programming the inbound/outbound ATUs, we call usleep_range() after each checking PCIE_ATU_ENABLE bit. Unfortunately, the ATU programming can be executed in atomic context:
inbound ATU programming could be called throughpci_epc_write_header()
=>dw_pcie_ep_write_header()
=>dw_pcie_prog_inbound_atu()
outbound ATU programming could be called throughpci_bus_read_config_dword()
=>dw_pcie_rd_conf()
=>dw_pcie_prog_outbound_atu()
Fix this issue by calling mdelay() instead.
--- drivers/pci/controller/dwc/pcie-designware.c | 8 ++++---- drivers/pci/controller/dwc/pcie-designware.h | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-)
drivers/pci/controller/dwc/pcie-designware.c +4 -4
@@ -135,7 +135,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,if (val &PCIE_ATU_ENABLE )return ; -usleep_range (LINK_WAIT_IATU_MIN ,LINK_WAIT_IATU_MAX ); +mdelay (LINK_WAIT_IATU ); }dev_err (pci->dev,"Outbound iATU is not being enabled\n" ); }
@@ -178,7 +178,7 @@ void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,if (val &PCIE_ATU_ENABLE )return ; -usleep_range (LINK_WAIT_IATU_MIN ,LINK_WAIT_IATU_MAX ); +mdelay (LINK_WAIT_IATU ); }dev_err (pci->dev,"Outbound iATU is not being enabled\n" ); }
@@ -236,7 +236,7 @@ static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,if (val &PCIE_ATU_ENABLE )return 0 ; -usleep_range (LINK_WAIT_IATU_MIN ,LINK_WAIT_IATU_MAX ); +mdelay (LINK_WAIT_IATU ); }dev_err (pci->dev,"Inbound iATU is not being enabled\n" );
@@ -282,7 +282,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,if (val &PCIE_ATU_ENABLE )return 0 ; -usleep_range (LINK_WAIT_IATU_MIN ,LINK_WAIT_IATU_MAX ); +mdelay (LINK_WAIT_IATU ); }dev_err (pci->dev,"Inbound iATU is not being enabled\n" );
drivers/pci/controller/dwc/pcie-designware.h +1 -2
@@ -26,8 +26,7 @@/* Parameters for the waiting for iATU enabled routine */ #define LINK_WAIT_MAX_IATU_RETRIES 5 -#define LINK_WAIT_IATU_MIN 9000 -#define LINK_WAIT_IATU_MAX 10000 +#define LINK_WAIT_IATU 9/* Synopsys-specific PCIe configuration registers */ #define PCIE_PORT_LINK_CONTROL 0x710
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 38/58 RDMA/uverbs: Fix validity check for modify QP
In reply to Sasha LevinFrom: Majd Dibbiny <[email protected]>
[ Upstream commit 4eeed3686981ff887bbdd7254139e2eca276534c ]
Uverbs shouldn't enforce QP state in the command unless the user set the QP state bit in the attribute mask.
In addition, only copy qp attr fields which have the corresponding bit set in the attribute mask over to the internal attr structure.
--- drivers/infiniband/core/uverbs_cmd.c | 68 ++++++++++++++++++---------- 1 file changed, 45 insertions(+), 23 deletions(-)
drivers/infiniband/core/uverbs_cmd.c +45 -23
@@ -2048,33 +2048,55 @@ static int modify_qp(struct ib_uverbs_file *file,if ((cmd->base.attr_mask &IB_QP_CUR_STATE && cmd->base.cur_qp_state >IB_QPS_ERR ) || - cmd->base.qp_state >IB_QPS_ERR ) { + (cmd->base.attr_mask &IB_QP_STATE && + cmd->base.qp_state >IB_QPS_ERR )) { ret = -EINVAL ; goto release_qp; } - attr->qp_state = cmd->base.qp_state; - attr->cur_qp_state = cmd->base.cur_qp_state; - attr->path_mtu = cmd->base.path_mtu; - attr->path_mig_state = cmd->base.path_mig_state; - attr->qkey = cmd->base.qkey; - attr->rq_psn = cmd->base.rq_psn; - attr->sq_psn = cmd->base.sq_psn; - attr->dest_qp_num = cmd->base.dest_qp_num; - attr->qp_access_flags = cmd->base.qp_access_flags; - attr->pkey_index = cmd->base.pkey_index; - attr->alt_pkey_index = cmd->base.alt_pkey_index; - attr->en_sqd_async_notify = cmd->base.en_sqd_async_notify; - attr->max_rd_atomic = cmd->base.max_rd_atomic; - attr->max_dest_rd_atomic = cmd->base.max_dest_rd_atomic; - attr->min_rnr_timer = cmd->base.min_rnr_timer; - attr->port_num = cmd->base.port_num; - attr->timeout = cmd->base.timeout; - attr->retry_cnt = cmd->base.retry_cnt; - attr->rnr_retry = cmd->base.rnr_retry; - attr->alt_port_num = cmd->base.alt_port_num; - attr->alt_timeout = cmd->base.alt_timeout; - attr->rate_limit = cmd->rate_limit; +if (cmd->base.attr_mask &IB_QP_STATE ) + attr->qp_state = cmd->base.qp_state; +if (cmd->base.attr_mask &IB_QP_CUR_STATE ) + attr->cur_qp_state = cmd->base.cur_qp_state; +if (cmd->base.attr_mask &IB_QP_PATH_MTU ) + attr->path_mtu = cmd->base.path_mtu; +if (cmd->base.attr_mask &IB_QP_PATH_MIG_STATE ) + attr->path_mig_state = cmd->base.path_mig_state; +if (cmd->base.attr_mask &IB_QP_QKEY ) + attr->qkey = cmd->base.qkey; +if (cmd->base.attr_mask &IB_QP_RQ_PSN ) + attr->rq_psn = cmd->base.rq_psn; +if (cmd->base.attr_mask &IB_QP_SQ_PSN ) + attr->sq_psn = cmd->base.sq_psn; +if (cmd->base.attr_mask &IB_QP_DEST_QPN ) + attr->dest_qp_num = cmd->base.dest_qp_num; +if (cmd->base.attr_mask &IB_QP_ACCESS_FLAGS ) + attr->qp_access_flags = cmd->base.qp_access_flags; +if (cmd->base.attr_mask &IB_QP_PKEY_INDEX ) + attr->pkey_index = cmd->base.pkey_index; +if (cmd->base.attr_mask &IB_QP_EN_SQD_ASYNC_NOTIFY ) + attr->en_sqd_async_notify = cmd->base.en_sqd_async_notify; +if (cmd->base.attr_mask &IB_QP_MAX_QP_RD_ATOMIC ) + attr->max_rd_atomic = cmd->base.max_rd_atomic; +if (cmd->base.attr_mask &IB_QP_MAX_DEST_RD_ATOMIC ) + attr->max_dest_rd_atomic = cmd->base.max_dest_rd_atomic; +if (cmd->base.attr_mask &IB_QP_MIN_RNR_TIMER ) + attr->min_rnr_timer = cmd->base.min_rnr_timer; +if (cmd->base.attr_mask &IB_QP_PORT ) + attr->port_num = cmd->base.port_num; +if (cmd->base.attr_mask &IB_QP_TIMEOUT ) + attr->timeout = cmd->base.timeout; +if (cmd->base.attr_mask &IB_QP_RETRY_CNT ) + attr->retry_cnt = cmd->base.retry_cnt; +if (cmd->base.attr_mask &IB_QP_RNR_RETRY ) + attr->rnr_retry = cmd->base.rnr_retry; +if (cmd->base.attr_mask &IB_QP_ALT_PATH ) { + attr->alt_port_num = cmd->base.alt_port_num; + attr->alt_timeout = cmd->base.alt_timeout; + attr->alt_pkey_index = cmd->base.alt_pkey_index; + } +if (cmd->base.attr_mask &IB_QP_RATE_LIMIT ) + attr->rate_limit = cmd->rate_limit;if (cmd->base.attr_mask &IB_QP_AV ) copy_ah_attr_from_uverbs(qp->device, &attr->ah_attr,
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 39/58 scsi: lpfc: Synchronize access to remoteport via rport
In reply to Sasha LevinFrom: James Smart <[email protected]>
[ Upstream commit 9e210178267b80c4eeb832fade7e146a18c84915 ]
The driver currently uses the ndlp to get the local rport which is then used to get the nvme transport remoteport pointer. There can be cases where a stale remoteport pointer is obtained as synchronization isn't done through the different dereferences.
Correct by using locks to synchronize the dereferences.
--- drivers/scsi/lpfc/lpfc_attr.c | 15 ++++++++++----- drivers/scsi/lpfc/lpfc_debugfs.c | 10 +++++----- drivers/scsi/lpfc/lpfc_nvme.c | 11 ++++++++--- 3 files changed, 23 insertions(+), 13 deletions(-)
drivers/scsi/lpfc/lpfc_attr.c +10 -5
@@ -320,12 +320,12 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr, localport->port_id, statep);list_for_each_entry (ndlp, &vport->fc_nodes, nlp_listp) { + nrport =NULL ; +spin_lock (&vport->phba->hbalock); rport =lpfc_ndlp_get_nrport (ndlp); -if (!rport) -continue ; - -/* local short-hand pointer. */ - nrport = rport->remoteport; +if (rport) + nrport = rport->remoteport; +spin_unlock (&vport->phba->hbalock);if (!nrport)continue ;
@@ -3304,6 +3304,7 @@ lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport)struct lpfc_nodelist *ndlp;#if (IS_ENABLED (CONFIG_NVME_FC ))struct lpfc_nvme_rport *rport; +struct nvme_fc_remote_port *remoteport =NULL ;#endif shost =lpfc_shost_from_vport (vport);
@@ -3314,8 +3315,12 @@ lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport)if (ndlp->rport) ndlp->rport->dev_loss_tmo = vport->cfg_devloss_tmo;#if (IS_ENABLED (CONFIG_NVME_FC )) +spin_lock (&vport->phba->hbalock); rport =lpfc_ndlp_get_nrport (ndlp);if (rport) + remoteport = rport->remoteport; +spin_unlock (&vport->phba->hbalock); +if (remoteport)nvme_fc_set_remoteport_devloss (rport->remoteport, vport->cfg_devloss_tmo);#endif
drivers/scsi/lpfc/lpfc_debugfs.c +5 -5
@@ -551,7 +551,7 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)unsigned char *statep;struct nvme_fc_local_port *localport;struct lpfc_nvmet_tgtport *tgtp; -struct nvme_fc_remote_port *nrport; +struct nvme_fc_remote_port *nrport =NULL ;struct lpfc_nvme_rport *rport; cnt = (LPFC_NODELIST_SIZE /LPFC_NODELIST_ENTRY_SIZE );
@@ -696,11 +696,11 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size) len +=snprintf (buf + len, size - len,"\tRport List:\n" );list_for_each_entry (ndlp, &vport->fc_nodes, nlp_listp) {/* local short-hand pointer. */ +spin_lock (&phba->hbalock); rport =lpfc_ndlp_get_nrport (ndlp); -if (!rport) -continue ; - - nrport = rport->remoteport; +if (rport) + nrport = rport->remoteport; +spin_unlock (&phba->hbalock);if (!nrport)continue ;
drivers/scsi/lpfc/lpfc_nvme.c +8 -3
@@ -2718,7 +2718,9 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp) rpinfo.port_name =wwn_to_u64 (ndlp->nlp_portname.u.wwn); rpinfo.node_name =wwn_to_u64 (ndlp->nlp_nodename.u.wwn); +spin_lock_irq (&vport->phba->hbalock); oldrport =lpfc_ndlp_get_nrport (ndlp); +spin_unlock_irq (&vport->phba->hbalock);if (!oldrport)lpfc_nlp_get (ndlp);
@@ -2833,7 +2835,7 @@ lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)struct nvme_fc_local_port *localport;struct lpfc_nvme_lport *lport;struct lpfc_nvme_rport *rport; -struct nvme_fc_remote_port *remoteport; +struct nvme_fc_remote_port *remoteport =NULL ; localport = vport->localport;
@@ -2847,11 +2849,14 @@ lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)if (!lport) goto input_err; +spin_lock_irq (&vport->phba->hbalock); rport =lpfc_ndlp_get_nrport (ndlp); -if (!rport) +if (rport) + remoteport = rport->remoteport; +spin_unlock_irq (&vport->phba->hbalock); +if (!remoteport) goto input_err; - remoteport = rport->remoteport; lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_DISC, "6033 Unreg nvme remoteport %p, portname x%llx, " "port_id x%06x, portstate x%x port type x%x\n",
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 40/58 drm: mali-dp: Call drm_crtc_vblank_reset on device init
In reply to Sasha LevinFrom: Alexandru Gheorghe <[email protected]>
[ Upstream commit 69be1984ded00a11b1ed0888c6d8e4f35370372f ]
Currently, if userspace calls drm_wait_vblank before the crtc is activated the crtc vblank_enable hook is called, which in case of malidp driver triggers some warninngs. This happens because on device init we don't inform the drm core about the vblank state by calling drm_crtc_vblank_on/off/reset which together with drm_vblank_get have some magic that prevents calling drm_vblank_enable when crtc is off.
--- drivers/gpu/drm/arm/malidp_drv.c | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/arm/malidp_drv.c +1 -0
@@ -615,6 +615,7 @@ static int malidp_bind(struct device *dev) drm->irq_enabled = true; ret =drm_vblank_init (drm, drm->mode_config.num_crtc); +drm_crtc_vblank_reset (&malidp->crtc);if (ret <0 ) {DRM_ERROR ("failed to initialise vblank\n" ); goto vblank_fail;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 41/58 net: mscc: fix the frame extraction into the skb
In reply to Sasha LevinFrom: Antoine Tenart <[email protected]>
[ Upstream commit 652ef42c134da1bbb03bd4c9b4291dfaf8d7febb ]
When extracting frames from the Ocelot switch, the frame check sequence (FCS) is present at the end of the data extracted. The FCS was put into the sk buffer which introduced some issues (as length related ones), as the FCS shouldn't be part of an Rx sk buffer.
This patch fixes the Ocelot switch extraction behaviour by discarding the FCS.
--- drivers/net/ethernet/mscc/ocelot_board.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/net/ethernet/mscc/ocelot_board.c +9 -3
@@ -91,7 +91,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)struct sk_buff *skb;struct net_device *dev;u32 *buf; -int sz, len; +int sz, len, buf_len;u32 ifh[4 ];u32 val;struct frame_info info;
@@ -116,14 +116,20 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg) err = -ENOMEM ;break ; } - buf = (u32 *)skb_put (skb, info.len); + buf_len = info.len -ETH_FCS_LEN ; + buf = (u32 *)skb_put (skb, buf_len); len =0 ;do { sz =ocelot_rx_frame_word (ocelot, grp, false, &val); *buf++ = val; len += sz; - }while ((sz ==4 ) && (len < info.len)); + }while (len < buf_len); + +/* Read the FCS and discard it */ + sz =ocelot_rx_frame_word (ocelot, grp, false, &val); +/* Update the statistics if part of the FCS was read before */ + len -=ETH_FCS_LEN - sz;if (sz <0 ) { err = sz;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 42/58 scsi: ipr: System hung while dlpar adding primary ipr adapter back
In reply to Sasha LevinFrom: Wen Xiong <[email protected]>
[ Upstream commit 318ddb34b2052f838aa243d07173e2badf3e630e ]
While dlpar adding primary ipr adapter back, driver goes through adapter initialization then schedule ipr_worker_thread to start te disk scan by dropping the host lock, calling scsi_add_device. Then get the adapter reset request again, so driver does scsi_block_requests, this will cause the scsi_add_device get hung until we unblock. But we can't run ipr_worker_thread to do the unblock because its stuck in scsi_add_device.
This patch fixes the issue.
[mkp: typo and whitespace fixes]
--- drivers/scsi/ipr.c | 106 ++++++++++++++++++++++++++------------------- drivers/scsi/ipr.h | 1 + 2 files changed, 62 insertions(+), 45 deletions(-)
drivers/scsi/ipr.c +61 -45
@@ -3310,6 +3310,65 @@ static void ipr_release_dump(struct kref *kref)LEAVE ; } +static void ipr_add_remove_thread (struct work_struct *work) +{ +unsigned long lock_flags; +struct ipr_resource_entry *res; +struct scsi_device *sdev; +struct ipr_ioa_cfg *ioa_cfg = + container_of(work, struct ipr_ioa_cfg, scsi_add_work_q); +u8 bus, target, lun; +int did_work; + +ENTER ; +spin_lock_irqsave (ioa_cfg->host->host_lock, lock_flags); + +restart: +do { + did_work =0 ; +if (!ioa_cfg->hrrq[IPR_INIT_HRRQ ].allow_cmds) { +spin_unlock_irqrestore (ioa_cfg->host->host_lock, lock_flags); +return ; + } + +list_for_each_entry (res, &ioa_cfg->used_res_q, queue) { +if (res->del_from_ml && res->sdev) { + did_work =1 ; + sdev = res->sdev; +if (!scsi_device_get (sdev)) { +if (!res->add_to_ml) +list_move_tail (&res->queue, &ioa_cfg->free_res_q); +else + res->del_from_ml =0 ; +spin_unlock_irqrestore (ioa_cfg->host->host_lock, lock_flags); +scsi_remove_device (sdev); +scsi_device_put (sdev); +spin_lock_irqsave (ioa_cfg->host->host_lock, lock_flags); + } +break ; + } + } + }while (did_work); + +list_for_each_entry (res, &ioa_cfg->used_res_q, queue) { +if (res->add_to_ml) { + bus = res->bus; + target = res->target; + lun = res->lun; + res->add_to_ml =0 ; +spin_unlock_irqrestore (ioa_cfg->host->host_lock, lock_flags); +scsi_add_device (ioa_cfg->host, bus, target, lun); +spin_lock_irqsave (ioa_cfg->host->host_lock, lock_flags); + goto restart; + } + } + + ioa_cfg->scan_done =1 ; +spin_unlock_irqrestore (ioa_cfg->host->host_lock, lock_flags); +kobject_uevent (&ioa_cfg->host->shost_dev.kobj,KOBJ_CHANGE ); +LEAVE ; +} +/** * ipr_worker_thread - Worker thread * @work: ioa config struct
@@ -3324,13 +3383,9 @@ static void ipr_release_dump(struct kref *kref)static void ipr_worker_thread (struct work_struct *work) {unsigned long lock_flags; -struct ipr_resource_entry *res; -struct scsi_device *sdev;struct ipr_dump *dump;struct ipr_ioa_cfg *ioa_cfg = container_of(work, struct ipr_ioa_cfg, work_q); -u8 bus, target, lun; -int did_work;ENTER ;spin_lock_irqsave (ioa_cfg->host->host_lock, lock_flags);
@@ -3368,49 +3423,9 @@ static void ipr_worker_thread(struct work_struct *work)return ; } -restart: -do { - did_work =0 ; -if (!ioa_cfg->hrrq[IPR_INIT_HRRQ ].allow_cmds) { -spin_unlock_irqrestore (ioa_cfg->host->host_lock, lock_flags); -return ; - } +schedule_work (&ioa_cfg->scsi_add_work_q); -list_for_each_entry (res, &ioa_cfg->used_res_q, queue) { -if (res->del_from_ml && res->sdev) { - did_work =1 ; - sdev = res->sdev; -if (!scsi_device_get (sdev)) { -if (!res->add_to_ml) -list_move_tail (&res->queue, &ioa_cfg->free_res_q); -else - res->del_from_ml =0 ; -spin_unlock_irqrestore (ioa_cfg->host->host_lock, lock_flags); -scsi_remove_device (sdev); -scsi_device_put (sdev); -spin_lock_irqsave (ioa_cfg->host->host_lock, lock_flags); - } -break ; - } - } - }while (did_work); - -list_for_each_entry (res, &ioa_cfg->used_res_q, queue) { -if (res->add_to_ml) { - bus = res->bus; - target = res->target; - lun = res->lun; - res->add_to_ml =0 ; -spin_unlock_irqrestore (ioa_cfg->host->host_lock, lock_flags); -scsi_add_device (ioa_cfg->host, bus, target, lun); -spin_lock_irqsave (ioa_cfg->host->host_lock, lock_flags); - goto restart; - } - } - - ioa_cfg->scan_done =1 ;spin_unlock_irqrestore (ioa_cfg->host->host_lock, lock_flags); -kobject_uevent (&ioa_cfg->host->shost_dev.kobj,KOBJ_CHANGE );LEAVE ; }
@@ -9908,6 +9923,7 @@ static void ipr_init_ioa_cfg(struct ipr_ioa_cfg *ioa_cfg,INIT_LIST_HEAD (&ioa_cfg->free_res_q);INIT_LIST_HEAD (&ioa_cfg->used_res_q);INIT_WORK (&ioa_cfg->work_q, ipr_worker_thread); +INIT_WORK (&ioa_cfg->scsi_add_work_q, ipr_add_remove_thread);init_waitqueue_head (&ioa_cfg->reset_wait_q);init_waitqueue_head (&ioa_cfg->msi_wait_q);init_waitqueue_head (&ioa_cfg->eeh_wait_q);
drivers/scsi/ipr.h +1 -0
@@ -1568,6 +1568,7 @@ struct ipr_ioa_cfg {u8 saved_mode_page_len;struct work_struct work_q; +struct work_struct scsi_add_work_q;struct workqueue_struct *reset_work_q;wait_queue_head_t reset_wait_q;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 43/58 scsi: sd: don't crash the host on invalid commands
In reply to Sasha LevinFrom: Johannes Thumshirn <[email protected]>
[ Upstream commit f1f1fadacaf08b7cf11714c0c29f8fa4d4ef68a9 ]
When sd_init_command() get's a command with a unknown req_op() it crashes the system via BUG().
This makes debugging the actual reason for the broken request cmd_flags pretty hard as the system is down before it's able to write out debugging data on the serial console or the trace buffer.
Change the BUG() to a WARN_ON() and return BLKPREP_KILL to fail gracefully and return an I/O error to the producer of the request.
--- drivers/scsi/sd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/scsi/sd.c +2 -1
@@ -1277,7 +1277,8 @@ static int sd_init_command(struct scsi_cmnd *cmd)case REQ_OP_ZONE_RESET :return sd_zbc_setup_reset_cmnd (cmd);default : -BUG (); +WARN_ON_ONCE (1 ); +return BLKPREP_KILL ; } }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 44/58 bpf: sockmap only allow ESTABLISHED sock state
In reply to Sasha LevinFrom: John Fastabend <[email protected]>
[ Upstream commit 5607fff303636d48b88414c6be353d9fed700af2 ]
After this patch we only allow socks that are in ESTABLISHED state or are being added via a sock_ops event that is transitioning into an ESTABLISHED state. By allowing sock_ops events we allow users to manage sockmaps directly from sock ops programs. The two supported sock_ops ops are BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB and BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB.
Similar to TLS ULP this ensures sk_user_data is correct.
--- kernel/bpf/sockmap.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
kernel/bpf/sockmap.c +30 -1
@@ -2092,8 +2092,12 @@ static int sock_map_update_elem(struct bpf_map *map,return -EINVAL ; } +/* ULPs are currently supported only for TCP sockets in ESTABLISHED +* state. +*/ if (skops.sk->sk_type !=SOCK_STREAM || - skops.sk->sk_protocol !=IPPROTO_TCP ) { + skops.sk->sk_protocol !=IPPROTO_TCP || + skops.sk->sk_state !=TCP_ESTABLISHED ) {fput (socket->file);return -EOPNOTSUPP ; }
@@ -2448,6 +2452,16 @@ static int sock_hash_update_elem(struct bpf_map *map,return -EINVAL ; } +/* ULPs are currently supported only for TCP sockets in ESTABLISHED +* state. +*/ +if (skops.sk->sk_type !=SOCK_STREAM || + skops.sk->sk_protocol !=IPPROTO_TCP || + skops.sk->sk_state !=TCP_ESTABLISHED ) { +fput (socket->file); +return -EOPNOTSUPP ; + } +lock_sock (skops.sk);preempt_disable ();rcu_read_lock ();
@@ -2536,10 +2550,22 @@ const struct bpf_map_ops sock_hash_ops = { .map_release_uref = sock_map_release, }; +static bool bpf_is_valid_sock_op (struct bpf_sock_ops_kern *ops) +{ +return ops->op ==BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB || + ops->op ==BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB ; +} BPF_CALL_4(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock, struct bpf_map *, map, void *, key, u64, flags) {WARN_ON_ONCE (!rcu_read_lock_held ()); + +/* ULPs are currently supported only for TCP sockets in ESTABLISHED +* state. This checks that the sock ops triggering the update is +* one indicating we are (or will be soon) in an ESTABLISHED state. +*/ +if (!bpf_is_valid_sock_op (bpf_sock)) +return -EOPNOTSUPP ;return sock_map_ctx_update_elem (bpf_sock, map, key, flags); }
@@ -2558,6 +2584,9 @@ BPF_CALL_4(bpf_sock_hash_update, struct bpf_sock_ops_kern *, bpf_sock, struct bpf_map *, map, void *, key, u64, flags) { WARN_ON_ONCE(!rcu_read_lock_held()); + +if (!bpf_is_valid_sock_op (bpf_sock)) +return -EOPNOTSUPP ;return sock_hash_ctx_update_elem (bpf_sock, map, key, flags); }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 45/58 bpf: sockmap, fix transition through disconnect without close
In reply to Sasha LevinFrom: John Fastabend <[email protected]>
[ Upstream commit b05545e15e1ff1d6a6a8593971275f9cc3e6b92b ]
It is possible (via shutdown()) for TCP socks to go trough TCP_CLOSE state via tcp_disconnect() without actually calling tcp_close which would then call our bpf_tcp_close() callback. Because of this a user could disconnect a socket then put it in a LISTEN state which would break our assumptions about sockets always being ESTABLISHED state.
To resolve this rely on the unhash hook, which is called in the disconnect case, to remove the sock from the sockmap.
--- kernel/bpf/sockmap.c | 60 ++++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 19 deletions(-)
kernel/bpf/sockmap.c +41 -19
@@ -132,6 +132,7 @@ struct smap_psock {struct work_struct gc_work;struct proto *sk_proto; +void (*save_unhash)(struct sock *sk);void (*save_close)(struct sock *sk,long timeout);void (*save_data_ready)(struct sock *sk);void (*save_write_space)(struct sock *sk);
@@ -143,6 +144,7 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,static int bpf_tcp_sendmsg (struct sock *sk,struct msghdr *msg,size_t size);static int bpf_tcp_sendpage (struct sock *sk,struct page *page,int offset,size_t size,int flags); +static void bpf_tcp_unhash (struct sock *sk);static void bpf_tcp_close (struct sock *sk,long timeout);static inline struct smap_psock *smap_psock_sk (const struct sock *sk)
@@ -184,6 +186,7 @@ static void build_protos(struct proto prot[SOCKMAP_NUM_CONFIGS], struct proto *base) { prot[SOCKMAP_BASE ] = *base; + prot[SOCKMAP_BASE ].unhash = bpf_tcp_unhash; prot[SOCKMAP_BASE ].close = bpf_tcp_close; prot[SOCKMAP_BASE ].recvmsg = bpf_tcp_recvmsg; prot[SOCKMAP_BASE ].stream_memory_read = bpf_tcp_stream_read;
@@ -217,6 +220,7 @@ static int bpf_tcp_init(struct sock *sk)return -EBUSY ; } + psock->save_unhash = sk->sk_prot->unhash; psock->save_close = sk->sk_prot->close; psock->sk_proto = sk->sk_prot;
@@ -305,30 +309,12 @@ static struct smap_psock_map_entry *psock_map_pop(struct sock *sk,return e; } -static void bpf_tcp_close (struct sock *sk,long timeout) +static void bpf_tcp_remove (struct sock *sk,struct smap_psock *psock) { -void (*close_fun)(struct sock *sk,long timeout);struct smap_psock_map_entry *e;struct sk_msg_buff *md, *mtmp; -struct smap_psock *psock;struct sock *osk; -lock_sock (sk); -rcu_read_lock (); - psock =smap_psock_sk (sk); -if (unlikely (!psock)) { -rcu_read_unlock (); -release_sock (sk); -return sk->sk_prot->close (sk, timeout); - } - -/* The psock may be destroyed anytime after exiting the RCU critial -* section so by the time we use close_fun the psock may no longer -* be valid. However, bpf_tcp_close is called with the sock lock -* held so the close hook and sk are still valid. -*/ - close_fun = psock->save_close; -if (psock->cork) {free_start_sg (psock->sock, psock->cork);kfree (psock->cork);
@@ -379,6 +365,42 @@ static void bpf_tcp_close(struct sock *sk, long timeout)kfree (e); e =psock_map_pop (sk, psock); } +} + +static void bpf_tcp_unhash (struct sock *sk) +{ +void (*unhash_fun)(struct sock *sk); +struct smap_psock *psock; + +rcu_read_lock (); + psock =smap_psock_sk (sk); +if (unlikely (!psock)) { +rcu_read_unlock (); +if (sk->sk_prot->unhash) + sk->sk_prot->unhash (sk); +return ; + } + unhash_fun = psock->save_unhash; +bpf_tcp_remove (sk, psock); +rcu_read_unlock (); +unhash_fun (sk); +} + +static void bpf_tcp_close (struct sock *sk,long timeout) +{ +void (*close_fun)(struct sock *sk,long timeout); +struct smap_psock *psock; + +lock_sock (sk); +rcu_read_lock (); + psock =smap_psock_sk (sk); +if (unlikely (!psock)) { +rcu_read_unlock (); +release_sock (sk); +return sk->sk_prot->close (sk, timeout); + } + close_fun = psock->save_close; +bpf_tcp_remove (sk, psock);rcu_read_unlock ();release_sock (sk);close_fun (sk, timeout);
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 46/58 bpf: test_maps, only support ESTABLISHED socks
In reply to Sasha LevinFrom: John Fastabend <[email protected]>
[ Upstream commit 5028027844cfc6168e39650abecd817ba64c9d98 ]
Ensure that sockets added to a sock{map|hash} that is not in the ESTABLISHED state is rejected.
--- tools/testing/selftests/bpf/test_maps.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
tools/testing/selftests/bpf/test_maps.c +7 -3
@@ -566,7 +566,11 @@ static void test_sockmap(int tasks, void *data)/* Test update without programs */ for (i =0 ; i <6 ; i++) { err =bpf_map_update_elem (fd, &i, &sfd[i],BPF_ANY ); -if (err) { +if (i <2 && !err) { +printf ("Allowed update sockmap '%i:%i' not in ESTABLISHED\n" , + i, sfd[i]); + goto out_sockmap; + }else if (i >=2 && err) {printf ("Failed noprog update sockmap '%i:%i'\n" , i, sfd[i]); goto out_sockmap;
@@ -727,7 +731,7 @@ static void test_sockmap(int tasks, void *data) }/* Test map update elem afterwards fd lives in fd and map_fd */ -for (i =0 ; i <6 ; i++) { +for (i =2 ; i <6 ; i++) { err =bpf_map_update_elem (map_fd_rx, &i, &sfd[i],BPF_ANY );if (err) { printf("Failed map_fd_rx update sockmap %i '%i:%i'\n",
@@ -831,7 +835,7 @@ static void test_sockmap(int tasks, void *data) }/* Delete the elems without programs */ -for (i =0 ; i <6 ; i++) { +for (i =2 ; i <6 ; i++) { err =bpf_map_delete_elem (fd, &i);if (err) { printf("Failed delete sockmap %i '%i:%i'\n",
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 47/58 net/mlx4: Use cpumask_available for eq->affinity_mask
In reply to Sasha LevinFrom: Nathan Chancellor <[email protected]>
[ Upstream commit 8ac1ee6f4d62e781e3b3fd8b9c42b70371427669 ]
Clang warns that the address of a pointer will always evaluated as true in a boolean context:
drivers/net/ethernet/mellanox/mlx4/eq.c:243:11: warning: address of array 'eq->affinity_mask' will always evaluate to 'true' [-Wpointer-bool-conversion]
if (!eq->affinity_mask || cpumask_empty(eq->affinity_mask))
~~~~~^~~~~~~~~~~~~1 warning generated.
Use cpumask_available, introduced in commit f7e30f01a9e2 ("cpumask: Add helper cpumask_available()"), which does the proper checking and avoids this warning.
--- drivers/net/ethernet/mellanox/mlx4/eq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlx4/eq.c +2 -1
@@ -240,7 +240,8 @@ static void mlx4_set_eq_affinity_hint(struct mlx4_priv *priv, int vec)struct mlx4_dev *dev = &priv->dev;struct mlx4_eq *eq = &priv->eq_table.eq[vec]; -if (!eq->affinity_mask ||cpumask_empty (eq->affinity_mask)) +if (!cpumask_available (eq->affinity_mask) || +cpumask_empty (eq->affinity_mask))return ; hint_err =irq_set_affinity_hint (eq->irq, eq->affinity_mask);
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 48/58 clocksource/drivers/fttmr010: Fix set_next_event handler
In reply to Sasha LevinFrom: Tao Ren <[email protected]>
[ Upstream commit 4451d3f59f2a6f95e5d205c2d04ea072955d080d ]
Currently, the aspeed MATCH1 register is updated to <current_count - cycles> in set_next_event handler, with the assumption that COUNT register value is preserved when the timer is disabled and it continues decrementing after the timer is enabled. But the assumption is wrong:
RELOAD register is loaded into COUNT register when the aspeed timer is enabled, which means the next event may be delayed because timer interrupt won't be generated until <0xFFFFFFFF - current_count + cycles>.
The problem can be fixed by updating RELOAD register to <cycles>, and COUNT register will be re-loaded when the timer is enabled and interrupt is generated when COUNT register overflows.
The test result on Facebook Backpack-CMM BMC hardware (AST2500) shows the issue is fixed: without the patch, usleep(100) suspends the process for several milliseconds (and sometimes even over 40 milliseconds); after applying the fix, usleep(100) takes averagely 240 microseconds to return under the same workload level.
--- drivers/clocksource/timer-fttmr010.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
drivers/clocksource/timer-fttmr010.c +11 -7
@@ -130,13 +130,17 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, cr &= ~fttmr010->t1_enable_val;writel (cr, fttmr010->base +TIMER_CR ); -/* Setup the match register forward/backward in time */ - cr =readl (fttmr010->base +TIMER1_COUNT ); -if (fttmr010->count_down) - cr -= cycles; -else - cr += cycles; -writel (cr, fttmr010->base +TIMER1_MATCH1 ); +if (fttmr010->count_down) { +/* +* ASPEED Timer Controller will load TIMER1_LOAD register +* into TIMER1_COUNT register when the timer is re-enabled. +*/ +writel (cycles, fttmr010->base +TIMER1_LOAD ); + }else { +/* Setup the match register forward in time */ + cr =readl (fttmr010->base +TIMER1_COUNT ); +writel (cr + cycles, fttmr010->base +TIMER1_MATCH1 ); + }/* Start */ cr =readl (fttmr010->base +TIMER_CR );
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 49/58 net: aquantia: memory corruption on jumbo frames
In reply to Sasha LevinFrom: Friedemann Gerold <[email protected]>
[ Upstream commit d26ed6b0e5e23190d43ab34bc69cbecdc464a2cf ]
This patch fixes skb_shared area, which will be corrupted upon reception of 4K jumbo packets.
Originally build_skb usage purpose was to reuse page for skb to eliminate needs of extra fragments. But that logic does not take into account that skb_shared_info should be reserved at the end of skb data area.
In case packet data consumes all the page (4K), skb_shinfo location overflows the page. As a consequence, __build_skb zeroed shinfo data above the allocated page, corrupting next page.
The issue is rarely seen in real life because jumbo are normally larger than 4K and that causes another code path to trigger.
But it 100% reproducible with simple scapy packet, like:
sendp(IP(dst="192.168.100.3") / TCP(dport=443) \
/ Raw(RandString(size=(4096-40))), iface="enp1s0")--- .../net/ethernet/aquantia/atlantic/aq_ring.c | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-)
drivers/net/ethernet/aquantia/atlantic/aq_ring.c +18 -14
@@ -225,9 +225,10 @@ int aq_ring_rx_clean(struct aq_ring_s *self, }/* for single fragment packets use build_skb() */ -if (buff->is_eop) { +if (buff->is_eop && + buff->len <=AQ_CFG_RX_FRAME_MAX -AQ_SKB_ALIGN ) { skb =build_skb (page_address (buff->page), - buff->len +AQ_SKB_ALIGN ); +AQ_CFG_RX_FRAME_MAX );if (unlikely (!skb)) { err = -ENOMEM ; goto err_exit;
@@ -247,18 +248,21 @@ int aq_ring_rx_clean(struct aq_ring_s *self, buff->len -ETH_HLEN , SKB_TRUESIZE(buff->len - ETH_HLEN)); -for (i =1U , next_ = buff->next, - buff_ = &self->buff_ring[next_]; true; - next_ = buff_->next, - buff_ = &self->buff_ring[next_], ++i) { -skb_add_rx_frag (skb, i, buff_->page,0 , - buff_->len, -SKB_TRUESIZE (buff->len - -ETH_HLEN )); - buff_->is_cleaned =1 ; - -if (buff_->is_eop) -break ; +if (!buff->is_eop) { +for (i =1U , next_ = buff->next, + buff_ = &self->buff_ring[next_]; + true; next_ = buff_->next, + buff_ = &self->buff_ring[next_], ++i) { +skb_add_rx_frag (skb, i, + buff_->page,0 , + buff_->len, +SKB_TRUESIZE (buff->len - +ETH_HLEN )); + buff_->is_cleaned =1 ; + +if (buff_->is_eop) +break ; + } } }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 50/58 RDMA/bnxt_re: Fix system crash during RDMA resource initialization
In reply to Sasha LevinFrom: Selvin Xavier <[email protected]>
[ Upstream commit de5c95d0f518537f59ee5aef762abc46f868c377 ]
bnxt_re_ib_reg acquires and releases the rtnl lock whenever it accesses the L2 driver.
The following sequence can trigger a crash
Acquires the rtnl_lock ->
Registers roce driver callback with L2 driver -> release the rtnl lock bnxt_re acquires the rtnl_lock -> Request for MSIx vectors -> release the rtnl_lock
Issue happens when bnxt_re proceeds with remaining part of initialization and L2 driver invokes bnxt_ulp_irq_stop as a part of bnxt_open_nic.
The crash is in bnxt_qplib_nq_stop_irq as the NQ structures are not initialized yet,
<snip>
[ 3551.726647] BUG: unable to handle kernel NULL pointer dereference at (null) [ 3551.726656] IP: [<ffffffffc0840ee9>] bnxt_qplib_nq_stop_irq+0x59/0xb0 [bnxt_re] [ 3551.726674] PGD 0 [ 3551.726679] Oops: 0002 1 SMP ... [ 3551.726822] Hardware name: Dell Inc. PowerEdge R720/08RW36, BIOS 2.4.3 07/09/2014 [ 3551.726826] task: ffff97e30eec5ee0 ti: ffff97e3173bc000 task.ti: ffff97e3173bc000 [ 3551.726829] RIP: 0010:[<ffffffffc0840ee9>] [<ffffffffc0840ee9>] bnxt_qplib_nq_stop_irq+0x59/0xb0 [bnxt_re] ... [ 3551.726872] Call Trace: [ 3551.726886] [<ffffffffc082cb9e>] bnxt_re_stop_irq+0x4e/0x70 [bnxt_re] [ 3551.726899] [<ffffffffc07d6a53>] bnxt_ulp_irq_stop+0x43/0x70 [bnxt_en] [ 3551.726908] [<ffffffffc07c82f4>] bnxt_reserve_rings+0x174/0x1e0 [bnxt_en] [ 3551.726917] [<ffffffffc07cafd8>] __bnxt_open_nic+0x368/0x9a0 [bnxt_en] [ 3551.726925] [<ffffffffc07cb62b>] bnxt_open_nic+0x1b/0x50 [bnxt_en] [ 3551.726934] [<ffffffffc07cc62f>] bnxt_setup_mq_tc+0x11f/0x260 [bnxt_en] [ 3551.726943] [<ffffffffc07d5f58>] bnxt_dcbnl_ieee_setets+0xb8/0x1f0 [bnxt_en] [ 3551.726954] [<ffffffff890f983a>] dcbnl_ieee_set+0x9a/0x250 [ 3551.726966] [<ffffffff88fd6d21>] ? __alloc_skb+0xa1/0x2d0 [ 3551.726972] [<ffffffff890f72fa>] dcb_doit+0x13a/0x210 [ 3551.726981] [<ffffffff89003ff7>] rtnetlink_rcv_msg+0xa7/0x260 [ 3551.726989] [<ffffffff88ffdb00>] ? rtnl_unicast+0x20/0x30 [ 3551.726996] [<ffffffff88bf9dc8>] ? __kmalloc_node_track_caller+0x58/0x290 [ 3551.727002] [<ffffffff890f7326>] ? dcb_doit+0x166/0x210 [ 3551.727007] [<ffffffff88fd6d0d>] ? __alloc_skb+0x8d/0x2d0 [ 3551.727012] [<ffffffff89003f50>] ? rtnl_newlink+0x880/0x880 ... [ 3551.727104] [<ffffffff8911f7d5>] system_call_fastpath+0x1c/0x21 ... [ 3551.727164] RIP [<ffffffffc0840ee9>] bnxt_qplib_nq_stop_irq+0x59/0xb0 [bnxt_re] [ 3551.727175] RSP <ffff97e3173bf788> [ 3551.727177] CR2: 0000000000000000
Avoid this inconsistent state and system crash by acquiring the rtnl lock for the entire duration of device initialization. Re-factor the code to remove the rtnl lock from the individual function and acquire and release it from the caller.
--- drivers/infiniband/hw/bnxt_re/main.c | 93 ++++++++++++---------------- 1 file changed, 38 insertions(+), 55 deletions(-)
drivers/infiniband/hw/bnxt_re/main.c +38 -55
@@ -78,7 +78,7 @@ static struct list_head bnxt_re_dev_list = LIST_HEAD_INIT(bnxt_re_dev_list);/* Mutex to protect the list of bnxt_re devices added */ static DEFINE_MUTEX (bnxt_re_dev_lock);static struct workqueue_struct *bnxt_re_wq; -static void bnxt_re_ib_unreg (struct bnxt_re_dev *rdev,bool lock_wait); +static void bnxt_re_ib_unreg (struct bnxt_re_dev *rdev);/* SR-IOV helper functions */
@@ -182,7 +182,7 @@ static void bnxt_re_shutdown(void *p)if (!rdev)return ; -bnxt_re_ib_unreg (rdev, false); +bnxt_re_ib_unreg (rdev); }static void bnxt_re_stop_irq (void *handle)
@@ -251,7 +251,7 @@ static struct bnxt_ulp_ops bnxt_re_ulp_ops = {/* Driver registration routines used to let the networking driver (bnxt_en) * to know that the RoCE driver is now installed */ -static int bnxt_re_unregister_netdev (struct bnxt_re_dev *rdev,bool lock_wait) +static int bnxt_re_unregister_netdev (struct bnxt_re_dev *rdev) {struct bnxt_en_dev *en_dev;int rc;
@@ -260,14 +260,9 @@ static int bnxt_re_unregister_netdev(struct bnxt_re_dev *rdev, bool lock_wait)return -EINVAL ; en_dev = rdev->en_dev; -/* Acquire rtnl lock if it is not invokded from netdev event */ -if (lock_wait) -rtnl_lock (); rc = en_dev->en_ops->bnxt_unregister_device (rdev->en_dev,BNXT_ROCE_ULP ); -if (lock_wait) -rtnl_unlock ();return rc; }
@@ -281,14 +276,12 @@ static int bnxt_re_register_netdev(struct bnxt_re_dev *rdev) en_dev = rdev->en_dev; -rtnl_lock (); rc = en_dev->en_ops->bnxt_register_device (en_dev,BNXT_ROCE_ULP , &bnxt_re_ulp_ops, rdev); -rtnl_unlock ();return rc; } -static int bnxt_re_free_msix (struct bnxt_re_dev *rdev,bool lock_wait) +static int bnxt_re_free_msix (struct bnxt_re_dev *rdev) {struct bnxt_en_dev *en_dev;int rc;
@@ -298,13 +291,9 @@ static int bnxt_re_free_msix(struct bnxt_re_dev *rdev, bool lock_wait) en_dev = rdev->en_dev; -if (lock_wait) -rtnl_lock (); rc = en_dev->en_ops->bnxt_free_msix (rdev->en_dev,BNXT_ROCE_ULP ); -if (lock_wait) -rtnl_unlock ();return rc; }
@@ -320,7 +309,6 @@ static int bnxt_re_request_msix(struct bnxt_re_dev *rdev) num_msix_want =min_t (u32,BNXT_RE_MAX_MSIX ,num_online_cpus ()); -rtnl_lock (); num_msix_got = en_dev->en_ops->bnxt_request_msix (en_dev,BNXT_ROCE_ULP , rdev->msix_entries, num_msix_want);
@@ -335,7 +323,6 @@ static int bnxt_re_request_msix(struct bnxt_re_dev *rdev) } rdev->num_msix = num_msix_got; done: -rtnl_unlock ();return rc; }
@@ -358,24 +345,18 @@ static void bnxt_re_fill_fw_msg(struct bnxt_fw_msg *fw_msg, void *msg, fw_msg->timeout = timeout; } -static int bnxt_re_net_ring_free (struct bnxt_re_dev *rdev,u16 fw_ring_id, -bool lock_wait) +static int bnxt_re_net_ring_free (struct bnxt_re_dev *rdev,u16 fw_ring_id) {struct bnxt_en_dev *en_dev = rdev->en_dev;struct hwrm_ring_free_input req = {0 };struct hwrm_ring_free_output resp;struct bnxt_fw_msg fw_msg; -bool do_unlock = false;int rc = -EINVAL ;if (!en_dev)return rc;memset (&fw_msg,0 ,sizeof (fw_msg)); -if (lock_wait) { -rtnl_lock (); - do_unlock = true; - }bnxt_re_init_hwrm_hdr (rdev, (void *)&req,HWRM_RING_FREE ,-1 ,-1 ); req.ring_type =RING_ALLOC_REQ_RING_TYPE_L2_CMPL ;
@@ -386,8 +367,6 @@ static int bnxt_re_net_ring_free(struct bnxt_re_dev *rdev, u16 fw_ring_id,if (rc)dev_err (rdev_to_dev (rdev),"Failed to free HW ring:%d :%#x" , req.ring_id, rc); -if (do_unlock) -rtnl_unlock ();return rc; }
@@ -405,7 +384,6 @@ static int bnxt_re_net_ring_alloc(struct bnxt_re_dev *rdev, dma_addr_t *dma_arr,return rc;memset (&fw_msg,0 ,sizeof (fw_msg)); -rtnl_lock ();bnxt_re_init_hwrm_hdr (rdev, (void *)&req,HWRM_RING_ALLOC ,-1 ,-1 ); req.enables =0 ; req.page_tbl_addr =cpu_to_le64 (dma_arr[0 ]);
@@ -426,27 +404,21 @@ static int bnxt_re_net_ring_alloc(struct bnxt_re_dev *rdev, dma_addr_t *dma_arr,if (!rc) *fw_ring_id =le16_to_cpu (resp.ring_id); -rtnl_unlock ();return rc; }static int bnxt_re_net_stats_ctx_free (struct bnxt_re_dev *rdev, -u32 fw_stats_ctx_id,bool lock_wait) +u32 fw_stats_ctx_id) {struct bnxt_en_dev *en_dev = rdev->en_dev;struct hwrm_stat_ctx_free_input req = {0 };struct bnxt_fw_msg fw_msg; -bool do_unlock = false;int rc = -EINVAL ;if (!en_dev)return rc;memset (&fw_msg,0 ,sizeof (fw_msg)); -if (lock_wait) { -rtnl_lock (); - do_unlock = true; - }bnxt_re_init_hwrm_hdr (rdev, (void *)&req,HWRM_STAT_CTX_FREE ,-1 ,-1 ); req.stat_ctx_id =cpu_to_le32 (fw_stats_ctx_id);
@@ -457,8 +429,6 @@ static int bnxt_re_net_stats_ctx_free(struct bnxt_re_dev *rdev,dev_err (rdev_to_dev (rdev),"Failed to free HW stats context %#x" , rc); -if (do_unlock) -rtnl_unlock ();return rc; }
@@ -478,7 +448,6 @@ static int bnxt_re_net_stats_ctx_alloc(struct bnxt_re_dev *rdev,return rc;memset (&fw_msg,0 ,sizeof (fw_msg)); -rtnl_lock ();bnxt_re_init_hwrm_hdr (rdev, (void *)&req,HWRM_STAT_CTX_ALLOC ,-1 ,-1 ); req.update_period_ms =cpu_to_le32 (1000 );
@@ -490,7 +459,6 @@ static int bnxt_re_net_stats_ctx_alloc(struct bnxt_re_dev *rdev,if (!rc) *fw_stats_ctx_id =le32_to_cpu (resp.stat_ctx_id); -rtnl_unlock ();return rc; }
@@ -929,19 +897,19 @@ static int bnxt_re_init_res(struct bnxt_re_dev *rdev)return rc; } -static void bnxt_re_free_nq_res (struct bnxt_re_dev *rdev,bool lock_wait) +static void bnxt_re_free_nq_res (struct bnxt_re_dev *rdev) {int i;for (i =0 ; i < rdev->num_msix -1 ; i++) { -bnxt_re_net_ring_free (rdev, rdev->nq[i].ring_id, lock_wait); +bnxt_re_net_ring_free (rdev, rdev->nq[i].ring_id);bnxt_qplib_free_nq (&rdev->nq[i]); } } -static void bnxt_re_free_res (struct bnxt_re_dev *rdev,bool lock_wait) +static void bnxt_re_free_res (struct bnxt_re_dev *rdev) { -bnxt_re_free_nq_res (rdev, lock_wait); +bnxt_re_free_nq_res (rdev);if (rdev->qplib_res.dpi_tbl.max) { bnxt_qplib_dealloc_dpi(&rdev->qplib_res,
@@ -1219,7 +1187,7 @@ static int bnxt_re_setup_qos(struct bnxt_re_dev *rdev)return 0 ; } -static void bnxt_re_ib_unreg (struct bnxt_re_dev *rdev,bool lock_wait) +static void bnxt_re_ib_unreg (struct bnxt_re_dev *rdev) {int i, rc;
@@ -1234,28 +1202,27 @@ static void bnxt_re_ib_unreg(struct bnxt_re_dev *rdev, bool lock_wait)cancel_delayed_work (&rdev->worker);bnxt_re_cleanup_res (rdev); -bnxt_re_free_res (rdev, lock_wait); +bnxt_re_free_res (rdev);if (test_and_clear_bit (BNXT_RE_FLAG_RCFW_CHANNEL_EN , &rdev->flags)) { rc =bnxt_qplib_deinit_rcfw (&rdev->rcfw);if (rc)dev_warn (rdev_to_dev (rdev),"Failed to deinitialize RCFW: %#x" , rc); -bnxt_re_net_stats_ctx_free (rdev, rdev->qplib_ctx.stats.fw_id, - lock_wait); +bnxt_re_net_stats_ctx_free (rdev, rdev->qplib_ctx.stats.fw_id);bnxt_qplib_free_ctx (rdev->en_dev->pdev, &rdev->qplib_ctx);bnxt_qplib_disable_rcfw_channel (&rdev->rcfw); -bnxt_re_net_ring_free (rdev, rdev->rcfw.creq_ring_id, lock_wait); +bnxt_re_net_ring_free (rdev, rdev->rcfw.creq_ring_id);bnxt_qplib_free_rcfw_channel (&rdev->rcfw); }if (test_and_clear_bit (BNXT_RE_FLAG_GOT_MSIX , &rdev->flags)) { - rc =bnxt_re_free_msix (rdev, lock_wait); + rc =bnxt_re_free_msix (rdev);if (rc)dev_warn (rdev_to_dev (rdev),"Failed to free MSI-X vectors: %#x" , rc); }if (test_and_clear_bit (BNXT_RE_FLAG_NETDEV_REGISTERED , &rdev->flags)) { - rc =bnxt_re_unregister_netdev (rdev, lock_wait); + rc =bnxt_re_unregister_netdev (rdev);if (rc)dev_warn (rdev_to_dev (rdev),"Failed to unregister with netdev: %#x" , rc);
@@ -1276,6 +1243,12 @@ static int bnxt_re_ib_reg(struct bnxt_re_dev *rdev) {int i, j, rc; +bool locked; + +/* Acquire rtnl lock through out this function */ +rtnl_lock (); + locked = true; +/* Registered a new RoCE device instance to netdev */ rc =bnxt_re_register_netdev (rdev);if (rc) {
@@ -1374,12 +1347,16 @@ static int bnxt_re_ib_reg(struct bnxt_re_dev *rdev)schedule_delayed_work (&rdev->worker,msecs_to_jiffies (30000 )); } +rtnl_unlock (); + locked = false; +/* Register ib dev */ rc =bnxt_re_register_ib (rdev);if (rc) {pr_err ("Failed to register with IB: %#x\n" , rc); goto fail; } +set_bit (BNXT_RE_FLAG_IBDEV_REGISTERED , &rdev->flags);dev_info (rdev_to_dev (rdev),"Device registered successfully" );for (i =0 ; i <ARRAY_SIZE (bnxt_re_attributes); i++) { rc = device_create_file(&rdev->ibdev.dev,
@@ -1395,7 +1372,6 @@ static int bnxt_re_ib_reg(struct bnxt_re_dev *rdev) goto fail; } } -set_bit (BNXT_RE_FLAG_IBDEV_REGISTERED , &rdev->flags);ib_get_eth_speed (&rdev->ibdev,1 , &rdev->active_speed, &rdev->active_width);set_bit (BNXT_RE_FLAG_ISSUE_ROCE_STATS , &rdev->flags);
@@ -1404,17 +1380,21 @@ static int bnxt_re_ib_reg(struct bnxt_re_dev *rdev)return 0 ; free_sctx: -bnxt_re_net_stats_ctx_free (rdev, rdev->qplib_ctx.stats.fw_id, true); +bnxt_re_net_stats_ctx_free (rdev, rdev->qplib_ctx.stats.fw_id); free_ctx:bnxt_qplib_free_ctx (rdev->en_dev->pdev, &rdev->qplib_ctx); disable_rcfw:bnxt_qplib_disable_rcfw_channel (&rdev->rcfw); free_ring: -bnxt_re_net_ring_free (rdev, rdev->rcfw.creq_ring_id, true); +bnxt_re_net_ring_free (rdev, rdev->rcfw.creq_ring_id); free_rcfw:bnxt_qplib_free_rcfw_channel (&rdev->rcfw); fail: -bnxt_re_ib_unreg (rdev, true); +if (!locked) +rtnl_lock (); +bnxt_re_ib_unreg (rdev); +rtnl_unlock (); +return rc; }
@@ -1567,7 +1547,7 @@ static int bnxt_re_netdev_event(struct notifier_block *notifier,*/ if (atomic_read (&rdev->sched_count) >0 ) goto exit; -bnxt_re_ib_unreg (rdev, false); +bnxt_re_ib_unreg (rdev);bnxt_re_remove_one (rdev);bnxt_re_dev_unreg (rdev);break ;
@@ -1646,7 +1626,10 @@ static void __exit bnxt_re_mod_exit(void)*/ flush_workqueue (bnxt_re_wq);bnxt_re_dev_stop (rdev); -bnxt_re_ib_unreg (rdev, true); +/* Acquire the rtnl_lock as the L2 resources are freed here */ +rtnl_lock (); +bnxt_re_ib_unreg (rdev); +rtnl_unlock ();bnxt_re_remove_one (rdev);bnxt_re_dev_unreg (rdev); }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 51/58 RISC-V: include linux/ftrace.h in asm-prototypes.h
In reply to Sasha LevinFrom: James Cowgill <[email protected]>
[ Upstream commit 57a489786de9ec37d6e25ef1305dc337047f0236 ]
Building a riscv kernel with CONFIG_FUNCTION_TRACER and CONFIG_MODVERSIONS enabled results in these two warnings:
MODPOST vmlinux.o
WARNING: EXPORT symbol "return_to_handler" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "_mcount" [vmlinux] version generation failed, symbol will not be versioned.
When exporting symbols from an assembly file, the MODVERSIONS code requires their prototypes to be defined in asm-prototypes.h (see scripts/Makefile.build). Since both of these symbols have prototypes defined in linux/ftrace.h, include this header from RISC-V's asm-prototypes.h.
--- arch/riscv/include/asm/asm-prototypes.h | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 arch/riscv/include/asm/asm-prototypes.h
arch/riscv/include/asm/asm-prototypes.h new file +7 -0
@@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_RISCV_PROTOTYPES_H + +#include <linux/ftrace.h> +#include <asm-generic/asm-prototypes.h> + +#endif /* _ASM_RISCV_PROTOTYPES_H */
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 52/58 iommu/rockchip: Free irqs in shutdown handler
In reply to Sasha LevinFrom: Heiko Stuebner <[email protected]>
[ Upstream commit 74bc2abca7603c956d1e331e8b9bee7b874c1eec ]
In the iommu's shutdown handler we disable runtime-pm which could result in the irq-handler running unclocked and since commit
3fc7c5c0cff3 ("iommu/rockchip: Handle errors returned from PM framework")
we warn about that fact.
This can cause warnings on shutdown on some Rockchip machines, so free the irqs in the shutdown handler before we disable runtime-pm.
--- drivers/iommu/rockchip-iommu.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/iommu/rockchip-iommu.c +6 -0
@@ -1242,6 +1242,12 @@ static int rk_iommu_probe(struct platform_device *pdev)static void rk_iommu_shutdown (struct platform_device *pdev) { +struct rk_iommu *iommu =platform_get_drvdata (pdev); +int i =0 , irq; + +while ((irq =platform_get_irq (pdev, i++)) != -ENXIO ) +devm_free_irq (iommu->dev, irq, iommu); +pm_runtime_force_suspend (&pdev->dev); }
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 53/58 pinctrl/amd: poll InterruptEnable bits in amd_gpio_irq_set_type
In reply to Sasha LevinFrom: Daniel Kurtz <[email protected]>
[ Upstream commit b85bfa246efd24ea3fdb5ee949c28e3110c6d299 ]
From the AMD BKDG, if WAKE_INT_MASTER_REG.MaskStsEn is set, a software write to the debounce registers of *any* gpio will block wake/interrupt status generation for *all* gpios for a length of time that depends on WAKE_INT_MASTER_REG.MaskStsLength[11:0]. During this period the Interrupt Delivery bit (INTERRUPT_ENABLE) will read as 0.
In commit 4c1de0414a1340 ("pinctrl/amd: poll InterruptEnable bits in enable_irq") we tried to fix this same "gpio Interrupts are blocked immediately after writing debounce registers" problem, but incorrectly assumed it only affected the gpio whose debounce was being configured and not ALL gpios.
To solve this for all gpios, we move the polling loop from amd_gpio_irq_enable() to amd_gpio_irq_set_type(), while holding the gpio spinlock. This ensures that another gpio operation (e.g. amd_gpio_irq_unmask()) can read a temporarily disabled IRQ and incorrectly disable it while trying to modify some other register bits.
--- drivers/pinctrl/pinctrl-amd.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
drivers/pinctrl/pinctrl-amd.c +23 -10
@@ -348,21 +348,12 @@ static void amd_gpio_irq_enable(struct irq_data *d)unsigned long flags;struct gpio_chip *gc =irq_data_get_irq_chip_data (d);struct amd_gpio *gpio_dev =gpiochip_get_data (gc); -u32 mask =BIT (INTERRUPT_ENABLE_OFF ) |BIT (INTERRUPT_MASK_OFF );raw_spin_lock_irqsave (&gpio_dev->lock, flags); pin_reg =readl (gpio_dev->base + (d->hwirq)*4 ); pin_reg |=BIT (INTERRUPT_ENABLE_OFF ); pin_reg |=BIT (INTERRUPT_MASK_OFF );writel (pin_reg, gpio_dev->base + (d->hwirq)*4 ); -/* -* When debounce logic is enabled it takes ~900 us before interrupts -* can be enabled. During this "debounce warm up" period the -* "INTERRUPT_ENABLE" bit will read as 0. Poll the bit here until it -* reads back as 1, signaling that interrupts are now enabled. -*/ -while ((readl (gpio_dev->base + (d->hwirq)*4 ) & mask) != mask) -continue ;raw_spin_unlock_irqrestore (&gpio_dev->lock, flags); }
@@ -426,7 +417,7 @@ static void amd_gpio_irq_eoi(struct irq_data *d)static int amd_gpio_irq_set_type (struct irq_data *d,unsigned int type) {int ret =0 ; -u32 pin_reg; +u32 pin_reg, pin_reg_irq_en, mask;unsigned long flags, irq_flags;struct gpio_chip *gc =irq_data_get_irq_chip_data (d);struct amd_gpio *gpio_dev =gpiochip_get_data (gc);
@@ -495,6 +486,28 @@ static int amd_gpio_irq_set_type(struct irq_data *d, unsigned int type) } pin_reg |=CLR_INTR_STAT <<INTERRUPT_STS_OFF ; +/* +* If WAKE_INT_MASTER_REG.MaskStsEn is set, a software write to the +* debounce registers of any GPIO will block wake/interrupt status +* generation for *all* GPIOs for a lenght of time that depends on +* WAKE_INT_MASTER_REG.MaskStsLength[11:0]. During this period the +* INTERRUPT_ENABLE bit will read as 0. +* +* We temporarily enable irq for the GPIO whose configuration is +* changing, and then wait for it to read back as 1 to know when +* debounce has settled and then disable the irq again. +* We do this polling with the spinlock held to ensure other GPIO +* access routines do not read an incorrect value for the irq enable +* bit of other GPIOs. We keep the GPIO masked while polling to avoid +* spurious irqs, and disable the irq again after polling. +*/ + mask =BIT (INTERRUPT_ENABLE_OFF ); + pin_reg_irq_en = pin_reg; + pin_reg_irq_en |= mask; + pin_reg_irq_en &= ~BIT (INTERRUPT_MASK_OFF ); +writel (pin_reg_irq_en, gpio_dev->base + (d->hwirq)*4 ); +while ((readl (gpio_dev->base + (d->hwirq)*4 ) & mask) != mask) +continue ;writel (pin_reg, gpio_dev->base + (d->hwirq)*4 );raw_spin_unlock_irqrestore (&gpio_dev->lock, flags);
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 54/58 powerpc/tm: Fix userspace r13 corruption
In reply to Sasha LevinFrom: Michael Neuling <[email protected]>
[ Upstream commit cf13435b730a502e814c63c84d93db131e563f5f ]
When we treclaim we store the userspace checkpointed r13 to a scratch SPR and then later save the scratch SPR to the user thread struct.
Unfortunately, this doesn't work as accessing the user thread struct can take an SLB fault and the SLB fault handler will write the same scratch SPRG that now contains the userspace r13.
To fix this, we store r13 to the kernel stack (which can't fault) before we access the user thread struct.
Found by running P8 guest + powervm + disable_1tb_segments + TM. Seen as a random userspace segfault with r13 looking like a kernel address.
--- arch/powerpc/kernel/tm.S | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
arch/powerpc/kernel/tm.S +9 -2
@@ -175,13 +175,20 @@ _GLOBAL(tm_reclaim) std r1, PACATMSCRATCH(r13) ld r1, PACAR1(r13) - /* Store the PPR in r11 and reset to decent value */ std r11, GPR11(r1) /* Temporary stash */ + /* + * Store r13 away so we can free up the scratch SPR for the SLB fault + * handler (needed once we start accessing the thread_struct). + */ + GET_SCRATCH0(r11) + std r11, GPR13(r1) + /* Reset MSR RI so we can take SLB faults again */ li r11, MSR_RI mtmsrd r11, 1 + /* Store the PPR in r11 and reset to decent value */ mfspr r11, SPRN_PPR HMT_MEDIUM
@@ -210,7 +217,7 @@ _GLOBAL(tm_reclaim) ld r4, GPR7(r1) /* user r7 */ ld r5, GPR11(r1) /* user r11 */ ld r6, GPR12(r1) /* user r12 */ - GET_SCRATCH0(8) /* user r13 */ + ld r8, GPR13(r1) /* user r13 */ std r3, GPR1(r7) std r4, GPR7(r7) std r5, GPR11(r7)
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 55/58 powerpc/tm: Avoid possible userspace r1 corruption on reclaim
In reply to Sasha LevinFrom: Michael Neuling <[email protected]>
[ Upstream commit 96dc89d526ef77604376f06220e3d2931a0bfd58 ]
Current we store the userspace r1 to PACATMSCRATCH before finally saving it to the thread struct.
In theory an exception could be taken here (like a machine check or SLB miss) that could write PACATMSCRATCH and hence corrupt the userspace r1. The SLB fault currently doesn't touch PACATMSCRATCH, but others do.
We've never actually seen this happen but it's theoretically possible. Either way, the code is fragile as it is.
This patch saves r1 to the kernel stack (which can't fault) before we turn MSR[RI] back on. PACATMSCRATCH is still used but only with MSR[RI] off. We then copy r1 from the kernel stack to the thread struct once we have MSR[RI] back on.
--- arch/powerpc/kernel/tm.S | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
arch/powerpc/kernel/tm.S +8 -1
@@ -177,6 +177,13 @@ _GLOBAL(tm_reclaim) std r11, GPR11(r1) /* Temporary stash */ + /* + * Move the saved user r1 to the kernel stack in case PACATMSCRATCH is + * clobbered by an exception once we turn on MSR_RI below. + */ + ld r11, PACATMSCRATCH(r13) + std r11, GPR1(r1) + /* * Store r13 away so we can free up the scratch SPR for the SLB fault * handler (needed once we start accessing the thread_struct).
@@ -213,7 +220,7 @@ _GLOBAL(tm_reclaim) SAVE_GPR(8, r7) /* user r8 */ SAVE_GPR(9, r7) /* user r9 */ SAVE_GPR(10, r7) /* user r10 */ - ld r3, PACATMSCRATCH(r13) /* user r1 */ + ld r3, GPR1(r1) /* user r1 */ ld r4, GPR7(r1) /* user r7 */ ld r5, GPR11(r1) /* user r11 */ ld r6, GPR12(r1) /* user r12 */
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 56/58 powerpc/numa: Use associativity if VPHN hcall is successful
In reply to Sasha LevinFrom: Srikar Dronamraju <[email protected]>
[ Upstream commit 2483ef056f6e42f61cd266452e2841165dfe1b5c ]
Currently associativity is used to lookup node-id even if the preceding VPHN hcall failed. However this can cause CPU to be made part of the wrong node, (most likely to be node 0). This is because VPHN is not enabled on KVM guests.
With 2ea6263 ("powerpc/topology: Get topology for shared processors at boot"), associativity is used to set to the wrong node. Hence KVM guest topology is broken.
For example : A 4 node KVM guest before would have reported.
[root@localhost ~]# numactl -H
available: 4 nodes (0-3)
node 0 cpus: 0 1 2 3
node 0 size: 1746 MB
node 0 free: 1604 MB
node 1 cpus: 4 5 6 7
node 1 size: 2044 MB
node 1 free: 1765 MB
node 2 cpus: 8 9 10 11
node 2 size: 2044 MB
node 2 free: 1837 MB
node 3 cpus: 12 13 14 15
node 3 size: 2044 MB
node 3 free: 1903 MB
node distances:
node 0 1 2 3
0: 10 40 40 40
1: 40 10 40 40
2: 40 40 10 40
3: 40 40 40 10Would now report:
[root@localhost ~]# numactl -H
available: 4 nodes (0-3)
node 0 cpus: 0 2 3 4 5 6 7 8 9 10 11 12 13 14 15 node 0 size: 1746 MB
node 0 free: 1244 MB
node 1 cpus:
node 1 size: 2044 MB
node 1 free: 2032 MB
node 2 cpus: 1
node 2 size: 2044 MB
node 2 free: 2028 MB
node 3 cpus:
node 3 size: 2044 MB
node 3 free: 2032 MB
node distances:
node 0 1 2 3
0: 10 40 40 40
1: 40 10 40 40
2: 40 40 10 40
3: 40 40 40 10Fix this by skipping associativity lookup if the VPHN hcall failed.
--- arch/powerpc/mm/numa.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
arch/powerpc/mm/numa.c +3 -1
@@ -1204,7 +1204,9 @@ int find_and_online_cpu_nid(int cpu)int new_nid;/* Use associativity from first thread for all siblings */ -vphn_get_associativity (cpu, associativity); +if (vphn_get_associativity (cpu, associativity)) +return cpu_to_node (cpu); + new_nid =associativity_to_nid (associativity);if (new_nid <0 || !node_possible (new_nid)) new_nid = first_online_node;
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 57/58 iommu/amd: Return devid as alias for ACPI HID devices
In reply to Sasha LevinFrom: Arindam Nath <[email protected]>
[ Upstream commit 5ebb1bc2d63d90dd204169e21fd7a0b4bb8c776e ]
ACPI HID devices do not actually have an alias for them in the IVRS. But dev_data->alias is still used for indexing into the IOMMU device table for devices being handled by the IOMMU. So for ACPI HID devices, we simply return the corresponding devid as an alias, as parsed from IVRS table.
--- drivers/iommu/amd_iommu.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/iommu/amd_iommu.c +6 -0
@@ -246,7 +246,13 @@ static u16 get_alias(struct device *dev)/* The callers make sure that get_device_id() does not fail here */ devid =get_device_id (dev); + +/* For ACPI HID devices, we simply return the devid as such */ +if (!dev_is_pci (dev)) +return devid; + ivrs_alias = amd_iommu_alias_table[devid]; +pci_for_each_dma_alias (pdev, __last_alias, &pci_alias);if (ivrs_alias == pci_alias)
Signature · 1 line
2.17.1
Sasha Levin
maintainerPATCH 58/58 x86/boot: Fix kexec booting failure in the SEV bit detection code
In reply to Sasha LevinFrom: Kairui Song <[email protected]>
[ Upstream commit bdec8d7fa55e6f5314ed72e5a0b435d90ff90548 ]
Commit
1958b5fc4010 ("x86/boot: Add early boot support when running with SEV active")
can occasionally cause system resets when kexec-ing a second kernel even if SEV is not active.
That's because get_sev_encryption_bit() uses 32-bit rIP-relative addressing to read the value of enc_bit - a variable which caches a previously detected encryption bit position - but kexec may allocate the early boot code to a higher location, beyond the 32-bit addressing limit.
In this case, garbage will be read and get_sev_encryption_bit() will return the wrong value, leading to accessing memory with the wrong encryption setting.
Therefore, remove enc_bit, and thus get rid of the need to do 32-bit rIP-relative addressing in the first place.
[ bp: massage commit message heavily. ]
--- arch/x86/boot/compressed/mem_encrypt.S | 19 ------------------- 1 file changed, 19 deletions(-)
arch/x86/boot/compressed/mem_encrypt.S +0 -19
@@ -25,20 +25,6 @@ ENTRY(get_sev_encryption_bit) push %ebx push %ecx push %edx - push %edi - - /* - * RIP-relative addressing is needed to access the encryption bit - * variable. Since we are running in 32-bit mode we need this call/pop - * sequence to get the proper relative addressing. - */ - call 1f -1: popl %edi - subl $1b, %edi - - movl enc_bit(%edi), %eax - cmpl $0, %eax - jge .Lsev_exit /* Check if running under a hypervisor */ movl $1, %eax
@@ -69,15 +55,12 @@ ENTRY(get_sev_encryption_bit) movl %ebx, %eax andl $0x3f, %eax /* Return the encryption bit location */ - movl %eax, enc_bit(%edi) jmp .Lsev_exit .Lno_sev: xor %eax, %eax - movl %eax, enc_bit(%edi) .Lsev_exit: - pop %edi pop %edx pop %ecx pop %ebx
@@ -113,8 +96,6 @@ ENTRY(set_sev_encryption_mask) ENDPROC(set_sev_encryption_mask) .data -enc_bit: - .int 0xffffffff #ifdef CONFIG_AMD_MEM_ENCRYPT .balign 8
Signature · 1 line
2.17.1