Re: tmpfs status

[Samuel Thibault]

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.

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

Samuel