Maksym Planeta, le Sun 08 Apr 2012 00:17:43 +0300, a écrit :
> Samuel Thibault <samuel.thiba...@gnu.org> writes:
> > 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.
> >
>
> But thus you will not avoid conditions, because most of them are applied
> to second level.
Existing conditions, agreed. But the conditions you introduce are about
one-level only.
> Also you will need to allocate memory map for every object is created
> even it would not ever use pageout.
Well, that's already what we do in pager_alloc, don't we?
> >> >> >> > 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.
>
> But pager_offset is filled anyway: either in condition or after.
Yes, so why using a "goto" and not a mere "else"? AIUI, the whole point
of that part of the code is to set pager_offset.
Samuel
