[RFTv2 3/5] ath10k: rework peer accounting

[Kalle Valo]

Michal Kazior <michal.kazior at tieto.com> writes:

> It was troublesome to iterate over peers and
> perform sleepable calls. This will be necessary
> for some upcomming changes to tx flushing.
>
> Make peer allocation and initial setup
> protected by both ar->conf_mutex and
> ar->data_lock. This way it's possible to iterate
> over peers with conf_mutex and call sleepable
> functions.
>
> Signed-off-by: Michal Kazior <michal.kazior at tieto.com>

First comments, but I need to read this much more carefully still:

> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -199,9 +199,14 @@ struct ath10k_dfs_stats {
>  #define ATH10K_MAX_NUM_PEER_IDS (1 << 11) /* htt rx_desc limit */
>  
>  struct ath10k_peer {
> +	/* protected by conf_mutex + data_lock */
>  	struct list_head list;

This really needs a lot more documentation. And besides, don't we
actually want to protect struct ath10k::peers, not this?

> +/* hold conf_mutex for simple iteration, or conf_mutex+data_lock for
> + * modifications */
>  struct ath10k_peer *ath10k_peer_find(struct ath10k *ar, int vdev_id,
>  				     const u8 *addr)
>  {
>  	struct ath10k_peer *peer;
>  
> -	lockdep_assert_held(&ar->data_lock);
> -
>  	list_for_each_entry(peer, &ar->peers, list) {
>  		if (peer->vdev_id != vdev_id)
>  			continue;

The comment here makes me suspicious. How can we safely iterate the list
if we don't take data_lock? Doesn't it mean that the list can change
while we have conf_mutex?

-- 
Kalle Valo