linux-kernel - Re: [PATCH V6 2/2] mm: shmem: implement POSIX_FADV

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 8 Feb 2023 14:51:56 -0800
From:   Suren Baghdasaryan <surenb@...gle.com>
To:     Charan Teja Kalla <quic_charante@...cinc.com>
Cc:     akpm@...ux-foundation.org, hughd@...gle.com, willy@...radead.org,
        markhemm@...glemail.com, rientjes@...gle.com, shakeelb@...gle.com,
        mhocko@...e.com, vbabka@...e.cz, quic_pkondeti@...cinc.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V6 2/2] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED
 for shmem

On Wed, Feb 8, 2023 at 6:55 AM Charan Teja Kalla
<quic_charante@...cinc.com> wrote:
>
> 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.

Yes, you got my point and checking for mapping_unevictable() should
work fine I think.

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