Hi Hannes, Just want to chime in to clarify that in the case of a 'device reset', I think the driver's use of LUN Reset to the Smart Array controller is the correct choice here. This is because the 'device' is actually a single logical unit on the controller (logical RAID volume). Sending a Target Reset to the controller would cause it to reset *all* its logical units (i.e., abort *all* pending commands on the entire controller, not just this single device), which isn't what we want here, I don't think. So, I like the code as written in the sense that it's doing a LUN Reset to the controller. I wasn't sure if your reply below implied you thought a Target Reset was correct instead, or if you were just suggesting the correct use of eh_X callbacks... so again, just wanted to clarify this one detail. Thanks, Matt > -----Original Message----- > From: Hannes Reinecke [mailto:hare@xxxxxxx] > Sent: Thursday, January 16, 2014 2:37 AM > To: Stephen M. Cameron; jbottomley@xxxxxxxxxxxxx > Cc: stephenmcameron@xxxxxxxxx; mikem@xxxxxxxxxxxxxxxxxx; Gates, Matt; > linux-scsi@xxxxxxxxxxxxxxx; Teel, Scott Stacy > Subject: Re: [PATCH 25/41] hpsa: teach hpsa_device_reset to do either > target or lun reset > > On 01/15/2014 11:38 PM, Stephen M. Cameron wrote: > > From: Scott Teel <scott.teel@xxxxxx> > > > > Signed-off-by: Scott Teel <scott.teel@xxxxxx> > > Acked-by: Stephen M. Cameron <scameron@xxxxxxxxxxxxxxxxxx> > > --- > > drivers/scsi/hpsa.c | 9 +++++---- > > 1 files changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > > index 9628e12..7cab95f 100644 > > --- a/drivers/scsi/hpsa.c > > +++ b/drivers/scsi/hpsa.c > > @@ -1858,7 +1858,8 @@ out: > > return rc; > > } > > > > -static int hpsa_send_reset(struct ctlr_info *h, unsigned char > *scsi3addr) > > +static int hpsa_send_reset(struct ctlr_info *h, unsigned char > *scsi3addr, > > + u8 reset_type) > > { > > int rc = IO_OK; > > struct CommandList *c; > > @@ -1872,8 +1873,8 @@ static int hpsa_send_reset(struct ctlr_info *h, > unsigned char *scsi3addr) > > } > > > > /* fill_cmd can't fail here, no data buffer to map. */ > > - (void) fill_cmd(c, HPSA_DEVICE_RESET_MSG, h, > > - NULL, 0, 0, scsi3addr, TYPE_MSG); > > + (void) fill_cmd(c, HPSA_DEVICE_RESET_MSG, h, NULL, 0, 0, scsi3addr, > TYPE_MSG); > > + c->Request.CDB[1] = reset_type; /* fill_cmd defaults to LUN reset > type */ > > hpsa_scsi_do_simple_cmd_core(h, c); > > /* no unmap needed here because no data xfer. */ > > > > @@ -3397,7 +3398,7 @@ static int hpsa_eh_device_reset_handler(struct > scsi_cmnd *scsicmd) > > dev_warn(&h->pdev->dev, "resetting device %d:%d:%d:%d\n", > > h->scsi_host->host_no, dev->bus, dev->target, dev->lun); > > /* send a reset to the SCSI LUN which the command was sent to */ > > - rc = hpsa_send_reset(h, dev->scsi3addr); > > + rc = hpsa_send_reset(h, dev->scsi3addr, HPSA_RESET_TYPE_LUN); > > if (rc == 0 && wait_for_device_to_become_ready(h, dev->scsi3addr) == > 0) > > return SUCCESS; > > > > > Would you mind using the correct eh_X callbacks here? > LUN Reset is _not_ identical to TARGET Reset (conceptually, at > least). And I'm currently trying to disentangle those; > idea is to have the actual device as the argument, not the command. > > Having yet another driver which mangles those only serves to > increase the patch here ... > > Thanks. > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke zSeries & Storage > hare@xxxxxxx +49 911 74053 688 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) ��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
