* [PATCH v6 0/3] btrfs: balance: improve kernel logs
@ 2018-11-20 8:12 Anand Jain
2018-11-20 8:12 ` [PATCH v6 1/3] btrfs: add helper function describe_block_group() Anand Jain
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Anand Jain @ 2018-11-20 8:12 UTC (permalink / raw)
To: linux-btrfs
v5->v6:
Mostly the defines non functional changes.
Use goto instead of return in middle of the define.
Pls ref individual patches 1/3 and 2/3 for more info.
v4->v5:
Mainly address David review comment [1].
[1]
https://patchwork.kernel.org/patch/10425987/
pls ref to individual patch 2/3 for details.
v3->v4:
Pls ref to individual patches.
Based on misc-next.
v2->v3:
Inspried by describe_relocation(), improves it, makes it a helper
function and use it to log the balance operations.
Kernel logs are very important for the forensic investigations of the
issues, these patchs make balance logs easy to review.
Anand Jain (3):
btrfs: add helper function describe_block_group()
btrfs: balance: add args info during start and resume
btrfs: balance: add kernel log for end or paused
fs/btrfs/relocation.c | 30 +------
fs/btrfs/volumes.c | 216 +++++++++++++++++++++++++++++++++++++++++++++++++-
fs/btrfs/volumes.h | 1 +
3 files changed, 217 insertions(+), 30 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v6 1/3] btrfs: add helper function describe_block_group()
2018-11-20 8:12 [PATCH v6 0/3] btrfs: balance: improve kernel logs Anand Jain
@ 2018-11-20 8:12 ` Anand Jain
2018-11-20 8:12 ` [PATCH v6 2/3] btrfs: balance: add args info during start and resume Anand Jain
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Anand Jain @ 2018-11-20 8:12 UTC (permalink / raw)
To: linux-btrfs
Improve on describe_relocation() add a common helper function to describe
the block groups.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
---
v5->v6: Use () in the body for the args sent in defines
Use right indent to align '\'
Use goto to out_overflow instead of return
v4.1->v5: Initialize buf[128] to null.
v4->v4.1: Use strcpy(buf, "|NONE"); as in the original
v3->v4: Just pass full flag name in the define DESCRIBE_FLAG(flag,..),
so that it can be used at couple of more places.
Rename describe_block_groups() to btrfs_describe_block_groups().
Drop useless return u32.
v3: Born.
fs/btrfs/relocation.c | 30 +++---------------------------
fs/btrfs/volumes.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
fs/btrfs/volumes.h | 1 +
3 files changed, 50 insertions(+), 27 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 924116f654a1..0373b3cc1d36 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4184,37 +4184,13 @@ static struct reloc_control *alloc_reloc_control(void)
static void describe_relocation(struct btrfs_fs_info *fs_info,
struct btrfs_block_group_cache *block_group)
{
- char buf[128]; /* prefixed by a '|' that'll be dropped */
- u64 flags = block_group->flags;
+ char buf[128] = {'\0'};
- /* Shouldn't happen */
- if (!flags) {
- strcpy(buf, "|NONE");
- } else {
- char *bp = buf;
-
-#define DESCRIBE_FLAG(f, d) \
- if (flags & BTRFS_BLOCK_GROUP_##f) { \
- bp += snprintf(bp, buf - bp + sizeof(buf), "|%s", d); \
- flags &= ~BTRFS_BLOCK_GROUP_##f; \
- }
- DESCRIBE_FLAG(DATA, "data");
- DESCRIBE_FLAG(SYSTEM, "system");
- DESCRIBE_FLAG(METADATA, "metadata");
- DESCRIBE_FLAG(RAID0, "raid0");
- DESCRIBE_FLAG(RAID1, "raid1");
- DESCRIBE_FLAG(DUP, "dup");
- DESCRIBE_FLAG(RAID10, "raid10");
- DESCRIBE_FLAG(RAID5, "raid5");
- DESCRIBE_FLAG(RAID6, "raid6");
- if (flags)
- snprintf(bp, buf - bp + sizeof(buf), "|0x%llx", flags);
-#undef DESCRIBE_FLAG
- }
+ btrfs_describe_block_groups(block_group->flags, buf, sizeof(buf));
btrfs_info(fs_info,
"relocating block group %llu flags %s",
- block_group->key.objectid, buf + 1);
+ block_group->key.objectid, buf);
}
/*
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f435d397019e..3ab22e7e404e 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -123,6 +123,52 @@ const char *get_raid_name(enum btrfs_raid_types type)
return btrfs_raid_array[type].raid_name;
}
+void btrfs_describe_block_groups(u64 bg_flags, char *buf, u32 size_buf)
+{
+ int i;
+ int ret;
+ char *bp = buf;
+ u64 flags = bg_flags;
+ u32 size_bp = size_buf;
+
+ if (!flags) {
+ strcpy(bp, "NONE");
+ return;
+ }
+
+#define DESCRIBE_FLAG(f, d) \
+ do { \
+ if (flags & (f)) { \
+ ret = snprintf(bp, size_bp, "%s|", (d));\
+ if (ret < 0 || ret >= size_bp) \
+ goto out_overflow; \
+ size_bp -= ret; \
+ bp += ret; \
+ flags &= ~(f); \
+ } \
+ } while (0)
+
+ DESCRIBE_FLAG(BTRFS_BLOCK_GROUP_DATA, "data");
+ DESCRIBE_FLAG(BTRFS_BLOCK_GROUP_SYSTEM, "system");
+ DESCRIBE_FLAG(BTRFS_BLOCK_GROUP_METADATA, "metadata");
+ DESCRIBE_FLAG(BTRFS_AVAIL_ALLOC_BIT_SINGLE, "single");
+ for (i = 0; i < BTRFS_NR_RAID_TYPES; i++)
+ DESCRIBE_FLAG(btrfs_raid_array[i].bg_flag,
+ btrfs_raid_array[i].raid_name);
+#undef DESCRIBE_FLAG
+
+ if (flags) {
+ ret = snprintf(bp, size_bp, "0x%llx|", flags);
+ size_bp -= ret;
+ }
+
+ if (size_bp < size_buf)
+ buf[size_buf - size_bp - 1] = '\0'; /* remove last | */
+
+out_overflow:
+ return;
+}
+
static int init_first_rw_device(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info);
static int btrfs_relocate_sys_chunks(struct btrfs_fs_info *fs_info);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index aefce895e994..3e914effcdf6 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -430,6 +430,7 @@ struct btrfs_device *btrfs_find_device(struct btrfs_fs_info *fs_info, u64 devid,
int btrfs_balance(struct btrfs_fs_info *fs_info,
struct btrfs_balance_control *bctl,
struct btrfs_ioctl_balance_args *bargs);
+void btrfs_describe_block_groups(u64 flags, char *buf, u32 size_buf);
int btrfs_resume_balance_async(struct btrfs_fs_info *fs_info);
int btrfs_recover_balance(struct btrfs_fs_info *fs_info);
int btrfs_pause_balance(struct btrfs_fs_info *fs_info);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v6 2/3] btrfs: balance: add args info during start and resume
2018-11-20 8:12 [PATCH v6 0/3] btrfs: balance: improve kernel logs Anand Jain
2018-11-20 8:12 ` [PATCH v6 1/3] btrfs: add helper function describe_block_group() Anand Jain
@ 2018-11-20 8:12 ` Anand Jain
2018-11-20 8:12 ` [PATCH v6 3/3] btrfs: balance: add kernel log for end or paused Anand Jain
2018-11-20 14:48 ` [PATCH v6 0/3] btrfs: balance: improve kernel logs David Sterba
3 siblings, 0 replies; 5+ messages in thread
From: Anand Jain @ 2018-11-20 8:12 UTC (permalink / raw)
To: linux-btrfs
Balance arg info is an important information to be reviewed for the
system audit. So this patch adds them to the kernel log.
Example:
->btrfs bal start -f -mprofiles=raid1,convert=single,soft -dlimit=10..20,usage=50 /btrfs
kernel: BTRFS info (device sdb): balance: start -f -dusage=50,limit=10..20 -msoft,profiles=raid1,convert=single -ssoft,profiles=raid1,convert=single
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v5->v6: Use () in the body for the args sent in defines
Use right indent to align '\'
Use goto to out_overflow instead of return (also fixes a mem leak in v5)
Rename CHECK_UPDATE_BP_XXX to CHECK_APPEND_XXX
v4.1->v5: Per David review comment the code..
bp += snprintf(bp, buf - bp + size_buf, "soft,");
is not safe if 'buf - bp + size_buf' becomes accidentally
negative, so fix this by using following snprintf.
#define CHECK_UPDATE_BP_1(a) \
do { \
ret = snprintf(bp, size_bp, a); \
if (ret < 0 || ret >= size_bp) \
goto out; \
size_bp -= ret; \
bp += ret; \
} while (0)
And initialize the tmp_buf to null.
v4->v4.1: Rename last missed sz to size_buf.
v3->v4: Change log updated with new example.
Log format is changed to almost match with the cli.
snprintf drop %s for inline string.
Print range as =%u..%u instead of min=%umax=%u.
soft is under the args now.
force is printed as -f.
v2->v3: Change log updated.
Make use of describe_block_group() added in 1/4
Drop using strcat instead use snprintf.
Logs string format updated as shown in the example above.
v1->v2: Change log update.
Move adding the logs for balance complete and end to a new patch
fs/btrfs/volumes.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 160 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 3ab22e7e404e..eb18739f19ef 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3757,6 +3757,164 @@ static inline int validate_convert_profile(struct btrfs_balance_args *bctl_arg,
(bctl_arg->target & ~allowed)));
}
+static void describe_balance_args(struct btrfs_balance_args *bargs, char *buf,
+ u32 size_buf)
+{
+ int ret;
+ u32 size_bp = size_buf;
+ char *bp = buf;
+ u64 flags = bargs->flags;
+ char tmp_buf[128] = {'\0'};
+
+ if (!flags)
+ return;
+
+#define CHECK_APPEND_NOARG(a) \
+ do { \
+ ret = snprintf(bp, size_bp, (a)); \
+ if (ret < 0 || ret >= size_bp) \
+ goto out_overflow; \
+ size_bp -= ret; \
+ bp += ret; \
+ } while (0)
+
+#define CHECK_APPEND_1ARG(a, v1) \
+ do { \
+ ret = snprintf(bp, size_bp, (a), (v1)); \
+ if (ret < 0 || ret >= size_bp) \
+ goto out_overflow; \
+ size_bp -= ret; \
+ bp += ret; \
+ } while (0)
+
+#define CHECK_APPEND_2ARG(a, v1, v2) \
+ do { \
+ ret = snprintf(bp, size_bp, (a), (v1), (v2)); \
+ if (ret < 0 || ret >= size_bp) \
+ goto out_overflow; \
+ size_bp -= ret; \
+ bp += ret; \
+ } while (0)
+
+ if (flags & BTRFS_BALANCE_ARGS_SOFT)
+ CHECK_APPEND_NOARG("soft,");
+
+ if (flags & BTRFS_BALANCE_ARGS_PROFILES) {
+ btrfs_describe_block_groups(bargs->profiles, tmp_buf,
+ sizeof(tmp_buf));
+ CHECK_APPEND_1ARG("profiles=%s,", tmp_buf);
+ }
+
+ if (flags & BTRFS_BALANCE_ARGS_USAGE)
+ CHECK_APPEND_1ARG("usage=%llu,", bargs->usage);
+
+ if (flags & BTRFS_BALANCE_ARGS_USAGE_RANGE)
+ CHECK_APPEND_2ARG("usage=%u..%u,",
+ bargs->usage_min, bargs->usage_max);
+
+ if (flags & BTRFS_BALANCE_ARGS_DEVID)
+ CHECK_APPEND_1ARG("devid=%llu,", bargs->devid);
+
+ if (flags & BTRFS_BALANCE_ARGS_DRANGE)
+ CHECK_APPEND_2ARG("drange=%llu..%llu,",
+ bargs->pstart, bargs->pend);
+
+ if (flags & BTRFS_BALANCE_ARGS_VRANGE)
+ CHECK_APPEND_2ARG("vrange%llu..%llu,",
+ bargs->vstart, bargs->vend);
+
+ if (flags & BTRFS_BALANCE_ARGS_LIMIT)
+ CHECK_APPEND_1ARG("limit=%llu,", bargs->limit);
+
+ if (flags & BTRFS_BALANCE_ARGS_LIMIT_RANGE)
+ CHECK_APPEND_2ARG("limit=%u..%u,",
+ bargs->limit_min, bargs->limit_max);
+
+ if (flags & BTRFS_BALANCE_ARGS_STRIPES_RANGE)
+ CHECK_APPEND_2ARG("stripes=%u..%u,",
+ bargs->stripes_min, bargs->stripes_max);
+
+ if (flags & BTRFS_BALANCE_ARGS_CONVERT) {
+ int index = btrfs_bg_flags_to_raid_index(bargs->target);
+
+ CHECK_APPEND_1ARG("convert=%s,", get_raid_name(index));
+ }
+
+out_overflow:
+#undef CHECK_APPEND_2ARG
+#undef CHECK_APPEND_1ARG
+#undef CHECK_APPEND_NOARG
+
+ if (size_bp < size_buf)
+ buf[size_buf - size_bp - 1] = '\0'; /* remove last , */
+ else
+ buf[0] = '\0';
+}
+
+static void describe_balance_start_or_resume(struct btrfs_fs_info *fs_info)
+{
+ u32 size_buf = 1024;
+ char tmp_buf[192] = {'\0'};
+ char *buf;
+ char *bp;
+ u32 size_bp = size_buf;
+ int ret;
+ struct btrfs_balance_control *bctl = fs_info->balance_ctl;
+
+ buf = kzalloc(size_buf, GFP_KERNEL);
+ if (!buf)
+ return;
+
+ bp = buf;
+#define CHECK_APPEND_NOARG(a) \
+ do { \
+ ret = snprintf(bp, size_bp, (a)); \
+ if (ret < 0 || ret >= size_bp) \
+ goto out_overflow; \
+ size_bp -= ret; \
+ bp += ret; \
+ } while (0)
+
+#define CHECK_APPEND_1ARG(a, v1) \
+ do { \
+ ret = snprintf(bp, size_bp, (a), (v1)); \
+ if (ret < 0 || ret >= size_bp) \
+ goto out_overflow; \
+ size_bp -= ret; \
+ bp += ret; \
+ } while (0)
+
+ if (bctl->flags & BTRFS_BALANCE_FORCE)
+ CHECK_APPEND_NOARG("-f ");
+
+ if (bctl->flags & BTRFS_BALANCE_DATA) {
+ describe_balance_args(&bctl->data, tmp_buf, sizeof(tmp_buf));
+ CHECK_APPEND_1ARG("-d%s ", tmp_buf);
+ }
+
+ if (bctl->flags & BTRFS_BALANCE_METADATA) {
+ describe_balance_args(&bctl->meta, tmp_buf, sizeof(tmp_buf));
+ CHECK_APPEND_1ARG("-m%s ", tmp_buf);
+ }
+
+ if (bctl->flags & BTRFS_BALANCE_SYSTEM) {
+ describe_balance_args(&bctl->sys, tmp_buf, sizeof(tmp_buf));
+ CHECK_APPEND_1ARG("-s%s ", tmp_buf);
+ }
+
+out_overflow:
+#undef CHECK_APPEND_1ARG
+#undef CHECK_APPEND_NOARG
+
+ if (size_bp < size_buf)
+ buf[size_buf - size_bp - 1] = '\0'; /* remove last " "*/
+ btrfs_info(fs_info, "%s %s",
+ bctl->flags & BTRFS_BALANCE_RESUME ?
+ "balance: resume":"balance: start", buf);
+
+ kfree(buf);
+}
+
/*
* Should be called with balance mutexe held
*/
@@ -3896,6 +4054,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
ASSERT(!test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags));
set_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags);
+ describe_balance_start_or_resume(fs_info);
mutex_unlock(&fs_info->balance_mutex);
ret = __btrfs_balance(fs_info);
@@ -3933,10 +4092,8 @@ static int balance_kthread(void *data)
int ret = 0;
mutex_lock(&fs_info->balance_mutex);
- if (fs_info->balance_ctl) {
- btrfs_info(fs_info, "balance: resuming");
+ if (fs_info->balance_ctl)
ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);
- }
mutex_unlock(&fs_info->balance_mutex);
return ret;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v6 3/3] btrfs: balance: add kernel log for end or paused
2018-11-20 8:12 [PATCH v6 0/3] btrfs: balance: improve kernel logs Anand Jain
2018-11-20 8:12 ` [PATCH v6 1/3] btrfs: add helper function describe_block_group() Anand Jain
2018-11-20 8:12 ` [PATCH v6 2/3] btrfs: balance: add args info during start and resume Anand Jain
@ 2018-11-20 8:12 ` Anand Jain
2018-11-20 14:48 ` [PATCH v6 0/3] btrfs: balance: improve kernel logs David Sterba
3 siblings, 0 replies; 5+ messages in thread
From: Anand Jain @ 2018-11-20 8:12 UTC (permalink / raw)
To: linux-btrfs
Add a kernel log when the balance ends, either for cancel or completed
or if it is paused.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v5->v6: Quite soul. nothing.
v4->v5: nothing.
v3->v4: nothing.
v2->v3: nothing.
v1->v2: Moved from 2/3 to 3/3
fs/btrfs/volumes.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index eb18739f19ef..0684f14ff70d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4060,6 +4060,13 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
ret = __btrfs_balance(fs_info);
mutex_lock(&fs_info->balance_mutex);
+ if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req))
+ btrfs_info(fs_info, "balance: paused");
+ else if (ret == -ECANCELED && atomic_read(&fs_info->balance_cancel_req))
+ btrfs_info(fs_info, "balance: canceled");
+ else
+ btrfs_info(fs_info, "balance: ended with status: %d", ret);
+
clear_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags);
if (bargs) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v6 0/3] btrfs: balance: improve kernel logs
2018-11-20 8:12 [PATCH v6 0/3] btrfs: balance: improve kernel logs Anand Jain
` (2 preceding siblings ...)
2018-11-20 8:12 ` [PATCH v6 3/3] btrfs: balance: add kernel log for end or paused Anand Jain
@ 2018-11-20 14:48 ` David Sterba
3 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2018-11-20 14:48 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs
On Tue, Nov 20, 2018 at 04:12:54PM +0800, Anand Jain wrote:
> v5->v6:
> Mostly the defines non functional changes.
> Use goto instead of return in middle of the define.
> Pls ref individual patches 1/3 and 2/3 for more info.
>
> v4->v5:
> Mainly address David review comment [1].
> [1]
> https://patchwork.kernel.org/patch/10425987/
> pls ref to individual patch 2/3 for details.
>
> v3->v4:
> Pls ref to individual patches.
>
> Based on misc-next.
>
> v2->v3:
> Inspried by describe_relocation(), improves it, makes it a helper
> function and use it to log the balance operations.
>
> Kernel logs are very important for the forensic investigations of the
> issues, these patchs make balance logs easy to review.
>
> Anand Jain (3):
> btrfs: add helper function describe_block_group()
> btrfs: balance: add args info during start and resume
> btrfs: balance: add kernel log for end or paused
Now in topic branch ext/anand/balance-description in my devel repos, I
made some changes as I don't want to take the rounds through mail. There
are some cosmetic changes like reordering the balance args, added
comments and aligned the \ even farther to the right. Please check the
committed patches for some inspiration.
I'll add that to misc-next after passes some tests, until then it'll be
in for-next. Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-20 14:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-20 8:12 [PATCH v6 0/3] btrfs: balance: improve kernel logs Anand Jain
2018-11-20 8:12 ` [PATCH v6 1/3] btrfs: add helper function describe_block_group() Anand Jain
2018-11-20 8:12 ` [PATCH v6 2/3] btrfs: balance: add args info during start and resume Anand Jain
2018-11-20 8:12 ` [PATCH v6 3/3] btrfs: balance: add kernel log for end or paused Anand Jain
2018-11-20 14:48 ` [PATCH v6 0/3] btrfs: balance: improve kernel logs David Sterba
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).