Linux filesystem development
 help / color / mirror / Atom feed
* [PATCH] orangefs: use folio_pos() and folio_size() in orangefs_page_mkwrite()
@ 2026-08-09 20:07 Tal Zussman
  2026-08-10  3:57 ` Matthew Wilcox
  2026-08-10 15:21 ` Mike Marshall
  0 siblings, 2 replies; 3+ messages in thread
From: Tal Zussman @ 2026-08-09 20:07 UTC (permalink / raw)
  To: Mike Marshall, Martin Brandenburg
  Cc: Matthew Wilcox (Oracle), devel, linux-fsdevel, linux-kernel,
	Tal Zussman

orangefs_page_mkwrite() records the faulted range with
page_offset(vmf->page) and PAGE_SIZE, although the write range it sets
is attached to the folio and the rest of the function already operates
on folios. Use folio_pos() and folio_size() instead. This gets rid of
two calls to page_offset(), removing two calls to compound_head().

No functional change. orangefs folios are always order-0, so the values
are identical. However, if orangefs ever enables large folios, this
change is necessary for correctness with the current write range
tracking scheme. Tracking only a single page of a larger folio would
leave the rest of the folio's dirty data outside the range that gets
written back, leading to data loss.

Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 fs/orangefs/inode.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 7143b64b5b25..cd3273c88e03 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -652,8 +652,8 @@ vm_fault_t orangefs_page_mkwrite(struct vm_fault *vmf)
 		wr = folio_get_private(folio);
 		if (uid_eq(wr->uid, current_fsuid()) &&
 		    gid_eq(wr->gid, current_fsgid())) {
-			wr->pos = page_offset(vmf->page);
-			wr->len = PAGE_SIZE;
+			wr->pos = folio_pos(folio);
+			wr->len = folio_size(folio);
 			goto okay;
 		} else {
 			if (orangefs_launder_folio(folio)) {
@@ -667,8 +667,8 @@ vm_fault_t orangefs_page_mkwrite(struct vm_fault *vmf)
 		ret = VM_FAULT_LOCKED|VM_FAULT_RETRY;
 		goto out;
 	}
-	wr->pos = page_offset(vmf->page);
-	wr->len = PAGE_SIZE;
+	wr->pos = folio_pos(folio);
+	wr->len = folio_size(folio);
 	wr->uid = current_fsuid();
 	wr->gid = current_fsgid();
 	folio_attach_private(folio, wr);

---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260809-orangefs-mkwrite-folio-ec562f7625b4

Best regards,
-- 
Tal Zussman <tz2294@columbia.edu>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] orangefs: use folio_pos() and folio_size() in orangefs_page_mkwrite()
  2026-08-09 20:07 [PATCH] orangefs: use folio_pos() and folio_size() in orangefs_page_mkwrite() Tal Zussman
@ 2026-08-10  3:57 ` Matthew Wilcox
  2026-08-10 15:21 ` Mike Marshall
  1 sibling, 0 replies; 3+ messages in thread
From: Matthew Wilcox @ 2026-08-10  3:57 UTC (permalink / raw)
  To: Tal Zussman
  Cc: Mike Marshall, Martin Brandenburg, devel, linux-fsdevel,
	linux-kernel

On Sun, Aug 09, 2026 at 04:07:10PM -0400, Tal Zussman wrote:
> orangefs_page_mkwrite() records the faulted range with
> page_offset(vmf->page) and PAGE_SIZE, although the write range it sets
> is attached to the folio and the rest of the function already operates
> on folios. Use folio_pos() and folio_size() instead. This gets rid of
> two calls to page_offset(), removing two calls to compound_head().
> 
> No functional change. orangefs folios are always order-0, so the values
> are identical. However, if orangefs ever enables large folios, this
> change is necessary for correctness with the current write range
> tracking scheme. Tracking only a single page of a larger folio would
> leave the rest of the folio's dirty data outside the range that gets
> written back, leading to data loss.

Yeah, I agree.

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] orangefs: use folio_pos() and folio_size() in orangefs_page_mkwrite()
  2026-08-09 20:07 [PATCH] orangefs: use folio_pos() and folio_size() in orangefs_page_mkwrite() Tal Zussman
  2026-08-10  3:57 ` Matthew Wilcox
@ 2026-08-10 15:21 ` Mike Marshall
  1 sibling, 0 replies; 3+ messages in thread
From: Mike Marshall @ 2026-08-10 15:21 UTC (permalink / raw)
  To: Tal Zussman
  Cc: Matthew Wilcox (Oracle), devel, linux-fsdevel, linux-kernel,
	Mike Marshall

Thanks! I'm trying to finish up some stuff to get on
linux-next in time, I'll let you know when I've run your
patch through xfstests...

-Mike

On Sun, Aug 9, 2026 at 4:07 PM Tal Zussman <tz2294@columbia.edu> wrote:
>
> orangefs_page_mkwrite() records the faulted range with
> page_offset(vmf->page) and PAGE_SIZE, although the write range it sets
> is attached to the folio and the rest of the function already operates
> on folios. Use folio_pos() and folio_size() instead. This gets rid of
> two calls to page_offset(), removing two calls to compound_head().
>
> No functional change. orangefs folios are always order-0, so the values
> are identical. However, if orangefs ever enables large folios, this
> change is necessary for correctness with the current write range
> tracking scheme. Tracking only a single page of a larger folio would
> leave the rest of the folio's dirty data outside the range that gets
> written back, leading to data loss.
>
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>  fs/orangefs/inode.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
> index 7143b64b5b25..cd3273c88e03 100644
> --- a/fs/orangefs/inode.c
> +++ b/fs/orangefs/inode.c
> @@ -652,8 +652,8 @@ vm_fault_t orangefs_page_mkwrite(struct vm_fault *vmf)
>                 wr = folio_get_private(folio);
>                 if (uid_eq(wr->uid, current_fsuid()) &&
>                     gid_eq(wr->gid, current_fsgid())) {
> -                       wr->pos = page_offset(vmf->page);
> -                       wr->len = PAGE_SIZE;
> +                       wr->pos = folio_pos(folio);
> +                       wr->len = folio_size(folio);
>                         goto okay;
>                 } else {
>                         if (orangefs_launder_folio(folio)) {
> @@ -667,8 +667,8 @@ vm_fault_t orangefs_page_mkwrite(struct vm_fault *vmf)
>                 ret = VM_FAULT_LOCKED|VM_FAULT_RETRY;
>                 goto out;
>         }
> -       wr->pos = page_offset(vmf->page);
> -       wr->len = PAGE_SIZE;
> +       wr->pos = folio_pos(folio);
> +       wr->len = folio_size(folio);
>         wr->uid = current_fsuid();
>         wr->gid = current_fsgid();
>         folio_attach_private(folio, wr);
>
> ---
> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
> change-id: 20260809-orangefs-mkwrite-folio-ec562f7625b4
>
> Best regards,
> --
> Tal Zussman <tz2294@columbia.edu>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-10 15:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-09 20:07 [PATCH] orangefs: use folio_pos() and folio_size() in orangefs_page_mkwrite() Tal Zussman
2026-08-10  3:57 ` Matthew Wilcox
2026-08-10 15:21 ` Mike Marshall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox