* [bug report] mm: convert ksm_might_need_to_copy() to work on folios
@ 2023-12-13 7:01 Dan Carpenter
2023-12-13 15:37 ` Matthew Wilcox
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2023-12-13 7:01 UTC (permalink / raw)
To: willy; +Cc: linux-mm
Hello Matthew Wilcox (Oracle),
The patch 6faef19bdfcc: "mm: convert ksm_might_need_to_copy() to work
on folios" from Dec 11, 2023 (linux-next), leads to the following
Smatch static checker warning:
mm/memory.c:4118 do_swap_page()
error: 'folio' dereferencing possible ERR_PTR()
mm/memory.c
3940 /*
3941 * KSM sometimes has to copy on read faults, for example, if
3942 * page->index of !PageKSM() pages would be nonlinear inside the
3943 * anon VMA -- PageKSM() is lost on actual swapout.
3944 */
3945 folio = ksm_might_need_to_copy(folio, vma, vmf->address);
3946 if (unlikely(!folio)) {
3947 ret = VM_FAULT_OOM;
3948 goto out_page;
3949 } else if (unlikely(folio == ERR_PTR(-EHWPOISON))) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This used to be page instead of folio
3950 ret = VM_FAULT_HWPOISON;
3951 goto out_page;
This goto will crash
3952 }
3953 page = folio_page(folio, 0);
3954
3955 /*
3956 * If we want to map a page that's in the swapcache writable, we
3957 * have to detect via the refcount if we're really the exclusive
3958 * owner. Try removing the extra reference from the local LRU
3959 * caches if required.
3960 */
3961 if ((vmf->flags & FAULT_FLAG_WRITE) && folio == swapcache &&
3962 !folio_test_ksm(folio) && !folio_test_lru(folio))
3963 lru_add_drain();
3964 }
3965
3966 folio_throttle_swaprate(folio, GFP_KERNEL);
3967
3968 /*
3969 * Back out if somebody else already faulted in this pte.
3970 */
3971 vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
3972 &vmf->ptl);
3973 if (unlikely(!vmf->pte || !pte_same(ptep_get(vmf->pte), vmf->orig_pte)))
3974 goto out_nomap;
3975
3976 if (unlikely(!folio_test_uptodate(folio))) {
3977 ret = VM_FAULT_SIGBUS;
3978 goto out_nomap;
3979 }
3980
3981 /*
3982 * PG_anon_exclusive reuses PG_mappedtodisk for anon pages. A swap pte
3983 * must never point at an anonymous page in the swapcache that is
3984 * PG_anon_exclusive. Sanity check that this holds and especially, that
3985 * no filesystem set PG_mappedtodisk on a page in the swapcache. Sanity
3986 * check after taking the PT lock and making sure that nobody
3987 * concurrently faulted in this page and set PG_anon_exclusive.
3988 */
3989 BUG_ON(!folio_test_anon(folio) && folio_test_mappedtodisk(folio));
3990 BUG_ON(folio_test_anon(folio) && PageAnonExclusive(page));
3991
3992 /*
3993 * Check under PT lock (to protect against concurrent fork() sharing
3994 * the swap entry concurrently) for certainly exclusive pages.
3995 */
3996 if (!folio_test_ksm(folio)) {
3997 exclusive = pte_swp_exclusive(vmf->orig_pte);
3998 if (folio != swapcache) {
3999 /*
4000 * We have a fresh page that is not exposed to the
4001 * swapcache -> certainly exclusive.
4002 */
4003 exclusive = true;
4004 } else if (exclusive && folio_test_writeback(folio) &&
4005 data_race(si->flags & SWP_STABLE_WRITES)) {
4006 /*
4007 * This is tricky: not all swap backends support
4008 * concurrent page modifications while under writeback.
4009 *
4010 * So if we stumble over such a page in the swapcache
4011 * we must not set the page exclusive, otherwise we can
4012 * map it writable without further checks and modify it
4013 * while still under writeback.
4014 *
4015 * For these problematic swap backends, simply drop the
4016 * exclusive marker: this is perfectly fine as we start
4017 * writeback only if we fully unmapped the page and
4018 * there are no unexpected references on the page after
4019 * unmapping succeeded. After fully unmapped, no
4020 * further GUP references (FOLL_GET and FOLL_PIN) can
4021 * appear, so dropping the exclusive marker and mapping
4022 * it only R/O is fine.
4023 */
4024 exclusive = false;
4025 }
4026 }
4027
4028 /*
4029 * Some architectures may have to restore extra metadata to the page
4030 * when reading from swap. This metadata may be indexed by swap entry
4031 * so this must be called before swap_free().
4032 */
4033 arch_swap_restore(entry, folio);
4034
4035 /*
4036 * Remove the swap entry and conditionally try to free up the swapcache.
4037 * We're already holding a reference on the page but haven't mapped it
4038 * yet.
4039 */
4040 swap_free(entry);
4041 if (should_try_to_free_swap(folio, vma, vmf->flags))
4042 folio_free_swap(folio);
4043
4044 inc_mm_counter(vma->vm_mm, MM_ANONPAGES);
4045 dec_mm_counter(vma->vm_mm, MM_SWAPENTS);
4046 pte = mk_pte(page, vma->vm_page_prot);
4047
4048 /*
4049 * Same logic as in do_wp_page(); however, optimize for pages that are
4050 * certainly not shared either because we just allocated them without
4051 * exposing them to the swapcache or because the swap entry indicates
4052 * exclusivity.
4053 */
4054 if (!folio_test_ksm(folio) &&
4055 (exclusive || folio_ref_count(folio) == 1)) {
4056 if (vmf->flags & FAULT_FLAG_WRITE) {
4057 pte = maybe_mkwrite(pte_mkdirty(pte), vma);
4058 vmf->flags &= ~FAULT_FLAG_WRITE;
4059 }
4060 rmap_flags |= RMAP_EXCLUSIVE;
4061 }
4062 flush_icache_page(vma, page);
4063 if (pte_swp_soft_dirty(vmf->orig_pte))
4064 pte = pte_mksoft_dirty(pte);
4065 if (pte_swp_uffd_wp(vmf->orig_pte))
4066 pte = pte_mkuffd_wp(pte);
4067 vmf->orig_pte = pte;
4068
4069 /* ksm created a completely new copy */
4070 if (unlikely(folio != swapcache && swapcache)) {
4071 folio_add_new_anon_rmap(folio, vma, vmf->address);
4072 folio_add_lru_vma(folio, vma);
4073 } else {
4074 page_add_anon_rmap(page, vma, vmf->address, rmap_flags);
4075 }
4076
4077 VM_BUG_ON(!folio_test_anon(folio) ||
4078 (pte_write(pte) && !PageAnonExclusive(page)));
4079 set_pte_at(vma->vm_mm, vmf->address, vmf->pte, pte);
4080 arch_do_swap_page(vma->vm_mm, vma, vmf->address, pte, vmf->orig_pte);
4081
4082 folio_unlock(folio);
4083 if (folio != swapcache && swapcache) {
4084 /*
4085 * Hold the lock to avoid the swap entry to be reused
4086 * until we take the PT lock for the pte_same() check
4087 * (to avoid false positives from pte_same). For
4088 * further safety release the lock after the swap_free
4089 * so that the swap count won't change under a
4090 * parallel locked swapcache.
4091 */
4092 folio_unlock(swapcache);
4093 folio_put(swapcache);
4094 }
4095
4096 if (vmf->flags & FAULT_FLAG_WRITE) {
4097 ret |= do_wp_page(vmf);
4098 if (ret & VM_FAULT_ERROR)
4099 ret &= VM_FAULT_ERROR;
4100 goto out;
4101 }
4102
4103 /* No need to invalidate - it was non-present before */
4104 update_mmu_cache_range(vmf, vma, vmf->address, vmf->pte, 1);
4105 unlock:
4106 if (vmf->pte)
4107 pte_unmap_unlock(vmf->pte, vmf->ptl);
4108 out:
4109 if (si)
4110 put_swap_device(si);
4111 return ret;
4112 out_nomap:
4113 if (vmf->pte)
4114 pte_unmap_unlock(vmf->pte, vmf->ptl);
4115 out_page:
4116 folio_unlock(folio);
4117 out_release:
--> 4118 folio_put(folio);
^^^^^^^^^^^^^^^^
4119 if (folio != swapcache && swapcache) {
4120 folio_unlock(swapcache);
4121 folio_put(swapcache);
4122 }
4123 if (si)
4124 put_swap_device(si);
4125 return ret;
4126 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [bug report] mm: convert ksm_might_need_to_copy() to work on folios
2023-12-13 7:01 [bug report] mm: convert ksm_might_need_to_copy() to work on folios Dan Carpenter
@ 2023-12-13 15:37 ` Matthew Wilcox
0 siblings, 0 replies; 2+ messages in thread
From: Matthew Wilcox @ 2023-12-13 15:37 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-mm
On Wed, Dec 13, 2023 at 10:01:46AM +0300, Dan Carpenter wrote:
> Hello Matthew Wilcox (Oracle),
>
> The patch 6faef19bdfcc: "mm: convert ksm_might_need_to_copy() to work
> on folios" from Dec 11, 2023 (linux-next), leads to the following
> Smatch static checker warning:
>
> mm/memory.c:4118 do_swap_page()
> error: 'folio' dereferencing possible ERR_PTR()
Right! Andrew, please squash in:
diff --git a/mm/memory.c b/mm/memory.c
index 318f923134e4..1270d70dcb80 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3945,9 +3945,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
folio = ksm_might_need_to_copy(folio, vma, vmf->address);
if (unlikely(!folio)) {
ret = VM_FAULT_OOM;
+ folio = swapcache;
goto out_page;
} else if (unlikely(folio == ERR_PTR(-EHWPOISON))) {
ret = VM_FAULT_HWPOISON;
+ folio = swapcache;
goto out_page;
}
page = folio_page(folio, 0);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-12-13 15:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-13 7:01 [bug report] mm: convert ksm_might_need_to_copy() to work on folios Dan Carpenter
2023-12-13 15:37 ` Matthew Wilcox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox