[PATCH v2 1/2] mm: shmem: update iocb->ki_pos directly to simplify tmpfs read logic

Baolin Wang posted 2 patches 1 month, 1 week ago
[PATCH v2 1/2] mm: shmem: update iocb->ki_pos directly to simplify tmpfs read logic
Posted by Baolin Wang 1 month, 1 week ago
Use iocb->ki_pos to check if the read bytes exceeds the file size and to
calculate the bytes to be read can help simplify the code logic. Meanwhile,
this is also a preparation for improving tmpfs large folios read performace
in the following patch.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/shmem.c | 35 +++++++++++------------------------
 1 file changed, 11 insertions(+), 24 deletions(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 66eae800ffab..93642aa8d1aa 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3106,27 +3106,19 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	unsigned long offset;
 	int error = 0;
 	ssize_t retval = 0;
-	loff_t *ppos = &iocb->ki_pos;
 
-	index = *ppos >> PAGE_SHIFT;
-	offset = *ppos & ~PAGE_MASK;
+	offset = iocb->ki_pos & ~PAGE_MASK;
 
 	for (;;) {
 		struct folio *folio = NULL;
 		struct page *page = NULL;
-		pgoff_t end_index;
 		unsigned long nr, ret;
-		loff_t i_size = i_size_read(inode);
+		loff_t end_offset, i_size = i_size_read(inode);
 
-		end_index = i_size >> PAGE_SHIFT;
-		if (index > end_index)
+		if (unlikely(iocb->ki_pos >= i_size))
 			break;
-		if (index == end_index) {
-			nr = i_size & ~PAGE_MASK;
-			if (nr <= offset)
-				break;
-		}
 
+		index = iocb->ki_pos >> PAGE_SHIFT;
 		error = shmem_get_folio(inode, index, 0, &folio, SGP_READ);
 		if (error) {
 			if (error == -EINVAL)
@@ -3148,18 +3140,14 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 		 * We must evaluate after, since reads (unlike writes)
 		 * are called without i_rwsem protection against truncate
 		 */
-		nr = PAGE_SIZE;
 		i_size = i_size_read(inode);
-		end_index = i_size >> PAGE_SHIFT;
-		if (index == end_index) {
-			nr = i_size & ~PAGE_MASK;
-			if (nr <= offset) {
-				if (folio)
-					folio_put(folio);
-				break;
-			}
+		if (unlikely(iocb->ki_pos >= i_size)) {
+			if (folio)
+				folio_put(folio);
+			break;
 		}
-		nr -= offset;
+		end_offset = min_t(loff_t, i_size, iocb->ki_pos + to->count);
+		nr = min_t(loff_t, end_offset - iocb->ki_pos, PAGE_SIZE - offset);
 
 		if (folio) {
 			/*
@@ -3199,8 +3187,8 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 
 		retval += ret;
 		offset += ret;
-		index += offset >> PAGE_SHIFT;
 		offset &= ~PAGE_MASK;
+		iocb->ki_pos += ret;
 
 		if (!iov_iter_count(to))
 			break;
@@ -3211,7 +3199,6 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 		cond_resched();
 	}
 
-	*ppos = ((loff_t) index << PAGE_SHIFT) + offset;
 	file_accessed(file);
 	return retval ? retval : error;
 }
-- 
2.39.3
Re: [PATCH v2 1/2] mm: shmem: update iocb->ki_pos directly to simplify tmpfs read logic
Posted by Yang Shi 1 month, 1 week ago
On Thu, Oct 17, 2024 at 8:00 PM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
> Use iocb->ki_pos to check if the read bytes exceeds the file size and to
> calculate the bytes to be read can help simplify the code logic. Meanwhile,
> this is also a preparation for improving tmpfs large folios read performace

s/performace/performance

> in the following patch.
>
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>

The patch looks good to me. Reviewed-by: Yang Shi <shy828301@gmail.com>

> ---
>  mm/shmem.c | 35 +++++++++++------------------------
>  1 file changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 66eae800ffab..93642aa8d1aa 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3106,27 +3106,19 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>         unsigned long offset;
>         int error = 0;
>         ssize_t retval = 0;
> -       loff_t *ppos = &iocb->ki_pos;
>
> -       index = *ppos >> PAGE_SHIFT;
> -       offset = *ppos & ~PAGE_MASK;
> +       offset = iocb->ki_pos & ~PAGE_MASK;
>
>         for (;;) {
>                 struct folio *folio = NULL;
>                 struct page *page = NULL;
> -               pgoff_t end_index;
>                 unsigned long nr, ret;
> -               loff_t i_size = i_size_read(inode);
> +               loff_t end_offset, i_size = i_size_read(inode);
>
> -               end_index = i_size >> PAGE_SHIFT;
> -               if (index > end_index)
> +               if (unlikely(iocb->ki_pos >= i_size))
>                         break;
> -               if (index == end_index) {
> -                       nr = i_size & ~PAGE_MASK;
> -                       if (nr <= offset)
> -                               break;
> -               }
>
> +               index = iocb->ki_pos >> PAGE_SHIFT;
>                 error = shmem_get_folio(inode, index, 0, &folio, SGP_READ);
>                 if (error) {
>                         if (error == -EINVAL)
> @@ -3148,18 +3140,14 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>                  * We must evaluate after, since reads (unlike writes)
>                  * are called without i_rwsem protection against truncate
>                  */
> -               nr = PAGE_SIZE;
>                 i_size = i_size_read(inode);
> -               end_index = i_size >> PAGE_SHIFT;
> -               if (index == end_index) {
> -                       nr = i_size & ~PAGE_MASK;
> -                       if (nr <= offset) {
> -                               if (folio)
> -                                       folio_put(folio);
> -                               break;
> -                       }
> +               if (unlikely(iocb->ki_pos >= i_size)) {
> +                       if (folio)
> +                               folio_put(folio);
> +                       break;
>                 }
> -               nr -= offset;
> +               end_offset = min_t(loff_t, i_size, iocb->ki_pos + to->count);
> +               nr = min_t(loff_t, end_offset - iocb->ki_pos, PAGE_SIZE - offset);
>
>                 if (folio) {
>                         /*
> @@ -3199,8 +3187,8 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>
>                 retval += ret;
>                 offset += ret;
> -               index += offset >> PAGE_SHIFT;
>                 offset &= ~PAGE_MASK;
> +               iocb->ki_pos += ret;
>
>                 if (!iov_iter_count(to))
>                         break;
> @@ -3211,7 +3199,6 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>                 cond_resched();
>         }
>
> -       *ppos = ((loff_t) index << PAGE_SHIFT) + offset;
>         file_accessed(file);
>         return retval ? retval : error;
>  }
> --
> 2.39.3
>