| Message ID | 20220117115440.60296-28-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | IEEE 802.15.4 scan support | expand |
Hi, On Mon, 17 Jan 2022 at 06:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote: ... > > /* stop hardware - this must stop RX */ > diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h > index 0291e49058f2..37d5438fdb3f 100644 > --- a/net/mac802154/ieee802154_i.h > +++ b/net/mac802154/ieee802154_i.h > @@ -122,6 +122,7 @@ extern struct ieee802154_mlme_ops mac802154_mlme_wpan; > > void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb); > void ieee802154_xmit_sync_worker(struct work_struct *work); > +void ieee802154_sync_tx(struct ieee802154_local *local); > netdev_tx_t > ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev); > netdev_tx_t > diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c > index de5ecda80472..d1fd2cc67cbe 100644 > --- a/net/mac802154/tx.c > +++ b/net/mac802154/tx.c > @@ -48,6 +48,7 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) > > kfree_skb(skb); > atomic_dec(&local->phy->ongoing_txs); > + wake_up(&local->phy->sync_txq); if (atomic_dec_and_test(&hw->phy->ongoing_txs)) wake_up(&hw->phy->sync_txq); > netdev_dbg(dev, "transmission failed\n"); > } > > @@ -117,6 +118,11 @@ ieee802154_hot_tx(struct ieee802154_local *local, struct sk_buff *skb) > return ieee802154_tx(local, skb); > } > > +void ieee802154_sync_tx(struct ieee802154_local *local) > +{ > + wait_event(local->phy->sync_txq, !atomic_read(&local->phy->ongoing_txs)); > +} > + > netdev_tx_t > ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev) > { > diff --git a/net/mac802154/util.c b/net/mac802154/util.c > index db2ac53b937e..230fe3390df7 100644 > --- a/net/mac802154/util.c > +++ b/net/mac802154/util.c > @@ -90,6 +90,7 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb, > after_wakeup: > dev_consume_skb_any(skb); > atomic_dec(&hw->phy->ongoing_txs); > + wake_up(&hw->phy->sync_txq); if (atomic_dec_and_test(&hw->phy->ongoing_txs)) wake_up(&hw->phy->sync_txq); - Alex
Hi Alexander, alex.aring@gmail.com wrote on Mon, 17 Jan 2022 17:43:49 -0500: > Hi, > > On Mon, 17 Jan 2022 at 06:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > ... > > > > /* stop hardware - this must stop RX */ > > diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h > > index 0291e49058f2..37d5438fdb3f 100644 > > --- a/net/mac802154/ieee802154_i.h > > +++ b/net/mac802154/ieee802154_i.h > > @@ -122,6 +122,7 @@ extern struct ieee802154_mlme_ops mac802154_mlme_wpan; > > > > void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb); > > void ieee802154_xmit_sync_worker(struct work_struct *work); > > +void ieee802154_sync_tx(struct ieee802154_local *local); > > netdev_tx_t > > ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev); > > netdev_tx_t > > diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c > > index de5ecda80472..d1fd2cc67cbe 100644 > > --- a/net/mac802154/tx.c > > +++ b/net/mac802154/tx.c > > @@ -48,6 +48,7 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) > > > > kfree_skb(skb); > > atomic_dec(&local->phy->ongoing_txs); > > + wake_up(&local->phy->sync_txq); > > if (atomic_dec_and_test(&hw->phy->ongoing_txs)) > wake_up(&hw->phy->sync_txq); As we test this condition in the waiting path I assumed it was fine to do it this way, but the additional check does not hurt, so I'll add it. Thanks, Miquèl
Hi, On Tue, 18 Jan 2022 at 13:14, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > alex.aring@gmail.com wrote on Mon, 17 Jan 2022 17:43:49 -0500: > > > Hi, > > > > On Mon, 17 Jan 2022 at 06:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > ... > > > > > > /* stop hardware - this must stop RX */ > > > diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h > > > index 0291e49058f2..37d5438fdb3f 100644 > > > --- a/net/mac802154/ieee802154_i.h > > > +++ b/net/mac802154/ieee802154_i.h > > > @@ -122,6 +122,7 @@ extern struct ieee802154_mlme_ops mac802154_mlme_wpan; > > > > > > void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb); > > > void ieee802154_xmit_sync_worker(struct work_struct *work); > > > +void ieee802154_sync_tx(struct ieee802154_local *local); > > > netdev_tx_t > > > ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev); > > > netdev_tx_t > > > diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c > > > index de5ecda80472..d1fd2cc67cbe 100644 > > > --- a/net/mac802154/tx.c > > > +++ b/net/mac802154/tx.c > > > @@ -48,6 +48,7 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) > > > > > > kfree_skb(skb); > > > atomic_dec(&local->phy->ongoing_txs); > > > + wake_up(&local->phy->sync_txq); > > > > if (atomic_dec_and_test(&hw->phy->ongoing_txs)) > > wake_up(&hw->phy->sync_txq); > > As we test this condition in the waiting path I assumed it was fine to > do it this way, but the additional check does not hurt, so I'll add it. it's just a nitpick... to avoid scheduling and this triggers a checking if the condition is true in cases we know it can't.... although we can talk about this because ongoing_txs should never be higher than 1... so any atomic_dec() should make the condition true... - Alex
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index 56aa672e1912..0848896120fa 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -286,6 +286,7 @@ struct wpan_phy { /* Transmission monitoring and control */ atomic_t ongoing_txs; atomic_t hold_txs; + wait_queue_head_t sync_txq; char priv[] __aligned(NETDEV_ALIGN); }; diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c index de259b5170ab..0953cacafbff 100644 --- a/net/ieee802154/core.c +++ b/net/ieee802154/core.c @@ -129,6 +129,7 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size) wpan_phy_net_set(&rdev->wpan_phy, &init_net); init_waitqueue_head(&rdev->dev_wait); + init_waitqueue_head(&rdev->wpan_phy.sync_txq); return &rdev->wpan_phy; } diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c index e8aabf215286..e2900c9b788c 100644 --- a/net/mac802154/cfg.c +++ b/net/mac802154/cfg.c @@ -48,6 +48,7 @@ static int ieee802154_suspend(struct wpan_phy *wpan_phy) atomic_inc(&wpan_phy->hold_txs); ieee802154_stop_queue(&local->hw); + ieee802154_sync_tx(local); synchronize_net(); /* stop hardware - this must stop RX */ diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index 0291e49058f2..37d5438fdb3f 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -122,6 +122,7 @@ extern struct ieee802154_mlme_ops mac802154_mlme_wpan; void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb); void ieee802154_xmit_sync_worker(struct work_struct *work); +void ieee802154_sync_tx(struct ieee802154_local *local); netdev_tx_t ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev); netdev_tx_t diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index de5ecda80472..d1fd2cc67cbe 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -48,6 +48,7 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) kfree_skb(skb); atomic_dec(&local->phy->ongoing_txs); + wake_up(&local->phy->sync_txq); netdev_dbg(dev, "transmission failed\n"); } @@ -117,6 +118,11 @@ ieee802154_hot_tx(struct ieee802154_local *local, struct sk_buff *skb) return ieee802154_tx(local, skb); } +void ieee802154_sync_tx(struct ieee802154_local *local) +{ + wait_event(local->phy->sync_txq, !atomic_read(&local->phy->ongoing_txs)); +} + netdev_tx_t ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev) { diff --git a/net/mac802154/util.c b/net/mac802154/util.c index db2ac53b937e..230fe3390df7 100644 --- a/net/mac802154/util.c +++ b/net/mac802154/util.c @@ -90,6 +90,7 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb, after_wakeup: dev_consume_skb_any(skb); atomic_dec(&hw->phy->ongoing_txs); + wake_up(&hw->phy->sync_txq); } EXPORT_SYMBOL(ieee802154_xmit_complete);
Right now we are able to stop a queue but we have no indication if a transmission is ongoing or not. We recently introduced an ongoing tx count variable so let's use it to wake up a queue. Waiters on the queue will be woken up once all the ongoing transmissions are over. Thanks to this feature, we will soon be able to introduce a synchronous API. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- include/net/cfg802154.h | 1 + net/ieee802154/core.c | 1 + net/mac802154/cfg.c | 1 + net/mac802154/ieee802154_i.h | 1 + net/mac802154/tx.c | 6 ++++++ net/mac802154/util.c | 1 + 6 files changed, 11 insertions(+)