[Crash-utility] [PATCH v2 1/1] task: also filter ps output by ->policy

Oleksandr Natalenko oleksandr at redhat.com
Tue Oct 24 06:15:54 UTC 2017


Hi.

Interesting catch, I think I should definitely use %lu here, but I'd
rather wonder why my GCC v7.2.0 didn't notice this while clang just
did…

Will post v3 shortly.

Thanks.


On Mon, Oct 23, 2017 at 9:55 PM, Dave Anderson <anderson at redhat.com> wrote:
> Hi Oleksandr,
>
> Can you take a look at this warning?
>
> $ make warn
> ... [ cut ] ...
> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6  task.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
> task.c: In function ‘sched_policy_bit_from_str’:
> task.c:7473:3: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 4 has type ‘ulong’ [-Wformat=]
>    snprintf(digit, sizeof digit, "%u", info->value);
>    ^
> task.c:7473:3: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 4 has type ‘ulong’ [-Wformat=]
> ...
>
> Thanks,
>   Dave
>
>
> ----- Original Message -----
>> This patch introduces -y option for ps builtin to filter
>> tasks by scheduling policy. Applicable to both standalone
>> ps invocation as well as via foreach.
>>
>> Signed-off-by: Oleksandr Natalenko <oleksandr at redhat.com>
>> ---
>>  defs.h  |  15 +++++++-
>>  help.c  |   7 +++-
>>  task.c  | 135
>>  +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>  tools.c |   5 ++-
>>  4 files changed, 152 insertions(+), 10 deletions(-)
>>
>> diff --git a/defs.h b/defs.h
>> index a694a66..e0e2c2b 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 {
>> @@ -1201,6 +1203,7 @@ struct offset_table {                    /* stash of
>> commonly-used offsets */
>>       long task_struct_pidhash_next;
>>       long task_struct_next_run;
>>       long task_struct_flags;
>> +     long task_struct_policy;
>>       long task_struct_sig;
>>       long task_struct_signal;
>>       long task_struct_blocked;
>> @@ -2134,6 +2137,7 @@ struct size_table {         /* stash of commonly-used
>> sizes */
>>       long tnt;
>>       long trace_print_flags;
>>       long task_struct_flags;
>> +     long task_struct_policy;
>>       long timer_base;
>>       long taint_flag;
>>       long nlmsghdr;
>> @@ -4573,6 +4577,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_)
>> @@ -4600,6 +4611,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)
>>
>> @@ -4617,6 +4629,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))
>> @@ -4820,7 +4833,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..d37adb7 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] [-s] [-p|-c|-t|-[l|m][-C cpu]|-a|-g|-r|-S|-y policy]\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",
>> @@ -1318,6 +1318,9 @@ char *help_ps[] = {
>>  "       -g  display tasks by thread group, of selected, or all, tasks.",
>>  "       -r  display resource limits (rlimits) of selected, or all, tasks.",
>>  "       -S  display a summary consisting of the number of tasks in a task
>>  state.",
>> +"-y policy  filter tasks by comma-separated list of scheduling policies.",
>> +"           Acceptable values: NORMAL, FIFO, RR, BATCH, ISO, IDLE,
>> DEADLINE",
>> +"           (case-insensitive) or integer equivalents, from 0 to 6."
>>  "\nEXAMPLES",
>>  "  Show the process status of all current tasks:\n",
>>  "    %s> ps",
>> diff --git a/task.c b/task.c
>> index 362822c..09d9c37 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;
>> @@ -5365,6 +5392,27 @@ task_flags(ulong task)
>>       return flags;
>>  }
>>
>> +/*
>> + * 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.
>>   */
>> @@ -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
>>                                */
>> @@ -7388,6 +7445,74 @@ is_kernel_thread(ulong task)
>>       return FALSE;
>>  }
>>
>> +/*
>> + * 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 = calloc(1, strlen(policy_str) + 1);
>> +     upper_case(policy_str, upper);
>> +
>> +     for (info = sched_policy_info; info->name; info++) {
>> +             snprintf(digit, sizeof digit, "%u", info->value);
>> +         if (strncmp(upper, info->name, strlen(info->name)) == 0 ||
>> +                     strncmp(upper, digit, sizeof digit) == 0) {
>> +                     policy = 1 << info->value;
>> +                     found = 1;
>> +                     break;
>> +             }
>> +     }
>> +
>> +     if (!found) {
>> +             error(INFO,
>> +                     "Scheduling policy \"%s\" is incorrect, defaulting to %s\n",
>> +                     policy_str, sched_policy_info[0].name);
>> +             policy = 1 << sched_policy_info[0].value;
>> +     }
>> +
>> +     free(upper);
>> +
>> +     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 = strdup(policy_str);
>> +     orig = iter;
>> +
>> +     while ((cur = strsep(&iter, ",")))
>> +             policy |= sched_policy_bit_from_str(cur);
>> +
>> +     free(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
>> 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;
>> --
>> 2.14.2
>>
>>



-- 
Best regards,
  Oleksandr Natalenko (post-factum)
  Software Maintenance Engineer




More information about the Crash-utility mailing list