From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA7E7C021B8 for ; Thu, 27 Feb 2025 02:30:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2D796280002; Wed, 26 Feb 2025 21:30:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2867A280001; Wed, 26 Feb 2025 21:30:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 14E67280002; Wed, 26 Feb 2025 21:30:27 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id EA3EB280001 for ; Wed, 26 Feb 2025 21:30:26 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 5C9C91C8FE8 for ; Thu, 27 Feb 2025 02:30:26 +0000 (UTC) X-FDA: 83164145652.14.53DEEE7 Received: from out-180.mta0.migadu.com (out-180.mta0.migadu.com [91.218.175.180]) by imf26.hostedemail.com (Postfix) with ESMTP id 5DBEA140008 for ; Thu, 27 Feb 2025 02:30:24 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=NWnE91Al; spf=pass (imf26.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.180 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740623424; a=rsa-sha256; cv=none; b=ezkDTpJQtlhvMl65bqa3Pl/UVEkeFDaQ1WDzBlA2iCVxByBhVWCPOwWEUJNLLLyu6SX78U 6wQXhy48kv+33Ue/VNMhxsas5YLitQuEvurnTRsHXl+NmBH2sG5lV1qdqFW2+iMQ6AEM3g P4XSl5NOfTJ0anoqPhe/QzBKyboxDGY= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=NWnE91Al; spf=pass (imf26.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.180 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740623424; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=sbOYPimeqO3tk023X9SrOfwdCZQmqLyKIJkaK9p9QJE=; b=TOM9/N5Y/YGeh6oZf+41vuvryOuWc3YDP0OU467YZ6fsFR1Gv60X/BQgHnmkaeaUV6lrRY 1nbSOxwc2P/sdg0b7BqP4zbz+Rhk7fXPe9ipQpYHH/vhr6zkO8rPjVVDH2flV3vhOOHFJG G/YftJDSx4VgRPXz/JhV1O6sOEbcX7g= Message-ID: <7486a582-5143-4b4f-ae97-3a06089b630c@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1740623422; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=sbOYPimeqO3tk023X9SrOfwdCZQmqLyKIJkaK9p9QJE=; b=NWnE91AlMZmfwO4cduShqKb8KSd5wIDIOmzp5Yqy9P8th8GFxZiVDZHkTJzPXwBVIuBsEb wYP06zvgyKCw3v1MDEGBDamqyMG7CqCVdmGguIvWvQv3Ap20oSgns5xuHmfl/fHi0yTBqy 2hRf/OtuhD7fOAqrXQwl7NikHodjg2U= Date: Thu, 27 Feb 2025 10:30:15 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v2] mm: zswap: fix crypto_free_acomp() deadlock in zswap_cpu_comp_dead() To: Nhat Pham , Yosry Ahmed Cc: Eric Biggers , Andrew Morton , Johannes Weiner , "David S. Miller" , Herbert Xu , linux-mm@kvack.org, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com, syzbot+1a517ccfcbc6a7ab0f82@syzkaller.appspotmail.com, stable@vger.kernel.org References: <20250226185625.2672936-1-yosry.ahmed@linux.dev> <20250226200016.GB3949421@google.com> <20250226211628.GD3949421@google.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Stat-Signature: oosex8gq1dxye9eb7jj8dsxpeq9cxhc7 X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 5DBEA140008 X-Rspam-User: X-HE-Tag: 1740623424-884988 X-HE-Meta: U2FsdGVkX1+HMCoBRi8Lpq/iOsDDotQUsAvfv8lDH7+vQ5X9SocssgaPX+vt3VHhWK0I/73F4d3Ocfla7FIw/IevWYTE5FcGDykxr9b7UL2hrdtxXt8gbJQ3deh/m3FtcilSCzwQxRXBYxmu67kEC0IQcQQCv8VMkxgcnQ9Y6vmR//4vFrF4z57UGCVHtNGbFPcU1FAYN6noM1Ms/jIywE2pmpVaR2Z2NAB+kLdhSQCsiK5DHIHTr69yR9BUmcNtU6ZcEqYFlCDDtfiOFCDr2QGSDxNkWOkPo3CqM5qxyCmkhZffihIJDBPgT/csGPg1YBGQQrHrhKGdBFaiEKuPO6gIl+EfBhuyms8x6SU87K6JMds6SPQMvOc/d/q1mIlPDhP0RbrFNK6HJ0BUMqFlo/RKTTgNB+qLMMItNqnb0uyb3uJnlY9nIL3r29ajJxmnxxNceIvjuemq54Z5SSLxUmiUBWpP/H6afdotg6UrVCnVdK7XydoDbAnZ14F7NrvrrI2IL8mmK+uwSyoBCBSUzKCita28aJ3VX9gYwryrxQWL95dpl8LmlNpWD490FpxjMtRbK47ATysp60tJxhOFPTFutx63OeRNpur5y2HWUIJuwHdAOdek7u5uOvIHM9CX229sWE1g6EX+WXWYjh+/y3yT5ec3omU68UgsU2+DkImabPIH52zBr98LJgsJt+hmb4lvj+o2rUw8F/OYwN8wmAjVOoQZqGMAJEyodBFWwBLsAgBT+cPaLZmVRdIvyujpXFSpgm72Gquon5f/cmpUpM08vn3TiMK/P+9gPoReOBkZSuPKiM26vg1mIOCIV4p11v0ZHs2hXnP4vReX3/GJO0dpHLZK4ov5n6VyObWj/a0ZaHqhNEj2GXEUaBj5ZAzQCeHGPXPpUQ6kBvyEQsBTnZ0/s2c+POexkJO99zGF5UNROMAaDZbJzou6cy1vAFBeyP/tbymqYVMIRnhFVV3 igz6X9xT L3NnRTY1NGhH583aucN7ddf48WDcN57Oni8PHnOqrvJZJOvWvo3HOVIGgmkaM0W3wuPy23ryGhskeClGLT+rIBmQE17ukVrqW7YHmqgR1TJgD0VSoMwSgehaRWKxToFxAVfMKikSTX4mHU6oWLoURL2gsSHojFu05kkq1Sid0xVw8/IwahdtuaBJ3o1g+4uFnmDAdd7uc+kftoF2dLq5DHNNe5HLs0J3SPfgTmFtOkQwTP26zTv/uq2Rm4kM7rkraMon1tq2+7Qbki69eUXnPHr4IMt2Ce1EbQK+EisuD3UYXko0l2O2gHER5yaHwGdPcRGN21P4fvGRS9ZalQkNP21G+NwMc51sQPWt7nmJhF2QTTpJp5Z220eiiCst5Cfxk3yHRfDQpHSCvWaIdcLY8UCqztAkIEVN4wH7dNh/R2jyVDsBQw/zdSASAOg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2025/2/27 07:47, Nhat Pham wrote: > On Wed, Feb 26, 2025 at 1:23 PM Yosry Ahmed wrote: >> >> On Wed, Feb 26, 2025 at 09:16:28PM +0000, Eric Biggers wrote: >>> On Wed, Feb 26, 2025 at 08:32:22PM +0000, Yosry Ahmed wrote: >>>> On Wed, Feb 26, 2025 at 08:00:16PM +0000, Eric Biggers wrote: >>>>> On Wed, Feb 26, 2025 at 06:56:25PM +0000, Yosry Ahmed wrote: >>>>>> Currently, zswap_cpu_comp_dead() calls crypto_free_acomp() while holding >>>>>> the per-CPU acomp_ctx mutex. crypto_free_acomp() then holds scomp_lock >>>>>> (through crypto_exit_scomp_ops_async()). >>>>>> >>>>>> On the other hand, crypto_alloc_acomp_node() holds the scomp_lock >>>>>> (through crypto_scomp_init_tfm()), and then allocates memory. >>>>>> If the allocation results in reclaim, we may attempt to hold the per-CPU >>>>>> acomp_ctx mutex. >>>>> >>>>> The bug is in acomp. crypto_free_acomp() should never have to wait for a memory >>>>> allocation. That is what needs to be fixed. >>>> >>>> crypto_free_acomp() does not explicitly wait for an allocation, but it >>>> waits for scomp_lock (in crypto_exit_scomp_ops_async()), which may be >>>> held while allocating memory from crypto_scomp_init_tfm(). >>>> >>>> Are you suggesting that crypto_exit_scomp_ops_async() should not be >>>> holding scomp_lock? >>> >>> I think the solution while keeping the bounce buffer in place would be to do >>> what the patch >>> https://lore.kernel.org/linux-crypto/Z6w7Pz8jBeqhijut@gondor.apana.org.au/ does, >>> i.e. make the actual allocation and free happen outside the lock. >> >> I am fine with a solution like that if Herbert is fine with it. Although >> as I mentioned, I think this patch is nice to have anyway. >> >>> >>>>> But really the bounce buffering in acomp (which is what is causing this problem) >>>>> should not exist at all. There is really no practical use case for it; it's >>>>> just there because of the Crypto API's insistence on shoehorning everything into >>>>> scatterlists for no reason... >>>> >>>> I am assuming this about scomp_scratch logic, which is what we need to >>>> hold the scomp_lock for, resulting in this problem. >>> >>> Yes. >>> >>>> If this is something that can be done right away I am fine with dropping >>>> this patch for an alternative fix, although it may be nice to reduce the >>>> lock critical section in zswap_cpu_comp_dead() to the bare minimum >>>> anyway. >>> >>> Well, unfortunately the whole Crypto API philosophy of having a single interface >>> for software and for hardware offload doesn't really work. This is just yet >>> another example of that; it's a problem caused by shoehorning software >>> compression into an interface designed for hardware offload. zcomp really >>> should just use the compression libs directly (like most users of compression in >>> the kernel already do), and have an alternate code path specifically for >>> hardware offload (using acomp) for the few people who really want that. >> >> zcomp is for zram, zswap does not use it. If zswap is not going to use >> the crypto API we'll want something like zcomp or maybe reuse zcomp >> itself. That's a problem for another day :) > > I'm actually thinking whether we should expose the zcomp API and use > it for zswap. There are a couple of parameters for zstd I wanna play > with, which zcomp/zram seems to already support, but not the crypto > API (zstd level, dictionary, etc.). Ah, agree! Actually I also think we should use the zcomp API in zswap, if its API meets our requirements. > > But yes, a different problem for another day :)