Sender: charante=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.4.1 smtp.codeaurora.org 247D1C4338F
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>
From:   Charan Teja Kalla <charante@codeaurora.org>
Message-ID: <4a5cde83-c673-6af0-702f-f8b59e05a397@codeaurora.org>
Date:   Mon, 6 Dec 2021 12:59:06 +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: <CALvZod6fSkdV6oSyxsv0+gcRfZ=H9Uaw=7=t902U85fWJStuzA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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