[PATCH 0/3] Minor locking fixes and debug output

[PATCH 0/3] Minor locking fixes and debug output

[Valerie Aurora]

Fix some minor locking issues found by lockdep and add a little
helpful debug output.

Valerie Aurora (3):
  rpdfs: Use write_seqlock_bh() for balloc info
  rpdfs: Initialize block seqlock before using it and fix memleak
  rpdfs: Add debugging for block not found case in
    rpdfs_use_txn_prepared

 fs/rpdfs/balloc.c | 12 ++++++------
 fs/rpdfs/block.c  | 13 +++++++------
 fs/rpdfs/txn.c    |  1 +
 3 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.49.0

[PATCH 1/3] rpdfs: Use write_seqlock_bh() for balloc info

[Valerie Aurora]

The balloc info seqlock is taken from bh context due to
rpdfs_balloc_publish_region() being called from an RCU callback, so it
needs to use write_seqlock_bh() instead of write_seqlock(). Found via
lockdep.

Signed-off-by: Valerie Aurora <val@versity.com>
---
 fs/rpdfs/balloc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/rpdfs/balloc.c b/fs/rpdfs/balloc.c
index 6a43a7c08aa6..ec1070b4a2d6 100644
--- a/fs/rpdfs/balloc.c
+++ b/fs/rpdfs/balloc.c
@@ -136,9 +136,9 @@ void rpdfs_balloc_publish_region(struct rpdfs_fs_info *rfi, struct rpdfs_balloc_
 		rpdfs_balloc_free_region(reg);
 		atomic_inc(&balinf->enospc_count);
 	} else {
-		write_seqlock(&balinf->seqlock);
+		write_seqlock_bh(&balinf->seqlock);
 		list_add_tail(&reg->head, &balinf->region_list);
-		write_sequnlock(&balinf->seqlock);
+		write_sequnlock_bh(&balinf->seqlock);
 	}
 	wake_up(&balinf->waitq);
 }
@@ -192,12 +192,12 @@ struct rpdfs_balloc_region *rpdfs_balloc_take_region(struct rpdfs_fs_info *rfi)
 		while_read_seqretry(&balinf->seqlock)
 			empty = list_empty(&balinf->region_list);
 		if (!empty) {
-			write_seqlock(&balinf->seqlock);
+			write_seqlock_bh(&balinf->seqlock);
 			reg = list_first_entry_or_null(&balinf->region_list,
 						       struct rpdfs_balloc_region, head);
 			if (reg)
 				list_del_init(&reg->head);
-			write_sequnlock(&balinf->seqlock);
+			write_sequnlock_bh(&balinf->seqlock);
 		}
 		if (reg)
 			break;
@@ -252,9 +252,9 @@ void rpdfs_balloc_return_region(struct rpdfs_fs_info *rfi, struct rpdfs_balloc_r
 	put_cpu_ptr(balinf->pcpu_region);
 
 	if (reg) {
-		write_seqlock(&balinf->seqlock);
+		write_seqlock_bh(&balinf->seqlock);
 		list_add(&reg->head, &balinf->region_list);
-		write_sequnlock(&balinf->seqlock);
+		write_sequnlock_bh(&balinf->seqlock);
 		wake_up(&balinf->waitq);
 	}
 }
-- 
2.49.0

[PATCH 2/3] rpdfs: Initialize block seqlock before using it and fix memleak

[Valerie Aurora]

alloc_data_page() was using bk->seqlock before it was
initialized. Move the lock initializations to before the
alloc_data_page() call and free the block struct if it fails.

Signed-off-by: Valerie Aurora <val@versity.com>
---
 fs/rpdfs/block.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/rpdfs/block.c b/fs/rpdfs/block.c
index b614d13f4a2b..825098c0de75 100644
--- a/fs/rpdfs/block.c
+++ b/fs/rpdfs/block.c
@@ -322,19 +322,20 @@ static struct rpdfs_block *alloc_block(bool with_page, gfp_t gfp)
 		goto out;
 	}
 
+	INIT_LIST_HEAD(&bk->lru_head);
+	atomic64_set(&bk->refcount, 0);
+	seqlock_init(&bk->seqlock);
+	init_waitqueue_head(&bk->waitq);
+	INIT_LIST_HEAD(&bk->dirty_head);
+
 	if (with_page) {
 		ret = alloc_data_page(bk, gfp);
 		if (ret < 0) {
+			kfree(bk);
 			bk = ERR_PTR(ret);
 			goto out;
 		}
 	}
-
-	INIT_LIST_HEAD(&bk->lru_head);
-	atomic64_set(&bk->refcount, 0);
-	seqlock_init(&bk->seqlock);
-	init_waitqueue_head(&bk->waitq);
-	INIT_LIST_HEAD(&bk->dirty_head);
 out:
 	return bk;
 }
-- 
2.49.0

[PATCH 3/3] rpdfs: Add debugging for block not found case in rpdfs_use_txn_prepared

[Valerie Aurora]

Add a bnr debug print for the case of calling rpdfs_use_txn_prepared
on a non-existent block.

Signed-off-by: Valerie Aurora <val@versity.com>
---
 fs/rpdfs/txn.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/rpdfs/txn.c b/fs/rpdfs/txn.c
index ffb6402877bf..c7b430de77af 100644
--- a/fs/rpdfs/txn.c
+++ b/fs/rpdfs/txn.c
@@ -444,6 +444,7 @@ int rpdfs_txn_use_prepared(struct rpdfs_fs_info *rfi, struct rpdfs_transaction *
 
 	tblk = get_tblk(&txn->blocks, bnr, 0, false);
 	if (WARN_ON_ONCE(IS_ERR_OR_NULL(tblk))) {
+		rpdfs_prd("bnr %llu", bnr);
 		ret = -EINVAL;
 		goto out;
 	}
-- 
2.49.0

Re: [PATCH 1/3] rpdfs: Use write_seqlock_bh() for balloc info

[Zach Brown]

On Thu, Mar 19, 2026 at 10:37:25AM +0100, Valerie Aurora wrote:
> The balloc info seqlock is taken from bh context due to
> rpdfs_balloc_publish_region() being called from an RCU callback, so it
> needs to use write_seqlock_bh() instead of write_seqlock(). Found via
> lockdep.

I'll admit, I was being a bit lazy and not running with debug kernels as
consistently as I should :).  I'll work on that.

> @@ -192,12 +192,12 @@ struct rpdfs_balloc_region *rpdfs_balloc_take_region(struct rpdfs_fs_info *rfi)
>  		while_read_seqretry(&balinf->seqlock)
>  			empty = list_empty(&balinf->region_list);
>  		if (!empty) {
> -			write_seqlock(&balinf->seqlock);
> +			write_seqlock_bh(&balinf->seqlock);

But let's do this another way.  Let's not punish all the users of the
region for the single weirdly implemented publish.

While the builder is protected by RCU, the region isn't.  It was just
bad reasoning to think that we needed to publish from call_rcu.  We only
need to free the builder there.

For publishing, first we can move the use of the region up above the
decrements of in_flight.  Then we save the result of the last in_flight
decrement for each case (initial trying to send, other responses
arriving).  Whoever won and got in_flight down to 0 can take exclusive
ownership of the region and publish it.  (The spin_unlock _release
semantics ensures that everyone's done with the region by the time they
start meessing with in_flight).

I'll that assume you want to take a swing at this but let me know if
you'd rather I clean up the mess I made.

- z

Re: [PATCH 2/3] rpdfs: Initialize block seqlock before using it and fix memleak

[Zach Brown]

On Thu, Mar 19, 2026 at 10:37:26AM +0100, Valerie Aurora wrote:
> alloc_data_page() was using bk->seqlock before it was
> initialized. Move the lock initializations to before the
> alloc_data_page() call and free the block struct if it fails.
> 
> Signed-off-by: Valerie Aurora <val@versity.com>

Applied, thanks!

- z
(donning my favourite brown paper bag over here)

Re: [PATCH 3/3] rpdfs: Add debugging for block not found case in rpdfs_use_txn_prepared

[Zach Brown]

On Thu, Mar 19, 2026 at 10:37:27AM +0100, Valerie Aurora wrote:
> Add a bnr debug print for the case of calling rpdfs_use_txn_prepared
> on a non-existent block.

Applied, though this'll go away as the txn model changes :).

- z