On Thu, Aug 05 2021 at 15:04, Mel Gorman wrote:

On Thu, Aug 05, 2021 at 02:56:53PM +0200, Thomas Gleixner wrote:

quoted

On Wed, Aug 04 2021 at 15:23, Mel Gorman wrote:
Mel,

quoted

On Wed, Aug 04, 2021 at 03:42:25PM +0200, Vlastimil Babka wrote:

quoted

The idea was not build-time, but runtime (hidden behind lockdep, VM_DEBUG or
whatnot), i.e.:

<sched_expert> what that code needs is switch(item) { case foo1: case foo2:
lockdep_assert_irqs_disabled(); break; case bar1: case bar2:
lockdep_assert_preempt_disabled(); lockdep_assert_no_in_irq(); break; } or
something along those lines

Ok, that would potentially work. It may not even need to split the stats
into different enums. Simply document which stats need protection from
IRQ or preemption and use PROVE_LOCKING to check if preemption or IRQs
are disabled depending on the kernel config. I don't think it gets rid
of preempt_disable_rt unless the API was completely reworked with entry
points that describe the locking requirements. That would be tricky
because the requirements differ between kernel configurations.

Right. This won't get rid of the preempt disabling on RT, but I think we
should rather open code this

       if (IS_ENABLED(CONFIG_PREEMPT_RT))
       		preempt_dis/enable();

instead of proliferating these helper macros which have only one user left.

Ok, that is reasonable. I tried creating a vmstat-specific helper but the
names were misleading so I ended up with the patch below which open-codes
it as you suggest. The comment is not accurate because "locking/local_lock:
Add RT support" is not upstream but it'll eventually be accurate.

Is this ok?

Looks good.

Acked-by: Thomas Gleixner <redacted>