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=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham 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 EC26DC433DB for ; Thu, 25 Mar 2021 09:39:37 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 628AC61A14 for ; Thu, 25 Mar 2021 09:39:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 628AC61A14 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id E03AB6B006C; Thu, 25 Mar 2021 05:39:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DDA686B006E; Thu, 25 Mar 2021 05:39:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CA3036B0070; Thu, 25 Mar 2021 05:39:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0039.hostedemail.com [216.40.44.39]) by kanga.kvack.org (Postfix) with ESMTP id ADE3F6B006C for ; Thu, 25 Mar 2021 05:39:36 -0400 (EDT) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 617C018069D9A for ; Thu, 25 Mar 2021 09:39:36 +0000 (UTC) X-FDA: 77957899152.15.5A537E6 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf18.hostedemail.com (Postfix) with ESMTP id 7D6D3200024C for ; Thu, 25 Mar 2021 09:39:35 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 4179EAD9F; Thu, 25 Mar 2021 09:39:34 +0000 (UTC) Date: Thu, 25 Mar 2021 10:39:24 +0100 From: Oscar Salvador To: Mike Kravetz Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Roman Gushchin , Michal Hocko , Shakeel Butt , David Hildenbrand , Muchun Song , David Rientjes , Miaohe Lin , Peter Zijlstra , Matthew Wilcox , HORIGUCHI NAOYA , "Aneesh Kumar K . V" , Waiman Long , Peter Xu , Mina Almasry , Hillf Danton , Andrew Morton Subject: Re: [PATCH 1/8] mm: cma: introduce cma_release_nowait() Message-ID: References: <20210325002835.216118-1-mike.kravetz@oracle.com> <20210325002835.216118-2-mike.kravetz@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210325002835.216118-2-mike.kravetz@oracle.com> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 7D6D3200024C X-Stat-Signature: kdyqrs3mjb39g5uhhpqrung8ts438ann Received-SPF: none (suse.de>: No applicable sender policy available) receiver=imf18; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: none/none X-HE-Tag: 1616665175-939408 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 Wed, Mar 24, 2021 at 05:28:28PM -0700, Mike Kravetz wrote: > +struct cma_clear_bitmap_work { > + struct work_struct work; > + struct cma *cma; > + unsigned long pfn; > + unsigned int count; > +}; > + > struct cma cma_areas[MAX_CMA_AREAS]; > unsigned cma_area_count; > > +struct workqueue_struct *cma_release_wq; should this be static? > + > phys_addr_t cma_get_base(const struct cma *cma) > { > return PFN_PHYS(cma->base_pfn); > @@ -146,6 +155,10 @@ static int __init cma_init_reserved_areas(void) > for (i = 0; i < cma_area_count; i++) > cma_activate_area(&cma_areas[i]); > > + cma_release_wq = create_workqueue("cma_release"); > + if (!cma_release_wq) > + return -ENOMEM; > + I did not check the code that goes through the initcalls and maybe we cannot really have this scneario, but what happens if we return -ENOMEM? Because I can see that later in cma_release_nowait() you mess with cma_release_wq. Can it be that at that stage cma_release_wq == NULL? due to -ENOMEM, or are we guaranteed to never reach that point? Also, should the cma_release_wq go before the cma_activate_area? > +bool cma_release_nowait(struct cma *cma, const struct page *pages, > + unsigned int count) > +{ > + struct cma_clear_bitmap_work *work; > + unsigned long pfn; > + > + if (!cma || !pages) > + return false; > + > + pr_debug("%s(page %p)\n", __func__, (void *)pages); cma_release() seems to have: pr_debug("%s(page %p, count %u)\n", __func__, (void *)pages, count); any reason to not have the same here? > + > + pfn = page_to_pfn(pages); > + > + if (pfn < cma->base_pfn || pfn >= cma->base_pfn + cma->count) > + return false; > + > + VM_BUG_ON(pfn + count > cma->base_pfn + cma->count); > + > + /* > + * Set CMA_DELAYED_RELEASE flag: subsequent cma_alloc()'s > + * will wait for the async part of cma_release_nowait() to > + * finish. > + */ > + if (unlikely(!test_bit(CMA_DELAYED_RELEASE, &cma->flags))) > + set_bit(CMA_DELAYED_RELEASE, &cma->flags); It seems this cannot really happen? This is the only place we set the bit, right? Why not set the bit unconditionally? Against what this is guarding us? > + > + /* > + * To make cma_release_nowait() non-blocking, cma bitmap is cleared > + * from a work context (see cma_clear_bitmap_fn()). The first page > + * in the cma allocation is used to store the work structure, > + * so it's released after the cma bitmap clearance. Other pages > + * are released immediately as previously. > + */ > + if (count > 1) > + free_contig_range(pfn + 1, count - 1); > + > + work = (struct cma_clear_bitmap_work *)page_to_virt(pages); > + INIT_WORK(&work->work, cma_clear_bitmap_fn); > + work->cma = cma; > + work->pfn = pfn; > + work->count = count; > + queue_work(cma_release_wq, &work->work); As I said above, can cma_release_wq be NULL if we had -ENOMEM before? -- Oscar Salvador SUSE L3