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 73BCBC27C77 for ; Fri, 14 Jun 2024 03:19:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E68286B00C9; Thu, 13 Jun 2024 23:19:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 388D86B00F6; Thu, 13 Jun 2024 23:19:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A1BA06B00EF; Thu, 13 Jun 2024 23:19:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 045D88D0001 for ; Thu, 13 Jun 2024 23:18:26 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 7AD54C0476 for ; Fri, 14 Jun 2024 03:18:26 +0000 (UTC) X-FDA: 82228036212.23.AB730D5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf16.hostedemail.com (Postfix) with ESMTP id B55BF18000C for ; Fri, 14 Jun 2024 03:18:23 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=jPQB1kuz; spf=pass (imf16.hostedemail.com: domain of bhe@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bhe@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1718335101; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=zID3RBBtLNCqI825Aa6VeGA/djgVKIddglnLoE0RH84=; b=r8xrPFcAyQGMK1wqwOP1UhPw9jRdEA9Q2or4WilL3NIcXbgYyvtrWqeXJ30PS3bnCYDqCg wvjPLeAXoT8yG7eykP/qJR/ENcvAob0AmI7QZ0MQo8n0fTHqA8qylCLos224+5hRKEfBPu PbuNLHAb3Ice61MF+mbrcpqXdFGi7Uo= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=jPQB1kuz; spf=pass (imf16.hostedemail.com: domain of bhe@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bhe@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718335101; a=rsa-sha256; cv=none; b=bsDHJYI3M1JAL1oD48Bxi4TlgJZyEd7l/T7BpYnMnddmWvTO5IPCKmUvWS2IzqbbAhQxcz 4OntjqOAtrBELMXcaYdmYz5vuNNukoaXdtS2IH9B8q3qffs8+6EC4d2zyDV0nwmVX1P+AS HZnoSmmz94ZaTz3Pozzk7PlFk00QCMU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718335103; h=from:from: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; bh=zID3RBBtLNCqI825Aa6VeGA/djgVKIddglnLoE0RH84=; b=jPQB1kuzXIMahRfXcgzuFPw9SXQJc627vlPDwUm0CXI+o6Ut+Va1RzUVd+JSWxdNR8TofA JMz3Qot599wy0ULHoMMgnyrQlBisLvy7b4b1SOKYFdbBpx+2SsxA+sWpRe18mMc6KOJt7I LRbZciDybnrkAZEKxOlTU0UE2wlvAKI= Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-657-b1U1-brmOTeIaZtQF2mftg-1; Thu, 13 Jun 2024 23:18:18 -0400 X-MC-Unique: b1U1-brmOTeIaZtQF2mftg-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 71D6419560A0; Fri, 14 Jun 2024 03:18:16 +0000 (UTC) Received: from localhost (unknown [10.72.112.37]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 470863000219; Fri, 14 Jun 2024 03:18:13 +0000 (UTC) Date: Fri, 14 Jun 2024 11:18:08 +0800 From: Baoquan He To: "zhaoyang.huang" Cc: Andrew Morton , Uladzislau Rezki , Christoph Hellwig , Lorenzo Stoakes , Thomas Gleixner , hailong liu , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Zhaoyang Huang , steve.kang@unisoc.com, stable@vger.kernel.org Subject: Re: [PATCHv5 1/1] mm: fix incorrect vbq reference in purge_fragmented_block Message-ID: References: <20240614010557.1821327-1-zhaoyang.huang@unisoc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240614010557.1821327-1-zhaoyang.huang@unisoc.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: B55BF18000C X-Stat-Signature: ttnt9mgyjnfjwnybkdqtm5fj6zk7aay4 X-Rspam-User: X-HE-Tag: 1718335103-208411 X-HE-Meta: U2FsdGVkX1/QLj8L7D8r2rNJ/I7bPBEEtJpN+BEaITFweT3ZAdqX7V0aLQFKgveYpOBBtSSAxrD8xmgzbYwmLHVEHHyxeV/NUH9hMEvCkO6YBt6am4IRrPIcJIZNe6+rOX8VI5/PlPW1fIujq9t8HKooitVTNiAuLQKUkO82bxZxhJoE9QDKdORuR3S7s/qNqsZ9v25Y1MogJqKKypzMVl/a0ojoDd2bRQBwcnb4hnAKVNxeCm9mLhLB9/qurY5LLGLa5qmbk+EcbpT+QwMmAUEkG9x9h1BF30Wk7zCSrJzxYQxxL/Oeii84ynd02XkQ3wsfFZPLIwPAS5RSbUM/ULg8ZMMxHZ/ckcyzf8zgy8v4iWC033TF21xakMuD3zdWJ/OxuPeBXu2dY6gnXfuG8Dt9h4Av+1/R5w2eU5GdEswthiWQ9zVeo3dDjvBqnfe0xeKz9OE5MAdLzggg8gFxXqtx2rWW0Gi2KZtVzdrWkeXX5rMnPQj0p6Bg7SDuVyikdd6dDb1O6o/LorjasB/XBiUg+hhof8m4U5VOSTV0x4PVe1Fze5QlB3v6+bVoYw8gvoWPAZYJomtnLNR51JseGSkcTH9oMBrJ8n/XoLQj4jcx/DXbhzeSUdIuQUtNF5k1oaP4JJp5kU1C9is65JZVfwWSDlxw/l5tcVBgS73PMR/X9t2FNO+LnFh7oinohkzGD4pjMy4sVFZme17JuLeQ9AmzNclCLd82xYOvvz1xFimRpgQyCC3BfbGQI1keOaMsO83dFLJxJFI5B/zVQR8JTsmoQ+8JIOTIh+qLUf4ioUGgVncN1h9Y5BCkaGQznZew/qeMh+pRoKxxQBZbXiTk46rpm3s0fzdZT/K3oC5+6KXk7o47Di7k14JhwyGrqn2aKP7UR0E2sXNEOpS5qtI8mRYwcvCgV37r5XD+RVuhE6sFap4LJD8pnBfVomDKx+l5o0wl+PXw+VZjE+EMUe+ h8PY+NCm 65brVqTJEcTTxiNdvS4ZtpxlVaHGhnb1Ro4IUv/gqZFQBRv39eNh9BM//aqH+x3+oFPee8Vp7QVEeCFBNPLduwDJcrLlDkjG8hp+cOyVxygegzZvvidAxOd4nDXIX9LiOXH3n78+NiTgIJ6U+nm73k3vesU0ikhuxcZZTFjYECP99fwon0w5NLw4MtgLaN9Fu2B/R/RyjrZpwoP6IoC5KwpBDaW6DBXjhNmMfYwkk41hNfSecGMEklMFqKl1COtFJu7OdN3bAqXrIWldBMx40nKUWWH877dMOwKnVtAQhJJuzJXInN9faFZ4nGz99xLuU4t1JM+6rrik7VLSpMXEn06fsk1K6rbdLdMqTpD7I3n92z1L+68oOHVXi/GNTejsFZM7oN69kG2BgI0yLr8U4fBh+mQmDeZkgXU8XRN4XJNV8FHcYHiLdasL4qBzmtrJapSWWAb5Gv8RDsS5UPmiSbxV7/sY9ROqVVJpdslmsR36xr88OLv5A3da6kY3XDoYsBPuA9nTFQmLVvPGC443lzVVizxwQf+uTD0qg13QMBytz3RMjTwJYnyfUBa0ahKPfklC/Z4iNyBiX0bI= 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: List-Subscribe: List-Unsubscribe: On 06/14/24 at 09:05am, zhaoyang.huang wrote: > From: Zhaoyang Huang > > The function xa_for_each() in _vm_unmap_aliases() loops through all > vbs. However, since commit 062eacf57ad9 ("mm: vmalloc: remove a global > vmap_blocks xarray") the vb from xarray may not be on the corresponding > CPU vmap_block_queue. Consequently, purge_fragmented_block() might > use the wrong vbq->lock to protect the free list, leading to vbq->free > breakage. > > Incorrect lock protection can exhaust all vmalloc space as follows: > CPU0 CPU1 > +--------------------------------------------+ > | +--------------------+ +-----+ | > +--> | |---->| |------+ > | CPU1:vbq free_list | | vb1 | > +--- | |<----| |<-----+ > | +--------------------+ +-----+ | > +--------------------------------------------+ > > _vm_unmap_aliases() vb_alloc() > new_vmap_block() > xa_for_each(&vbq->vmap_blocks, idx, vb) > --> vb in CPU1:vbq->freelist > > purge_fragmented_block(vb) > spin_lock(&vbq->lock) spin_lock(&vbq->lock) > --> use CPU0:vbq->lock --> use CPU1:vbq->lock > > list_del_rcu(&vb->free_list) list_add_tail_rcu(&vb->free_list, &vbq->free) > __list_del(vb->prev, vb->next) > next->prev = prev > +--------------------+ > | | > | CPU1:vbq free_list | > +---| |<--+ > | +--------------------+ | > +----------------------------+ > __list_add(new, head->prev, head) > +--------------------------------------------+ > | +--------------------+ +-----+ | > +--> | |---->| |------+ > | CPU1:vbq free_list | | vb2 | > +--- | |<----| |<-----+ > | +--------------------+ +-----+ | > +--------------------------------------------+ > > prev->next = next > +--------------------------------------------+ > |----------------------------+ | > | +--------------------+ | +-----+ | > +--> | |--+ | |------+ > | CPU1:vbq free_list | | vb2 | > +--- | |<----| |<-----+ > | +--------------------+ +-----+ | > +--------------------------------------------+ > Here’s a list breakdown. All vbs, which were to be added to > ‘prev’, cannot be used by list_for_each_entry_rcu(vb, &vbq->free, > free_list) in vb_alloc(). Thus, vmalloc space is exhausted. > > This issue affects both erofs and f2fs, the stacktrace is as follows: > erofs: > [] __switch_to+0x174 > [] __schedule+0x624 > [] schedule+0x7c > [] schedule_preempt_disabled+0x24 > [] __mutex_lock+0x374 > [] __mutex_lock_slowpath+0x14 > [] mutex_lock+0x24 > [] reclaim_and_purge_vmap_areas+0x44 > [] alloc_vmap_area+0x2e0 > [] vm_map_ram+0x1b0 > [] z_erofs_lz4_decompress+0x278 > [] z_erofs_decompress_queue+0x650 > [] z_erofs_runqueue+0x7f4 > [] z_erofs_read_folio+0x104 > [] filemap_read_folio+0x6c > [] filemap_fault+0x300 > [] __do_fault+0xc8 > [] handle_mm_fault+0xb38 > [] do_page_fault+0x288 > [] do_translation_fault[jt]+0x40 > [] do_mem_abort+0x58 > [] el0_ia+0x70 > [] el0t_64_sync_handler[jt]+0xb0 > [] ret_to_user[jt]+0x0 > > f2fs: > [] __switch_to+0x174 > [] __schedule+0x624 > [] schedule+0x7c > [] schedule_preempt_disabled+0x24 > [] __mutex_lock+0x374 > [] __mutex_lock_slowpath+0x14 > [] mutex_lock+0x24 > [] reclaim_and_purge_vmap_areas+0x44 > [] alloc_vmap_area+0x2e0 > [] vm_map_ram+0x1b0 > [] f2fs_prepare_decomp_mem+0x144 > [] f2fs_alloc_dic+0x264 > [] f2fs_read_multi_pages+0x428 > [] f2fs_mpage_readpages+0x314 > [] f2fs_readahead+0x50 > [] read_pages+0x80 > [] page_cache_ra_unbounded+0x1a0 > [] page_cache_ra_order+0x274 > [] do_sync_mmap_readahead+0x11c > [] filemap_fault+0x1a0 > [] f2fs_filemap_fault+0x28 > [] __do_fault+0xc8 > [] handle_mm_fault+0xb38 > [] do_page_fault+0x288 > [] do_translation_fault[jt]+0x40 > [] do_mem_abort+0x58 > [] el0_ia+0x70 > [] el0t_64_sync_handler[jt]+0xb0 > [] ret_to_user[jt]+0x0 > > To fix this, replace xa_for_each() with list_for_each_entry_rcu() > which reverts commit fc1e0d980037 ("mm/vmalloc: prevent stale TLBs > in fully utilized blocks") ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This paragraph need be updated? > > Fixes: fc1e0d980037 ("mm/vmalloc: prevent stale TLBs in fully utilized blocks") > > Cc: stable@vger.kernel.org > Suggested-by: Hailong.Liu > Signed-off-by: Zhaoyang Huang > --- > v2: introduce cpu in vmap_block to record the right CPU number > v3: use get_cpu/put_cpu to prevent schedule between core > v4: replace get_cpu/put_cpu by another API to avoid disabling preemption > v5: update the commit message by Hailong.Liu > --- > --- > mm/vmalloc.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 22aa63f4ef63..89eb034f4ac6 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -2458,6 +2458,7 @@ struct vmap_block { > struct list_head free_list; > struct rcu_head rcu_head; > struct list_head purge; > + unsigned int cpu; > }; > > /* Queue of free and dirty vmap blocks, for allocation and flushing purposes */ > @@ -2585,8 +2586,15 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask) > free_vmap_area(va); > return ERR_PTR(err); > } > - > - vbq = raw_cpu_ptr(&vmap_block_queue); > + /* > + * list_add_tail_rcu could happened in another core > + * rather than vb->cpu due to task migration, which > + * is safe as list_add_tail_rcu will ensure the list's > + * integrity together with list_for_each_rcu from read > + * side. > + */ Do we still need above code comment? No lock gurantees vb->cpu is from the cpu where vb_alloc() is called, it could be from the cpu where migration moved to. We don't care about it as long as the vbq->lock correctly protect the vb on its vbq->free? > + vb->cpu = raw_smp_processor_id(); > + vbq = per_cpu_ptr(&vmap_block_queue, vb->cpu); > spin_lock(&vbq->lock); > list_add_tail_rcu(&vb->free_list, &vbq->free); > spin_unlock(&vbq->lock); > @@ -2614,9 +2622,10 @@ static void free_vmap_block(struct vmap_block *vb) > } > > static bool purge_fragmented_block(struct vmap_block *vb, > - struct vmap_block_queue *vbq, struct list_head *purge_list, > - bool force_purge) > + struct list_head *purge_list, bool force_purge) > { > + struct vmap_block_queue *vbq = &per_cpu(vmap_block_queue, vb->cpu); > + > if (vb->free + vb->dirty != VMAP_BBMAP_BITS || > vb->dirty == VMAP_BBMAP_BITS) > return false; > @@ -2664,7 +2673,7 @@ static void purge_fragmented_blocks(int cpu) > continue; > > spin_lock(&vb->lock); > - purge_fragmented_block(vb, vbq, &purge, true); > + purge_fragmented_block(vb, &purge, true); > spin_unlock(&vb->lock); > } > rcu_read_unlock(); > @@ -2801,7 +2810,7 @@ static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush) > * not purgeable, check whether there is dirty > * space to be flushed. > */ > - if (!purge_fragmented_block(vb, vbq, &purge_list, false) && > + if (!purge_fragmented_block(vb, &purge_list, false) && > vb->dirty_max && vb->dirty != VMAP_BBMAP_BITS) { > unsigned long va_start = vb->va->va_start; > unsigned long s, e; > -- > 2.25.1 > >