On Apr 6, 2005, at 5:22 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2005-04-06 at 11:44 -0500, Kumar Gala wrote:
>
> > Ben, I agree with you about having the flags in a single word so we
> can
> > lock them properly.? In the short term it appears that the issue I'm
> > running into is explicit with ptep_get_and_clear():
> >
> > static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned
> > long addr,
> >???????????????????????????????????????? pte_t *ptep)
> > {
> >????????? return __pte(pte_update(ptep, ~_PAGE_HASHPTE, 0));
> > }
> >
> > It appears that we should be returning the pte that was passed in,
> > before its modified?? (seems a little silly to me, why bother, the
> > caller could do this -- i've posted to lkml on the issue?).
>
> No, we should return the "old" PTE.
>
> >?? Anyways,
> > since pte_update only returns the lower 32-bits the wrong thing
> > happens.? The following seems to be a better implementation of
> > ptep_get_and_clear() for ppc32 which resolves my issue:
> >
> > static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned
> > long addr,
> >???????????????????????????????????????? pte_t *ptep)
> > {
> >????????? pte_t tmp = *ptep;
> >????????? pte_update(ptep, ~_PAGE_HASHPTE, 0);
> >????????? return tmp;
> > }
>
> Hrm... I would still do it differently. I would read the "rpn only"
> part
> non atomically and load/clear the other half atomically. Withotu that,
> you may miss a bit set between the load and the update (for example,
> _PAGE_HASHPTE may have been put in in between).
Ben,
I'll let you catch up with the rest of the thread, I realized most of
what you have here and created a new implementation of pte_update that
should be doing what you are asking for.
- kumar
