[PATCH v6 08/11] wifi: ath12k: allow specific mgmt frame tx while vdev is not up

[Kang Yang]

On 2/7/2024 12:24 AM, Kalle Valo wrote:
> Kang Yang <quic_kangyang at quicinc.com> writes:
> 
>> In current code, the management frames must be sent after vdev is started.
>> But for P2P device, vdev won't start until P2P negotiation is done. So
>> this logic doesn't make sense for P2P device.
>>
>> Also use ar->conf_mutex to synchronize ar to avoid potential conflicts.
> 
> Please do locking changes in a separate followup patch, I removed this
> in the pending branch:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=d357dcb3cd0cd3bf57064dc673b5477d884454b3
> 
> I assume you are referring to ar->allocated_vdev_map and access to that
> indeed doesn't look consistent. Most of the places take conf->mutex but
> I see some places which it's accessed without the mutex, for example
> ath12k_mac_get_arvif_by_vdev_id() and ath12k_mac_get_ar_by_vdev_id().
> 


Hi, kalle:

I just take a look for ath12k_mac_get_arvif_by_vdev_id() and 
ath12k_mac_get_ar_by_vdev_id.

Both of them use rcu_read_lock(), so we don't need mutex_lock() anymore, 
right?

I also tried to add mutex_lock() for them, cannot work well:
[ 7804.291286] BUG: sleeping function called from invalid context at 
kernel/locking/mutex.c:585
[ 7804.291349] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 0, 
name: swapper/8
[ 7804.291358] preempt_count: 101, expected: 0
[ 7804.291366] RCU nest depth: 1, expected: 0
[ 7804.291374] 1 lock held by swapper/8/0:
……


> I recommend in the followup patch checking all the access to
> ar->allocated_vdev_map, fixing that if needed and adding documentation
> to struct ath12k::allocated_vdev_map how it's supposed to be protected.
> 


So i think the initial change is sufficient: just use mutex_lock() in
ath12k_mgmt_over_wmi_tx_work().