diff mbox series

net: fec: Use unlocked timecounter reads for saving state

Message ID 20220829114039.56195-1-csokas.bence@prolan.hu (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: fec: Use unlocked timecounter reads for saving state | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 3 maintainers not CCed: edumazet@google.com qiangqing.zhang@nxp.com pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes fail Problems with Fixes tag: 1
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 88 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Csókás Bence Aug. 29, 2022, 11:40 a.m. UTC 
`fec_ptp_save_state()` may be called from an atomic context,
which makes `fec_ptp_gettime()` unable to acquire a mutex.
Using the lower-level timecounter ops remedies the problem.

Reported-by: Marc Kleine-Budde <mkl@pengutronix.de>
Fixes: f79959220fa5

Signed-off-by: Csókás Bence <csokas.bence@prolan.hu>
---
 drivers/net/ethernet/freescale/fec.h     |  4 ++--
 drivers/net/ethernet/freescale/fec_ptp.c | 13 +++++++------
 2 files changed, 9 insertions(+), 8 deletions(-)

Comments

Richard Cochran Aug. 29, 2022, 2:45 p.m. UTC | #1 
On Mon, Aug 29, 2022 at 01:40:41PM +0200, Csókás Bence wrote:
> `fec_ptp_save_state()` may be called from an atomic context,
> which makes `fec_ptp_gettime()` unable to acquire a mutex.
> Using the lower-level timecounter ops remedies the problem.
> 
> Reported-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Fixes: f79959220fa5

> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
> index 78fb8818d168..fdd22c6ca909 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -636,7 +636,7 @@ void fec_ptp_save_state(struct fec_enet_private *fep)
>  {
>  	u32 atime_inc_corr;
>  
> -	fec_ptp_gettime(&fep->ptp_caps, &fep->ptp_saved_state.ts_phc);
> +	fep->ptp_saved_state.ns_phc = timecounter_read(&fep->tc);

The function, fec_ptp_read, does RMW register access and thus needs
protection against concurrent callers.

So you can't simply avoid mutex acquisition to fix the splat.

Thanks,
Richard
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index b656cda75c92..7bc7ab4b5d3a 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -597,7 +597,7 @@  struct fec_enet_private {
 	unsigned int next_counter;
 
 	struct {
-		struct timespec64 ts_phc;
+		u64 ns_phc;
 		u64 ns_sys;
 		u32 at_corr;
 		u8 at_inc_corr;
@@ -613,7 +613,7 @@  int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr);
 int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr);
 
 void fec_ptp_save_state(struct fec_enet_private *fep);
-int fec_ptp_restore_state(struct fec_enet_private *fep);
+void fec_ptp_restore_state(struct fec_enet_private *fep);
 
 /****************************************************************************/
 #endif /* FEC_H */
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 78fb8818d168..fdd22c6ca909 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -636,7 +636,7 @@  void fec_ptp_save_state(struct fec_enet_private *fep)
 {
 	u32 atime_inc_corr;
 
-	fec_ptp_gettime(&fep->ptp_caps, &fep->ptp_saved_state.ts_phc);
+	fep->ptp_saved_state.ns_phc = timecounter_read(&fep->tc);
 	fep->ptp_saved_state.ns_sys = ktime_get_ns();
 
 	fep->ptp_saved_state.at_corr = readl(fep->hwp + FEC_ATIME_CORR);
@@ -644,16 +644,17 @@  void fec_ptp_save_state(struct fec_enet_private *fep)
 	fep->ptp_saved_state.at_inc_corr = (u8)(atime_inc_corr >> FEC_T_INC_CORR_OFFSET);
 }
 
-int fec_ptp_restore_state(struct fec_enet_private *fep)
+void fec_ptp_restore_state(struct fec_enet_private *fep)
 {
 	u32 atime_inc = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK;
-	u64 ns_sys;
+	u64 ns;
 
 	writel(fep->ptp_saved_state.at_corr, fep->hwp + FEC_ATIME_CORR);
 	atime_inc |= ((u32)fep->ptp_saved_state.at_inc_corr) << FEC_T_INC_CORR_OFFSET;
 	writel(atime_inc, fep->hwp + FEC_ATIME_INC);
 
-	ns_sys = ktime_get_ns() - fep->ptp_saved_state.ns_sys;
-	timespec64_add_ns(&fep->ptp_saved_state.ts_phc, ns_sys);
-	return fec_ptp_settime(&fep->ptp_caps, &fep->ptp_saved_state.ts_phc);
+	ns = ktime_get_ns() - fep->ptp_saved_state.ns_sys + fep->ptp_saved_state.ns_phc;
+
+	writel(ns & fep->cc.mask, fep->hwp + FEC_ATIME);
+	timecounter_init(&fep->tc, &fep->cc, ns);
 }