Skip to contentSkip to conversation

blkcg: add rcu lock to bio_clone_blkg_association()

PATCH In review Dennis Zhou 2 participants 4 messages Dec 21, 2018 Block layer

lore
Share & tools
lore

Dennis Zhou

author
#0 lore raw

PATCH 1/1 blkcg: add rcu lock to bio_clone_blkg_association()

I cleaned up blkg_tryget_closest() to require rcu_read_lock() earlier. However, this was a subtle case too which clearly was too subtle for me. The idea was the src bio should be holding a ref to the blkg so rcu wasn't technically needed. If it doesn't hold a ref, it should be %NULL and the blkg->parent pointers are unused.

This adds the appropriate read lock in bio_clone_blkg_association().

Fixes 80fd3c272c1a ("blkcg: clean up blkg_tryget_closest()")
Reported-by [email protected]
Signed-off-by Dennis Zhou <[email protected]>
---
 block/bio.c | 4 ++++
 1 file changed, 4 insertions(+)
block/bio.c +4 -0
index c288b9057042..9194d8ad3d5e 100644
@@ -2096,8 +2096,12 @@ EXPORT_SYMBOL_GPL(bio_associate_blkg);
  */
 void bio_clone_blkg_association(struct bio *dst, struct bio *src)
 {
+	rcu_read_lock();
+
 	if (src->bi_blkg)
 		__bio_associate_blkg(dst, src->bi_blkg);
+
+	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(bio_clone_blkg_association);
 #endif /* CONFIG_BLK_CGROUP */
Signature · 1 line
2.17.1
maintainer
#1 lore raw
Quoted 7 lines · Dennis Zhou

I cleaned up blkg_tryget_closest() to require rcu_read_lock() earlier. However, this was a subtle case too which clearly was too subtle for me. The idea was the src bio should be holding a ref to the blkg so rcu wasn't technically needed. If it doesn't hold a ref, it should be %NULL and the blkg->parent pointers are unused.

This adds the appropriate read lock in bio_clone_blkg_association().

Shall I just fold this with the previous? I staged it in a later-in-merge-cycle branch, so that's not an issue to amend.

Signature · 1 line
Jens Axboe
author
#2 lore raw
Quoted 7 lines · Jens Axboe
Quoted 6 lines · Dennis Zhou

I cleaned up blkg_tryget_closest() to require rcu_read_lock() earlier. However, this was a subtle case too which clearly was too subtle for me. The idea was the src bio should be holding a ref to the blkg so rcu wasn't technically needed. If it doesn't hold a ref, it should be %NULL and the blkg->parent pointers are unused.

Quoted 5 lines
Quoted 1 line

This adds the appropriate read lock in bio_clone_blkg_association().

Shall I just fold this with the previous? I staged it in a later-in-merge-cycle branch, so that's not an issue to amend.

Yeah that would be great!

Thanks,
Dennis

maintainer
#3 lore raw
Quoted 8 lines · Dennis Zhou
Quoted 7 lines · Jens Axboe
Quoted 6 lines · Dennis Zhou

I cleaned up blkg_tryget_closest() to require rcu_read_lock() earlier. However, this was a subtle case too which clearly was too subtle for me. The idea was the src bio should be holding a ref to the blkg so rcu wasn't technically needed. If it doesn't hold a ref, it should be %NULL and the blkg->parent pointers are unused.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

This adds the appropriate read lock in bio_clone_blkg_association().

Shall I just fold this with the previous? I staged it in a later-in-merge-cycle branch, so that's not an issue to amend.

Yeah that would be great!

Done

Signature · 1 line
Jens Axboe