On Tue, Mar 14, 2023 at 11:02:40AM -0700, Linus Torvalds wrote:
> On Tue, Mar 14, 2023 at 9:43 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> >
> > The problem is that we might have swapped out the shmem folio. So we
> > don't want to clear the page, but ask swap to fill the page.
>
> Doesn't shmem_swapin_folio() already basically do all that work?
>
> The real oddity with shmem - compared to other filesystems - is that
> the xarray has a value entry instead of being a real folio. And yes,
> the current filemap code will then just ignore such entries as
> "doesn't exist", and so the regular read iterators will all fail on
> it.
>
> But while filemap_get_read_batch() will stop at a value-folio, I feel
> like filemap_create_folio() should be able to turn a value page into a
> "real" page. Right now it already allocates said page, but then I
> think filemap_add_folio() will return -EEXIST when said entry exists
> as a value.
>
> But *if* instead of -EEXIST we could just replace the value with the
> (already locked) page, and have some sane way to pass that value
> (which is the swap entry data) to readpage(), I think that should just
> do it all.
This was basically what I had in mind:
I don't think this will handle a case like:
Alloc order-0 folio at index 4
Alloc order-0 folio at index 7
Swap out both folios
Alloc order-9 folio at indices 0-511
But I don't see where shmem currently handles that either. Maybe it
falls back to order-0 folios instead of the crude BUG_ON I put in.
Hugh?
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index 82c1262f396f..30f2502314de 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -114,12 +114,6 @@ int shmem_get_folio(struct inode *inode, pgoff_t index, struct folio **foliop,
struct folio *shmem_read_folio_gfp(struct address_space *mapping,
pgoff_t index, gfp_t gfp);
-static inline struct folio *shmem_read_folio(struct address_space *mapping,
- pgoff_t index)
-{
- return shmem_read_folio_gfp(mapping, index, mapping_gfp_mask(mapping));
-}
-
static inline struct page *shmem_read_mapping_page(
struct address_space *mapping, pgoff_t index)
{
diff --git a/mm/filemap.c b/mm/filemap.c
index 57c1b154fb5a..8e4f95c5b65a 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -877,6 +877,8 @@ noinline int __filemap_add_folio(struct address_space *mapping,
order, gfp);
xas_lock_irq(&xas);
xas_for_each_conflict(&xas, entry) {
+ if (old)
+ BUG_ON(shmem_mapping(mapping));
old = entry;
if (!xa_is_value(entry)) {
xas_set_err(&xas, -EEXIST);
@@ -885,7 +887,12 @@ noinline int __filemap_add_folio(struct address_space *mapping,
}
if (old) {
- if (shadowp)
+ if (shmem_mapping(mapping)) {
+ folio_set_swap_entry(folio,
+ radix_to_swp_entry(old));
+ folio_set_swapcache(folio);
+ folio_set_swapbacked(folio);
+ } else if (shadowp)
*shadowp = old;
/* entry may have been split before we acquired lock */
order = xa_get_order(xas.xa, xas.xa_index);
diff --git a/mm/shmem.c b/mm/shmem.c
index 8e60826e4246..ea75c7dcf5ec 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2059,6 +2059,18 @@ int shmem_get_folio(struct inode *inode, pgoff_t index, struct folio **foliop,
mapping_gfp_mask(inode->i_mapping), NULL, NULL, NULL);
}
+static int shmem_read_folio(struct file *file, struct folio *folio)
+{
+ if (folio_test_swapcache(folio)) {
+ swap_readpage(&folio->page, true, NULL);
+ } else {
+ folio_zero_segment(folio, 0, folio_size(folio));
+ folio_mark_uptodate(folio);
+ folio_unlock(folio);
+ }
+ return 0;
+}
+
/*
* This is like autoremove_wake_function, but it removes the wait queue
* entry unconditionally - even if something else had already woken the
@@ -2396,7 +2408,8 @@ static int shmem_fadvise_willneed(struct address_space *mapping,
xa_for_each_range(&mapping->i_pages, index, folio, start, end) {
if (!xa_is_value(folio))
continue;
- folio = shmem_read_folio(mapping, index);
+ folio = shmem_read_folio_gfp(mapping, index,
+ mapping_gfp_mask(mapping));
if (!IS_ERR(folio))
folio_put(folio);
}
@@ -4037,6 +4050,7 @@ static int shmem_error_remove_page(struct address_space *mapping,
}
const struct address_space_operations shmem_aops = {
+ .read_folio = shmem_read_folio,
.writepage = shmem_writepage,
.dirty_folio = noop_dirty_folio,
#ifdef CONFIG_TMPFS
