On Mon, 2017-08-28 at 08:49 +0200, Greg KH wrote:
> On Mon, Aug 28, 2017 at 04:41:27PM +1000, Stephen Rothwell wrote:
> > Hi James,
> >
> > Today's linux-next merge of the scsi tree got a conflict in:
> >
> > drivers/staging/unisys/visorhba/visorhba_main.c
> >
> > between commits:
> >
> > 781facd05eb9 ("staging: unisys: visorhba: visorhba_main.c: fixed comment formatting issues")
> >
> > from the staging tree and commit:
> >
> > 7bc4e528d9f6 ("scsi: visorhba: sanitze private device data allocation")
> >
> > from the scsi tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging. You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
>
> Ick, messy merge, thanks for doing this.
Hello Greg,
If you agree with the following, please communicate this to the visorhba
authors:
* Most SCSI drivers exist under drivers/scsi, including the virtio-scsi and
xen-scsifront drivers. So why has the visorhba driver been added under
unisys/visorhba?
* Since the SCSI core already keeps track of which commands are pending, the
visorhba driver should not do this. I'm referring here to the scsipending
data structure and the pending[] array in struct visorhb_devdata.
* The pending[] array should be removed and should be converted into
driver-private command data by setting the .cmd_size member of the SCSI host
template.
* scsi_host_find_tag() should be used when a command completion is received
to look up the pointer of the SCSI command that has been completed.
* All code that calls for_each_vdisk_match() looks buggy to me in some way.
E.g. visorhba_abort_handler() should only affect the command passed as an
argument to that function and no other SCSI commands.
* The device and bus reset handlers should call scsi_done() for all affected
commands if forward_taskmgmt_command() succeeds instead of only the command
passed as an argument.
* The for_each_vdisk_match() macro does not protect against concurrent device
removal and hence is unsafe. This macro should be removed and in contexts
where it is *really* needed to iterate over SCSI devices
shost_for_each_device() should be used instead.
* Instead of creating a kernel thread to poll for responses the visorhba driver
should only process responses after it has received an interrupt that tells it
to do so (see also the visor_thread_start() calls).
* SCSI drivers should not have special-case code for host shutdown.
* The 'dev_info_list' member in struct visorhba_data is not used and hence
should be removed.
Please note that I have not attempted to do a full review of the visorhba driver.
Thanks,
Bart.
��.n��������+%������w��{.n�����{��w����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
