linux-kernel - Re: [RFC] mmiotrace full patch, preview 1

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: <20080226220217.747d47a5@daedalus.pq.iki.fi>
Date:	Tue, 26 Feb 2008 22:02:17 +0200
From:	Pekka Paalanen <pq@....fi>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Christoph Hellwig <hch@...radead.org>,
	Arjan van de Ven <arjan@...radead.org>,
	Pavel Roskin <proski@....org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Andy Whitcroft <apw@...dowen.org>
Subject: Re: [RFC] mmiotrace full patch, preview 1

On Mon, 25 Feb 2008 14:49:22 -0800
Andrew Morton <akpm@...ux-foundation.org> wrote:

> Please feed the diff through scritps/checkpatch.pl and consider addressing
> the things which it finds.

I checked that, but I didn't think any of them were worth fixing. And
since this is a work in progress and a in a state which I do not want
committed yet, I did not sign this patch. mmio-mod.c is still a mess.

> WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
> #1097: FILE: arch/x86/mm/mmio-mod.c:437:
> +void iounmap_trace(volatile void __iomem *addr)

I believe the 'volatile' belongs here, it is also in the declaration of
iounmap() in arch/x86/mm/ioremap.c.

> WARNING: externs should be avoided in .c files
> #1766: FILE: arch/x86/mm/testmmiotrace.c:7:
> +extern void __iomem *ioremap_nocache_trace(unsigned long offset,
> 
> WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
> #1768: FILE: arch/x86/mm/testmmiotrace.c:9:
> +extern void iounmap_trace(volatile void __iomem *addr);
> 
> WARNING: externs should be avoided in .c files
> #1768: FILE: arch/x86/mm/testmmiotrace.c:9:
> +extern void iounmap_trace(volatile void __iomem *addr);

These are all in testmmiotrace.c which calls the traced ioremap
functions directly. These direct calls will go away and it will
call the normal ioremap functions.

> WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
> #1790: FILE: arch/x86/mm/testmmiotrace.c:31:
> +       volatile unsigned int v;

Since testmmiotrace does not use v for anything other than return value
of ioread8/16/32(), I wanted to prevent the compiler from optimizing
it away. Now thinking it again, ioread*() must guarantee that the read
is always executed. I'll remove v altogether.

Patch for the other issues you mentioned is brewing.


Thanks.

-- 
Pekka Paalanen
http://www.iki.fi/pq/
--
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/