lkml.org 
[lkml]   [2020]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 15/17] ibmvfc: send Cancel MAD down each hw scsi channel
    From
    Date
    On 12/1/20 6:53 PM, Tyrel Datwyler wrote:
    > In general the client needs to send Cancel MADs and task management
    > commands down the same channel as the command(s) intended to cancel or
    > abort. The client assigns cancel keys per LUN and thus must send a
    > Cancel down each channel commands were submitted for that LUN. Further,
    > the client then must wait for those cancel completions prior to
    > submitting a LUN RESET or ABORT TASK SET.
    >
    > Allocate event pointers for each possible scsi channel and assign an
    > event for each channel that requires a cancel. Wait for completion each
    > submitted cancel.
    >
    > Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
    > ---
    > drivers/scsi/ibmvscsi/ibmvfc.c | 106 +++++++++++++++++++++------------
    > 1 file changed, 68 insertions(+), 38 deletions(-)
    >
    > diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
    > index 0b6284020f06..97e8eed04b01 100644
    > --- a/drivers/scsi/ibmvscsi/ibmvfc.c
    > +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
    > @@ -2339,32 +2339,52 @@ static int ibmvfc_cancel_all(struct scsi_device *sdev, int type)
    > {
    > struct ibmvfc_host *vhost = shost_priv(sdev->host);
    > struct ibmvfc_event *evt, *found_evt;
    > - union ibmvfc_iu rsp;
    > - int rsp_rc = -EBUSY;
    > + struct ibmvfc_event **evt_list;
    > + union ibmvfc_iu *rsp;
    > + int rsp_rc = 0;
    > unsigned long flags;
    > u16 status;
    > + int num_hwq = 1;
    > + int i;
    > + int ret = 0;
    >
    > ENTER;
    > spin_lock_irqsave(vhost->host->host_lock, flags);
    > - found_evt = NULL;
    > - list_for_each_entry(evt, &vhost->sent, queue) {
    > - if (evt->cmnd && evt->cmnd->device == sdev) {
    > - found_evt = evt;
    > - break;
    > + if (vhost->using_channels && vhost->scsi_scrqs.active_queues)
    > + num_hwq = vhost->scsi_scrqs.active_queues;
    > +
    > + evt_list = kcalloc(num_hwq, sizeof(*evt_list), GFP_KERNE> + rsp = kcalloc(num_hwq, sizeof(*rsp), GFP_KERNEL);

    Can't this just go on the stack? We don't want to be allocating memory
    during error recovery. Or, alternatively, you could put this in the
    vhost structure and protect it with a mutex. We only have enough events
    to single thread these anyway.

    > +
    > + for (i = 0; i < num_hwq; i++) {
    > + sdev_printk(KERN_INFO, sdev, "Cancelling outstanding commands on queue %d.\n", i);

    Prior to this patch, if there was nothing outstanding to the device and cancel_all was called,
    no messages would get printed. This is changing that behavior. Is that intentional? Additionally,
    it looks like this will get a lot more vebose, logging a message for each hw queue, regardless
    of whether there was anything outstanding. Perhaps you want to move this down to after the check
    for !found_evt?

    > +
    > + found_evt = NULL;
    > + list_for_each_entry(evt, &vhost->sent, queue) {
    > + if (evt->cmnd && evt->cmnd->device == sdev && evt->hwq == i) {
    > + found_evt = evt;
    > + break;
    > + }
    > }
    > - }
    >



    --
    Brian King
    Power Linux I/O
    IBM Linux Technology Center

    \
     
     \ /
      Last update: 2020-12-02 19:30    [W:5.044 / U:0.824 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site