On Sat, Mar 10, 2018 at 11:15:20AM +0100, Christoph Hellwig wrote:
> This looks generally fine to me:
> 
> Reviewed-by: Christoph Hellwig <h...@lst.de>
> 
> As a follow on we should probably kill virtscsi_queuecommand_single and
> thus virtscsi_host_template_single as well.
> > Given storage IO is always C/S model, there isn't such issue with 
> > SCSI_MQ(blk-mq),
> 
> What does C/S mean here?

Client–Server.

> 
> > @@ -580,10 +573,7 @@ static int virtscsi_queuecommand_single(struct 
> > Scsi_Host *sh,
> >                                     struct scsi_cmnd *sc)
> >  {
> >     struct virtio_scsi *vscsi = shost_priv(sh);
> > -   struct virtio_scsi_target_state *tgt =
> > -                           scsi_target(sc->device)->hostdata;
> >  
> > -   atomic_inc(&tgt->reqs);
> >     return virtscsi_queuecommand(vscsi, &vscsi->req_vqs[0], sc);
> >  }
> 
> >  static int virtscsi_queuecommand_multi(struct Scsi_Host *sh,
> >                                    struct scsi_cmnd *sc)
> >  {
> >     struct virtio_scsi *vscsi = shost_priv(sh);
> > -   struct virtio_scsi_target_state *tgt =
> > -                           scsi_target(sc->device)->hostdata;
> > -   struct virtio_scsi_vq *req_vq;
> > -
> > -   if (shost_use_blk_mq(sh))
> > -           req_vq = virtscsi_pick_vq_mq(vscsi, sc);
> > -   else
> > -           req_vq = virtscsi_pick_vq(vscsi, tgt);
> > +   struct virtio_scsi_vq *req_vq = virtscsi_pick_vq_mq(vscsi, sc);
> >  
> >     return virtscsi_queuecommand(vscsi, req_vq, sc);
> 
> Given how virtscsi_pick_vq_mq works virtscsi_queuecommand_single and
> virtscsi_queuecommand_multi now have identical behavior.  That means
> virtscsi_queuecommand_single should be removed, and
> virtscsi_queuecommand_multi should be merged into virtscsi_queuecommand,

OK.

> 
> > @@ -823,6 +768,7 @@ static struct scsi_host_template 
> > virtscsi_host_template_single = {
> >     .target_alloc = virtscsi_target_alloc,
> >     .target_destroy = virtscsi_target_destroy,
> >     .track_queue_depth = 1,
> > +   .force_blk_mq = 1,
> 
> This probably isn't strictly needed.  That being said with your
> change we could probably just drop virtscsi_host_template_single entirely.
> 

OK.

Thanks,
Ming

Reply via email to