On 2/7/2024 12:24 AM, Kalle Valo wrote:
Kang Yang <quic_kangyang@xxxxxxxxxxx> 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().
