On Wed, Feb 15, 2017 at 7:17 AM, Dilip Kumar <dilipbal...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers