[bug report] btrfs: make can_nocow_extent nowait compatible

* [bug report] btrfs: make can_nocow_extent nowait compatible
@ 2022-09-23 14:55 Dan Carpenter
  2022-09-23 15:22 ` Josef Bacik
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-09-23 14:55 UTC (permalink / raw)
  To: josef; +Cc: linux-btrfs

Hello Josef Bacik,

The patch 9ec9594f6de7: "btrfs: make can_nocow_extent nowait
compatible" from Sep 12, 2022, leads to the following Smatch static
checker warning:

	fs/btrfs/extent-tree.c:2225 check_delayed_ref()
	warn: refcount leak 'cur_trans->use_count.refs.counter': lines='2225'

fs/btrfs/extent-tree.c
    2193 static noinline int check_delayed_ref(struct btrfs_root *root,
    2194                                       struct btrfs_path *path,
    2195                                       u64 objectid, u64 offset, u64 bytenr)
    2196 {
    2197         struct btrfs_delayed_ref_head *head;
    2198         struct btrfs_delayed_ref_node *ref;
    2199         struct btrfs_delayed_data_ref *data_ref;
    2200         struct btrfs_delayed_ref_root *delayed_refs;
    2201         struct btrfs_transaction *cur_trans;
    2202         struct rb_node *node;
    2203         int ret = 0;
    2204 
    2205         spin_lock(&root->fs_info->trans_lock);
    2206         cur_trans = root->fs_info->running_transaction;
    2207         if (cur_trans)
    2208                 refcount_inc(&cur_trans->use_count);
    2209         spin_unlock(&root->fs_info->trans_lock);
    2210         if (!cur_trans)
    2211                 return 0;
    2212 
    2213         delayed_refs = &cur_trans->delayed_refs;
    2214         spin_lock(&delayed_refs->lock);
    2215         head = btrfs_find_delayed_ref_head(delayed_refs, bytenr);
    2216         if (!head) {
    2217                 spin_unlock(&delayed_refs->lock);
    2218                 btrfs_put_transaction(cur_trans);
    2219                 return 0;
    2220         }
    2221 
    2222         if (!mutex_trylock(&head->mutex)) {
    2223                 if (path->nowait) {
    2224                         spin_unlock(&delayed_refs->lock);
--> 2225                         return -EAGAIN;

Call btrfs_put_transaction(cur_trans); before returning?

    2226                 }
    2227 
    2228                 refcount_inc(&head->refs);
    2229                 spin_unlock(&delayed_refs->lock);
    2230 
    2231                 btrfs_release_path(path);
    2232 
    2233                 /*
    2234                  * Mutex was contended, block until it's released and let
    2235                  * caller try again
    2236                  */
    2237                 mutex_lock(&head->mutex);
    2238                 mutex_unlock(&head->mutex);
    2239                 btrfs_put_delayed_ref_head(head);
    2240                 btrfs_put_transaction(cur_trans);
    2241                 return -EAGAIN;
    2242         }
    2243         spin_unlock(&delayed_refs->lock);
    2244 
    2245         spin_lock(&head->lock);
    2246         /*
    2247          * XXX: We should replace this with a proper search function in the
    2248          * future.
    2249          */
    2250         for (node = rb_first_cached(&head->ref_tree); node;
    2251              node = rb_next(node)) {
    2252                 ref = rb_entry(node, struct btrfs_delayed_ref_node, ref_node);
    2253                 /* If it's a shared ref we know a cross reference exists */
    2254                 if (ref->type != BTRFS_EXTENT_DATA_REF_KEY) {
    2255                         ret = 1;
    2256                         break;
    2257                 }
    2258 
    2259                 data_ref = btrfs_delayed_node_to_data_ref(ref);
    2260 
    2261                 /*
    2262                  * If our ref doesn't match the one we're currently looking at
    2263                  * then we have a cross reference.
    2264                  */
    2265                 if (data_ref->root != root->root_key.objectid ||
    2266                     data_ref->objectid != objectid ||
    2267                     data_ref->offset != offset) {
    2268                         ret = 1;
    2269                         break;
    2270                 }
    2271         }
    2272         spin_unlock(&head->lock);
    2273         mutex_unlock(&head->mutex);
    2274         btrfs_put_transaction(cur_trans);
    2275         return ret;
    2276 }

regards,
dan carpenter

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