[PATCH] btrfs: trace: Allow trace_qgroup_update_counters() to record old rfer/excl value
Origin trace_qgroup_update_counters() only records qgroup id and its
reference count change.
It's good enough to debug qgroup accounting change, but when rescan race
is involved, it's pretty hard to distinguish which modification belongs
to which rescan.
So add old_rfer and old_excl trace output to help distinguishing
different rescan instance.
(Different rescan instance should reset its qgroup->rfer to 0)
For trace event parameter, it just changes from u64 qgroup_id to struct
btrfs_qgroup *qgroup, so number of parameters is not changed at all.
Signed-off-by: Qu Wenruo
---
fs/btrfs/qgroup.c| 4 ++--
include/trace/events/btrfs.h | 18 +++---
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 9fb758d5077a..ec2339a49ec3 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1882,8 +1882,8 @@ static int qgroup_update_counters(struct btrfs_fs_info
*fs_info,
cur_old_count = btrfs_qgroup_get_old_refcnt(qg, seq);
cur_new_count = btrfs_qgroup_get_new_refcnt(qg, seq);
- trace_qgroup_update_counters(fs_info, qg->qgroupid,
-cur_old_count, cur_new_count);
+ trace_qgroup_update_counters(fs_info, qg, cur_old_count,
+cur_new_count);
/* Rfer update part */
if (cur_old_count == 0 && cur_new_count > 0) {
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 38f444bf55fe..cfcbcd2eb338 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -1607,27 +1607,31 @@ TRACE_EVENT(btrfs_qgroup_account_extent,
TRACE_EVENT(qgroup_update_counters,
- TP_PROTO(const struct btrfs_fs_info *fs_info, u64 qgid,
+ TP_PROTO(const struct btrfs_fs_info *fs_info,
+struct btrfs_qgroup *qgroup,
u64 cur_old_count, u64 cur_new_count),
- TP_ARGS(fs_info, qgid, cur_old_count, cur_new_count),
+ TP_ARGS(fs_info, qgroup, cur_old_count, cur_new_count),
TP_STRUCT__entry_btrfs(
__field(u64, qgid )
+ __field(u64, old_rfer )
+ __field(u64, old_excl )
__field(u64, cur_old_count )
__field(u64, cur_new_count )
),
TP_fast_assign_btrfs(fs_info,
- __entry->qgid = qgid;
+ __entry->qgid = qgroup->qgroupid;
+ __entry->old_rfer = qgroup->rfer;
+ __entry->old_excl = qgroup->excl;
__entry->cur_old_count = cur_old_count;
__entry->cur_new_count = cur_new_count;
),
- TP_printk_btrfs("qgid=%llu cur_old_count=%llu cur_new_count=%llu",
- __entry->qgid,
- __entry->cur_old_count,
- __entry->cur_new_count)
+ TP_printk_btrfs("qgid=%llu old_rfer=%llu old_excl=%llu
cur_old_count=%llu cur_new_count=%llu",
+ __entry->qgid, __entry->old_rfer, __entry->old_excl,
+ __entry->cur_old_count, __entry->cur_new_count)
);
TRACE_EVENT(qgroup_update_reserve,
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] btrfs: fix BUG trying to resume balance without resume flag
We set the BTRFS_BALANCE_RESUME flag in the btrfs_recover_balance(), which is not called during the remount. So when resuming from the paused balance we hit BUG. kernel: kernel BUG at fs/btrfs/volumes.c:3890! :: kernel: balance_kthread+0x51/0x60 [btrfs] kernel: kthread+0x111/0x130 :: kernel: RIP: btrfs_balance+0x12e1/0x1570 [btrfs] RSP: ba7d0090bde8 Reproducer: On a mounted BTRFS. btrfs balance start --full-balance /btrfs btrfs balance pause /btrfs mount -o remount,ro /dev/sdb /btrfs mount -o remount,rw /dev/sdb /btrfs To fix this set the BTRFS_BALANCE_RESUME flag in btrfs_resume_balance_async() instead of btrfs_recover_balance(). Signed-off-by: Anand Jain --- v1->v2: btrfs_resume_balance_async() can be called only from remount or mount, we don't need to hold fs_info->balance_lock. Strictly speaking we should rather keep the balance at the paused state unless it is resumed by the user again, that means neither mount nor remount-rw should resume the balance automatically, former case needs writing balance status to the disk. Which needs compatibility verification. So for now just avoid BUG. fs/btrfs/volumes.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 3e6983a169c4..64bcaf25908b 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -4115,6 +4115,8 @@ int btrfs_resume_balance_async(struct btrfs_fs_info *fs_info) return 0; } + fs_info->balance_ctl->b_flags |= BTRFS_BALANCE_RESUME; + tsk = kthread_run(balance_kthread, fs_info, "btrfs-balance"); return PTR_ERR_OR_ZERO(tsk); } @@ -4156,7 +4158,6 @@ int btrfs_recover_balance(struct btrfs_fs_info *fs_info) bctl->fs_info = fs_info; bctl->flags = btrfs_balance_flags(leaf, item); - bctl->flags |= BTRFS_BALANCE_RESUME; btrfs_balance_data(leaf, item, &disk_bargs); btrfs_disk_balance_args_to_cpu(&bctl->data, &disk_bargs); -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs: rename btrfs_balance_control::flags to b_flags
As flags is a generic name, its often difficult to search, rename
btrfs_balance_control::flags to btrfs_balance_control::b_flags.
Signed-off-by: Anand Jain
---
fs/btrfs/ioctl.c | 10 +-
fs/btrfs/volumes.c | 30 +++---
fs/btrfs/volumes.h | 2 +-
3 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e1dedc78d1ad..dbfb6d691275 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4310,7 +4310,7 @@ void btrfs_update_ioctl_balance_args(struct btrfs_fs_info
*fs_info,
{
struct btrfs_balance_control *bctl = fs_info->balance_ctl;
- bargs->flags = bctl->flags;
+ bargs->flags = bctl->b_flags;
if (test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags))
bargs->state |= BTRFS_BALANCE_STATE_RUNNING;
@@ -4408,7 +4408,7 @@ static long btrfs_ioctl_balance(struct file *file, void
__user *arg)
bctl = fs_info->balance_ctl;
spin_lock(&fs_info->balance_lock);
- bctl->flags |= BTRFS_BALANCE_RESUME;
+ bctl->b_flags |= BTRFS_BALANCE_RESUME;
spin_unlock(&fs_info->balance_lock);
goto do_balance;
@@ -4434,13 +4434,13 @@ static long btrfs_ioctl_balance(struct file *file, void
__user *arg)
memcpy(&bctl->meta, &bargs->meta, sizeof(bctl->meta));
memcpy(&bctl->sys, &bargs->sys, sizeof(bctl->sys));
- bctl->flags = bargs->flags;
+ bctl->b_flags = bargs->flags;
} else {
/* balance everything - no filters */
- bctl->flags |= BTRFS_BALANCE_TYPE_MASK;
+ bctl->b_flags |= BTRFS_BALANCE_TYPE_MASK;
}
- if (bctl->flags & ~(BTRFS_BALANCE_ARGS_MASK | BTRFS_BALANCE_TYPE_MASK))
{
+ if (bctl->b_flags & ~(BTRFS_BALANCE_ARGS_MASK |
BTRFS_BALANCE_TYPE_MASK)) {
ret = -EINVAL;
goto out_bctl;
}
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 64bcaf25908b..1326e8e5fc88 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3136,7 +3136,7 @@ static int insert_balance_item(struct btrfs_fs_info
*fs_info,
btrfs_cpu_balance_args_to_disk(&disk_bargs, &bctl->sys);
btrfs_set_balance_sys(leaf, item, &disk_bargs);
- btrfs_set_balance_flags(leaf, item, bctl->flags);
+ btrfs_set_balance_flags(leaf, item, bctl->b_flags);
btrfs_mark_buffer_dirty(leaf);
out:
@@ -3435,7 +3435,7 @@ static int should_balance_chunk(struct btrfs_fs_info
*fs_info,
/* type filter */
if (!((chunk_type & BTRFS_BLOCK_GROUP_TYPE_MASK) &
- (bctl->flags & BTRFS_BALANCE_TYPE_MASK))) {
+ (bctl->b_flags & BTRFS_BALANCE_TYPE_MASK))) {
return 0;
}
@@ -3872,27 +3872,27 @@ static void print_balance_start_or_resume(struct
btrfs_fs_info *fs_info)
return;
}
- if (bctl->flags & BTRFS_BALANCE_ARGS_SOFT) {
+ if (bctl->b_flags & BTRFS_BALANCE_ARGS_SOFT) {
strcat(args, "soft ");
}
- if (bctl->flags & BTRFS_BALANCE_FORCE) {
+ if (bctl->b_flags & BTRFS_BALANCE_FORCE) {
strcat(args, "force ");
}
- if (bctl->flags & BTRFS_BALANCE_DATA) {
+ if (bctl->b_flags & BTRFS_BALANCE_DATA) {
strcat(args, "data ");
get_balance_args(&bctl->data, args);
}
- if (bctl->flags & BTRFS_BALANCE_METADATA) {
+ if (bctl->b_flags & BTRFS_BALANCE_METADATA) {
if (strlen(args) > 0)
strcat(args, " ");
strcat(args, "metadata ");
get_balance_args(&bctl->meta, args);
}
- if (bctl->flags & BTRFS_BALANCE_SYSTEM) {
+ if (bctl->b_flags & BTRFS_BALANCE_SYSTEM) {
if (strlen(args) > 0)
strcat(args, " ");
strcat(args, "system ");
@@ -3901,7 +3901,7 @@ static void print_balance_start_or_resume(struct
btrfs_fs_info *fs_info)
BUG_ON(strlen(args) > log_size);
btrfs_info(fs_info, "%s %s",
- bctl->flags & BTRFS_BALANCE_RESUME ?\
+ bctl->b_flags & BTRFS_BALANCE_RESUME ?\
"balance: resume":"balance: start", args);
kfree(args);
@@ -3937,9 +3937,9 @@ int btrfs_balance(struct btrfs_balance_control *bctl,
* and identical options should be given for both of them.
*/
allowed = BTRFS_BALANCE_DATA | BTRFS_BALANCE_METADATA;
- if (mixed && (bctl->flags & allowed)) {
- if (!(bctl->flags & BTRFS_BALANCE_DATA) ||
- !(bctl->flags & BTRFS_BALANCE_METADATA) ||
+ if (mixed && (bctl->b_flags & allowed)) {
+ if (!(bctl->b_flags & BTRFS_BALANCE_DATA) ||
+ !(bctl->b_flags & BTRFS_BALANCE_METADATA) ||
Re: [PATCH 1/3] btrfs: qgroups, fix rescan worker running races
On 4/30/18 2:20 AM, Qu Wenruo wrote:
>
>
> On 2018年04月27日 03:23, je...@suse.com wrote:
>> From: Jeff Mahoney
>>
>> Commit d2c609b834d6 (Btrfs: fix qgroup rescan worker initialization)
>> fixed the issue with BTRFS_IOC_QUOTA_RESCAN_WAIT being racy, but
>> ended up reintroducing the hang-on-unmount bug that the commit it
>> intended to fix addressed.
>>
>> The race this time is between qgroup_rescan_init setting
>> ->qgroup_rescan_running = true and the worker starting. There are
>> many scenarios where we initialize the worker and never start it. The
>> completion btrfs_ioctl_quota_rescan_wait waits for will never come.
>> This can happen even without involving error handling, since mounting
>> the file system read-only returns between initializing the worker and
>> queueing it.
>>
>> The right place to do it is when we're queuing the worker. The flag
>> really just means that btrfs_ioctl_quota_rescan_wait should wait for
>> a completion.
>>
>> This patch introduces a new helper, queue_rescan_worker, that handles
>> the ->qgroup_rescan_running flag, including any races with umount.
>>
>> While we're at it, ->qgroup_rescan_running is protected only by the
>> ->qgroup_rescan_mutex. btrfs_ioctl_quota_rescan_wait doesn't need
>> to take the spinlock too.
>>
>> Fixes: d2c609b834d6 (Btrfs: fix qgroup rescan worker initialization)
>> Signed-off-by: Jeff Mahoney
>
> A little off-topic, (thanks Nikolay for reporting this) sometimes
> btrfs/017 could report qgroup corruption, and it turns out it's related
> to rescan racy, which double account existing tree blocks twice.
> (One by btrfs quota enable, another by btrfs quota rescan -w)
>
> Would this patch help in such case?
It shouldn't. This only fixes races between the rescan worker getting
initialized and running vs waiting for it to complete.
-Jeff
>> fs/btrfs/ctree.h | 1 +
>> fs/btrfs/qgroup.c | 40
>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index da308774b8a4..dbba615f4d0f 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -1045,6 +1045,7 @@ struct btrfs_fs_info {
>> struct btrfs_workqueue *qgroup_rescan_workers;
>> struct completion qgroup_rescan_completion;
>> struct btrfs_work qgroup_rescan_work;
>> +/* qgroup rescan worker is running or queued to run */
>> bool qgroup_rescan_running; /* protected by qgroup_rescan_lock */
>>
>> /* filesystem state */
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index aa259d6986e1..be491b6c020a 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -2072,6 +2072,30 @@ int btrfs_qgroup_account_extents(struct
>> btrfs_trans_handle *trans,
>> return ret;
>> }
>>
>> +static void queue_rescan_worker(struct btrfs_fs_info *fs_info)
>> +{
>> +mutex_lock(&fs_info->qgroup_rescan_lock);
>> +if (btrfs_fs_closing(fs_info)) {
>> +mutex_unlock(&fs_info->qgroup_rescan_lock);
>> +return;
>> +}
>> +if (WARN_ON(fs_info->qgroup_rescan_running)) {
>> +btrfs_warn(fs_info, "rescan worker already queued");
>> +mutex_unlock(&fs_info->qgroup_rescan_lock);
>> +return;
>> +}
>> +
>> +/*
>> + * Being queued is enough for btrfs_qgroup_wait_for_completion
>> + * to need to wait.
>> + */
>> +fs_info->qgroup_rescan_running = true;
>> +mutex_unlock(&fs_info->qgroup_rescan_lock);
>> +
>> +btrfs_queue_work(fs_info->qgroup_rescan_workers,
>> + &fs_info->qgroup_rescan_work);
>> +}
>> +
>> /*
>> * called from commit_transaction. Writes all changed qgroups to disk.
>> */
>> @@ -2123,8 +2147,7 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans,
>> ret = qgroup_rescan_init(fs_info, 0, 1);
>> if (!ret) {
>> qgroup_rescan_zero_tracking(fs_info);
>> -btrfs_queue_work(fs_info->qgroup_rescan_workers,
>> - &fs_info->qgroup_rescan_work);
>> +queue_rescan_worker(fs_info);
>> }
>> ret = 0;
>> }
>> @@ -2713,7 +2736,6 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64
>> progress_objectid,
>> sizeof(fs_info->qgroup_rescan_progress));
>> fs_info->qgroup_rescan_progress.objectid = progress_objectid;
>> init_completion(&fs_info->qgroup_rescan_completion);
>> -fs_info->qgroup_rescan_running = true;
>>
>> spin_unlock(&fs_info->qgroup_lock);
>> mutex_unlock(&fs_info->qgroup_rescan_lock);
>> @@ -2785,9 +2807,7 @@ btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info)
>>
>> qgroup_rescan_zero_tracking(fs_info);
>>
>> -btrfs_queue_work(fs_info->qgroup_rescan_workers,
>> - &fs_info->qgroup_rescan_work);
>> -
>> +queue_rescan_worker(fs_info);
>> return 0;
>> }
>>
>> @@ -2798,9 +2818,7 @@ int btrfs_qgroup_wait_
Re: [PATCH v2] btrfs: fix BUG trying to resume balance without resume flag
Hi Anand,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on btrfs/next]
[also build test ERROR on v4.17-rc3 next-20180430]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Anand-Jain/btrfs-fix-BUG-trying-to-resume-balance-without-resume-flag/20180430-192532
base: https://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git
next
config: i386-randconfig-x000-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
fs//btrfs/volumes.c: In function 'btrfs_resume_balance_async':
>> fs//btrfs/volumes.c:3971:24: error: 'struct btrfs_balance_control' has no
>> member named 'b_flags'; did you mean 'flags'?
fs_info->balance_ctl->b_flags |= BTRFS_BALANCE_RESUME;
^~~
flags
vim +3971 fs//btrfs/volumes.c
3954
3955 int btrfs_resume_balance_async(struct btrfs_fs_info *fs_info)
3956 {
3957 struct task_struct *tsk;
3958
3959 spin_lock(&fs_info->balance_lock);
3960 if (!fs_info->balance_ctl) {
3961 spin_unlock(&fs_info->balance_lock);
3962 return 0;
3963 }
3964 spin_unlock(&fs_info->balance_lock);
3965
3966 if (btrfs_test_opt(fs_info, SKIP_BALANCE)) {
3967 btrfs_info(fs_info, "force skipping balance");
3968 return 0;
3969 }
3970
> 3971 fs_info->balance_ctl->b_flags |= BTRFS_BALANCE_RESUME;
3972
3973 tsk = kthread_run(balance_kthread, fs_info, "btrfs-balance");
3974 return PTR_ERR_OR_ZERO(tsk);
3975 }
3976
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH v2] btrfs: fix BUG trying to resume balance without resume flag
Hi Anand,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on btrfs/next]
[also build test ERROR on v4.17-rc3 next-20180430]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Anand-Jain/btrfs-fix-BUG-trying-to-resume-balance-without-resume-flag/20180430-192532
base: https://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git
next
config: i386-randconfig-a1-201817 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
fs/btrfs/volumes.c: In function 'btrfs_resume_balance_async':
>> fs/btrfs/volumes.c:3971:22: error: 'struct btrfs_balance_control' has no
>> member named 'b_flags'
fs_info->balance_ctl->b_flags |= BTRFS_BALANCE_RESUME;
^
vim +3971 fs/btrfs/volumes.c
3954
3955 int btrfs_resume_balance_async(struct btrfs_fs_info *fs_info)
3956 {
3957 struct task_struct *tsk;
3958
3959 spin_lock(&fs_info->balance_lock);
3960 if (!fs_info->balance_ctl) {
3961 spin_unlock(&fs_info->balance_lock);
3962 return 0;
3963 }
3964 spin_unlock(&fs_info->balance_lock);
3965
3966 if (btrfs_test_opt(fs_info, SKIP_BALANCE)) {
3967 btrfs_info(fs_info, "force skipping balance");
3968 return 0;
3969 }
3970
> 3971 fs_info->balance_ctl->b_flags |= BTRFS_BALANCE_RESUME;
3972
3973 tsk = kthread_run(balance_kthread, fs_info, "btrfs-balance");
3974 return PTR_ERR_OR_ZERO(tsk);
3975 }
3976
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: Inconsistent behavior of fsync in btrfs
On 29 Apr 2018, at 18:16, Theodore Y. Ts'o wrote: On Sun, Apr 29, 2018 at 03:55:39PM -0500, Vijay Chidambaram wrote: In the spirit of clarifying fsync behavior, we have one more case where we'd like to find out what should be expected. Consider this: Mkdir A Creat A/bar Fsync A/bar Rename A to B Fsync B/bar -- Crash -- A/bar has been fsynced previously, so its not a newly created file. After the crash, in ext4 and btrfs, can we expect directory B and B/bar to exist? or ext4, no. The POSIX semantics apply: bar will *either* be in A, or in B. Same for btrfs. If the rename for B goes down, it'll be a side effect of other decisions and not on purpose. I'd actually like for the rename to be on disk in the normal case, but we won't always be able to catch it. If you modify the file bar such that the mod time has been updated, then fsync(2) --- but not necessarily fdatasync(2) --- will cause the inode modifications to be written committed, and this will cause the updates to directory B from the rename to be committed as a side-effect. Note though that there are plenty of people who consider this to be a performance bug, and not a feature, and there have been papers proposed by your fellow academics that if implemented, would change this to no longer be true. In general with these sorts of things it would be useful to reason about this in the context of real world applications and why they want such guarantees. These guarantees can cost performance hits, and so there is a cost/benefit tradeoff involved. So my preference is to negotiate with applicationt writes, and ask *why* they want such guarantees, and to explore whether there better ways of achieving their high level goals before we legislate this to be an iron-clad commitment which might application A happy, but performance-seeking user B unhappy. I know this is not POSIX compliant, but from prior comments, it seems like both ext4 and btrfs would like to persist directory entries upon fsync of newly created files. So we were wondering if this extended to this case. We had real world examples of users/applications who suffered data loss when the directory entries for newly created files were not persisted. It was on the basis of these complaints that we made this commitment, since it seemed more important than the relatively minor performance hit. Agreeing with Ted and expanding a bit. If fsync(some file) doesn't persist the name for that file, applications need to fsync the directories, which can be double the log commits. Getting everything down to disk in one fsync() is much better for both the application and the FS. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3 v2] btrfs-progs: build: detect whether -std=gnu90 is supported
From: Jeff Mahoney
GCC releases prior to 4.5.0 don't support -std=gnu90 so btrfs-progs won't
build at all on older distros. We can detect whether the compiler
supports -std=gnu90 and fall back to -std=gnu89 if it doesn't.
AX_CHECK_COMPILE_FLAG is the right way to do this, but it depends on
autoconf 2.64. AX_GCC_VERSION has been deprecated, so we'll use that
only for earlier autoconf versions so we can drop it when we drop
support for older autoconf releases.
Signed-off-by: Jeff Mahoney
---
Makefile| 1 -
Makefile.inc.in | 1 +
configure.ac| 1 +
m4/ax_check_compile_flag.m4 | 74 +
m4/ax_gcc_version.m4| 37 +++
m4/btrfs_detect_cstd.m4 | 20
6 files changed, 133 insertions(+), 1 deletion(-)
create mode 100644 m4/ax_check_compile_flag.m4
create mode 100644 m4/ax_gcc_version.m4
create mode 100644 m4/btrfs_detect_cstd.m4
diff --git a/Makefile b/Makefile
index 5ba76d2..c0eb3d6 100644
--- a/Makefile
+++ b/Makefile
@@ -63,7 +63,6 @@ ABSTOPDIR = $(shell pwd)
TOPDIR := .
# Common build flags
-CSTD = -std=gnu90
CFLAGS = $(SUBST_CFLAGS) \
$(CSTD) \
-include config.h \
diff --git a/Makefile.inc.in b/Makefile.inc.in
index 52410f6..fb32461 100644
--- a/Makefile.inc.in
+++ b/Makefile.inc.in
@@ -4,6 +4,7 @@
export
CC = @CC@
+CSTD = @BTRFS_CSTD_FLAGS@
LN_S = @LN_S@
AR = @AR@
RM = @RM@
diff --git a/configure.ac b/configure.ac
index 9e5603b..4f63bb6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -26,6 +26,7 @@ AC_CONFIG_SRCDIR([btrfs.c])
AC_PREFIX_DEFAULT([/usr/local])
AC_PROG_CC
+BTRFS_DETECT_CSTD
AC_CANONICAL_HOST
AC_C_CONST
AC_C_VOLATILE
diff --git a/m4/ax_check_compile_flag.m4 b/m4/ax_check_compile_flag.m4
new file mode 100644
index 000..dcabb92
--- /dev/null
+++ b/m4/ax_check_compile_flag.m4
@@ -0,0 +1,74 @@
+# ===
+# https://www.gnu.org/software/autoconf-archive/ax_check_compile_flag.html
+# ===
+#
+# SYNOPSIS
+#
+# AX_CHECK_COMPILE_FLAG(FLAG, [ACTION-SUCCESS], [ACTION-FAILURE],
[EXTRA-FLAGS], [INPUT])
+#
+# DESCRIPTION
+#
+# Check whether the given FLAG works with the current language's compiler
+# or gives an error. (Warnings, however, are ignored)
+#
+# ACTION-SUCCESS/ACTION-FAILURE are shell commands to execute on
+# success/failure.
+#
+# If EXTRA-FLAGS is defined, it is added to the current language's default
+# flags (e.g. CFLAGS) when the check is done. The check is thus made with
+# the flags: "CFLAGS EXTRA-FLAGS FLAG". This can for example be used to
+# force the compiler to issue an error when a bad flag is given.
+#
+# INPUT gives an alternative input source to AC_COMPILE_IFELSE.
+#
+# NOTE: Implementation based on AX_CFLAGS_GCC_OPTION. Please keep this
+# macro in sync with AX_CHECK_{PREPROC,LINK}_FLAG.
+#
+# LICENSE
+#
+# Copyright (c) 2008 Guido U. Draheim
+# Copyright (c) 2011 Maarten Bosmans
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by the
+# Free Software Foundation, either version 3 of the License, or (at your
+# option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+# Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program. If not, see
[PATCH 1/3] btrfs-progs: convert: fix support for e2fsprogs < 1.42
From: Jeff Mahoney
Commit 324d4c1857a (btrfs-progs: convert: Add larger device support)
introduced new dependencies on the 64-bit API provided by e2fsprogs.
That API was introduced in v1.42 (along with bigalloc).
This patch maps the following to their equivalents in e2fsprogs < 1.42.
- ext2fs_get_block_bitmap_range2
- ext2fs_inode_data_blocks2
- ext2fs_read_ext_attr2
Since we need to detect and define EXT2_FLAG_64BITS for compatibilty
anyway, it makes sense to use that to detect the older e2fsprogs instead
of defining a new flag ourselves.
Signed-off-by: Jeff Mahoney
---
configure.ac | 6 +-
convert/source-ext2.h | 12 +++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/configure.ac b/configure.ac
index af13a95..2dea1c6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -140,11 +140,7 @@ BTRFSCONVERT_EXT2=0
BTRFSCONVERT_REISERFS=0
if test "x$enable_convert" = xyes; then
if test "x$with_convert" = "xauto" || echo "$with_convert" | grep -q
"ext2"; then
- PKG_CHECK_MODULES(EXT2FS, [ext2fs >= 1.42],,
- [PKG_CHECK_MODULES(EXT2FS, [ext2fs],
- [AC_DEFINE([HAVE_OLD_E2FSPROGS], [1],
- [E2fsprogs does not support
BIGALLOC])]
- )])
+ PKG_CHECK_MODULES(EXT2FS, [ext2fs])
PKG_CHECK_MODULES(COM_ERR, [com_err])
convertfs="${convertfs:+$convertfs,}ext2"
BTRFSCONVERT_EXT2=1
diff --git a/convert/source-ext2.h b/convert/source-ext2.h
index 80833b2..c321412 100644
--- a/convert/source-ext2.h
+++ b/convert/source-ext2.h
@@ -33,8 +33,18 @@
* BIGALLOC.
* Unlike normal RO compat flag, BIGALLOC affects how e2fsprogs check used
* space, and btrfs-convert heavily relies on it.
+ *
+ * e2fsprogs 1.42 also introduced the 64-bit API. Any file system
+ * that requires it will have EXT4_FEATURE_INCOMPAT_64BIT set and
+ * will fail to open with earlier releases. We can map it to the
+ * older API without risk of corruption.
*/
-#ifdef HAVE_OLD_E2FSPROGS
+#ifndef EXT2_FLAG_64BITS
+#define EXT2_FLAG_64BITS (0)
+#define ext2fs_get_block_bitmap_range2 ext2fs_get_block_bitmap_range
+#define ext2fs_inode_data_blocks2 ext2fs_inode_data_blocks
+#define ext2fs_read_ext_attr2 ext2fs_read_ext_attr
+#define ext2fs_blocks_count(s) ((s)->s_blocks_count)
#define EXT2FS_CLUSTER_RATIO(fs) (1)
#define EXT2_CLUSTERS_PER_GROUP(s) (EXT2_BLOCKS_PER_GROUP(s))
#define EXT2FS_B2C(fs, blk)(blk)
--
1.7.12.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] btrfs-progs: build: autoconf 2.63 compatibility
From: Jeff Mahoney Commit 2e1932e6a38 (btrfs-progs: build: simplify version tracking) started m4_chomp to strip the newlines from the version file. m4_chomp was introduced in autoconf 2.64 but SLE11 ships with autoconf 2.63. For purposes of just stripping the newline, m4_flatten is sufficient. Signed-off-by: Jeff Mahoney --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2dea1c6..9e5603b 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ AC_INIT([btrfs-progs], - m4_chomp(m4_include([VERSION])), + m4_flatten(m4_include([VERSION])), [linux-btrfs@vger.kernel.org],, [http://btrfs.wiki.kernel.org]) -- 1.7.12.4 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Strange behavior (possible bugs) in btrfs
Hi, We found two more cases where the btrfs behavior is a little strange. In one case, an fsync-ed file goes missing after a crash. In the other, a renamed file shows up in both directories after a crash. Workload 1: mkdir A mkdir B mkdir A/C creat B/foo fsync B/foo link B/foo A/C/foo fsync A -- crash -- Expected state after recovery: B B/foo A A/C exist What we find: Only B B/foo exist A is lost even after explicit fsync to A. Workload 2: mkdir A mkdir A/C rename A/C B touch B/bar fsync B/bar rename B/bar A/bar rename A B (replacing B with A at this point) fsync B/bar -- crash -- Expected contents after recovery: A/bar What we find after recovery: A/bar B/bar We think this breaks rename's atomicity guarantee. bar should be present in either A or B, but now it is present in both. Thanks, Vijay -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Strange behavior (possible bugs) in btrfs
On 4/30/18 12:04 PM, Vijay Chidambaram wrote: > Hi, > > We found two more cases where the btrfs behavior is a little strange. > In one case, an fsync-ed file goes missing after a crash. In the > other, a renamed file shows up in both directories after a crash. Hi Vijay - What kernel version did you observe these with? These seem like bugs Filipe has already fixed. -Jeff > Workload 1: > > mkdir A > mkdir B > mkdir A/C > creat B/foo > fsync B/foo > link B/foo A/C/foo > fsync A > -- crash -- > > Expected state after recovery: > B B/foo A A/C exist > > What we find: > Only B B/foo exist > > A is lost even after explicit fsync to A. > > Workload 2: > > mkdir A > mkdir A/C > rename A/C B > touch B/bar > fsync B/bar > rename B/bar A/bar > rename A B (replacing B with A at this point) > fsync B/bar > -- crash -- > > Expected contents after recovery: > A/bar > > What we find after recovery: > A/bar > B/bar > > We think this breaks rename's atomicity guarantee. bar should be > present in either A or B, but now it is present in both. > > Thanks, > Vijay > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Jeff Mahoney SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Strange behavior (possible bugs) in btrfs
Hi Jeff, On Mon, Apr 30, 2018 at 11:51 AM, Jeff Mahoney wrote: > On 4/30/18 12:04 PM, Vijay Chidambaram wrote: >> Hi, >> >> We found two more cases where the btrfs behavior is a little strange. >> In one case, an fsync-ed file goes missing after a crash. In the >> other, a renamed file shows up in both directories after a crash. > > Hi Vijay - > > What kernel version did you observe these with? These seem like bugs > Filipe has already fixed. > > -Jeff > These bugs were observed on Kernel 4.16.0-041600-generic. Thanks, Jayashree -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: rename btrfs_balance_control::flags to b_flags
On Mon, Apr 30, 2018 at 05:49:13PM +0800, Anand Jain wrote: > As flags is a generic name, its often difficult to search, rename > btrfs_balance_control::flags to btrfs_balance_control::b_flags. I don't think this one is necessary as it's always tied to bctl. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Btrfs: send, fix missing truncate for inode with prealloc extent past eof
From: Filipe Manana
An incremental send operation can miss a truncate operation when an inode
has an increased size in the send snapshot and a prealloc extent beyond
its size.
Consider the following scenario where a necessary truncate operation is
missing in the incremental send stream:
1) In the parent snapshot an inode has a size of 1282957 bytes and it has
no prealloc extents beyond its size;
2) In the the send snapshot it has a size of 5738496 bytes and has a new
extent at offsets 1884160 (length of 106496 bytes) and a prealloc
extent beyond eof at offset 6729728 (and a length of 339968 bytes);
3) When processing the prealloc extent, at offset 6729728, we end up at
send.c:send_write_or_clone() and set the @len variable to a value of
18446744073708560384 because @offset plus the original @len value is
larger then the inode's size (6729728 + 339968 > 5738496). We then
call send_extent_data(), with that @offset and @len, which in turn
calls send_write(), and then the later calls fill_read_buf(). Because
the offset passed to fill_read_buf() is greater then inode's i_size,
this function returns 0 immediately, which makes send_write() and
send_extent_data() do nothing and return immediately as well. When
we get back to send.c:send_write_or_clone() we adjust the value
of sctx->cur_inode_next_write_offset to @offset plus @len, which
corresponds to 6729728 + 18446744073708560384 = 5738496, which is
precisely the the size of the inode in the send snapshot;
4) Later when at send.c:finish_inode_if_needed() we determine that
we don't need to issue a truncate operation because the value of
sctx->cur_inode_next_write_offset corresponds to the inode's new
size, 5738496 bytes. This is wrong because the last write operation
that was issued started at offset 1884160 with a length of 106496
bytes, so the correct value for sctx->cur_inode_next_write_offset
should be 1990656 (1884160 + 106496), so that a truncate operation
with a value of 5738496 bytes would have been sent to insert a
trailing hole at the destination.
So fix the issue by making send.c:send_write_or_clone() not attempt
to send write or clone operations for extents that start beyond the
inode's size, since such attempts do nothing but waste time by
calling helper functions and allocating path structures, and send
currently has no fallocate command in order to create prealloc extents
at the destination (either beyond a file's eof or not).
The issue was found running the test btrfs/007 from fstests using a seed
value of 1524346151 for fsstress.
Reported-by: Gu, Jinxiang
Fixes: ffa7c4296e93 ("Btrfs: send, do not issue unnecessary truncate
operations")
Signed-off-by: Filipe Manana
---
fs/btrfs/send.c | 4
1 file changed, 4 insertions(+)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 1f5748c7d1c7..fff44ed15927 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5249,6 +5249,10 @@ static int send_write_or_clone(struct send_ctx *sctx,
len = btrfs_file_extent_num_bytes(path->nodes[0], ei);
}
+ if (offset >= sctx->cur_inode_size) {
+ ret = 0;
+ goto out;
+ }
if (offset + len > sctx->cur_inode_size)
len = sctx->cur_inode_size - offset;
if (len == 0) {
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] test online label ioctl
This tests the online label ioctl that btrfs has, which has been
recently proposed for XFS.
To run, it requires an updated xfs_io with the label command and a
filesystem that supports it
A slight change here to _require_xfs_io_command as well, so that tests
which simply fail with "Inappropriate ioctl" can be caught in the
common case.
Signed-off-by: Eric Sandeen
---
this passes on btrfs, _notruns on xfs/ext4 of yore, and passes
on xfs w/ my online label patchset (as long as xfs_io has the new
capability)
diff --git a/common/rc b/common/rc
index 9ffab7f..c53a721 100644
--- a/common/rc
+++ b/common/rc
@@ -2158,6 +2158,9 @@ _require_xfs_io_command()
echo $testio | grep -q "Inappropriate ioctl" && \
_notrun "xfs_io $command support is missing"
;;
+ "label")
+ testio=`$XFS_IO_PROG -c "label" $TEST_DIR 2>&1`
+ ;;
"open")
# -c "open $f" is broken in xfs_io <= 4.8. Along with the fix,
# a new -C flag was introduced to execute one shot commands.
@@ -2196,7 +2199,7 @@ _require_xfs_io_command()
rm -f $testfile 2>&1 > /dev/null
echo $testio | grep -q "not found" && \
_notrun "xfs_io $command support is missing"
- echo $testio | grep -q "Operation not supported" && \
+ echo $testio | grep -q "Operation not supported\|Inappropriate ioctl"
&& \
_notrun "xfs_io $command failed (old kernel/wrong fs?)"
echo $testio | grep -q "Invalid" && \
_notrun "xfs_io $command failed (old kernel/wrong fs/bad args?)"
diff --git a/tests/generic/485 b/tests/generic/485
new file mode 100755
index 000..79902c2
--- /dev/null
+++ b/tests/generic/485
@@ -0,0 +1,99 @@
+#! /bin/bash
+# FS QA Test 485
+#
+# Test the online filesystem label set/get ioctls
+#
+#---
+# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved.
+# Author: Eric Sandeen
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#---
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command "label"
+
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount
+
+# Make sure we can set & clear the label
+$XFS_IO_PROG -c "label label.$seq" $SCRATCH_MNT
+$XFS_IO_PROG -c "label" $SCRATCH_MNT
+
+# And that userspace can see it now, while mounted
+blkid -s LABEL $SCRATCH_DEV | _filter_scratch
+
+# And that the it is still there when it's unmounted
+_scratch_unmount
+blkid -s LABEL $SCRATCH_DEV | _filter_scratch
+
+# And that it persists after a remount
+_scratch_mount
+$XFS_IO_PROG -c "label" $SCRATCH_MNT
+
+# And that a too-long label is rejected, beyond the interface max:
+LABEL=$(perl -e "print 'l' x 257;")
+$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT
+
+# And that it succeeds right at the filesystem max:
+case $FSTYP in
+xfs)
+ MAXLEN=12;
+ ;;
+btrfs)
+ MAXLEN=256
+ ;;
+*)
+ MAXLEN=256
+ echo "Your filesystem supports online label, please add max length"
+ ;;
+esac
+LABEL=$(perl -e "print 'o' x $MAXLEN;")
+$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT | sed -e 's/o\+/MAXLABEL/'
+
+# And that it fails just past the filesystem max:
+let TOOLONG=MAXLEN+1
+LABEL=$(perl -e "print 'o' x $TOOLONG;")
+$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/485.out b/tests/generic/485.out
new file mode 100644
index 000..bc54684
--- /dev/null
+++ b/tests/generic/485.out
@@ -0,0 +1,9 @@
+QA output created by 485
+label = "label.485"
+label = "label.485"
+SCRATCH_DEV: LABEL="label.485"
+SCRATCH_DEV: LABEL="label.485"
+label = "label.485"
+label: Invalid argument
+label = "MAXLABEL"
+label: Invalid argument
diff --git a/tests/generic/group b/tests/generic/group
index 19be926..cf6ac49 100644
--- a/tests/generic/group
+++ b/
