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 25508C4332F for ; Mon, 19 Dec 2022 09:09:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 66ADC8E0002; Mon, 19 Dec 2022 04:09:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 61A0E8E0001; Mon, 19 Dec 2022 04:09:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4BC4F8E0002; Mon, 19 Dec 2022 04:09:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 3821B8E0001 for ; Mon, 19 Dec 2022 04:09:30 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 0C9D040288 for ; Mon, 19 Dec 2022 09:09:30 +0000 (UTC) X-FDA: 80258482500.15.4D515E5 Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) by imf15.hostedemail.com (Postfix) with ESMTP id 43FC7A0005 for ; Mon, 19 Dec 2022 09:09:28 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=e603OMaa; spf=pass (imf15.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.52 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1671440968; a=rsa-sha256; cv=none; b=6C2PtFyEuByF/Nj1QUUDNfhpSjy3a626d7XPdWrJOl07b2ZMKi4/LbUm0SikU94Ybpfqgj A0pI/6FomUgCRNmCz6kT9O4LaNToWtK7K+iRT3DLzHrd0IoqJHCpgiJ2oBTD40YPGdtIQt AoiZzZLeFc2UnYjVFgo7+IpQ/o1Zmqo= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=e603OMaa; spf=pass (imf15.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.52 as permitted sender) smtp.mailfrom=lstoakes@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=1671440968; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=pPuIlazdNa8bEEQAv8f+ot8Iw/x3w5Arb8nwBFAXtxQ=; b=jKB5sJeYlUVRfHxI4LodDfIqqqS6mDVpzg3/9jAHgef0LloZShomGdv3h+SZcALh3OLief HfKHYNJ4OGzmBLGh4Kf0p0qTFTp2Ji61uMsudjg4xlybj+k393cCrQAB0IK0pUnS3Z+fF8 g1cEchEJcjRI+psrwZnkoOn52D5z4Ig= Received: by mail-wr1-f52.google.com with SMTP id h7so7930610wrs.6 for ; Mon, 19 Dec 2022 01:09:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=pPuIlazdNa8bEEQAv8f+ot8Iw/x3w5Arb8nwBFAXtxQ=; b=e603OMaaJGtgZrkEAXZi/CD/C9PkfwuC4uUsxJInKU/DhNwOSQlMu282cxpkbUHPAD bhcSHmxHAffCTgc+DiwNcBGC+nSmLmBnIkYco6Zernntd5HLPXGUfk5Ep7e4YhJGpCNd PeLTubNyxsP5eh26h31HyYFcK3Z+7h6V7QhYjJO+56JS0OIZ8fAUuo0iue5Q27Hm9DTo d7lQBnRXPHGgA4EwA5JvLYqVdc44ILdQ/jTStsFFO+s4ErMF9uIIS+Ki9/A4Dt1XHmp+ mYxC2DRYzq7ohtFoxSWHe8fkZY+8FDqgYq6SpeFCznW+/bgftIPSw+009nhZXbMTgc2v 0FXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=pPuIlazdNa8bEEQAv8f+ot8Iw/x3w5Arb8nwBFAXtxQ=; b=PLIqcQDG8Sla9+Rj1SYQLod1ZsMY0w5Ii03IJkZYkw8wZbc1i1l5PRhW/lpPufyYLU v/W8MDRuPrw6Gb0Q9JoVU7iFsd7hr/ikTXrBKVnr1YjNC4gREUmOKHPW6aMUrnjMnNrB MKKe/c4q9FDZjx405/Sf0sx2XODdsYWH6YI17YJmdRfXTZAlZWoPqgSu1zMTO75U6z59 8trUWTL79C8/fwD0o4UAglnnlh8Oo1QHodQocIGKaD0d9W9xKs7dzcP/znFAGCgI0As+ aJc2k3A3zLskD27yhYyvOsEzP+kb4oDz+F/HGb1FZ9RNazWZuGK+XBqLq7UYmibqmZ+D M0Ig== X-Gm-Message-State: ANoB5pmRYUc2UbxDhiiDbyS/rxGQIf8SZ2Olm3pJwu9G3Jk7OsOezZUs SpdxyVQKXpNwgax9VjtaZkM= X-Google-Smtp-Source: AA0mqf48gx6nM8+DueCd8yUWxuVBhHrh5VUYD64cb8pBNeaCVVyWNvX4Y0wJLm2nVA+r6bxOnYnp9A== X-Received: by 2002:a5d:4ac3:0:b0:242:31bc:e009 with SMTP id y3-20020a5d4ac3000000b0024231bce009mr23234923wrs.2.1671440966714; Mon, 19 Dec 2022 01:09:26 -0800 (PST) Received: from localhost ([2a00:23c5:dc8c:8701:1663:9a35:5a7b:1d76]) by smtp.gmail.com with ESMTPSA id s4-20020a5d5104000000b0024207ed4ce0sm9290164wrt.58.2022.12.19.01.09.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Dec 2022 01:09:25 -0800 (PST) Date: Mon, 19 Dec 2022 09:09:25 +0000 From: Lorenzo Stoakes To: Baoquan He Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, urezki@gmail.com, stephen.s.brennan@oracle.com, willy@infradead.org, akpm@linux-foundation.org, hch@infradead.org Subject: Re: [PATCH v2 2/7] mm/vmalloc.c: add flags to mark vm_map_ram area Message-ID: References: <20221217015435.73889-1-bhe@redhat.com> <20221217015435.73889-3-bhe@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: 43FC7A0005 X-Rspamd-Server: rspam01 X-Stat-Signature: wwjpbe3jikpkrq6651tdpcobxjsi64mu X-HE-Tag: 1671440968-386595 X-HE-Meta: U2FsdGVkX19fnrx0rKsQHKo25aLWFtNlaPYWRXBUd4H9GC2RkjjjstCqIeD+0Hg6liRreWMtF3vsL8OP+IL+mexQwlhHN06Y943/Bminf2dCkz/QgYI0L4gQ2hmMzkLxWJK+TzQhuTwremEqAWnES95A0b73PE+jUaAny2wPI1GIJQhSi0R2rdx7PL0TUq3+/rxE06Wg28+Z52GF/kjhpObr5O1rwkN0eCNRkk731zFiaA4ODEbTzWa5iIvnYfCuc/IN4uuTDOZhna47M2J7ar6pHumY6/hFOnc6Xwvn8OMwmMArvYbAQ/FA9eD7JisRq86RsnBazWeb1q4GK3oHSu4MzVUHFq4ttAm7A7pn6UEDnY/U50fHDYP0sD3yki9gLaJAVOTiQdmb/Nn24sUNaks9Jm4EYAnaUWm2CnYIGhgsVDpsraC+sepxcFupl20WzZsNT6fyBSrfkiWYEwucDjV19gmrt5jT45zzT4+mdgdvk96E02Y9axMZ+b4B52cCAAdSTP7R5BGki6tNBLJUSEAELw1hfj6wy4mT98nwxE9pGtJZW/9VtWDyyCgdw7WO335r/OrEL+LXrICgWXG5ftoLJQTSxWfApnuwJ8s0KZJ/RUs6tp5mn3doaeLNNV3Sb+pXTa3v+8/tAr1aMPpX5RbpPLXmZNvHbJMUvy8oVm21Txms/ZtLf3a2PcqUo70dLsK6d2mHFKjONoalxuQeg9YFweSE+GE5VY1rBO/oh/l/W6EJYsXTgIq+cQSYIchgcntSjTh7WPVloZC5vU+YDDVVgfgVpsN83qwW0eXgVqGxMN3QKtF9Zvv2LUhNxyPdC3G0xVNSqBryoW/ClE98Lxkc9EmZQgCvUt3gUUzUbOmSQtsGtHRoHEdV0JNJgCfjdp+wI+uLAZGkePYgb1Lj0gixg/PPSFzhKa/YtdlkgcE9cLHLKNCyznI20IQRJsfPwacX4ZWOwg3hInvEzaA TGBCZw5j qn5hLG3Yz80pRFtAMR6JOfAmmyfLrjZYM5NNEYbmMzi5jMYygyqMPkEr19uHCDcK4orBNqvAMc1G+/slJWZ5kl7uwnkq0R8sdWAwMNeuUJWLBRERVTLw63IVmSYr2L+L8HmsZ X-Bogosity: Ham, tests=bogofilter, spamicity=0.000102, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Dec 19, 2022 at 04:01:00PM +0800, Baoquan He wrote: > On 12/17/22 at 11:44am, Lorenzo Stoakes wrote: > > On Sat, Dec 17, 2022 at 09:54:30AM +0800, Baoquan He wrote: > > > @@ -2229,8 +2236,12 @@ void vm_unmap_ram(const void *mem, unsigned int count) > > > return; > > > } > > > > > > - va = find_vmap_area(addr); > > > + spin_lock(&vmap_area_lock); > > > + va = __find_vmap_area((unsigned long)addr, &vmap_area_root); > > > BUG_ON(!va); > > > + if (va) > > > + va->flags &= ~VMAP_RAM; > > > + spin_unlock(&vmap_area_lock); > > > debug_check_no_locks_freed((void *)va->va_start, > > > (va->va_end - va->va_start)); > > > free_unmap_vmap_area(va); > > > > Would it be better to perform the BUG_ON() after the lock is released? You > > already check if va exists before unmasking so it's safe. > > It's a little unclear to me why we care BUG_ON() is performed before or > after the lock released. We won't have a stable kernel after BUG_ON()(), > right? BUG_ON()'s can be recoverable in user context and it would be a very simple change that would not fundamentally alter anything to simply place the added lines prior to the BUG_ON(). The code as-is doesn't really make sense anyway, you BUG_ON(!va) then check if va is non-null, then immediately the function afterwards passes va around as if it were not null, so I think it'd also be an aesthetic and logical improvement :) > > > > Also, do we want to clear VMAP_BLOCK here? > > I do, but I don't find a good place to clear VMAP_BLOCK. > > In v1, I tried to clear it in free_vmap_area_noflush() as below, > Uladzislau dislikes it. So I remove it. My thinking is when we unmap and > free the vmap area, the vmap_area is moved from vmap_area_root into > &free_vmap_area_root. When we allocate a new vmap_area via > alloc_vmap_area(), we will allocate a new va by kmem_cache_alloc_node(), > the va->flags must be 0. Seems not initializing it to 0 won't impact > thing. > You are at this point clearing the VMAP_RAM flag though, so if it is unimportant what the flags are after this call, why are you clearing this one? It is just a little confusing, I wonder whether the VMAP_BLOCK flag is necessary at all, is it possible to just treat a non-VMAP_BLOCK VMAP_RAM area as if it were simply a fully occupied block? Do we gain much by the distinction? > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 5d3fd3e6fe09..d6f376060d83 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -1815,6 +1815,7 @@ static void free_vmap_area_noflush(struct vmap_area *va) > > spin_lock(&vmap_area_lock); > unlink_va(va, &vmap_area_root); > + va->flags = 0; > spin_unlock(&vmap_area_lock); > > nr_lazy = atomic_long_add_return((va->va_end - va->va_start) >> > > > >