diff mbox series

[v3,27/41] net: mac802154: Introduce a tx queue flushing mechanism

Message ID 20220117115440.60296-28-miquel.raynal@bootlin.com (mailing list archive)
State Not Applicable
Delegated to: Johannes Berg
Headers show
Series IEEE 802.15.4 scan support | expand

Commit Message

Miquel Raynal Jan. 17, 2022, 11:54 a.m. UTC
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(+)

Comments

Alexander Aring Jan. 17, 2022, 10:43 p.m. UTC | #1
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
Miquel Raynal Jan. 18, 2022, 6:14 p.m. UTC | #2
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
Alexander Aring Jan. 18, 2022, 11:16 p.m. UTC | #3
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 mbox series

Patch

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);