linux-kernel - Re: [PATCH 1/3] kvfree_rcu: Allocate a page for a single argument

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]

Message-ID: <20210122113409.w55kul5bpjxhkgm4@linutronix.de>
Date:   Fri, 22 Jan 2021 12:34:09 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Uladzislau Rezki <urezki@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>, RCU <rcu@...r.kernel.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Daniel Axtens <dja@...ens.net>,
        Frederic Weisbecker <frederic@...nel.org>,
        Neeraj Upadhyay <neeraju@...eaurora.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Michal Hocko <mhocko@...e.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Theodore Y . Ts'o" <tytso@....edu>,
        Oleksiy Avramchenko <oleksiy.avramchenko@...ymobile.com>
Subject: Re: [PATCH 1/3] kvfree_rcu: Allocate a page for a single argument

On 2021-01-21 13:38:34 [+0100], Uladzislau Rezki wrote:
> __get_free_page() returns "unsigned long" whereas a bnode is a pointer
> to kvfree_rcu_bulk_data struct, without a casting the compiler will
> emit a warning.

Yes, learned about it, sorry.

> >> You think that a CPU migration is a bad thing. But why?
> >>
> It is not a bad thing. But if it happens we might queue a new bnode
> to a drain list of another CPU where a previous element of a new
> bnode may be just underutilized. So that is why i use migrate_disable()/enable()
> to prevent it.

If you allocate a node for queueing and then you realize that you
already have one then you either free it or queue it for later.
Given that all this is a slower-path and that this is used on every-CPU,
sooner or later that page will be used avoids a later allocation, right?

> If there are some hidden issues with migrate_disable()/enable() or you
> think it is a bad idea to use it, it would be appreciated if you could
> describe your view in more detail.

Just what I mentioned in my previous email:
- the whole operation isn't cheap but it is more efficient in tree
  compared to what we used to have in RT.
- the task can no be freely placed while it could run. So if the task
  gets preempted, it will have to wait until it can run on the CPU
  again.

Sebastian