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 83F55C4332F for ; Sat, 17 Dec 2022 12:06:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D845A8E0002; Sat, 17 Dec 2022 07:06:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D34C78E0001; Sat, 17 Dec 2022 07:06:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C24B98E0002; Sat, 17 Dec 2022 07:06:12 -0500 (EST) 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 AF4768E0001 for ; Sat, 17 Dec 2022 07:06:12 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 84F471606E7 for ; Sat, 17 Dec 2022 12:06:12 +0000 (UTC) X-FDA: 80251670184.29.21B1E52 Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) by imf10.hostedemail.com (Postfix) with ESMTP id C72E4C0018 for ; Sat, 17 Dec 2022 12:06:10 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=jiZSEsI4; spf=pass (imf10.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.48 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=1671278770; 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=POKL28xeKckqwooEWuCkv5vGb2GjbJvUlEjyzAmbpY8=; b=J2qilt0uYaws0daqfKy9UV17BQeheofEqr/RzTMGUazRTQebRtvpN8YWyyWJb0+dZWGYsB yjymKKCYnoW9x5nmBf01VTTtZbLvH3WQ53MYAkBL3T2HtpYA5G+g3hr8CqOYknQ9XL8tYg 0RESyisYnDaebBuTm4I6P1gomHBEdw0= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=jiZSEsI4; spf=pass (imf10.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.48 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=1671278770; a=rsa-sha256; cv=none; b=pGNQI3cSPPNbw9uabPjOX2CRvPsneAtnCgH5ZBym7do3i6I2DgyyhZiNiM4fBJaAah3Odz VhbWtaMlq3jYSJ4TPGgtf0yyB28ePbn8xmB5SBxrxjUP+ONtYi+wsp4ygqVCJJwInNjoYp B1/fppXqazTifRMWNZXYjFYKUCBb8UE= Received: by mail-wr1-f48.google.com with SMTP id h12so4766235wrv.10 for ; Sat, 17 Dec 2022 04:06:10 -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=POKL28xeKckqwooEWuCkv5vGb2GjbJvUlEjyzAmbpY8=; b=jiZSEsI42oaWN/lhuulJgHTYd2WoL+sd2M+NYHisj8JvoEVAtdek3GPpyztYdzO1eW IAaTNQ+oU6u1qPvNhi7fgYTHUhBN+ch9JxG0C6l0272PdfBcHFrC2P/IfBKLq5nWTYO2 Og/brGySLw5EyoBGR4yIsKTUHwvqk7Ll0Vu0we1ZzLY5CB/CKEOdV6Fgx4H6cIdexr99 aDfC3p35CdKeQQptej8W8WqZta308wdnFNLiZldl/ZySdY3JhyIEAaxu4VQAGLDS6o0c cNDj4/UDS71NK1fZF6gWv8NnDJbV1JBjz5K2pW/WDnigmUyOEzAW0R5ZC81Veouf7jrj ycfQ== 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=POKL28xeKckqwooEWuCkv5vGb2GjbJvUlEjyzAmbpY8=; b=cV/oxuykgbA17qYXBlKyUQO3IBwhfbwkr/NLiCrZmu00Jrfw3kuz89+wwd5ZScfVYz rlQpZV6GBwWIcml9FZjNMetA79FLDEjoEVrdyqLnaEa39Txk7ee7jStMd6TleEcoJXLH /mWu6gIUwDTmyEbxfIrxVrvW6J/f/OSIz5WE9H1eONSGKQHtDnQG16rpBGWQouQ5gjNC QvFIbAnmKZz9MbphA0y5TXwYDdzk5qPWMr4chQ+oqrtRI//xlS5BY66B2s91utWP83ou eDvLa6J81fCTN2fxz5OjsZRZRrRdInPDXmfekjitaPTekkoR7WeH5cVOYfpV5H0xt8el JyAA== X-Gm-Message-State: ANoB5pn8q6+A4eUbRQnho7y3WgKOHBAtMIA+QpGtci9gBENsaRwGSxIt E2TFGpi7cJKt4i80iJF8nlA= X-Google-Smtp-Source: AA0mqf7KTCacAybkroui6e8EqfpF8Ukpby5PrdXTFPQ4zRHtpE2yeiH7GhzUv6GuNzRNsCP6NwdjfA== X-Received: by 2002:a5d:4a0e:0:b0:242:643e:6954 with SMTP id m14-20020a5d4a0e000000b00242643e6954mr24254256wrq.14.1671278769322; Sat, 17 Dec 2022 04:06:09 -0800 (PST) Received: from localhost ([2a00:23c5:dc8c:8701:1663:9a35:5a7b:1d76]) by smtp.gmail.com with ESMTPSA id y6-20020adfe6c6000000b00241e8d00b79sm5494067wrm.54.2022.12.17.04.06.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Dec 2022 04:06:08 -0800 (PST) Date: Sat, 17 Dec 2022 12:06:08 +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 3/7] mm/vmalloc.c: allow vread() to read out vm_map_ram areas Message-ID: References: <20221217015435.73889-1-bhe@redhat.com> <20221217015435.73889-4-bhe@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221217015435.73889-4-bhe@redhat.com> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: C72E4C0018 X-Stat-Signature: rtczqpa3aetn5yfdotuocd7wmx666kr9 X-HE-Tag: 1671278770-229559 X-HE-Meta: U2FsdGVkX1+jLNASk4WVP45Ux4QuxTZKFqub5aUZ0hj/LcCJhG/dpBzlJ9sDd2jCgEdQAQVNO5WwwObaDDXiBCB1S/+nOSinAE88htQZSEp8F7Ow2zm5IwzIoZLsfNpDrnf6b7GlaOnAVDzBq5woqCg/zS7/d8WyY5VeHVFi/y8gLUmkgml1SZOk7o0q3P1DeX7hLlJUe7rq2IjTA6Th67TbGHwu/BHD0oT09HbkOKEE5rn9JJPLf8BsGHDs+PZp5R8bzsLnsLWjkzajVREMJm9Jj/TbvFInNKfndypes+Wfmih/hfSNnwmjhPY0C2Hx7u6+V6fvDUsrxuCEygwc9TxcXr4DsfiR4g8j4LgCaL6dfBMXUnCGvejUjF1kktmaLLsisxf/SupUgT7/rHhEzmzcpS1VPstoibJyfmMUPNAOcyfK7PCSjliEdgCIRX5B/MKyH5qRfG0iMUgns/qG7ZFq6lINSdBuY+kodyRw+EX1YxLvr+qc+69pKhW1l4xpPstCXoEIeEkQoMUdPLMTzTfILZZGX5RpH4SgS+FuHJ1r5BMfSpTN9gNAbcf3yNGvlfvEdQNap4ANhu5a1GSiI1HIHDAvZa95tEJOUUddOVFrac+Zq5SqtQ9AOc0MRf1dXz/QZfVWsK1vbS0xdtDH6oVi+JzcMlKgaffsIl6Vs1gZtHjVbki5xSBJdP6AQJIk+OJcaUw8iNWgtbkA/K4WI9HoeCpqw9vOQ+Pob4QR/JeaOZlpPDRd54Zm3IBUOAmcW/4P63rSd+L3EuT8sXdxSKq1t6YqGQTvK2/KgGgXAd2VToJIwMhxxh4SlcAFyn+u0FWOWw9uDdIDZXJPdQklZpTiJ43PuAwdvSOcSubGIlnSuY5THa7CYVHrIzccZbwuwvmqUmob1Y8E9TjZ8r0nxd+QW7z3sZR7p1vwpLGcJeWLJ3KEMjyJkaW9jYcIKT1pcKylm/qCwBcW0/WCUEV m+X2A7+i 7Ng75wkbPvXVqRe3qADrp92Mml5R4okI2f5sWrss8v8h1+iXRztBl4tydx35rX9IGscAfNOVzqE/TvjEU2r6JQ0QeTIvF2wvkZGtR54x8CcTCLy0X2dlQx9m+JXCSBodL4BGj 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 Sat, Dec 17, 2022 at 09:54:31AM +0800, Baoquan He wrote: > Currently, vread can read out vmalloc areas which is associated with > a vm_struct. While this doesn't work for areas created by vm_map_ram() > interface because it doesn't have an associated vm_struct. Then in vread(), > these areas will be skipped. > > Here, add a new function vb_vread() to read out areas managed by > vmap_block specifically. Then recognize vm_map_ram areas via vmap->flags > and handle them respectively. > > Signed-off-by: Baoquan He > --- > mm/vmalloc.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 59 insertions(+), 7 deletions(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 190f29bbaaa7..6612914459cf 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -3515,6 +3515,51 @@ static int aligned_vread(char *buf, char *addr, unsigned long count) > return copied; > } > > +static void vb_vread(char *buf, char *addr, int count) > +{ > + char *start; > + struct vmap_block *vb; > + unsigned long offset; > + unsigned int rs, re, n; > + > + vb = xa_load(&vmap_blocks, addr_to_vb_idx((unsigned long)addr)); > + > + spin_lock(&vb->lock); > + if (bitmap_empty(vb->used_map, VMAP_BBMAP_BITS)) { > + spin_unlock(&vb->lock); > + memset(buf, 0, count); > + return; > + } > + for_each_set_bitrange(rs, re, vb->used_map, VMAP_BBMAP_BITS) { > + if (!count) > + break; > + start = vmap_block_vaddr(vb->va->va_start, rs); > + if (addr < start) { > + if (count == 0) > + break; > + *buf = '\0'; > + buf++; > + addr++; > + count--; > + } I may be missing something here, but is this not essentially 'if the address is below a used region, write a single null byte into the buffer and continue, assuming we are now in a used area?' This doesn't seem right, but I am happy to be corrected (perhaps we only expect to be a single byte below a start region?) > + /*it could start reading from the middle of used region*/ > + offset = offset_in_page(addr); > + n = (re - rs + 1) << PAGE_SHIFT - offset; The kernel bot has already picked up on this paren issue :) > + if (n > count) > + n = count; > + aligned_vread(buf, start+offset, n); > + > + buf += n; > + addr += n; > + count -= n; > + } > + spin_unlock(&vb->lock); > + > + /* zero-fill the left dirty or free regions */ > + if (count) > + memset(buf, 0, count); > +} > + > /** > * vread() - read vmalloc area in a safe way. > * @buf: buffer for reading data > @@ -3545,7 +3590,7 @@ long vread(char *buf, char *addr, unsigned long count) > struct vm_struct *vm; > char *vaddr, *buf_start = buf; > unsigned long buflen = count; > - unsigned long n; > + unsigned long n, size, flags; > > addr = kasan_reset_tag(addr); > > @@ -3566,12 +3611,16 @@ long vread(char *buf, char *addr, unsigned long count) > if (!count) > break; > > - if (!va->vm) > + vm = va->vm; > + flags = va->flags & VMAP_FLAGS_MASK; > + > + if (!vm && !flags) > continue; > This seems very delicate now as going forward, vm _could_ be NULL. In fact, a later patch in the series then goes on to use vm and assume it is not null (will comment). I feel we should be very explicit after here asserting that vm != NULL. > - vm = va->vm; > - vaddr = (char *) vm->addr; > - if (addr >= vaddr + get_vm_area_size(vm)) > + vaddr = (char *) va->va_start; > + size = flags ? va_size(va) : get_vm_area_size(vm); For example here, I feel that this ternary should be reversed and based on whether vm is null, unles we expect vm to ever be non-null _and_ flags to be set? > + > + if (addr >= vaddr + size) > continue; > while (addr < vaddr) { > if (count == 0) > @@ -3581,10 +3630,13 @@ long vread(char *buf, char *addr, unsigned long count) > addr++; > count--; > } > - n = vaddr + get_vm_area_size(vm) - addr; > + n = vaddr + size - addr; > if (n > count) > n = count; > - if (!(vm->flags & VM_IOREMAP)) > + > + if ((flags & (VMAP_RAM|VMAP_BLOCK)) == (VMAP_RAM|VMAP_BLOCK)) > + vb_vread(buf, addr, n); > + else if ((flags & VMAP_RAM) || !(vm->flags & VM_IOREMAP)) > aligned_vread(buf, addr, n); > else /* IOREMAP area is treated as memory hole */ > memset(buf, 0, n); > -- > 2.34.1 >