| Message ID | 20191003041909.23187-19-amitay@ozlabs.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add system device tree to libpdbg | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (8a10a05c89db666bf98734139334166da7c370a4) |
| snowpatch_ozlabs/build-multiarch | success | Test build-multiarch on branch master |
On Thursday, 3 October 2019 2:18:57 PM AEDT Amitay Isaacs wrote: > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > --- > src/main.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/src/main.c b/src/main.c > index f7f891a..09c236e 100644 > --- a/src/main.c > +++ b/src/main.c > @@ -513,6 +513,21 @@ static bool parse_options(int argc, char *argv[]) > return true; > } > > +static bool child_enabled(struct pdbg_target *target) > +{ > + struct pdbg_target *child; > + > + pdbg_for_each_child_target(target, child) { > + if (child_enabled(child)) > + return true; We shouldn't have to re-curse down the tree here. If a child target is enabled it's parent must also be enabled. Unless this series has changed something here? > + if (pdbg_target_status(child) == PDBG_TARGET_ENABLED) > + return true; > + } > + > + return false; > +} > + > static void print_target(struct pdbg_target *target, int level) > { > int i; > @@ -520,6 +535,9 @@ static void print_target(struct pdbg_target *target, int level) > enum pdbg_target_status status; > const char *classname; > > + if (level == 0 && !child_enabled(target)) I must have missed something as I don't understand why this is necessary. The top level target should only be enabled if explicitly selected on the command line or if any of it's children are enabled so in either case it should be printed. - Alistair > + return; > + > /* Does this target actually exist? */ > status = pdbg_target_status(target); > if (status != PDBG_TARGET_ENABLED) >
On Wed, 2019-10-09 at 15:20 +1100, Alistair Popple wrote: > On Thursday, 3 October 2019 2:18:57 PM AEDT Amitay Isaacs wrote: > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > > --- > > src/main.c | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/src/main.c b/src/main.c > > index f7f891a..09c236e 100644 > > --- a/src/main.c > > +++ b/src/main.c > > @@ -513,6 +513,21 @@ static bool parse_options(int argc, char > > *argv[]) > > return true; > > } > > > > +static bool child_enabled(struct pdbg_target *target) > > +{ > > + struct pdbg_target *child; > > + > > + pdbg_for_each_child_target(target, child) { > > + if (child_enabled(child)) > > + return true; > > We shouldn't have to re-curse down the tree here. If a child target > is enabled > it's parent must also be enabled. Unless this series has changed > something > here? > > > + if (pdbg_target_status(child) == PDBG_TARGET_ENABLED) > > + return true; > > + } > > + > > + return false; > > +} > > + > > static void print_target(struct pdbg_target *target, int level) > > { > > int i; > > @@ -520,6 +535,9 @@ static void print_target(struct pdbg_target > > *target, int > level) > > enum pdbg_target_status status; > > const char *classname; > > > > + if (level == 0 && !child_enabled(target)) > > I must have missed something as I don't understand why this is > necessary. The > top level target should only be enabled if explicitly selected on the > command > line or if any of it's children are enabled so in either case it > should be > printed. > > - Alistair > > > + return; > > + > > /* Does this target actually exist? */ > > status = pdbg_target_status(target); > > if (status != PDBG_TARGET_ENABLED) > > > > > pdbg -a probe on any of the palm machines would produce following output: # pdbg -a probe proc0: fsi0: Kernel based FSI master (*) pib0: POWER FSI2PIB (*) core9: POWER8 Core (*) thread0: POWER8 Thread (*) thread1: POWER8 Thread (*) thread2: POWER8 Thread (*) thread3: POWER8 Thread (*) thread4: POWER8 Thread (*) thread5: POWER8 Thread (*) thread6: POWER8 Thread (*) thread7: POWER8 Thread (*) core11: POWER8 Core (*) thread0: POWER8 Thread (*) thread1: POWER8 Thread (*) thread2: POWER8 Thread (*) thread3: POWER8 Thread (*) thread4: POWER8 Thread (*) thread5: POWER8 Thread (*) thread6: POWER8 Thread (*) thread7: POWER8 Thread (*) core12: POWER8 Core (*) thread0: POWER8 Thread (*) thread1: POWER8 Thread (*) thread2: POWER8 Thread (*) thread3: POWER8 Thread (*) thread4: POWER8 Thread (*) thread5: POWER8 Thread (*) thread6: POWER8 Thread (*) thread7: POWER8 Thread (*) core13: POWER8 Core (*) thread0: POWER8 Thread (*) thread1: POWER8 Thread (*) thread2: POWER8 Thread (*) thread3: POWER8 Thread (*) thread4: POWER8 Thread (*) thread5: POWER8 Thread (*) thread6: POWER8 Thread (*) thread7: POWER8 Thread (*) proc1: Since proc1 is virtual, it gets enabled and printed in the output. This patch is to avoid priting proc1 in the probe output. Amitay.
> pdbg -a probe on any of the palm machines would produce following output: > > # pdbg -a probe > proc0: > fsi0: Kernel based FSI master (*) > pib0: POWER FSI2PIB (*) > core9: POWER8 Core (*) > thread0: POWER8 Thread (*) > thread1: POWER8 Thread (*) > thread2: POWER8 Thread (*) > thread3: POWER8 Thread (*) > thread4: POWER8 Thread (*) > thread5: POWER8 Thread (*) > thread6: POWER8 Thread (*) > thread7: POWER8 Thread (*) > core11: POWER8 Core (*) > thread0: POWER8 Thread (*) > thread1: POWER8 Thread (*) > thread2: POWER8 Thread (*) > thread3: POWER8 Thread (*) > thread4: POWER8 Thread (*) > thread5: POWER8 Thread (*) > thread6: POWER8 Thread (*) > thread7: POWER8 Thread (*) > core12: POWER8 Core (*) > thread0: POWER8 Thread (*) > thread1: POWER8 Thread (*) > thread2: POWER8 Thread (*) > thread3: POWER8 Thread (*) > thread4: POWER8 Thread (*) > thread5: POWER8 Thread (*) > thread6: POWER8 Thread (*) > thread7: POWER8 Thread (*) > core13: POWER8 Core (*) > thread0: POWER8 Thread (*) > thread1: POWER8 Thread (*) > thread2: POWER8 Thread (*) > thread3: POWER8 Thread (*) > thread4: POWER8 Thread (*) > thread5: POWER8 Thread (*) > thread6: POWER8 Thread (*) > thread7: POWER8 Thread (*) > proc1: > > > Since proc1 is virtual, it gets enabled and printed in the output. This patch is to avoid priting proc1 in the probe output. Ok, figured this may have been the answer. But I think the probe should just leave virtual nodes without any children disabled in the first place (must've missed this when looking at that patch). Unless there is some other reason we can't do this? - Alistair > Amitay. >
diff --git a/src/main.c b/src/main.c index f7f891a..09c236e 100644 --- a/src/main.c +++ b/src/main.c @@ -513,6 +513,21 @@ static bool parse_options(int argc, char *argv[]) return true; } +static bool child_enabled(struct pdbg_target *target) +{ + struct pdbg_target *child; + + pdbg_for_each_child_target(target, child) { + if (child_enabled(child)) + return true; + + if (pdbg_target_status(child) == PDBG_TARGET_ENABLED) + return true; + } + + return false; +} + static void print_target(struct pdbg_target *target, int level) { int i; @@ -520,6 +535,9 @@ static void print_target(struct pdbg_target *target, int level) enum pdbg_target_status status; const char *classname; + if (level == 0 && !child_enabled(target)) + return; + /* Does this target actually exist? */ status = pdbg_target_status(target); if (status != PDBG_TARGET_ENABLED)
Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> --- src/main.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)