Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220915043527.737133-1-venkatch@gmail.com> <238a21a8-c97b-5a38-6c08-9057055bf73f@nbd.name>
 <CABHL29H75U-0dBP14d-6ds-xMPt1eqrvQPuMp3oY8qQS8Y3Dbw@mail.gmail.com>
 <9acc4159-8223-bbca-a83f-d075660ac6db@nbd.name> <CABHL29GB6DCrdu4FzRfBDSj9A7pwqnWE+wgH2xfkBMLjT3bAxA@mail.gmail.com>
 <f75ad975-8b76-f3e6-151b-6eda552c0431@nbd.name>
In-Reply-To: <f75ad975-8b76-f3e6-151b-6eda552c0431@nbd.name>
From:   Venkat Ch <venkatch@gmail.com>
Date:   Wed, 21 Sep 2022 00:53:27 +0530
Message-ID: <CABHL29GUgta4Ke3sxbongMSK__cruTaDYCXe1-yEDL3WA3mO9Q@mail.gmail.com>
Subject: Re: [PATCH v2] wifi: mac80211: Fix performance issue with mutex_lock
To:     Felix Fietkau <nbd@nbd.name>
Cc:     johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
        Venkat Chimata <venkata@shasta.cloud>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Felix,

 Following is the background of the problem, how I traced to
mutex_lock and why I propose rcu locks.

Issue:
 On a 10Mbps upload / 50 Mbps download connection, the following issue reported.

Video periodically freezes and/or appears delayed when on Zoom or Teams.
1. Video will freeze for 10 or 15 seconds periodically when on a call,
but audio continues and the session doesn't drop.
2. The video  still works but it appears delay (I move, but the video
of my movement is noticeably delay by a second or so)

Tracing to mutex_lock(sta_mutex):

 When I investigated, I found that the ucentral agent in openwifi
fetches the station list periodically.  Without the station list
fetch, the video quality is just fine. I investigated the station list
path and found this mutex_lock. I also see that earlier it was
rcu_lock which protected the station list in this path. In this
commit, https://github.com/torvalds/linux/commit/66572cfc30a4b764150c83ee5d842a3ce17991c9,
rcu lock was changed to mutex lock without providing any reason.

I also saw this comment just above the sta_mutex declaration.
        /* Station data */
        /*
         * The mutex only protects the list, hash table and
         * counter, reads are done with RCU.
         */
        struct mutex sta_mtx;

So I reverted back the mutex_lock with rcu_lock and it just worked
fine. We tested for more than 2 weeks before concluding this analysis.

I think the usage of mutex_lock is impacting the tx / rx path
somewhere and hence the issue. It is a challenge to trace the exact
function though.

Thanks & Regards
Venkat

On Wed, 21 Sept 2022 at 00:21, Felix Fietkau <nbd@nbd.name> wrote:
>
>
> On 20.09.22 20:27, Venkat Ch wrote:
> > Hi Felix,
> >
> >   Thank you. I browsed through the code. There seems to be sleep
> > operations wcn36xx platform.ath11k  does n't seem to have the sleep
> > operations in sta_statistics. We are using ath11k based chipset. Will
> > it impact things if we apply this patch for ath11k boards only as a
> > platform specific patch?
> I think it's a bad idea to keep this hack without understanding why it
> helps at all.
>
> - Felix


-- 
If you rest, you rust