Skip to contentSkip to conversation

nfc: use system_nrt_wq instead of custom ones

PATCH In review Tejun Heo 2 participants 2 messages Aug 22, 2012 – Aug 23, 2012

Subsystems (2)
lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 nfc: use system_nrt_wq instead of custom ones

NFC is using a number of custom ordered workqueues w/ WQ_MEM_RECLAIM. WQ_MEM_RECLAIM is unnecessary unless NFC is gonna be used as transport for storage device, and all use cases match one work item to one ordered workqueue - IOW, there's no actual ordering going on at all and using system_nrt_wq gives the same behavior.

There's nothing to be gained by using custom workqueues. Use system_nrt_wq instead and drop all the custom ones.

Only compile tested.

---
 include/net/nfc/hci.h   |    2 -
 include/net/nfc/nfc.h   |    1 
 include/net/nfc/shdlc.h |    1 
 net/nfc/core.c          |   13 +---------
 net/nfc/hci/core.c      |   42 +++++------------------------------
 net/nfc/hci/hcp.c       |    2 -
 net/nfc/hci/shdlc.c     |   27 +++++++---------------
 net/nfc/llcp/llcp.c     |   57 ++++++------------------------------------------
 net/nfc/llcp/llcp.h     |    3 --
 9 files changed, 26 insertions(+), 122 deletions(-)
include/net/nfc/hci.h +0 -2
index f5169b0..6f065d5 100644
@@ -74,7 +74,6 @@ struct nfc_hci_dev {
 
 	struct list_head msg_tx_queue;
 
-	struct workqueue_struct *msg_tx_wq;
 	struct work_struct msg_tx_work;
 
 	struct timer_list cmd_timer;
@@ -82,7 +81,6 @@ struct nfc_hci_dev {
 
 	struct sk_buff_head rx_hcp_frags;
 
-	struct workqueue_struct *msg_rx_wq;
 	struct work_struct msg_rx_work;
 
 	struct sk_buff_head msg_rx_queue;
include/net/nfc/nfc.h +0 -1
index 6431f5e..493cc0e 100644
@@ -112,7 +112,6 @@ struct nfc_dev {
 	int tx_tailroom;
 
 	struct timer_list check_pres_timer;
-	struct workqueue_struct *check_pres_wq;
 	struct work_struct check_pres_work;
 
 	struct nfc_ops *ops;
include/net/nfc/shdlc.h +0 -1
index 35e930d..3424273 100644
@@ -79,7 +79,6 @@ struct nfc_shdlc {
 
 	struct sk_buff_head ack_pending_q;
 
-	struct workqueue_struct *sm_wq;
 	struct work_struct sm_work;
 
 	struct nfc_shdlc_ops *ops;
net/nfc/core.c +2 -11
index ff74979..c9eacc1 100644
@@ -679,7 +679,7 @@ static void nfc_release(struct device *d)
 
 	if (dev->ops->check_presence) {
 		del_timer_sync(&dev->check_pres_timer);
-		destroy_workqueue(dev->check_pres_wq);
+		cancel_work_sync(&dev->check_pres_work);
 	}
 
 	nfc_genl_data_exit(&dev->genl_data);
@@ -715,7 +715,7 @@ static void nfc_check_pres_timeout(unsigned long data)
 {
 	struct nfc_dev *dev = (struct nfc_dev *)data;
 
-	queue_work(dev->check_pres_wq, &dev->check_pres_work);
+	queue_work(system_nrt_wq, &dev->check_pres_work);
 }
 
 struct class nfc_class = {
@@ -784,20 +784,11 @@ struct nfc_dev *nfc_allocate_device(struct nfc_ops *ops,
 	dev->targets_generation = 1;
 
 	if (ops->check_presence) {
-		char name[32];
 		init_timer(&dev->check_pres_timer);
 		dev->check_pres_timer.data = (unsigned long)dev;
 		dev->check_pres_timer.function = nfc_check_pres_timeout;
 
 		INIT_WORK(&dev->check_pres_work, nfc_check_pres_work);
-		snprintf(name, sizeof(name), "nfc%d_check_pres_wq", dev->idx);
-		dev->check_pres_wq = alloc_workqueue(name, WQ_NON_REENTRANT |
-						     WQ_UNBOUND |
-						     WQ_MEM_RECLAIM, 1);
-		if (dev->check_pres_wq == NULL) {
-			kfree(dev);
-			return NULL;
-		}
 	}
 
 	return dev;
net/nfc/hci/core.c +6 -36

86 lines not on this page · raw message

net/nfc/hci/hcp.c +1 -1
index f4dad1a..2372b55 100644
@@ -108,7 +108,7 @@ int nfc_hci_hcp_message_tx(struct nfc_hci_dev *hdev, u8 pipe,
 	list_add_tail(&cmd->msg_l, &hdev->msg_tx_queue);
 	mutex_unlock(&hdev->msg_tx_mutex);
 
-	queue_work(hdev->msg_tx_wq, &hdev->msg_tx_work);
+	queue_work(system_nrt_wq, &hdev->msg_tx_work);
 
 	return 0;
 
net/nfc/hci/shdlc.c +9 -18
index 6f840c1..39b51ea 100644
@@ -540,7 +540,7 @@ static void nfc_shdlc_connect_timeout(unsigned long data)
 
 	pr_debug("\n");
 
-	queue_work(shdlc->sm_wq, &shdlc->sm_work);
+	queue_work(system_nrt_wq, &shdlc->sm_work);
 }
 
 static void nfc_shdlc_t1_timeout(unsigned long data)
@@ -549,7 +549,7 @@ static void nfc_shdlc_t1_timeout(unsigned long data)
 
 	pr_debug("SoftIRQ: need to send ack\n");
 
-	queue_work(shdlc->sm_wq, &shdlc->sm_work);
+	queue_work(system_nrt_wq, &shdlc->sm_work);
 }
 
 static void nfc_shdlc_t2_timeout(unsigned long data)
@@ -558,7 +558,7 @@ static void nfc_shdlc_t2_timeout(unsigned long data)
 
 	pr_debug("SoftIRQ: need to retransmit\n");
 
-	queue_work(shdlc->sm_wq, &shdlc->sm_work);
+	queue_work(system_nrt_wq, &shdlc->sm_work);
 }
 
 static void nfc_shdlc_sm_work(struct work_struct *work)
@@ -598,7 +598,7 @@ static void nfc_shdlc_sm_work(struct work_struct *work)
 	case SHDLC_NEGOCIATING:
 		if (timer_pending(&shdlc->connect_timer) == 0) {
 			shdlc->state = SHDLC_CONNECTING;
-			queue_work(shdlc->sm_wq, &shdlc->sm_work);
+			queue_work(system_nrt_wq, &shdlc->sm_work);
 		}
 
 		nfc_shdlc_handle_rcv_queue(shdlc);
@@ -662,7 +662,7 @@ static int nfc_shdlc_connect(struct nfc_shdlc *shdlc)
 
 	mutex_unlock(&shdlc->state_mutex);
 
-	queue_work(shdlc->sm_wq, &shdlc->sm_work);
+	queue_work(system_nrt_wq, &shdlc->sm_work);
 
 	wait_event(connect_wq, shdlc->connect_result != 1);
 
@@ -679,7 +679,7 @@ static void nfc_shdlc_disconnect(struct nfc_shdlc *shdlc)
 
 	mutex_unlock(&shdlc->state_mutex);
 
-	queue_work(shdlc->sm_wq, &shdlc->sm_work);
+	queue_work(system_nrt_wq, &shdlc->sm_work);
 }
 
 /*
@@ -697,7 +697,7 @@ void nfc_shdlc_recv_frame(struct nfc_shdlc *shdlc, struct sk_buff *skb)
 		skb_queue_tail(&shdlc->rcv_q, skb);
 	}
 
-	queue_work(shdlc->sm_wq, &shdlc->sm_work);
+	queue_work(system_nrt_wq, &shdlc->sm_work);
 }
 EXPORT_SYMBOL(nfc_shdlc_recv_frame);
 
@@ -754,7 +754,7 @@ static int nfc_shdlc_xmit(struct nfc_hci_dev *hdev, struct sk_buff *skb)
 
 	skb_queue_tail(&shdlc->send_q, skb);
 
-	queue_work(shdlc->sm_wq, &shdlc->sm_work);
+	queue_work(system_nrt_wq, &shdlc->sm_work);
 
 	return 0;
 }
@@ -843,7 +843,6 @@ struct nfc_shdlc *nfc_shdlc_allocate(struct nfc_shdlc_ops *ops,
 {
 	struct nfc_shdlc *shdlc;
 	int r;
-	char name[32];
 
 	if (ops->xmit == NULL)
 		return NULL;
@@ -876,11 +875,6 @@ struct nfc_shdlc *nfc_shdlc_allocate(struct nfc_shdlc_ops *ops,
 	skb_queue_head_init(&shdlc->ack_pending_q);
 
 	INIT_WORK(&shdlc->sm_work, nfc_shdlc_sm_work);
-	snprintf(name, sizeof(name), "%s_shdlc_sm_wq", devname);
-	shdlc->sm_wq = alloc_workqueue(name, WQ_NON_REENTRANT | WQ_UNBOUND |
-				       WQ_MEM_RECLAIM, 1);
-	if (shdlc->sm_wq == NULL)
-		goto err_allocwq;
 
 	shdlc->client_headroom = tx_headroom;
 	shdlc->client_tailroom = tx_tailroom;
@@ -904,9 +898,6 @@ err_regdev:
 	nfc_hci_free_device(shdlc->hdev);
 
 err_allocdev:
-	destroy_workqueue(shdlc->sm_wq);
-
-err_allocwq:
 	kfree(shdlc);
 
 	return NULL;
@@ -920,7 +911,7 @@ void nfc_shdlc_free(struct nfc_shdlc *shdlc)
 	nfc_hci_unregister_device(shdlc->hdev);
 	nfc_hci_free_device(shdlc->hdev);
 
-	destroy_workqueue(shdlc->sm_wq);
+	cancel_work_sync(&shdlc->sm_work);
 
 	skb_queue_purge(&shdlc->rcv_q);
 	skb_queue_purge(&shdlc->send_q);
net/nfc/llcp/llcp.c +8 -49

117 lines not on this page · raw message

net/nfc/llcp/llcp.h +0 -3
index 83b8bba..af395c9 100644
@@ -56,12 +56,9 @@ struct nfc_llcp_local {
 
 	struct timer_list link_timer;
 	struct sk_buff_head tx_queue;
-	struct workqueue_struct	*tx_wq;
 	struct work_struct	 tx_work;
-	struct workqueue_struct	*rx_wq;
 	struct work_struct	 rx_work;
 	struct sk_buff *rx_pending;
-	struct workqueue_struct	*timeout_wq;
 	struct work_struct	 timeout_work;
 
 	u32 target_idx;
#1 lore raw

Hi Tejun,

Quoted 10 lines · Tejun Heo

NFC is using a number of custom ordered workqueues w/ WQ_MEM_RECLAIM. WQ_MEM_RECLAIM is unnecessary unless NFC is gonna be used as transport for storage device, and all use cases match one work item to one ordered workqueue - IOW, there's no actual ordering going on at all and using system_nrt_wq gives the same behavior.

There's nothing to be gained by using custom workqueues. Use system_nrt_wq instead and drop all the custom ones.

Only compile tested.

Tested here, and it works fine.
I'm queueing this one up to my nfc-next branch, thanks a lot for the follow up.

Cheers,
Samuel.

Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/