diff mbox series

[RFC,v2,08/18] libata: Queue ATA internal commands as requests

Message ID 1654770559-101375-9-git-send-email-john.garry@huawei.com (mailing list archive)
State Changes Requested
Headers show
Series blk-mq/libata/scsi: SCSI driver tagging improvements | expand

Commit Message

John Garry June 9, 2022, 10:29 a.m. UTC
Follow the normal path for requests and queue through the block layer.

We hold the qc pointer in the scmd host scribble, which is less than
ideal. In future we can hold the ata_queued_cmd in the scmd priv_data,
allowing for easy lookup.

We need to use the shost sdev to queue these requests. That is because we
initially do not have the target disk scsi_device allocated yet when
queueing internal commands. This is less than ideal, and makes it hard for
libsas to know when queueing an internal command whether it is a ATA
internal command or not.

Also make ata_exec_internal_sg() static - there are no users outside
libata-core.c

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/ata/libata-core.c | 137 +++++++++++++++++++++-----------------
 drivers/ata/libata-sata.c |   5 +-
 drivers/ata/libata.h      |   4 --
 3 files changed, 80 insertions(+), 66 deletions(-)

Comments

Damien Le Moal June 13, 2022, 7:22 a.m. UTC | #1
On 6/9/22 19:29, John Garry wrote:
> Follow the normal path for requests and queue through the block layer.
> 
> We hold the qc pointer in the scmd host scribble, which is less than
> ideal. In future we can hold the ata_queued_cmd in the scmd priv_data,
> allowing for easy lookup.
> 
> We need to use the shost sdev to queue these requests. That is because we
> initially do not have the target disk scsi_device allocated yet when
> queueing internal commands. This is less than ideal, and makes it hard for
> libsas to know when queueing an internal command whether it is a ATA
> internal command or not.
> 
> Also make ata_exec_internal_sg() static - there are no users outside
> libata-core.c
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  drivers/ata/libata-core.c | 137 +++++++++++++++++++++-----------------
>  drivers/ata/libata-sata.c |   5 +-
>  drivers/ata/libata.h      |   4 --
>  3 files changed, 80 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 40e816419f48..6b1aaeccb253 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -1438,9 +1438,18 @@ unsigned long ata_id_xfermask(const u16 *id)
>  }
>  EXPORT_SYMBOL_GPL(ata_id_xfermask);
>  
> -static void ata_qc_complete_internal(struct ata_queued_cmd *qc)
> +void ata_qc_complete_internal(struct ata_queued_cmd *qc)
>  {
> -	struct completion *waiting = qc->private_data;
> +	struct scsi_cmnd *scmd = qc->scsicmd;
> +
> +	scsi_done(scmd);
> +}
> +
> +static void ata_internal_end_rq(struct request *rq, blk_status_t error)
> +{
> +	struct completion *waiting = rq->end_io_data;
> +
> +	rq->end_io_data = (void *)(uintptr_t)error;
>  
>  	complete(waiting);
>  }
> @@ -1467,52 +1476,74 @@ static void ata_qc_complete_internal(struct ata_queued_cmd *qc)
>   *	RETURNS:
>   *	Zero on success, AC_ERR_* mask on failure
>   */
> -unsigned ata_exec_internal_sg(struct ata_device *dev,
> +static unsigned ata_exec_internal_sg(struct ata_device *dev,
>  			      struct ata_taskfile *tf, const u8 *cdb,
> -			      int dma_dir, struct scatterlist *sgl,
> -			      unsigned int n_elem, unsigned long timeout)
> +			      int dma_dir, void *buf, unsigned int buflen,
> +			      unsigned long timeout)
>  {
>  	struct ata_link *link = dev->link;
>  	struct ata_port *ap = link->ap;
> +	struct Scsi_Host *scsi_host = ap->scsi_host;
> +	struct scsi_device *sdev = scsi_host->sdev;
>  	u8 command = tf->command;
>  	int auto_timeout = 0;
>  	struct ata_queued_cmd *qc;
> -	unsigned int preempted_tag;
> -	u32 preempted_sactive;
> -	u64 preempted_qc_active;
> -	int preempted_nr_active_links;
>  	DECLARE_COMPLETION_ONSTACK(wait);
>  	unsigned long flags;
>  	unsigned int err_mask;
> +	struct scsi_cmnd *scmd;
> +	struct request *req;
>  	int rc;
>  
> -	spin_lock_irqsave(ap->lock, flags);
> +	/*
> +	 * We only support a single reserved command, so this guarantees
> +	 * serialization. However the code already assumed that (we are
> +	 * serialized here per-port).
> +	 */
> +	req = scsi_alloc_request(sdev->request_queue,
> +			dma_dir == DMA_TO_DEVICE ?
> +			REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
> +			BLK_MQ_REQ_RESERVED);
> +	if (IS_ERR(req))
> +		return AC_ERR_OTHER;
>  
> -	/* no internal command while frozen */
> -	if (ap->pflags & ATA_PFLAG_FROZEN) {
> -		spin_unlock_irqrestore(ap->lock, flags);
> -		return AC_ERR_SYSTEM;
> +
> +	if (!timeout) {
> +		if (ata_probe_timeout)
> +			timeout = ata_probe_timeout * 1000;
> +		else {
> +			timeout = ata_internal_cmd_timeout(dev, command);
> +			auto_timeout = 1;
> +		}
>  	}
>  
> -	/* initialize internal qc */
> +	scmd = blk_mq_rq_to_pdu(req);
> +	scmd->allowed = 0;
> +	req->timeout = timeout;
> +	//TODO: Hook up timeout handler
> +	req->rq_flags |= RQF_QUIET;
> +	scmd->device = sdev;
>  	qc = __ata_qc_from_tag(ap, ATA_TAG_INTERNAL);
>  
> +	/* Do this until we can hold ata_queued_cmd in the SCMD priv data */
> +	scmd->host_scribble = (unsigned char *)qc;
> +
> +	if (buflen) {
> +		int ret = blk_rq_map_kern(sdev->request_queue, req,
> +					  buf, buflen, GFP_NOIO);
> +		if (ret) {
> +			blk_mq_free_request(req);
> +			return AC_ERR_OTHER;
> +		}
> +	}
> +
>  	qc->tag = ATA_TAG_INTERNAL;
>  	qc->hw_tag = 0;
> -	qc->scsicmd = NULL;
> +	qc->scsicmd = scmd;
>  	qc->ap = ap;
>  	qc->dev = dev;
>  	ata_qc_reinit(qc);
>  
> -	preempted_tag = link->active_tag;
> -	preempted_sactive = link->sactive;
> -	preempted_qc_active = ap->qc_active;
> -	preempted_nr_active_links = ap->nr_active_links;
> -	link->active_tag = ATA_TAG_POISON;
> -	link->sactive = 0;
> -	ap->qc_active = 0;
> -	ap->nr_active_links = 0;
> -
>  	/* prepare & issue qc */
>  	qc->tf = *tf;
>  	if (cdb)
> @@ -1525,32 +1556,14 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
>  
>  	qc->flags |= ATA_QCFLAG_RESULT_TF;
>  	qc->dma_dir = dma_dir;
> -	if (dma_dir != DMA_NONE) {
> -		unsigned int i, buflen = 0;
> -		struct scatterlist *sg;
> -
> -		for_each_sg(sgl, sg, n_elem, i)
> -			buflen += sg->length;
> -
> -		ata_sg_init(qc, sgl, n_elem);
> -		qc->nbytes = buflen;
> -	}
>  
> -	qc->private_data = &wait;
> +	qc->private_data = ap;
>  	qc->complete_fn = ata_qc_complete_internal;
>  
> -	ata_qc_issue(qc);
> -
> -	spin_unlock_irqrestore(ap->lock, flags);
> +	req->end_io_data = &wait;
> +	req->end_io = ata_internal_end_rq;
>  
> -	if (!timeout) {
> -		if (ata_probe_timeout)
> -			timeout = ata_probe_timeout * 1000;
> -		else {
> -			timeout = ata_internal_cmd_timeout(dev, command);
> -			auto_timeout = 1;
> -		}
> -	}
> +	blk_execute_rq_nowait(req, true);

How do you get guarantee that this req ends up being issued with
ATA_TAG_INTERNAL as the tag ? Because you have the reserved commands *in
addition to can queue* ? I can see how that works if can_queue is indeed
32, but what if the user changes the max qd ? That breaks, no ?

>  
>  	if (ap->ops->error_handler)
>  		ata_eh_release(ap);
> @@ -1610,13 +1623,15 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
>  	err_mask = qc->err_mask;
>  
>  	ata_qc_free(qc);
> -	link->active_tag = preempted_tag;
> -	link->sactive = preempted_sactive;
> -	ap->qc_active = preempted_qc_active;
> -	ap->nr_active_links = preempted_nr_active_links;
> +	link->active_tag = link->preempted_tag;
> +	link->sactive = link->preempted_sactive;
> +	ap->qc_active = ap->preempted_qc_active;
> +	ap->nr_active_links = ap->preempted_nr_active_links;
>  
>  	spin_unlock_irqrestore(ap->lock, flags);
>  
> +	blk_mq_free_request(req);
> +
>  	if ((err_mask & AC_ERR_TIMEOUT) && auto_timeout)
>  		ata_internal_cmd_timed_out(dev, command);
>  
> @@ -1647,18 +1662,20 @@ unsigned ata_exec_internal(struct ata_device *dev,
>  			   int dma_dir, void *buf, unsigned int buflen,
>  			   unsigned long timeout)
>  {
> -	struct scatterlist *psg = NULL, sg;
> -	unsigned int n_elem = 0;
> +	/* buf may not be aligned, so copy to/from an aligned buffer */
> +	void *tmpbuf = kmemdup(buf, buflen, GFP_KERNEL);
> +	unsigned res;
>  
> -	if (dma_dir != DMA_NONE) {
> -		WARN_ON(!buf);
> -		sg_init_one(&sg, buf, buflen);
> -		psg = &sg;
> -		n_elem++;
> -	}
> +	if (!tmpbuf)
> +		return AC_ERR_OTHER;
>  
> -	return ata_exec_internal_sg(dev, tf, cdb, dma_dir, psg, n_elem,
> +	res = ata_exec_internal_sg(dev, tf, cdb, dma_dir, tmpbuf, buflen,
>  				    timeout);
> +
> +	memcpy(buf, tmpbuf, buflen);
> +	kfree(tmpbuf);
> +
> +	return res;
>  }
>  
>  /**
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index 7a5fe41aa5ae..3cecc45d54ab 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -1258,9 +1258,10 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
>  {
>  	int rc = 0;
>  
> -	if (likely(ata_dev_enabled(ap->link.device)))
> +	if (likely(ata_dev_enabled(ap->link.device)) ||
> +		scsi_is_reserved_cmd(cmd)) {
>  		rc = __ata_scsi_queuecmd(cmd, ap->link.device);
> -	else {
> +	} else {
>  		cmd->result = (DID_BAD_TARGET << 16);
>  		scsi_done(cmd);
>  	}
> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
> index 926a7f41303d..1446a482835d 100644
> --- a/drivers/ata/libata.h
> +++ b/drivers/ata/libata.h
> @@ -53,10 +53,6 @@ extern unsigned ata_exec_internal(struct ata_device *dev,
>  				  struct ata_taskfile *tf, const u8 *cdb,
>  				  int dma_dir, void *buf, unsigned int buflen,
>  				  unsigned long timeout);
> -extern unsigned ata_exec_internal_sg(struct ata_device *dev,
> -				     struct ata_taskfile *tf, const u8 *cdb,
> -				     int dma_dir, struct scatterlist *sg,
> -				     unsigned int n_elem, unsigned long timeout);
>  extern int ata_wait_ready(struct ata_link *link, unsigned long deadline,
>  			  int (*check_ready)(struct ata_link *link));
>  extern int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
John Garry June 13, 2022, 8:13 a.m. UTC | #2
On 13/06/2022 08:22, Damien Le Moal wrote:
> On 6/9/22 19:29, John Garry wrote:
>> Follow the normal path for requests and queue through the block layer.
>>
>> We hold the qc pointer in the scmd host scribble, which is less than
>> ideal. In future we can hold the ata_queued_cmd in the scmd priv_data,
>> allowing for easy lookup.
>>
>> We need to use the shost sdev to queue these requests. That is because we
>> initially do not have the target disk scsi_device allocated yet when
>> queueing internal commands. This is less than ideal, and makes it hard for
>> libsas to know when queueing an internal command whether it is a ATA
>> internal command or not.
>>
>> Also make ata_exec_internal_sg() static - there are no users outside
>> libata-core.c
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>   drivers/ata/libata-core.c | 137 +++++++++++++++++++++-----------------
>>   drivers/ata/libata-sata.c |   5 +-
>>   drivers/ata/libata.h      |   4 --
>>   3 files changed, 80 insertions(+), 66 deletions(-)
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 40e816419f48..6b1aaeccb253 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -1438,9 +1438,18 @@ unsigned long ata_id_xfermask(const u16 *id)
>>   }
>>   EXPORT_SYMBOL_GPL(ata_id_xfermask);
>>   
>> -static void ata_qc_complete_internal(struct ata_queued_cmd *qc)
>> +void ata_qc_complete_internal(struct ata_queued_cmd *qc)
>>   {
>> -	struct completion *waiting = qc->private_data;
>> +	struct scsi_cmnd *scmd = qc->scsicmd;
>> +
>> +	scsi_done(scmd);
>> +}
>> +
>> +static void ata_internal_end_rq(struct request *rq, blk_status_t error)
>> +{
>> +	struct completion *waiting = rq->end_io_data;
>> +
>> +	rq->end_io_data = (void *)(uintptr_t)error;
>>   
>>   	complete(waiting);
>>   }
>> @@ -1467,52 +1476,74 @@ static void ata_qc_complete_internal(struct ata_queued_cmd *qc)
>>    *	RETURNS:
>>    *	Zero on success, AC_ERR_* mask on failure
>>    */
>> -unsigned ata_exec_internal_sg(struct ata_device *dev,
>> +static unsigned ata_exec_internal_sg(struct ata_device *dev,
>>   			      struct ata_taskfile *tf, const u8 *cdb,
>> -			      int dma_dir, struct scatterlist *sgl,
>> -			      unsigned int n_elem, unsigned long timeout)
>> +			      int dma_dir, void *buf, unsigned int buflen,
>> +			      unsigned long timeout)
>>   {
>>   	struct ata_link *link = dev->link;
>>   	struct ata_port *ap = link->ap;
>> +	struct Scsi_Host *scsi_host = ap->scsi_host;
>> +	struct scsi_device *sdev = scsi_host->sdev;
>>   	u8 command = tf->command;
>>   	int auto_timeout = 0;
>>   	struct ata_queued_cmd *qc;
>> -	unsigned int preempted_tag;
>> -	u32 preempted_sactive;
>> -	u64 preempted_qc_active;
>> -	int preempted_nr_active_links;
>>   	DECLARE_COMPLETION_ONSTACK(wait);
>>   	unsigned long flags;
>>   	unsigned int err_mask;
>> +	struct scsi_cmnd *scmd;
>> +	struct request *req;
>>   	int rc;
>>   
>> -	spin_lock_irqsave(ap->lock, flags);
>> +	/*
>> +	 * We only support a single reserved command, so this guarantees
>> +	 * serialization. However the code already assumed that (we are
>> +	 * serialized here per-port).
>> +	 */
>> +	req = scsi_alloc_request(sdev->request_queue,
>> +			dma_dir == DMA_TO_DEVICE ?
>> +			REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
>> +			BLK_MQ_REQ_RESERVED);
>> +	if (IS_ERR(req))
>> +		return AC_ERR_OTHER;
>>   
>> -	/* no internal command while frozen */
>> -	if (ap->pflags & ATA_PFLAG_FROZEN) {
>> -		spin_unlock_irqrestore(ap->lock, flags);
>> -		return AC_ERR_SYSTEM;
>> +
>> +	if (!timeout) {
>> +		if (ata_probe_timeout)
>> +			timeout = ata_probe_timeout * 1000;
>> +		else {
>> +			timeout = ata_internal_cmd_timeout(dev, command);
>> +			auto_timeout = 1;
>> +		}
>>   	}
>>   
>> -	/* initialize internal qc */
>> +	scmd = blk_mq_rq_to_pdu(req);
>> +	scmd->allowed = 0;
>> +	req->timeout = timeout;
>> +	//TODO: Hook up timeout handler
>> +	req->rq_flags |= RQF_QUIET;
>> +	scmd->device = sdev;
>>   	qc = __ata_qc_from_tag(ap, ATA_TAG_INTERNAL);
>>   
>> +	/* Do this until we can hold ata_queued_cmd in the SCMD priv data */
>> +	scmd->host_scribble = (unsigned char *)qc;
>> +
>> +	if (buflen) {
>> +		int ret = blk_rq_map_kern(sdev->request_queue, req,
>> +					  buf, buflen, GFP_NOIO);
>> +		if (ret) {
>> +			blk_mq_free_request(req);
>> +			return AC_ERR_OTHER;
>> +		}
>> +	}
>> +
>>   	qc->tag = ATA_TAG_INTERNAL;
>>   	qc->hw_tag = 0;
>> -	qc->scsicmd = NULL;
>> +	qc->scsicmd = scmd;
>>   	qc->ap = ap;
>>   	qc->dev = dev;
>>   	ata_qc_reinit(qc);
>>   
>> -	preempted_tag = link->active_tag;
>> -	preempted_sactive = link->sactive;
>> -	preempted_qc_active = ap->qc_active;
>> -	preempted_nr_active_links = ap->nr_active_links;
>> -	link->active_tag = ATA_TAG_POISON;
>> -	link->sactive = 0;
>> -	ap->qc_active = 0;
>> -	ap->nr_active_links = 0;
>> -
>>   	/* prepare & issue qc */
>>   	qc->tf = *tf;
>>   	if (cdb)
>> @@ -1525,32 +1556,14 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
>>   
>>   	qc->flags |= ATA_QCFLAG_RESULT_TF;
>>   	qc->dma_dir = dma_dir;
>> -	if (dma_dir != DMA_NONE) {
>> -		unsigned int i, buflen = 0;
>> -		struct scatterlist *sg;
>> -
>> -		for_each_sg(sgl, sg, n_elem, i)
>> -			buflen += sg->length;
>> -
>> -		ata_sg_init(qc, sgl, n_elem);
>> -		qc->nbytes = buflen;
>> -	}
>>   
>> -	qc->private_data = &wait;
>> +	qc->private_data = ap;
>>   	qc->complete_fn = ata_qc_complete_internal;
>>   
>> -	ata_qc_issue(qc);
>> -
>> -	spin_unlock_irqrestore(ap->lock, flags);
>> +	req->end_io_data = &wait;
>> +	req->end_io = ata_internal_end_rq;
>>   
>> -	if (!timeout) {
>> -		if (ata_probe_timeout)
>> -			timeout = ata_probe_timeout * 1000;
>> -		else {
>> -			timeout = ata_internal_cmd_timeout(dev, command);
>> -			auto_timeout = 1;
>> -		}
>> -	}

Hi Damien,

>> +	blk_execute_rq_nowait(req, true);
> 
> How do you get guarantee that this req ends up being issued with
> ATA_TAG_INTERNAL as the tag ?

We don't, but it's not important, as we don't use the request tag index 
in libata to infer that a request is an ATA_TAG_INTERNAL request. Nobody 
should assume that the tag index for the ATA_TAG_INTERNAL req is 32. We 
have other methods to check that, like using scsi_is_reserved_cmd() and 
continuing to use __ata_qc_from_tag(ap, ATA_TAG_INTERNAL) to get the ATA 
internal qc.

But it is a good question, as if we start to put the ata_queued_cmd 
structure in the SCSI cmnd priv data - as suggested by Hannes - then 
__ata_qc_from_tag(ap, ATA_TAG_INTERNAL) may need to be reworked to 
handle that.

> Because you have the reserved commands *in
> addition to can queue* ? I can see how that works if can_queue is indeed
> 32, but what if the user changes the max qd ? That breaks, no ?

Thanks,
john

> 
>>   
>>   	if (ap->ops->error_handler)
>>   		ata_eh_release(ap);
>> @@ -1610,13 +1623,15 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
>>   	err_mask = qc->err_mask;
>>   
>>   	ata_qc_free(qc);
>> -	link->active_tag = preempted_tag;
>> -	link->sactive = preempted_sactive;
>> -	ap->qc_active = preempted_qc_active;
>> -	ap->nr_active_links = preempted_nr_active_links;
>> +	link->active_tag = link->preempted_tag;
>> +	link->sactive = link->preempted_sactive;
>> +	ap->qc_active = ap->preempted_qc_active;
>> +	ap->nr_active_links = ap->preempted_nr_active_links;
>>   
>>   	spin_unlock_irqrestore(ap->lock, flags);
>>   
>> +	blk_mq_free_request(req);
>> +
>>   	if ((err_mask & AC_ERR_TIMEOUT) && auto_timeout)
>>   		ata_internal_cmd_timed_out(dev, command);
>>   
>> @@ -1647,18 +1662,20 @@ unsigned ata_exec_internal(struct ata_device *dev,
>>   			   int dma_dir, void *buf, unsigned int buflen,
>>   			   unsigned long timeout)
>>   {
>> -	struct scatterlist *psg = NULL, sg;
>> -	unsigned int n_elem = 0;
>> +	/* buf may not be aligned, so copy to/from an aligned buffer */
>> +	void *tmpbuf = kmemdup(buf, buflen, GFP_KERNEL);
>> +	unsigned res;
>>   
>> -	if (dma_dir != DMA_NONE) {
>> -		WARN_ON(!buf);
>> -		sg_init_one(&sg, buf, buflen);
>> -		psg = &sg;
>> -		n_elem++;
>> -	}
>> +	if (!tmpbuf)
>> +		return AC_ERR_OTHER;
>>   
>> -	return ata_exec_internal_sg(dev, tf, cdb, dma_dir, psg, n_elem,
>> +	res = ata_exec_internal_sg(dev, tf, cdb, dma_dir, tmpbuf, buflen,
>>   				    timeout);
>> +
>> +	memcpy(buf, tmpbuf, buflen);
>> +	kfree(tmpbuf);
>> +
>> +	return res;
>>   }
>>   
>>   /**
>> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
>> index 7a5fe41aa5ae..3cecc45d54ab 100644
>> --- a/drivers/ata/libata-sata.c
>> +++ b/drivers/ata/libata-sata.c
>> @@ -1258,9 +1258,10 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
>>   {
>>   	int rc = 0;
>>   
>> -	if (likely(ata_dev_enabled(ap->link.device)))
>> +	if (likely(ata_dev_enabled(ap->link.device)) ||
>> +		scsi_is_reserved_cmd(cmd)) {
>>   		rc = __ata_scsi_queuecmd(cmd, ap->link.device);
>> -	else {
>> +	} else {
>>   		cmd->result = (DID_BAD_TARGET << 16);
>>   		scsi_done(cmd);
>>   	}
>> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
>> index 926a7f41303d..1446a482835d 100644
>> --- a/drivers/ata/libata.h
>> +++ b/drivers/ata/libata.h
>> @@ -53,10 +53,6 @@ extern unsigned ata_exec_internal(struct ata_device *dev,
>>   				  struct ata_taskfile *tf, const u8 *cdb,
>>   				  int dma_dir, void *buf, unsigned int buflen,
>>   				  unsigned long timeout);
>> -extern unsigned ata_exec_internal_sg(struct ata_device *dev,
>> -				     struct ata_taskfile *tf, const u8 *cdb,
>> -				     int dma_dir, struct scatterlist *sg,
>> -				     unsigned int n_elem, unsigned long timeout);
>>   extern int ata_wait_ready(struct ata_link *link, unsigned long deadline,
>>   			  int (*check_ready)(struct ata_link *link));
>>   extern int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
> 
>
diff mbox series

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 40e816419f48..6b1aaeccb253 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1438,9 +1438,18 @@  unsigned long ata_id_xfermask(const u16 *id)
 }
 EXPORT_SYMBOL_GPL(ata_id_xfermask);
 
-static void ata_qc_complete_internal(struct ata_queued_cmd *qc)
+void ata_qc_complete_internal(struct ata_queued_cmd *qc)
 {
-	struct completion *waiting = qc->private_data;
+	struct scsi_cmnd *scmd = qc->scsicmd;
+
+	scsi_done(scmd);
+}
+
+static void ata_internal_end_rq(struct request *rq, blk_status_t error)
+{
+	struct completion *waiting = rq->end_io_data;
+
+	rq->end_io_data = (void *)(uintptr_t)error;
 
 	complete(waiting);
 }
@@ -1467,52 +1476,74 @@  static void ata_qc_complete_internal(struct ata_queued_cmd *qc)
  *	RETURNS:
  *	Zero on success, AC_ERR_* mask on failure
  */
-unsigned ata_exec_internal_sg(struct ata_device *dev,
+static unsigned ata_exec_internal_sg(struct ata_device *dev,
 			      struct ata_taskfile *tf, const u8 *cdb,
-			      int dma_dir, struct scatterlist *sgl,
-			      unsigned int n_elem, unsigned long timeout)
+			      int dma_dir, void *buf, unsigned int buflen,
+			      unsigned long timeout)
 {
 	struct ata_link *link = dev->link;
 	struct ata_port *ap = link->ap;
+	struct Scsi_Host *scsi_host = ap->scsi_host;
+	struct scsi_device *sdev = scsi_host->sdev;
 	u8 command = tf->command;
 	int auto_timeout = 0;
 	struct ata_queued_cmd *qc;
-	unsigned int preempted_tag;
-	u32 preempted_sactive;
-	u64 preempted_qc_active;
-	int preempted_nr_active_links;
 	DECLARE_COMPLETION_ONSTACK(wait);
 	unsigned long flags;
 	unsigned int err_mask;
+	struct scsi_cmnd *scmd;
+	struct request *req;
 	int rc;
 
-	spin_lock_irqsave(ap->lock, flags);
+	/*
+	 * We only support a single reserved command, so this guarantees
+	 * serialization. However the code already assumed that (we are
+	 * serialized here per-port).
+	 */
+	req = scsi_alloc_request(sdev->request_queue,
+			dma_dir == DMA_TO_DEVICE ?
+			REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
+			BLK_MQ_REQ_RESERVED);
+	if (IS_ERR(req))
+		return AC_ERR_OTHER;
 
-	/* no internal command while frozen */
-	if (ap->pflags & ATA_PFLAG_FROZEN) {
-		spin_unlock_irqrestore(ap->lock, flags);
-		return AC_ERR_SYSTEM;
+
+	if (!timeout) {
+		if (ata_probe_timeout)
+			timeout = ata_probe_timeout * 1000;
+		else {
+			timeout = ata_internal_cmd_timeout(dev, command);
+			auto_timeout = 1;
+		}
 	}
 
-	/* initialize internal qc */
+	scmd = blk_mq_rq_to_pdu(req);
+	scmd->allowed = 0;
+	req->timeout = timeout;
+	//TODO: Hook up timeout handler
+	req->rq_flags |= RQF_QUIET;
+	scmd->device = sdev;
 	qc = __ata_qc_from_tag(ap, ATA_TAG_INTERNAL);
 
+	/* Do this until we can hold ata_queued_cmd in the SCMD priv data */
+	scmd->host_scribble = (unsigned char *)qc;
+
+	if (buflen) {
+		int ret = blk_rq_map_kern(sdev->request_queue, req,
+					  buf, buflen, GFP_NOIO);
+		if (ret) {
+			blk_mq_free_request(req);
+			return AC_ERR_OTHER;
+		}
+	}
+
 	qc->tag = ATA_TAG_INTERNAL;
 	qc->hw_tag = 0;
-	qc->scsicmd = NULL;
+	qc->scsicmd = scmd;
 	qc->ap = ap;
 	qc->dev = dev;
 	ata_qc_reinit(qc);
 
-	preempted_tag = link->active_tag;
-	preempted_sactive = link->sactive;
-	preempted_qc_active = ap->qc_active;
-	preempted_nr_active_links = ap->nr_active_links;
-	link->active_tag = ATA_TAG_POISON;
-	link->sactive = 0;
-	ap->qc_active = 0;
-	ap->nr_active_links = 0;
-
 	/* prepare & issue qc */
 	qc->tf = *tf;
 	if (cdb)
@@ -1525,32 +1556,14 @@  unsigned ata_exec_internal_sg(struct ata_device *dev,
 
 	qc->flags |= ATA_QCFLAG_RESULT_TF;
 	qc->dma_dir = dma_dir;
-	if (dma_dir != DMA_NONE) {
-		unsigned int i, buflen = 0;
-		struct scatterlist *sg;
-
-		for_each_sg(sgl, sg, n_elem, i)
-			buflen += sg->length;
-
-		ata_sg_init(qc, sgl, n_elem);
-		qc->nbytes = buflen;
-	}
 
-	qc->private_data = &wait;
+	qc->private_data = ap;
 	qc->complete_fn = ata_qc_complete_internal;
 
-	ata_qc_issue(qc);
-
-	spin_unlock_irqrestore(ap->lock, flags);
+	req->end_io_data = &wait;
+	req->end_io = ata_internal_end_rq;
 
-	if (!timeout) {
-		if (ata_probe_timeout)
-			timeout = ata_probe_timeout * 1000;
-		else {
-			timeout = ata_internal_cmd_timeout(dev, command);
-			auto_timeout = 1;
-		}
-	}
+	blk_execute_rq_nowait(req, true);
 
 	if (ap->ops->error_handler)
 		ata_eh_release(ap);
@@ -1610,13 +1623,15 @@  unsigned ata_exec_internal_sg(struct ata_device *dev,
 	err_mask = qc->err_mask;
 
 	ata_qc_free(qc);
-	link->active_tag = preempted_tag;
-	link->sactive = preempted_sactive;
-	ap->qc_active = preempted_qc_active;
-	ap->nr_active_links = preempted_nr_active_links;
+	link->active_tag = link->preempted_tag;
+	link->sactive = link->preempted_sactive;
+	ap->qc_active = ap->preempted_qc_active;
+	ap->nr_active_links = ap->preempted_nr_active_links;
 
 	spin_unlock_irqrestore(ap->lock, flags);
 
+	blk_mq_free_request(req);
+
 	if ((err_mask & AC_ERR_TIMEOUT) && auto_timeout)
 		ata_internal_cmd_timed_out(dev, command);
 
@@ -1647,18 +1662,20 @@  unsigned ata_exec_internal(struct ata_device *dev,
 			   int dma_dir, void *buf, unsigned int buflen,
 			   unsigned long timeout)
 {
-	struct scatterlist *psg = NULL, sg;
-	unsigned int n_elem = 0;
+	/* buf may not be aligned, so copy to/from an aligned buffer */
+	void *tmpbuf = kmemdup(buf, buflen, GFP_KERNEL);
+	unsigned res;
 
-	if (dma_dir != DMA_NONE) {
-		WARN_ON(!buf);
-		sg_init_one(&sg, buf, buflen);
-		psg = &sg;
-		n_elem++;
-	}
+	if (!tmpbuf)
+		return AC_ERR_OTHER;
 
-	return ata_exec_internal_sg(dev, tf, cdb, dma_dir, psg, n_elem,
+	res = ata_exec_internal_sg(dev, tf, cdb, dma_dir, tmpbuf, buflen,
 				    timeout);
+
+	memcpy(buf, tmpbuf, buflen);
+	kfree(tmpbuf);
+
+	return res;
 }
 
 /**
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 7a5fe41aa5ae..3cecc45d54ab 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -1258,9 +1258,10 @@  int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
 {
 	int rc = 0;
 
-	if (likely(ata_dev_enabled(ap->link.device)))
+	if (likely(ata_dev_enabled(ap->link.device)) ||
+		scsi_is_reserved_cmd(cmd)) {
 		rc = __ata_scsi_queuecmd(cmd, ap->link.device);
-	else {
+	} else {
 		cmd->result = (DID_BAD_TARGET << 16);
 		scsi_done(cmd);
 	}
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 926a7f41303d..1446a482835d 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -53,10 +53,6 @@  extern unsigned ata_exec_internal(struct ata_device *dev,
 				  struct ata_taskfile *tf, const u8 *cdb,
 				  int dma_dir, void *buf, unsigned int buflen,
 				  unsigned long timeout);
-extern unsigned ata_exec_internal_sg(struct ata_device *dev,
-				     struct ata_taskfile *tf, const u8 *cdb,
-				     int dma_dir, struct scatterlist *sg,
-				     unsigned int n_elem, unsigned long timeout);
 extern int ata_wait_ready(struct ata_link *link, unsigned long deadline,
 			  int (*check_ready)(struct ata_link *link));
 extern int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,