linux-kernel - Re: [PATCH 05/37] kdb: core for kgdb back end

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B3932D3.8020608@windriver.com>
Date:	Mon, 28 Dec 2009 16:36:03 -0600
From:	Jason Wessel <jason.wessel@...driver.com>
To:	Andi Kleen <andi@...stfloor.org>
CC:	linux-kernel@...r.kernel.org, kgdb-bugreport@...ts.sourceforge.net,
	Peter Zijlstra <peterz@...radead.org>, mingo@...e.hu,
	mort@....com, linux-arch@...r.kernel.org
Subject: Re: [PATCH 05/37] kdb: core for kgdb back end

Andi Kleen wrote:
> Jason Wessel <jason.wessel@...driver.com> writes:
> 
> I remember going with kaos through all the code
> outside kdb/ in his own patch and for nearly all hooks
> outside we found some way to eliminate them. 
> 
> I think a lot of this is here too.

If this has been solved before somewhere, please point me in that direction.  There are whole lot few changes in this series vs original kdb outside of the kdb core area.

I already split all the files out of the large kdb patch that are modified to support the kdb for the next time I post this series.  I included the updated version of the non-kdb files patch in this mail.

> 
>>  
>> +#ifdef	CONFIG_KGDB_KDB
>> +/* Like meminfo_proc_show() but without the locks and using kdb_printf() */
>> +void kdb_meminfo_proc_show(void)
> 
> Are there even any locks in meminfo_proc_show()? I don't see any on a quick look.
> Ah or is that only for swap_info? That could be a flag or perhaps that
> access can be even made lockless (it looks like it could)

Yup it definitely needs to be lockless but can be controlled with a flag.

> 
> I guess a better way would be to have a kdb specific seq file implementation
> and then just use the normal function, instead of copying everything.
> 

Sure, this seems a whole lot more reasonable than the bulk copy and maintenance of the function specifically for kdb.  I went ahead and implemented a (kdb_seq_file *) in the kdb core so I could just directly call the _meminfo_proc_show() without the locks.

>>  void get_vmalloc_info(struct vmalloc_info *vmi)
>>  {
>>  	struct vm_struct *vma;
>>  	unsigned long free_area_size;
>>  	unsigned long prev_end;
>> +#ifdef CONFIG_KGDB_KDB
>> +	int get_lock = !KDB_IS_RUNNING();
>> +#else
>> +#define	get_lock 1
>> +#endif
>> +
> 
> A standard way to do such would be a __get_vmalloc_info with the 
> lock in the caller

That particular #ifdef is gone now, and traded for a flag passed in with the function.

Between the changes that you recommended and that Peter recommended, the scope of changes to files out side the kdb area has been reduced by several hundred lines.

Thanks,
Jason.

View attachment "kdb_core_external_changes.patch" of type "text/x-diff" (10939 bytes)