diff mbox series

[2/2] fnic: move fnic_fnic_flush_tx() to a work queue

Message ID 9c51ef07a04413fb2f2bd20f1534f96e004e4e59.1706632031.git.lduncan@suse.com
State Superseded
Headers show
Series Ensure FCoE target interrupts work | expand

Commit Message

Lee Duncan Jan. 30, 2024, 4:42 p.m. UTC
From: Hannes Reinecke <hare@suse.de>

Rather than call 'fnic_flush_tx()' from interrupt context we should
be moving it onto a work queue to avoid any locking issues.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 drivers/scsi/fnic/fnic.h      | 3 ++-
 drivers/scsi/fnic/fnic_fcs.c  | 3 ++-
 drivers/scsi/fnic/fnic_main.c | 1 +
 drivers/scsi/fnic/fnic_scsi.c | 4 ++--
 4 files changed, 7 insertions(+), 4 deletions(-)

Comments

kernel test robot Jan. 31, 2024, 11:50 a.m. UTC | #1
Hi Lee,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jejb-scsi/for-next]
[also build test WARNING on mkp-scsi/for-next linus/master v6.8-rc2 next-20240131]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lee-Duncan/Revert-scsi-fcoe-Fix-potential-deadlock-on-fip-ctlr_lock/20240131-004656
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
patch link:    https://lore.kernel.org/r/9c51ef07a04413fb2f2bd20f1534f96e004e4e59.1706632031.git.lduncan%40suse.com
patch subject: [PATCH 2/2] fnic: move fnic_fnic_flush_tx() to a work queue
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20240131/202401311947.cPDhv2xa-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240131/202401311947.cPDhv2xa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401311947.cPDhv2xa-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/scsi/fnic/fnic_fcs.c:1194: warning: Function parameter or struct member 'work' not described in 'fnic_flush_tx'
>> drivers/scsi/fnic/fnic_fcs.c:1194: warning: Excess function parameter 'fnic' description in 'fnic_flush_tx'


vim +1194 drivers/scsi/fnic/fnic_fcs.c

5df6d737dd4b0fe Abhijeet Joglekar 2009-04-17  1182  
78112e5558064cb Joe Eykholt       2009-11-03  1183  /**
78112e5558064cb Joe Eykholt       2009-11-03  1184   * fnic_flush_tx() - send queued frames.
78112e5558064cb Joe Eykholt       2009-11-03  1185   * @fnic: fnic device
78112e5558064cb Joe Eykholt       2009-11-03  1186   *
78112e5558064cb Joe Eykholt       2009-11-03  1187   * Send frames that were waiting to go out in FC or Ethernet mode.
78112e5558064cb Joe Eykholt       2009-11-03  1188   * Whenever changing modes we purge queued frames, so these frames should
78112e5558064cb Joe Eykholt       2009-11-03  1189   * be queued for the stable mode that we're in, either FC or Ethernet.
78112e5558064cb Joe Eykholt       2009-11-03  1190   *
78112e5558064cb Joe Eykholt       2009-11-03  1191   * Called without fnic_lock held.
78112e5558064cb Joe Eykholt       2009-11-03  1192   */
7ea34b1ffb4e1aa Hannes Reinecke   2024-01-30  1193  void fnic_flush_tx(struct work_struct *work)
78112e5558064cb Joe Eykholt       2009-11-03 @1194  {
7ea34b1ffb4e1aa Hannes Reinecke   2024-01-30  1195  	struct fnic *fnic = container_of(work, struct fnic, flush_work);
78112e5558064cb Joe Eykholt       2009-11-03  1196  	struct sk_buff *skb;
78112e5558064cb Joe Eykholt       2009-11-03  1197  	struct fc_frame *fp;
5df6d737dd4b0fe Abhijeet Joglekar 2009-04-17  1198  
d9e9ab56b687da0 Brian Uchino      2010-04-09  1199  	while ((skb = skb_dequeue(&fnic->tx_queue))) {
78112e5558064cb Joe Eykholt       2009-11-03  1200  		fp = (struct fc_frame *)skb;
78112e5558064cb Joe Eykholt       2009-11-03  1201  		fnic_send_frame(fnic, fp);
78112e5558064cb Joe Eykholt       2009-11-03  1202  	}
78112e5558064cb Joe Eykholt       2009-11-03  1203  }
5df6d737dd4b0fe Abhijeet Joglekar 2009-04-17  1204
Hannes Reinecke Feb. 6, 2024, 1:23 a.m. UTC | #2
On 1/31/24 00:42, Lee Duncan wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> Rather than call 'fnic_flush_tx()' from interrupt context we should
> be moving it onto a work queue to avoid any locking issues.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>   drivers/scsi/fnic/fnic.h      | 3 ++-
>   drivers/scsi/fnic/fnic_fcs.c  | 3 ++-
>   drivers/scsi/fnic/fnic_main.c | 1 +
>   drivers/scsi/fnic/fnic_scsi.c | 4 ++--
>   4 files changed, 7 insertions(+), 4 deletions(-)
> 
Can you please add the 'fixes' tag to this one, too?
Just the first patch doesn't fix anything, one really would
need to apply both to get the issue fixed.

And, of course, fix the kbuild robot warning :-)

Cheers,

Hannes
diff mbox series

Patch

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 2074937c05bc..3b8eb7dee500 100644
--- a/drivers/scsi/fnic/fnic.h
+++ b/drivers/scsi/fnic/fnic.h
@@ -305,6 +305,7 @@  struct fnic {
 	unsigned int copy_wq_base;
 	struct work_struct link_work;
 	struct work_struct frame_work;
+	struct work_struct flush_work;
 	struct sk_buff_head frame_queue;
 	struct sk_buff_head tx_queue;
 
@@ -363,7 +364,7 @@  void fnic_handle_event(struct work_struct *work);
 int fnic_rq_cmpl_handler(struct fnic *fnic, int);
 int fnic_alloc_rq_frame(struct vnic_rq *rq);
 void fnic_free_rq_buf(struct vnic_rq *rq, struct vnic_rq_buf *buf);
-void fnic_flush_tx(struct fnic *);
+void fnic_flush_tx(struct work_struct *);
 void fnic_eth_send(struct fcoe_ctlr *, struct sk_buff *skb);
 void fnic_set_port_id(struct fc_lport *, u32, struct fc_frame *);
 void fnic_update_mac(struct fc_lport *, u8 *new);
diff --git a/drivers/scsi/fnic/fnic_fcs.c b/drivers/scsi/fnic/fnic_fcs.c
index 5e312a55cc7d..1bf1418bbde4 100644
--- a/drivers/scsi/fnic/fnic_fcs.c
+++ b/drivers/scsi/fnic/fnic_fcs.c
@@ -1190,8 +1190,9 @@  int fnic_send(struct fc_lport *lp, struct fc_frame *fp)
  *
  * Called without fnic_lock held.
  */
-void fnic_flush_tx(struct fnic *fnic)
+void fnic_flush_tx(struct work_struct *work)
 {
+	struct fnic *fnic = container_of(work, struct fnic, flush_work);
 	struct sk_buff *skb;
 	struct fc_frame *fp;
 
diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
index 5ed1d897311a..29eead383eb9 100644
--- a/drivers/scsi/fnic/fnic_main.c
+++ b/drivers/scsi/fnic/fnic_main.c
@@ -830,6 +830,7 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		spin_lock_init(&fnic->vlans_lock);
 		INIT_WORK(&fnic->fip_frame_work, fnic_handle_fip_frame);
 		INIT_WORK(&fnic->event_work, fnic_handle_event);
+		INIT_WORK(&fnic->flush_work, fnic_flush_tx);
 		skb_queue_head_init(&fnic->fip_frame_queue);
 		INIT_LIST_HEAD(&fnic->evlist);
 		INIT_LIST_HEAD(&fnic->vlans);
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 8d7fc5284293..fc4cee91b175 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -680,7 +680,7 @@  static int fnic_fcpio_fw_reset_cmpl_handler(struct fnic *fnic,
 
 	spin_unlock_irqrestore(&fnic->fnic_lock, flags);
 
-	fnic_flush_tx(fnic);
+	queue_work(fnic_event_queue, &fnic->flush_work);
 
  reset_cmpl_handler_end:
 	fnic_clear_state_flags(fnic, FNIC_FLAGS_FWRESET);
@@ -736,7 +736,7 @@  static int fnic_fcpio_flogi_reg_cmpl_handler(struct fnic *fnic,
 		}
 		spin_unlock_irqrestore(&fnic->fnic_lock, flags);
 
-		fnic_flush_tx(fnic);
+		queue_work(fnic_event_queue, &fnic->flush_work);
 		queue_work(fnic_event_queue, &fnic->frame_work);
 	} else {
 		spin_unlock_irqrestore(&fnic->fnic_lock, flags);