Messages in this thread | | | Date | Thu, 11 Oct 2007 09:37:29 +0200 | From | "Vegard Nossum" <> | Subject | Re: [PATCH] mm: Fix printk format strings |
| |
On 10/11/07, Andrew Morton <akpm@linux-foundation.org> wrote: > On Thu, 11 Oct 2007 09:04:57 +0200 "Vegard Nossum" <vegard.nossum@gmail.com> wrote: > > > > > - printk("Active:%lu inactive:%lu dirty:%lu writeback:%lu unstable:%lu\n" > > > > - " free:%lu slab:%lu mapped:%lu pagetables:%lu bounce:%lu\n", > > > > + printk("Active:%lu inactive:%lu dirty:%lu writeback:%lu unstable:%lu\n", > > > > global_page_state(NR_ACTIVE), > > > > global_page_state(NR_INACTIVE), > > > > global_page_state(NR_FILE_DIRTY), > > > > global_page_state(NR_WRITEBACK), > > > > - global_page_state(NR_UNSTABLE_NFS), > > > > + global_page_state(NR_UNSTABLE_NFS)); > > > > + printk(" free:%lu slab:%lu mapped:%lu pagetables:%lu bounce:%lu\n", > > > > global_page_state(NR_FREE_PAGES), > > > > global_page_state(NR_SLAB_RECLAIMABLE) + > > > > global_page_state(NR_SLAB_UNRECLAIMABLE), > > > > > > I don't understand the reason for this change. > > > > I'm sorry :). It helps to make one line per call only, since this > > allows changing the printk internals for the better by reducing some > > complexity. I believe this is a good thing. I have a patch that > > changes printk, but it assumes that each format string only contains a > > single line. Is this a very bad assumption to make? Or maybe I should > > have sent that change first and made a reference to it? > > > > But don't you also agree, on the grounds of principle, that a single > > line per call is better style? > > Well we do multiple-lines-per-printk in rather a lot of places and it has > two advantages: > > - less text size (I expect) > > - the printk is "atomic" in that the logically-connected output lines > won't get tangled up with an intervening printk from another CPU, or from > an interrupt handler on this CPU. > > Those are pretty modest advantages and I guess we could live without them > if we got a significant gain from doing so. But it'd take quite some > effort hunting down all the callsites, and preventing new ones from > occuring.
I have written a sparse program to detect all callers that don't use single-line literal strings for the printk format (posted to sparse mailing list yesterday). So in fact, what I was doing was just that, hunting them down and cleaning it up.
It so happens that my other patch also introduce a buffered printk that supports atomic line continuations and multiple uninterrupted lines, though this requires of course that the callers use these functions. Other benefits include writing directly to the ring-buffer, saving a 1K temporary buffer. The printk patch(es) can be viewed at http://folk.uio.no/vegardno/xprintf/
Vegard - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |