pte_update and 64-bit PTEs on PPC32?

[Benjamin Herrenschmidt]

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>