| Message ID | 578309AE.1080704@huawei.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 11/07/16 04:51, Bin Wu wrote: > During scsi command queueing, if mapping data fails, we need to > reclaim the failed request. Otherwise, the garbage request will > be pushed into the ring for the backend to work. Well spotted. There is another instance of this problem in scsifront_action_handler(). Would you mind correcting this one, too? Juergen > > Signed-off-by: Bin Wu <wu.wubin@huawei.com> > --- > drivers/scsi/xen-scsifront.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c > index 9dc8687..655163d 100644 > --- a/drivers/scsi/xen-scsifront.c > +++ b/drivers/scsi/xen-scsifront.c > @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host > *shost, > err = map_data_for_request(info, sc, ring_req, shadow); > if (err < 0) { > pr_debug("%s: err %d\n", __func__, err); > + info->ring.req_prod_pvt--; > scsifront_put_rqid(info, rqid); > scsifront_return(info); > spin_unlock_irqrestore(shost->host_lock, flags); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/07/16 10:33, Juergen Gross wrote: > On 11/07/16 04:51, Bin Wu wrote: >> During scsi command queueing, if mapping data fails, we need to >> reclaim the failed request. Otherwise, the garbage request will >> be pushed into the ring for the backend to work. > > Well spotted. There is another instance of this problem in > scsifront_action_handler(). Would you mind correcting this one, too? Would it make more sense to advance req_prod_pvt only if the request has been successfully created? David >> Signed-off-by: Bin Wu <wu.wubin@huawei.com> >> --- >> drivers/scsi/xen-scsifront.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c >> index 9dc8687..655163d 100644 >> --- a/drivers/scsi/xen-scsifront.c >> +++ b/drivers/scsi/xen-scsifront.c >> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host >> *shost, >> err = map_data_for_request(info, sc, ring_req, shadow); >> if (err < 0) { >> pr_debug("%s: err %d\n", __func__, err); >> + info->ring.req_prod_pvt--; >> scsifront_put_rqid(info, rqid); >> scsifront_return(info); >> spin_unlock_irqrestore(shost->host_lock, flags); > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/07/16 11:50, David Vrabel wrote: > On 11/07/16 10:33, Juergen Gross wrote: >> On 11/07/16 04:51, Bin Wu wrote: >>> During scsi command queueing, if mapping data fails, we need to >>> reclaim the failed request. Otherwise, the garbage request will >>> be pushed into the ring for the backend to work. >> >> Well spotted. There is another instance of this problem in >> scsifront_action_handler(). Would you mind correcting this one, too? > > Would it make more sense to advance req_prod_pvt only if the request has > been successfully created? Yeah, probably as the first action in scsifront_do_request(). Juergen > > David > >>> Signed-off-by: Bin Wu <wu.wubin@huawei.com> >>> --- >>> drivers/scsi/xen-scsifront.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c >>> index 9dc8687..655163d 100644 >>> --- a/drivers/scsi/xen-scsifront.c >>> +++ b/drivers/scsi/xen-scsifront.c >>> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host >>> *shost, >>> err = map_data_for_request(info, sc, ring_req, shadow); >>> if (err < 0) { >>> pr_debug("%s: err %d\n", __func__, err); >>> + info->ring.req_prod_pvt--; >>> scsifront_put_rqid(info, rqid); >>> scsifront_return(info); >>> spin_unlock_irqrestore(shost->host_lock, flags); >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> https://lists.xen.org/xen-devel >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016/7/11 17:53, Juergen Gross wrote: > On 11/07/16 11:50, David Vrabel wrote: >> On 11/07/16 10:33, Juergen Gross wrote: >>> On 11/07/16 04:51, Bin Wu wrote: >>>> During scsi command queueing, if mapping data fails, we need to >>>> reclaim the failed request. Otherwise, the garbage request will >>>> be pushed into the ring for the backend to work. >>> Well spotted. There is another instance of this problem in >>> scsifront_action_handler(). Would you mind correcting this one, too? >> Would it make more sense to advance req_prod_pvt only if the request has >> been successfully created? > Yeah, probably as the first action in scsifront_do_request(). > > > Juergen ok, I will send a new patch : ) > >> David >> >>>> Signed-off-by: Bin Wu <wu.wubin@huawei.com> >>>> --- >>>> drivers/scsi/xen-scsifront.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c >>>> index 9dc8687..655163d 100644 >>>> --- a/drivers/scsi/xen-scsifront.c >>>> +++ b/drivers/scsi/xen-scsifront.c >>>> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host >>>> *shost, >>>> err = map_data_for_request(info, sc, ring_req, shadow); >>>> if (err < 0) { >>>> pr_debug("%s: err %d\n", __func__, err); >>>> + info->ring.req_prod_pvt--; >>>> scsifront_put_rqid(info, rqid); >>>> scsifront_return(info); >>>> spin_unlock_irqrestore(shost->host_lock, flags); >>> >>> _______________________________________________ >>> Xen-devel mailing list >>> Xen-devel@lists.xen.org >>> https://lists.xen.org/xen-devel >>> >> > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index 9dc8687..655163d 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host *shost, err = map_data_for_request(info, sc, ring_req, shadow); if (err < 0) { pr_debug("%s: err %d\n", __func__, err); + info->ring.req_prod_pvt--; scsifront_put_rqid(info, rqid); scsifront_return(info); spin_unlock_irqrestore(shost->host_lock, flags);
During scsi command queueing, if mapping data fails, we need to reclaim the failed request. Otherwise, the garbage request will be pushed into the ring for the backend to work. Signed-off-by: Bin Wu <wu.wubin@huawei.com> --- drivers/scsi/xen-scsifront.c | 1 + 1 file changed, 1 insertion(+)