5.16.13-rc1 review
Subsystems (4)
Greg Kroah-Hartman
maintainerPATCH 150/186 iavf: Rework mutexes for better synchronisation
In reply to Greg Kroah-HartmanFrom: Slawomir Laba <[email protected]>
[ Upstream commit fc2e6b3b132a907378f6af08356b105a4139c4fb ]
The driver used to crash in multiple spots when put to stress testing of the init, reset and remove paths.
The user would experience call traces or hangs when creating, resetting, removing VFs. Depending on the machines, the call traces are happening in random spots, like reset restoring resources racing with driver remove.
Make adapter->crit_lock mutex a mandatory lock for guarding the operations performed on all workqueues and functions dealing with resource allocation and disposal.
Make __IAVF_REMOVE a final state of the driver respected by workqueues that shall not requeue, when they fail to obtain the crit_lock.
Make the IRQ handler not to queue the new work for adminq_task when the __IAVF_REMOVE state is set.
--- drivers/net/ethernet/intel/iavf/iavf.h | 1 - drivers/net/ethernet/intel/iavf/iavf_main.c | 66 +++++++++++---------- 2 files changed, 36 insertions(+), 31 deletions(-)
drivers/net/ethernet/intel/iavf/iavf.h +0 -1
@@ -233,7 +233,6 @@ struct iavf_adapter {struct list_head mac_filter_list;struct mutex crit_lock;struct mutex client_lock; -struct mutex remove_lock;/* Lock to protect accesses to MAC and VLAN lists */ spinlock_t mac_vlan_list_lock;char misc_vector_name[IFNAMSIZ +9 ];
drivers/net/ethernet/intel/iavf/iavf_main.c +36 -30
149 lines not on this page · raw message
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 151/186 iavf: Add waiting so the port is initialized in remove
In reply to Greg Kroah-HartmanFrom: Slawomir Laba <[email protected]>
[ Upstream commit 974578017fc1fdd06cea8afb9dfa32602e8529ed ]
There exist races when port is being configured and remove is triggered.
unregister_netdev is not and can't be called under crit_lock mutex since it is calling ndo_stop -> iavf_close which requires this lock. Depending on init state the netdev could be still unregistered so unregister_netdev never cleans up, when shortly after that the device could become registered.
Make iavf_remove wait until port finishes initialization. All critical state changes are atomic (under crit_lock). Crashes that come from iavf_reset_interrupt_capability and iavf_free_traffic_irqs should now be solved in a graceful manner.
--- drivers/net/ethernet/intel/iavf/iavf_main.c | 27 ++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-)
drivers/net/ethernet/intel/iavf/iavf_main.c +16 -11
@@ -3981,7 +3981,6 @@ static int __maybe_unused iavf_resume(struct device *dev_d)static void iavf_remove (struct pci_dev *pdev) {struct iavf_adapter *adapter =iavf_pdev_to_adapter (pdev); -enum iavf_state_t prev_state = adapter->last_state;struct net_device *netdev = adapter->netdev;struct iavf_fdir_fltr *fdir, *fdirtmp;struct iavf_vlan_filter *vlf, *vlftmp;
@@ -3991,6 +3990,22 @@ static void iavf_remove(struct pci_dev *pdev)struct iavf_hw *hw = &adapter->hw;int err; +/* Wait until port initialization is complete. +* There are flows where register/unregister netdev may race. +*/ +while (1 ) { +mutex_lock (&adapter->crit_lock); +if (adapter->state == __IAVF_RUNNING || + adapter->state == __IAVF_DOWN) { +mutex_unlock (&adapter->crit_lock); +break ; + } + +mutex_unlock (&adapter->crit_lock); +usleep_range (500 ,1000 ); + } +cancel_delayed_work_sync (&adapter->watchdog_task); +if (adapter->netdev_registered) {unregister_netdev (netdev); adapter->netdev_registered = false;
@@ -4028,16 +4043,6 @@ static void iavf_remove(struct pci_dev *pdev)iavf_free_all_rx_resources (adapter);iavf_free_misc_irq (adapter); -/* In case we enter iavf_remove from erroneous state, free traffic irqs -* here, so as to not cause a kernel crash, when calling -* iavf_reset_interrupt_capability. -*/ -if ((adapter->last_state == __IAVF_RESETTING && - prev_state != __IAVF_DOWN) || - (adapter->last_state == __IAVF_RUNNING && - !(netdev->flags &IFF_UP ))) -iavf_free_traffic_irqs (adapter); -iavf_reset_interrupt_capability (adapter);iavf_free_q_vectors (adapter);
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 152/186 iavf: Fix init state closure on remove
In reply to Greg Kroah-HartmanFrom: Slawomir Laba <[email protected]>
[ Upstream commit 3ccd54ef44ebfa0792c5441b6d9c86618f3378d1 ]
When init states of the adapter work, the errors like lack of communication with the PF might hop in. If such events occur the driver restores previous states in order to retry initialization in a proper way. When remove task kicks in, this situation could lead to races with unregistering the netdevice as well as resources cleanup. With the commit introducing the waiting in remove for init to complete, this problem turns into an endless waiting if init never recovers from errors.
Introduce __IAVF_IN_REMOVE_TASK bit to indicate that the remove thread has started.
Make __IAVF_COMM_FAILED adapter state respect the __IAVF_IN_REMOVE_TASK bit and set the __IAVF_INIT_FAILED state and return without any action instead of trying to recover.
Make __IAVF_INIT_FAILED adapter state respect the __IAVF_IN_REMOVE_TASK bit and return without any further actions.
Make the loop in the remove handler break when adapter has __IAVF_INIT_FAILED state set.
--- drivers/net/ethernet/intel/iavf/iavf.h | 4 ++++ drivers/net/ethernet/intel/iavf/iavf_main.c | 24 ++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-)
drivers/net/ethernet/intel/iavf/iavf.h +4 -0
@@ -188,6 +188,10 @@ enum iavf_state_t { __IAVF_RUNNING, /* opened, working */ }; +enum iavf_critical_section_t { + __IAVF_IN_REMOVE_TASK,/* device being removed */ +}; +#define IAVF_CLOUD_FIELD_OMAC 0x01#define IAVF_CLOUD_FIELD_IMAC 0x02#define IAVF_CLOUD_FIELD_IVLAN 0x04
drivers/net/ethernet/intel/iavf/iavf_main.c +23 -1
@@ -2019,6 +2019,15 @@ static void iavf_watchdog_task(struct work_struct *work) msecs_to_jiffies(1));return ;case __IAVF_INIT_FAILED: +if (test_bit (__IAVF_IN_REMOVE_TASK, + &adapter->crit_section)) { +/* Do not update the state and do not reschedule +* watchdog task, iavf_remove should handle this state +* as it can loop forever +*/ +mutex_unlock (&adapter->crit_lock); +return ; + }if (++adapter->aq_wait_count >IAVF_AQ_MAX_ERR ) {dev_err (&adapter->pdev->dev,"Failed to communicate with PF; waiting before retry\n" );
@@ -2035,6 +2044,17 @@ static void iavf_watchdog_task(struct work_struct *work)queue_delayed_work (iavf_wq, &adapter->watchdog_task,HZ );return ;case __IAVF_COMM_FAILED: +if (test_bit (__IAVF_IN_REMOVE_TASK, + &adapter->crit_section)) { +/* Set state to __IAVF_INIT_FAILED and perform remove +* steps. Remove IAVF_FLAG_PF_COMMS_FAILED so the task +* doesn't bring the state back to __IAVF_COMM_FAILED. +*/ +iavf_change_state (adapter, __IAVF_INIT_FAILED); + adapter->flags &= ~IAVF_FLAG_PF_COMMS_FAILED ; +mutex_unlock (&adapter->crit_lock); +return ; + } reg_val =rd32 (hw,IAVF_VFGEN_RSTAT ) &IAVF_VFGEN_RSTAT_VFR_STATE_MASK ; if (reg_val == VIRTCHNL_VFR_VFACTIVE ||
@@ -3990,13 +4010,15 @@ static void iavf_remove(struct pci_dev *pdev)struct iavf_hw *hw = &adapter->hw;int err; +set_bit (__IAVF_IN_REMOVE_TASK, &adapter->crit_section);/* Wait until port initialization is complete. * There are flows where register/unregister netdev may race. */ while (1 ) {mutex_lock (&adapter->crit_lock);if (adapter->state == __IAVF_RUNNING || - adapter->state == __IAVF_DOWN) { + adapter->state == __IAVF_DOWN || + adapter->state == __IAVF_INIT_FAILED) {mutex_unlock (&adapter->crit_lock);break ; }
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 153/186 iavf: Fix locking for VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS
In reply to Greg Kroah-HartmanFrom: Slawomir Laba <[email protected]>
[ Upstream commit 0579fafd37fb7efe091f0e6c8ccf968864f40f3e ]
iavf_virtchnl_completion is called under crit_lock but when the code for VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS is called, this lock is released in order to obtain rtnl_lock to avoid ABBA deadlock with unregister_netdev.
Along with the new way iavf_remove behaves, there exist many risks related to the lock release and attmepts to regrab it. The driver faces crashes related to races between unregister_netdev and netdev_update_features. Yet another risk is that the driver could already obtain the crit_lock in order to destroy it and iavf_virtchnl_completion could crash or block forever.
Make iavf_virtchnl_completion never relock crit_lock in it's call paths.
Extract rtnl_lock locking logic to the driver for unregister_netdev in order to set the netdev_registered flag inside the lock.
Introduce a new flag that will inform adminq_task to perform the code from VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS right after it finishes processing messages. Guard this code with remove flags so it's never called when the driver is in remove state.
--- drivers/net/ethernet/intel/iavf/iavf.h | 1 + drivers/net/ethernet/intel/iavf/iavf_main.c | 16 +++++++++++++++- drivers/net/ethernet/intel/iavf/iavf_virtchnl.c | 14 +------------- 3 files changed, 17 insertions(+), 14 deletions(-)
drivers/net/ethernet/intel/iavf/iavf.h +1 -0
@@ -274,6 +274,7 @@ struct iavf_adapter {#define IAVF_FLAG_LEGACY_RX BIT(15)#define IAVF_FLAG_REINIT_ITR_NEEDED BIT(16)#define IAVF_FLAG_QUEUES_DISABLED BIT(17) +#define IAVF_FLAG_SETUP_NETDEV_FEATURES BIT(18)/* duplicates for common code */ #define IAVF_FLAG_DCB_ENABLED 0/* flags for admin queue service task */
drivers/net/ethernet/intel/iavf/iavf_main.c +15 -1
@@ -2465,6 +2465,18 @@ static void iavf_adminq_task(struct work_struct *work) }while (pending);mutex_unlock (&adapter->crit_lock); +if ((adapter->flags &IAVF_FLAG_SETUP_NETDEV_FEATURES )) { +if (adapter->netdev_registered || + !test_bit (__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) { +struct net_device *netdev = adapter->netdev; + +rtnl_lock (); +netdev_update_features (netdev); +rtnl_unlock (); + } + + adapter->flags &= ~IAVF_FLAG_SETUP_NETDEV_FEATURES ; + }if ((adapter->flags & (IAVF_FLAG_RESET_PENDING |IAVF_FLAG_RESET_NEEDED )) || adapter->state == __IAVF_RESETTING)
@@ -4029,8 +4041,10 @@ static void iavf_remove(struct pci_dev *pdev)cancel_delayed_work_sync (&adapter->watchdog_task);if (adapter->netdev_registered) { -unregister_netdev (netdev); +rtnl_lock (); +unregister_netdevice (netdev); adapter->netdev_registered = false; +rtnl_unlock (); }if (CLIENT_ALLOWED (adapter)) { err =iavf_lan_del_device (adapter);
drivers/net/ethernet/intel/iavf/iavf_virtchnl.c +1 -13
@@ -1752,19 +1752,7 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,spin_unlock_bh (&adapter->mac_vlan_list_lock);iavf_process_config (adapter); - -/* unlock crit_lock before acquiring rtnl_lock as other -* processes holding rtnl_lock could be waiting for the same -* crit_lock -*/ -mutex_unlock (&adapter->crit_lock); -rtnl_lock (); -netdev_update_features (adapter->netdev); -rtnl_unlock (); -if (iavf_lock_timeout (&adapter->crit_lock,10000 )) -dev_warn (&adapter->pdev->dev,"failed to acquire crit_lock in %s\n" , - __FUNCTION__); - + adapter->flags |=IAVF_FLAG_SETUP_NETDEV_FEATURES ; }break ;case VIRTCHNL_OP_ENABLE_QUEUES :
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 154/186 iavf: Fix race in init state
In reply to Greg Kroah-HartmanFrom: Slawomir Laba <[email protected]>
[ Upstream commit a472eb5cbaebb5774672c565e024336c039e9128 ]
When iavf_init_version_check sends VIRTCHNL_OP_GET_VF_RESOURCES message, the driver will wait for the response after requeueing the watchdog task in iavf_init_get_resources call stack. The logic is implemented this way that iavf_init_get_resources has to be called in order to allocate adapter->vf_res. It is polling for the AQ response in iavf_get_vf_config function. Expect a call trace from kernel when adminq_task worker handles this message first. adapter->vf_res will be NULL in iavf_virtchnl_completion.
Make the watchdog task not queue the adminq_task if the init process is not finished yet.
--- drivers/net/ethernet/intel/iavf/iavf_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/ethernet/intel/iavf/iavf_main.c +2 -1
@@ -2124,7 +2124,8 @@ static void iavf_watchdog_task(struct work_struct *work)schedule_delayed_work (&adapter->client_task,msecs_to_jiffies (5 ));mutex_unlock (&adapter->crit_lock); restart_watchdog: -queue_work (iavf_wq, &adapter->adminq_task); +if (adapter->state >= __IAVF_DOWN) +queue_work (iavf_wq, &adapter->adminq_task);if (adapter->aq_required)queue_delayed_work (iavf_wq, &adapter->watchdog_task,msecs_to_jiffies (20 ));
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 155/186 iavf: Fix __IAVF_RESETTING state usage
In reply to Greg Kroah-HartmanFrom: Slawomir Laba <[email protected]>
[ Upstream commit 14756b2ae265d526b8356e86729090b01778fdf6 ]
The setup of __IAVF_RESETTING state in watchdog task had no effect and could lead to slow resets in the driver as the task for __IAVF_RESETTING state only requeues watchdog. Till now the __IAVF_RESETTING was interpreted by reset task as running state which could lead to errors with allocating and resources disposal.
Make watchdog_task queue the reset task when it's necessary. Do not update the state to __IAVF_RESETTING so the reset task knows exactly what is the current state of the adapter.
--- drivers/net/ethernet/intel/iavf/iavf_main.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/net/ethernet/intel/iavf/iavf_main.c +6 -7
@@ -1073,8 +1073,7 @@ void iavf_down(struct iavf_adapter *adapter) rss->state =IAVF_ADV_RSS_DEL_REQUEST ;spin_unlock_bh (&adapter->adv_rss_lock); -if (!(adapter->flags &IAVF_FLAG_PF_COMMS_FAILED ) && - adapter->state != __IAVF_RESETTING) { +if (!(adapter->flags &IAVF_FLAG_PF_COMMS_FAILED )) {/* cancel any current operation */ adapter->current_op =VIRTCHNL_OP_UNKNOWN ;/* Schedule operations to close down the HW. Don't wait
@@ -1992,11 +1991,12 @@ static void iavf_watchdog_task(struct work_struct *work)if (adapter->flags &IAVF_FLAG_PF_COMMS_FAILED )iavf_change_state (adapter, __IAVF_COMM_FAILED); -if (adapter->flags &IAVF_FLAG_RESET_NEEDED && - adapter->state != __IAVF_RESETTING) { -iavf_change_state (adapter, __IAVF_RESETTING); +if (adapter->flags &IAVF_FLAG_RESET_NEEDED ) { adapter->aq_required =0 ; adapter->current_op =VIRTCHNL_OP_UNKNOWN ; +mutex_unlock (&adapter->crit_lock); +queue_work (iavf_wq, &adapter->reset_task); +return ; }switch (adapter->state) {
@@ -2289,8 +2289,7 @@ static void iavf_reset_task(struct work_struct *work)* ndo_open() returning, so we can't assume it means all our open * tasks have finished, since we're not holding the rtnl_lock here. */ - running = ((adapter->state == __IAVF_RUNNING) || - (adapter->state == __IAVF_RESETTING)); + running = adapter->state == __IAVF_RUNNING;if (running) { netdev->flags &= ~IFF_UP ;
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 156/186 drm/i915/guc/slpc: Correct the param count for unset param
In reply to Greg Kroah-HartmanFrom: Vinay Belgaumkar <[email protected]>
[ Upstream commit 1b279f6ad467535c3b8a66b4edefaca2cdd5bdc3 ]
SLPC unset param H2G only needs one parameter - the id of the param.
--- drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c +1 -1
@@ -133,7 +133,7 @@ static int guc_action_slpc_unset_param(struct intel_guc *guc, u8 id) {u32 request[] = {GUC_ACTION_HOST2GUC_PC_SLPC_REQUEST , -SLPC_EVENT (SLPC_EVENT_PARAMETER_UNSET ,2 ), +SLPC_EVENT (SLPC_EVENT_PARAMETER_UNSET ,1 ), id, };
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 157/186 drm/bridge: ti-sn65dsi86: Properly undo autosuspend
In reply to Greg Kroah-HartmanFrom: Douglas Anderson <[email protected]>
[ Upstream commit 26d3474348293dc752c55fe6d41282199f73714c ]
The PM Runtime docs say:
Drivers in ->remove() callback should undo the runtime PM changes done in ->probe(). Usually this means calling pm_runtime_disable(), pm_runtime_dont_use_autosuspend() etc.
We weren't doing that for autosuspend. Let's do it.
--- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/ti-sn65dsi86.c +3 -2
@@ -1474,6 +1474,7 @@ static inline void ti_sn_gpio_unregister(void) {}static void ti_sn65dsi86_runtime_disable (void *data) { +pm_runtime_dont_use_autosuspend (data);pm_runtime_disable (data); }
@@ -1533,11 +1534,11 @@ static int ti_sn65dsi86_probe(struct i2c_client *client, "failed to get reference clock\n");pm_runtime_enable (dev); +pm_runtime_set_autosuspend_delay (pdata->dev,500 ); +pm_runtime_use_autosuspend (pdata->dev); ret =devm_add_action_or_reset (dev, ti_sn65dsi86_runtime_disable, dev);if (ret)return ret; -pm_runtime_set_autosuspend_delay (pdata->dev,500 ); -pm_runtime_use_autosuspend (pdata->dev);ti_sn65dsi86_debugfs_init (pdata);
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 158/186 e1000e: Fix possible HW unit hang after an s0ix exit
In reply to Greg Kroah-HartmanFrom: Sasha Neftin <[email protected]>
[ Upstream commit 1866aa0d0d6492bc2f8d22d0df49abaccf50cddd ]
Disable the OEM bit/Gig Disable/restart AN impact and disable the PHY LAN connected device (LCD) reset during power management flows. This fixes possible HW unit hangs on the s0ix exit on some corporate ADL platforms.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=214821
--- drivers/net/ethernet/intel/e1000e/hw.h | 1 + drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++++ drivers/net/ethernet/intel/e1000e/ich8lan.h | 1 + drivers/net/ethernet/intel/e1000e/netdev.c | 26 +++++++++++++++++++++ 4 files changed, 32 insertions(+)
drivers/net/ethernet/intel/e1000e/hw.h +1 -0
@@ -630,6 +630,7 @@ struct e1000_phy_info {bool disable_polarity_correction;bool is_mdix;bool polarity_correction; +bool reset_disable;bool speed_downgraded;bool autoneg_wait_to_complete; };
drivers/net/ethernet/intel/e1000e/ich8lan.c +4 -0
@@ -2050,6 +2050,10 @@ static s32 e1000_check_reset_block_ich8lan(struct e1000_hw *hw)bool blocked = false;int i =0 ; +/* Check the PHY (LCD) reset flag */ +if (hw->phy.reset_disable) +return true; +while ((blocked = !(er32 (FWSM ) &E1000_ICH_FWSM_RSPCIPHY )) && (i++ <30 ))usleep_range (10000 ,11000 );
drivers/net/ethernet/intel/e1000e/ich8lan.h +1 -0
@@ -271,6 +271,7 @@#define I217_CGFREG_ENABLE_MTA_RESET 0x0002#define I217_MEMPWR PHY_REG(772, 26)#define I217_MEMPWR_DISABLE_SMB_RELEASE 0x0010 +#define I217_MEMPWR_MOEM 0x1000/* Receive Address Initial CRC Calculation */ #define E1000_PCH_RAICC (_n) (0x05F50 + ((_n) * 4))
drivers/net/ethernet/intel/e1000e/netdev.c +26 -0
@@ -6991,8 +6991,21 @@ static __maybe_unused int e1000e_pm_suspend(struct device *dev)struct net_device *netdev =pci_get_drvdata (to_pci_dev (dev));struct e1000_adapter *adapter =netdev_priv (netdev);struct pci_dev *pdev =to_pci_dev (dev); +struct e1000_hw *hw = &adapter->hw; +u16 phy_data;int rc; +if (er32 (FWSM ) &E1000_ICH_FWSM_FW_VALID && + hw->mac.type >= e1000_pch_adp) { +/* Mask OEM Bits / Gig Disable / Restart AN (772_26[12] = 1) */ +e1e_rphy (hw,I217_MEMPWR , &phy_data); + phy_data |=I217_MEMPWR_MOEM ; +e1e_wphy (hw,I217_MEMPWR , phy_data); + +/* Disable LCD reset */ + hw->phy.reset_disable = true; + } +e1000e_flush_lpic (pdev);e1000e_pm_freeze (dev);
@@ -7014,6 +7027,8 @@ static __maybe_unused int e1000e_pm_resume(struct device *dev)struct net_device *netdev =pci_get_drvdata (to_pci_dev (dev));struct e1000_adapter *adapter =netdev_priv (netdev);struct pci_dev *pdev =to_pci_dev (dev); +struct e1000_hw *hw = &adapter->hw; +u16 phy_data;int rc;/* Introduce S0ix implementation */
@@ -7024,6 +7039,17 @@ static __maybe_unused int e1000e_pm_resume(struct device *dev)if (rc)return rc; +if (er32 (FWSM ) &E1000_ICH_FWSM_FW_VALID && + hw->mac.type >= e1000_pch_adp) { +/* Unmask OEM Bits / Gig Disable / Restart AN 772_26[12] = 0 */ +e1e_rphy (hw,I217_MEMPWR , &phy_data); + phy_data &= ~I217_MEMPWR_MOEM ; +e1e_wphy (hw,I217_MEMPWR , phy_data); + +/* Enable LCD reset */ + hw->phy.reset_disable = false; + } +return e1000e_pm_thaw (dev); }
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 159/186 MIPS: ralink: mt7621: use bitwise NOT instead of logical
In reply to Greg Kroah-HartmanFrom: Ilya Lipnitskiy <[email protected]>
[ Upstream commit 5d8965704fe5662e2e4a7e4424a2cbe53e182670 ]
It was the intention to reverse the bits, not make them all zero by using logical NOT operator.
--- arch/mips/ralink/mt7621.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/ralink/mt7621.c +1 -1
@@ -38,7 +38,7 @@ static bool __init mt7621_addr_wraparound_test(phys_addr_t size)__raw_writel (MT7621_MEM_TEST_PATTERN , dm);if (__raw_readl (dm) !=__raw_readl (dm + size))return false; -__raw_writel (!MT7621_MEM_TEST_PATTERN , dm); +__raw_writel (~MT7621_MEM_TEST_PATTERN , dm);return __raw_readl (dm) ==__raw_readl (dm + size); }
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 160/186 nl80211: Handle nla_memdup failures in handle_nan_filter
In reply to Greg Kroah-HartmanFrom: Jiasheng Jiang <[email protected]>
[ Upstream commit 6ad27f522cb3b210476daf63ce6ddb6568c0508b ]
As there's potential for failure of the nla_memdup(), check the return value.
--- net/wireless/nl80211.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
net/wireless/nl80211.c +12 -0
@@ -13379,6 +13379,9 @@ static int handle_nan_filter(struct nlattr *attr_filter, i =0 ;nla_for_each_nested (attr, attr_filter, rem) { filter[i].filter =nla_memdup (attr,GFP_KERNEL ); +if (!filter[i].filter) + goto err; + filter[i].len =nla_len (attr); i++; }
@@ -13391,6 +13394,15 @@ static int handle_nan_filter(struct nlattr *attr_filter, }return 0 ; + +err: + i =0 ; +nla_for_each_nested (attr, attr_filter, rem) { +kfree (filter[i].filter); + i++; + } +kfree (filter); +return -ENOMEM ; } static int nl80211_nan_add_func(struct sk_buff *skb,
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 161/186 ptp: ocp: Add ptp_ocp_adjtime_coarse for large adjustments
In reply to Greg Kroah-HartmanFrom: Jonathan Lemon <[email protected]>
[ Upstream commit 90f8f4c0e3cebd541deaa45cf0e470bb9810dd4f ]
In ("ptp: ocp: Have FPGA fold in ns adjustment for adjtime."), the ns adjustment was written to the FPGA register, so the clock could accurately perform adjustments.
However, the adjtime() call passes in a s64, while the clock adjustment registers use a s32. When trying to perform adjustments with a large value (37 sec), things fail.
Examine the incoming delta, and if larger than 1 sec, use the original (coarse) adjustment method. If smaller than 1 sec, then allow the FPGA to fold in the changes over a 1 second window.
--- drivers/ptp/ptp_ocp.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
drivers/ptp/ptp_ocp.c +23 -2
@@ -607,7 +607,7 @@ ptp_ocp_settime(struct ptp_clock_info *ptp_info, const struct timespec64 *ts) }static void -__ptp_ocp_adjtime_locked (struct ptp_ocp *bp,u64 adj_val) +__ptp_ocp_adjtime_locked (struct ptp_ocp *bp,u32 adj_val) {u32 select, ctrl;
@@ -615,7 +615,7 @@ __ptp_ocp_adjtime_locked(struct ptp_ocp *bp, u64 adj_val)iowrite32 (OCP_SELECT_CLK_REG , &bp->reg->select);iowrite32 (adj_val, &bp->reg->offset_ns); -iowrite32 (adj_val &0x7f , &bp->reg->offset_window_ns); +iowrite32 (NSEC_PER_SEC , &bp->reg->offset_window_ns); ctrl =OCP_CTRL_ADJUST_OFFSET |OCP_CTRL_ENABLE ;iowrite32 (ctrl, &bp->reg->ctrl);
@@ -624,6 +624,22 @@ __ptp_ocp_adjtime_locked(struct ptp_ocp *bp, u64 adj_val)iowrite32 (select >>16 , &bp->reg->select); } +static void +ptp_ocp_adjtime_coarse (struct ptp_ocp *bp,u64 delta_ns) +{ +struct timespec64 ts; +unsigned long flags; +int err; + +spin_lock_irqsave (&bp->lock, flags); + err =__ptp_ocp_gettime_locked (bp, &ts,NULL ); +if (likely (!err)) { +timespec64_add_ns (&ts, delta_ns); +__ptp_ocp_settime_locked (bp, &ts); + } +spin_unlock_irqrestore (&bp->lock, flags); +} +static int ptp_ocp_adjtime (struct ptp_clock_info *ptp_info,s64 delta_ns) {
@@ -631,6 +647,11 @@ ptp_ocp_adjtime(struct ptp_clock_info *ptp_info, s64 delta_ns)unsigned long flags;u32 adj_ns, sign; +if (delta_ns >NSEC_PER_SEC || -delta_ns >NSEC_PER_SEC ) { +ptp_ocp_adjtime_coarse (bp, delta_ns); +return 0 ; + } + sign = delta_ns <0 ?BIT (31 ) :0 ; adj_ns = sign ? -delta_ns : delta_ns;
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 162/186 drm/amdgpu: fix suspend/resume hang regression
In reply to Greg Kroah-HartmanFrom: Qiang Yu <[email protected]>
[ Upstream commit f1ef17011c765495c876fa75435e59eecfdc1ee4 ]
Regression has been reported that suspend/resume may hang with the previous vm ready check commit.
So bring back the evicted list check as a temp fix.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +2 -1
@@ -777,7 +777,8 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)amdgpu_vm_eviction_lock (vm); ret = !vm->evicting;amdgpu_vm_eviction_unlock (vm); -return ret; + +return ret &&list_empty (&vm->evicted); }/**
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 163/186 net: dcb: disable softirqs in dcbnl_flush_dev()
In reply to Greg Kroah-HartmanFrom: Vladimir Oltean <[email protected]>
[ Upstream commit 10b6bb62ae1a49ee818fc479cf57b8900176773e ]
Ido Schimmel points out that since commit 52cff74eef5d ("dcbnl : Disable software interrupts before taking dcb_lock"), the DCB API can be called by drivers from softirq context.
One such in-tree example is the chelsio cxgb4 driver:dcb_rpl
-> cxgb4_dcb_handle_fw_update
-> dcb_ieee_setapp
If the firmware for this driver happened to send an event which resulted in a call to dcb_ieee_setapp() at the exact same time as another DCB-enabled interface was unregistering on the same CPU, the softirq would deadlock, because the interrupted process was already holding the dcb_lock in dcbnl_flush_dev().
Fix this unlikely event by using spin_lock_bh() in dcbnl_flush_dev() as in the rest of the dcbnl code.
--- net/dcb/dcbnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
net/dcb/dcbnl.c +2 -2
@@ -2077,7 +2077,7 @@ static void dcbnl_flush_dev(struct net_device *dev) {struct dcb_app_type *itr, *tmp; -spin_lock (&dcb_lock); +spin_lock_bh (&dcb_lock);list_for_each_entry_safe (itr, tmp, &dcb_app_list, list) {if (itr->ifindex == dev->ifindex) {
@@ -2086,7 +2086,7 @@ static void dcbnl_flush_dev(struct net_device *dev) } } -spin_unlock (&dcb_lock); +spin_unlock_bh (&dcb_lock); } static int dcbnl_netdevice_event(struct notifier_block *nb,
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 164/186 selftests: mlxsw: resource_scale: Fix return value
In reply to Greg Kroah-HartmanFrom: Amit Cohen <[email protected]>
[ Upstream commit 196f9bc050cbc5085b4cbb61cce2efe380bc66d0 ]
The test runs several test cases and is supposed to return an error in case at least one of them failed.
Currently, the check of the return value of each test case is in the wrong place, which can result in the wrong return value. For example:
# TESTS='tc_police' ./resource_scale.sh
TEST: 'tc_police' [default] 968 [FAIL]
tc police offload count failed
Error: mlxsw_spectrum: Failed to allocate policer index. We have an error talking to the kernel
Command failed /tmp/tmp.i7Oc5HwmXY:969
TEST: 'tc_police' [default] overflow 969 [ OK ]
...
TEST: 'tc_police' [ipv4_max] overflow 969 [ OK ]
$ echo $? 0
Fix this by moving the check to be done after each test case.
--- .../selftests/drivers/net/mlxsw/spectrum/resource_scale.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh +1 -1
@@ -50,8 +50,8 @@ for current_test in ${TESTS:-$ALL_TESTS}; do else log_test "'$current_test' [$profile] overflow $target" fi + RET_FIN=$(( RET_FIN || RET )) done - RET_FIN=$(( RET_FIN || RET )) done done current_test=""
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerPATCH 165/186 net: stmmac: perserve TX and RX coalesce value during XDP setup
In reply to Greg Kroah-HartmanFrom: Ong Boon Leong <[email protected]>
commit 61da6ac715700bcfeef50d187e15c6cc7c9d079b upstream.
When XDP program is loaded, it is desirable that the previous TX and RX coalesce values are not re-inited to its default value. This prevents unnecessary re-configurig the coalesce values that were working fine before.
--- 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
@@ -6596,6 +6596,9 @@ int stmmac_xdp_open(struct net_device *d tx_q->tx_tail_addr = tx_q->dma_tx_phy;stmmac_set_tx_tail_ptr (priv, priv->ioaddr, tx_q->tx_tail_addr, chan); + +hrtimer_init (&tx_q->txtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_REL ); + tx_q->txtimer.function = stmmac_tx_timer; }/* Enable the MAC Rx/Tx */
@@ -6604,8 +6607,6 @@ int stmmac_xdp_open(struct net_device *d/* Start Rx & Tx DMA Channels */ stmmac_start_all_dma (priv); -stmmac_init_coalesce (priv); - ret =stmmac_request_irq (dev);if (ret) goto irq_error;
Greg Kroah-Hartman
maintainerPATCH 166/186 Input: elan_i2c - move regulator_[en|dis]able() out of elan_[en|dis]able_power()
In reply to Greg Kroah-HartmanFrom: Hans de Goede <[email protected]>
commit 81a36d8ce554b82b0a08e2b95d0bd44fcbff339b upstream.
elan_disable_power() is called conditionally on suspend, where as elan_enable_power() is always called on resume. This leads to an imbalance in the regulator's enable count.
Move the regulator_[en|dis]able() calls out of elan_[en|dis]able_power() in preparation of fixing this.
No functional changes intended.
--- drivers/input/mouse/elan_i2c_core.c | 62 ++++++++++++------------------------ 1 file changed, 22 insertions(+), 40 deletions(-)
drivers/input/mouse/elan_i2c_core.c +22 -40
@@ -186,55 +186,21 @@ static int elan_get_fwinfo(u16 ic_type,return 0 ; } -static int elan_enable_power (struct elan_tp_data *data) +static int elan_set_power (struct elan_tp_data *data,bool on) {int repeat =ETP_RETRY_COUNT ;int error; - error =regulator_enable (data->vcc); -if (error) { -dev_err (&data->client->dev, -"failed to enable regulator: %d\n" , error); -return error; - } -do { - error = data->ops->power_control (data->client, true); + error = data->ops->power_control (data->client, on);if (error >=0 )return 0 ;msleep (30 ); }while (--repeat >0 ); -dev_err (&data->client->dev,"failed to enable power: %d\n" , error); -return error; -} - -static int elan_disable_power (struct elan_tp_data *data) -{ -int repeat =ETP_RETRY_COUNT ; -int error; - -do { - error = data->ops->power_control (data->client, false); -if (!error) { - error =regulator_disable (data->vcc); -if (error) { -dev_err (&data->client->dev, -"failed to disable regulator: %d\n" , - error); -/* Attempt to power the chip back up */ - data->ops->power_control (data->client, true); -break ; - } - -return 0 ; - } - -msleep (30 ); - }while (--repeat >0 ); - -dev_err (&data->client->dev,"failed to disable power: %d\n" , error); +dev_err (&data->client->dev,"failed to set power %s: %d\n" , + on ?"on" :"off" , error);return error; }
@@ -1399,9 +1365,19 @@ static int __maybe_unused elan_suspend(s/* Enable wake from IRQ */ data->irq_wake = (enable_irq_wake (client->irq) ==0 ); }else { - ret =elan_disable_power (data); + ret =elan_set_power (data, false); +if (ret) + goto err; + + ret =regulator_disable (data->vcc); +if (ret) { +dev_err (dev,"error %d disabling regulator\n" , ret); +/* Attempt to power the chip back up */ +elan_set_power (data, true); + } } +err:mutex_unlock (&data->sysfs_mutex);return ret; }
@@ -1417,7 +1393,13 @@ static int __maybe_unused elan_resume(st data->irq_wake = false; } - error =elan_enable_power (data); + error =regulator_enable (data->vcc); +if (error) { +dev_err (dev,"error %d enabling regulator\n" , error); + goto err; + } + + error =elan_set_power (data, true);if (error) {dev_err (dev,"power up when resuming failed: %d\n" , error); goto err;
Greg Kroah-Hartman
maintainerPATCH 167/186 Input: elan_i2c - fix regulator enable count imbalance after suspend/resume
In reply to Greg Kroah-HartmanFrom: Hans de Goede <[email protected]>
commit 04b7762e37c95d9b965d16bb0e18dbd1fa2e2861 upstream.
Before these changes elan_suspend() would only disable the regulator when device_may_wakeup() returns false; whereas elan_resume() would unconditionally enable it, leading to an enable count imbalance when device_may_wakeup() returns true.
This triggers the "WARN_ON(regulator->enable_count)" in regulator_put() when the elan_i2c driver gets unbound, this happens e.g. with the hot-plugable dock with Elan I2C touchpad for the Asus TF103C 2-in-1.
Fix this by making the regulator_enable() call also be conditional on device_may_wakeup() returning false.
--- drivers/input/mouse/elan_i2c_core.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/input/mouse/elan_i2c_core.c +7 -7
@@ -1388,17 +1388,17 @@ static int __maybe_unused elan_resume(ststruct elan_tp_data *data =i2c_get_clientdata (client);int error; -if (device_may_wakeup (dev) && data->irq_wake) { +if (!device_may_wakeup (dev)) { + error =regulator_enable (data->vcc); +if (error) { +dev_err (dev,"error %d enabling regulator\n" , error); + goto err; + } + }else if (data->irq_wake) {disable_irq_wake (client->irq); data->irq_wake = false; } - error =regulator_enable (data->vcc); -if (error) { -dev_err (dev,"error %d enabling regulator\n" , error); - goto err; - } - error =elan_set_power (data, true);if (error) {dev_err (dev,"power up when resuming failed: %d\n" , error);
Greg Kroah-Hartman
maintainerPATCH 168/186 Input: samsung-keypad - properly state IOMEM dependency
In reply to Greg Kroah-HartmanFrom: David Gow <[email protected]>
commit ba115adf61b36b8c167126425a62b0efc23f72c0 upstream.
Make the samsung-keypad driver explicitly depend on CONFIG_HAS_IOMEM, as it calls devm_ioremap(). This prevents compile errors in some configs (e.g, allyesconfig/randconfig under UML):
/usr/bin/ld: drivers/input/keyboard/samsung-keypad.o: in function `samsung_keypad_probe':
samsung-keypad.c:(.text+0xc60): undefined reference to `devm_ioremap'
--- drivers/input/keyboard/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/input/keyboard/Kconfig +1 -1
@@ -556,7 +556,7 @@ config KEYBOARD_PMIC8XXX config KEYBOARD_SAMSUNG tristate "Samsung keypad support" - depends on HAVE_CLK + depends on HAS_IOMEM && HAVE_CLK select INPUT_MATRIXKMAP help Say Y here if you want to use the keypad on your Samsung mobile
Greg Kroah-Hartman
maintainerPATCH 169/186 HID: add mapping for KEY_DICTATE
In reply to Greg Kroah-HartmanFrom: William Mahon <[email protected]>
commit bfa26ba343c727e055223be04e08f2ebdd43c293 upstream.
Numerous keyboards are adding dictate keys which allows for text messages to be dictated by a microphone.
This patch adds a new key definition KEY_DICTATE and maps 0x0c/0x0d8 usage code to this new keycode. Additionally hid-debug is adjusted to recognize this new usage code as well.
--- drivers/hid/hid-debug.c | 1 + drivers/hid/hid-input.c | 1 + include/uapi/linux/input-event-codes.h | 1 + 3 files changed, 3 insertions(+)
drivers/hid/hid-debug.c +1 -0
@@ -934,6 +934,7 @@ static const char *keys[KEY_MAX + 1] = { [KEY_ASSISTANT] = "Assistant", [KEY_KBD_LAYOUT_NEXT] = "KbdLayoutNext", [KEY_EMOJI_PICKER] = "EmojiPicker", + [KEY_DICTATE] = "Dictate", [KEY_BRIGHTNESS_MIN] = "BrightnessMin", [KEY_BRIGHTNESS_MAX] = "BrightnessMax", [KEY_BRIGHTNESS_AUTO] = "BrightnessAuto",
drivers/hid/hid-input.c +1 -0
@@ -994,6 +994,7 @@ static void hidinput_configure_usage(strcase 0x0cd :map_key_clear (KEY_PLAYPAUSE );break ;case 0x0cf :map_key_clear (KEY_VOICECOMMAND );break ; +case 0x0d8 :map_key_clear (KEY_DICTATE );break ;case 0x0d9 :map_key_clear (KEY_EMOJI_PICKER );break ;case 0x0e0 :map_abs_clear (ABS_VOLUME );break ;
include/uapi/linux/input-event-codes.h +1 -0
@@ -612,6 +612,7 @@#define KEY_ASSISTANT 0x247/* AL Context-aware desktop assistant */ #define KEY_KBD_LAYOUT_NEXT 0x248/* AC Next Keyboard Layout Select */ #define KEY_EMOJI_PICKER 0x249/* Show/hide emoji picker (HUTRR101) */ +#define KEY_DICTATE 0x24a/* Start or Stop Voice Dictation Session (HUTRR99) */ #define KEY_BRIGHTNESS_MIN 0x250/* Set Brightness to Minimum */ #define KEY_BRIGHTNESS_MAX 0x251/* Set Brightness to Maximum */
Greg Kroah-Hartman
maintainerPATCH 170/186 HID: add mapping for KEY_ALL_APPLICATIONS
In reply to Greg Kroah-HartmanFrom: William Mahon <[email protected]>
commit 327b89f0acc4c20a06ed59e4d9af7f6d804dc2e2 upstream.
This patch adds a new key definition for KEY_ALL_APPLICATIONS and aliases KEY_DASHBOARD to it.
It also maps the 0x0c/0x2a2 usage code to KEY_ALL_APPLICATIONS.
--- drivers/hid/hid-debug.c | 4 +++- drivers/hid/hid-input.c | 2 ++ include/uapi/linux/input-event-codes.h | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-)
drivers/hid/hid-debug.c +3 -1
@@ -825,7 +825,9 @@ static const char *keys[KEY_MAX + 1] = { [KEY_F22] = "F22", [KEY_F23] = "F23", [KEY_F24] = "F24", [KEY_PLAYCD] = "PlayCD", [KEY_PAUSECD] = "PauseCD", [KEY_PROG3] = "Prog3", - [KEY_PROG4] = "Prog4", [KEY_SUSPEND] = "Suspend", + [KEY_PROG4] = "Prog4", + [KEY_ALL_APPLICATIONS] = "AllApplications", + [KEY_SUSPEND] = "Suspend", [KEY_CLOSE] = "Close", [KEY_PLAY] = "Play", [KEY_FASTFORWARD] = "FastForward", [KEY_BASSBOOST] = "BassBoost", [KEY_PRINT] = "Print", [KEY_HP] = "HP",
drivers/hid/hid-input.c +2 -0
@@ -1086,6 +1086,8 @@ static void hidinput_configure_usage(strcase 0x29d :map_key_clear (KEY_KBD_LAYOUT_NEXT );break ; +case 0x2a2 :map_key_clear (KEY_ALL_APPLICATIONS );break ; +case 0x2c7 :map_key_clear (KEY_KBDINPUTASSIST_PREV );break ;case 0x2c8 :map_key_clear (KEY_KBDINPUTASSIST_NEXT );break ;case 0x2c9 :map_key_clear (KEY_KBDINPUTASSIST_PREVGROUP );break ;
include/uapi/linux/input-event-codes.h +2 -1
@@ -278,7 +278,8 @@#define KEY_PAUSECD 201#define KEY_PROG3 202#define KEY_PROG4 203 -#define KEY_DASHBOARD 204/* AL Dashboard */ +#define KEY_ALL_APPLICATIONS 204/* AC Desktop Show All Applications */ +#define KEY_DASHBOARD KEY_ALL_APPLICATIONS#define KEY_SUSPEND 205#define KEY_CLOSE 206/* AC Close */ #define KEY_PLAY 207
Greg Kroah-Hartman
maintainerPATCH 171/186 tracing/histogram: Fix sorting on old "cpu" value
In reply to Greg Kroah-HartmanFrom: Steven Rostedt (Google) <[email protected]>
commit 1d1898f65616c4601208963c3376c1d828cbf2c7 upstream.
When trying to add a histogram against an event with the "cpu" field, it was impossible due to "cpu" being a keyword to key off of the running CPU. So to fix this, it was changed to "common_cpu" to match the other generic fields (like "common_pid"). But since some scripts used "cpu" for keying off of the CPU (for events that did not have "cpu" as a field, which is most of them), a backward compatibility trick was added such that if "cpu" was used as a key, and the event did not have "cpu" as a field name, then it would fallback and switch over to "common_cpu".
This fix has a couple of subtle bugs. One was that when switching over to "common_cpu", it did not change the field name, it just set a flag. But the code still found a "cpu" field. The "cpu" field is used for filtering and is returned when the event does not have a "cpu" field.
This was found by:
# cd /sys/kernel/tracing # echo hist:key=cpu,pid:sort=cpu > events/sched/sched_wakeup/trigger # cat events/sched/sched_wakeup/hist
Which showed the histogram unsorted:
{ cpu: 19, pid: 1175 } hitcount: 1
{ cpu: 6, pid: 239 } hitcount: 2
{ cpu: 23, pid: 1186 } hitcount: 14
{ cpu: 12, pid: 249 } hitcount: 2
{ cpu: 3, pid: 994 } hitcount: 5Instead of hard coding the "cpu" checks, take advantage of the fact that trace_event_field_field() returns a special field for "cpu" and "CPU" if the event does not have "cpu" as a field. This special field has the "filter_type" of "FILTER_CPU". Check that to test if the returned field is of the CPU type instead of doing the string compare.
Also, fix the sorting bug by testing for the hist_field flag of HIST_FIELD_FL_CPU when setting up the sort routine. Otherwise it will use the special CPU field to know what compare routine to use, and since that special field does not have a size, it returns tracing_map_cmp_none.
--- kernel/trace/trace_events_hist.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
kernel/trace/trace_events_hist.c +3 -3
@@ -2273,9 +2273,9 @@ parse_field(struct hist_trigger_data *hi/* * For backward compatibility, if field_name * was "cpu", then we treat this the same as -* common_cpu. +* common_cpu. This also works for "CPU". */ -if (strcmp (field_name,"cpu" ) ==0 ) { +if (field && field->filter_type ==FILTER_CPU ) { *flags |=HIST_FIELD_FL_CPU ; }else { hist_err(tr, HIST_ERR_FIELD_NOT_FOUND,
@@ -4816,7 +4816,7 @@ static int create_tracing_map_fields(strif (hist_field->flags &HIST_FIELD_FL_STACKTRACE ) cmp_fn = tracing_map_cmp_none; -else if (!field) +else if (!field || hist_field->flags &HIST_FIELD_FL_CPU ) cmp_fn =tracing_map_cmp_num (hist_field->size, hist_field->is_signed);else if (is_string_field (field))
Greg Kroah-Hartman
maintainerPATCH 172/186 tracing: Fix return value of __setup handlers
In reply to Greg Kroah-HartmanFrom: Randy Dunlap <[email protected]>
commit 1d02b444b8d1345ea4708db3bab4db89a7784b55 upstream.
__setup() handlers should generally return 1 to indicate that the boot options have been handled.
Using invalid option values causes the entire kernel boot option string to be reported as Unknown and added to init's environment strings, polluting it.
Unknown kernel command line parameters "BOOT_IMAGE=/boot/bzImage-517rc6
kprobe_event=p,syscall_any,$arg1 trace_options=quiet trace_clock=jiffies", will be passed to user space.
Run /sbin/init as init process
with arguments:
/sbin/init
with environment:
HOME=/
TERM=linux
BOOT_IMAGE=/boot/bzImage-517rc6
kprobe_event=p,syscall_any,$arg1
trace_options=quiet
trace_clock=jiffiesReturn 1 from the __setup() handlers so that init's environment is not polluted with kernel boot options.
--- kernel/trace/trace.c | 4 ++-- kernel/trace/trace_kprobe.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
kernel/trace/trace.c +2 -2
@@ -235,7 +235,7 @@ static char trace_boot_options_buf[MAX_Tstatic int __init set_trace_boot_options (char *str) {strlcpy (trace_boot_options_buf, str,MAX_TRACER_SIZE ); -return 0 ; +return 1 ; }__setup ("trace_options=" , set_trace_boot_options);
@@ -246,7 +246,7 @@ static int __init set_trace_boot_clock(c {strlcpy (trace_boot_clock_buf, str,MAX_TRACER_SIZE ); trace_boot_clock = trace_boot_clock_buf; -return 0 ; +return 1 ; }__setup ("trace_clock=" , set_trace_boot_clock);
kernel/trace/trace_kprobe.c +1 -1
@@ -31,7 +31,7 @@ static int __init set_kprobe_boot_eventsstrlcpy (kprobe_boot_events_buf, str,COMMAND_LINE_SIZE );disable_tracing_selftest ("running kprobe events" ); -return 0 ; +return 1 ; }__setup ("kprobe_event=" , set_kprobe_boot_events);
Greg Kroah-Hartman
maintainerPATCH 173/186 btrfs: fix lost prealloc extents beyond eof after full fsync
In reply to Greg Kroah-HartmanFrom: Filipe Manana <[email protected]>
commit d99478874355d3a7b9d86dfb5d7590d5b1754b1f upstream.
When doing a full fsync, if we have prealloc extents beyond (or at) eof, and the leaves that contain them were not modified in the current transaction, we end up not logging them. This results in losing those extents when we replay the log after a power failure, since the inode is truncated to the current value of the logged i_size.
Just like for the fast fsync path, we need to always log all prealloc extents starting at or beyond i_size. The fast fsync case was fixed in commit 471d557afed155 ("Btrfs: fix loss of prealloc extents past i_size after fsync log replay") but it missed the full fsync path. The problem exists since the very early days, when the log tree was added by commit e02119d5a7b439 ("Btrfs: Add a write ahead tree log to optimize synchronous operations").
Example reproducer:
$ mkfs.btrfs -f /dev/sdc $ mount /dev/sdc /mnt
# Create our test file with many file extent items, so that they span
# several leaves of metadata, even if the node/page size is 64K. Use # direct IO and not fsync/O_SYNC because it's both faster and it avoids # clearing the full sync flag from the inode - we want the fsync below # to trigger the slow full sync code path. $ xfs_io -f -d -c "pwrite -b 4K 0 16M" /mnt/foo
# Now add two preallocated extents to our file without extending the # file's size. One right at i_size, and another further beyond, leaving
# a gap between the two prealloc extents. $ xfs_io -c "falloc -k 16M 1M" /mnt/foo $ xfs_io -c "falloc -k 20M 1M" /mnt/foo
# Make sure everything is durably persisted and the transaction is
# committed. This makes all created extents to have a generation lower # than the generation of the transaction used by the next write and # fsync. sync
# Now overwrite only the first extent, which will result in modifying
# only the first leaf of metadata for our inode. Then fsync it. This # fsync will use the slow code path (inode full sync bit is set) because # it's the first fsync since the inode was created/loaded. $ xfs_io -c "pwrite 0 4K" -c "fsync" /mnt/foo
# Extent list before power failure.
$ xfs_io -c "fiemap -v" /mnt/foo
/mnt/foo:
EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
0: [0..7]: 2178048..2178055 8 0x0
1: [8..16383]: 26632..43007 16376 0x0
2: [16384..32767]: 2156544..2172927 16384 0x0
3: [32768..34815]: 2172928..2174975 2048 0x800
4: [34816..40959]: hole 6144
5: [40960..43007]: 2174976..2177023 2048 0x801<power fail>
# Mount fs again, trigger log replay.
$ mount /dev/sdc /mnt
# Extent list after power failure and log replay.
$ xfs_io -c "fiemap -v" /mnt/foo
/mnt/foo:
EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
0: [0..7]: 2178048..2178055 8 0x0
1: [8..16383]: 26632..43007 16376 0x0
2: [16384..32767]: 2156544..2172927 16384 0x1# The prealloc extents at file offsets 16M and 20M are missing.
So fix this by calling btrfs_log_prealloc_extents() when we are doing a full fsync, so that we always log all prealloc extents beyond eof.
A test case for fstests will follow soon.
--- fs/btrfs/tree-log.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-)
fs/btrfs/tree-log.c +31 -12
@@ -4658,7 +4658,7 @@ static int log_one_extent(struct btrfs_t/* * Log all prealloc extents beyond the inode's i_size to make sure we do not -* lose them after doing a fast fsync and replaying the log. We scan the +* lose them after doing a full/fast fsync and replaying the log. We scan the * subvolume's root instead of iterating the inode's extent map tree because * otherwise we can log incorrect extent items based on extent map conversion. * That can happen due to the fact that extent maps are merged when they
@@ -5437,6 +5437,7 @@ static int copy_inode_items_to_log(strucstruct btrfs_log_ctx *ctx, bool *need_log_inode_item) { + const u64 i_size = i_size_read(&inode->vfs_inode);struct btrfs_root *root = inode->root;int ins_start_slot =0 ;int ins_nr =0 ;
@@ -5457,13 +5458,21 @@ again:if (min_key->type > max_key->type)break ; -if (min_key->type ==BTRFS_INODE_ITEM_KEY ) +if (min_key->type ==BTRFS_INODE_ITEM_KEY ) { *need_log_inode_item = false; - -if ((min_key->type ==BTRFS_INODE_REF_KEY || - min_key->type ==BTRFS_INODE_EXTREF_KEY ) && - inode->generation == trans->transid && - !recursive_logging) { + }else if (min_key->type ==BTRFS_EXTENT_DATA_KEY && + min_key->offset >= i_size) { +/* +* Extents at and beyond eof are logged with +* btrfs_log_prealloc_extents(). +* Only regular files have BTRFS_EXTENT_DATA_KEY keys, +* and no keys greater than that, so bail out. +*/ +break ; + }else if ((min_key->type ==BTRFS_INODE_REF_KEY || + min_key->type ==BTRFS_INODE_EXTREF_KEY ) && + inode->generation == trans->transid && + !recursive_logging) {u64 other_ino =0 ;u64 other_parent =0 ;
@@ -5494,10 +5503,8 @@ again:btrfs_release_path (path); goto next_key; } - } - -/* Skip xattrs, we log them later with btrfs_log_all_xattrs() */ -if (min_key->type ==BTRFS_XATTR_ITEM_KEY ) { + }else if (min_key->type ==BTRFS_XATTR_ITEM_KEY ) { +/* Skip xattrs, logged later with btrfs_log_all_xattrs() */ if (ins_nr ==0 ) goto next_slot; ret = copy_items(trans, inode, dst_path, path,
@@ -5550,9 +5557,21 @@ next_key:break ; } } -if (ins_nr) +if (ins_nr) { ret =copy_items (trans, inode, dst_path, path, ins_start_slot, ins_nr, inode_only, logged_isize); +if (ret) +return ret; + } + +if (inode_only ==LOG_INODE_ALL &&S_ISREG (inode->vfs_inode.i_mode)) { +/* +* Release the path because otherwise we might attempt to double +* lock the same leaf with btrfs_log_prealloc_extents() below. +*/ +btrfs_release_path (path); + ret =btrfs_log_prealloc_extents (trans, inode, dst_path); + }return ret; }
Greg Kroah-Hartman
maintainerPATCH 174/186 btrfs: fix relocation crash due to premature return from btrfs_commit_transaction()
In reply to Greg Kroah-HartmanFrom: Omar Sandoval <[email protected]>
commit 5fd76bf31ccfecc06e2e6b29f8c809e934085b99 upstream.
We are seeing crashes similar to the following trace:
[38.969182] WARNING: CPU: 20 PID: 2105 at fs/btrfs/relocation.c:4070 btrfs_relocate_block_group+0x2dc/0x340 [btrfs] [38.973556] CPU: 20 PID: 2105 Comm: btrfs Not tainted 5.17.0-rc4 #54 [38.974580] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 [38.976539] RIP: 0010:btrfs_relocate_block_group+0x2dc/0x340 [btrfs] [38.980336] RSP: 0000:ffffb0dd42e03c20 EFLAGS: 00010206 [38.981218] RAX: ffff96cfc4ede800 RBX: ffff96cfc3ce0000 RCX: 000000000002ca14 [38.982560] RDX: 0000000000000000 RSI: 4cfd109a0bcb5d7f RDI: ffff96cfc3ce0360 [38.983619] RBP: ffff96cfc309c000 R08: 0000000000000000 R09: 0000000000000000 [38.984678] R10: ffff96cec0000001 R11: ffffe84c80000000 R12: ffff96cfc4ede800 [38.985735] R13: 0000000000000000 R14: 0000000000000000 R15: ffff96cfc3ce0360 [38.987146] FS: 00007f11c15218c0(0000) GS:ffff96d6dfb00000(0000) knlGS:0000000000000000 [38.988662] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [38.989398] CR2: 00007ffc922c8e60 CR3: 00000001147a6001 CR4: 0000000000370ee0 [38.990279] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [38.991219] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [38.992528] Call Trace: [38.992854] <TASK> [38.993148] btrfs_relocate_chunk+0x27/0xe0 [btrfs] [38.993941] btrfs_balance+0x78e/0xea0 [btrfs] [38.994801] ? vsnprintf+0x33c/0x520 [38.995368] ? __kmalloc_track_caller+0x351/0x440 [38.996198] btrfs_ioctl_balance+0x2b9/0x3a0 [btrfs] [38.997084] btrfs_ioctl+0x11b0/0x2da0 [btrfs] [38.997867] ? mod_objcg_state+0xee/0x340 [38.998552] ? seq_release+0x24/0x30 [38.999184] ? proc_nr_files+0x30/0x30 [38.999654] ? call_rcu+0xc8/0x2f0 [39.000228] ? __x64_sys_ioctl+0x84/0xc0 [39.000872] ? btrfs_ioctl_get_supported_features+0x30/0x30 [btrfs] [39.001973] __x64_sys_ioctl+0x84/0xc0 [39.002566] do_syscall_64+0x3a/0x80 [39.003011] entry_SYSCALL_64_after_hwframe+0x44/0xae [39.003735] RIP: 0033:0x7f11c166959b [39.007324] RSP: 002b:00007fff2543e998 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [39.008521] RAX: ffffffffffffffda RBX: 00007f11c1521698 RCX: 00007f11c166959b [39.009833] RDX: 00007fff2543ea40 RSI: 00000000c4009420 RDI: 0000000000000003 [39.011270] RBP: 0000000000000003 R08: 0000000000000013 R09: 00007f11c16f94e0 [39.012581] R10: 0000000000000000 R11: 0000000000000246 R12: 00007fff25440df3 [39.014046] R13: 0000000000000000 R14: 00007fff2543ea40 R15: 0000000000000001 [39.015040] </TASK> [39.015418] ---[ end trace 0000000000000000 ]--- [43.131559] ------------[ cut here ]------------ [43.132234] kernel BUG at fs/btrfs/extent-tree.c:2717! [43.133031] invalid opcode: 0000 [#1] PREEMPT SMP PTI [43.133702] CPU: 1 PID: 1839 Comm: btrfs Tainted: G W 5.17.0-rc4 #54 [43.134863] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 [43.136426] RIP: 0010:unpin_extent_range+0x37a/0x4f0 [btrfs] [43.139913] RSP: 0000:ffffb0dd4216bc70 EFLAGS: 00010246 [43.140629] RAX: 0000000000000000 RBX: ffff96cfc34490f8 RCX: 0000000000000001 [43.141604] RDX: 0000000080000001 RSI: 0000000051d00000 RDI: 00000000ffffffff [43.142645] RBP: 0000000000000000 R08: 0000000000000000 R09: ffff96cfd07dca50 [43.143669] R10: ffff96cfc46e8a00 R11: fffffffffffec000 R12: 0000000041d00000 [43.144657] R13: ffff96cfc3ce0000 R14: ffffb0dd4216bd08 R15: 0000000000000000 [43.145686] FS: 00007f7657dd68c0(0000) GS:ffff96d6df640000(0000) knlGS:0000000000000000 [43.146808] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [43.147584] CR2: 00007f7fe81bf5b0 CR3: 00000001093ee004 CR4: 0000000000370ee0 [43.148589] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [43.149581] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [43.150559] Call Trace: [43.150904] <TASK> [43.151253] btrfs_finish_extent_commit+0x88/0x290 [btrfs] [43.152127] btrfs_commit_transaction+0x74f/0xaa0 [btrfs] [43.152932] ? btrfs_attach_transaction_barrier+0x1e/0x50 [btrfs] [43.153786] btrfs_ioctl+0x1edc/0x2da0 [btrfs] [43.154475] ? __check_object_size+0x150/0x170 [43.155170] ? preempt_count_add+0x49/0xa0 [43.155753] ? __x64_sys_ioctl+0x84/0xc0 [43.156437] ? btrfs_ioctl_get_supported_features+0x30/0x30 [btrfs] [43.157456] __x64_sys_ioctl+0x84/0xc0 [43.157980] do_syscall_64+0x3a/0x80 [43.158543] entry_SYSCALL_64_after_hwframe+0x44/0xae [43.159231] RIP: 0033:0x7f7657f1e59b [43.161819] RSP: 002b:00007ffda5cd1658 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [43.162702] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007f7657f1e59b [43.163526] RDX: 0000000000000000 RSI: 0000000000009408 RDI: 0000000000000003 [43.164358] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000 [43.165208] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 [43.166029] R13: 00005621b91c3232 R14: 00005621b91ba580 R15: 00007ffda5cd1800 [43.166863] </TASK> [43.167125] Modules linked in: btrfs blake2b_generic xor pata_acpi ata_piix libata raid6_pq scsi_mod libcrc32c virtio_net virtio_rng net_failover rng_core failover scsi_common [43.169552] ---[ end trace 0000000000000000 ]--- [43.171226] RIP: 0010:unpin_extent_range+0x37a/0x4f0 [btrfs] [43.174767] RSP: 0000:ffffb0dd4216bc70 EFLAGS: 00010246 [43.175600] RAX: 0000000000000000 RBX: ffff96cfc34490f8 RCX: 0000000000000001 [43.176468] RDX: 0000000080000001 RSI: 0000000051d00000 RDI: 00000000ffffffff [43.177357] RBP: 0000000000000000 R08: 0000000000000000 R09: ffff96cfd07dca50 [43.178271] R10: ffff96cfc46e8a00 R11: fffffffffffec000 R12: 0000000041d00000 [43.179178] R13: ffff96cfc3ce0000 R14: ffffb0dd4216bd08 R15: 0000000000000000 [43.180071] FS: 00007f7657dd68c0(0000) GS:ffff96d6df800000(0000) knlGS:0000000000000000 [43.181073] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [43.181808] CR2: 00007fe09905f010 CR3: 00000001093ee004 CR4: 0000000000370ee0 [43.182706] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [43.183591] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
We first hit the WARN_ON(rc->block_group->pinned > 0) in btrfs_relocate_block_group() and then the BUG_ON(!cache) in unpin_extent_range(). This tells us that we are exiting relocation and removing the block group with bytes still pinned for that block group. This is supposed to be impossible: the last thing relocate_block_group() does is commit the transaction to get rid of pinned extents.
Commit d0c2f4fa555e ("btrfs: make concurrent fsyncs wait less when waiting for a transaction commit") introduced an optimization so that commits from fsync don't have to wait for the previous commit to unpin extents. This was only intended to affect fsync, but it inadvertently made it possible for any commit to skip waiting for the previous commit to unpin. This is because if a call to btrfs_commit_transaction() finds that another thread is already committing the transaction, it waits for the other thread to complete the commit and then returns. If that other thread was in fsync, then it completes the commit without completing the previous commit. This makes the following sequence of events possible:
Thread 1____________________|Thread 2 (fsync)_____________________|Thread 3 (balance)___________________
btrfs_commit_transaction(N) | |
btrfs_run_delayed_refs | |
pin extents | |
... | |
state = UNBLOCKED |btrfs_sync_file |
| btrfs_start_transaction(N + 1) |relocate_block_group
| | btrfs_join_transaction(N + 1)
| btrfs_commit_transaction(N + 1) |
... | trans->state = COMMIT_START |
| | btrfs_commit_transaction(N + 1)
| | wait_for_commit(N + 1, COMPLETED)
| wait_for_commit(N, SUPER_COMMITTED)|
state = SUPER_COMMITTED | ... |
btrfs_finish_extent_commit| |
unpin_extent_range() | trans->state = COMPLETED |
| | return
| |
... | |Thread 1 isn't done, so pinned > 0
| |and we WARN
| |
| |btrfs_remove_block_group
unpin_extent_range() | |
Thread 3 removed the | |
block group, so we BUG| |There are other sequences involving SUPER_COMMITTED transactions that can cause a similar outcome.
We could fix this by making relocation explicitly wait for unpinning, but there may be other cases that need it. Josef mentioned ENOSPC flushing and the free space cache inode as other potential victims. Rather than playing whack-a-mole, this fix is conservative and makes all commits not in fsync wait for all previous transactions, which is what the optimization intended.
--- fs/btrfs/transaction.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)
fs/btrfs/transaction.c +31 -1
@@ -846,7 +846,37 @@ btrfs_attach_transaction_barrier(struct static noinline void wait_for_commit(struct btrfs_transaction *commit,const enum btrfs_trans_state min_state) { -wait_event (commit->commit_wait, commit->state >= min_state); +struct btrfs_fs_info *fs_info = commit->fs_info; +u64 transid = commit->transid; +bool put = false; + +while (1 ) { +wait_event (commit->commit_wait, commit->state >= min_state); +if (put) +btrfs_put_transaction (commit); + +if (min_state <TRANS_STATE_COMPLETED ) +break ; + +/* +* A transaction isn't really completed until all of the +* previous transactions are completed, but with fsync we can +* end up with SUPER_COMMITTED transactions before a COMPLETED +* transaction. Wait for those. +*/ + +spin_lock (&fs_info->trans_lock); + commit =list_first_entry_or_null (&fs_info->trans_list, + struct btrfs_transaction, + list); +if (!commit || commit->transid > transid) { +spin_unlock (&fs_info->trans_lock); +break ; + } +refcount_inc (&commit->use_count); + put = true; +spin_unlock (&fs_info->trans_lock); + } }int btrfs_wait_for_commit (struct btrfs_fs_info *fs_info,u64 transid)
Greg Kroah-Hartman
maintainerPATCH 175/186 btrfs: subpage: fix a wrong check on subpage->writers
In reply to Greg Kroah-HartmanFrom: Qu Wenruo <[email protected]>
commit c992fa1fd52380d0c4ced7b07479e877311ae645 upstream.
[BUG]
When looping btrfs/074 with 64K page size and 4K sectorsize, there is a low chance (1/50~1/100) to crash with the following ASSERT() triggered in btrfs_subpage_start_writer():
ret = atomic_add_return(nbits, &subpage->writers); ASSERT(ret == nbits); <<< This one <<<
[CAUSE]
With more debugging output on the parameters of btrfs_subpage_start_writer(), it shows a very concerning error:
ret=29 nbits=13 start=393216 len=53248
For @nbits it's correct, but @ret which is the returned value from atomic_add_return(), it's not only larger than nbits, but also larger than max sectors per page value (for 64K page size and 4K sector size, it's 16).
This indicates that some call sites are not properly decreasing the value.
And that's exactly the case, in btrfs_page_unlock_writer(), due to the fact that we can have page locked either by lock_page() or process_one_page(), we have to check if the subpage has any writer.
If no writers, it's locked by lock_page() and we only need to unlock it.
But unfortunately the check for the writers are completely opposite:
if (atomic_read(&subpage->writers)) /* No writers, locked by plain lock_page() */ return unlock_page(page);
We directly unlock the page if it has writers, which is the completely opposite what we want.
Thankfully the affected call site is only limited to extent_write_locked_range(), so it's mostly affecting compressed write.
[FIX]
Just fix the wrong check condition to fix the bug.
--- fs/btrfs/subpage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/btrfs/subpage.c +1 -1
@@ -736,7 +736,7 @@ void btrfs_page_unlock_writer(struct btr* Since we own the page lock, no one else could touch subpage::writers * and we are safe to do several atomic operations without spinlock. */ -if (atomic_read (&subpage->writers)) +if (atomic_read (&subpage->writers) ==0 )/* No writers, locked by plain lock_page() */ return unlock_page (page);
Greg Kroah-Hartman
maintainerPATCH 176/186 btrfs: do not WARN_ON() if we have PageError set
In reply to Greg Kroah-HartmanFrom: Josef Bacik <[email protected]>
commit a50e1fcbc9b85fd4e95b89a75c0884cb032a3e06 upstream.
Whenever we do any extent buffer operations we call assert_eb_page_uptodate() to complain loudly if we're operating on an non-uptodate page. Our overnight tests caught this warning earlier this week
WARNING: CPU: 1 PID: 553508 at fs/btrfs/extent_io.c:6849 assert_eb_page_uptodate+0x3f/0x50 CPU: 1 PID: 553508 Comm: kworker/u4:13 Tainted: G W 5.17.0-rc3+ #564 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-2.fc32 04/01/2014 Workqueue: btrfs-cache btrfs_work_helper RIP: 0010:assert_eb_page_uptodate+0x3f/0x50 RSP: 0018:ffffa961440a7c68 EFLAGS: 00010246 RAX: 0017ffffc0002112 RBX: ffffe6e74453f9c0 RCX: 0000000000001000 RDX: ffffe6e74467c887 RSI: ffffe6e74453f9c0 RDI: ffff8d4c5efc2fc0 RBP: 0000000000000d56 R08: ffff8d4d4a224000 R09: 0000000000000000 R10: 00015817fa9d1ef0 R11: 000000000000000c R12: 00000000000007b1 R13: ffff8d4c5efc2fc0 R14: 0000000001500000 R15: 0000000001cb1000 FS: 0000000000000000(0000) GS:ffff8d4dbbd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007ff31d3448d8 CR3: 0000000118be8004 CR4: 0000000000370ee0 Call Trace: extent_buffer_test_bit+0x3f/0x70 free_space_test_bit+0xa6/0xc0 load_free_space_tree+0x1f6/0x470 caching_thread+0x454/0x630 ? rcu_read_lock_sched_held+0x12/0x60 ? rcu_read_lock_sched_held+0x12/0x60 ? rcu_read_lock_sched_held+0x12/0x60 ? lock_release+0x1f0/0x2d0 btrfs_work_helper+0xf2/0x3e0 ? lock_release+0x1f0/0x2d0 ? finish_task_switch.isra.0+0xf9/0x3a0 process_one_work+0x26d/0x580 ? process_one_work+0x580/0x580 worker_thread+0x55/0x3b0 ? process_one_work+0x580/0x580 kthread+0xf0/0x120 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork+0x1f/0x30
This was partially fixed by c2e39305299f01 ("btrfs: clear extent buffer uptodate when we fail to write it"), however all that fix did was keep us from finding extent buffers after a failed writeout. It didn't keep us from continuing to use a buffer that we already had found.
In this case we're searching the commit root to cache the block group, so we can start committing the transaction and switch the commit root and then start writing. After the switch we can look up an extent buffer that hasn't been written yet and start processing that block group. Then we fail to write that block out and clear Uptodate on the page, and then we start spewing these errors.
Normally we're protected by the tree lock to a certain degree here. If we read a block we have that block read locked, and we block the writer from locking the block before we submit it for the write. However this isn't necessarily fool proof because the read could happen before we do the submit_bio and after we locked and unlocked the extent buffer.
Also in this particular case we have path->skip_locking set, so that won't save us here. We'll simply get a block that was valid when we read it, but became invalid while we were using it.
What we really want is to catch the case where we've "read" a block but it's not marked Uptodate. On read we ClearPageError(), so if we're !Uptodate and !Error we know we didn't do the right thing for reading the page.
Fix this by checking !Uptodate && !Error, this way we will not complain if our buffer gets invalidated while we're using it, and we'll maintain the spirit of the check which is to make sure we have a fully in-cache block while we're messing with it.
--- fs/btrfs/extent_io.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
fs/btrfs/extent_io.c +13 -3
@@ -6865,14 +6865,24 @@ static void assert_eb_page_uptodate(cons {struct btrfs_fs_info *fs_info = eb->fs_info; +/* +* If we are using the commit root we could potentially clear a page +* Uptodate while we're using the extent buffer that we've previously +* looked up. We don't want to complain in this case, as the page was +* valid before, we just didn't write it out. Instead we want to catch +* the case where we didn't actually read the block properly, which +* would have !PageUptodate && !PageError, as we clear PageError before +* reading. +*/ if (fs_info->sectorsize <PAGE_SIZE ) { -bool uptodate; +bool uptodate, error; uptodate =btrfs_subpage_test_uptodate (fs_info, page, eb->start, eb->len); -WARN_ON (!uptodate); + error =btrfs_subpage_test_error (fs_info, page, eb->start, eb->len); +WARN_ON (!uptodate && !error); }else { -WARN_ON (!PageUptodate (page)); +WARN_ON (!PageUptodate (page) && !PageError (page)); } }
Greg Kroah-Hartman
maintainerPATCH 177/186 btrfs: qgroup: fix deadlock between rescan worker and remove qgroup
In reply to Greg Kroah-HartmanFrom: Sidong Yang <[email protected]>
commit d4aef1e122d8bbdc15ce3bd0bc813d6b44a7d63a upstream.
The commit e804861bd4e6 ("btrfs: fix deadlock between quota disable and qgroup rescan worker") by Kawasaki resolves deadlock between quota disable and qgroup rescan worker. But also there is a deadlock case like it. It's about enabling or disabling quota and creating or removing qgroup. It can be reproduced in simple script below.
for i in {1..100}
do
btrfs quota enable /mnt &
btrfs qgroup create 1/0 /mnt &
btrfs qgroup destroy 1/0 /mnt &
btrfs quota disable /mnt &
done
Here's why the deadlock happens:
1) The quota rescan task is running.
2) Task A calls btrfs_quota_disable(), locks the qgroup_ioctl_lock mutex, and then calls btrfs_qgroup_wait_for_completion(), to wait for the quota rescan task to complete.
3) Task B calls btrfs_remove_qgroup() and it blocks when trying to lock the qgroup_ioctl_lock mutex, because it's being held by task A. At that point task B is holding a transaction handle for the current transaction.
4) The quota rescan task calls btrfs_commit_transaction(). This results in it waiting for all other tasks to release their handles on the transaction, but task B is blocked on the qgroup_ioctl_lock mutex while holding a handle on the transaction, and that mutex is being held by task A, which is waiting for the quota rescan task to complete, resulting in a deadlock between these 3 tasks.
To resolve this issue, the thread disabling quota should unlock qgroup_ioctl_lock before waiting rescan completion. Move btrfs_qgroup_wait_for_completion() after unlock of qgroup_ioctl_lock.
--- fs/btrfs/qgroup.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
fs/btrfs/qgroup.c +8 -1
@@ -1197,13 +1197,20 @@ int btrfs_quota_disable(struct btrfs_fs_ goto out;/* +* Unlock the qgroup_ioctl_lock mutex before waiting for the rescan worker to +* complete. Otherwise we can deadlock because btrfs_remove_qgroup() needs +* to lock that mutex while holding a transaction handle and the rescan +* worker needs to commit a transaction. +*/ +mutex_unlock (&fs_info->qgroup_ioctl_lock); + +/* * Request qgroup rescan worker to complete and wait for it. This wait * must be done before transaction start for quota disable since it may * deadlock with transaction by the qgroup rescan worker. */ clear_bit (BTRFS_FS_QUOTA_ENABLED , &fs_info->flags);btrfs_qgroup_wait_for_completion (fs_info, false); -mutex_unlock (&fs_info->qgroup_ioctl_lock);/* * 1 For the root item
Greg Kroah-Hartman
maintainerPATCH 178/186 btrfs: add missing run of delayed items after unlink during log replay
In reply to Greg Kroah-HartmanFrom: Filipe Manana <[email protected]>
commit 4751dc99627e4d1465c5bfa8cb7ab31ed418eff5 upstream.
During log replay, whenever we need to check if a name (dentry) exists in a directory we do searches on the subvolume tree for inode references or or directory entries (BTRFS_DIR_INDEX_KEY keys, and BTRFS_DIR_ITEM_KEY keys as well, before kernel 5.17). However when during log replay we unlink a name, through btrfs_unlink_inode(), we may not delete inode references and dir index keys from a subvolume tree and instead just add the deletions to the delayed inode's delayed items, which will only be run when we commit the transaction used for log replay. This means that after an unlink operation during log replay, if we attempt to search for the same name during log replay, we will not see that the name was already deleted, since the deletion is recorded only on the delayed items.
We run delayed items after every unlink operation during log replay, except at unlink_old_inode_refs() and at add_inode_ref(). This was due to an overlook, as delayed items should be run after evert unlink, for the reasons stated above.
So fix those two cases.
--- fs/btrfs/tree-log.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
fs/btrfs/tree-log.c +18 -0
@@ -1357,6 +1357,15 @@ again: inode, name, namelen);kfree (name);iput (dir); +/* +* Whenever we need to check if a name exists or not, we +* check the subvolume tree. So after an unlink we must +* run delayed items, so that future checks for a name +* during log replay see that the name does not exists +* anymore. +*/ +if (!ret) + ret =btrfs_run_delayed_items (trans);if (ret) goto out; goto again;
@@ -1609,6 +1618,15 @@ static noinline int add_inode_ref(struct*/ if (!ret && inode->i_nlink ==0 )inc_nlink (inode); +/* +* Whenever we need to check if a name exists or +* not, we check the subvolume tree. So after an +* unlink we must run delayed items, so that future +* checks for a name during log replay see that the +* name does not exists anymore. +*/ +if (!ret) + ret =btrfs_run_delayed_items (trans); }if (ret <0 ) goto out;
Greg Kroah-Hartman
maintainerPATCH 179/186 btrfs: fallback to blocking mode when doing async dio over multiple extents
In reply to Greg Kroah-HartmanFrom: Filipe Manana <[email protected]>
commit ca93e44bfb5fd7996b76f0f544999171f647f93b upstream.
Some users recently reported that MariaDB was getting a read corruption when using io_uring on top of btrfs. This started to happen in 5.16, after commit 51bd9563b6783d ("btrfs: fix deadlock due to page faults during direct IO reads and writes"). That changed btrfs to use the new iomap flag IOMAP_DIO_PARTIAL and to disable page faults before calling iomap_dio_rw(). This was necessary to fix deadlocks when the iovector corresponds to a memory mapped file region. That type of scenario is exercised by test case generic/647 from fstests.
For this MariaDB scenario, we attempt to read 16K from file offset X using IOCB_NOWAIT and io_uring. In that range we have 4 extents, each with a size of 4K, and what happens is the following:
1) btrfs_direct_read() disables page faults and calls iomap_dio_rw();
2) iomap creates a struct iomap_dio object, its reference count is initialized to 1 and its ->size field is initialized to 0;
3) iomap calls btrfs_dio_iomap_begin() with file offset X, which finds the first 4K extent, and setups an iomap for this extent consisting of a single page;
4) At iomap_dio_bio_iter(), we are able to access the first page of the buffer (struct iov_iter) with bio_iov_iter_get_pages() without triggering a page fault;
5) iomap submits a bio for this 4K extent (iomap_dio_submit_bio() -> btrfs_submit_direct()) and increments the refcount on the struct iomap_dio object to 2; The ->size field of the struct iomap_dio object is incremented to 4K;
6) iomap calls btrfs_iomap_begin() again, this time with a file offset of X + 4K. There we setup an iomap for the next extent that also has a size of 4K;
7) Then at iomap_dio_bio_iter() we call bio_iov_iter_get_pages(), which tries to access the next page (2nd page) of the buffer. This triggers a page fault and returns -EFAULT;
8) At __iomap_dio_rw() we see the -EFAULT, but we reset the error to 0 because we passed the flag IOMAP_DIO_PARTIAL to iomap and the struct iomap_dio object has a ->size value of 4K (we submitted a bio for an extent already). The 'wait_for_completion' variable is not set to true, because our iocb has IOCB_NOWAIT set;
9) At the bottom of __iomap_dio_rw(), we decrement the reference count of the struct iomap_dio object from 2 to 1. Because we were not the only ones holding a reference on it and 'wait_for_completion' is set to false, -EIOCBQUEUED is returned to btrfs_direct_read(), which just returns it up the callchain, up to io_uring;
10) The bio submitted for the first extent (step 5) completes and its bio endio function, iomap_dio_bio_end_io(), decrements the last reference on the struct iomap_dio object, resulting in calling iomap_dio_complete_work() -> iomap_dio_complete().
11) At iomap_dio_complete() we adjust the iocb->ki_pos from X to X + 4K
and return 4K (the amount of io done) to iomap_dio_complete_work();
12) iomap_dio_complete_work() calls the iocb completion callback, iocb->ki_complete() with a second argument value of 4K (total io done) and the iocb with the adjust ki_pos of X + 4K. This results in completing the read request for io_uring, leaving it with a result of 4K bytes read, and only the first page of the buffer filled in, while the remaining 3 pages, corresponding to the other 3 extents, were not filled;
13) For the application, the result is unexpected because if we ask to read N bytes, it expects to get N bytes read as long as those N bytes don't cross the EOF (i_size).
MariaDB reports this as an error, as it's not expecting a short read, since it knows it's asking for read operations fully within the i_size boundary. This is typical in many applications, but it may also be questionable if they should react to such short reads by issuing more read calls to get the remaining data. Nevertheless, the short read happened due to a change in btrfs regarding how it deals with page faults while in the middle of a read operation, and there's no reason why btrfs can't have the previous behaviour of returning the whole data that was requested by the application.
The problem can also be triggered with the following simple program:
/* Get O_DIRECT */
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <liburing.h>
int main(int argc, char *argv[])
{
char *foo_path;
struct io_uring ring;
struct io_uring_sqe *sqe;
struct io_uring_cqe *cqe;
struct iovec iovec;
int fd;
long pagesize;
void *write_buf;
void *read_buf;
ssize_t ret;
int i; if (argc != 2) {
fprintf(stderr, "Use: %s <directory>\n", argv[0]);
return 1;
}
foo_path = malloc(strlen(argv[1]) + 5);
if (!foo_path) {
fprintf(stderr, "Failed to allocate memory for file path\n");
return 1;
}
strcpy(foo_path, argv[1]);
strcat(foo_path, "/foo");
/*
* Create file foo with 2 extents, each with a size matching
* the page size. Then allocate a buffer to read both extents
* with io_uring, using O_DIRECT and IOCB_NOWAIT. Before doing
* the read with io_uring, access the first page of the buffer
* to fault it in, so that during the read we only trigger a
* page fault when accessing the second page of the buffer.
*/
fd = open(foo_path, O_CREAT | O_TRUNC | O_WRONLY |
O_DIRECT, 0666);
if (fd == -1) {
fprintf(stderr,
"Failed to create file 'foo': %s (errno %d)",
strerror(errno), errno);
return 1;
}
pagesize = sysconf(_SC_PAGE_SIZE);
ret = posix_memalign(&write_buf, pagesize, 2 * pagesize);
if (ret) {
fprintf(stderr, "Failed to allocate write buffer\n");
return 1;
}
memset(write_buf, 0xab, pagesize);
memset(write_buf + pagesize, 0xcd, pagesize);
/* Create 2 extents, each with a size matching page size. */
for (i = 0; i < 2; i++) {
ret = pwrite(fd, write_buf + i * pagesize, pagesize,
i * pagesize);
if (ret != pagesize) {
fprintf(stderr,
"Failed to write to file, ret = %ld errno %d (%s)\n",
ret, errno, strerror(errno));
return 1;
}
ret = fsync(fd);
if (ret != 0) {
fprintf(stderr, "Failed to fsync file\n");
return 1;
}
}
close(fd);
fd = open(foo_path, O_RDONLY | O_DIRECT);
if (fd == -1) {
fprintf(stderr,
"Failed to open file 'foo': %s (errno %d)",
strerror(errno), errno);
return 1;
}
ret = posix_memalign(&read_buf, pagesize, 2 * pagesize);
if (ret) {
fprintf(stderr, "Failed to allocate read buffer\n");
return 1;
}
/*
* Fault in only the first page of the read buffer.
* We want to trigger a page fault for the 2nd page of the
* read buffer during the read operation with io_uring
* (O_DIRECT and IOCB_NOWAIT).
*/
memset(read_buf, 0, 1);
ret = io_uring_queue_init(1, &ring, 0);
if (ret != 0) {
fprintf(stderr, "Failed to create io_uring queue\n");
return 1;
}
sqe = io_uring_get_sqe(&ring);
if (!sqe) {
fprintf(stderr, "Failed to get io_uring sqe\n");
return 1;
}
iovec.iov_base = read_buf;
iovec.iov_len = 2 * pagesize;
io_uring_prep_readv(sqe, fd, &iovec, 1, 0);
ret = io_uring_submit_and_wait(&ring, 1);
if (ret != 1) {
fprintf(stderr,
"Failed at io_uring_submit_and_wait()\n");
return 1;
}
ret = io_uring_wait_cqe(&ring, &cqe);
if (ret < 0) {
fprintf(stderr, "Failed at io_uring_wait_cqe()\n");
return 1;
}
printf("io_uring read result for file foo:\n\n");
printf(" cqe->res == %d (expected %d)\n", cqe->res, 2 * pagesize);
printf(" memcmp(read_buf, write_buf) == %d (expected 0)\n",
memcmp(read_buf, write_buf, 2 * pagesize));
io_uring_cqe_seen(&ring, cqe);
io_uring_queue_exit(&ring);
return 0;}
When running it on an unpatched kernel:
$ gcc io_uring_test.c -luring $ mkfs.btrfs -f /dev/sda $ mount /dev/sda /mnt/sda $ ./a.out /mnt/sda io_uring read result for file foo:
cqe->res == 4096 (expected 8192)
memcmp(read_buf, write_buf) == -205 (expected 0)After this patch, the read always returns 8192 bytes, with the buffer filled with the correct data. Although that reproducer always triggers the bug in my test vms, it's possible that it will not be so reliable on other environments, as that can happen if the bio for the first extent completes and decrements the reference on the struct iomap_dio object before we do the atomic_dec_and_test() on the reference at __iomap_dio_rw().
Fix this in btrfs by having btrfs_dio_iomap_begin() return -EAGAIN whenever we try to satisfy a non blocking IO request (IOMAP_NOWAIT flag set) over a range that spans multiple extents (or a mix of extents and holes). This avoids returning success to the caller when we only did partial IO, which is not optimal for writes and for reads it's actually incorrect, as the caller doesn't expect to get less bytes read than it has requested (unless EOF is crossed), as previously mentioned. This is also the type of behaviour that xfs follows (xfs_direct_write_iomap_begin()), even though it doesn't use IOMAP_DIO_PARTIAL.
A test case for fstests will follow soon.
--- fs/btrfs/inode.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
fs/btrfs/inode.c +28 -0
@@ -7965,6 +7965,34 @@ static int btrfs_dio_iomap_begin(struct } len =min (len, em->len - (start - em->start)); + +/* +* If we have a NOWAIT request and the range contains multiple extents +* (or a mix of extents and holes), then we return -EAGAIN to make the +* caller fallback to a context where it can do a blocking (without +* NOWAIT) request. This way we avoid doing partial IO and returning +* success to the caller, which is not optimal for writes and for reads +* it can result in unexpected behaviour for an application. +* +* When doing a read, because we use IOMAP_DIO_PARTIAL when calling +* iomap_dio_rw(), we can end up returning less data then what the caller +* asked for, resulting in an unexpected, and incorrect, short read. +* That is, the caller asked to read N bytes and we return less than that, +* which is wrong unless we are crossing EOF. This happens if we get a +* page fault error when trying to fault in pages for the buffer that is +* associated to the struct iov_iter passed to iomap_dio_rw(), and we +* have previously submitted bios for other extents in the range, in +* which case iomap_dio_rw() may return us EIOCBQUEUED if not all of +* those bios have completed by the time we get the page fault error, +* which we return back to our caller - we should only return EIOCBQUEUED +* after we have submitted bios for all the extents in the range. +*/ +if ((flags &IOMAP_NOWAIT ) && len < length) { +free_extent_map (em); + ret = -EAGAIN ; + goto unlock_err; + } +if (write) { ret =btrfs_get_blocks_direct_write (&em, inode, dio_data, start, len);
Greg Kroah-Hartman
maintainerPATCH 180/186 btrfs: do not start relocation until in progress drops are done
In reply to Greg Kroah-HartmanFrom: Josef Bacik <[email protected]>
commit b4be6aefa73c9a6899ef3ba9c5faaa8a66e333ef upstream.
We hit a bug with a recovering relocation on mount for one of our file systems in production. I reproduced this locally by injecting errors into snapshot delete with balance running at the same time. This presented as an error while looking up an extent item
WARNING: CPU: 5 PID: 1501 at fs/btrfs/extent-tree.c:866 lookup_inline_extent_backref+0x647/0x680 CPU: 5 PID: 1501 Comm: btrfs-balance Not tainted 5.16.0-rc8+ #8 RIP: 0010:lookup_inline_extent_backref+0x647/0x680 RSP: 0018:ffffae0a023ab960 EFLAGS: 00010202 RAX: 0000000000000001 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 000000000000000c RDI: 0000000000000000 RBP: ffff943fd2a39b60 R08: 0000000000000000 R09: 0000000000000001 R10: 0001434088152de0 R11: 0000000000000000 R12: 0000000001d05000 R13: ffff943fd2a39b60 R14: ffff943fdb96f2a0 R15: ffff9442fc923000 FS: 0000000000000000(0000) GS:ffff944e9eb40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f1157b1fca8 CR3: 000000010f092000 CR4: 0000000000350ee0 Call Trace: <TASK> insert_inline_extent_backref+0x46/0xd0 __btrfs_inc_extent_ref.isra.0+0x5f/0x200 ? btrfs_merge_delayed_refs+0x164/0x190 __btrfs_run_delayed_refs+0x561/0xfa0 ? btrfs_search_slot+0x7b4/0xb30 ? btrfs_update_root+0x1a9/0x2c0 btrfs_run_delayed_refs+0x73/0x1f0 ? btrfs_update_root+0x1a9/0x2c0 btrfs_commit_transaction+0x50/0xa50 ? btrfs_update_reloc_root+0x122/0x220 prepare_to_merge+0x29f/0x320 relocate_block_group+0x2b8/0x550 btrfs_relocate_block_group+0x1a6/0x350 btrfs_relocate_chunk+0x27/0xe0 btrfs_balance+0x777/0xe60 balance_kthread+0x35/0x50 ? btrfs_balance+0xe60/0xe60 kthread+0x16b/0x190 ? set_kthread_struct+0x40/0x40 ret_from_fork+0x22/0x30 </TASK>
Normally snapshot deletion and relocation are excluded from running at the same time by the fs_info->cleaner_mutex. However if we had a pending balance waiting to get the ->cleaner_mutex, and a snapshot deletion was running, and then the box crashed, we would come up in a state where we have a half deleted snapshot.
Again, in the normal case the snapshot deletion needs to complete before relocation can start, but in this case relocation could very well start before the snapshot deletion completes, as we simply add the root to the dead roots list and wait for the next time the cleaner runs to clean up the snapshot.
Fix this by setting a bit on the fs_info if we have any DEAD_ROOT's that had a pending drop_progress key. If they do then we know we were in the middle of the drop operation and set a flag on the fs_info. Then balance can wait until this flag is cleared to start up again.
If there are DEAD_ROOT's that don't have a drop_progress set then we're safe to start balance right away as we'll be properly protected by the cleaner_mutex.
--- fs/btrfs/ctree.h | 10 ++++++++++ fs/btrfs/disk-io.c | 10 ++++++++++ fs/btrfs/extent-tree.c | 10 ++++++++++ fs/btrfs/relocation.c | 13 +++++++++++++ fs/btrfs/root-tree.c | 15 +++++++++++++++ fs/btrfs/transaction.c | 33 ++++++++++++++++++++++++++++++++- fs/btrfs/transaction.h | 1 + 7 files changed, 91 insertions(+), 1 deletion(-)
fs/btrfs/ctree.h +10 -0
@@ -601,6 +601,9 @@ enum {/* Indicate whether there are any tree modification log users */ BTRFS_FS_TREE_MOD_LOG_USERS , +/* Indicate we have half completed snapshot deletions pending. */ +BTRFS_FS_UNFINISHED_DROPS , + #if BITS_PER_LONG == 32/* Indicate if we have error/warn message printed on 32bit systems */ BTRFS_FS_32BIT_ERROR,
@@ -1110,8 +1113,15 @@ enum {BTRFS_ROOT_HAS_LOG_TREE ,/* Qgroup flushing is in progress */ BTRFS_ROOT_QGROUP_FLUSHING , +/* This root has a drop operation that was started previously. */ + BTRFS_ROOT_UNFINISHED_DROP, }; +static inline void btrfs_wake_unfinished_drop (struct btrfs_fs_info *fs_info) +{ +clear_and_wake_up_bit (BTRFS_FS_UNFINISHED_DROPS , &fs_info->flags); +} +/* * Record swapped tree blocks of a subvolume tree for delayed subtree trace * code. For detail check comment in fs/btrfs/qgroup.c.
fs/btrfs/disk-io.c +10 -0
@@ -3665,6 +3665,10 @@ int __cold open_ctree(struct super_blockset_bit (BTRFS_FS_OPEN , &fs_info->flags); +/* Kick the cleaner thread so it'll start deleting snapshots. */ +if (test_bit (BTRFS_FS_UNFINISHED_DROPS , &fs_info->flags)) +wake_up_process (fs_info->cleaner_kthread); + clear_oneshot:btrfs_clear_oneshot_options (fs_info);return 0 ;
@@ -4348,6 +4352,12 @@ void __cold close_ctree(struct btrfs_fs_*/ kthread_park (fs_info->cleaner_kthread); +/* +* If we had UNFINISHED_DROPS we could still be processing them, so +* clear that bit and wake up relocation so it can stop. +*/ +btrfs_wake_unfinished_drop (fs_info); +/* wait for the qgroup rescan worker to stop */ btrfs_qgroup_wait_for_completion (fs_info, false);
fs/btrfs/extent-tree.c +10 -0
@@ -5604,6 +5604,7 @@ int btrfs_drop_snapshot(struct btrfs_rooint ret;int level;bool root_dropped = false; +bool unfinished_drop = false;btrfs_debug (fs_info,"Drop subvolume %llu" , root->root_key.objectid);
@@ -5646,6 +5647,8 @@ int btrfs_drop_snapshot(struct btrfs_roo* already dropped. */ set_bit (BTRFS_ROOT_DELETING , &root->state); + unfinished_drop =test_bit (BTRFS_ROOT_UNFINISHED_DROP , &root->state); +if (btrfs_disk_key_objectid (&root_item->drop_progress) ==0 ) { level =btrfs_header_level (root->node); path->nodes[level] =btrfs_lock_root_node (root);
@@ -5821,6 +5824,13 @@ out_free:btrfs_free_path (path); out:/* +* We were an unfinished drop root, check to see if there are any +* pending, and if not clear and wake up any waiters. +*/ +if (!err && unfinished_drop) +btrfs_maybe_wake_unfinished_drop (fs_info); + +/* * So if we need to stop dropping the snapshot for whatever reason we * need to make sure to add it back to the dead root list so that we * keep trying to do the work later. This also cleans up roots if we
fs/btrfs/relocation.c +13 -0
@@ -3971,6 +3971,19 @@ int btrfs_relocate_block_group(struct btint rw =0 ;int err =0 ; +/* +* This only gets set if we had a half-deleted snapshot on mount. We +* cannot allow relocation to start while we're still trying to clean up +* these pending deletions. +*/ + ret =wait_on_bit (&fs_info->flags,BTRFS_FS_UNFINISHED_DROPS ,TASK_INTERRUPTIBLE ); +if (ret) +return ret; + +/* We may have been woken up by close_ctree, so bail if we're closing. */ +if (btrfs_fs_closing (fs_info)) +return -EINTR ; + bg =btrfs_lookup_block_group (fs_info, group_start);if (!bg)return -ENOENT ;
fs/btrfs/root-tree.c +15 -0
@@ -278,6 +278,21 @@ int btrfs_find_orphan_roots(struct btrfsWARN_ON (!test_bit (BTRFS_ROOT_ORPHAN_ITEM_INSERTED , &root->state));if (btrfs_root_refs (&root->root_item) ==0 ) { +struct btrfs_key drop_key; + +btrfs_disk_key_to_cpu (&drop_key, &root->root_item.drop_progress); +/* +* If we have a non-zero drop_progress then we know we +* made it partly through deleting this snapshot, and +* thus we need to make sure we block any balance from +* happening until this snapshot is completely dropped. +*/ +if (drop_key.objectid !=0 || drop_key.type !=0 || + drop_key.offset !=0 ) { +set_bit (BTRFS_FS_UNFINISHED_DROPS , &fs_info->flags); +set_bit (BTRFS_ROOT_UNFINISHED_DROP , &root->state); + } +set_bit (BTRFS_ROOT_DEAD_TREE , &root->state);btrfs_add_dead_root (root); }
fs/btrfs/transaction.c +32 -1
@@ -1340,6 +1340,32 @@ again: }/* +* If we had a pending drop we need to see if there are any others left in our +* dead roots list, and if not clear our bit and wake any waiters. +*/ +void btrfs_maybe_wake_unfinished_drop (struct btrfs_fs_info *fs_info) +{ +/* +* We put the drop in progress roots at the front of the list, so if the +* first entry doesn't have UNFINISHED_DROP set we can wake everybody +* up. +*/ +spin_lock (&fs_info->trans_lock); +if (!list_empty (&fs_info->dead_roots)) { +struct btrfs_root *root =list_first_entry (&fs_info->dead_roots, + struct btrfs_root, + root_list); +if (test_bit (BTRFS_ROOT_UNFINISHED_DROP , &root->state)) { +spin_unlock (&fs_info->trans_lock); +return ; + } + } +spin_unlock (&fs_info->trans_lock); + +btrfs_wake_unfinished_drop (fs_info); +} + +/* * dead roots are old snapshots that need to be deleted. This allocates * a dirty root struct and adds it into the list of dead roots that need to * be deleted
@@ -1351,7 +1377,12 @@ void btrfs_add_dead_root(struct btrfs_rospin_lock (&fs_info->trans_lock);if (list_empty (&root->root_list)) {btrfs_grab_root (root); -list_add_tail (&root->root_list, &fs_info->dead_roots); + +/* We want to process the partially complete drops first. */ +if (test_bit (BTRFS_ROOT_UNFINISHED_DROP , &root->state)) +list_add (&root->root_list, &fs_info->dead_roots); +else +list_add_tail (&root->root_list, &fs_info->dead_roots); }spin_unlock (&fs_info->trans_lock); }
fs/btrfs/transaction.h +1 -0
@@ -217,6 +217,7 @@ int btrfs_wait_for_commit(struct btrfs_fvoid btrfs_add_dead_root (struct btrfs_root *root);int btrfs_defrag_root (struct btrfs_root *root); +void btrfs_maybe_wake_unfinished_drop (struct btrfs_fs_info *fs_info);int btrfs_clean_one_deleted_snapshot (struct btrfs_root *root);int btrfs_commit_transaction (struct btrfs_trans_handle *trans);int btrfs_commit_transaction_async (struct btrfs_trans_handle *trans);
Greg Kroah-Hartman
maintainerPATCH 181/186 Revert "xfrm: xfrm_state_mtu should return at least 1280 for ipv6"
In reply to Greg Kroah-HartmanFrom: Jiri Bohac <[email protected]>
commit a6d95c5a628a09be129f25d5663a7e9db8261f51 upstream.
This reverts commit b515d2637276a3810d6595e10ab02c13bfd0b63a.
Commit b515d2637276a3810d6595e10ab02c13bfd0b63a ("xfrm: xfrm_state_mtu should return at least 1280 for ipv6") in v5.14 breaks the TCP MSS calculation in ipsec transport mode, resulting complete stalls of TCP connections. This happens when the (P)MTU is 1280 or slighly larger.
The desired formula for the MSS is:
MSS = (MTU - ESP_overhead) - IP header - TCP header
However, the above commit clamps the (MTU - ESP_overhead) to a minimum of 1280, turning the formula into
MSS = max(MTU - ESP overhead, 1280) - IP header - TCP header
With the (P)MTU near 1280, the calculated MSS is too large and the resulting TCP packets never make it to the destination because they are over the actual PMTU.
The above commit also causes suboptimal double fragmentation in xfrm tunnel mode, as described in
https://lore.kernel.org/netdev/[email protected]/
The original problem the above commit was trying to fix is now fixed by commit 6596a0229541270fb8d38d989f91b78838e5e9da ("xfrm: fix MTU regression").
--- include/net/xfrm.h | 1 - net/ipv4/esp4.c | 2 +- net/ipv6/esp6.c | 2 +- net/xfrm/xfrm_state.c | 14 ++------------ 4 files changed, 4 insertions(+), 15 deletions(-)
include/net/xfrm.h +0 -1
@@ -1567,7 +1567,6 @@ void xfrm_sad_getinfo(struct net *net, svoid xfrm_spd_getinfo (struct net *net,struct xfrmk_spdinfo *si);u32 xfrm_replay_seqhi (struct xfrm_state *x,__be32 net_seq);int xfrm_init_replay (struct xfrm_state *x); -u32 __xfrm_state_mtu (struct xfrm_state *x,int mtu);u32 xfrm_state_mtu (struct xfrm_state *x,int mtu);int __xfrm_init_state (struct xfrm_state *x,bool init_replay,bool offload);int xfrm_init_state (struct xfrm_state *x);
net/ipv4/esp4.c +1 -1
@@ -671,7 +671,7 @@ static int esp_output(struct xfrm_statestruct xfrm_dst *dst = (struct xfrm_dst *)skb_dst (skb);u32 padto; - padto =min (x->tfcpad,__xfrm_state_mtu (x, dst->child_mtu_cached)); + padto =min (x->tfcpad,xfrm_state_mtu (x, dst->child_mtu_cached));if (skb->len < padto) esp.tfclen = padto - skb->len; }
net/ipv6/esp6.c +1 -1
@@ -708,7 +708,7 @@ static int esp6_output(struct xfrm_statestruct xfrm_dst *dst = (struct xfrm_dst *)skb_dst (skb);u32 padto; - padto =min (x->tfcpad,__xfrm_state_mtu (x, dst->child_mtu_cached)); + padto =min (x->tfcpad,xfrm_state_mtu (x, dst->child_mtu_cached));if (skb->len < padto) esp.tfclen = padto - skb->len; }
net/xfrm/xfrm_state.c +2 -12
@@ -2571,7 +2571,7 @@ void xfrm_state_delete_tunnel(struct xfr }EXPORT_SYMBOL (xfrm_state_delete_tunnel); -u32 __xfrm_state_mtu (struct xfrm_state *x,int mtu) +u32 xfrm_state_mtu (struct xfrm_state *x,int mtu) {const struct xfrm_type *type =READ_ONCE (x->type);struct crypto_aead *aead;
@@ -2602,17 +2602,7 @@ u32 __xfrm_state_mtu(struct xfrm_state *return ((mtu - x->props.header_len -crypto_aead_authsize (aead) - net_adj) & ~(blksize -1 )) + net_adj -2 ; } -EXPORT_SYMBOL_GPL (__xfrm_state_mtu); - -u32 xfrm_state_mtu (struct xfrm_state *x,int mtu) -{ - mtu =__xfrm_state_mtu (x, mtu); - -if (x->props.family ==AF_INET6 && mtu <IPV6_MIN_MTU ) -return IPV6_MIN_MTU ; - -return mtu; -} +EXPORT_SYMBOL_GPL (xfrm_state_mtu);int __xfrm_init_state (struct xfrm_state *x,bool init_replay,bool offload) {
Greg Kroah-Hartman
maintainerPATCH 182/186 proc: fix documentation and description of pagemap
In reply to Greg Kroah-HartmanFrom: Yun Zhou <[email protected]>
commit dd21bfa425c098b95ca86845f8e7d1ec1ddf6e4a upstream.
Since bit 57 was exported for uffd-wp write-protected (commitfb8e37f35a2f: "mm/pagemap: export uffd-wp protection information"), fixing it can reduce some unnecessary confusion.
--- Documentation/admin-guide/mm/pagemap.rst | 2 +- fs/proc/task_mmu.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
Documentation/admin-guide/mm/pagemap.rst +1 -1
@@ -23,7 +23,7 @@ There are four components to pagemap: * Bit 56 page exclusively mapped (since 4.2) * Bit 57 pte is uffd-wp write-protected (since 5.13) (see :ref:`Documentation/admin-guide/mm/userfaultfd.rst <userfaultfd>`) - * Bits 57-60 zero + * Bits 58-60 zero * Bit 61 page is file-page or shared-anon (since 3.5) * Bit 62 page swapped * Bit 63 page present
fs/proc/task_mmu.c +2 -1
@@ -1586,7 +1586,8 @@ static const struct mm_walk_ops pagemap_* Bits 5-54 swap offset if swapped * Bit 55 pte is soft-dirty (see Documentation/admin-guide/mm/soft-dirty.rst) * Bit 56 page exclusively mapped -* Bits 57-60 zero +* Bit 57 pte is uffd-wp write-protected +* Bits 58-60 zero * Bit 61 page is file-page or shared-anon * Bit 62 page swapped * Bit 63 page present
Greg Kroah-Hartman
maintainerPATCH 183/186 x86/kvmclock: Fix Hyper-V Isolated VMs boot issue when vCPUs > 64
In reply to Greg Kroah-HartmanFrom: Dexuan Cui <[email protected]>
commit 92e68cc558774de01024c18e8b35cdce4731c910 upstream.
When Linux runs as an Isolated VM on Hyper-V, it supports AMD SEV-SNP but it's partially enlightened, i.e. cc_platform_has( CC_ATTR_GUEST_MEM_ENCRYPT) is true but sev_active() is false.
Commit 4d96f9109109 per se is good, but with it now kvm_setup_vsyscall_timeinfo() -> kvmclock_init_mem() calls set_memory_decrypted(), and later gets stuck when trying to zere out the pages pointed by 'hvclock_mem', if Linux runs as an Isolated VM on Hyper-V. The cause is that here now the Linux VM should no longer access the original guest physical addrss (GPA); instead the VM should do memremap() and access the original GPA + ms_hyperv.shared_gpa_boundary:
see the example code in drivers/hv/connection.c: vmbus_connect() or drivers/hv/ring_buffer.c: hv_ringbuffer_init(). If the VM tries to access the original GPA, it keepts getting injected a fault by Hyper-V and gets stuck there.
Here the issue happens only when the VM has >=65 vCPUs, because the global static array hv_clock_boot[] can hold 64 "struct pvclock_vsyscall_time_info" (the sizeof of the struct is 64 bytes), so kvmclock_init_mem() only allocates memory in the case of vCPUs > 64.
Since the 'hvclock_mem' pages are only useful when the kvm clock is supported by the underlying hypervisor, fix the issue by returning early when Linux VM runs on Hyper-V, which doesn't support kvm clock.
Message-Id: <[email protected]>
--- arch/x86/kernel/kvmclock.c | 3 +++ 1 file changed, 3 insertions(+)
arch/x86/kernel/kvmclock.c +3 -0
@@ -239,6 +239,9 @@ static void __init kvmclock_init_mem(voistatic int __init kvm_setup_vsyscall_timeinfo (void ) { +if (!kvm_para_available ()) +return 0 ; +kvmclock_init_mem ();#ifdef CONFIG_X86_64
Greg Kroah-Hartman
maintainerPATCH 184/186 s390/ftrace: fix arch_ftrace_get_regs implementation
In reply to Greg Kroah-HartmanFrom: Heiko Carstens <[email protected]>
commit 1389f17937a03fe4ec71b094e1aa6530a901963e upstream.
arch_ftrace_get_regs is supposed to return a struct pt_regs pointer only if the pt_regs structure contains all register contents, which means it must have been populated when created via ftrace_regs_caller.
If it was populated via ftrace_caller the contents are not complete (the psw mask part is missing), and therefore a NULL pointer needs be returned.
The current code incorrectly always returns a struct pt_regs pointer.
Fix this by adding another pt_regs flag which indicates if the contents are complete, and fix arch_ftrace_get_regs accordingly.
--- arch/s390/include/asm/ftrace.h | 10 ++++++---- arch/s390/include/asm/ptrace.h | 2 ++ arch/s390/kernel/ftrace.c | 2 +- arch/s390/kernel/mcount.S | 9 +++++++++ 4 files changed, 18 insertions(+), 5 deletions(-)
arch/s390/include/asm/ftrace.h +6 -4
@@ -47,15 +47,17 @@ struct ftrace_regs { static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs) { -return &fregs->regs; +struct pt_regs *regs = &fregs->regs; + +if (test_pt_regs_flag (regs,PIF_FTRACE_FULL_REGS )) +return regs; +return NULL ; } static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *fregs,unsigned long ip) { -struct pt_regs *regs =arch_ftrace_get_regs (fregs); - - regs->psw.addr = ip; + fregs->regs.psw.addr = ip; }/*
arch/s390/include/asm/ptrace.h +2 -0
@@ -15,11 +15,13 @@#define PIF_EXECVE_PGSTE_RESTART 1/* restart execve for PGSTE binaries */ #define PIF_SYSCALL_RET_SET 2/* return value was set via ptrace */ #define PIF_GUEST_FAULT 3/* indicates program check in sie64a */ +#define PIF_FTRACE_FULL_REGS 4/* all register contents valid (ftrace) */ #define _PIF_SYSCALL BIT(PIF_SYSCALL)#define _PIF_EXECVE_PGSTE_RESTART BIT(PIF_EXECVE_PGSTE_RESTART)#define _PIF_SYSCALL_RET_SET BIT(PIF_SYSCALL_RET_SET)#define _PIF_GUEST_FAULT BIT(PIF_GUEST_FAULT) +#define _PIF_FTRACE_FULL_REGS BIT(PIF_FTRACE_FULL_REGS)#ifndef __ASSEMBLY__
arch/s390/kernel/ftrace.c +1 -1
@@ -291,7 +291,7 @@ void kprobe_ftrace_handler(unsigned long regs =ftrace_get_regs (fregs); p =get_kprobe ((kprobe_opcode_t *)ip); -if (unlikely (!p) ||kprobe_disabled (p)) +if (!regs ||unlikely (!p) ||kprobe_disabled (p)) goto out;if (kprobe_running ()) {
arch/s390/kernel/mcount.S +9 -0
@@ -27,6 +27,7 @@ ENDPROC(ftrace_stub) #define STACK_PTREGS_GPRS (STACK_PTREGS + __PT_GPRS) #define STACK_PTREGS_PSW (STACK_PTREGS + __PT_PSW) #define STACK_PTREGS_ORIG_GPR2 (STACK_PTREGS + __PT_ORIG_GPR2) +#define STACK_PTREGS_FLAGS (STACK_PTREGS + __PT_FLAGS) #ifdef __PACK_STACK /* allocate just enough for r14, r15 and backchain */ #define TRACED_FUNC_FRAME_SIZE 24
@@ -57,6 +58,14 @@ ENDPROC(ftrace_stub) .if \allregs == 1 stg %r14,(STACK_PTREGS_PSW)(%r15) stosm (STACK_PTREGS_PSW)(%r15),0 +#ifdef CONFIG_HAVE_MARCH_Z10_FEATURES + mvghi STACK_PTREGS_FLAGS(%r15),_PIF_FTRACE_FULL_REGS +#else + lghi %r14,_PIF_FTRACE_FULL_REGS + stg %r14,STACK_PTREGS_FLAGS(%r15) +#endif + .else + xc STACK_PTREGS_FLAGS(8,%r15),STACK_PTREGS_FLAGS(%r15) .endif lg %r14,(__SF_GPRS+8*8)(%r1) # restore original return address
Greg Kroah-Hartman
maintainerPATCH 185/186 s390/ftrace: fix ftrace_caller/ftrace_regs_caller generation
In reply to Greg Kroah-HartmanFrom: Heiko Carstens <[email protected]>
commit 9fa881f7e3c74ce6626d166bca9397e5d925937f upstream.
ftrace_caller was used for both ftrace_caller and ftrace_regs_caller, which means that the target address of the hotpatch trampoline was never updated.
With commit 894979689d3a ("s390/ftrace: provide separate ftrace_caller/ftrace_regs_caller implementations") a separate ftrace_regs_caller entry point was implemeted, however it was forgotten to implement the necessary changes for ftrace_modify_call and ftrace_make_call, where the branch target has to be modified accordingly.
Therefore add the missing code now.
--- arch/s390/kernel/ftrace.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
arch/s390/kernel/ftrace.c +35 -0
@@ -159,9 +159,38 @@ int ftrace_init_nop(struct module *mod,return 0 ; } +static struct ftrace_hotpatch_trampoline *ftrace_get_trampoline (struct dyn_ftrace *rec) +{ +struct ftrace_hotpatch_trampoline *trampoline; +struct ftrace_insn insn; +s64 disp; +u16 opc; + +if (copy_from_kernel_nofault (&insn, (void *)rec->ip,sizeof (insn))) +return ERR_PTR (-EFAULT ); + disp = (s64 )insn.disp *2 ; + trampoline = (void *)(rec->ip + disp); +if (get_kernel_nofault (opc, &trampoline->brasl_opc)) +return ERR_PTR (-EFAULT ); +if (opc !=0xc015 ) +return ERR_PTR (-EINVAL ); +return trampoline; +} +int ftrace_modify_call (struct dyn_ftrace *rec,unsigned long old_addr,unsigned long addr) { +struct ftrace_hotpatch_trampoline *trampoline; +u64 old; + + trampoline =ftrace_get_trampoline (rec); +if (IS_ERR (trampoline)) +return PTR_ERR (trampoline); +if (get_kernel_nofault (old, &trampoline->interceptor)) +return -EFAULT ; +if (old != old_addr) +return -EINVAL ; +s390_kernel_write (&trampoline->interceptor, &addr,sizeof (addr));return 0 ; }
@@ -188,6 +217,12 @@ static void brcl_enable(void *brcl)int ftrace_make_call (struct dyn_ftrace *rec,unsigned long addr) { +struct ftrace_hotpatch_trampoline *trampoline; + + trampoline =ftrace_get_trampoline (rec); +if (IS_ERR (trampoline)) +return PTR_ERR (trampoline); +s390_kernel_write (&trampoline->interceptor, &addr,sizeof (addr));brcl_enable ((void *)rec->ip);return 0 ; }
Greg Kroah-Hartman
maintainerPATCH 186/186 KVM: x86/mmu: Passing up the error state of mmu_alloc_shadow_roots()
In reply to Greg Kroah-HartmanFrom: Like Xu <[email protected]>
commit c6c937d673aaa1d603f62f134e1ca9c173eeeed3 upstream.
Just like on the optional mmu_alloc_direct_roots() path, once shadow path reaches "r = -EIO" somewhere, the caller needs to know the actual state in order to enter error handling and avoid something worse.
Message-Id: <[email protected]>
--- arch/x86/kvm/mmu/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kvm/mmu/mmu.c +1 -1
@@ -3573,7 +3573,7 @@ set_root_pgd: out_unlock:write_unlock (&vcpu->kvm->mmu_lock); -return 0 ; +return r; }static int mmu_alloc_special_roots (struct kvm_vcpu *vcpu)
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.16.13 release. There are 186 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 09 Mar 2022 09:16:25 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 9 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.16.13-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.16.y
and the diffstat can be found below.thanks,
greg k-h
5.16.13-rc1 Successfully Compiled and booted on my Raspberry PI 4b (8g) (bcm2711)