Maksym Planeta, le Sat 07 Apr 2012 23:20:13 +0300, a écrit :
> > 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?
I'm not saying to fill the whole two-level system.
I'm saying to allocate the one-level system when truncation brings from
two-level to one-level while the first map of the two-level happens to
be NULL.
> >> >> > 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?
In both cases pager_offset is filled with something, so I don't see why
it should be different. In the indirect map case, it's a mere else.
Samuel
