Re: [PATCH] qla2xxx: Fix dpc_thread race on the module unload

[Vladislav Bolkhovitin <vst@vlnb.net>]

Andrew Vasquez wrote:
> On Thu, 31 Jul 2008, Vladislav Bolkhovitin wrote:
> 
>> Gal Rosen wrote:
>>> Andrew,
>>>
>>> Add checking of the flag online still does not answer the race that Vlad
>>> stated before,
>> Yes, that's true.
>>
>> And, since ha->flags.online set to 0 not only on shutdown, I'm afraid you 
>> could introduce a new set of subtle bugs, if not for the moment, but in the 
>> future, because with your patch it gets impossible to wake up the DPC 
>> thread if HA is offline.
> 
> You are both missing a subtle point, it's incidental if
> qla2xxx_wake_dpc() misses a 'wake-up' due the HBA being 'offline', as
> the qla2x00_timer(), woken up every second, will do the wake-up if
> necessary. The code changes offered, close the window as tightly as
> possible without introducing needlessly complex changes.  All this
> infrastructure is legacy constucts from a time long-before work-queues
> and the like.
> 
> This area is one of several where we are moving to clean-up and
> modernize in our upstream offering:
> 
> * dropping the heavy DPC thread in favor of work-queues.
> * using proper life-cycle and reference-handling of fcport objects
>   (yes, the objects will be freed after use).
> * refactoring the HA respresentations for physical and virtual ports,
>   the current memcpy() physical-HA to vport-HA and slight-mods is
>   error-prone and doesn't scale.
> * drop the illogical single physical-HA maintains all fcports across
>   N-vports.

Nice to hear that!

> The changes above are large (170k diffs so far), and at this point are
> being run-through our testing.  The hope is to get the changes
> upstream during one of the next two merge windows.
> 
> Given the infrustructure mods and our focus on that front, if there's
> something small and contained you can offer above what I've proposed
> we'll be interested in reviewing any patches you'd push forward.

Then, I believe, my patch should go in as a temporal measure. I don't 
think we should crash users for 2 more major releases. The same is true 
for my other patch "Proposed protection of fcports field of struct 
scsi_qla_host" as well, because without it there should be no big 
problems to crash the driver via sysfs.

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