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).
> If we are ok with this I'll send a patch upstream for it. I'd like to
> still discuss how to make this all proper long term. Currently,
> ptep_get_and_clear was the only user of pte_update that used the return
> value for anything but flags. One change would be for it to return
> just the flags portion of the pte it was given. Another would be for
> us to implement a proper 64-bit pte version of pte_update.
>
> - kumar
>
--
Benjamin Herrenschmidt <benh at kernel.crashing.org>
