> >
> >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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
