Re: [PATCH v2] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem

[Charan Teja Kalla <charante@codeaurora.org>]

Firstly, Apologies for taking such a long time for the next spin. Posted
V3 @
https://patchwork.kernel.org/project/linux-mm/patch/1641395025-7922-1-git-send-email-quic_charante@quicinc.com/
. Please provide your comments.

On 12/6/2021 12:59 PM, Charan Teja Kalla wrote:
> Thanks Shakeel for your valuable inputs!!
> 
> On 12/2/2021 11:24 PM, Shakeel Butt wrote:
>> On Thu, Dec 2, 2021 at 2:51 AM Charan Teja Reddy
>> <quic_charante@quicinc.com> wrote:
>>>
>>> From: Charan Teja Reddy <charante@codeaurora.org>
>>>
>>> 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
>>> objets which are currenlty 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.
>>
>> 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.
> 
> man pages [1] says that "POSIX_FADV_DONTNEED attempts to free cached
> pages associated with the specified region." This statement, IIUC,  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.  And thus
> for Shmem files this is being done by dirtying the page and pushing out
> to swap.
> 
> So, Isn't the FADV_DONTNEED also covers the semantics of MADV_PAGEOUT
> for file pages? IOW, what is the purpose of PAGEOUT for the file pages.
> Or I am missing some trivial logic in your comment here?
> 
> Coming to MADV_DONTNEED[2], on the mapped shmem files doesn't have any
> effect as the pages of those shmem files can still be in RAM. Subsequent
> accesses of pages in the range will succeed from the up-to-date contents
> of the underlying mapped file. IOW, the pages are still be present in
> the cache. Am I wrong here?
> 
> [1] https://man7.org/linux/man-pages/man2/posix_fadvise.2.html
> [2] https://man7.org/linux/man-pages/man2/madvise.2.html
> 
> 
>>
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project