Re: [v2 PATCH 4/5] bnx2fc: Broadcom FCoE Offload driver submission - part 2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2011-02-02 at 01:24 -0800, Mike Christie wrote: 
> On 01/17/2011 06:37 PM, Bhanu Gollapudi wrote:
> >>
> >> Are you sending a abort when a lun or target reset has been successfully
> >> completed? Does your hw need the reset? SCSI spec wise the lun or target
> >> reset will take care of the scsi commands on its side, so there is no
> >> need to send an abort.
> >
> > It is better to send ABTS on all the outstanding IOs after issuing
> > lun/target reset.  targets may take care of scsi commands on its side.
> > But the commands on the flight may have to be aborted. This is as per
> > the FCP-4 spec in "Table 8 - Clearing effects of initiator FCP-Port
> > actions" :-
> > "Exchanges are cleared internally within the target FCP_Port, but open
> > FCP Sequences shall be individually aborted by the initiator FCP_Port
> > using ABTS-LS that also has the effect of aborting the associated FCP
> > Exchange. See 12.3."
> 
> Ughh. I must have misread that before. I think libfc is broken then.
> 
> 
> 
> >>
> >>> +}
> >>> +
> >>> +static int bnx2fc_map_sg(struct bnx2fc_cmd *io_req)
> >>> +{
> >>> +     struct bnx2fc_hba *hba = io_req->port->hba;
> >>> +     struct scsi_cmnd *sc = io_req->sc_cmd;
> >>> +     struct fcoe_bd_ctx *bd = io_req->bd_tbl->bd_tbl;
> >>> +     struct scatterlist *sg;
> >>> +     int byte_count = 0;
> >>> +     int sg_count = 0;
> >>> +     int bd_count = 0;
> >>> +     int sg_frags;
> >>> +     unsigned int sg_len;
> >>> +     u64 addr;
> >>> +     int i;
> >>> +
> >>> +     sg = scsi_sglist(sc);
> >>> +     sg_count = pci_map_sg(hba->pcidev, sg, scsi_sg_count(sc),
> >>> +                           sc->sc_data_direction);
> >>
> >>
> >> I think you could also use scsi_dma_map instead.
> >>
> >> And if not I think we are supposed to be using the dma functions instead
> >> of the pci ones.
> >
> > OK. I can use dma_map_sg() intead. But this function inturn calls
> > pci_map_sg(). Is there any reason why we cannot directly call it?
> >
> 
> People like abstractions. I guess we are trying to move to the dma 
> functions. If you had some other bus then the dma wrappers would hide that.

We handles this anyway.

> 
> 
> >
> >>
> >>
> >>
> >>> +
> >>> +static int bnx2fc_post_io_req(struct bnx2fc_rport *tgt,
> >>> +                            struct bnx2fc_cmd *io_req)
> >>> +{
> >>
> >>
> >>> +
> >>> +     /* Time IO req */
> >>> +     bnx2fc_cmd_timer_set(io_req, BNX2FC_IO_TIMEOUT);
> >>
> >> Hard coding this does not seem right becuase the scsi cmnd timer can be
> >> set through sysfs. It could be set lower than this which makes it
> >> useless. I think libfc is dropping their similar timer like this and
> >> just relying on the scsi_cmnd timer now (I think they just do the rec
> >> but not the abort now).
> >
> > if it is a lower value, eh_abort_handler() code kicks in. Default scsi
> > cmd timer is 60 secs, which may be too long to wait to sends an ABTS.
> 
> 
> Do not work around scsi-ml and the classes. If really useful it seems 
> this should be in libfc or the fc class so all drivers do the same thing 
> if capable.
> 
> Also if the abort fails you still have to wait for the scsi eh to run.
> 
> And if the the scsi timer is too long why don't you just set it lower?
> 
> Also, it is 30 secs by default in the kernel, but some distro's have 
> udev's that set it to 60.


On 02/02/2011 03:24 AM, Mike Christie wrote:
> >
> > Also if the abort fails you still have to wait for the scsi eh to
> run.
> >
> 
> Actually you do not have to wait for the scsi eh to run, right. It
> looks 
> like bnx2fc would log out the port, which ends up calling 
> fc_remote_port_delete and that would cause the fc timed out function
> to 
> return BLK_EH_RESET_TIMER to prevent the scsi eh from running. Is
> that 
> right? That type of eh strategy behavior seems like something you
> want 
> to sync up with libfc or the fc class so all drivers do something
> similar.

As per FCP-4, if the ABTS times out, we will have to explicitly LOGO the
target and relogin back. If we rely on 60 sec eh_abort_handler, and if
ABTS times out, SCSI error handling will go to LUN RESET, TGT reset
path, which is a generic error handling than transport specific error
handling.



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux