How is supposed to be protected the units list?

[Attilio Rao]

2010/3/4 Alexander Motin <mav at freebsd.org>:
> Hi.
>
> Attilio Rao wrote:
>> 2010/3/1 Attilio Rao <attilio at freebsd.org>:
>>> I have a question that I've been unable to reply reading the code.
>>> Someone could point me to documentation explaining how the unit tailq
>>> (within a struct periph_driver) is supposed to be locked?
>>> I'm not sure how it is assured consistency of accesses to the list and
>>> more important how is ensured that the periphs composing it doesn't go
>>> away as I don't see any reference bump for objects inserted there.
>
> I am not sure that existing implementation is complete, but at least in
> some places it is protected by xpt_lock_buses(), which is wrapper for
> mtx_lock(&xsoftc.xpt_topo_lock). camperiphfree() called under the lock,
> same as many (all?) traverses.

Initially I thought about using xpt_lock_buses() but then I regret in
order to avoid any unconvenient LOR.
Using a new lock would decrease the chances to see this happening.

>> I don't think the lists are protected at all so I made this simple
>> patch taking advantage by a global lock:
>> http://www.freebsd.org/~attilio/Sandvine/pdrv/pdrv_lock.diff
>>
>> The patch is simple enough but I just test-compiled it (will need some
>> time to run in a debugging kernel, hope to do tonight) and maybe you
>> can already give your opinions here.
>
> It is not obvious to me, how your new lock expected to interoperate with
> existing xpt_topo_lock. I have feeling that it duplicates one in some
> places. If you believe that second lock is needed there, then probably
> first one should be partially removed. But then we should be careful to
> not create LORs between them, as they are not natively nested.
>
> Changes in cam_xpt.c break splbreaknum meaning of temporary dropping
> xpt_topo_lock. I have doubts whether it is required now at all now, but
> leaving dead code it definitely not good.

If you think that using xpt_topo_lock in all the other places
accessing units that don't do right now is suitable and safe I'm fine
with it. Otherwise the new global lock should be fine I assume.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein