linux-kernel - Re: [Xen-devel] [PATCH v12 2/5] xenbus/backend: Protect xenbus callback with lock

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:   Wed, 18 Dec 2019 14:30:44 +0100
From:   Jürgen Groß <jgross@...e.com>
To:     SeongJae Park <sjpark@...zon.com>
Cc:     axboe@...nel.dk, konrad.wilk@...cle.com, roger.pau@...rix.com,
        linux-block@...r.kernel.org, pdurrant@...zon.com,
        SeongJae Park <sjpark@...zon.de>, linux-kernel@...r.kernel.org,
        sj38.park@...il.com, xen-devel@...ts.xenproject.org
Subject: Re: [Xen-devel] [PATCH v12 2/5] xenbus/backend: Protect xenbus
 callback with lock

On 18.12.19 13:42, SeongJae Park wrote:
> On Wed, 18 Dec 2019 13:27:37 +0100 "Jürgen Groß" <jgross@...e.com> wrote:
> 
>> On 18.12.19 11:42, SeongJae Park wrote:
>>> From: SeongJae Park <sjpark@...zon.de>
>>>
>>> 'reclaim_memory' callback can race with a driver code as this callback
>>> will be called from any memory pressure detected context.  To deal with
>>> the case, this commit adds a spinlock in the 'xenbus_device'.  Whenever
>>> 'reclaim_memory' callback is called, the lock of the device which passed
>>> to the callback as its argument is locked.  Thus, drivers registering
>>> their 'reclaim_memory' callback should protect the data that might race
>>> with the callback with the lock by themselves.
>>
>> Any reason you don't take the lock around the .probe() and .remove()
>> calls of the backend (xenbus_dev_probe() and xenbus_dev_remove())? This
>> would eliminate the need to do that in each backend instead.
> 
> First of all, I would like to keep the critical section as small as possible.
> With my small test, I could see slightly increasing memory pressure as the
> critical section becomes wider.  Also, some drivers might share the data their
> 'reclaim_memory' callback touches with other functions.  I think only the
> driver owners can know what data is shared and what is the minimum critical
> section to protect it.

But this kind of serialization can still be added on top.

And with the trylock in the reclaim path I believe you can even avoid
the irq variants of the spinlock. But I might be wrong, so you should
try that with lockdep enabled. If it is working there is no harm done
when making the critical section larger, as memory allocations will
work as before.


Juergen