On Thu, 10 Mar 2022, Leah Leshchinsky wrote:
> On Thu, Mar 10, 2022 at 12:17:36PM -0500, John Kacur wrote:
> >
> >
> > On Wed, 9 Mar 2022, Leah Leshchinsky wrote:
> >
> > > These patches introduce a new command-line interface that provides
> > > distinct subcommands for the user.
> > > Through the use of argparse, users see simpler help menus and can
> > > view positional and optional arguments for all tuna commands.
> > > The internal implementation is also simplified by removing the need
> > > for a saved state for each iteration of parsing.
> > >
> > > v2 Removes unnecessary sample.py file added in previous version,
> > > moves changes to show_irqs() from patch 1/3 to 3/3.
> > >
> > > Leah Leshchinsky (3):
> > > tuna: Update command-line interface
> > > tuna: Edit param variable to print full policy name
> > > tuna: Remove unused functions and globals
> > >
> > > tuna-cmd.py | 633 ++++++++++++++++++++-------------------------------
> > > tuna/tuna.py | 7 +-
> > > 2 files changed, 246 insertions(+), 394 deletions(-)
> > >
> >
> > This is a welcome change, thanks!
> >
> > Some initial comments from running this without looking at the code.
> >
> > /tuna-cmd.py -h
> > usage: tuna-cmd.py [-h] [-v]
> >
> > {isolate,include,move,spread,priority,run,save,apply,show,what_is,gui}
> > ...
> >
> > tuna - Application Tuning GUI
> >
> > subcommands:
> > {isolate,include,move,spread,priority,run,save,apply,show,what_is,gui}
> > isolate Allow all threads to run on CPU-LIST
> > include Allow all threads to run on CPU-LIST
> > move Move selected entities to CPU-LIST
> > spread Spread selected entities over CPU-LIST
> > priority Set thread scheduler tunables: POLICY and RTPRIO
> > run fork a new process and run the COMMAND
> > save Save kthreads sched tunables to FILENAME
> > apply Apply changes described in profile
> > show Show a list for given SUBCOMMAND
> > what_is Provides help about selected entities
> > gui Start the GUI
> >
> > options:
> > -h, --help show this help message and exit
> > -v, --version show version
> >
> > In the above the same message appears next to isolate and include.
> > isolate should say, "Move all allowed threads and IRQs away from
CPU-LIST"
> >
> > For include, don't forget to add the words "all allowed", this is
> > important.
> >
> > When I run help on the commands, I only get usage and options, I'd like to
> > see a little more information, or at least a repeat of the above lines.
> > (look at rhpkg, or fedpkg as an example)
> >
> > I know I suggested "subcommands" because that's what we call them
in
> > python argparse but from a user perspective, I think "Commands" makes
> > more sense.
> >
> > There appears to be missing functionality. Are you planning on adding that
> > in the future?
> >
> > For example, current tuna has -G Display the processes with the type of
> > cgroups they are in, which could be added to the show command.
> >
> > Also, current tuna allows you to specify that operations apply only to
> > kthreads or only to uthreads that could be added to a few commands as
> > well.
> >
> > After you make a few more changes (I need to look a bit more closely at
> > the implementation and comment there as well) I'll put this in a separate
> > branch, but I don't think it needs to live there very long, I'd like to
> > integrate this relatively soon.
> >
> > Thanks
> >
> > John
> >
>
> Hi John,
>
> Thank you for the review.
> I will make the necessary changes to the usage menus.
> The functionality for options like -D to display cgroups,
> and filtering by kthreads/uthreads is actually implemented
> and can be viewed by commands such as "tuna show threads -h",
> but I will add a note on the help menus to make it clearer
> to users that these optionals exist.
>
> Best,
> Leah
Hmnn, that is cool in a way, however I think one level of commands is
enough, otherwise that interface is confusing for users.
Here is my suggestion for tuna show
separate "config" as a separate command for processing configs.
with no arguments
tuna config
lists preloaded files like current tuna with -l
tuna config -a PROFILENAME
applies the profile without printing the preloaded files
Then for the tuna show command irqs and threads should be options
and it should be possible to have both options at the same time.
Something like this
tuna show [-h] [-P] [--show_threads] [-Q] [--show_irqs]
so it would be possible to do
tuna show -P -Q
to get a list of threads and irqs at the same time.
There still is some missing functionality, I think you need to go through
old tuna and reaudit what it can do. I know you are simplifying somethings
on purpose, because they were too complicated, but other things we don't
want to omit.
For example sockets. Sockets is an overloaded word, so that can be
confusing, but I'm talking about cpu sockets here. If you have a machine
with multiple cpu sockets you should be able to limit the output to the
sockets you are interested in. For example
tuna show [-h] [-P] [--show_threads] [-Q] [--show_irqs] [-S
CPU_SOCKET_NUM]
and the user can do tuna show -P -Q -S1
to see only socket 1 if available.
When it comes to things like -S that we find missing as we go over the
code, it's okay to add that in pieces, that doesn't have to be done all at
once, so we can put that in latter patches. Be careful about deleting
functions prematurely though that we still might need.
Thanks
John
_______________________________________________
tuna-devel mailing list -- tuna-devel(a)lists.fedorahosted.org
To unsubscribe send an email to tuna-devel-leave(a)lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/tuna-devel@lists.fedorahoste...
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
Hi John,
I believe the socket functionality is available, but it depends on
'have_inet_diag' evaluating to true.
This is to stay faithful to the original implementation of tuna, which conceals the
'--show_sockets' option
on systems that do not support it. I can see how this can be confusing and prevent a user
from understanding
what functionality is available in the tool. Do you have any suggestions for how to
implement this features
in such a way that it is clear to the user that it exists, but prevents the use of the
argument when it cannot
be supported? Perhaps a simple warning message will do?
Best,
Leah