Hi Oleksandr,
I finally got around to doing a code review today, and ended up making a few
minor changes to your patch:
(1) Instead of defaulting to NORMAL, invalid arguments generate a FATAL
error.
(2) I reworded the help page entry, and moved it up into the restricted
process section instead of down in the "alternatives" section.
(3) I moved the offset_table[] and size_table[] task_struct_policy members
to the end of the arrays to prevent any breakage of pre-compiled
extension modules.
(4) Display the task_struct_policy offset and size table values in the
dump_offset_table() function for "help -o".
(5) Replaced the strdup() call with STRDUPBUF(), calloc() with GETBUF(),
and free() calls with FREEBUF() to avoid using them during runtime.
But I need you to re-work your sched_policy_bit_from_str() function so that it
will accept a hexadecimal number, i.e, "ps -y 0x2". Granted it's not explicitly
allowed in the help page, but a user may see the policy expressed as a hexadecimal
value by "task" or "struct", and it seems too picky to just disallow it.
I've attached an updated version of the patch with the changes above.
Thanks,
Dave
----- Original Message -----
> While analyzing vmcores from customers sometimes we want
> to filter tasks by scheduling policy, for instance, to identify
> if customer runs realtime tasks. Doing this via foreach grepping
> and then feeding back pointers to task_struct and grepping it again
> is very slow, especially if customer runs thousands of tasks.
>
> So, let's add another option for ps to filter tasks by scheduling
> policy.
>
> Changes in v3:
>
> * fix snprintf format specifier for ulong
> * fix snprintf block indentation
> * add one more comma to help output
>
> Changes in v2:
>
> * handle task_struct.policy member size correctly
> * accept comma-separated list of policies
> instead of requiring multiple -y arguments
> * policy can be specified as a number too
> * policy name is case-insensitive now
> * warn about incorrect policy name
> * fix help message formatting
> * mark upper_case() source string pointer as a const
> (minor cleanup)
>
> Oleksandr Natalenko (1):
> task: also filter ps output by ->policy
>
> defs.h | 15 +++++++-
> help.c | 7 +++-
> task.c | 135
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> tools.c | 5 ++-
> 4 files changed, 152 insertions(+), 10 deletions(-)
>
> --
> 2.14.2
>
>
diff --git a/defs.h b/defs.h
index 76e5512..4b4e331 100644
--- a/defs.h
+++ b/defs.h
@@ -1139,6 +1139,7 @@ extern struct machdep_table *machdep;
#define FOREACH_a_FLAG (0x4000000)
#define FOREACH_G_FLAG (0x8000000)
#define FOREACH_F_FLAG2 (0x10000000)
+#define FOREACH_y_FLAG (0x20000000)
#define FOREACH_PS_EXCLUSIVE \
(FOREACH_g_FLAG|FOREACH_a_FLAG|FOREACH_t_FLAG|FOREACH_c_FLAG|FOREACH_p_FLAG|FOREACH_l_FLAG|FOREACH_r_FLAG|FOREACH_m_FLAG)
@@ -1162,6 +1163,7 @@ struct foreach_data {
int comms;
int args;
int regexs;
+ int policy;
};
struct reference {
@@ -1992,6 +1994,7 @@ struct offset_table { /* stash of commonly-used offsets */
long mod_arch_specific_num_orcs;
long mod_arch_specific_orc_unwind_ip;
long mod_arch_specific_orc_unwind;
+ long task_struct_policy;
};
struct size_table { /* stash of commonly-used sizes */
@@ -2141,6 +2144,7 @@ struct size_table { /* stash of commonly-used sizes */
long sk_buff_head_qlen;
long sk_buff_len;
long orc_entry;
+ long task_struct_policy;
};
struct array_table {
@@ -4576,6 +4580,13 @@ enum type_code {
*/
#define PF_EXITING 0x00000004 /* getting shut down */
#define PF_KTHREAD 0x00200000 /* I am a kernel thread */
+#define SCHED_NORMAL 0
+#define SCHED_FIFO 1
+#define SCHED_RR 2
+#define SCHED_BATCH 3
+#define SCHED_ISO 4
+#define SCHED_IDLE 5
+#define SCHED_DEADLINE 6
extern long _ZOMBIE_;
#define IS_ZOMBIE(task) (task_state(task) & _ZOMBIE_)
@@ -4603,6 +4614,7 @@ extern long _ZOMBIE_;
#define PS_NO_HEADER (0x10000)
#define PS_MSECS (0x20000)
#define PS_SUMMARY (0x40000)
+#define PS_POLICY (0x80000)
#define PS_EXCLUSIVE (PS_TGID_LIST|PS_ARGV_ENVP|PS_TIMES|PS_CHILD_LIST|PS_PPID_LIST|PS_LAST_RUN|PS_RLIMIT|PS_MSECS|PS_SUMMARY)
@@ -4620,6 +4632,7 @@ struct psinfo {
} regex_data[MAX_PS_ARGS];
int regexs;
ulong *cpus;
+ int policy;
};
#define IS_A_NUMBER(X) (decimal(X, 0) || hexadecimal(X, 0))
@@ -4823,7 +4836,7 @@ char *strip_ending_char(char *, char);
char *strip_beginning_char(char *, char);
char *strip_comma(char *);
char *strip_hex(char *);
-char *upper_case(char *, char *);
+char *upper_case(const char *, char *);
char *first_nonspace(char *);
char *first_space(char *);
char *replace_string(char *, char *, char);
diff --git a/help.c b/help.c
index f9c5792..efa55e0 100644
--- a/help.c
+++ b/help.c
@@ -844,7 +844,7 @@ char *help_foreach[] = {
" net run the \"net\" command (optional flags: -s -S -R -d -x)",
" set run the \"set\" command",
" ps run the \"ps\" command (optional flags: -G -s -p -c -t -l -a",
-" -g -r)",
+" -g -r -y)",
" sig run the \"sig\" command (optional flag: -g)",
" vtop run the \"vtop\" command (optional flags: -c -u -k)\n",
" flag Pass this optional flag to the command selected.",
@@ -1250,7 +1250,7 @@ NULL
char *help_ps[] = {
"ps",
"display process status information",
-"[-k|-u|-G] [-s] [-p|-c|-t|-[l|m][-C cpu]|-a|-g|-r|-S]\n [pid | task | command] ...",
+"[-k|-u|-G|-y policy] [-s] [-p|-c|-t|-[l|m][-C cpu]|-a|-g|-r|-S]\n [pid | task | command] ...",
" This command displays process status for selected, or all, processes" ,
" in the system. If no arguments are entered, the process data is",
" is displayed for all processes. Specific processes may be selected",
@@ -1267,6 +1267,16 @@ char *help_ps[] = {
" -k restrict the output to only kernel threads.",
" -u restrict the output to only user tasks.",
" -G display only the thread group leader in a thread group.",
+" -y policy restrict the output to tasks having a specified scheduling policy",
+" expressed by its integer value or by its (case-insensitive) name;",
+" multiple policies may be entered in a comma-separated list:",
+" 0 or NORMAL",
+" 1 or FIFO",
+" 2 or RR",
+" 3 or BATCH",
+" 4 or ISO",
+" 5 or IDLE",
+" 6 or DEADLINE",
" ",
" The process identifier types may be mixed. For each task, the following",
" items are displayed:",
diff --git a/symbols.c b/symbols.c
index b2f2796..f7599e8 100644
--- a/symbols.c
+++ b/symbols.c
@@ -8584,6 +8584,8 @@ dump_offset_table(char *spec, ulong makestruct)
OFFSET(task_struct_prio));
fprintf(fp, " task_struct_on_rq: %ld\n",
OFFSET(task_struct_on_rq));
+ fprintf(fp, " task_struct_policy: %ld\n",
+ OFFSET(task_struct_policy));
fprintf(fp, " thread_info_task: %ld\n",
OFFSET(thread_info_task));
@@ -10211,6 +10213,7 @@ dump_offset_table(char *spec, ulong makestruct)
fprintf(fp, " pt_regs: %ld\n", SIZE(pt_regs));
fprintf(fp, " task_struct: %ld\n", SIZE(task_struct));
fprintf(fp, " task_struct_flags: %ld\n", SIZE(task_struct_flags));
+ fprintf(fp, " task_struct_policy: %ld\n", SIZE(task_struct_policy));
fprintf(fp, " thread_info: %ld\n", SIZE(thread_info));
fprintf(fp, " softirq_state: %ld\n",
SIZE(softirq_state));
diff --git a/task.c b/task.c
index 362822c..ab33211 100644
--- a/task.c
+++ b/task.c
@@ -109,6 +109,24 @@ static void show_ps_summary(ulong);
static void irqstacks_init(void);
static void parse_task_thread(int argcnt, char *arglist[], struct task_context *);
static void stack_overflow_check_init(void);
+static int has_sched_policy(ulong, ulong);
+static ulong task_policy(ulong);
+static ulong sched_policy_bit_from_str(const char *);
+static ulong make_sched_policy(const char *);
+
+static struct sched_policy_info {
+ ulong value;
+ char *name;
+} sched_policy_info[] = {
+ { SCHED_NORMAL, "NORMAL" },
+ { SCHED_FIFO, "FIFO" },
+ { SCHED_RR, "RR" },
+ { SCHED_BATCH, "BATCH" },
+ { SCHED_ISO, "ISO" },
+ { SCHED_IDLE, "IDLE" },
+ { SCHED_DEADLINE, "DEADLINE" },
+ { ULONG_MAX, NULL }
+};
/*
* Figure out how much space will be required to hold the task context
@@ -273,6 +291,8 @@ task_init(void)
MEMBER_OFFSET_INIT(task_struct_next_run, "task_struct", "next_run");
MEMBER_OFFSET_INIT(task_struct_flags, "task_struct", "flags");
MEMBER_SIZE_INIT(task_struct_flags, "task_struct", "flags");
+ MEMBER_OFFSET_INIT(task_struct_policy, "task_struct", "policy");
+ MEMBER_SIZE_INIT(task_struct_policy, "task_struct", "policy");
MEMBER_OFFSET_INIT(task_struct_pidhash_next,
"task_struct", "pidhash_next");
MEMBER_OFFSET_INIT(task_struct_pgrp, "task_struct", "pgrp");
@@ -2974,7 +2994,7 @@ cmd_ps(void)
cpuspec = NULL;
flag = 0;
- while ((c = getopt(argcnt, args, "SgstcpkuGlmarC:")) != EOF) {
+ while ((c = getopt(argcnt, args, "SgstcpkuGlmarC:y:")) != EOF) {
switch(c)
{
case 'k':
@@ -3075,6 +3095,11 @@ cmd_ps(void)
make_cpumask(cpuspec, psinfo.cpus, FAULT_ON_ERROR, NULL);
break;
+ case 'y':
+ flag |= PS_POLICY;
+ psinfo.policy = make_sched_policy(optarg);
+ break;
+
default:
argerrs++;
break;
@@ -3218,6 +3243,8 @@ show_ps_data(ulong flag, struct task_context *tc, struct psinfo *psi)
return;
if ((flag & PS_KERNEL) && !is_kernel_thread(tc->task))
return;
+ if ((flag & PS_POLICY) && !has_sched_policy(tc->task, psi->policy))
+ return;
if (flag & PS_GROUP) {
if (flag & (PS_LAST_RUN|PS_MSECS))
error(FATAL, "-G not supported with -%c option\n",
@@ -3336,7 +3363,7 @@ show_ps(ulong flag, struct psinfo *psi)
tc = FIRST_CONTEXT();
for (i = 0; i < RUNNING_TASKS(); i++, tc++)
- show_ps_data(flag, tc, NULL);
+ show_ps_data(flag, tc, psi);
return;
}
@@ -3391,7 +3418,7 @@ show_ps(ulong flag, struct psinfo *psi)
if (flag & PS_TIMES)
show_task_times(tc, flag);
else
- show_ps_data(flag, tc, NULL);
+ show_ps_data(flag, tc, psi);
}
}
}
@@ -3546,7 +3573,7 @@ show_milliseconds(struct task_context *tc, struct psinfo *psi)
sprintf(format, "[%c%dll%c] ", '%', c,
pc->output_radix == 10 ? 'u' : 'x');
- if (psi) {
+ if (psi && psi->cpus) {
for (c = others = 0; c < kt->cpus; c++) {
if (!NUM_IN_BITMAP(psi->cpus, c))
continue;
@@ -5366,6 +5393,27 @@ task_flags(ulong task)
}
/*
+ * Return task's policy as bitmask bit.
+ */
+static ulong
+task_policy(ulong task)
+{
+ ulong policy = 0;
+
+ fill_task_struct(task);
+
+ if (!tt->last_task_read)
+ return policy;
+
+ if (SIZE(task_struct_policy) == sizeof(unsigned int))
+ policy = 1 << UINT(tt->task_struct + OFFSET(task_struct_policy));
+ else
+ policy = 1 << ULONG(tt->task_struct + OFFSET(task_struct_policy));
+
+ return policy;
+}
+
+/*
* Return a task's tgid.
*/
ulong
@@ -5797,7 +5845,7 @@ cmd_foreach(void)
BZERO(&foreach_data, sizeof(struct foreach_data));
fd = &foreach_data;
- while ((c = getopt(argcnt, args, "R:vomlgersStTpukcfFxhdaG")) != EOF) {
+ while ((c = getopt(argcnt, args, "R:vomlgersStTpukcfFxhdaGy:")) != EOF) {
switch(c)
{
case 'R':
@@ -5892,6 +5940,11 @@ cmd_foreach(void)
fd->flags |= FOREACH_G_FLAG;
break;
+ case 'y':
+ fd->flags |= FOREACH_y_FLAG;
+ fd->policy = make_sched_policy(optarg);
+ break;
+
default:
argerrs++;
break;
@@ -6554,6 +6607,10 @@ foreach(struct foreach_data *fd)
cmdflags |= PS_GROUP;
if (fd->flags & FOREACH_s_FLAG)
cmdflags |= PS_KSTACKP;
+ if (fd->flags & FOREACH_y_FLAG) {
+ cmdflags |= PS_POLICY;
+ psinfo.policy = fd->policy;
+ }
/*
* mutually exclusive flags
*/
@@ -7389,6 +7446,70 @@ is_kernel_thread(ulong task)
}
/*
+ * Checks if task policy corresponds to given mask.
+ */
+static int
+has_sched_policy(ulong task, ulong policy)
+{
+ return !!(task_policy(task) & policy);
+}
+
+/*
+ * Converts sched policy name into mask bit.
+ */
+static ulong
+sched_policy_bit_from_str(const char *policy_str)
+{
+ struct sched_policy_info *info = NULL;
+ ulong policy = 0;
+ int found = 0;
+ char *upper = NULL;
+ char digit[2] = { 0, 0 };
+
+ upper = GETBUF(strlen(policy_str) + 1);
+ upper_case(policy_str, upper);
+ for (info = sched_policy_info; info->name; info++) {
+ snprintf(digit, sizeof digit, "%lu", info->value);
+ if (strncmp(upper, info->name, strlen(info->name)) == 0 ||
+ strncmp(upper, digit, sizeof digit) == 0) {
+ policy = 1 << info->value;
+ found = 1;
+ break;
+ }
+ }
+
+ FREEBUF(upper);
+
+ if (!found)
+ error(FATAL,
+ "%s: invalid scheduling policy\n", policy_str);
+
+ return policy;
+}
+
+/*
+ * Converts sched policy string set into bitmask.
+ */
+static ulong
+make_sched_policy(const char *policy_str)
+{
+ ulong policy = 0;
+ char *iter = NULL;
+ char *orig = NULL;
+ char *cur = NULL;
+
+ iter = STRDUPBUF(policy_str);
+ orig = iter;
+
+ while ((cur = strsep(&iter, ",")))
+ policy |= sched_policy_bit_from_str(cur);
+
+ FREEBUF(orig);
+
+ return policy;
+}
+
+/*
* Gather an arry of pointers to the per-cpu idle tasks. The tasklist
* argument must be at least the size of ulong[NR_CPUS]. There may be
* junk in everything after the first entry on a single CPU box, so the
diff --git a/tools.c b/tools.c
index 886d7fb..186b703 100644
--- a/tools.c
+++ b/tools.c
@@ -423,9 +423,10 @@ strip_hex(char *line)
* Turn a string into upper-case.
*/
char *
-upper_case(char *s, char *buf)
+upper_case(const char *s, char *buf)
{
- char *p1, *p2;
+ const char *p1;
+ char *p2;
p1 = s;
p2 = buf;
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
