From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S261614AbVGKNgP (ORCPT ); Mon, 11 Jul 2005 09:36:15 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S261674AbVGKNgO (ORCPT ); Mon, 11 Jul 2005 09:36:14 -0400 Received: from pentafluge.infradead.org ([213.146.154.40]:3761 "EHLO pentafluge.infradead.org") by vger.kernel.org with ESMTP id S261614AbVGKNgM (ORCPT ); Mon, 11 Jul 2005 09:36:12 -0400 Subject: Re: Dirty page tracking patch From: Arjan van de Ven To: Kimball Murray Cc: linux-kernel@vger.kernel.org In-Reply-To: <20050711131531.8257.62845.sendpatchset@dhcp83-73.boston.redhat.com> References: <20050711131531.8257.62845.sendpatchset@dhcp83-73.boston.redhat.com> Content-Type: text/plain Date: Mon, 11 Jul 2005 15:35:58 +0200 Message-Id: <1121088959.3177.24.camel@laptopd505.fenrus.org> Mime-Version: 1.0 X-Mailer: Evolution 2.2.2 (2.2.2-5) Content-Transfer-Encoding: 7bit X-Spam-Score: 2.9 (++) X-Spam-Report: SpamAssassin version 3.0.4 on pentafluge.infradead.org summary: Content analysis details: (2.9 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.1 RCVD_IN_SORBS_DUL RBL: SORBS: sent directly from dynamic IP address [80.57.133.107 listed in dnsbl.sorbs.net] 2.8 RCVD_IN_DSBL RBL: Received via a relay in list.dsbl.org [] X-SRS-Rewrite: SMTP reverse-path rewritten from by pentafluge.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2005-07-11 at 09:16 -0400, Kimball Murray wrote: > Hello to all. > > On behalf of Stratus Technologies (www.stratus.com) I'd like to > present a patch to the i386 kernel code that will allow developers to track > dirty memory pages. Stratus uses this technique to facilitate bringing > separate cpu and memory module "nodes" into lockstep with each other, with > a minimum of OS down time. This feature could also be used to provide inputs > into memory management algorithms, or to support hot-plug memory dimm modules > for specially developed hardware. > 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. Also this patch is just plain weird/really corner case... +#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) - 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?