How is supposed to be protected the units list?

[Alexander Motin]

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.

> 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.

-- 
Alexander Motin