2005-04-06 16:38:59

by Kumar Gala

[permalink] [raw]
Subject: return value of ptep_get_and_clear

ptep_get_and_clear has a signature that looks something like:

static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned
long addr,
pte_t *ptep)

It appears that its suppose to return the pte_t pointed to by ptep
before its modified. Why do we bother doing this? The caller seems
perfectly able to dereference ptep and hold on to it. Am I missing
something here?

If not, I'll work up a set of patches to change ptep_get_and_clear and
its callers for post 2.6.12 release.

- kumar


2005-04-06 16:49:09

by Nick Piggin

[permalink] [raw]
Subject: Re: return value of ptep_get_and_clear

Kumar Gala wrote:
> ptep_get_and_clear has a signature that looks something like:
>
> static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned
> long addr,
> pte_t *ptep)
>
> It appears that its suppose to return the pte_t pointed to by ptep
> before its modified. Why do we bother doing this? The caller seems
> perfectly able to dereference ptep and hold on to it. Am I missing
> something here?
>

You need to be able to *atomically* clear the pte and retrieve the
old value.

Nick

--
SUSE Labs, Novell Inc.

2005-04-06 17:08:57

by Brian Gerst

[permalink] [raw]
Subject: Re: return value of ptep_get_and_clear

Kumar Gala wrote:
> ptep_get_and_clear has a signature that looks something like:
>
> static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned
> long addr,
> pte_t *ptep)
>
> It appears that its suppose to return the pte_t pointed to by ptep
> before its modified. Why do we bother doing this? The caller seems
> perfectly able to dereference ptep and hold on to it. Am I missing
> something here?
>
> If not, I'll work up a set of patches to change ptep_get_and_clear and
> its callers for post 2.6.12 release.
>
> - kumar

Because it is an atomic operation. If you dereference it before
ptep_get_and_clear() it could change in between.

--
Brian Gerst

2005-04-06 17:10:16

by Christoph Lameter

[permalink] [raw]
Subject: Re: return value of ptep_get_and_clear

On Thu, 7 Apr 2005, Nick Piggin wrote:

> Kumar Gala wrote:
> > ptep_get_and_clear has a signature that looks something like:
> >
> > static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned
> > long addr,
> > pte_t *ptep)
> >
> > It appears that its suppose to return the pte_t pointed to by ptep
> > before its modified. Why do we bother doing this? The caller seems
> > perfectly able to dereference ptep and hold on to it. Am I missing
> > something here?
> >
>
> You need to be able to *atomically* clear the pte and retrieve the
> old value.

The effect of the clearing is that the present bit is cleared which makes
the CPU generate a fault if this pte is referenced.

The problem with replacing pte values is that the code executing is racing
with cpu mmu access to the pte (which may set bits on i386 I believe). So
if you would access the pte and then clear it later then there would be a
small window where the MMU could modify the pte. These changes would not
be detected since you later overwrite the pte.

Using ptep_get_and_clear insures that this does not happen...

2005-04-06 17:55:47

by Kumar Gala

[permalink] [raw]
Subject: Re: return value of ptep_get_and_clear


On Apr 6, 2005, at 12:09 PM, Christoph Lameter wrote:

> On Thu, 7 Apr 2005, Nick Piggin wrote:
>
> > Kumar Gala wrote:
> > > ptep_get_and_clear has a signature that looks something like:
> > >
> > > static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
> unsigned
> > > long addr,
> > >??????????????????????????????????????? pte_t *ptep)
> > >
> > > It appears that its suppose to return the pte_t pointed to by ptep
> > > before its modified.? Why do we bother doing this?? The caller
> seems
> > > perfectly able to dereference ptep and hold on to it.? Am I
> missing
> > > something here?
> > >
> >
> > You need to be able to *atomically* clear the pte and retrieve the
> > old value.
>
> The effect of the clearing is that the present bit is cleared which
> makes
> the CPU generate a fault if this pte is referenced.
>
> The problem with replacing pte values is that the code executing is
> racing
> with cpu mmu access to the pte (which may set bits on i386 I
> believe). So
> if you would access the pte and then clear it later then there would
> be a
> small window where the MMU could modify the pte. These changes would
> not
> be detected since you later overwrite the pte.
>
> Using ptep_get_and_clear insures that this does not happen...

Thanks, I was guessing that getting the value atomically was why this
was done after I give it a bit more thought.

- kumar