Re: tmpfs status

[Maksym Planeta]

Samuel Thibault <samuel.thibault@gnu.org> writes:

> Maksym Planeta, le Sat 07 Apr 2012 21:42:04 +0300, a écrit :
>> Samuel Thibault <samuel.thibault@gnu.org> writes:
>> 
>> > Maksym Planeta, le Sat 07 Apr 2012 19:51:56 +0300, a écrit :
>> >> Here is initialization code from pager_alloc():
>> >>           if (INDIRECT_PAGEMAP(size)) {
>> >>                   alloc_size = INDIRECT_PAGEMAP_SIZE(size);
>> >>                   init_value = (dp_map_t)0;
>> >> 
>> >> And from pager_extend():
>> >>       for (; i < INDIRECT_PAGEMAP_ENTRIES(new_size); i++)
>> >>           new_mapptr[i].indirect = (dp_map_t)0;
>> >> 
>> >> As you can see instead of NULL, (dp_map_t)0 is used.
>> >
>> > And can be put into pager->map in pager_truncate, ok. I'm however not
>> > sure we really want to put ifs everywhere. The comment in the truncation
>> > says
>> >
>> >      /* We are truncating to a size small enough that it goes to using
>> >         a one-level map.  We already have that map, as the first and only
>> >         nonempty element in our indirect map.  */
>> >
>> > i.e. the code assumes that map[0].indirect is not NULL. I'd say we
>> > should rather allocate an empty map in such case, to keep the rest of
>> > the code simple.
>> >
>> 
>> And what is the alternative for ifs? longjumps and setjumps?
>
> No: as I said, allocate an empty map, so that the existing code can poke
> at it without testing for its presence or not.
>
>> Purpose of this conditions is checking whether map (or submap) is
>> already empty.
>
> Not empty, but allocated.
>

So, if, for instance, only one page of large object (that needs indirect
mapping) was evicted, the whole map would be allocated? And what the
purpose of two-level system in this case?

>> >> > There is also an issue with
>> >> >
>> >> > +           if (!pager->map) {
>> >> > +                invalidate_block (pager_offset);
>> >> > +                goto done;
>> >> > +            }
>> >> >             pager_offset = pager->map[f_page];
>> >> >
>> >> > at that point, pager_offset is not initialized yet...
>> >> >
>> >> 
>> >> invalidate_block is a macro that sets pager_offset, so, really,
>> >> pager_offset shouldn't been initialized yet.
>> >
>> > It sets the *content* pointed by pager_offset. It does not set the
>> > pager_offset pointer.
>> 
>> pager_offset is not a pointer, it is a union.
>
> Oops, indeed, sorry about that. I'm still wondering, however: rather
> than a goto, why not just putting pager_offset = pager->map[f_page] in
> the else part?
>

pager_offset = pager->map[f_page] could be just put in else block, but I
did so because "goto done" shows clearer that everything was done and
execution could be move to finalization part. Just a matter of style. Do
you think that this should be changed?

Regards,
Maksym Planeta.