Hi Stefan, Alex, stefan@xxxxxxxxxxxxxxxxxx wrote on Wed, 1 Jun 2022 23:01:51 +0200: > Hello. > > On 01.06.22 05:30, Alexander Aring wrote: > > Hi, > > > > On Thu, May 19, 2022 at 11:06 AM Miquel Raynal > > <miquel.raynal@xxxxxxxxxxx> wrote: > >> > >> Hello, > >> > >> This series brings support for that famous synchronous Tx API for MLME > >> commands. > >> > >> MLME commands will be used during scan operations. In this situation, > >> we need to be sure that all transfers finished and that no transfer > >> will be queued for a short moment. > >> > > > > Acked-by: Alexander Aring <aahringo@xxxxxxxxxx> > > These patches have been applied to the wpan-next tree. Thanks! > > > There will be now functions upstream which will never be used, Stefan > > should wait until they are getting used before sending it to net-next. > > Indeed this can wait until we have a consumer of the functions before pushing this forward to net-next. Pretty sure Miquel is happy to finally move on to other pieces of his puzzle and use them. :-) Next part is coming! In the mean time I've experienced a new lockdep warning: All the netlink commands are executed with the rtnl taken. In my current implementation, when I configure/edit a scan request or a beacon request I take a scan_lock or a beacons_lock, so they may only be taken after the rtnl in this case, which leads to this sequence of events: - the rtnl is taken (by the net core) - the beacon's lock is taken But now in a beacon's work or an active scan work, what happens is: - work gets woken up - the beacon/scan lock is taken - a beacon/beacon-request frame is transmitted - the rtnl lock is taken during this transmission Lockdep then detects a possible circular dependency: [ 490.153387] CPU0 CPU1 [ 490.153391] ---- ---- [ 490.153394] lock(&local->beacons_lock); [ 490.153400] lock(rtnl_mutex); [ 490.153406] lock(&local->beacons_lock); [ 490.153412] lock(rtnl_mutex); So in practice, I always need to have the rtnl lock taken when acquiring these other locks (beacon/scan_lock) which I think is far from optimal. 1# One solution is to drop the beacons/scan locks because they are not useful anymore and simply rely on the rtnl. 2# Another solution would be to change the mlme_tx() implementation to finally not need the rtnl at all. Note that just calling ASSERT_RTNL() makes no difference in 2#, it still means that I always need to acquire the rtnl before acquiring the beacons/scan locks, which greatly reduces their usefulness and leads to solution 1# in the end. IIRC I decided to introduce the rtnl to avoid ->ndo_stop() calls during an MLME transmission. I don't know if it has another use there. If not, we may perhaps get rid of the rtnl in mlme_tx() by really handling the stop calls (but I was too lazy so far to do that). What direction would you advise? Thanks, Miquèl
