On Sat, Feb 18, 2023 at 09:16:43AM +0800, Shiyang Ruan wrote:
> 在 2023/2/18 0:14, Matthew Wilcox 写道:
> > On Fri, Feb 17, 2023 at 02:48:31PM +0000, Shiyang Ruan wrote:
> > > - invalidate_mapping_pages(inode->i_mapping, 0, -1);
> > > - iput(toput_inode);
> > > - toput_inode = inode;
> > > -
> > > - cond_resched();
> > > - spin_lock(&sb->s_inode_list_lock);
> > > - }
> > > - spin_unlock(&sb->s_inode_list_lock);
> > > - iput(toput_inode);
> > > + super_drop_pagecache(sb, invalidate_inode_pages);
> >
> > I thought I explained last time that you can do this with
> > invalidate_mapping_pages() / invalidate_inode_pages2_range() ?
> > Then you don't need to introduce invalidate_inode_pages().
> >
> > > +void super_drop_pagecache(struct super_block *sb,
> > > + int (*invalidator)(struct address_space *))
> >
> > void super_drop_pagecache(struct super_block *sb,
> > int (*invalidate)(struct address_space *, pgoff_t, pgoff_t))
> >
> > > + invalidator(inode->i_mapping);
> >
> > invalidate(inode->i_mapping, 0, -1)
> >
> > ... then all the changes to mm/truncate.c and filemap.h go away.
>
> Yes, I tried as you suggested, but I found that they don't have same type of
> return value.
>
> int invalidate_inode_pages2_range(struct address_space *mapping,
> pgoff_t start, pgoff_t end);
>
> unsigned long invalidate_mapping_pages(struct address_space *mapping,
> pgoff_t start, pgoff_t end);
Oh, that's annoying. Particularly annoying is that the return value
for invalidate_mapping_pages() is used by fs/inode.c to account for
the number of pages invalidate, and the return value for
invalidate_inode_pages2_range() is used by several filesystems
to know whether an error occurred.
Hm. Shouldn't you be checking for an error from
invalidate_inode_pages2_range()? Seems like it can return -EBUSY for
DAX entries.
With that in mind, the wrapper you actually want to exist is
static int invalidate_inode_pages_range(struct address_space *mapping,
pgoff_t start, pgoff_t end)
{
invalidate_mapping_pages(mapping, start, end);
return 0;
}
Right?
