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 X-Spam-Level: X-Spam-Status: No, score=-13.2 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B1BA6C433E0 for ; Mon, 21 Dec 2020 19:50:32 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 40E43225A9 for ; Mon, 21 Dec 2020 19:50:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 40E43225A9 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B5EF66B005C; Mon, 21 Dec 2020 14:50:31 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B0F9A6B005D; Mon, 21 Dec 2020 14:50:31 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9D6B96B0068; Mon, 21 Dec 2020 14:50:31 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0051.hostedemail.com [216.40.44.51]) by kanga.kvack.org (Postfix) with ESMTP id 8210B6B005C for ; Mon, 21 Dec 2020 14:50:31 -0500 (EST) Received: from smtpin11.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 448944DDC for ; Mon, 21 Dec 2020 19:50:31 +0000 (UTC) X-FDA: 77618331462.11.drop04_010f0162745a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin11.hostedemail.com (Postfix) with ESMTP id 26600180FA142 for ; Mon, 21 Dec 2020 19:50:31 +0000 (UTC) X-HE-Tag: drop04_010f0162745a X-Filterd-Recvd-Size: 5341 Received: from mail-lf1-f48.google.com (mail-lf1-f48.google.com [209.85.167.48]) by imf35.hostedemail.com (Postfix) with ESMTP for ; Mon, 21 Dec 2020 19:50:30 +0000 (UTC) Received: by mail-lf1-f48.google.com with SMTP id b26so16995141lff.9 for ; Mon, 21 Dec 2020 11:50:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xqmRYxnikPCq9Gg2hYUnPNz7l0TnTiioQ91HmpF8cHA=; b=Lbdw5kBkygrGYB8qOV/e44EUiRWpjNlF0K8JCBtD6ySkL7I6G//EbNDFNDUtCcBXoi K+zWPcc+uyHkHNaNbR8G6Dcg0SBkp/HYRbrV40nT7q4MYjmpiWMlFFOTlImixUfGTUb6 8AimKwng3RUWgDPVaqMZnmqhIhiU6RkNlqwyHL7eu12F4zaNgotyJLibOvX3hbgTu0Pj DesLPBAzm2RiT5BM5teRxFH/grBcEm4VCiOBSjGJQHr2GXJ5sXE8bm3XmqgtxnI5IIgY yEA13HmGRIO2VLpQmrySNWxiEEDYidT3ylSG+DvordKln3mkGma0h5ISMCrc0Q7ETCyI 38HA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xqmRYxnikPCq9Gg2hYUnPNz7l0TnTiioQ91HmpF8cHA=; b=tzOIn5Wy6A/q2BZOFjaTi6nFTPCLQtInoIqX0x5a9Ta7MEg91UhJzDzjC3vbKf/EWE 63WZray6xIjzQZpB5jl0ngvHUd9jofbnH/ao6naNlbzOmOQg6Gu4emcQeNW+p6Guo7kn qpT1JjrRosqaQW3IyErFO6E7pSM3/97iGN6riSm7Xt5P/5O5Kat7SIl2dkG/TuZw6Sn0 zwrSbQ8g6AekGfvZsbygS7kR1hn1GjX2Jm4LJk7O/eCn5HHIpmZ/fwenhKMH1dblCsZA YI5yD+t4wxwisEPhrWMJOZ5faeMTspixHcuLoGM0NvuhsQVA58G86QYoGNMrCejFIuZn y/ug== X-Gm-Message-State: AOAM532c8DmsvRyf7v0XcQGn1YTHZTaSH54BxJ8eNy1Eexq8PHSdWbC5 KurAEEnPE+sMGMI/nBdP28i1DB4/0Fjlbur+wCDleg== X-Google-Smtp-Source: ABdhPJxnHF3lgjv4wK6z/Y0RWoJLdh0bkRjwcQOK4yZ9LuRUu9O/PpGzE3gf+Aib+VErHRK2pcCflmqzHK+vd9aR21I= X-Received: by 2002:a2e:850f:: with SMTP id j15mr7797655lji.34.1608580229003; Mon, 21 Dec 2020 11:50:29 -0800 (PST) MIME-Version: 1.0 References: <18669bd607ae9efbf4e00e36532c7aa167d0fa12.camel@gmx.de> <20201220002228.38697-1-vitaly.wool@konsulko.com> In-Reply-To: From: Shakeel Butt Date: Mon, 21 Dec 2020 11:50:17 -0800 Message-ID: Subject: Re: [PATCH] zsmalloc: do not use bit_spin_lock To: Vitaly Wool Cc: Minchan Kim , Mike Galbraith , LKML , linux-mm , Barry Song , Sebastian Andrzej Siewior , NitinGupta , Sergey Senozhatsky , Andrew Morton Content-Type: text/plain; charset="UTF-8" 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: On Mon, Dec 21, 2020 at 11:20 AM Vitaly Wool wrote: > > On Mon, Dec 21, 2020 at 6:24 PM Minchan Kim wrote: > > > > On Sun, Dec 20, 2020 at 02:22:28AM +0200, Vitaly Wool wrote: > > > zsmalloc takes bit spinlock in its _map() callback and releases it > > > only in unmap() which is unsafe and leads to zswap complaining > > > about scheduling in atomic context. > > > > > > To fix that and to improve RT properties of zsmalloc, remove that > > > bit spinlock completely and use a bit flag instead. > > > > I don't want to use such open code for the lock. > > > > I see from Mike's patch, recent zswap change introduced the lockdep > > splat bug and you want to improve zsmalloc to fix the zswap bug and > > introduce this patch with allowing preemption enabling. > > This understanding is upside down. The code in zswap you are referring > to is not buggy. You may claim that it is suboptimal but there is > nothing wrong in taking a mutex. > Is this suboptimal for all or just the hardware accelerators? Sorry, I am not very familiar with the crypto API. If I select lzo or lz4 as a zswap compressor will the [de]compression be async or sync? > > https://lore.kernel.org/linux-mm/fae85e4440a8ef6f13192476bd33a4826416fc58.camel@gmx.de/ > > > > zs_[un/map]_object is designed to be used in fast path(i.e., > > zs_map_object/4K page copy/zs_unmap_object) so the spinlock is > > perfectly fine for API point of view. However, zswap introduced > > using the API with mutex_lock/crypto_wait_req where allowing > > preemption, which was wrong. > > Taking a spinlock in one callback and releasing it in another is > unsafe and error prone. What if unmap was called on completion of a > DMA-like transfer from another context, like a threaded IRQ handler? > In that case this spinlock might never be released. > > Anyway I can come up with a zswap patch explicitly stating that > zsmalloc is not fully compliant with zswap / zpool API The documentation of zpool_map_handle() clearly states "This may hold locks, disable interrupts, and/or preemption, ...", so how come zsmalloc is not fully compliant? > to avoid > confusion for the time being. Would that be ok with you? > > Best regards, > Vitaly >