[PATCH RESEND V5,0/2]mm: shmem: support POSIX_FADV_[WILL|DONT]NEED for shmem files

[PATCH RESEND V5,0/2]mm: shmem: support POSIX_FADV_[WILL|DONT]NEED for shmem files

[Charan Teja Kalla]

From: Charan Teja Reddy <quic_charante@quicinc.com>

This patch aims to implement POSIX_FADV_WILLNEED and POSIX_FADV_DONTNEED
advices to shmem files which can be helpful for the drivers who may want
to manage the pages of shmem files on their own, like, that are created
through shmem_file_setup[_with_mnt]().

Changes in V5:
 -- Moved the 'endbyte' calculations to a header function for use by shmem_fadvise().
 -- Addressed comments from suren.
 -- No changes in resend. Retested on the latest tip.

Changes in V4:
  -- Changed the code to use reclaim_pages() to writeout the shmem pages to swap and then reclaim.
  -- Addressed comments from Mark Hemment and Matthew.
  -- fadvise() on shmem file may even unmap a page.
  -- https://patchwork.kernel.org/project/linux-mm/patch/1644572051-24091-1-git-send-email-quic_charante@quicinc.com/

Changes in V3:
  -- Considered THP pages while doing FADVISE_[DONT|WILL]NEED, identified by Matthew.
  -- xarray used properly, as identified by Matthew.
  -- Excluded mapped pages as it requires unmapping and the man pages of fadvise don't talk about them.
  -- RESEND: Fixed the compilation issue when CONFIG_TMPFS is not defined.
  -- https://patchwork.kernel.org/project/linux-mm/patch/1641488717-13865-1-git-send-email-quic_charante@quicinc.com/

Changes in V2:
  -- Rearranged the code to not to sleep with rcu_lock while using xas_() functionality.
  -- Addressed the comments from Suren.
  -- https://patchwork.kernel.org/project/linux-mm/patch/1638442253-1591-1-git-send-email-quic_charante@quicinc.com/

changes in V1:
  -- Created the interface for fadvise(2) to work on shmem files.
  -- https://patchwork.kernel.org/project/linux-mm/patch/1633701982-22302-1-git-send-email-charante@codeaurora.org/

Charan Teja Reddy (2):
  mm: fadvise: move 'endbyte' calculations to helper function
  mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem

 mm/fadvise.c  |  11 +-----
 mm/internal.h |  21 ++++++++++
 mm/shmem.c    | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH RESEND V5,1/2] mm: fadvise: move 'endbyte' calculations to helper function

[Charan Teja Kalla]

From: Charan Teja Reddy <quic_charante@quicinc.com>

Move the 'endbyte' calculations that determines last byte that fadvise
can to a helper function. This is a preparatory change made for
shmem_fadvise() functionality in the next patch. No functional changes
in this patch.

Signed-off-by: Charan Teja Reddy <quic_charante@quicinc.com>
---
Changes in V5:
 -- Moved the 'endbyte' calculation to a helper function.
 -- This patch is newly raised in V5 thus no change exists from v1 to v4.

 mm/fadvise.c  | 11 +----------
 mm/internal.h | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/mm/fadvise.c b/mm/fadvise.c
index 338f160..0c82be2 100644
--- a/mm/fadvise.c
+++ b/mm/fadvise.c
@@ -65,16 +65,7 @@ int generic_fadvise(struct file *file, loff_t offset, loff_t len, int advice)
 		return 0;
 	}
 
-	/*
-	 * Careful about overflows. Len == 0 means "as much as possible".  Use
-	 * unsigned math because signed overflows are undefined and UBSan
-	 * complains.
-	 */
-	endbyte = (u64)offset + (u64)len;
-	if (!len || endbyte < len)
-		endbyte = -1;
-	else
-		endbyte--;		/* inclusive */
+	endbyte = fadvise_calc_endbyte(offset, len);
 
 	switch (advice) {
 	case POSIX_FADV_NORMAL:
diff --git a/mm/internal.h b/mm/internal.h
index 58dc6ad..b02f07e 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -546,6 +546,27 @@ static inline void vunmap_range_noflush(unsigned long start, unsigned long end)
 #endif /* !CONFIG_MMU */
 
 /*
+ * Helper function to get the endbyte of a file that fadvise can operate on.
+ */
+static inline loff_t fadvise_calc_endbyte(loff_t offset, loff_t len)
+{
+	loff_t endbyte;
+
+	/*
+	 * Careful about overflows. Len == 0 means "as much as possible".  Use
+	 * unsigned math because signed overflows are undefined and UBSan
+	 * complains.
+	 */
+	endbyte = (u64)offset + (u64)len;
+	if (!len || endbyte < len)
+		endbyte = -1;
+	else
+		endbyte--;		/* inclusive */
+
+	return endbyte;
+}
+
+/*
  * Return the mem_map entry representing the 'offset' subpage within
  * the maximally aligned gigantic page 'base'.  Handle any discontiguity
  * in the mem_map at MAX_ORDER_NR_PAGES boundaries.
-- 
2.7.4

[PATCH RESEND V5,2/2] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem

[Charan Teja Kalla]

From: Charan Teja Reddy <quic_charante@quicinc.com>

Currently fadvise(2) is supported only for the files that doesn't
associated with noop_backing_dev_info thus for the files, like shmem,
fadvise results into NOP. But then there is file_operations->fadvise()
that lets the file systems to implement their own fadvise
implementation. Use this support to implement some of the POSIX_FADV_XXX
functionality for shmem files.

This patch aims to implement POSIX_FADV_WILLNEED and POSIX_FADV_DONTNEED
advices to shmem files which can be helpful for the drivers who may want
to manage the shmem pages of the files that are created through
shmem_file_setup[_with_mnt]().  An example usecase may be like, driver
can create the shmem file of the size equal to its requirements and
map the pages for DMA and then pass the fd to user. The user who knows
well about the usage of these pages can now decide when these pages are
not required push them to swap through DONTNEED thus free up memory well
in advance rather than relying on the reclaim and use WILLNEED when it
decide that they are useful in the near future. IOW, it lets the clients
to free up/read the memory when it wants to. Another usecase is that GEM
objects which are currently allocated and managed through shmem files
can use vfs_fadvise(DONT|WILLNEED) on shmem fd when the driver comes to
know(like through some hints from user space) that GEM objects are not
going to use/will need in the near future.

Some questions asked while reviewing this patch:

Q) Can the same thing be achieved with FD mapped to user and use
madvise?
A) All drivers are not mapping all the shmem fd's to user space and want
to manage them with in the kernel. Ex: shmem memory can be mapped to the
other subsystems and they fill in the data and then give it to other
subsystem for further processing, where, the user mapping is not at all
required.  A simple example, memory that is given for gpu subsystem
which can be filled directly and give to display subsystem. And the
respective drivers know well about when to keep that memory in ram or
swap based on may be a user activity.

Q) Should we add the documentation section in Manual pages?
A) The man[1] pages for the fadvise() whatever says is also applicable
for shmem files. so couldn't feel it correct to add specific to shmem
files separately.
[1] https://linux.die.net/man/2/fadvise

Q) The proposed semantics of POSIX_FADV_DONTNEED is actually similar to
MADV_PAGEOUT and different from MADV_DONTNEED. This is a user facing API
and this difference will cause confusion?
A) man pages [1] says that "POSIX_FADV_DONTNEED attempts to free cached
pages associated with the specified region." This means on issuing this
FADV, it is expected to free the file cache pages. And it is
implementation defined If the dirty pages may be attempted to writeback.
And the unwritten dirty pages will not be freed. So, FADV_DONTNEED also
covers the semantics of MADV_PAGEOUT for file pages and there is no
purpose of PAGEOUT for file pages.
[1] https://man7.org/linux/man-pages/man2/posix_fadvise.2.html

Signed-off-by: Charan Teja Reddy <quic_charante@quicinc.com>
---
Changes in V5:
  -- Moved the 'endbyte' calculations to a helper function which is also needed for shmem_fadvise.
  -- Addressed comments from Suren.

Changes in V4:
  -- Changed the code to use reclaim_pages() to writeout the shmem pages to swap and then reclaim.
  -- Addressed comments from Mark Hemment and Matthew.
  -- fadvise() on shmem file may even unmap a page.
  -- https://patchwork.kernel.org/project/linux-mm/patch/1644572051-24091-1-git-send-email-quic_charante@quicinc.com/

Changes in V3:
  -- Considered THP pages while doing FADVISE_[DONT|WILL]NEED, identified by Matthew.
  -- xarray used properly, as identified by Matthew.
  -- Excluded mapped pages as it requires unmapping and the man pages of fadvise don't talk about them.
  -- RESEND: Fixed the compilation issue when CONFIG_TMPFS is not defined.
  -- https://patchwork.kernel.org/project/linux-mm/patch/1641488717-13865-1-git-send-email-quic_charante@quicinc.com/

Changes in V2:
  -- Rearranged the code to not to sleep with rcu_lock while using xas_() functionality.
  -- Addressed the comments from Suren.
  -- https://patchwork.kernel.org/project/linux-mm/patch/1638442253-1591-1-git-send-email-quic_charante@quicinc.com/

changes in V1:
  -- Created the interface for fadvise(2) to work on shmem files.
  -- https://patchwork.kernel.org/project/linux-mm/patch/1633701982-22302-1-git-send-email-charante@codeaurora.org/


 mm/shmem.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index 529c9ad..3be800c 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -38,6 +38,9 @@
 #include <linux/hugetlb.h>
 #include <linux/fs_parser.h>
 #include <linux/swapfile.h>
+#include <linux/mm_inline.h>
+#include <linux/fadvise.h>
+#include <linux/page_idle.h>
 
 static struct vfsmount *shm_mnt;
 
@@ -2247,6 +2250,125 @@ static int shmem_mmap(struct file *file, struct vm_area_struct *vma)
 	return 0;
 }
 
+static void shmem_isolate_pages_range(struct address_space *mapping, loff_t start,
+				loff_t end, struct list_head *list)
+{
+	XA_STATE(xas, &mapping->i_pages, start);
+	struct page *page;
+
+	rcu_read_lock();
+	xas_for_each(&xas, page, end) {
+		if (xas_retry(&xas, page))
+			continue;
+		if (xa_is_value(page))
+			continue;
+
+		if (!get_page_unless_zero(page))
+			continue;
+		if (isolate_lru_page(page)) {
+			put_page(page);
+			continue;
+		}
+		put_page(page);
+
+		if (PageUnevictable(page) || page_mapcount(page) > 1) {
+			putback_lru_page(page);
+			continue;
+		}
+
+		/*
+		 * Prepare the page to be passed to the reclaim_pages().
+		 * VM couldn't reclaim the page unless we clear PG_young.
+		 * Also, to ensure that the pages are written before
+		 * reclaiming, page is set to dirty.
+		 * Since we are not clearing the pte_young in the mapped
+		 * page pte's, its reclaim may not be attempted.
+		 */
+		ClearPageReferenced(page);
+		test_and_clear_page_young(page);
+		list_add(&page->lru, list);
+		if (need_resched()) {
+			xas_pause(&xas);
+			cond_resched_rcu();
+		}
+	}
+	rcu_read_unlock();
+}
+
+static int shmem_fadvise_dontneed(struct address_space *mapping, loff_t start,
+				loff_t end)
+{
+	LIST_HEAD(list);
+
+	if (!shmem_mapping(mapping))
+		return -EINVAL;
+
+	if (!total_swap_pages)
+		return 0;
+
+	lru_add_drain();
+	shmem_isolate_pages_range(mapping, start, end, &list);
+	reclaim_pages(&list);
+
+	return 0;
+}
+
+static int shmem_fadvise_willneed(struct address_space *mapping,
+				 pgoff_t start, pgoff_t long end)
+{
+	struct page *page;
+	pgoff_t index;
+
+	xa_for_each_range(&mapping->i_pages, index, page, start, end) {
+		if (!xa_is_value(page))
+			continue;
+		page = shmem_read_mapping_page(mapping, index);
+		if (!IS_ERR(page))
+			put_page(page);
+	}
+
+	return 0;
+}
+
+static int shmem_fadvise(struct file *file, loff_t offset, loff_t len, int advice)
+{
+	loff_t endbyte;
+	pgoff_t start_index;
+	pgoff_t end_index;
+	struct address_space *mapping;
+	int ret = 0;
+
+	mapping = file->f_mapping;
+	if (!mapping || len < 0)
+		return -EINVAL;
+
+	endbyte = fadvise_calc_endbyte(offset, len);
+
+	start_index = offset >> PAGE_SHIFT;
+	end_index   = endbyte >> PAGE_SHIFT;
+	switch (advice) {
+	case POSIX_FADV_DONTNEED:
+		ret = shmem_fadvise_dontneed(mapping, start_index, end_index);
+		break;
+	case POSIX_FADV_WILLNEED:
+		ret = shmem_fadvise_willneed(mapping, start_index, end_index);
+		break;
+	case POSIX_FADV_NORMAL:
+	case POSIX_FADV_RANDOM:
+	case POSIX_FADV_SEQUENTIAL:
+	case POSIX_FADV_NOREUSE:
+		/*
+		 * No bad return value, but ignore advice. May have to
+		 * implement in future.
+		 */
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
 static struct inode *shmem_get_inode(struct super_block *sb, const struct inode *dir,
 				     umode_t mode, dev_t dev, unsigned long flags)
 {
@@ -3780,6 +3902,7 @@ static const struct file_operations shmem_file_operations = {
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= shmem_fallocate,
 #endif
+	.fadvise	= shmem_fadvise,
 };
 
 static const struct inode_operations shmem_inode_operations = {
-- 
2.7.4

Re: [PATCH RESEND V5,2/2] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem

[Andrew Morton]

On Thu, 31 Mar 2022 12:08:21 +0530 Charan Teja Kalla <quic_charante@quicinc.com> wrote:

> From: Charan Teja Reddy <quic_charante@quicinc.com>
> 
> Currently fadvise(2) is supported only for the files that doesn't
> associated with noop_backing_dev_info thus for the files, like shmem,
> fadvise results into NOP. But then there is file_operations->fadvise()
> that lets the file systems to implement their own fadvise
> implementation. Use this support to implement some of the POSIX_FADV_XXX
> functionality for shmem files.
> 
> This patch aims to implement POSIX_FADV_WILLNEED and POSIX_FADV_DONTNEED
> advices to shmem files which can be helpful for the drivers who may want
> to manage the shmem pages of the files that are created through
> shmem_file_setup[_with_mnt]().  An example usecase may be like, driver
> can create the shmem file of the size equal to its requirements and
> map the pages for DMA and then pass the fd to user. The user who knows
> well about the usage of these pages can now decide when these pages are
> not required push them to swap through DONTNEED thus free up memory well
> in advance rather than relying on the reclaim and use WILLNEED when it
> decide that they are useful in the near future. IOW, it lets the clients
> to free up/read the memory when it wants to.

Is there an actual userspace/driver combination which will use this? 
Has the new feature been tested in such an arrangement?  And if so,
which driver(s)?

> Another usecase is that GEM
> objects which are currently allocated and managed through shmem files
> can use vfs_fadvise(DONT|WILLNEED) on shmem fd when the driver comes to
> know(like through some hints from user space) that GEM objects are not
> going to use/will need in the near future.

Again, is this just a theoretical bright idea, or can we be assured
that adding this code to the kernel will end up having been useful to
our users?

> Some questions asked while reviewing this patch:
> 
> Q) Can the same thing be achieved with FD mapped to user and use
> madvise?
> A) All drivers are not mapping all the shmem fd's to user space and want
> to manage them with in the kernel. Ex: shmem memory can be mapped to the
> other subsystems and they fill in the data and then give it to other
> subsystem for further processing, where, the user mapping is not at all
> required.  A simple example, memory that is given for gpu subsystem
> which can be filled directly and give to display subsystem. And the
> respective drivers know well about when to keep that memory in ram or
> swap based on may be a user activity.
> 
> Q) Should we add the documentation section in Manual pages?
> A) The man[1] pages for the fadvise() whatever says is also applicable
> for shmem files. so couldn't feel it correct to add specific to shmem
> files separately.
> [1] https://linux.die.net/man/2/fadvise
> 
> Q) The proposed semantics of POSIX_FADV_DONTNEED is actually similar to
> MADV_PAGEOUT and different from MADV_DONTNEED. This is a user facing API
> and this difference will cause confusion?
> A) man pages [1] says that "POSIX_FADV_DONTNEED attempts to free cached
> pages associated with the specified region." This means on issuing this
> FADV, it is expected to free the file cache pages. And it is
> implementation defined If the dirty pages may be attempted to writeback.
> And the unwritten dirty pages will not be freed. So, FADV_DONTNEED also
> covers the semantics of MADV_PAGEOUT for file pages and there is no
> purpose of PAGEOUT for file pages.

Re: [PATCH RESEND V5,2/2] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem

[Charan Teja Kalla]

Thanks Andrew for your review!!

Sorry for the delayed reply here as I was on vacation.

On 6/1/2022 2:51 AM, Andrew Morton wrote:
> On Thu, 31 Mar 2022 12:08:21 +0530 Charan Teja Kalla <quic_charante@quicinc.com> wrote:
> 
>> From: Charan Teja Reddy <quic_charante@quicinc.com>
>>
>> Currently fadvise(2) is supported only for the files that doesn't
>> associated with noop_backing_dev_info thus for the files, like shmem,
>> fadvise results into NOP. But then there is file_operations->fadvise()
>> that lets the file systems to implement their own fadvise
>> implementation. Use this support to implement some of the POSIX_FADV_XXX
>> functionality for shmem files.
>>
>> This patch aims to implement POSIX_FADV_WILLNEED and POSIX_FADV_DONTNEED
>> advices to shmem files which can be helpful for the drivers who may want
>> to manage the shmem pages of the files that are created through
>> shmem_file_setup[_with_mnt]().  An example usecase may be like, driver
>> can create the shmem file of the size equal to its requirements and
>> map the pages for DMA and then pass the fd to user. The user who knows
>> well about the usage of these pages can now decide when these pages are
>> not required push them to swap through DONTNEED thus free up memory well
>> in advance rather than relying on the reclaim and use WILLNEED when it
>> decide that they are useful in the near future. IOW, it lets the clients
>> to free up/read the memory when it wants to.
> 
> Is there an actual userspace/driver combination which will use this? 
> Has the new feature been tested in such an arrangement?  And if so,
> which driver(s)?
> 

Currently my organization is using this setup where it does makes use of
the shmem infrastructure to allocate the pages and its fd is passed to
the user. The user is now deciding on when to reclaim these pages to
free up the memory through already presented vfs_fadvise(DONTNEED)
system call and bringing back them through vfs_fadvise(WILLNEED), when
they are needed.

This user decision, in just one of the usecases, is based on memory
pressure in the system. Using this fadvise(), the driver now has fully
managing the pages as the usecase requirement is such that when it is
running, all the pages should be present in the ram. And when it is not
running, I am making all those pages to goto swap there by making some
free memory.

This is for embedded system application where display drivers are involved.

>> Another usecase is that GEM
>> objects which are currently allocated and managed through shmem files
>> can use vfs_fadvise(DONT|WILLNEED) on shmem fd when the driver comes to
>> know(like through some hints from user space) that GEM objects are not
>> going to use/will need in the near future.
> 
> Again, is this just a theoretical bright idea, or can we be assured
> that adding this code to the kernel will end up having been useful to
> our users?

This is currently the idea I have and we really not have the code for
the usecase mentioned above mentioning about GEM objects. But I strongly
see that this will be definitely end up been useful to our users.

Because, we have another usecase which is close to the GEM buffer
allocation mechanism and using the vfs_fadvise() from the kernel to
manage those for DRM(display) drivers hence saying that can be useful to
others as well.

> 
>> Some questions asked while reviewing this patch:
>>
>> Q) Can the same thing be achieved with FD mapped to user and use
>> madvise?
>> A) All drivers are not mapping all the shmem fd's to user space and want
>> to manage them with in the kernel. Ex: shmem memory can be mapped to the
>> other subsystems and they fill in the data and then give it to other
>> subsystem for further processing, where, the user mapping is not at all
>> required.  A simple example, memory that is given for gpu subsystem
>> which can be filled directly and give to display subsystem. And the
>> respective drivers know well about when to keep that memory in ram or
>> swap based on may be a user activity.
>>
>> Q) Should we add the documentation section in Manual pages?
>> A) The man[1] pages for the fadvise() whatever says is also applicable
>> for shmem files. so couldn't feel it correct to add specific to shmem
>> files separately.
>> [1] https://linux.die.net/man/2/fadvise
>>
>> Q) The proposed semantics of POSIX_FADV_DONTNEED is actually similar to
>> MADV_PAGEOUT and different from MADV_DONTNEED. This is a user facing API
>> and this difference will cause confusion?
>> A) man pages [1] says that "POSIX_FADV_DONTNEED attempts to free cached
>> pages associated with the specified region." This means on issuing this
>> FADV, it is expected to free the file cache pages. And it is
>> implementation defined If the dirty pages may be attempted to writeback.
>> And the unwritten dirty pages will not be freed. So, FADV_DONTNEED also
>> covers the semantics of MADV_PAGEOUT for file pages and there is no
>> purpose of PAGEOUT for file pages.
> 

Re: [PATCH RESEND V5,2/2] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem

[Andrew Morton]

On Tue, 7 Jun 2022 20:22:34 +0530 Charan Teja Kalla <quic_charante@quicinc.com> wrote:

> > Is there an actual userspace/driver combination which will use this? 
> > Has the new feature been tested in such an arrangement?  And if so,
> > which driver(s)?
> > 
> 
> Currently my organization is using this setup where it does makes use of
> the shmem infrastructure to allocate the pages and its fd is passed to
> the user. The user is now deciding on when to reclaim these pages to
> free up the memory through already presented vfs_fadvise(DONTNEED)
> system call and bringing back them through vfs_fadvise(WILLNEED), when

OK, thanks.  Please capture all these thoughts in the [0/n] changelog,
leave it a few days for reviewer input then resend and I'll take a
closer look.

The patches don't seem to be getting a lot of traction.

Re: [PATCH RESEND V5,0/2]mm: shmem: support POSIX_FADV_[WILL|DONT]NEED for shmem files

[Charan Teja Kalla]

Hi Andrew and all,

I am trying to comeback and make another attempt to upstream this patch
by providing even more details:

We have the requirement similar to of GEM objects backed by shmem but to
reclaim those pages instantaneously when the user wants it.  And for
file reclaim, users can use POSIX_FADV_DONTNEED to reclaim the pages(as
per standard, the clean pages are dropped but the dirty pages writeback
is implementation defined).  And client uses the POSIX_FADV_WILLNEED
when he needs those pages back.

This requirement is implemented on the snapdragon chips for graphics
clients where buffers of which are allocated through shmem interface(not
mapped to the userspace) and reclaimed[1][2] through a function
implemented in downstream Android kernel. We know that shmem allocation
support is already implemented in the upstream but the instantaneous
reclaim of those pages(which fadvise() can be used for the file pages)
is missed in the upstream implementation. This patch aims to implement
the support of reclaiming the shmem pages through the shmem fadvise but
__currently we are unable to find any upstream users for this usecase
and may be that's why didn't get any active reviews on this patch__.

So this attempt is to equip the linux kernel with "fadvise support for
shmem page reclaim", which might be useful in the future.

If community sees there will be some real benefits with this patch set,
will port these against linux-next. Please provide your inputs.

[1]
https://git.codelinaro.org/clo/la/platform/vendor/qcom/opensource/graphics-kernel/-/blob/gfx-kernel.lnx.1.0.r3-rel/kgsl_reclaim.c#L289
[2]
https://android.googlesource.com/kernel/common/+/refs/heads/android12-5.10/mm/shmem.c#4310

Thanks,
Charan

On 3/31/2022 12:08 PM, Charan Teja Kalla wrote:
> From: Charan Teja Reddy <quic_charante@quicinc.com>
> 
> This patch aims to implement POSIX_FADV_WILLNEED and POSIX_FADV_DONTNEED
> advices to shmem files which can be helpful for the drivers who may want
> to manage the pages of shmem files on their own, like, that are created
> through shmem_file_setup[_with_mnt]().
> 
> Changes in V5:
>  -- Moved the 'endbyte' calculations to a header function for use by shmem_fadvise().
>  -- Addressed comments from suren.
>  -- No changes in resend. Retested on the latest tip.
> 
> Changes in V4:
>   -- Changed the code to use reclaim_pages() to writeout the shmem pages to swap and then reclaim.
>   -- Addressed comments from Mark Hemment and Matthew.
>   -- fadvise() on shmem file may even unmap a page.
>   -- https://patchwork.kernel.org/project/linux-mm/patch/1644572051-24091-1-git-send-email-quic_charante@quicinc.com/
> 
> Changes in V3:
>   -- Considered THP pages while doing FADVISE_[DONT|WILL]NEED, identified by Matthew.
>   -- xarray used properly, as identified by Matthew.
>   -- Excluded mapped pages as it requires unmapping and the man pages of fadvise don't talk about them.
>   -- RESEND: Fixed the compilation issue when CONFIG_TMPFS is not defined.
>   -- https://patchwork.kernel.org/project/linux-mm/patch/1641488717-13865-1-git-send-email-quic_charante@quicinc.com/
> 
> Changes in V2:
>   -- Rearranged the code to not to sleep with rcu_lock while using xas_() functionality.
>   -- Addressed the comments from Suren.
>   -- https://patchwork.kernel.org/project/linux-mm/patch/1638442253-1591-1-git-send-email-quic_charante@quicinc.com/
> 
> changes in V1:
>   -- Created the interface for fadvise(2) to work on shmem files.
>   -- https://patchwork.kernel.org/project/linux-mm/patch/1633701982-22302-1-git-send-email-charante@codeaurora.org/
> 
> Charan Teja Reddy (2):
>   mm: fadvise: move 'endbyte' calculations to helper function
>   mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem
> 
>  mm/fadvise.c  |  11 +-----
>  mm/internal.h |  21 ++++++++++
>  mm/shmem.c    | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 145 insertions(+), 10 deletions(-)
> 

Re: [PATCH RESEND V5,2/2] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem

[Matthew Wilcox]

On Thu, Mar 31, 2022 at 12:08:21PM +0530, Charan Teja Kalla wrote:
> +static void shmem_isolate_pages_range(struct address_space *mapping, loff_t start,
> +				loff_t end, struct list_head *list)
> +{
> +	XA_STATE(xas, &mapping->i_pages, start);
> +	struct page *page;
> +
> +	rcu_read_lock();
> +	xas_for_each(&xas, page, end) {
> +		if (xas_retry(&xas, page))
> +			continue;
> +		if (xa_is_value(page))
> +			continue;
> +
> +		if (!get_page_unless_zero(page))
> +			continue;
> +		if (isolate_lru_page(page)) {
> +			put_page(page);
> +			continue;
> +		}
> +		put_page(page);
> +
> +		if (PageUnevictable(page) || page_mapcount(page) > 1) {
> +			putback_lru_page(page);
> +			continue;
> +		}
> +
> +		/*
> +		 * Prepare the page to be passed to the reclaim_pages().
> +		 * VM couldn't reclaim the page unless we clear PG_young.
> +		 * Also, to ensure that the pages are written before
> +		 * reclaiming, page is set to dirty.
> +		 * Since we are not clearing the pte_young in the mapped
> +		 * page pte's, its reclaim may not be attempted.
> +		 */
> +		ClearPageReferenced(page);
> +		test_and_clear_page_young(page);
> +		list_add(&page->lru, list);
> +		if (need_resched()) {
> +			xas_pause(&xas);
> +			cond_resched_rcu();
> +		}
> +	}
> +	rcu_read_unlock();
> +}

This entire function needs to be converted to use folios instead of
pages if you're refreshing this patchset for current kernels.