Re: [PATCH 08/28] kdb: core for kgdb back end (2 of 2)

[ebiederm@xmission.com (Eric W. Biederman)]

Jason Wessel <jason.wessel@windriver.com> writes:

> Eric W. Biederman wrote:
>> Jason Wessel <jason.wessel@windriver.com> writes:
>>
>>   
>>> This patch contains the hooks and instrumentation into kernel which
>>> live outside the kernel/debug directory, which the kdb core
>>> will call to run commands like lsmod, dmesg, bt etc...
>>>     
>>
>> You know this dropping the locks from vmalloc_info and swap_info
>> is down right ugly, and I don't believe it is safe.  That code
>> was not designed to run while the write_lock is held.
>>   
>
> Perhaps we can find some middle ground.  I don't mind simply not
> allowing the information to be queried from kdb if the locks are not
> available. 
>
> Which is less ugly you, making the swap_lock global or adding a function
> to query it?

My recommendation would be to simply drop the swap_info and meminfo
information for now, and have kdb do what is good at.

Skimming through the history of the discussion it appears Christoph
Hellwig asked you to do something about these bits on the first
review.

From a maintenance point of view anything where you have to know which
locks a function will take is fragile.  It is entirely too easy to create
a change that is fine in it's normal context but breaks kdb.

Alt-sysrq already allows capturing that kind of information, without any
thorny maintenance issues.  By contrast kdb appears to be a much larger
and inferior tool.

Eric