| Message ID | 20220721131637.6306-5-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few cleanup and fixup patches for hugetlbfs | expand |
On 07/21/22 21:16, Miaohe Lin wrote: > The function generic_file_buffered_read has been renamed to filemap_read > since commit 87fa0f3eb267 ("mm/filemap: rename generic_file_buffered_read > to filemap_read"). Update the corresponding comment. And duplicated taken > in hugetlbfs_fill_super is removed. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > fs/hugetlbfs/inode.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c > index aa7a5b8fc724..19fc62a9c2fe 100644 > --- a/fs/hugetlbfs/inode.c > +++ b/fs/hugetlbfs/inode.c > @@ -313,8 +313,8 @@ hugetlbfs_read_actor(struct page *page, unsigned long offset, > > /* > * Support for read() - Find the page attached to f_mapping and copy out the > - * data. Its *very* similar to generic_file_buffered_read(), we can't use that > - * since it has PAGE_SIZE assumptions. > + * data. Its *very* similar to filemap_read(), we can't use that since it has > + * PAGE_SIZE assumptions. Since you are changing the comment, I would just say this provides functionality similar to filemap_read(). filemap_read is now operating on folios which may remove any PAGE_SIZE assumptions. One day when hugetlb is converted to folios it may end up using filemap_read instead of hugetlbfs_read_actor.
On 2022/7/22 7:23, Mike Kravetz wrote: > On 07/21/22 21:16, Miaohe Lin wrote: >> The function generic_file_buffered_read has been renamed to filemap_read >> since commit 87fa0f3eb267 ("mm/filemap: rename generic_file_buffered_read >> to filemap_read"). Update the corresponding comment. And duplicated taken >> in hugetlbfs_fill_super is removed. >> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> fs/hugetlbfs/inode.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c >> index aa7a5b8fc724..19fc62a9c2fe 100644 >> --- a/fs/hugetlbfs/inode.c >> +++ b/fs/hugetlbfs/inode.c >> @@ -313,8 +313,8 @@ hugetlbfs_read_actor(struct page *page, unsigned long offset, >> >> /* >> * Support for read() - Find the page attached to f_mapping and copy out the >> - * data. Its *very* similar to generic_file_buffered_read(), we can't use that >> - * since it has PAGE_SIZE assumptions. >> + * data. Its *very* similar to filemap_read(), we can't use that since it has >> + * PAGE_SIZE assumptions. > > Since you are changing the comment, I would just say this provides > functionality similar to filemap_read(). filemap_read is now operating > on folios which may remove any PAGE_SIZE assumptions. One day when > hugetlb is converted to folios it may end up using filemap_read instead > of hugetlbfs_read_actor. Sounds reasonable. > Do you mean changing the comment of hugetlbfs_read_iter like below ? /* * Support for read() - Find the page attached to f_mapping and copy out the * data. This provides functionality similar to filemap_read(). */ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to) Thanks.
On 07/22/22 14:19, Miaohe Lin wrote: > On 2022/7/22 7:23, Mike Kravetz wrote: > > On 07/21/22 21:16, Miaohe Lin wrote: > >> The function generic_file_buffered_read has been renamed to filemap_read > >> since commit 87fa0f3eb267 ("mm/filemap: rename generic_file_buffered_read > >> to filemap_read"). Update the corresponding comment. And duplicated taken > >> in hugetlbfs_fill_super is removed. > >> > >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > >> --- > >> fs/hugetlbfs/inode.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c > >> index aa7a5b8fc724..19fc62a9c2fe 100644 > >> --- a/fs/hugetlbfs/inode.c > >> +++ b/fs/hugetlbfs/inode.c > >> @@ -313,8 +313,8 @@ hugetlbfs_read_actor(struct page *page, unsigned long offset, > >> > >> /* > >> * Support for read() - Find the page attached to f_mapping and copy out the > >> - * data. Its *very* similar to generic_file_buffered_read(), we can't use that > >> - * since it has PAGE_SIZE assumptions. > >> + * data. Its *very* similar to filemap_read(), we can't use that since it has > >> + * PAGE_SIZE assumptions. > > > > Since you are changing the comment, I would just say this provides > > functionality similar to filemap_read(). filemap_read is now operating > > on folios which may remove any PAGE_SIZE assumptions. One day when > > hugetlb is converted to folios it may end up using filemap_read instead > > of hugetlbfs_read_actor. > > Sounds reasonable. > > > > > Do you mean changing the comment of hugetlbfs_read_iter like below ? > > /* > * Support for read() - Find the page attached to f_mapping and copy out the > * data. This provides functionality similar to filemap_read(). > */ > static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to) > Exactly! With such a change, you can add: Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index aa7a5b8fc724..19fc62a9c2fe 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -313,8 +313,8 @@ hugetlbfs_read_actor(struct page *page, unsigned long offset, /* * Support for read() - Find the page attached to f_mapping and copy out the - * data. Its *very* similar to generic_file_buffered_read(), we can't use that - * since it has PAGE_SIZE assumptions. + * data. Its *very* similar to filemap_read(), we can't use that since it has + * PAGE_SIZE assumptions. */ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to) { @@ -1383,7 +1383,7 @@ hugetlbfs_fill_super(struct super_block *sb, struct fs_context *fc) /* * Allocate and initialize subpool if maximum or minimum size is * specified. Any needed reservations (for minimum size) are taken - * taken when the subpool is created. + * when the subpool is created. */ if (ctx->max_hpages != -1 || ctx->min_hpages != -1) { sbinfo->spool = hugepage_new_subpool(ctx->hstate,
The function generic_file_buffered_read has been renamed to filemap_read since commit 87fa0f3eb267 ("mm/filemap: rename generic_file_buffered_read to filemap_read"). Update the corresponding comment. And duplicated taken in hugetlbfs_fill_super is removed. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- fs/hugetlbfs/inode.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)