linux-kernel - RE: [RFC 00/10] implement alternative and much simpler id allocator

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, 12 Dec 2016 17:35:17 +0000
From:   Matthew Wilcox <mawilcox@...rosoft.com>
To:     Tejun Heo <tj@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
CC:     Rasmus Villemoes <linux@...musvillemoes.dk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Jens Axboe <axboe@...nel.dk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "kent.overstreet@...il.com" <kent.overstreet@...il.com>
Subject: RE: [RFC 00/10] implement alternative and much simpler id allocator

From: Tejun Heo [mailto:htejun@...il.com] On Behalf Of Tejun Heo
> Ah, yeah, great to see the silly implementation being replaced the
> radix tree.  ida_pre_get() looks suspicious tho.  idr_preload()
> immedicately being followed by idr_preload_end() probably is broken.
> Maybe what we need is moving ida to idr like preload interface and
> then convert it to radix based interface?  ida currently assumes
> per-ida preloading.

Hey Tejun!  Great to have your comments on this reimplementation.

I know the preload followed by preload_end looks wrong.  I don't think it's broken though.  If we get preempted, then the worst situation is that we'll end up with the memory we preallocated being allocated to somebody else.  Then the attempt to allocate memory can fail, and we'll return -EAGAIN, at which point all callers are supposed to return to the pre_get() stage.  Certainly that's what ida_simple_get() does.

Hmm ... looking at the implementation again, we might return -ENOMEM when we should return -EAGAIN.  Let me fix that (and the test suite ...)

I'd definitely be open to changing the IDA API.  I know Kent had some thoughts on that including splitting the simple lock into a per-IDA lock.  His last work on it was here, I believe:

https://evilpiepirate.org/git/linux-bcache.git/log/?h=idr