ATA identify retries exceeded - kernel panic

[Thomas E. Spanjaard]

Matthew Dillon wrote:
:Fwiw, I haven't seen such yet with the new ATA code, except in the case 
:when I forgot to zero the structs I pulled out of my objcache. It might 
:be nice to have objcache_get() look at oc_flags and zero if & M_ZERO.
    The idea behind the freelist caching layer is to be able to hold the
    structures in a form that reduces the amount of initialization required.
    This is primarily used by the mbuf system, where certain associations
    (such as mbufs with mbuf cluster buffers) are retained in the freelist
    caching layer and only disentangled in the allocation and dispoal layer.

    So we can't just unconditionally bzero() in objcache_get() if M_ZERO is
    set because it would destroy these associations.  I think what we need
    to do is give the objcache some indication that M_ZERO is allowed, then
    either bzero() or panic.
    Since bzero is a procedure call, perhaps the solution here is to add
    another function pointer for 'zeroing' a structure when M_ZERO is
    specified.  This function would be specified in objcache_create() and
    default to a bzero if NULL.  This way M_ZERO will work as expected with
    simple objcache setups, and panic the system if someone tries to use
    it with more complex objcache setups (like mbufs).
I first attempted to do the zeroing in a ctor I passed to 
objcache_create(), but that only works the first time my data is 
objcache_get() from the objcache. If it's objcache_put() again, and then 
objcache_get(), it would contain the old data. So I also set up a 
zeroing dtor, but then I thought it to be too much effort for the common 
case (usually, there aren't many objects out of the cache at the same 
time in my case, so a limited number is reused all the time), so I 
wrapped objcache_get() in a function that does the zeroing. I expect 
that to result in less calls to bzero() overall.

Regarding the extra function pointer, that sounds good, but I'd like it 
more generic: that the extra pointer is for generic objcache_get()-time 
related actions, not just zeroing based on M_ZERO set in oc_flags. The 
default could well do something like that, but it shouldn't be 
documented as being a func pointer for zeroing funcs only.

On the other hand, one could argue that if one specifies M_ZERO in 
oc_flags, one expects a completely zeroed object, with no initialization 
whatsoever (panic if M_ZERO is set and a ctor is specified?).

Cheers,
--
        Thomas E. Spanjaard
        tgen at xxxxxxxxxxxxx
Attachment:
signature.asc
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pgp00006.pgp
Type: application/octet-stream
Size: 186 bytes
Desc: "Description: OpenPGP digital signature"
URL: <http://lists.dragonflybsd.org/pipermail/bugs/attachments/20061129/8c9eeb85/attachment-0017.obj>