Messages in this thread | | | From | Kees Cook <> | Date | Tue, 23 Oct 2018 13:04:59 -0700 | Subject | Re: [PATCH] mm: convert totalram_pages, totalhigh_pages and managed_pages to atomic. |
| |
On Mon, Oct 22, 2018 at 10:11 PM, Konstantin Khlebnikov <khlebnikov@yandex-team.ru> wrote: > On 23.10.2018 7:15, Joe Perches wrote:> On Mon, 2018-10-22 at 22:53 +0530, > Arun KS wrote: >>> Remove managed_page_count_lock spinlock and instead use atomic >>> variables. >> >> Perhaps better to define and use macros for the accesses >> instead of specific uses of atomic_long_<inc/dec/read> >> >> Something like: >> >> #define totalram_pages() (unsigned >> long)atomic_long_read(&_totalram_pages) > > or proper static inline > this code isn't so low level for breaking include dependencies with macro
BTW, I noticed a few places in the patch that did multiple evaluations of totalram_pages. It might be worth fixing those prior to doing the conversion, too. e.g.:
if (totalram_pages > something) foobar(totalram_pages); <- value may have changed here
should, instead, be:
var = totalram_pages; <- get stable view of the value if (var > something) foobar(var);
-Kees
> [dropped bloated cc - my server rejects this mess]
Thank you -- I was struggling to figure out the best way to reply to this. :)
-Kees
-- Kees Cook
| |