Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Aug 2023 14:00:26 -0700
From:   Yury Norov <yury.norov@gmail.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     andriy.shevchenko@linux.intel.com, linux@rasmusvillemoes.dk,
        Andrew Morton <akpm@linux-foundation.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Mateusz Guzik <mjguzik@gmail.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        tglx@linutronix.de, rppt@kernel.org
Subject: Re: [PATCH v2 2/2] mm,nodemask: Use nr_node_ids
Message-ID: <ZMwVanb0nTbOiWyn@yury-ThinkPad>
References: <20230802112458.230221601@infradead.org>
 <20230802112525.633758009@infradead.org>
 <20230802193616.GC231007@hirez.programming.kicks-ass.net>
 <ZMr4uBfjKY9dERl2@yury-ThinkPad>
 <20230803084125.GE212435@hirez.programming.kicks-ass.net>
 <ZMwRBqTgkvA7o9x3@yury-ThinkPad>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZMwRBqTgkvA7o9x3@yury-ThinkPad>
Precedence: bulk

> Consider MAX_NUMNODES == 64 and nr_node_ids == 4. Then
> small_nodemask_bits == 64.
> 
> The nodes_full() will set all 64 bits:
> 
>   #define nodes_full(nodemask) __nodes_full(&(nodemask), small_nodemask_bits)
>   static inline bool __nodes_full(const nodemask_t *srcp, unsigned int nbits)
>   {
>           return bitmap_full(srcp->bits, nbits);
>   }

Damn, copied the wrong function. This should be nodes_setall() of
course:

  #define nodes_setall(dst) __nodes_setall(&(dst), large_nodemask_bits)
  static inline void __nodes_setall(nodemask_t *dstp, unsigned int nbits)
  {
          bitmap_fill(dstp->bits, nbits);
  }

 
> And the following nodes_weight() will return 64:
> 
>   #define nodes_weight(nodemask) __nodes_weight(&(nodemask), small_nodemask_bits)
>   static inline int __nodes_weight(const nodemask_t *srcp, unsigned int nbits)
>   {
>           return bitmap_weight(srcp->bits, nbits);
>   }
> 
> Which is definitely wrong because there's 4 nodes at max. To solve
> this problem, both cpumask and nodemask implementations share the same
> rule: all bits beyond nr_{node,cpumask}_bits must be always cleared.
> 
> See how cpumask_setall() implements that:
> 
>   static inline void cpumask_setall(struct cpumask *dstp)
>   {
>           // Make sure we don't break the optimization
>           if (small_const_nbits(small_cpumask_bits)) {
>                   cpumask_bits(dstp)[0] = BITMAP_LAST_WORD_MASK(nr_cpumask_bits);
>                   return;
>           }
> 
>           // Pass the exact (runtime) number of bits
>           bitmap_fill(cpumask_bits(dstp), nr_cpumask_bits);
>   }
> 
> Hope that makes sense.
> 
> Thanks,
> Yury