linux-kernel - Re: [PATCH 6/6] mpt3sas: Used "synchronize_irq()"API to synchronize timed-out IO & TMs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 May 2016 15:11:49 +0200
From:	Tomas Henzl <thenzl@...hat.com>
To:	Chaitra P B <chaitra.basappa@...adcom.com>, jejb@...nel.org,
	hch@...radead.org
Cc:	martin.petersen@...cle.com, linux-scsi@...r.kernel.org,
	JBottomley@...allels.com, Sathya.Prakash@...adcom.com,
	suganath-prabu.subramani@...adcom.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/6] mpt3sas: Used "synchronize_irq()"API to synchronize
 timed-out IO & TMs

On 6.5.2016 10:59, Chaitra P B wrote:
> Replaced mpt3sas_base_flush_reply_queues()with
> mpt3sas_base_sync_reply_irqs(),as mpt3sas_base_flush_reply_queues()
> skips over reply queues that are currently busy (i.e. being handled
> by interrupt processing in another core). If a reply queue is busy,
> then call to synchronize_irq()in mpt3sas_base_sync_reply_irqs()make
> sures the other core has finished flushing the queue and completed
> any calls to the mid-layer scsi_done() routine.
>
> Signed-off-by: Chaitra P B <chaitra.basappa@...adcom.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c  | 15 +++++++--------
>  drivers/scsi/mpt3sas/mpt3sas_base.h  |  3 ++-
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c |  4 +++-
>  3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 4e9142f..fd9002d 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -1103,18 +1103,16 @@ _base_is_controller_msix_enabled(struct MPT3SAS_ADAPTER *ioc)
>  }
>  
>  /**
> - * mpt3sas_base_flush_reply_queues - flushing the MSIX reply queues
> + * mpt3sas_base_sync_reply_irqs - flush pending MSIX interrupts
>   * @ioc: per adapter object
> - * Context: ISR conext
> + * Context: non ISR conext
>   *
> - * Called when a Task Management request has completed. We want
> - * to flush the other reply queues so all the outstanding IO has been
> - * completed back to OS before we process the TM completetion.
> + * Called when a Task Management request has completed.
>   *
>   * Return nothing.
>   */
>  void
> -mpt3sas_base_flush_reply_queues(struct MPT3SAS_ADAPTER *ioc)
> +mpt3sas_base_sync_reply_irqs(struct MPT3SAS_ADAPTER *ioc)
>  {
>  	struct adapter_reply_queue *reply_q;
>  
> @@ -1125,12 +1123,13 @@ mpt3sas_base_flush_reply_queues(struct MPT3SAS_ADAPTER *ioc)
>  		return;
>  
>  	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
> -		if (ioc->shost_recovery)
> +		if (ioc->shost_recovery || ioc->remove_host ||
> +				ioc->pci_error_recovery)

Hi Chaitra,
how is this change + (ioc->remove_host || ioc->pci_error_recovery)
related to the subject? 

>  			return;
>  		/* TMs are on msix_index == 0 */
>  		if (reply_q->msix_index == 0)
>  			continue;
> -		_base_interrupt(reply_q->vector, (void *)reply_q);
> +		synchronize_irq(reply_q->vector);
>  	}

One thing I don't understand - what if an interrupt comes after
the synchronize_irq has finished ? 

Thanks,
Tomas 

>  }
>  
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
> index e1befba..1a614d7 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
> @@ -1236,7 +1236,8 @@ void *mpt3sas_base_get_msg_frame(struct MPT3SAS_ADAPTER *ioc, u16 smid);
>  void *mpt3sas_base_get_sense_buffer(struct MPT3SAS_ADAPTER *ioc, u16 smid);
>  __le32 mpt3sas_base_get_sense_buffer_dma(struct MPT3SAS_ADAPTER *ioc,
>  	u16 smid);
> -void mpt3sas_base_flush_reply_queues(struct MPT3SAS_ADAPTER *ioc);
> +
> +void mpt3sas_base_sync_reply_irqs(struct MPT3SAS_ADAPTER *ioc);
>  
>  /* hi-priority queue */
>  u16 mpt3sas_base_get_smid_hpr(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx);
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index abd8717..928214f 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -2126,7 +2126,6 @@ _scsih_tm_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
>  		return 1;
>  	if (ioc->tm_cmds.smid != smid)
>  		return 1;
> -	mpt3sas_base_flush_reply_queues(ioc);
>  	ioc->tm_cmds.status |= MPT3_CMD_COMPLETE;
>  	mpi_reply =  mpt3sas_base_get_reply_virt_addr(ioc, reply);
>  	if (mpi_reply) {
> @@ -2311,6 +2310,9 @@ mpt3sas_scsih_issue_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle, uint channel,
>  		}
>  	}
>  
> +	/* sync IRQs in case those were busy during flush. */
> +	mpt3sas_base_sync_reply_irqs(ioc);
> +
>  	if (ioc->tm_cmds.status & MPT3_CMD_REPLY_VALID) {
>  		mpt3sas_trigger_master(ioc, MASTER_TRIGGER_TASK_MANAGMENT);
>  		mpi_reply = ioc->tm_cmds.reply;