Michal Kazior <michal.kazior@xxxxxxxxx> 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@xxxxxxxxx>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
