[SSSD] [PATCH 3/3] mmap cache: invalidate cache on fatal error

Pavel Březina pbrezina at redhat.com
Thu Dec 20 15:14:11 UTC 2012


On 12/20/2012 02:20 PM, Simo Sorce wrote:
> On Thu, 2012-12-20 at 12:29 +0100, Pavel Březina wrote:
>> On 12/20/2012 06:10 AM, Simo Sorce wrote:
>>> -static errno_t sss_mc_get_record(struct sss_mc_ctx *mcc,
>>> +static errno_t sss_mc_get_record(struct sss_mc_ctx **_mcc,
>>>                                     size_t rec_len,
>>>                                     struct sized_string *key,
>>>                                     struct sss_mc_rec **_rec)
>>>    {
>>> +    struct sss_mc_ctx *mcc = *_mcc;
>>>        struct sss_mc_rec *old_rec = NULL;
>>>        struct sss_mc_rec *rec;
>>>        int old_slots;
>>> @@ -424,6 +425,16 @@ static errno_t sss_mc_get_record(struct sss_mc_ctx *mcc,
>>>        /* we are going to use more space, find enough free slots */
>>>        ret = sss_mc_find_free_slots(mcc, num_slots, &base_slot);
>>>        if (ret != EOK) {
>>> +        if (ret == EFAULT) {
>>> +            TALLOC_CTX *parent;
>>> +            DEBUG(SSSDBG_CRIT_FAILURE,
>>> +                  ("Fatal internal mmap cache error, invalidating cache!\n"));
>>> +            parent = talloc_parent(mcc);
>>> +            if (parent == NULL) {
>>> +                talloc_zfree(*_mcc);
>>> +            }
>>> +            (void)sss_mmap_cache_reinit(parent, -1, -1, _mcc);
>>> +        }
>>>            return ret;
>>>        }
>>
>> Hi,
>>
>> Is if (parent == NULL) only precaution or can it actually happen?
>
> Only a precaution, all the callers use an actual memory context.
>
>> If you talloc_zfree(*_mcc) then you will hit EINVAL in reinit() and the
>> cache won't be reinitialized. Is this intentional? If so, can you
>> comment it in the code?
>
> Well it was not intentional I meant to return after the free.
> However I am wondering if we should really care ...
>
> If the caller had NULL as the parent something may have gone wrong, but
> at worst we will leak a few bytes... maybe I shouldn't consider a NULL
> parent as an error after all. Opinions ?
>
> Simo.
>

Does the memory cache require a parent context for some reason (do you 
manipulate with the parent within the cache)?

If not I think you should not take care whether is has a parent or not. 
For what we know, it may be intentional by the caller. Otherwise you 
should check if mem_ctx != NULL in init().

Looking into the init() function I found out that you are using wrong 
context:

     mc_ctx->name = talloc_strdup(mem_ctx, name);
     if (!mc_ctx->name) {
         ret = ENOMEM;
         goto done;
     }

It should say mc_ctx not mem_ctx.




More information about the sssd-devel mailing list