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 CBBE4C46467 for ; Wed, 4 Jan 2023 20:20:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4B4A08E0002; Wed, 4 Jan 2023 15:20:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 464BD8E0001; Wed, 4 Jan 2023 15:20:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 32C9E8E0002; Wed, 4 Jan 2023 15:20:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 20AF98E0001 for ; Wed, 4 Jan 2023 15:20:55 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E594980CE2 for ; Wed, 4 Jan 2023 20:20:54 +0000 (UTC) X-FDA: 80318235228.02.2C3A3A0 Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) by imf18.hostedemail.com (Postfix) with ESMTP id 3211B1C0007 for ; Wed, 4 Jan 2023 20:20:52 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ncNwW2r7; spf=pass (imf18.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.47 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=1672863653; 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=U9D51sBkHzbum0wkfy6pYLD6DjeGvD3BV7AVetui/1c=; b=lXv3GkiaGqZSQ2nLusdKlJcgIddsAFLHpmsnYBrxbHFytraA/yvRnrGRQ4uNuZqAEJ5kx6 C5eoYfHNJrmdLqpdwiS2qKZWY9cSsgKTBME/5yqQmgJQsMDL8GTzEtaxi98eb/dStPhmRv X/Tdlrz0NoWC3HoE0vNKeu+0XwwAXP4= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ncNwW2r7; spf=pass (imf18.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.47 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=1672863653; a=rsa-sha256; cv=none; b=Dn/69jt+8E45vS5Tfulmy82qTiq2JUXInkajuJj8EWooVOfpGqX4oPmO9+2MfPI9+TpBY7 T3QL+saC3RB+VFMGi8bm/m/bWicdnk9tPVhqCIJ9Zf/qAfqfribA3jILlyOm9jh9mpd084 iGDF8n8oln5zB1QL7VwPG2bEogW0jaU= Received: by mail-wm1-f47.google.com with SMTP id i17-20020a05600c355100b003d99434b1cfso17278908wmq.1 for ; Wed, 04 Jan 2023 12:20:52 -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=U9D51sBkHzbum0wkfy6pYLD6DjeGvD3BV7AVetui/1c=; b=ncNwW2r7IzCm6QlXyN7XxBG5XWVWkMevZBTomy13qHLdiEaCQ3pEmYio8vo9yJteHY Zuso76/7bdzSQrFPjy11mzM/SmC/5H1Kr2rlaXY5x43Q2xsJkiOv6mfMIY9zAnSgGJxD jowcsBqatuJns6pvar+xj8ORL2qNQJHKP/PdZtJG/Samn0a5HvpDJeMFCo8xnSF1BLZ1 g6Y3gG5svqJ2SclfPYrY2zSaVN6F43cnfVJyykI31kXvnCcOOOwd3ZqgPYZEmDMRdMr4 AGcWuV18PscMnFpO9HiD9yb0d9q1nwsuITcNdCJUQ3q38lpIinn0P8+yKW4tqN2n4LFx 1OVg== 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=U9D51sBkHzbum0wkfy6pYLD6DjeGvD3BV7AVetui/1c=; b=17qVxxvRu5uLNnnOqZwP0S6ogrN00oqDBFwzKcN6B6MC3GK83WrAv9XQu4dsNrKBEc J8MAv0oranqKHb6m32aJ6v0t8hGOECwDlgy0L/vceyEu4ri5jinB6tyEfrF1Orv9llfz +ARpP3LwI/wkq5m+siM5LO4sASO72tXMj0nOC+rDuGfXP00ZjiIfzWIYaSeaWK4YTLjZ MGXFXaTzM7RHBvUVp1uuHhzFzHcrFiGYaqxt4ncXtWUfCyDkRPLaV1JOp9zOIrZw/Rt6 KOaHliR7ko5YEeXuJde7ZdNJp48xXAPvhCtATMOvz/UT7o3xDoyP2nOONuoU8JNE7ZEA 87LQ== X-Gm-Message-State: AFqh2komIqAvg1fZj4k6mr3a37YV+DtFLQK59klcUlf+byE5iwwJVlxk KQ2JfelpAECooH26w//+el4= X-Google-Smtp-Source: AMrXdXuRHdaqt4oF0yfOeB3UkSYeCEzYx2NUGm5/c7P6LHOWkrBeZmy2cA2HfUVOnwfFMJNKlTvQSQ== X-Received: by 2002:a05:600c:3acc:b0:3d9:a145:91a with SMTP id d12-20020a05600c3acc00b003d9a145091amr13691746wms.28.1672863651736; Wed, 04 Jan 2023 12:20:51 -0800 (PST) Received: from localhost (host81-157-153-247.range81-157.btcentralplus.com. [81.157.153.247]) by smtp.gmail.com with ESMTPSA id t184-20020a1c46c1000000b003b4a699ce8esm51816695wma.6.2023.01.04.12.20.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Jan 2023 12:20:50 -0800 (PST) Date: Wed, 4 Jan 2023 20:20:49 +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: X-Stat-Signature: kiskennqn7ud6r8u8qqe4tkek895fb51 X-Rspam-User: X-Rspamd-Queue-Id: 3211B1C0007 X-Rspamd-Server: rspam06 X-HE-Tag: 1672863652-774595 X-HE-Meta: U2FsdGVkX18+JbalaY1zvCxc1hR+Lul/f4jTOPoBQu00x65Jq5GJcB9WxPlzdBgpE1I+Uz7r6uXUAjHD3Dl+I6xHkZQOv3AGMkRwSh9nnnA/Z9RIlTImFbtO62n5vgUOATwcgBvddFv87kxdSuRJxAYLEe7lF/L1Q9Dtegyt3BBVxvJqiZKe617iwujUYQvmSdVdDFNemN+VKXoJVAktiLwX+lfzjOm8EjxZNOwe2SiGCqtFLzLBFtJQjlYEsM0A30cnekDjSkEbl27wZnJBPY73KC6h4fvYXXgJZcTieA/RYXtJD6/lYGUZc3aP2dWfzwoo+YKwz4N2QP5M4IQUoiYEjU9SHLnw6t5b5N6Xh6cdpsLOYYjE7dQqXwtrSqt8M67gOOJ2OPOlx2Va4mWvc7y6pkD7Fry7QYmmVRnYST5WhVAKUoFbzMEJldluOtQakwPW5cV1D6zzxmaiVlilkRbJFQgzvlQ3maRVqlpeCGaCc1GxrraPUoCi0GamH8soo3tZYVO1RY44EYDHaxn7qqNIZ7IuDkDTUZfDrALNI+EEuintuwoT/RQJQ5o/HxC8WBBYELy52ppAFEF9KFdJILdjpLdkjaMKg+4WUSQgXDPRScn3KR9we8TkPZQGCUvXBhZ3YVhc95mkrxe1McoEyr0y6PmOHwgWmk9/Li7O7lmMWJyRBJZ5lsRGZ+IcLiTd5U9l+nJ9UUzt647E9tiVVlWi7qVsydouBGNzDrJ1L5Czja4Ie6FwnTTD6iExLV9ELHMrrhDaN0dmEvIm0KMdohkaAJxkqIpRp+Z/7To+Y9s8FMnwpMqGQ0ReFx5QKyMRol1ylQlJqILoZIIoaWPr0la3aYVl9KuR4mQ0/X9ySmk1aIHC3LOJoFxAUWFGFhBIsHxHvIyMDBdWVID2jLif1H6BMTdXj+z115+LxIxgjERm+iKe8iyjKmF9F5rSrrB4w9WdfZxNFRtdSs7Ld75 FVSFozbq 83gs+po/0a7+cJGAkp45MjGl8jt0mDz0qzsNTomJTFCkr0JAqNCmyr+KFlcxTrUxDVqMZAoXi2OHh2hwSSob8QEebFgkcgS15MAW3zusUyL3cJrBP23BWxU4EPs3OMOePfcCbjjhqBgqlCag= 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, Jan 04, 2023 at 04:01:36PM +0800, Baoquan He wrote: > On 12/17/22 at 12:06pm, Lorenzo Stoakes wrote: > > 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--; > > > + } > > Very sorry, Lorenzo, I just noticed this mail. It's very weird. Earlier, > Uladzislau's reply to patch 2/7 got to be seen in my mutt mail client 10 > days later. I am not sure it's my mail client's problem, or a mail server > delivery issue. > Odd, maybe try lei with mutt I find that works well :) > > > > 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?' > > Not sure if I got you. for_each_set_bitrange only iterates the used > regions. So in the for loop, what we do is fill zero into the buffer > below the used region, then read out the used region. You said > 'continue', I don't understand what it means. > > Assume we have 3 used regions in one vmap block, see below diagram. > |_______|______________|________|_____________|_____|_____________|______| > |hole 0 |used region 0 |hole 1 |used region 1|hole2|used region2 |hole 3 | > > hole 0,1,2 will be set zero when we iterate to the used region above > them. And the last hole 3 is set at the end of this function. Please > help point it out if I got it wrong. Maybe let me rephrase:- - We want to read `count` bytes from `addr` into `buf` - We iterate over _used_ blocks, placing the start/end of each block in `rs`, `re` respectively. - If we hit a block whose start address is above the one in which we are interested then:- - Place a zero byte in the buffer - Increment `addr` by 1 byte - Decrement the `count` by 1 byte - Carry on I am seriously confused as to why we do this? Surely we should be checking whether the range [addr, addr + count) overlaps this block at all, and only then copying the relevant region? It's the fact that blocks are at base page granularity but then this condition is at byte granularity that is confusing to me (again it's _very_ possible I am just being dumb here and missing something, just really want to understand this better :) > > > - 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? > > Now only vm_map_ram area sets flags, all other types has vm not null. > Since those temporary state, e.g vm==NULL, flags==0 case has been > filtered out. Is below you suggested? > > size = (!vm&&flags)? va_size(va) : get_vm_area_size(vm); > or > size = (vm&&!flags)? get_vm_area_size(vm):va_size(va); > Sorry I didn't phrase this very well, my point is that the key thing you're relying on here is whether vm exists in order to use it so I simply meant:- size = vm ? get_vm_area_size(vm) : va_size(va); This just makes it really explicit that you need vm to be non-NULL, and you've already done the flags check before so this should suffice.