4:15 p.m.
There still are a couple of things which need to be done, viz
1. Extend to obtaining unwind info from modules as well(currently
doing only for the kernel)
2. Currently reading the unwind info from eh_frame section only(ie
__start_unwind to __end_unwind). Need to add facility to read from
the .debug_frame(if .debug_frame is present in cases where .eh_frame
is absent. Will have to read from the vmlinux if we want to read the
.debug_frame info)
Hi Rachita,
I hope to be able to come up with a new crash version
for you to continue working with by tomorrow, Monday at
the latest.
Off the top of my head, here's what I've done with your
initial patch:
1. As Ben mentioned, it need to be made compilable for
other architectures.
2. Renamed unwind_x86_64.c into unwind_x86_32_64.c,
because the unwind code should be architecture
neutral with respect to x86 and x86_64. It's currently
#ifdef'd to only be compile if X86_64, but when a
new "unwind_x86.h" file is ready to go, it can be
made usable by both arches.
3. Made it capable of reading .eh_frame data from the
vmlinux file if it is not in memory.
4. Made it capable of reading all of the module's unwind
tables.
5. Restored the unwind() function to reflect the kernel
version in that it new uses a new find_table() routine,
which returns a pointer to the local copy of the unwind
that contains the incoming pc.
6. Cleaned up a bunch of cruft...
A couple other notes:
I've restored the original x86_64_low_budget_back_trace_cmd()
function, and renamed your modified version of it. I can't
risk breaking the original, and having a new version that
can be swapped in dynamically to the machdep->back_trace
pointer will allow us to tinker with the new one. Ideally
the new one could continue to use the multiple stack walkers
of the original function, but as they walk through the stacks
from a "known good" starting point, they could use the new
unwind functionality to simply calculate the frame size,
and thereby skip over the leftovers -- as opposed to laboriously
printing every kernel text return address found -- and as
opposed to the way you send it off to dwarf_backtrace().
That being said, your dwarf_backtrace() function is an
excellent proof-of-concept!
The same thing goes for the x86 port. In a perfect scenario,
the back trace code can be left pretty much untouched
*except* for one key function, that being the get_framesize()
function in lkcd_x86_trace.c. As the backtrace code is
walking through the proces and IRQ stacks, it starts with
the "known good" PC, and calls get_framesize() to determine
how much of the stack to skip over to find the next text
return reference. That code has been hacked to hell, and
as the compiler has gotten more and more clever, it's been
more and more prone to miscalculating the frame size. It
disassembles the code from the start of the containing function
of the passed-in PC up to the PC, counts pushes and pops,
subtractions of the stack pointer, tries to handle embedded
returns, etc, etc. -- it's a nightmare. But it seemingly
could take the passed in PC, and just pass it the unwind
code to get a "confident" frame size, and go from there.
That's what I'm hoping for, anyway...
Anyway, the use of the new unwind info is turned "off" by
default, but you can turn it on during runtime by entering
"set unwind on", or turn it off by entering "set unwind off".
Of course it will not allow you to turn it on if the kernel
and vmlinux both don't contain any unwind info.
BTW, I also tried using the .debug_frame section of the vmlinux
file, but it apparently can not be used by the ported
kernel unwind code. The kernel unwind code presumes the
data is from the .eh_frame section; the .debug_frame section
must use a different layout.
This is looking good...
Thanks,
Dave
3:51 p.m.
New subject: [RFC] Crash patch for DWARF CFI based unwind support
On Thu, Oct 19, 2006 at 05:15:32PM -0400, Dave Anderson wrote:
> There still are a couple of things which need to be done, viz
> 1. Extend to obtaining unwind info from modules as well(currently
> doing only for the kernel)
> 2. Currently reading the unwind info from eh_frame section only(ie
> __start_unwind to __end_unwind). Need to add facility to read from
> the .debug_frame(if .debug_frame is present in cases where .eh_frame
> is absent. Will have to read from the vmlinux if we want to read the
> .debug_frame info)
Hi Rachita,
I hope to be able to come up with a new crash version
for you to continue working with by tomorrow, Monday at
the latest.
Off the top of my head, here's what I've done with your
initial patch:
1. As Ben mentioned, it need to be made compilable for
other architectures.
2. Renamed unwind_x86_64.c into unwind_x86_32_64.c,
because the unwind code should be architecture
neutral with respect to x86 and x86_64. It's currently
#ifdef'd to only be compile if X86_64, but when a
new "unwind_x86.h" file is ready to go, it can be
made usable by both arches.
3. Made it capable of reading .eh_frame data from the
vmlinux file if it is not in memory.
4. Made it capable of reading all of the module's unwind
tables.
5. Restored the unwind() function to reflect the kernel
version in that it new uses a new find_table() routine,
which returns a pointer to the local copy of the unwind
that contains the incoming pc.
6. Cleaned up a bunch of cruft...
Hi Dave
Thanks! I will try the new crash first thing tomorrow morning :)
Rachita
6 a.m.
New subject: [RFC] Crash patch for DWARF CFI based unwind support
On Thu, Oct 19, 2006 at 05:15:32PM -0400, Dave Anderson wrote:
> There still are a couple of things which need to be done, viz
> 1. Extend to obtaining unwind info from modules as well(currently
> doing only for the kernel)
> 2. Currently reading the unwind info from eh_frame section only(ie
> __start_unwind to __end_unwind). Need to add facility to read from
> the .debug_frame(if .debug_frame is present in cases where .eh_frame
> is absent. Will have to read from the vmlinux if we want to read the
> .debug_frame info)
Hi Rachita,
I hope to be able to come up with a new crash version
for you to continue working with by tomorrow, Monday at
the latest.
Off the top of my head, here's what I've done with your
initial patch:
1. As Ben mentioned, it need to be made compilable for
other architectures.
2. Renamed unwind_x86_64.c into unwind_x86_32_64.c,
because the unwind code should be architecture
neutral with respect to x86 and x86_64. It's currently
#ifdef'd to only be compile if X86_64, but when a
new "unwind_x86.h" file is ready to go, it can be
made usable by both arches.
3. Made it capable of reading .eh_frame data from the
vmlinux file if it is not in memory.
4. Made it capable of reading all of the module's unwind
tables.
5. Restored the unwind() function to reflect the kernel
version in that it new uses a new find_table() routine,
which returns a pointer to the local copy of the unwind
that contains the incoming pc.
6. Cleaned up a bunch of cruft...
Hi Dave
On the panic task, when we do the following:
set unwind on
bt
set unwind off
bt
This last bt does not give us the same backtrace as what we get when crash
first starts up(ie unwind is off by default). What is happening here is, when
unwind is set to on, and we do a 'bt', we go to get_netdump_regs_x86_64() to get
rsp and rip, where ASSIGN_SIZE(user_regs_struct) happens, thereby setting
VALID_STRUCT(user_regs_struct) to 1. Now when we next do 'set unwind off' and
'bt', we satisfy the following if condition in get_netdump_regs_x86_64() as
VALID_STRUCT(user_regs_struct) is set:
if (((NETDUMP_DUMPFILE() || KDUMP_DUMPFILE()) &&
VALID_STRUCT(user_regs_struct) && (bt->task == tt->panic_task)) ||
(KDUMP_DUMPFILE() && (kt->flags & DWARF_UNWIND) &&
(bt->flags & BT_DUMPFILE_SEARCH))) {
So this results in it reading the register values from the NT_PRSTATUS.
Hence the backtrace looks different from what we get from the existing
non-dwarf mechanism.
To avoid this, we could use a local variable for the user_regs_struct size
instead of changing things at the global scope with ASSIGN_SIZE(). Or
invalidate the user_regs_struct before we leave from get_netdump_regs_x86_64().
Or, if it is desired that registers be read for the panic task from the
NT_PRSTATUS section in the normal non-dwarf backtrace mechanism (which
currently does not work as expected because of the user_regs_struct
initialisation problem in x86_64), then probably it will have to be fixed
some other way.
Thanks
Rachita
8:31 a.m.
New subject: [RFC] Crash patch for DWARF CFI based unwind support
Rachita Kothiyal wrote:
On Thu, Oct 19, 2006 at 05:15:32PM -0400, Dave Anderson wrote:
>
> > There still are a couple of things which need to be done, viz
> > 1. Extend to obtaining unwind info from modules as well(currently
> > doing only for the kernel)
> > 2. Currently reading the unwind info from eh_frame section only(ie
> > __start_unwind to __end_unwind). Need to add facility to read from
> > the .debug_frame(if .debug_frame is present in cases where .eh_frame
> > is absent. Will have to read from the vmlinux if we want to read the
> > .debug_frame info)
>
> Hi Rachita,
>
> I hope to be able to come up with a new crash version
> for you to continue working with by tomorrow, Monday at
> the latest.
>
> Off the top of my head, here's what I've done with your
> initial patch:
>
> 1. As Ben mentioned, it need to be made compilable for
> other architectures.
> 2. Renamed unwind_x86_64.c into unwind_x86_32_64.c,
> because the unwind code should be architecture
> neutral with respect to x86 and x86_64. It's currently
> #ifdef'd to only be compile if X86_64, but when a
> new "unwind_x86.h" file is ready to go, it can be
> made usable by both arches.
> 3. Made it capable of reading .eh_frame data from the
> vmlinux file if it is not in memory.
> 4. Made it capable of reading all of the module's unwind
> tables.
> 5. Restored the unwind() function to reflect the kernel
> version in that it new uses a new find_table() routine,
> which returns a pointer to the local copy of the unwind
> that contains the incoming pc.
> 6. Cleaned up a bunch of cruft...
>
Hi Dave
On the panic task, when we do the following:
set unwind on
bt
set unwind off
bt
This last bt does not give us the same backtrace as what we get when crash
first starts up(ie unwind is off by default). What is happening here is, when
unwind is set to on, and we do a 'bt', we go to get_netdump_regs_x86_64() to get
rsp and rip, where ASSIGN_SIZE(user_regs_struct) happens, thereby setting
VALID_STRUCT(user_regs_struct) to 1. Now when we next do 'set unwind off' and
'bt', we satisfy the following if condition in get_netdump_regs_x86_64() as
VALID_STRUCT(user_regs_struct) is set:
if (((NETDUMP_DUMPFILE() || KDUMP_DUMPFILE()) &&
VALID_STRUCT(user_regs_struct) && (bt->task == tt->panic_task))
|| (KDUMP_DUMPFILE() && (kt->flags & DWARF_UNWIND) &&
(bt->flags & BT_DUMPFILE_SEARCH))) {
So this results in it reading the register values from the NT_PRSTATUS.
Hence the backtrace looks different from what we get from the existing
non-dwarf mechanism.
To avoid this, we could use a local variable for the user_regs_struct size
instead of changing things at the global scope with ASSIGN_SIZE(). Or
invalidate the user_regs_struct before we leave from get_netdump_regs_x86_64().
Or, if it is desired that registers be read for the panic task from the
NT_PRSTATUS section in the normal non-dwarf backtrace mechanism (which
currently does not work as expected because of the user_regs_struct
initialisation problem in x86_64), then probably it will have to be fixed
some other way.
Hmmm, yeah, good catch...
But what happens the second time around, anyway? Are the RSP/RIP
starting points so different such that the low_budget tracer's output
is so drastically different? Or does it go off into the weeds because
the other user_regs_struct register offsets (that don't get initialized)
cause an OFFSET() failure?
Dave
1:02 p.m.
New subject: [RFC] Crash patch for DWARF CFI based unwind support
On Mon, Oct 23, 2006 at 09:31:11AM -0400, Dave Anderson wrote:
Hmmm, yeah, good catch...
But what happens the second time around, anyway? Are the RSP/RIP
starting points so different such that the low_budget tracer's output
is so drastically different? Or does it go off into the weeds because
the other user_regs_struct register offsets (that don't get initialized)
cause an OFFSET() failure?
Hi Dave
This is what I get when I try it out on one my dumps:
crash> bt // ----------------------------------------------> 1
PID: 4102 TASK: ffff81022e94d1e0 CPU: 0 COMMAND: "bash"
#0 [ffff810223d73d78] crash_kexec at ffffffff801521d1
#1 [ffff810223d73dc0] machine_kexec at ffffffff8011a739
#2 [ffff810223d73e00] crash_kexec at ffffffff801521ed
#3 [ffff810223d73e88] crash_kexec at ffffffff801521d1
#4 [ffff810223d73eb8] __sysrq_get_key_op at ffffffff80288b29
#5 [ffff810223d73ec0] __handle_sysrq at ffffffff80288d37
#6 [ffff810223d73f00] write_sysrq_trigger at ffffffff801adf95
#7 [ffff810223d73f10] vfs_write at ffffffff80179bf0
#8 [ffff810223d73f40] sys_write at ffffffff8017a187
#9 [ffff810223d73f80] system_call at ffffffff801096da
RIP: 00002b3979ef3900 RSP: 00007fff3122f360 RFLAGS: 00010287
RAX: 0000000000000001 RBX: ffffffff801096da RCX: 00000000fbad2a84
RDX: 0000000000000002 RSI: 00002b397a181000 RDI: 0000000000000001
RBP: 0000000000000002 R8: 00000000ffffffff R9: 00002b397a072ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 00002b397a06c780
R13: 00002b397a181000 R14: 0000000000000002 R15: 0000000000000000
ORIG_RAX: 0000000000000001 CS: 0033 SS: 002b
crash> set unwind on // ------------------------------------------> 2
unwind: on
crash> bt
PID: 4102 TASK: ffff81022e94d1e0 CPU: 0 COMMAND: "bash"
#0 [ffff810223d73e08] crash_kexec at ffffffff801521d1
#1 [ffff810223d73ec8] __handle_sysrq at ffffffff80288d37
#2 [ffff810223d73f08] write_sysrq_trigger at ffffffff801adf95
#3 [ffff810223d73f18] vfs_write at ffffffff80179bf0
#4 [ffff810223d73f48] sys_write at ffffffff8017a187
#5 [ffff810223d73f88] system_call at ffffffff801096da
RIP: 00002b3979ef3900 RSP: 00007fff3122f360 RFLAGS: 00010287
RAX: 0000000000000001 RBX: ffffffff801096da RCX: 00000000fbad2a84
RDX: 0000000000000002 RSI: 00002b397a181000 RDI: 0000000000000001
RBP: 0000000000000002 R8: 00000000ffffffff R9: 00002b397a072ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 00002b397a06c780
R13: 00002b397a181000 R14: 0000000000000002 R15: 0000000000000000
ORIG_RAX: 0000000000000001 CS: 0033 SS: 002b
crash> set unwind off //-------------------------------------> 3
unwind: off
crash> bt
PID: 4102 TASK: ffff81022e94d1e0 CPU: 0 COMMAND: "bash"
#0 [ffff810223d73e88] crash_kexec at ffffffff801521d1
#1 [ffff810223d73eb8] __sysrq_get_key_op at ffffffff80288b29
#2 [ffff810223d73ec0] __handle_sysrq at ffffffff80288d37
#3 [ffff810223d73f00] write_sysrq_trigger at ffffffff801adf95
#4 [ffff810223d73f10] vfs_write at ffffffff80179bf0
#5 [ffff810223d73f40] sys_write at ffffffff8017a187
#6 [ffff810223d73f80] system_call at ffffffff801096da
RIP: 00002b3979ef3900 RSP: 00007fff3122f360 RFLAGS: 00010287
RAX: 0000000000000001 RBX: ffffffff801096da RCX: 00000000fbad2a84
RDX: 0000000000000002 RSI: 00002b397a181000 RDI: 0000000000000001
RBP: 0000000000000002 R8: 00000000ffffffff R9: 00002b397a072ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 00002b397a06c780
R13: 00002b397a181000 R14: 0000000000000002 R15: 0000000000000000
ORIG_RAX: 0000000000000001 CS: 0033 SS: 002b
The register values which the last bt starts working with(marked 3 above),
are rsp=ffff810223d73e08 and rip=ffffffff801521d1 (from NT_PRSTATUS).
So from that point in stack, output 3 and 1 are same.
We also see that stack addresses in 2 and 3 are off by '0x8'.
eg #1 ffff810223d73ec8 ------------> 2
ffff810223d73ec0 ------------> 3
This is because what crash is reporting is the stack address at which
the return address was pushed on stack, while what the dwarf based bt is
reporting is the CFA. In most cases, return address is stored at a location
(CFA - 8). That is why the offset of 0x8.
The low-budget tracer's backtraces are different from the dwarf-tracer
because when the low-budget tracer is unwinding the stack by trying to read
kernel text addresses, it actually comes across many addresses which were
actually not pushed onto stack because of function calls.
Specially for the panic task on kdumps, where after 'crash_kexec' is called,
the registers are dumped onto stack(for creating NT_PRSTATUS section), this
becomes misleading for the low-budget tracer mechanism. Thats why we see
multiple crash_kexec entries in the backtrace. Static inline functions can
also aggrevate this problem.
In other cases, stale frames on the stack can also mislead the low-budget
tracer.
AFAICT, user_regs_struct register offsets are not the culprits here.
Thanks
Rachita
1:34 p.m.
New subject: [RFC] Crash patch for DWARF CFI based unwind support
Rachita Kothiyal wrote:
On Mon, Oct 23, 2006 at 09:31:11AM -0400, Dave Anderson wrote:
>
> Hmmm, yeah, good catch...
>
> But what happens the second time around, anyway? Are the RSP/RIP
> starting points so different such that the low_budget tracer's output
> is so drastically different? Or does it go off into the weeds because
> the other user_regs_struct register offsets (that don't get initialized)
> cause an OFFSET() failure?
>
Hi Dave
This is what I get when I try it out on one my dumps:
crash> bt // ----------------------------------------------> 1
PID: 4102 TASK: ffff81022e94d1e0 CPU: 0 COMMAND: "bash"
#0 [ffff810223d73d78] crash_kexec at ffffffff801521d1
#1 [ffff810223d73dc0] machine_kexec at ffffffff8011a739
#2 [ffff810223d73e00] crash_kexec at ffffffff801521ed
#3 [ffff810223d73e88] crash_kexec at ffffffff801521d1
#4 [ffff810223d73eb8] __sysrq_get_key_op at ffffffff80288b29
#5 [ffff810223d73ec0] __handle_sysrq at ffffffff80288d37
#6 [ffff810223d73f00] write_sysrq_trigger at ffffffff801adf95
#7 [ffff810223d73f10] vfs_write at ffffffff80179bf0
#8 [ffff810223d73f40] sys_write at ffffffff8017a187
#9 [ffff810223d73f80] system_call at ffffffff801096da
RIP: 00002b3979ef3900 RSP: 00007fff3122f360 RFLAGS: 00010287
RAX: 0000000000000001 RBX: ffffffff801096da RCX: 00000000fbad2a84
RDX: 0000000000000002 RSI: 00002b397a181000 RDI: 0000000000000001
RBP: 0000000000000002 R8: 00000000ffffffff R9: 00002b397a072ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 00002b397a06c780
R13: 00002b397a181000 R14: 0000000000000002 R15: 0000000000000000
ORIG_RAX: 0000000000000001 CS: 0033 SS: 002b
crash> set unwind on // ------------------------------------------> 2
unwind: on
crash> bt
PID: 4102 TASK: ffff81022e94d1e0 CPU: 0 COMMAND: "bash"
#0 [ffff810223d73e08] crash_kexec at ffffffff801521d1
#1 [ffff810223d73ec8] __handle_sysrq at ffffffff80288d37
#2 [ffff810223d73f08] write_sysrq_trigger at ffffffff801adf95
#3 [ffff810223d73f18] vfs_write at ffffffff80179bf0
#4 [ffff810223d73f48] sys_write at ffffffff8017a187
#5 [ffff810223d73f88] system_call at ffffffff801096da
RIP: 00002b3979ef3900 RSP: 00007fff3122f360 RFLAGS: 00010287
RAX: 0000000000000001 RBX: ffffffff801096da RCX: 00000000fbad2a84
RDX: 0000000000000002 RSI: 00002b397a181000 RDI: 0000000000000001
RBP: 0000000000000002 R8: 00000000ffffffff R9: 00002b397a072ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 00002b397a06c780
R13: 00002b397a181000 R14: 0000000000000002 R15: 0000000000000000
ORIG_RAX: 0000000000000001 CS: 0033 SS: 002b
crash> set unwind off //-------------------------------------> 3
unwind: off
crash> bt
PID: 4102 TASK: ffff81022e94d1e0 CPU: 0 COMMAND: "bash"
#0 [ffff810223d73e88] crash_kexec at ffffffff801521d1
#1 [ffff810223d73eb8] __sysrq_get_key_op at ffffffff80288b29
#2 [ffff810223d73ec0] __handle_sysrq at ffffffff80288d37
#3 [ffff810223d73f00] write_sysrq_trigger at ffffffff801adf95
#4 [ffff810223d73f10] vfs_write at ffffffff80179bf0
#5 [ffff810223d73f40] sys_write at ffffffff8017a187
#6 [ffff810223d73f80] system_call at ffffffff801096da
RIP: 00002b3979ef3900 RSP: 00007fff3122f360 RFLAGS: 00010287
RAX: 0000000000000001 RBX: ffffffff801096da RCX: 00000000fbad2a84
RDX: 0000000000000002 RSI: 00002b397a181000 RDI: 0000000000000001
RBP: 0000000000000002 R8: 00000000ffffffff R9: 00002b397a072ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 00002b397a06c780
R13: 00002b397a181000 R14: 0000000000000002 R15: 0000000000000000
ORIG_RAX: 0000000000000001 CS: 0033 SS: 002b
The register values which the last bt starts working with(marked 3 above),
are rsp=ffff810223d73e08 and rip=ffffffff801521d1 (from NT_PRSTATUS).
So from that point in stack, output 3 and 1 are same.
We also see that stack addresses in 2 and 3 are off by '0x8'.
eg #1 ffff810223d73ec8 ------------> 2
ffff810223d73ec0 ------------> 3
This is because what crash is reporting is the stack address at which
the return address was pushed on stack, while what the dwarf based bt is
reporting is the CFA. In most cases, return address is stored at a location
(CFA - 8). That is why the offset of 0x8.
The low-budget tracer's backtraces are different from the dwarf-tracer
because when the low-budget tracer is unwinding the stack by trying to read
kernel text addresses, it actually comes across many addresses which were
actually not pushed onto stack because of function calls.
Specially for the panic task on kdumps, where after 'crash_kexec' is called,
the registers are dumped onto stack(for creating NT_PRSTATUS section), this
becomes misleading for the low-budget tracer mechanism. Thats why we see
multiple crash_kexec entries in the backtrace. Static inline functions can
also aggrevate this problem.
In other cases, stale frames on the stack can also mislead the low-budget
tracer.
AFAICT, user_regs_struct register offsets are not the culprits here.
Thanks
Rachita
So, in other words, if we hardwire the user_regs_struct so that
it uses the NT_PRSTATUS registers all the time, then we get
the second (preferred/better) budget back trace when unwind
is off.
That being the case, I argue for hardwiring them all the time.
Dave
1:57 p.m.
New subject: [RFC] Crash patch for DWARF CFI based unwind support
On Mon, Oct 23, 2006 at 02:34:32PM -0400, Dave Anderson wrote:
Rachita Kothiyal wrote:
> This is because what crash is reporting is the stack address at which
> the return address was pushed on stack, while what the dwarf based bt is
> reporting is the CFA. In most cases, return address is stored at a location
> (CFA - 8). That is why the offset of 0x8.
>
> The low-budget tracer's backtraces are different from the dwarf-tracer
> because when the low-budget tracer is unwinding the stack by trying to read
> kernel text addresses, it actually comes across many addresses which were
> actually not pushed onto stack because of function calls.
> Specially for the panic task on kdumps, where after 'crash_kexec' is
called,
> the registers are dumped onto stack(for creating NT_PRSTATUS section), this
> becomes misleading for the low-budget tracer mechanism. Thats why we see
> multiple crash_kexec entries in the backtrace. Static inline functions can
> also aggrevate this problem.
>
> In other cases, stale frames on the stack can also mislead the low-budget
> tracer.
>
> AFAICT, user_regs_struct register offsets are not the culprits here.
>
> Thanks
> Rachita
So, in other words, if we hardwire the user_regs_struct so that
it uses the NT_PRSTATUS registers all the time, then we get
the second (preferred/better) budget back trace when unwind
is off.
That being the case, I argue for hardwiring them all the time.
Yes, we can(should) do that for all the active tasks.
Rachita
6639
days inactive
6643
days old
devel@lists.crash-utility.osci.io
6 comments
2 participants
participants (2)
-
Dave Anderson -
Rachita Kothiyal