From: Matthew Wilcox <mawilcox@microsoft.com>
To: Tejun Heo <tj@kernel.org>, Andrew Morton <akpm@linux-foundation.org>
CC: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>, Jens Axboe <axboe@kernel.dk>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "kent.overstreet@gmail.com" <kent.overstreet@gmail.com>
Subject: RE: [RFC 00/10] implement alternative and much simpler id allocator
Thread-Topic: [RFC 00/10] implement alternative and much simpler id allocator
Thread-Index: AQHSUme+SjIWPmjllE++nkCQM7WotKEEkFOAgAAAv/A=
Date: Mon, 12 Dec 2016 17:35:17 +0000
Message-ID: <BY2PR21MB0036238FFF7D84F17509B651CB980@BY2PR21MB0036.namprd21.prod.outlook.com>
References: <1481160187-9652-1-git-send-email-linux@rasmusvillemoes.dk>
 <20161209140140.5e0a68e2e1cf9861335bdf3b@linux-foundation.org>
 <20161212170959.GB13864@htj.duckdns.org>
In-Reply-To: <20161212170959.GB13864@htj.duckdns.org>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Dec 2016 17:35:17.8902
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR21MB0033
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id uBCHZVk2006306
Content-Length: 1305
Lines: 18

From: Tejun Heo [mailto:htejun@gmail.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