Message ID | 1515819518-6501-2-git-send-email-fred@fredlawl.com |
---|---|
State | Accepted |
Headers | show |
Series | PCI: Add and use pci_<level>: dev_<level> equivalents | expand |
[+to Joe] On Fri, Jan 12, 2018 at 10:58:36PM -0600, Frederick Lawler wrote: > Add PCI-specific dev_printk() wrappers so we can do: > > pci_info(dev, "message\n"); > > instead of > > dev_info(&dev->dev, "message\n"); Hi Joe, I proposed that Fred add pci_info(), pci_warn(), pci_err(), etc. You've done sort of similar things in the past: 256df2f3879e ("netdevice.h net/core/dev.c: Convert netdev_<level> logging macros to functions") a9a79dfec239 ("ata: Convert ata_<foo>_printk(KERN_<LEVEL> to ata_<foo>_<level>") Do you have any implementation advice for us? Fred started with inline functions, but tripped over some issues with varargs in inlines, so switched to the simple #defines below. I notice your commits above use either multiple non-inline functions (netdev) or a single non-inline *_printk() with macros to insert the loglevel (ata). Would you recommend: - simple #defines as below, - multiple non-inline functions like netdev, - single non-inline *_printk() with macros like ata, - something else? Thanks for any advice! Bjorn > --- > V2: > * Replace static inline varadic functions with macros instead > V3: > * No changes to this file > > include/linux/pci.h | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index c170c92..7a5012b 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -2281,4 +2281,31 @@ static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev) > /* provide the legacy pci_dma_* API */ > #include <linux/pci-dma-compat.h> > > +#define pci_printk(level, pdev, fmt, arg...) \ > + dev_printk(level, &(pdev)->dev, fmt, ##arg) > + > +#define pci_emerg(pdev, fmt, arg...) \ > + dev_emerg(&(pdev)->dev, fmt, ##arg) > + > +#define pci_crit(pdev, fmt, arg...) \ > + dev_crit(&(pdev)->dev, fmt, ##arg) > + > +#define pci_alert(pdev, fmt, arg...) \ > + dev_alert(&(pdev)->dev, fmt, ##arg) > + > +#define pci_err(pdev, fmt, arg...) \ > + dev_err(&(pdev)->dev, fmt, ##arg) > + > +#define pci_notice(pdev, fmt, arg...) \ > + dev_notice(&(pdev)->dev, fmt, ##arg) > + > +#define pci_warn(pdev, fmt, arg...) \ > + dev_warn(&(pdev)->dev, fmt, ##arg) > + > +#define pci_info(pdev, fmt, arg...) \ > + dev_info(&(pdev)->dev, fmt, ##arg) > + > +#define pci_dbg(pdev, fmt, arg...) \ > + dev_dbg(&(pdev)->dev, fmt, ##arg) > + > #endif /* LINUX_PCI_H */ > -- > 2.7.4 >
On Wed, 2018-01-17 at 14:12 -0600, Bjorn Helgaas wrote: > [+to Joe] > > On Fri, Jan 12, 2018 at 10:58:36PM -0600, Frederick Lawler wrote: > > Add PCI-specific dev_printk() wrappers so we can do: > > > > pci_info(dev, "message\n"); > > > > instead of > > > > dev_info(&dev->dev, "message\n"); > > Hi Joe, > > I proposed that Fred add pci_info(), pci_warn(), pci_err(), etc. > You've done sort of similar things in the past: > > 256df2f3879e ("netdevice.h net/core/dev.c: Convert netdev_<level> logging macros to functions") > a9a79dfec239 ("ata: Convert ata_<foo>_printk(KERN_<LEVEL> to ata_<foo>_<level>") > > Do you have any implementation advice for us? I try to use separate functions only when there is some object code savings that is greater than the increase in object size of the new function(s). In this case, only the object indirection dev->dev is being saved/reused so likely unless this is going to be used multiple dozens of times, most likely a simple macro wrapper might be best. > Would you recommend: > > - simple #defines as below, For this case, yes. > - multiple non-inline functions like netdev, Probably not unless you want to additionally prefix the output with some new PCI specific content. > - single non-inline *_printk() with macros like ata, Individual ata_<foo>_printk functions were used along with multiple ata_<foo>_<level> macros there. ata_<foo>_printk was used because there weren't enough callers of individual ata_<foo>_<level> functions to justify their creation to reduce overall object code size and macros created overall smaller objects. > - something else? > > Thanks for any advice! > diff --git a/include/linux/pci.h b/include/linux/pci.h [] > > @@ -2281,4 +2281,31 @@ static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev) > > /* provide the legacy pci_dma_* API */ > > #include <linux/pci-dma-compat.h> > > > > +#define pci_printk(level, pdev, fmt, arg...) \ > > + dev_printk(level, &(pdev)->dev, fmt, ##arg) I would generally use the ##__VA_ARGS__ form #define pci_printk(level, pdev, fmt, ...) \ dev_printk(level, &(pdev)->dev, fmt, ##__VA_ARGS__) > > +#define pci_emerg(pdev, fmt, arg...) \ > > + dev_emerg(&(pdev)->dev, fmt, ##arg) #define pci_emerg(level, pdev, fmt, ...) \ dev_emerg(level , &(pdev)->dev, fmt, ##__VA_ARGS__) etc... cheers, Joe
diff --git a/include/linux/pci.h b/include/linux/pci.h index c170c92..7a5012b 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2281,4 +2281,31 @@ static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev) /* provide the legacy pci_dma_* API */ #include <linux/pci-dma-compat.h> +#define pci_printk(level, pdev, fmt, arg...) \ + dev_printk(level, &(pdev)->dev, fmt, ##arg) + +#define pci_emerg(pdev, fmt, arg...) \ + dev_emerg(&(pdev)->dev, fmt, ##arg) + +#define pci_crit(pdev, fmt, arg...) \ + dev_crit(&(pdev)->dev, fmt, ##arg) + +#define pci_alert(pdev, fmt, arg...) \ + dev_alert(&(pdev)->dev, fmt, ##arg) + +#define pci_err(pdev, fmt, arg...) \ + dev_err(&(pdev)->dev, fmt, ##arg) + +#define pci_notice(pdev, fmt, arg...) \ + dev_notice(&(pdev)->dev, fmt, ##arg) + +#define pci_warn(pdev, fmt, arg...) \ + dev_warn(&(pdev)->dev, fmt, ##arg) + +#define pci_info(pdev, fmt, arg...) \ + dev_info(&(pdev)->dev, fmt, ##arg) + +#define pci_dbg(pdev, fmt, arg...) \ + dev_dbg(&(pdev)->dev, fmt, ##arg) + #endif /* LINUX_PCI_H */