lkml.org 
[lkml]   [2005]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: Dirty page tracking patch
From
Date

> >
> >is the stratus code entirely open source/GPL ? (I assume it is since you
> >EXPORT_SYMBOL_GPL and also use other similar stuff). If so.. could you
> >post an URL to that? It's customary to do so when you post interface
> >patches for review so that the users of the interfaces can be seen, and
> >thus the interface better reviewed.
> >
> I don't have access to a Stratus web server where I could post the code,
> since I'm currently sitting at a Redhat desk :) So if you'll indulge
> me, I'll attach the harvest code to this note. The harvest code is one
> component of a GPL'd kernel module. The harvest component is the only
> one that interacts with the dirty page tracking patch. Other parts of
> the module do PCI-hotplug related things that are specific to our platform.

ok so your entire kernel side stack is GPL? Good.
> >
> >+#define mm_track(ptep)
> >
> >you have to make that a do { ; } while (0) define as per kernel
> >convention/need
> >
> >also you now make set_pte() and co more than trivial assignments, please
> >convert them to inlines so that typechecking is performed and no double
> >evaluation of arguments is done!
> >(this is a real problem for code that would do set_pte(pte++, value) in
> >a loop or so)
> >
> thanks, I'll make those changes and re-build/re-test/re-post.
>
> >- if (!pte_dirty(*ptep))
> >- return 0;
> >- return test_and_clear_bit(_PAGE_BIT_DIRTY, &ptep->pte_low);
> >+ mm_track(ptep);
> >+ return (test_and_clear_bit(_PAGE_BIT_DIRTY,
> >&ptep->pte_low) |
> >+ test_and_clear_bit(_PAGE_BIT_SOFTDIRTY,
> >&ptep->pte_low));
> > }
> >
> >
> >are you sure you're not introducing a race condition there?
> >and if you're sure, why do you need 2 atomic ops in sequence?
> >
> I think we're OK there. That code only appears in the sys_msync path
> (sync_page_range), after the page table lock is taken. The only code
> that moves hardware dirty bits to software dirty bits is in the harvest
> itself (attached). And that code runs with all other cpus parked.

then you didn't answer my question about why the lock; atomic bit is
needed ;)


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-07-11 20:36    [W:0.034 / U:3.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site