| Message ID | 20180625163822.25681-1-dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25.06.2018 19:38, David Sterba wrote: > There are several WARN_ON calls that catch incrorrect reference counter > updates, but this is what the refcount_t does already: > > * refcount_inc from 0 will warn > * refcount_dec_and_test from 0 will warn > But these warnings are only going to be triggered in the case of CONFIG_REFCOUNT_FULL otherwise refcount falls back to plain atomic operations. > Signed-off-by: David Sterba <dsterba@suse.com> > --- > fs/btrfs/delayed-ref.h | 1 - > fs/btrfs/extent_map.c | 2 +- > fs/btrfs/transaction.c | 1 - > fs/btrfs/volumes.c | 1 - > 4 files changed, 1 insertion(+), 4 deletions(-) > > diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h > index ea1aecb6a50d..16ee92c541bf 100644 > --- a/fs/btrfs/delayed-ref.h > +++ b/fs/btrfs/delayed-ref.h > @@ -210,7 +210,6 @@ btrfs_free_delayed_extent_op(struct btrfs_delayed_extent_op *op) > > static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref) > { > - WARN_ON(refcount_read(&ref->refs) == 0); > if (refcount_dec_and_test(&ref->refs)) { > WARN_ON(ref->in_tree); > switch (ref->type) { > diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c > index 6648d55e5339..6808ad4ed3c9 100644 > --- a/fs/btrfs/extent_map.c > +++ b/fs/btrfs/extent_map.c > @@ -72,7 +72,7 @@ void free_extent_map(struct extent_map *em) > { > if (!em) > return; > - WARN_ON(refcount_read(&em->refs) == 0); > + > if (refcount_dec_and_test(&em->refs)) { > WARN_ON(extent_map_in_tree(em)); > WARN_ON(!list_empty(&em->list)); > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > index 4485eae41e88..f6b68099b767 100644 > --- a/fs/btrfs/transaction.c > +++ b/fs/btrfs/transaction.c > @@ -41,7 +41,6 @@ static const unsigned int btrfs_blocked_trans_types[TRANS_STATE_MAX] = { > > void btrfs_put_transaction(struct btrfs_transaction *transaction) > { > - WARN_ON(refcount_read(&transaction->use_count) == 0); > if (refcount_dec_and_test(&transaction->use_count)) { > BUG_ON(!list_empty(&transaction->list)); > WARN_ON(!RB_EMPTY_ROOT(&transaction->delayed_refs.href_root)); > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index e034ad9e23b4..f64d26b4f278 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -5330,7 +5330,6 @@ static struct btrfs_bio *alloc_btrfs_bio(int total_stripes, int real_stripes) > > void btrfs_get_bbio(struct btrfs_bio *bbio) > { > - WARN_ON(!refcount_read(&bbio->refs)); > refcount_inc(&bbio->refs); > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 26, 2018 at 05:00:46PM +0300, Nikolay Borisov wrote: > On 25.06.2018 19:38, David Sterba wrote: > > There are several WARN_ON calls that catch incrorrect reference counter > > updates, but this is what the refcount_t does already: > > > > * refcount_inc from 0 will warn > > * refcount_dec_and_test from 0 will warn > > > > But these warnings are only going to be triggered in the case of > CONFIG_REFCOUNT_FULL otherwise refcount falls back to plain atomic > operations. Right, I'm for REFCOUNT_FULL on by default obviously, but we need to take handle the case where it's not. And there isn't a refcount type with the full semantics. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 29, 2018 at 04:05:40PM +0200, David Sterba wrote: > On Tue, Jun 26, 2018 at 05:00:46PM +0300, Nikolay Borisov wrote: > > On 25.06.2018 19:38, David Sterba wrote: > > > There are several WARN_ON calls that catch incrorrect reference counter > > > updates, but this is what the refcount_t does already: > > > > > > * refcount_inc from 0 will warn > > > * refcount_dec_and_test from 0 will warn > > > > > > > But these warnings are only going to be triggered in the case of > > CONFIG_REFCOUNT_FULL otherwise refcount falls back to plain atomic > > operations. > > Right, I'm for REFCOUNT_FULL on by default obviously, but we need to > take handle the case where it's not. And there isn't a refcount type > with the full semantics. There refcount helpers will be extended to allow the full check semantics so I'll use them and drop the WARN_ONs once the patch gets merged. https://lore.kernel.org/lkml/20180703100102.16615-1-mark.rutland@arm.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h index ea1aecb6a50d..16ee92c541bf 100644 --- a/fs/btrfs/delayed-ref.h +++ b/fs/btrfs/delayed-ref.h @@ -210,7 +210,6 @@ btrfs_free_delayed_extent_op(struct btrfs_delayed_extent_op *op) static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref) { - WARN_ON(refcount_read(&ref->refs) == 0); if (refcount_dec_and_test(&ref->refs)) { WARN_ON(ref->in_tree); switch (ref->type) { diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c index 6648d55e5339..6808ad4ed3c9 100644 --- a/fs/btrfs/extent_map.c +++ b/fs/btrfs/extent_map.c @@ -72,7 +72,7 @@ void free_extent_map(struct extent_map *em) { if (!em) return; - WARN_ON(refcount_read(&em->refs) == 0); + if (refcount_dec_and_test(&em->refs)) { WARN_ON(extent_map_in_tree(em)); WARN_ON(!list_empty(&em->list)); diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index 4485eae41e88..f6b68099b767 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -41,7 +41,6 @@ static const unsigned int btrfs_blocked_trans_types[TRANS_STATE_MAX] = { void btrfs_put_transaction(struct btrfs_transaction *transaction) { - WARN_ON(refcount_read(&transaction->use_count) == 0); if (refcount_dec_and_test(&transaction->use_count)) { BUG_ON(!list_empty(&transaction->list)); WARN_ON(!RB_EMPTY_ROOT(&transaction->delayed_refs.href_root)); diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index e034ad9e23b4..f64d26b4f278 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -5330,7 +5330,6 @@ static struct btrfs_bio *alloc_btrfs_bio(int total_stripes, int real_stripes) void btrfs_get_bbio(struct btrfs_bio *bbio) { - WARN_ON(!refcount_read(&bbio->refs)); refcount_inc(&bbio->refs); }
There are several WARN_ON calls that catch incrorrect reference counter updates, but this is what the refcount_t does already: * refcount_inc from 0 will warn * refcount_dec_and_test from 0 will warn Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/delayed-ref.h | 1 - fs/btrfs/extent_map.c | 2 +- fs/btrfs/transaction.c | 1 - fs/btrfs/volumes.c | 1 - 4 files changed, 1 insertion(+), 4 deletions(-)