On Wed, 12 Feb 2014 11:13:22 -0800
Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>
> Acked-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>
> I think this is the right move, and some follow-up cleanups / fixes
> come to mind:
Dan -- Thanks for taking a look. A few comments below.
> 1/ Why is sas_bsg_remove() multiplexing 2 use cases? Let's just have
> a sas_host_bsg_remove() and a sas_device_bsg_remove().
Splitting the two use cases reduces them into a simple conditional call
to bsg_unregister_queue(), that could just be done in place. On the
other hand, sas_bsg_initialize() looks like it handles both cases as
well.
> 2/ I think sas_rphy_free() mishandles the reference counts in the
> sas_rphy_add() failing case.
> sas_rphy_free() does:
> transport_destroy_device(dev); /* which is just put_device(dev) */
> put_device(dev);
>
> ...but if sas_rphy_add() failed we never took the reference from
> transport_add_device() and I expect we only have the one reference
> from device_initialize() to drop.
>
> We also ignore the return code from transport_add_device() which is
> another hole.
Ugh, I think fixing this properly would require piping the return code
of the trigger function back out of the attribute container code:
void transport_add_device()
void attribute_container_device_trigger()
loop through the attribute_container_list
if match
int transport_add_class_device()
... return error ...
Not to mention several callers of sas_rphy_add keep on truckin' when it
does return failure.
Perhaps this can be handled in another way?
> Commenting on this mail should refresh this in James' queue. But you
> may want to resend anyways given this is a few months old.
Whatever is easier for James.
Regards,
-- Joe
--
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
