LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Jonas Bonn <jonas@southpole.se>
Cc: linux-kernel@vger.kernel.org, olof@lixom.net, sam@ravnborg.com
Subject: Re: [PATCH] Add PCI_DEVICE_TABLE macro
Date: Wed, 20 Feb 2008 08:11:37 -0800	[thread overview]
Message-ID: <20080220161137.GB2658@kroah.com> (raw)
In-Reply-To: <47B93512.6000107@southpole.se>

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.

thanks,

greg k-h

  reply	other threads:[~2008-02-20 16:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-17 12:10 Jonas Bonn
2008-02-17 12:13 ` Jonas Bonn
2008-02-18  4:48   ` Greg KH
2008-02-18  7:34     ` Jonas Bonn
2008-02-20 16:11       ` Greg KH [this message]
2008-02-20 17:05         ` Jeff Garzik
2008-02-22 10:02           ` [PATCH] Add DECLARE_PCI_DEVICE_TABLE macro Jonas Bonn
2008-02-17 18:14 ` [PATCH] Add PCI_DEVICE_TABLE macro Randy Dunlap
2008-02-18  8:03 Jonas Bonn
2008-02-20 12:53 pci_device_id cleanups Jonas Bonn
2008-02-20 12:53 ` [PATCH] Add PCI_DEVICE_TABLE macro Jonas Bonn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080220161137.GB2658@kroah.com \
    --to=greg@kroah.com \
    --cc=jonas@southpole.se \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olof@lixom.net \
    --cc=sam@ravnborg.com \
    --subject='Re: [PATCH] Add PCI_DEVICE_TABLE macro' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).