Sender: charante=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.4.1 smtp.codeaurora.org ED612C4338F
Subject: Re: [PATCH v2] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for
 shmem
To:     Shakeel Butt <shakeelb@google.com>,
        Charan Teja Reddy <quic_charante@quicinc.com>
Cc:     hughd@google.com, akpm@linux-foundation.org, vbabka@suse.cz,
        rientjes@google.com, david@redhat.com, mhocko@suse.com,
        surenb@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org
References: <1638442253-1591-1-git-send-email-quic_charante@quicinc.com>
 <CALvZod6fSkdV6oSyxsv0+gcRfZ=H9Uaw=7=t902U85fWJStuzA@mail.gmail.com>
 <4a5cde83-c673-6af0-702f-f8b59e05a397@codeaurora.org>
From:   Charan Teja Kalla <charante@codeaurora.org>
Message-ID: <08d3c298-1568-1ea2-8215-0dffa1c5a089@codeaurora.org>
Date:   Wed, 5 Jan 2022 20:47:25 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <4a5cde83-c673-6af0-702f-f8b59e05a397@codeaurora.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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