Re: ATA identify retries exceeded - kernel panic

["Thomas E. Spanjaard" <tgen@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx

Attachment: signature.asc
Description: OpenPGP digital signature