bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: tmpfs status


From: Samuel Thibault
Subject: Re: tmpfs status
Date: Sat, 7 Apr 2012 19:03:35 +0200
User-agent: Mutt/1.5.21+34 (58baf7c9f32f) (2010-12-30)

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.

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

> > Also I don't understand the use of
> >
> > +               if (no_block (*pager->map))
> > +                   goto done;
> >                 entry = pager->map[offset];
> >                 invalidate_block(pager->map[offset]);
> >
> > *pager->map would be pager->map[0], I'd believe that it is independent
> > from pager->map[offset], and clearing it can only bring issues.
> >
> 
> Sorry, didn't get you.

*pager->map is the same as pager->map[0]. AIUI, it has nothing to do
with pager->map[offset]. So why testing it at all?

Samuel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]