| Message ID | 1654770559-101375-9-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk-mq/libata/scsi: SCSI driver tagging improvements | expand |
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,
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 --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,
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(-)