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=-3.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 6D569C433DF for ; Mon, 17 Aug 2020 22:58:29 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1EB472063A for ; Mon, 17 Aug 2020 22:58:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="byIpvA6P" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1EB472063A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6E2796B0002; Mon, 17 Aug 2020 18:58:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 66B576B0005; Mon, 17 Aug 2020 18:58:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 533596B0006; Mon, 17 Aug 2020 18:58:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0157.hostedemail.com [216.40.44.157]) by kanga.kvack.org (Postfix) with ESMTP id 3A1576B0002 for ; Mon, 17 Aug 2020 18:58:28 -0400 (EDT) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id EC3473647 for ; Mon, 17 Aug 2020 22:58:27 +0000 (UTC) X-FDA: 77161576254.24.rate85_57184392701a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin24.hostedemail.com (Postfix) with ESMTP id C0CC21A4A0 for ; Mon, 17 Aug 2020 22:58:27 +0000 (UTC) X-HE-Tag: rate85_57184392701a X-Filterd-Recvd-Size: 5637 Received: from mail-io1-f68.google.com (mail-io1-f68.google.com [209.85.166.68]) by imf37.hostedemail.com (Postfix) with ESMTP for ; Mon, 17 Aug 2020 22:58:27 +0000 (UTC) Received: by mail-io1-f68.google.com with SMTP id t15so19384073iob.3 for ; Mon, 17 Aug 2020 15:58:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CUinOKHyVodz4jvTS1Tm95hUcIyZXDORsxNhMipeHNI=; b=byIpvA6PXapCvY4kbX9hsHPmMFKCY69IW6PMIvbws4/x8AN3hC6qzvqvx9OoH0NDvs K0+ur+V+gik/czeG85229oQgaJMslt/bpivK5ELrz0IRun/ymrxVU3pKlTz/rce9mJT5 oQ+qmEmqZJ48NuA2NMUtEhzYNqRqAcKYfhLcV6LSxNzWtoCVDiosbiVVcrX7w+BkVx1w 884NUizKW61AP8E0defZhjbkHF3jdtVA3osKGJbKcWwqHVB4nLH36OeiyJh5y2sxkp1Z XPcZiDkxup87SeHU138rtD0cipsLApPga25ffrPjsj7opfL3vKkN2yGeFCUprlbS0XDQ pbpA== 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=CUinOKHyVodz4jvTS1Tm95hUcIyZXDORsxNhMipeHNI=; b=XGRBUfTDRnjr/GeNHwZk1WNFiKhKPN5IUQVeINQ6r8dxhOEoAZrQJQT3Tl8nRL/sG1 H/ccqVnQaQNyI7smGDb7jebGRuYlYqnZ5mvPtHe5Go9H8muo3WY8uxbxF2NmTqghrDm5 FGciLlUlFTLoJmCbnExXEo6UsRY/AkBJWOngBxZ3AJkQNkWb76riVM11nAcHTVKNoqDf t8Px0QO2dWv7VptO9QJ9HdYS7FlnAKvVKYhMRCdEduL7YRZMA92DNzFUQybDWFy1KIvV B7aLjuLi7wD/iHZGqZvcCfGQzGWhw3swE+/nqkihrwIyu2KxJ/9A7SG9Ih+OjrXUXpO3 4orQ== X-Gm-Message-State: AOAM533g3c2/VQEx9Pf/ytWZsfqGVaBSIPsZFcEezlZrCxUPcv2MSGEF 5zfIkjeJ9fyCmP+ue9TRRtiVDZBYMorXmcKJJhg= X-Google-Smtp-Source: ABdhPJy34kcBAP3BK2E4PxuPNKUrvks5mcEY80IvzNnvrGyoBR82Uo/yk/nsMsmVM+1ZuY5AwRRcBR5TOKdk/ZHJfNI= X-Received: by 2002:a05:6638:1508:: with SMTP id b8mr16744973jat.96.1597705106519; Mon, 17 Aug 2020 15:58:26 -0700 (PDT) MIME-Version: 1.0 References: <1595681998-19193-1-git-send-email-alex.shi@linux.alibaba.com> <1595681998-19193-15-git-send-email-alex.shi@linux.alibaba.com> In-Reply-To: <1595681998-19193-15-git-send-email-alex.shi@linux.alibaba.com> From: Alexander Duyck Date: Mon, 17 Aug 2020 15:58:15 -0700 Message-ID: Subject: Re: [PATCH v17 14/21] mm/compaction: do page isolation first in compaction To: Alex Shi Cc: Andrew Morton , Mel Gorman , Tejun Heo , Hugh Dickins , Konstantin Khlebnikov , Daniel Jordan , Yang Shi , Matthew Wilcox , Johannes Weiner , kbuild test robot , linux-mm , LKML , cgroups@vger.kernel.org, Shakeel Butt , Joonsoo Kim , Wei Yang , "Kirill A. Shutemov" , Rong Chen Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: C0CC21A4A0 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam05 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: > @@ -1691,17 +1680,34 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, > * only when the page is being freed somewhere else. > */ > scan += nr_pages; > - switch (__isolate_lru_page(page, mode)) { > + switch (__isolate_lru_page_prepare(page, mode)) { > case 0: > + /* > + * Be careful not to clear PageLRU until after we're > + * sure the page is not being freed elsewhere -- the > + * page release code relies on it. > + */ > + if (unlikely(!get_page_unless_zero(page))) > + goto busy; > + > + if (!TestClearPageLRU(page)) { > + /* > + * This page may in other isolation path, > + * but we still hold lru_lock. > + */ > + put_page(page); > + goto busy; > + } > + So I was reviewing the code and came across this piece. It has me a bit concerned since we are calling put_page while holding the LRU lock which was taken before calling the function. We should be fine in terms of not encountering a deadlock since the LRU bit is cleared the page shouldn't grab the LRU lock again, however we could end up grabbing the zone lock while holding the LRU lock which would be an issue. One other thought I had is that this might be safe because the assumption would be that another thread is holding a reference on the page, has already called TestClearPageLRU on the page and retrieved the LRU bit, and is waiting on us to release the LRU lock before it can pull the page off of the list. In that case put_page will never decrement the reference count to 0. I believe that is the current case but I cannot be certain. I'm just wondering if we should just replace the put_page(page) with a WARN_ON(put_page_testzero(page)) and a bit more documentation. If I am not mistaken it should never be possible for the reference count to actually hit zero here. Thanks. - Alex