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 D5C84CCFA05 for ; Wed, 25 Sep 2024 22:59:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 10C5A6B00B7; Wed, 25 Sep 2024 18:59:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0C8266B00B8; Wed, 25 Sep 2024 18:59:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC6106B00B9; Wed, 25 Sep 2024 18:59:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id CAA0D6B00B7 for ; Wed, 25 Sep 2024 18:59:11 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 29FFF401E6 for ; Wed, 25 Sep 2024 22:59:11 +0000 (UTC) X-FDA: 82604778102.28.D853D8F Received: from mail-lf1-f45.google.com (mail-lf1-f45.google.com [209.85.167.45]) by imf09.hostedemail.com (Postfix) with ESMTP id 2A2D5140008 for ; Wed, 25 Sep 2024 22:59:08 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=QE8xNw0x; spf=pass (imf09.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.167.45 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727304989; h=from:from:sender:reply-to: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=nPHAaStQaII888K+8FlasHr0p/vMD/r1ZG30txthNAs=; b=UHgAIkqBfpEIOBYWCbZVs+dWYA+UKnIJQWD8tY6flipM5viy0hqZVGoE4dapdKtfEcJ2Dn 6boC5O3Ex7fMS4PBdY8Seogkx4BJDh3IeNoyF412fNubmFXKK+MHxE/0TaY+8ChBnCAJLl bj4VlHe6qmsM9WsLhOTNYX8Vibl7a04= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=QE8xNw0x; spf=pass (imf09.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.167.45 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727304989; a=rsa-sha256; cv=none; b=Shn+85SLljzk2CHE7lNnPA/7UG7h3wNHTE8waYG6kkTZ5JwN4E3UeIfSwadUr8De0yHNbh bTAk5OdhcKCrYGdg30X5ralXRooGrQLeF84fNvtZWu8thPC4KIEToD/K0S7JYmHaNUEMyK +61ypzaEifbBsU28J83wlPehgFJv6B8= Received: by mail-lf1-f45.google.com with SMTP id 2adb3069b0e04-5366fd6fdf1so515220e87.0 for ; Wed, 25 Sep 2024 15:59:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727305147; x=1727909947; darn=kvack.org; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:reply-to:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=nPHAaStQaII888K+8FlasHr0p/vMD/r1ZG30txthNAs=; b=QE8xNw0xSfiwOMBTJ0NqUCOuGOd8SWjsdCwdrN2dE/dFxNJjuaLJPWvqcZeceEQ+CQ mRwqEndrgGkCHBEStT+TZV2E+eTpz6bZoSkDoFFhCz+Imc88AWwvpKWCGdsGmjpUCZBs lPJ1Ol0pnYyRCy7PV6yrmPUV2qr1UH3+R9HeVhi06w912/d8uBclVgIZ8g7at4DSIid4 FjGAqavWLdnvzae222FIBkL4CYamsX+vJQbR27lqK4TjoP0yVghj2/xRhEhLJ/DqGrcp HTNnTTykd2sAi2tymWzjXyvdyD4yFXAi1d07mEI9Zz5e/ukm0X3mv4nvWkLTGolSmdFA 6YcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727305147; x=1727909947; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:reply-to:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=nPHAaStQaII888K+8FlasHr0p/vMD/r1ZG30txthNAs=; b=RxFThzC+WThDg9iEd0fI9pcOAKnTHyK5zbRvA/oxcNPdWh8uPv4p8bTbPq3kpBRp1L qJE5yuqcsN8pF6EifXzVpLPRYjL2A7WzgpW1KrpzJwyiXBIUkjWcWWLpa/JZIvw6qF4D W4bXXBken1m+gbaXf0wVmhc2gdEGOOhmKEMaCIHoDe5Z/iNj1pWiQTdrOCIuHcVOIOwb Q7PdJit/gjSzbhtv2z/LGWfjXIveG6wtaf5+dhbPfRV6vzthy0+5FuJs9TrCs34TXogC RqUetH/D+TJ8iAS+0USxg0H72q1urbOpDtKy9PLNadGNsEsaX8LY75QMZ54GJxZ+uofb WOkA== X-Forwarded-Encrypted: i=1; AJvYcCX8HlxHWur6ftf5ZFTx/Fn2Wjo28WlRQP1PRiWgOh2qXDJz/IIq8vmhbxAKrLluMUWaoBUaEDuqLA==@kvack.org X-Gm-Message-State: AOJu0Yya05GGV1TFR2ecatt8zgFO0dLtRY4rFRSnQ4nwhODIEWNcgcCf dOx33jYCfd7Jn9jBlI1RG7vw1xxMWeg4dQLOj+F0IbM923CMY2t3 X-Google-Smtp-Source: AGHT+IFh5RcaK0Z1IWIgAeZgenLV2GEOBr1rLWF6USOcFDQF3Fg28CqDAPvm4tYKWZ25FuX1fWFf2g== X-Received: by 2002:a05:6512:b03:b0:52c:9383:4c16 with SMTP id 2adb3069b0e04-538704986bamr2828395e87.22.1727305146963; Wed, 25 Sep 2024 15:59:06 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9392f4faebsm277812166b.52.2024.09.25.15.59.05 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 25 Sep 2024 15:59:05 -0700 (PDT) Date: Wed, 25 Sep 2024 22:59:04 +0000 From: Wei Yang To: Sid Kumar Cc: Wei Yang , linux-kernel@vger.kernel.org, maple-tree@lists.infradead.org, linux-mm@kvack.org, akpm@linux-foundation.org, liam.howlett@oracle.com, willy@infradead.org, surenb@google.com Subject: Re: [PATCH v4 04/17] maple_tree: introduce mas_wr_store_type() Message-ID: <20240925225904.jryrxogfzxsbacsj@master> Reply-To: Wei Yang References: <20240814161944.55347-1-sidhartha.kumar@oracle.com> <20240814161944.55347-5-sidhartha.kumar@oracle.com> <20240925020431.joykmu4zzahoglcl@master> <47463235-bc45-46e2-8d9d-b62c201c6215@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <47463235-bc45-46e2-8d9d-b62c201c6215@oracle.com> User-Agent: NeoMutt/20170113 (1.7.2) X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 2A2D5140008 X-Stat-Signature: hrx1osat9rn7p4dp676phned77tnuu58 X-Rspam-User: X-HE-Tag: 1727305148-571852 X-HE-Meta: U2FsdGVkX18gCyToNQrLnNSDxgAi9C1YTVlr1dB5NHc1mRNRQuRt5bHrSwIr+Fq1zf2eDRQ0ZmUz6fWI2AqIyvy+1y+7nXDzkZL/niSskhmQlXjBLW4h/6HU9o89LkQu2xw8LahNzsuBdoSTACnfMz5syCYctWVAYNQD3jDIKeea9brIoxKpAPY9LNlowbPGpQItFKzhsOJ5s17WZSN7aAz3W9SCjLLNCvGVK7j9cJHvmYouZdN7gfzhfokUoD8GLtBzOWQiveF1EjT/XCXlnIrHnUNFj87ZLrPYNOcL/xpDI9/1KNTCvYAK54VsXlU18pIa/zakD94hRc0rJ4aIFZPE1u/0P+WiwMlE4Q4KqLGgEc6ubSf3CB7Y0y+kX4NtpriX7SqZR+puEtCqA76+38mLXz4sWS1TYRK/y0hYoBxZnkpalXf8dWwAPzYaBx+el9vbBg9RVbdKrLS6SmJScvmDmJXq6BzkLFYeshivaDfovhQfcBK2/7noYcr4c9r4Ie1q4LJHvCYsqVHEL6I2Jo2SBS8qW7QInKk4n2QFRrC9ZxYC4qY3PLjyrEjnipgqYHiBJ6rWJqvtgHUiuHmST1QdbYRsoARPkNkAr4GU862cyP3kA7s0M7M4Awsn9EsfinBiuyC85s6/AOV/sXrrTh5WWp4lDwrIwOQfpQJjOV1ir+DqVcStnbM62HECFWF6pPMOqd3yxbpEQ5dbeeTjvJpQouAjnRYNctlIHazNVO8ESE8Y3laivbYw4Tp9lNhpGAUKI1XDjMK3R4ZQ6S2wTl4HGvfNVhShjCaaPMR5iDspnUR6/9JczCXstIbYhg4CxaLdDnpuN18u/YW9C9s7Q6brsQ2Bx3O72XjKiERclSoNaVqcM93BMRl4BTjE9KnVMkKpoUvEqUy/PcIiUi8Gx7pTPLCCvRmM2xTMx70qcS9tOk9/DeeTkSsRA0rBrfb0G5HSpfexqghwNBFuk9n m5lPzGyi g9iTCh94xNhUPnO/5s74WKkeyQHExn8j36UkcW3c5l+iSd5IhS414hQ4Fh/REcSzyCPVDZ26nMVLZZ7GHj3IMo1oCrgjZEAH+3hRULQVI7tltht/YvhUG3txBrWmvlQTF4zZQPu+hcby3AruYAVOEfBs6co4v5DwtMuKRX0PhUHsiqARpHKosjo7qSRJsqLnRfB3g8Usbw/AV5NXIylXDhw2ZGyV2/JrOegVv3N2BxMQ3jxzKqpvF4bh0yoY6a2qQFmLoerMhdV2loJd5ucRrdbBXjGj36txFGI0shrn2lhj+k1hiGBPovLMCcNQC/d6U+WmvUKhUhtPQrjf8iHBEO8ga+B/slexeKg3PzVyy1YF3sA/o2TqFCIoPKkaUtKyGLAo9jwucM8bFOBrCf+V2kBcpBfdcdU68Xb+G+QEQXYlpXZPcVhE3OsGvVZy1+XY/JZi2Y5fifF4k9lZ1SyQnZApTuAFzNvIU/sUjICLjZBkBem2h6757eAQcE4hBP8td6L6r3s7rQAaJk085NBOBGlPqWC28pnamiRVepmNuuDHQcExacnM22qyxDcH69LQcLaYfsBmAUcdLJG8QhtxGFtQyPQWYFjNEEEgNomG/aEpAmrIXdn9KUgGYWBqsMHn+hjLHrynVDugb1aglRLHEzpj0mYOh1gdOkAavpLcG28WZLjoMzjMaAUNf2BB9fN+OhG6y X-Bogosity: Ham, tests=bogofilter, spamicity=0.020797, 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 Wed, Sep 25, 2024 at 02:36:21PM -0500, Sid Kumar wrote: > >On 9/25/24 2:33 PM, Sid Kumar wrote: >> >> On 9/24/24 9:04 PM, Wei Yang wrote: >> > On Wed, Aug 14, 2024 at 12:19:31PM -0400, Sidhartha Kumar wrote: >> > >> > Sorry for a late reply, I just see this change. >> > >> > > + >> > > +/* >> > > + * mas_wr_store_type() - Set the store type for a given >> > > + * store operation. >> > > + * @wr_mas: The maple write state >> > > + */ >> > > +static inline void mas_wr_store_type(struct ma_wr_state *wr_mas) >> > > +{ >> > > +    struct ma_state *mas = wr_mas->mas; >> > > +    unsigned char new_end; >> > > + >> > > +    if (unlikely(mas_is_none(mas) || mas_is_ptr(mas))) { >> > > +        mas->store_type = wr_store_root; >> > > +        return; >> > > +    } >> > > + >> > > +    if (unlikely(!mas_wr_walk(wr_mas))) { >> > > +        mas->store_type = wr_spanning_store; >> > > +        return; >> > > +    } >> > > + >> > > +    /* At this point, we are at the leaf node that needs to be >> > > altered. */ >> > > +    mas_wr_end_piv(wr_mas); >> > > +    if (!wr_mas->entry) >> > > +        mas_wr_extend_null(wr_mas); >> > > + >> > > +    new_end = mas_wr_new_end(wr_mas); >> > > +    if ((wr_mas->r_min == mas->index) && (wr_mas->r_max == >> > > mas->last)) { >> > > +        mas->store_type = wr_exact_fit; >> > > +        return; >> > > +    } >> > > + >> > > +    if (unlikely(!mas->index && mas->last == ULONG_MAX)) { >> > > +        mas->store_type = wr_new_root; >> > > +        return; >> > > +    } >> > > + >> > > +    /* Potential spanning rebalance collapsing a node */ >> > > +    if (new_end < mt_min_slots[wr_mas->type]) { >> > > +        if (!mte_is_root(mas->node)) { >> > > +            mas->store_type = wr_rebalance; >> > > +            return; >> > > +        } >> > > +        mas->store_type = wr_node_store; >> > > +        return; >> > > +    } >> > After this check, we are sure new_end >= mt_min_slots[wr_mas->type]. >> > >> > > + >> > > +    if (new_end >= mt_slots[wr_mas->type]) { >> > > +        mas->store_type = wr_split_store; >> > > +        return; >> > > +    } >> > > + >> > > +    if (!mt_in_rcu(mas->tree) && (mas->offset == mas->end)) { >> > > +        mas->store_type = wr_append; >> > > +        return; >> > > +    } >> > > + >> > > +    if ((new_end == mas->end) && (!mt_in_rcu(mas->tree) || >> > > +        (wr_mas->offset_end - mas->offset == 1))) { >> > > +        mas->store_type = wr_slot_store; >> > > +        return; >> > > +    } >> > > + >> > > +    if (mte_is_root(mas->node) || (new_end >= >> > > mt_min_slots[wr_mas->type]) || >> > > +        (mas->mas_flags & MA_STATE_BULK)) { >> > The check (new_end >= mt_min_slots[wr_mas->type]) here seems always >> > be true. >> > >> > So the if here seems not necessary. Do I miss something? >> >> It is true that at this point new_end >= mt_min_slots[wr_mas->type] must >> be true but if we remove that check we won't catch this wr_node_store >> case if !mte_is_root() and !(mas->mas_flags & MA_STATE_BULK). >> >> We could change the default store type to be wr_node_store and get rid of >> that whole if statement entirely. >> >> This diff passes the tests: >> >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c index >> 4f34e50c92b5..2ae0c4da9d74 100644 --- a/lib/maple_tree.c +++ >> b/lib/maple_tree.c @@ -4242,14 +4242,7 @@ static inline void >> mas_wr_store_type(struct ma_wr_state *wr_mas) return; } - if >> (mte_is_root(mas->node) || (new_end >= mt_min_slots[wr_mas->type]) || - >> (mas->mas_flags & MA_STATE_BULK)) { - mas->store_type = wr_node_store; - >> return; - } - - mas->store_type = wr_invalid; - MAS_WARN_ON(mas, 1); + >> mas->store_type = wr_node_store; } >> >> do you think this makes sense? >> >Sorry this diff wasn't formatted correctly, it should look normal now: > >diff --git a/lib/maple_tree.c b/lib/maple_tree.c >index 4f34e50c92b5..2ae0c4da9d74 100644 >--- a/lib/maple_tree.c >+++ b/lib/maple_tree.c >@@ -4242,14 +4242,7 @@ static inline void mas_wr_store_type(struct >ma_wr_state *wr_mas) >                return; >        } > >-       if (mte_is_root(mas->node) || (new_end >= mt_min_slots[wr_mas->type]) >|| >-               (mas->mas_flags & MA_STATE_BULK)) { >-               mas->store_type = wr_node_store; >-               return; >-       } >- >-       mas->store_type = wr_invalid; >-       MAS_WARN_ON(mas, 1); >+       mas->store_type = wr_node_store; > } > I am ok for this one. >> Thanks, >> >> Sid >> >> > > +        mas->store_type = wr_node_store; >> > > +        return; >> > > +    } >> > > + >> > > +    mas->store_type = wr_invalid; >> > > +    MAS_WARN_ON(mas, 1); >> > > +} >> > > + -- Wei Yang Help you, Help me