On Wed, Feb 15, 2017 at 7:17 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> This is easily doable and it will look much cleaner. While doing this
> I am facing one problem related to
> relptr_store and relptr_access. The problem is that when I call
> "relptr_store (base, rp, val)" with both base and val as a same
> address then it will store offset as 0 which is fine, but when we use
> relptr_access with 0 offset it is returning NULL instead of giving the
> actual address. I expect it should return directly base when offset is
> 0. (I am facing this problem because in this case (TBM_ONE_PAGE),
> there will be only one page and address of that will be same as
> base.).
>
> There can be multiple solutions to this but none of them looks cleaner to me.
> sol1: If relptr_access return NULL then directly use the base address
> as our PagetableEntry.
> sol2: Instead of using base as start of the element array, just try to
> use some modified base as e.g base=base - some number.
> sol3: change relptr_access to not return NULL in this case. But, this
> will change the current behaviour of this interface and need to
> analyze the side effects.
Hmm, yeah, that's a problem. How about not using relative pointers
here, and instead just using array indexes?
>> I don't see why you think you need to add sizeof(dsa_pointer) to the
>> allocation and store an extra copy of the dsa_pointer in the
>> additional space. You are already storing it in tbm->dsa_data and
>> that seems good enough. pagetable_free() needs the value, but it has
>> a pointer to the TIDBitmap and could just pass tbm->dsa_data directly
>> instead of fishing the pointer out of the extra space.
>
> If you see the code of SH_GROW, first it needs to allocate the bigger
> chunk copy data from smaller chunk to the bigger chunk and then free
> the smaller chunk.
Oh, I see.
> So by the time it call the pagetable_free, it would have already
> called the pagetable_allocate for the newer bigger chunk of memory so
> now, tbm->dsa_data points to the latest memory, but pagetable_free
> wants to free older one.
>
> One solution to this could be that I keep two dsa_pointer in TBM, one
> point to old memory and another to new. (After this here we will get
> the same problem of relptr_access because now we will have first entry
> pointer is same as base pointer)
Yes, two dsa_pointers seems fine. Maybe that's not totally beautiful,
but it seems better than what you had before.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company