Thanks Suren for your comments!!
On 3/5/2022 2:00 AM, Suren Baghdasaryan wrote:
>> + test_and_clear_page_young(page);
>> + SetPageDirty(page);
> I asked Hugh about how clean shmem pages are handled during normal
> reclaim and his reply is:
>
> Clean shmem pages are rare: they correspond to where a hole in a
> sparse file has been mapped read-only to userspace. Those get dropped
> from pagecache without being written to swap, when memory pressure
> comes to reclaim them. Otherwise, shmem pages are dirty: as soon as
> they've been read from swap and identified as shmem (moved from swap
> cache to page cache), that swap is freed so they're no longer clean
> representations of anything on swap. (Our use of "swap cache" and/or
> "page cache" may have inconsistencies in what I've written: sometimes
> we use them interchangeably, sometimes we're making a distinction.)
>
> So, IIUC his explanation, you don't really need to mark clean shmem
> pages dirty for FADV_DONTNEED since normal reclaim does not do that
> either.
Thanks for the details here. Will change it in the next patch.
>
>> + 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 = (u64)offset + (u64)len;
>> + if (!len || endbyte < len)
>> + endbyte = -1;
>> + else
>> + endbyte--;
> The above block is exactly the same as in
> https://elixir.bootlin.com/linux/latest/source/mm/fadvise.c#L73 with
> the exception that generic_fadvise has comments with explanations of
> this math. You might consider consolidating them into a helper
> function to calculate the endbyte.
Yes, I simply copy pasted these. Will try to consolidate them into one.
>
