Re: 2.4.10pre13aa1

Re: 2.4.10pre13aa1

[Benjamin LaHaise]

On Fri, Sep 21, 2001 at 09:57:21AM +0200, Andrea Arcangeli wrote:
> Only in 2.4.10pre13aa1: 00_unmap-dirty-pte-1
> 
> 	I grepped over the whole 600 pages of the latest x86 system developer
> 	manual and I couldn't find the proof that I'm wrong.
> 
> 	We can have pagecache pages with pte writeable and non dirty at some
> 	point.
> 
> 	Now what happens if the userspace task in the other cpu touches the
> 	writeable page between our "ptep_get_and_clear" and the
> 	"flush_tlb_page"? Is the resulting pte still zero and the task get into
> 	a page fault? Or as I am worried it could also just end with the pte
> 	with only the dirty bit set?  Does somebody know for sure? I can
> 	imagine the cpu finding the tlb state writeable, and issuing just a
> 	locked bit test and set in the pte without caring to check if the pte
> 	is zero or not.
> 
> 	If the cpu just set the bit this patch will avoid to lose a shared
> 	mapping update. Otherwise it's a safe noop so I keep it applied
> 	until this issue is sorted out.

I've tested this on all the machines I could get my hands on, and every 
single CPU will take a page fault if the pte is not present on dirtying 
the page.  If people are truely paranoid, then make it a boot time assertion.

		-ben

Re: 2.4.10pre13aa1

[Andrea Arcangeli]

On Fri, Sep 21, 2001 at 01:18:41PM -0400, Benjamin LaHaise wrote:
> the page.  If people are truely paranoid, then make it a boot time assertion.

What do you think if I replace the mkdirty with a BUG() in case the pte
gets marked dirty? Just to be sure no hardware gets it wrong.

Andrea

Re: 2.4.10pre13aa1

[Benjamin LaHaise]

On Sat, Sep 22, 2001 at 09:28:59AM +0200, Andrea Arcangeli wrote:
> On Fri, Sep 21, 2001 at 01:18:41PM -0400, Benjamin LaHaise wrote:
> > the page.  If people are truely paranoid, then make it a boot time assertion.
> 
> What do you think if I replace the mkdirty with a BUG() in case the pte
> gets marked dirty? Just to be sure no hardware gets it wrong.

Sure.  At some point it should be made part of debugging-only builds.  Cheers,

		-ben

Re: 2.4.10pre13aa1

[Manfred Spraul]

 >> with only the dirty bit set?  Does somebody know for sure? I can
 >> imagine the cpu finding the tlb state writeable, and issuing
 >> just a locked bit test and set in the pte without caring to
 >> check if the pte is zero or not.
 >>
 >> If the cpu just set the bit this patch will avoid to lose a shared
 >> mapping update. Otherwise it's a safe noop so I keep it applied
 >> until this issue is sorted out
 >
 >I've tested this on all the machines I could get my hands on, and every
 >single CPU will take a page fault if the pte is not present on dirtying
 >the page.  If people are truely paranoid, then make it a boot time
 > assertion.
 >

I don't think that this is a valid argument:
you are testing on i386 and make design decisions for the architecture
independant part.

I'd prefer ptep_get_and_clear_and_flush(), then the arch part can do
what's needed to get the final pte value. (if a single page is modified,
otherwise the arch can define a suitable mmu_gather)

--
     Manfred

Re: 2.4.10pre13aa1

[Andrea Arcangeli]

On Sun, Sep 23, 2001 at 11:46:18AM +0200, Manfred Spraul wrote:
> I'd prefer ptep_get_and_clear_and_flush(), then the arch part can do

agreed. In the meantime I'll just keep the assert.

Andrea

Re: 2.4.10pre13aa1

[Benjamin LaHaise]

On Sun, Sep 23, 2001 at 11:46:18AM +0200, Manfred Spraul wrote:
> I don't think that this is a valid argument:
> you are testing on i386 and make design decisions for the architecture
> independant part.

The hook is only needed for x86-like architectures, as all of the risc like 
cpus perform the dirty state modification in the page fault handler while 
holding the page_table_lock.

> I'd prefer ptep_get_and_clear_and_flush(), then the arch part can do
> what's needed to get the final pte value. (if a single page is modified,
> otherwise the arch can define a suitable mmu_gather)

mmu_gather is the hook.  ia64 for instance can create a gather that uses 
the hardware tlb flush instruction to shootdown the entry across all cpus 
and eliminates the race.

		-ben