From: Dan Carpenter <dan.carpenter@oracle.com>
To: jeffm@suse.com
Cc: linux-btrfs@vger.kernel.org
Subject: re: btrfs: add tracing for failed reservations
Date: Tue, 10 Nov 2015 12:08:27 +0300 [thread overview]
Message-ID: <20151110090827.GA2934@mwanda> (raw)
[ This is old. I don't know why it's only complaining now. Anyway this
looks like one of those rarely used code paths so it might be a real
bug. ]
Hello Jeff Mahoney,
The patch cab45e22da48: "btrfs: add tracing for failed reservations"
from Oct 16, 2013, leads to the following static checker warning:
fs/btrfs/extent-tree.c:4116 btrfs_alloc_data_chunk_ondemand()
error: we previously assumed 'data_sinfo' could be null (see line 4016)
fs/btrfs/extent-tree.c
4014
4015 data_sinfo = fs_info->data_sinfo;
4016 if (!data_sinfo)
^^^^^^^^^^^
Check for NULL.
4017 goto alloc;
4018
4019 again:
4020 /* make sure we have enough space to handle the data first */
4021 spin_lock(&data_sinfo->lock);
4022 used = data_sinfo->bytes_used + data_sinfo->bytes_reserved +
4023 data_sinfo->bytes_pinned + data_sinfo->bytes_readonly +
4024 data_sinfo->bytes_may_use;
4025
4026 if (used + bytes > data_sinfo->total_bytes) {
4027 struct btrfs_trans_handle *trans;
4028
4029 /*
4030 * if we don't have enough free bytes in this space then we need
4031 * to alloc a new chunk.
4032 */
4033 if (!data_sinfo->full) {
4034 u64 alloc_target;
4035
4036 data_sinfo->force_alloc = CHUNK_ALLOC_FORCE;
4037 spin_unlock(&data_sinfo->lock);
4038 alloc:
We jump to here.
4039 alloc_target = btrfs_get_alloc_profile(root, 1);
4040 /*
4041 * It is ugly that we don't call nolock join
4042 * transaction for the free space inode case here.
4043 * But it is safe because we only do the data space
4044 * reservation for the free space cache in the
4045 * transaction context, the common join transaction
4046 * just increase the counter of the current transaction
4047 * handler, doesn't try to acquire the trans_lock of
4048 * the fs.
4049 */
4050 trans = btrfs_join_transaction(root);
4051 if (IS_ERR(trans))
4052 return PTR_ERR(trans);
4053
4054 ret = do_chunk_alloc(trans, root->fs_info->extent_root,
4055 alloc_target,
4056 CHUNK_ALLOC_NO_FORCE);
4057 btrfs_end_transaction(trans, root);
4058 if (ret < 0) {
4059 if (ret != -ENOSPC)
4060 return ret;
4061 else {
4062 have_pinned_space = 1;
4063 goto commit_trans;
The btrfs_end_transaction() call fails.
4064 }
4065 }
4066
4067 if (!data_sinfo)
4068 data_sinfo = fs_info->data_sinfo;
4069
4070 goto again;
4071 }
4072
4073 /*
4074 * If we don't have enough pinned space to deal with this
4075 * allocation, and no removed chunk in current transaction,
4076 * don't bother committing the transaction.
4077 */
4078 have_pinned_space = percpu_counter_compare(
4079 &data_sinfo->total_bytes_pinned,
4080 used + bytes - data_sinfo->total_bytes);
4081 spin_unlock(&data_sinfo->lock);
4082
4083 /* commit the current transaction and try again */
4084 commit_trans:
We jump to here.
4085 if (need_commit &&
4086 !atomic_read(&root->fs_info->open_ioctl_trans)) {
4087 need_commit--;
4088
4089 if (need_commit > 0)
4090 btrfs_wait_ordered_roots(fs_info, -1);
4091
4092 trans = btrfs_join_transaction(root);
4093 if (IS_ERR(trans))
4094 return PTR_ERR(trans);
4095 if (have_pinned_space >= 0 ||
4096 test_bit(BTRFS_TRANS_HAVE_FREE_BGS,
4097 &trans->transaction->flags) ||
4098 need_commit > 0) {
4099 ret = btrfs_commit_transaction(trans, root);
4100 if (ret)
4101 return ret;
4102 /*
4103 * make sure that all running delayed iput are
4104 * done
4105 */
4106 down_write(&root->fs_info->delayed_iput_sem);
4107 up_write(&root->fs_info->delayed_iput_sem);
4108 goto again;
4109 } else {
4110 btrfs_end_transaction(trans, root);
4111 }
4112 }
4113
4114 trace_btrfs_space_reservation(root->fs_info,
4115 "space_info:enospc",
4116 data_sinfo->flags, bytes, 1);
^^^^^^^^^^^^^^^^^
Potential NULL deref.
4117 return -ENOSPC;
regards,
dan carpenter
next reply other threads:[~2015-11-10 9:08 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-10 9:08 Dan Carpenter [this message]
2015-11-10 13:55 ` btrfs: add tracing for failed reservations Jeff Mahoney
-- strict thread matches above, loose matches on Subject: below --
2013-11-13 20:01 Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151110090827.GA2934@mwanda \
--to=dan.carpenter@oracle.com \
--cc=jeffm@suse.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).