----- Original Message ----- > In an enclosure model where there are chaining expanders to a large body > of storage, it was discovered that libsas, responding to a broadcast > event change, would only revalidate the domain of first child expander > in the list. > > The issue is that the pointer value to the discovered source device was > used to break out of the loop, rather than the content of the pointer. > > This still remains non-compliant as the revalidate domain code is > supposed to loop through all child expanders, and not stop at the first > one it finds that reports a change count. However, the design of this > routine does not allow multiple device discoveries and that would be a > more complicated set of patches reserved for another day. We are fixing > the glaring bug rather than refactoring the code. Obviously I've tested this both when I was at Adaptec and at Vitesse. I'd connect 7-8 expanders, run iogen with 1000 threads to say 30-40 disks, and then unplug the port between a level 1 and 2 expander and the I/O would quiesce, iogen would report a subset of the disks missing, and then when the port was reestablished, I/O would restart. However I'm not sure that Bottomley tested this scenario after changing my code off-line before submitting it into the Linux kernel. Now a few notes to mention: Your patch patches a function called sas_find_bcast_dev(). My original code does NOT have such a function. Revalidation is much more subtle and the code looks simpler in my original version. In my original code there is a lot more recursion, symmetry and code mirroring. Granted, while such code is shorter, and simpler, it is harder to figure out what it does, and I feel this is exactly why we see the current state of libsas to be so explicit, simplistic and introducing bugs. See this: http://marc.info/?l=linux-scsi&m=131480962006471&w=2 where I described the state of libsas recently. > Please note, as I am *stuck* on Outlook as per company policy, the > following inline content will likely not patch clean even emailed as > 'Plain Text', the enclosed attached file should do the job. I have > Cc'd > all the folks that originated the files in libsas, as there was no > listed MAINTAINERs. > > Checkpatch.pl reports clean. Patch applies cleanly to a WIDE variety of > kernels up to latest. > > Sincerely -- Mark Salyzyn > > Cc: Luben Tuikov <tuikov@xxxxxxxxx> > Cc: Darrick J Wong <djwong@xxxxxxxxxx> > Cc: James Bottomley <jbottomley@xxxxxxxxxxxxx> > > Signed-off-by: Mark Salyzyn <msalyzyn@xxxxxxxxxxxxxx> > > sas_expander.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff -ru scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c > scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c > --- scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c 2011-08-31 > 08:32:21.000000000 -0400 > +++ scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c > 2011-09-01 08:57:55.000000000 -0400 > @@ -1721,7 +1721,7 @@ > list_for_each_entry(ch, &ex->children, siblings) { > if (ch->dev_type == EDGE_DEV || ch->dev_type == > FANOUT_DEV) { > res = sas_find_bcast_dev(ch, src_dev); > - if (src_dev) > + if (*src_dev) > return res; > } > } > -- 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
