From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-4.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from smtp-relay-canonical-0.canonical.com (smtp-relay-canonical-0.canonical.com [185.125.188.120]) by gnuweeb.org (Postfix) with ESMTPS id BD23E7FD42 for ; Tue, 21 Jun 2022 20:27:49 +0000 (UTC) Authentication-Results: gnuweeb.org; dkim=pass (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=REhG/YvE; dkim-atps=neutral Received: from [192.168.192.153] (unknown [50.126.114.69]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-0.canonical.com (Postfix) with ESMTPSA id 942D241625; Tue, 21 Jun 2022 20:27:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1655843265; bh=FHOjmjAGhb/jKh/X38HObStif09xq2VOvZEQFXt3zqo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=REhG/YvE5Y5HZv8PaXPYcPM44vP8IEJ8IMzZFA32j0rDjJBvP54vgsg5TljnEVb2C 6l9Gw8xSjfI1zJUVeqU706hHjnfTfXBQg2mjFR6AwzXgBae9AkqRIO3MW8Nnl/QIYx a1aXnHf31tgETQ71gAcLHrFbepqdBlwyN+w4B9rXjnOL0oVRtEXzi0mdktd1n9nhlu c44GHwZtTKZGR9nZxxQNYa84k7b7lT8U8Rrb1bb1ojiaT2SzJ9CuwKDkd0rVfWt/0m kvplSORt2LLsRq1jQmvt49GetCZG97LSsyDGbhwWhE+cCGxDZom8/JBlMrQKyYTzOo 3XS9YZXjztRIg== Message-ID: Date: Tue, 21 Jun 2022 13:27:33 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: Linux 5.18-rc4 Content-Language: en-US To: Matthew Wilcox Cc: Linus Torvalds , "Eric W. Biederman" , Ammar Faizi , James Morris , LSM List , Linux Kernel Mailing List , Al Viro , Kees Cook , linux-fsdevel@vger.kernel.org, Linux-MM , gwml@vger.gnuweeb.org References: <226cee6a-6ca1-b603-db08-8500cd8f77b7@gnuweeb.org> <87r1414y5v.fsf@email.froward.int.ebiederm.org> <266e648a-c537-66bc-455b-37105567c942@canonical.com> From: John Johansen Organization: Canonical In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit List-Id: On 6/13/22 15:48, Matthew Wilcox wrote: > On Mon, Jun 06, 2022 at 02:00:33PM -0700, John Johansen wrote: >> On 6/6/22 13:23, Matthew Wilcox wrote: >>> On Mon, Jun 06, 2022 at 12:19:36PM -0700, John Johansen wrote: >>>>> I suspect that part is that both Apparmor and IPC use the idr local lock. >>>>> >>>> bingo, >>>> >>>> apparmor moved its secids allocation from a custom radix tree to idr in >>>> >>>> 99cc45e48678 apparmor: Use an IDR to allocate apparmor secids >>>> >>>> and ipc is using the idr for its id allocation as well >>>> >>>> I can easily lift the secid() allocation out of the ctx->lock but that >>>> would still leave it happening under the file_lock and not fix the problem. >>>> I think the quick solution would be for apparmor to stop using idr, reverting >>>> back at least temporarily to the custom radix tree. >>> >>> How about moving forward to the XArray that doesn't use that horrid >>> prealloc gunk? Compile tested only. >>> >> >> I'm not very familiar with XArray but it does seem like a good fit. We do try >> to keep the secid allocation dense, ideally no holes. Wrt the current locking >> issue I want to hear what Thomas has to say. Regardless I am looking into >> whether we should just switch to XArrays going forward. > > Nothing from Thomas ... shall we just go with this? Do you want a > commit message, etc for the patch? Hey Matthew, I have done testing with this and the patch looks good. We will certainly go this route going forward so a commit message, would be good. As for fixing the issue in current kernels. I am not opposed to pulling this back as fixes for 99cc45e48678 apparmor: Use an IDR to allocate apparmor secids but I would like some other peoples opinions on doing that, because we don't understand why we are getting the current lock splat, and without understanding it is a fix by avoiding the issue rather than being sure the actual issue is fixed.