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

[Charan Teja Kalla <quic_charante@quicinc.com>]

Thanks Suren!!

On 2/8/2023 4:18 AM, Suren Baghdasaryan wrote:
>> +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;
>> +       struct inode *inode = file_inode(file);
>> +       int ret = 0;
>> +
>> +       if (S_ISFIFO(inode->i_mode))
>> +               return -ESPIPE;
>> +
>> +       mapping = file->f_mapping;
>> +       if (!mapping || len < 0 || !shmem_mapping(mapping))
>> +               return -EINVAL;
>> +
>> +       endbyte = fadvise_calc_endbyte(offset, len);
>> +
>> +       start_index = offset >> PAGE_SHIFT;
>> +       end_index   = endbyte >> PAGE_SHIFT;
>> +       switch (advice) {
>> +       case POSIX_FADV_DONTNEED:
> Should (SHMEM_I(inode)->flags & VM_LOCKED) be checked here too?
> 
Is this w.r.t context from shmem_lock() perspective which does set this
flag?  If so, Isn't the PageUnevictable check cover this part? And to
avoid unnecessary Unevictable check later on the locked shmem file, How
about just checking mapping_unevictable() before performing
shmem_fadvise_dontneed)()? Please let me know If I failed to get your point.

>> +               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:

--Charan