linux-kernel - Re: [git pull] vfs.git pile 3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 8 Aug 2022 15:43:07 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [git pull] vfs.git pile 3 - dcache

On Mon, Aug 8, 2022 at 3:06 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> The use cases in mm/vmstat are not really all under spinlocks. That code
> gets called also just from plain local_irq or even just preempt disabled
> regions (depending on the stats item), which makes the proposed name
> less accurate than you describe.

Augh.

How about "preempt_disable_nested()" with a big comment about how some
operations normally disable preemption (interrupts off, spinlocks,
anything else?) but not on PREEMPT_RT?

> A worse case is the u64_stat code which is an ifdef maze (only partially
> due to RT). Those stats updates can also be called from various contexts
> where no spinlock is involved. That code is extra convoluted due to
> irqsave variants and "optimizations" for 32bit UP. Removing the latter
> would make a cleanup with write_seqcount_...() wrappers pretty simple.

I think we most definitely can start removing optimisations for 32-bit
UP by now.

Let's not do them without any reason, but any time you hit a code that
makes you go "this makes it harder to do better", feel free to go all
Alexander the Great on the 32-bit UP code and just cut through the
problem by removing it.

> Aside of that we have RT conditional preempt related code in
> page_alloc() and kmap_atomic(). Both care only about the task staying
> pinned on a CPU. In page_alloc() using preempt_disable() on !RT is more
> lightweight than migrate_disable(). So something like
> task_[un]pin_temporary() might work and be descriptive enough.

Yeah, that was the other odd pattern. I'm not sure "temporary" is all
that relevant, but yes, if we end up having more of those, some kind
of "thread_{un]pin_cpu()" would probably be worth it.

But the kmap code may be so special that nothing else has _that_
particular issue.

                   Linus