diff mbox series

[net-next,2/7] net: ipa: synchronize NAPI only for suspend

Message ID 20210203152855.11866-3-elder@linaro.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: ipa: a mix of small improvements | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 25 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Alex Elder Feb. 3, 2021, 3:28 p.m. UTC
When stopping a channel, gsi_channel_stop() will ensure NAPI
polling is complete when it calls napi_disable().  So there is no
need to call napi_synchronize() in that case.

Move the call to napi_synchronize() out of __gsi_channel_stop()
and into gsi_channel_suspend(), so it's only used where needed.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Jakub Kicinski Feb. 5, 2021, 4:53 a.m. UTC | #1
On Wed,  3 Feb 2021 09:28:50 -0600 Alex Elder wrote:
>  int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop)
>  {
>  	struct gsi_channel *channel = &gsi->channel[channel_id];
> +	int ret;
>  
> -	return __gsi_channel_stop(channel, stop);
> +	/* Synchronize NAPI if successful, to ensure polling has finished. */
> +	ret = __gsi_channel_stop(channel, stop);
> +	if (!ret)
> +		napi_synchronize(&channel->napi);
> +
> +	return ret;

nit:

	ret = function();
	if (ret)
		return ret;

	/* success path: do something else */

	return 0;
Alex Elder Feb. 5, 2021, 1:39 p.m. UTC | #2
On 2/4/21 10:53 PM, Jakub Kicinski wrote:
> On Wed,  3 Feb 2021 09:28:50 -0600 Alex Elder wrote:
>>  int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop)
>>  {
>>  	struct gsi_channel *channel = &gsi->channel[channel_id];
>> +	int ret;
>>  
>> -	return __gsi_channel_stop(channel, stop);
>> +	/* Synchronize NAPI if successful, to ensure polling has finished. */
>> +	ret = __gsi_channel_stop(channel, stop);
>> +	if (!ret)
>> +		napi_synchronize(&channel->napi);
>> +
>> +	return ret;
> 
> nit:
> 
> 	ret = function();
> 	if (ret)
> 		return ret;
> 
> 	/* success path: do something else */
> 
> 	return 0;

No problem, I'm happy with it the way you suggest.  I will
update in v2.   Thank you.

					-Alex
diff mbox series

Patch

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 2671b76ebcfe3..420d0f3bfae9a 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -939,10 +939,6 @@  static int __gsi_channel_stop(struct gsi_channel *channel, bool stop)
 		mutex_unlock(&gsi->mutex);
 	}
 
-	/* Finally, ensure NAPI polling has finished. */
-	if (!ret)
-		napi_synchronize(&channel->napi);
-
 	return ret;
 }
 
@@ -984,8 +980,14 @@  void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool doorbell)
 int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
+	int ret;
 
-	return __gsi_channel_stop(channel, stop);
+	/* Synchronize NAPI if successful, to ensure polling has finished. */
+	ret = __gsi_channel_stop(channel, stop);
+	if (!ret)
+		napi_synchronize(&channel->napi);
+
+	return ret;
 }
 
 /* Resume a suspended channel (starting will be requested if STOPPED) */