[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <47BC5DBD.5090702@garzik.org>
Date: Wed, 20 Feb 2008 12:05:01 -0500
From: Jeff Garzik <jeff@...zik.org>
To: Greg KH <greg@...ah.com>
CC: Jonas Bonn <jonas@...thpole.se>, linux-kernel@...r.kernel.org,
olof@...om.net, sam@...nborg.com
Subject: Re: [PATCH] Add PCI_DEVICE_TABLE macro
Greg KH wrote:
> On Mon, Feb 18, 2008 at 08:34:42AM +0100, Jonas Bonn wrote:
>>> And again, what does this buy us?
>> Clarity and simplicity, I hope... there are a bunch of definitions
>> scattered about the kernel that omit the __devinitdata modifier despite the
>> documentation stating that it should always be there. The definition
>> really should have been const, which wasn't possible before but has become
>> so with the addition of the __devinitconst attribute.
>>
>> Furthermore, there are definitions that use "const" and __devinitdata,
>> which is explicitly wrong but the compiler doesn't catch section mismatches
>> if there's only one such one case in the module (which is often the case).
>>
>> Adding the __devinitconst modifier where there was nothing before buys us
>> memory. Adding the const modifier gives the compiler a chance to do its
>> thing. Changing __devinitdata to __devinitconst where it was wrong
>> actually fixes some compiler errors in older (mid-release) kernels that
>> were patched over by "removing" the section attribute altogether (which
>> wastes memory).
>>
>> Adding the macro (Olof's idea, not mine) makes it pretty difficult to get
>> this definition wrong... I'll do the rest of the cleanup, but I need to
>> know whether it's better to use a macro like this, or to open code the
>> definitions. I prefer the macro approach...
>>
>> Hope this makes some sense...
>
> Ok, yes it does, thanks for the explaination.
>
> Please, can you add this very good text to the changelog entry for the
> addition of the macro, and to the documentation somewhere? I'd be glad
> to take the patch if that was done.
I would suggest having a DECLARE_ prefix in there, like other subsystems
do...
Jeff
--
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