Linux NFS development
 help / color / mirror / Atom feed
* [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues
@ 2025-11-26  6:01 Mike Snitzer
  2025-11-26  6:01 ` [PATCH v2 1/3] nfs/localio: fix regression due to out-of-order __put_cred Mike Snitzer
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Mike Snitzer @ 2025-11-26  6:01 UTC (permalink / raw)
  To: Anna Schumaker; +Cc: Trond Myklebust, linux-nfs, zlang

Hi Anna,

Here are 3 LOCALIO fixes for issues discovered as a side-effect of
Zorro's recent bug report:
https://lore.kernel.org/linux-nfs/20251125144508.rxepvtwrubbuhzxs@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/

(note that the "generic/751 hang on nfs" still isn't fixed but that
one is a long-standing issue that can be reproduced against stock
v6.12.53 -- whereas these 3 fixes address code introduced during
v6.18-rcX).

Sorry for the trouble, have a wonderful Thanksgiving!

Mike

v2 changes:
- patch 1/3 is the same as was posted here:
  https://lore.kernel.org/linux-nfs/aSaTC51DkxEqQkrZ@kernel.org/
- added patches 2/3 and 3/3

Mike Snitzer (3):
  nfs/localio: fix regression due to out-of-order __put_cred
  nfs/localio: remove alignment size checking in nfs_is_local_dio_possible
  nfs/localio: remove 61 byte hole from needless ____cacheline_aligned

 fs/nfs/localio.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

-- 
2.44.0


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

* [PATCH v2 1/3] nfs/localio: fix regression due to out-of-order __put_cred
  2025-11-26  6:01 [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues Mike Snitzer
@ 2025-11-26  6:01 ` Mike Snitzer
  2025-11-26  6:01 ` [PATCH v2 2/3] nfs/localio: remove alignment size checking in nfs_is_local_dio_possible Mike Snitzer
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Mike Snitzer @ 2025-11-26  6:01 UTC (permalink / raw)
  To: Anna Schumaker; +Cc: Trond Myklebust, linux-nfs, zlang

Commit 86855311c117 ("nfs/localio: add refcounting for each iocb IO
associated with NFS pgio header") inadvertantly reintroduced the same
potential for __put_cred() triggering BUG_ON(cred == current->cred)
that commit 992203a1fba5 ("nfs/localio: restore creds before releasing
pageio data") fixed.

Fix this by saving and restoring the cred around each {read,write}_iter
call within the respective for loop of nfs_local_call_{read,write}.

Reported-by: Zorro Lang <zlang@redhat.com>
Fixes: 86855311c117 ("nfs/localio: add refcounting for each iocb IO associated with NFS pgio header")
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfs/localio.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
index 36c7d91014c7d..e30ea1c54c616 100644
--- a/fs/nfs/localio.c
+++ b/fs/nfs/localio.c
@@ -625,8 +625,6 @@ static void nfs_local_call_read(struct work_struct *work)
 	ssize_t status;
 	int n_iters;
 
-	save_cred = override_creds(filp->f_cred);
-
 	n_iters = atomic_read(&iocb->n_iters);
 	for (int i = 0; i < n_iters ; i++) {
 		if (iocb->iter_is_dio_aligned[i]) {
@@ -639,7 +637,10 @@ static void nfs_local_call_read(struct work_struct *work)
 		} else
 			iocb->kiocb.ki_flags &= ~IOCB_DIRECT;
 
+		save_cred = override_creds(filp->f_cred);
 		status = filp->f_op->read_iter(&iocb->kiocb, &iocb->iters[i]);
+		revert_creds(save_cred);
+
 		if (status != -EIOCBQUEUED) {
 			if (unlikely(status >= 0 && status < iocb->iters[i].count))
 				force_done = true; /* Partial read */
@@ -649,8 +650,6 @@ static void nfs_local_call_read(struct work_struct *work)
 			}
 		}
 	}
-
-	revert_creds(save_cred);
 }
 
 static int
@@ -832,7 +831,6 @@ static void nfs_local_call_write(struct work_struct *work)
 	int n_iters;
 
 	current->flags |= PF_LOCAL_THROTTLE | PF_MEMALLOC_NOIO;
-	save_cred = override_creds(filp->f_cred);
 
 	file_start_write(filp);
 	n_iters = atomic_read(&iocb->n_iters);
@@ -847,7 +845,10 @@ static void nfs_local_call_write(struct work_struct *work)
 		} else
 			iocb->kiocb.ki_flags &= ~IOCB_DIRECT;
 
+		save_cred = override_creds(filp->f_cred);
 		status = filp->f_op->write_iter(&iocb->kiocb, &iocb->iters[i]);
+		revert_creds(save_cred);
+
 		if (status != -EIOCBQUEUED) {
 			if (unlikely(status >= 0 && status < iocb->iters[i].count))
 				force_done = true; /* Partial write */
@@ -859,7 +860,6 @@ static void nfs_local_call_write(struct work_struct *work)
 	}
 	file_end_write(filp);
 
-	revert_creds(save_cred);
 	current->flags = old_flags;
 }
 
-- 
2.44.0


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

* [PATCH v2 2/3] nfs/localio: remove alignment size checking in nfs_is_local_dio_possible
  2025-11-26  6:01 [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues Mike Snitzer
  2025-11-26  6:01 ` [PATCH v2 1/3] nfs/localio: fix regression due to out-of-order __put_cred Mike Snitzer
@ 2025-11-26  6:01 ` Mike Snitzer
  2025-11-26  6:01 ` [PATCH v2 3/3] nfs/localio: remove 61 byte hole from needless ____cacheline_aligned Mike Snitzer
  2025-11-28 17:40 ` [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues Zorro Lang
  3 siblings, 0 replies; 5+ messages in thread
From: Mike Snitzer @ 2025-11-26  6:01 UTC (permalink / raw)
  To: Anna Schumaker; +Cc: Trond Myklebust, linux-nfs, zlang

From: Mike Snitzer <snitzer@hammerspace.com>

This check to ensure dio_offset_align isn't larger than PAGE_SIZE is
no longer relevant (older iterations of NFS Direct was allocating
misaligned head and tail pages but no longer does, so this check isn't
needed).

Fixes: c817248fc831 ("nfs/localio: add proper O_DIRECT support for READ and WRITE")
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfs/localio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
index e30ea1c54c616..560caa2e4238f 100644
--- a/fs/nfs/localio.c
+++ b/fs/nfs/localio.c
@@ -339,8 +339,6 @@ nfs_is_local_dio_possible(struct nfs_local_kiocb *iocb, int rw,
 
 	if (unlikely(!nf_dio_mem_align || !nf_dio_offset_align))
 		return false;
-	if (unlikely(nf_dio_offset_align > PAGE_SIZE))
-		return false;
 	if (unlikely(len < nf_dio_offset_align))
 		return false;
 
-- 
2.44.0


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

* [PATCH v2 3/3] nfs/localio: remove 61 byte hole from needless ____cacheline_aligned
  2025-11-26  6:01 [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues Mike Snitzer
  2025-11-26  6:01 ` [PATCH v2 1/3] nfs/localio: fix regression due to out-of-order __put_cred Mike Snitzer
  2025-11-26  6:01 ` [PATCH v2 2/3] nfs/localio: remove alignment size checking in nfs_is_local_dio_possible Mike Snitzer
@ 2025-11-26  6:01 ` Mike Snitzer
  2025-11-28 17:40 ` [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues Zorro Lang
  3 siblings, 0 replies; 5+ messages in thread
From: Mike Snitzer @ 2025-11-26  6:01 UTC (permalink / raw)
  To: Anna Schumaker; +Cc: Trond Myklebust, linux-nfs, zlang

struct nfs_local_kiocb used ____cacheline_aligned on its iters[] array
and as the structure evolved it caused a 61 byte hole to form.  Fix
this by removing ____cacheline_aligned and reordering iters[] before
iter_is_dio_aligned[].

Fixes: 6a218b9c3183 ("nfs/localio: do not issue misaligned DIO out-of-order")
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfs/localio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
index 560caa2e4238f..eeb05a0d8d260 100644
--- a/fs/nfs/localio.c
+++ b/fs/nfs/localio.c
@@ -43,8 +43,8 @@ struct nfs_local_kiocb {
 	size_t                  end_len;
 	short int		end_iter_index;
 	atomic_t		n_iters;
+	struct iov_iter		iters[NFSLOCAL_MAX_IOS];
 	bool			iter_is_dio_aligned[NFSLOCAL_MAX_IOS];
-	struct iov_iter		iters[NFSLOCAL_MAX_IOS] ____cacheline_aligned;
 	/* End mostly DIO-specific members */
 };
 
-- 
2.44.0


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

* Re: [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues
  2025-11-26  6:01 [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues Mike Snitzer
                   ` (2 preceding siblings ...)
  2025-11-26  6:01 ` [PATCH v2 3/3] nfs/localio: remove 61 byte hole from needless ____cacheline_aligned Mike Snitzer
@ 2025-11-28 17:40 ` Zorro Lang
  3 siblings, 0 replies; 5+ messages in thread
From: Zorro Lang @ 2025-11-28 17:40 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: Anna Schumaker, Trond Myklebust, linux-nfs

On Wed, Nov 26, 2025 at 01:01:24AM -0500, Mike Snitzer wrote:
> Hi Anna,
> 
> Here are 3 LOCALIO fixes for issues discovered as a side-effect of
> Zorro's recent bug report:
> https://lore.kernel.org/linux-nfs/20251125144508.rxepvtwrubbuhzxs@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/

Hi Mike,

I just tested this patchset (base on linux v6.18-rc7 which can reproduce that
cred crash bug). Then the test didn't trigger the crash. So I think this
patchset works for that crash bug. (But the g/751 hang bug is still there.)

> 
> (note that the "generic/751 hang on nfs" still isn't fixed but that
> one is a long-standing issue that can be reproduced against stock
> v6.12.53 -- whereas these 3 fixes address code introduced during
> v6.18-rcX).
> 
> Sorry for the trouble, have a wonderful Thanksgiving!

Have a nice Thanksgiving day!

Thanks,
Zorro

> 
> Mike
> 
> v2 changes:
> - patch 1/3 is the same as was posted here:
>   https://lore.kernel.org/linux-nfs/aSaTC51DkxEqQkrZ@kernel.org/
> - added patches 2/3 and 3/3
> 
> Mike Snitzer (3):
>   nfs/localio: fix regression due to out-of-order __put_cred
>   nfs/localio: remove alignment size checking in nfs_is_local_dio_possible
>   nfs/localio: remove 61 byte hole from needless ____cacheline_aligned
> 
>  fs/nfs/localio.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> -- 
> 2.44.0
> 


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

end of thread, other threads:[~2025-11-28 17:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-26  6:01 [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues Mike Snitzer
2025-11-26  6:01 ` [PATCH v2 1/3] nfs/localio: fix regression due to out-of-order __put_cred Mike Snitzer
2025-11-26  6:01 ` [PATCH v2 2/3] nfs/localio: remove alignment size checking in nfs_is_local_dio_possible Mike Snitzer
2025-11-26  6:01 ` [PATCH v2 3/3] nfs/localio: remove 61 byte hole from needless ____cacheline_aligned Mike Snitzer
2025-11-28 17:40 ` [for-v6.18 PATCH v2 0/3] nfs/localio: fix various new issues Zorro Lang

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