On Thu, Mar 02, 2023 at 11:47:35AM -0300, Marcelo Tosatti wrote: > So it will be: > > #ifdef CONFIG_HAVE_CMPXCHG_LOCAL > mod_zone_page_state > inc_zone_page_state > dec_zone_page_state > mod_node_page_state > inc_node_page_state > dec_node_page_state > __mod_zone_page_state (new function, calls mod_zone_page_state). > __mod_node_page_state (new function, calls mod_node_page_state). > __inc_zone_page_state > __inc_node_page_state > __dec_zone_page_state > __dec_node_page_state > #else > __mod_zone_page_state (old, shared function for both CONFIG_HAVE_CMPXCHG_LOCAL and not) > __mod_node_page_state > __inc_zone_page_state > __inc_node_page_state > __dec_zone_page_state > __dec_node_page_state > mod_zone_page_state > inc_zone_page_state > dec_zone_page_state > mod_node_page_state > inc_node_page_state > dec_node_page_state > #endif > > Any suggestion on how to split this into multiple patchsets for easier > reviewing? (can't think of anything obvious). I figured this out before saw this, but it did take me some time to read carefully into the code base.. maybe it'll be a good idea to mention something like above in the commit message to ease future reviewers (and more likelyhood to attract the experts to start chim in)? One fundamental (but maybe another naive.. :) question on this code piece (so not directly related to the changeset but maybe it is still..): AFAICT CONFIG_HAVE_CMPXCHG_LOCAL only means we can do cmpxchg() without locking memory bus, however when !CONFIG_HAVE_CMPXCHG_LOCAL here we're not using non-local version but using preempt_disable_nested() to make sure the read is atomic. Does it really worth it? What happens if we use cmpxchg() unconditionally, but just use local (e.g. no "LOCK" prefix) version when CONFIG_HAVE_CMPXCHG_LOCAL? -- Peter Xu
