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: <20141014165027.GA26886@redhat.com>
Date:	Tue, 14 Oct 2014 12:50:27 -0400
From:	Dave Jones <davej@...hat.com>
To:	Michal Hocko <mhocko@...e.cz>
Cc:	Linux Kernel <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	Sasha Levin <sasha.levin@...cle.com>
Subject: Re: [PATCH] mm, debug: mm-introduce-vm_bug_on_mm-fix-fix.patch

On Tue, Oct 14, 2014 at 01:55:54PM +0200, Michal Hocko wrote:

 > > -#ifdef CONFIG_NUMA_BALANCING
 > > -		"numa_next_scan %lu numa_scan_offset %lu numa_scan_seq %d\n"
 > > -#endif
 > > -#if defined(CONFIG_NUMA_BALANCING) || defined(CONFIG_COMPACTION)
 > > -		"tlb_flush_pending %d\n"
 > > -#endif
 > > -		"%s",	/* This is here to hold the comma */
 > > +	char *p = dumpmm_buffer;
 > > +
 > > +	memset(dumpmm_buffer, 0, 4096);
 > 
 > I do not see any locking here. Previously we had internal printk log as
 > a natural synchronization. Now two threads are allowed to scribble over
 > their messages leaving an unusable output in a better case.

That's why I asked in the part of the mail you didn't quote whether
we cared if it wasn't reentrant.  Ok we do. That's 3 lines of change to
add a lock.

 > Besides that the %s with "" trick is not really that ugly and handles
 > the situation quite nicely. So do we really want to make it more
 > complicated?

That hack goes away entirely with this diff.  And by keeping the
parameters with the format string they're associated with, it should be
more maintainable should we decide to add more fields to be output in the future.
The number of ifdefs in the function are halved (which becomes even
bigger deal if we do add more output).

We saw how many times we had to go around to get it right this time.
In its current incarnation, it looks like a matter of time before
someone screws it up again due to missing some CONFIG combination.

	Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ