linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/22] Add and use pci_zalloc_consistent
@ 2014-06-23 13:41 Joe Perches
  2014-06-23 13:41 ` Joe Perches
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-23 13:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: devel, linux-arch, linux-scsi, iss_storagedev, linux-rdma, netdev,
	linux-atm-general, linux-wireless, dri-devel, linux-crypto,
	linux-eata, linux-media

Adding the helper reduces object code size as well as overall
source size line count.

It's also consistent with all the various zalloc mechanisms
in the kernel.

Done with a simple cocci script and some typing.

Joe Perches (22):
  pci-dma-compat: Add pci_zalloc_consistent helper
  atm: Use pci_zalloc_consistent
  block: Use pci_zalloc_consistent
  crypto: Use pci_zalloc_consistent
  infiniband: Use pci_zalloc_consistent
  i810: Use pci_zalloc_consistent
  media: Use pci_zalloc_consistent
  amd: Use pci_zalloc_consistent
  atl1e: Use pci_zalloc_consistent
  enic: Use pci_zalloc_consistent
  sky2: Use pci_zalloc_consistent
  micrel: Use pci_zalloc_consistent
  qlogic: Use pci_zalloc_consistent
  irda: Use pci_zalloc_consistent
  ipw2100: Use pci_zalloc_consistent
  mwl8k: Use pci_zalloc_consistent
  rtl818x: Use pci_zalloc_consistent
  rtlwifi: Use pci_zalloc_consistent
  scsi: Use pci_zalloc_consistent
  staging: Use pci_zalloc_consistent
  synclink_gt: Use pci_zalloc_consistent
  vme: bridges: Use pci_zalloc_consistent

 drivers/atm/he.c                                   | 31 ++++++++---------
 drivers/atm/idt77252.c                             | 15 ++++----
 drivers/block/DAC960.c                             | 18 +++++-----
 drivers/block/cciss.c                              | 11 +++---
 drivers/block/skd_main.c                           | 25 +++++---------
 drivers/crypto/hifn_795x.c                         |  5 ++-
 drivers/gpu/drm/i810/i810_dma.c                    |  5 ++-
 drivers/infiniband/hw/amso1100/c2.c                |  6 ++--
 drivers/infiniband/hw/nes/nes_hw.c                 | 12 +++----
 drivers/infiniband/hw/nes/nes_verbs.c              |  5 ++-
 drivers/media/common/saa7146/saa7146_core.c        | 15 ++++----
 drivers/media/common/saa7146/saa7146_fops.c        |  5 +--
 drivers/media/pci/bt8xx/bt878.c                    | 16 +++------
 drivers/media/pci/ngene/ngene-core.c               |  7 ++--
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c  | 11 ++----
 drivers/media/usb/ttusb-dec/ttusb_dec.c            | 11 ++----
 drivers/net/ethernet/amd/pcnet32.c                 | 16 ++++-----
 drivers/net/ethernet/atheros/atl1e/atl1e_main.c    |  7 ++--
 drivers/net/ethernet/cisco/enic/vnic_dev.c         |  8 ++---
 drivers/net/ethernet/marvell/sky2.c                |  5 ++-
 drivers/net/ethernet/micrel/ksz884x.c              |  7 ++--
 .../net/ethernet/qlogic/netxen/netxen_nic_ctx.c    |  4 +--
 drivers/net/ethernet/qlogic/qlge/qlge_main.c       | 11 +++---
 drivers/net/irda/vlsi_ir.c                         |  4 +--
 drivers/net/wireless/ipw2x00/ipw2100.c             | 16 +++------
 drivers/net/wireless/mwl8k.c                       |  6 ++--
 drivers/net/wireless/rtl818x/rtl8180/dev.c         | 11 +++---
 drivers/net/wireless/rtlwifi/pci.c                 | 17 +++------
 drivers/scsi/3w-sas.c                              |  5 ++-
 drivers/scsi/a100u2w.c                             |  8 ++---
 drivers/scsi/be2iscsi/be_main.c                    | 10 +++---
 drivers/scsi/be2iscsi/be_mgmt.c                    |  3 +-
 drivers/scsi/csiostor/csio_wr.c                    |  8 +----
 drivers/scsi/eata.c                                |  5 ++-
 drivers/scsi/hpsa.c                                |  8 ++---
 drivers/scsi/megaraid/megaraid_mbox.c              | 16 ++++-----
 drivers/scsi/megaraid/megaraid_sas_base.c          |  8 ++---
 drivers/scsi/mesh.c                                |  6 ++--
 drivers/scsi/mvumi.c                               |  9 ++---
 drivers/scsi/pm8001/pm8001_sas.c                   |  5 ++-
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c       | 15 +++-----
 drivers/staging/rtl8192ee/pci.c                    | 37 +++++++-------------
 drivers/staging/rtl8821ae/pci.c                    | 36 +++++++------------
 drivers/staging/slicoss/slicoss.c                  |  9 ++---
 drivers/staging/vt6655/device_main.c               | 40 +++++++---------------
 drivers/tty/synclink_gt.c                          |  5 ++-
 drivers/vme/bridges/vme_ca91cx42.c                 |  6 ++--
 drivers/vme/bridges/vme_tsi148.c                   |  6 ++--
 include/asm-generic/pci-dma-compat.h               |  8 +++++
 49 files changed, 209 insertions(+), 354 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 13:41 [PATCH 00/22] Add and use pci_zalloc_consistent Joe Perches
@ 2014-06-23 13:41 ` Joe Perches
  2014-06-23 13:41 ` [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper Joe Perches
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-23 13:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-atm-general, netdev, iss_storagedev, linux-crypto,
	dri-devel, linux-rdma, linux-media, linux-wireless, linux-scsi,
	linux-eata, devel, linux-arch

Adding the helper reduces object code size as well as overall
source size line count.

It's also consistent with all the various zalloc mechanisms
in the kernel.

Done with a simple cocci script and some typing.

Joe Perches (22):
  pci-dma-compat: Add pci_zalloc_consistent helper
  atm: Use pci_zalloc_consistent
  block: Use pci_zalloc_consistent
  crypto: Use pci_zalloc_consistent
  infiniband: Use pci_zalloc_consistent
  i810: Use pci_zalloc_consistent
  media: Use pci_zalloc_consistent
  amd: Use pci_zalloc_consistent
  atl1e: Use pci_zalloc_consistent
  enic: Use pci_zalloc_consistent
  sky2: Use pci_zalloc_consistent
  micrel: Use pci_zalloc_consistent
  qlogic: Use pci_zalloc_consistent
  irda: Use pci_zalloc_consistent
  ipw2100: Use pci_zalloc_consistent
  mwl8k: Use pci_zalloc_consistent
  rtl818x: Use pci_zalloc_consistent
  rtlwifi: Use pci_zalloc_consistent
  scsi: Use pci_zalloc_consistent
  staging: Use pci_zalloc_consistent
  synclink_gt: Use pci_zalloc_consistent
  vme: bridges: Use pci_zalloc_consistent

 drivers/atm/he.c                                   | 31 ++++++++---------
 drivers/atm/idt77252.c                             | 15 ++++----
 drivers/block/DAC960.c                             | 18 +++++-----
 drivers/block/cciss.c                              | 11 +++---
 drivers/block/skd_main.c                           | 25 +++++---------
 drivers/crypto/hifn_795x.c                         |  5 ++-
 drivers/gpu/drm/i810/i810_dma.c                    |  5 ++-
 drivers/infiniband/hw/amso1100/c2.c                |  6 ++--
 drivers/infiniband/hw/nes/nes_hw.c                 | 12 +++----
 drivers/infiniband/hw/nes/nes_verbs.c              |  5 ++-
 drivers/media/common/saa7146/saa7146_core.c        | 15 ++++----
 drivers/media/common/saa7146/saa7146_fops.c        |  5 +--
 drivers/media/pci/bt8xx/bt878.c                    | 16 +++------
 drivers/media/pci/ngene/ngene-core.c               |  7 ++--
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c  | 11 ++----
 drivers/media/usb/ttusb-dec/ttusb_dec.c            | 11 ++----
 drivers/net/ethernet/amd/pcnet32.c                 | 16 ++++-----
 drivers/net/ethernet/atheros/atl1e/atl1e_main.c    |  7 ++--
 drivers/net/ethernet/cisco/enic/vnic_dev.c         |  8 ++---
 drivers/net/ethernet/marvell/sky2.c                |  5 ++-
 drivers/net/ethernet/micrel/ksz884x.c              |  7 ++--
 .../net/ethernet/qlogic/netxen/netxen_nic_ctx.c    |  4 +--
 drivers/net/ethernet/qlogic/qlge/qlge_main.c       | 11 +++---
 drivers/net/irda/vlsi_ir.c                         |  4 +--
 drivers/net/wireless/ipw2x00/ipw2100.c             | 16 +++------
 drivers/net/wireless/mwl8k.c                       |  6 ++--
 drivers/net/wireless/rtl818x/rtl8180/dev.c         | 11 +++---
 drivers/net/wireless/rtlwifi/pci.c                 | 17 +++------
 drivers/scsi/3w-sas.c                              |  5 ++-
 drivers/scsi/a100u2w.c                             |  8 ++---
 drivers/scsi/be2iscsi/be_main.c                    | 10 +++---
 drivers/scsi/be2iscsi/be_mgmt.c                    |  3 +-
 drivers/scsi/csiostor/csio_wr.c                    |  8 +----
 drivers/scsi/eata.c                                |  5 ++-
 drivers/scsi/hpsa.c                                |  8 ++---
 drivers/scsi/megaraid/megaraid_mbox.c              | 16 ++++-----
 drivers/scsi/megaraid/megaraid_sas_base.c          |  8 ++---
 drivers/scsi/mesh.c                                |  6 ++--
 drivers/scsi/mvumi.c                               |  9 ++---
 drivers/scsi/pm8001/pm8001_sas.c                   |  5 ++-
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c       | 15 +++-----
 drivers/staging/rtl8192ee/pci.c                    | 37 +++++++-------------
 drivers/staging/rtl8821ae/pci.c                    | 36 +++++++------------
 drivers/staging/slicoss/slicoss.c                  |  9 ++---
 drivers/staging/vt6655/device_main.c               | 40 +++++++---------------
 drivers/tty/synclink_gt.c                          |  5 ++-
 drivers/vme/bridges/vme_ca91cx42.c                 |  6 ++--
 drivers/vme/bridges/vme_tsi148.c                   |  6 ++--
 include/asm-generic/pci-dma-compat.h               |  8 +++++
 49 files changed, 209 insertions(+), 354 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-23 13:41 [PATCH 00/22] Add and use pci_zalloc_consistent Joe Perches
  2014-06-23 13:41 ` Joe Perches
@ 2014-06-23 13:41 ` Joe Perches
  2014-06-23 13:41   ` Joe Perches
                     ` (2 more replies)
  2014-06-23 17:25 ` [PATCH 00/22] Add and use pci_zalloc_consistent Luis R. Rodriguez
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-23 13:41 UTC (permalink / raw)
  To: linux-kernel; +Cc: Arnd Bergmann, linux-arch

Add this helper for consistency with pci_zalloc_coherent
and the ability to remove unnecessary memset(,0,) uses.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/asm-generic/pci-dma-compat.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/asm-generic/pci-dma-compat.h b/include/asm-generic/pci-dma-compat.h
index 1437b7d..c110843 100644
--- a/include/asm-generic/pci-dma-compat.h
+++ b/include/asm-generic/pci-dma-compat.h
@@ -19,6 +19,14 @@ pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
 	return dma_alloc_coherent(hwdev == NULL ? NULL : &hwdev->dev, size, dma_handle, GFP_ATOMIC);
 }
 
+static inline void *
+pci_zalloc_consistent(struct pci_dev *hwdev, size_t size,
+		      dma_addr_t *dma_handle)
+{
+	return dma_zalloc_coherent(hwdev == NULL ? NULL : &hwdev->dev,
+				   size, dma_handle, GFP_ATOMIC);
+}
+
 static inline void
 pci_free_consistent(struct pci_dev *hwdev, size_t size,
 		    void *vaddr, dma_addr_t dma_handle)
-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-23 13:41 ` [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper Joe Perches
@ 2014-06-23 13:41   ` Joe Perches
  2014-06-23 13:53   ` Arnd Bergmann
  2014-06-25 19:27   ` Andrew Morton
  2 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-23 13:41 UTC (permalink / raw)
  To: linux-kernel; +Cc: Arnd Bergmann, linux-arch

Add this helper for consistency with pci_zalloc_coherent
and the ability to remove unnecessary memset(,0,) uses.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/asm-generic/pci-dma-compat.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/asm-generic/pci-dma-compat.h b/include/asm-generic/pci-dma-compat.h
index 1437b7d..c110843 100644
--- a/include/asm-generic/pci-dma-compat.h
+++ b/include/asm-generic/pci-dma-compat.h
@@ -19,6 +19,14 @@ pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
 	return dma_alloc_coherent(hwdev == NULL ? NULL : &hwdev->dev, size, dma_handle, GFP_ATOMIC);
 }
 
+static inline void *
+pci_zalloc_consistent(struct pci_dev *hwdev, size_t size,
+		      dma_addr_t *dma_handle)
+{
+	return dma_zalloc_coherent(hwdev == NULL ? NULL : &hwdev->dev,
+				   size, dma_handle, GFP_ATOMIC);
+}
+
 static inline void
 pci_free_consistent(struct pci_dev *hwdev, size_t size,
 		    void *vaddr, dma_addr_t dma_handle)
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-23 13:41 ` [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper Joe Perches
  2014-06-23 13:41   ` Joe Perches
@ 2014-06-23 13:53   ` Arnd Bergmann
  2014-06-23 13:53     ` Arnd Bergmann
  2014-06-23 14:03     ` Joe Perches
  2014-06-25 19:27   ` Andrew Morton
  2 siblings, 2 replies; 21+ messages in thread
From: Arnd Bergmann @ 2014-06-23 13:53 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, linux-arch

On Monday 23 June 2014 06:41:29 Joe Perches wrote:
> Add this helper for consistency with pci_zalloc_coherent
> and the ability to remove unnecessary memset(,0,) uses.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> 

Shouldn't these drivers just use the normal dma-mapping API now?

	Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-23 13:53   ` Arnd Bergmann
@ 2014-06-23 13:53     ` Arnd Bergmann
  2014-06-23 14:03     ` Joe Perches
  1 sibling, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2014-06-23 13:53 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, linux-arch

On Monday 23 June 2014 06:41:29 Joe Perches wrote:
> Add this helper for consistency with pci_zalloc_coherent
> and the ability to remove unnecessary memset(,0,) uses.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> 

Shouldn't these drivers just use the normal dma-mapping API now?

	Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-23 13:53   ` Arnd Bergmann
  2014-06-23 13:53     ` Arnd Bergmann
@ 2014-06-23 14:03     ` Joe Perches
  1 sibling, 0 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-23 14:03 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linux-kernel, linux-arch

On Mon, 2014-06-23 at 15:53 +0200, Arnd Bergmann wrote:
> On Monday 23 June 2014 06:41:29 Joe Perches wrote:
> > Add this helper for consistency with pci_zalloc_coherent
> > and the ability to remove unnecessary memset(,0,) uses.

> Shouldn't these drivers just use the normal dma-mapping API now?

Maybe.  I wouldn't mind.

They do seem to have a trivial bit of unnecessary overhead for 

hwdev == NULL ? NULL : &hwdev->dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 13:41 [PATCH 00/22] Add and use pci_zalloc_consistent Joe Perches
  2014-06-23 13:41 ` Joe Perches
  2014-06-23 13:41 ` [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper Joe Perches
@ 2014-06-23 17:25 ` Luis R. Rodriguez
  2014-06-23 17:25   ` Luis R. Rodriguez
  2014-06-23 19:13   ` Joe Perches
       [not found] ` <cover.1403530604.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
  2014-06-25 19:41 ` John W. Linville
  4 siblings, 2 replies; 21+ messages in thread
From: Luis R. Rodriguez @ 2014-06-23 17:25 UTC (permalink / raw)
  To: Joe Perches
  Cc: devel, linux-arch, linux-scsi, iss_storagedev, linux-rdma, netdev,
	linux-atm-general, linux-wireless, linux-kernel, dri-devel,
	linux-crypto, linux-media, linux-eata

On Mon, Jun 23, 2014 at 06:41:28AM -0700, Joe Perches wrote:
> Adding the helper reduces object code size as well as overall
> source size line count.
> 
> It's also consistent with all the various zalloc mechanisms
> in the kernel.
> 
> Done with a simple cocci script and some typing.

Awesome, any chance you can paste in the SmPL? Also any chance
we can get this added to a make coccicheck so that maintainers
moving forward can use that to ensure that no new code is
added that uses the old school API?

  Luis

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 17:25 ` [PATCH 00/22] Add and use pci_zalloc_consistent Luis R. Rodriguez
@ 2014-06-23 17:25   ` Luis R. Rodriguez
  2014-06-23 19:13   ` Joe Perches
  1 sibling, 0 replies; 21+ messages in thread
From: Luis R. Rodriguez @ 2014-06-23 17:25 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, devel, linux-arch, linux-scsi, iss_storagedev,
	linux-rdma, netdev, linux-atm-general, linux-wireless, dri-devel,
	linux-crypto, linux-eata, linux-media

On Mon, Jun 23, 2014 at 06:41:28AM -0700, Joe Perches wrote:
> Adding the helper reduces object code size as well as overall
> source size line count.
> 
> It's also consistent with all the various zalloc mechanisms
> in the kernel.
> 
> Done with a simple cocci script and some typing.

Awesome, any chance you can paste in the SmPL? Also any chance
we can get this added to a make coccicheck so that maintainers
moving forward can use that to ensure that no new code is
added that uses the old school API?

  Luis

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 17:25 ` [PATCH 00/22] Add and use pci_zalloc_consistent Luis R. Rodriguez
  2014-06-23 17:25   ` Luis R. Rodriguez
@ 2014-06-23 19:13   ` Joe Perches
  2014-06-23 19:13     ` Joe Perches
  2014-06-23 23:27     ` Julian Calaby
  1 sibling, 2 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-23 19:13 UTC (permalink / raw)
  To: Luis R. Rodriguez, Arnd Bergmann
  Cc: devel, linux-arch, linux-scsi, iss_storagedev, linux-rdma, netdev,
	linux-atm-general, linux-wireless, linux-kernel, dri-devel,
	linux-crypto, linux-media, linux-eata

On Mon, 2014-06-23 at 10:25 -0700, Luis R. Rodriguez wrote:
> On Mon, Jun 23, 2014 at 06:41:28AM -0700, Joe Perches wrote:
> > Adding the helper reduces object code size as well as overall
> > source size line count.
> > 
> > It's also consistent with all the various zalloc mechanisms
> > in the kernel.
> > 
> > Done with a simple cocci script and some typing.
> 
> Awesome, any chance you can paste in the SmPL? Also any chance
> we can get this added to a make coccicheck so that maintainers
> moving forward can use that to ensure that no new code is
> added that uses the old school API?

Not many of these are recent.

Arnd Bergmann reasonably suggested that the pci_alloc_consistent
api be converted the the more widely used dma_alloc_coherent.

https://lkml.org/lkml/2014/6/23/513

> Shouldn't these drivers just use the normal dma-mapping API now?

and I replied:

https://lkml.org/lkml/2014/6/23/525

> Maybe.  I wouldn't mind.
> They do seem to have a trivial bit of unnecessary overhead for 
> hwdev == NULL ? NULL : &hwdev->dev

Anyway, here's the little script.
I'm not sure it's worthwhile to add it though.

$ cat ./scripts/coccinelle/api/alloc/pci_zalloc_consistent.cocci
///
/// Use pci_zalloc_consistent rather than
///     pci_alloc_consistent followed by memset with 0
///
/// This considers some simple cases that are common and easy to validate
/// Note in particular that there are no ...s in the rule, so all of the
/// matched code has to be contiguous
///
/// Blatantly cribbed from: scripts/coccinelle/api/alloc/kzalloc-simple.cocci

@@
type T, T2;
expression x;
expression E1,E2,E3;
statement S;
@@

- x = (T)pci_alloc_consistent(E1,E2,E3);
+ x = pci_zalloc_consistent(E1,E2,E3);
  if ((x==NULL) || ...) S
- memset((T2)x,0,E2);

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 19:13   ` Joe Perches
@ 2014-06-23 19:13     ` Joe Perches
  2014-06-23 23:27     ` Julian Calaby
  1 sibling, 0 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-23 19:13 UTC (permalink / raw)
  To: Luis R. Rodriguez, Arnd Bergmann
  Cc: linux-kernel, devel, linux-arch, linux-scsi, iss_storagedev,
	linux-rdma, netdev, linux-atm-general, linux-wireless, dri-devel,
	linux-crypto, linux-eata, linux-media

On Mon, 2014-06-23 at 10:25 -0700, Luis R. Rodriguez wrote:
> On Mon, Jun 23, 2014 at 06:41:28AM -0700, Joe Perches wrote:
> > Adding the helper reduces object code size as well as overall
> > source size line count.
> > 
> > It's also consistent with all the various zalloc mechanisms
> > in the kernel.
> > 
> > Done with a simple cocci script and some typing.
> 
> Awesome, any chance you can paste in the SmPL? Also any chance
> we can get this added to a make coccicheck so that maintainers
> moving forward can use that to ensure that no new code is
> added that uses the old school API?

Not many of these are recent.

Arnd Bergmann reasonably suggested that the pci_alloc_consistent
api be converted the the more widely used dma_alloc_coherent.

https://lkml.org/lkml/2014/6/23/513

> Shouldn't these drivers just use the normal dma-mapping API now?

and I replied:

https://lkml.org/lkml/2014/6/23/525

> Maybe.  I wouldn't mind.
> They do seem to have a trivial bit of unnecessary overhead for 
> hwdev == NULL ? NULL : &hwdev->dev

Anyway, here's the little script.
I'm not sure it's worthwhile to add it though.

$ cat ./scripts/coccinelle/api/alloc/pci_zalloc_consistent.cocci
///
/// Use pci_zalloc_consistent rather than
///     pci_alloc_consistent followed by memset with 0
///
/// This considers some simple cases that are common and easy to validate
/// Note in particular that there are no ...s in the rule, so all of the
/// matched code has to be contiguous
///
/// Blatantly cribbed from: scripts/coccinelle/api/alloc/kzalloc-simple.cocci

@@
type T, T2;
expression x;
expression E1,E2,E3;
statement S;
@@

- x = (T)pci_alloc_consistent(E1,E2,E3);
+ x = pci_zalloc_consistent(E1,E2,E3);
  if ((x==NULL) || ...) S
- memset((T2)x,0,E2);



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
       [not found] ` <cover.1403530604.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
@ 2014-06-23 21:49   ` David Miller
  2014-06-23 21:49     ` David Miller
  0 siblings, 1 reply; 21+ messages in thread
From: David Miller @ 2014-06-23 21:49 UTC (permalink / raw)
  To: joe-6d6DIl74uiNBDgjK7y7TUQ
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-atm-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	netdev-u79uwXL29TY76Z2rM5mHXA, iss_storagedev-VXdhtT5mjnY,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-eata-3umIvqjeSgIeIZ0/mPfg9Q,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-arch-u79uwXL29TY76Z2rM5mHXA

From: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
Date: Mon, 23 Jun 2014 06:41:28 -0700

> Adding the helper reduces object code size as well as overall
> source size line count.
> 
> It's also consistent with all the various zalloc mechanisms
> in the kernel.
> 
> Done with a simple cocci script and some typing.

For networking bits:

Acked-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 21:49   ` David Miller
@ 2014-06-23 21:49     ` David Miller
  0 siblings, 0 replies; 21+ messages in thread
From: David Miller @ 2014-06-23 21:49 UTC (permalink / raw)
  To: joe
  Cc: linux-kernel, linux-atm-general, netdev, iss_storagedev,
	linux-crypto, dri-devel, linux-rdma, linux-media, linux-wireless,
	linux-scsi, linux-eata, devel, linux-arch

From: Joe Perches <joe@perches.com>
Date: Mon, 23 Jun 2014 06:41:28 -0700

> Adding the helper reduces object code size as well as overall
> source size line count.
> 
> It's also consistent with all the various zalloc mechanisms
> in the kernel.
> 
> Done with a simple cocci script and some typing.

For networking bits:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 19:13   ` Joe Perches
  2014-06-23 19:13     ` Joe Perches
@ 2014-06-23 23:27     ` Julian Calaby
  2014-06-24 11:32       ` Johannes Berg
  1 sibling, 1 reply; 21+ messages in thread
From: Julian Calaby @ 2014-06-23 23:27 UTC (permalink / raw)
  To: Joe Perches
  Cc: Luis R. Rodriguez, Arnd Bergmann, linux-kernel@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-arch, linux-scsi,
	iss_storagedev, linux-rdma, netdev, linux-atm-general,
	linux-wireless, dri-devel, linux-crypto, linux-eata, linux-media

Hi Joe,

On Tue, Jun 24, 2014 at 5:13 AM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2014-06-23 at 10:25 -0700, Luis R. Rodriguez wrote:
>> On Mon, Jun 23, 2014 at 06:41:28AM -0700, Joe Perches wrote:
>> > Adding the helper reduces object code size as well as overall
>> > source size line count.
>> >
>> > It's also consistent with all the various zalloc mechanisms
>> > in the kernel.
>> >
>> > Done with a simple cocci script and some typing.
>>
>> Awesome, any chance you can paste in the SmPL? Also any chance
>> we can get this added to a make coccicheck so that maintainers
>> moving forward can use that to ensure that no new code is
>> added that uses the old school API?
>
> Not many of these are recent.
>
> Arnd Bergmann reasonably suggested that the pci_alloc_consistent
> api be converted the the more widely used dma_alloc_coherent.
>
> https://lkml.org/lkml/2014/6/23/513
>
>> Shouldn't these drivers just use the normal dma-mapping API now?
>
> and I replied:
>
> https://lkml.org/lkml/2014/6/23/525
>
>> Maybe.  I wouldn't mind.
>> They do seem to have a trivial bit of unnecessary overhead for
>> hwdev == NULL ? NULL : &hwdev->dev
>
> Anyway, here's the little script.
> I'm not sure it's worthwhile to add it though.
>
> $ cat ./scripts/coccinelle/api/alloc/pci_zalloc_consistent.cocci
> ///
> /// Use pci_zalloc_consistent rather than
> ///     pci_alloc_consistent followed by memset with 0
> ///
> /// This considers some simple cases that are common and easy to validate
> /// Note in particular that there are no ...s in the rule, so all of the
> /// matched code has to be contiguous
> ///
> /// Blatantly cribbed from: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
> @@
> type T, T2;
> expression x;
> expression E1,E2,E3;
> statement S;
> @@
>
> - x = (T)pci_alloc_consistent(E1,E2,E3);
> + x = pci_zalloc_consistent(E1,E2,E3);
>   if ((x==NULL) || ...) S
> - memset((T2)x,0,E2);

I don't know much about SmPL, but wouldn't having that if statement
there reduce your matches?

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 23:27     ` Julian Calaby
@ 2014-06-24 11:32       ` Johannes Berg
  0 siblings, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2014-06-24 11:32 UTC (permalink / raw)
  To: Julian Calaby
  Cc: Joe Perches, Luis R. Rodriguez, Arnd Bergmann,
	linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
	linux-arch, linux-scsi, iss_storagedev, linux-rdma, netdev,
	linux-atm-general, linux-wireless, dri-devel, linux-crypto,
	linux-eata, linux-media

On Tue, 2014-06-24 at 09:27 +1000, Julian Calaby wrote:

> > - x = (T)pci_alloc_consistent(E1,E2,E3);
> > + x = pci_zalloc_consistent(E1,E2,E3);
> >   if ((x==NULL) || ...) S
> > - memset((T2)x,0,E2);
> 
> I don't know much about SmPL, but wouldn't having that if statement
> there reduce your matches?

Code that matched without the if statement would be buggy, since it
wouldn't be checking the pci_zalloc_consistent return value properly.l

johannes

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-23 13:41 ` [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper Joe Perches
  2014-06-23 13:41   ` Joe Perches
  2014-06-23 13:53   ` Arnd Bergmann
@ 2014-06-25 19:27   ` Andrew Morton
  2014-06-25 21:51     ` Joe Perches
  2 siblings, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2014-06-25 19:27 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, Arnd Bergmann, linux-arch

On Mon, 23 Jun 2014 06:41:29 -0700 Joe Perches <joe@perches.com> wrote:

> Add this helper for consistency with pci_zalloc_coherent
> and the ability to remove unnecessary memset(,0,) uses.

While we're being anal..  I'm not a big fan of the patch titles.  Worst
is "amd: Use pci_zalloc_consistent".  "amd" is quite a poor identifier
- it's only when you get in and look at the diff that you realise it's
an ethernet driver.

People sometimes address this by using

	"drivers: net: ethernet: amd: use pci_zalloc_consistent"

which strikes me as utterly perverse.  We already have a nice way of
representing the hierarchy and that's using '/'.

So when the irritation gets too high and when I can be bothered I'll
rewrite things like that to

	"drivers/net/ethernet/amd: use pci_zalloc_consistent"

which strikes me as being blindingly obvious, but apparently I'm in a
small minority :(

> --- a/include/asm-generic/pci-dma-compat.h
> +++ b/include/asm-generic/pci-dma-compat.h
> @@ -19,6 +19,14 @@ pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
>  	return dma_alloc_coherent(hwdev == NULL ? NULL : &hwdev->dev, size, dma_handle, GFP_ATOMIC);
>  }
>  
> +static inline void *
> +pci_zalloc_consistent(struct pci_dev *hwdev, size_t size,
> +		      dma_addr_t *dma_handle)
> +{
> +	return dma_zalloc_coherent(hwdev == NULL ? NULL : &hwdev->dev,
> +				   size, dma_handle, GFP_ATOMIC);
> +}
> +

We'd get a smaller kernel by uninlining this.  It is hardly
performance-sensitive.  Uninlining would presumably use more stack,
but GFP_ATOMIC won't use a ton of stack anyway.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/22] Add and use pci_zalloc_consistent
  2014-06-23 13:41 [PATCH 00/22] Add and use pci_zalloc_consistent Joe Perches
                   ` (3 preceding siblings ...)
       [not found] ` <cover.1403530604.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
@ 2014-06-25 19:41 ` John W. Linville
  4 siblings, 0 replies; 21+ messages in thread
From: John W. Linville @ 2014-06-25 19:41 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, linux-atm-general, netdev, iss_storagedev,
	linux-crypto, dri-devel, linux-rdma, linux-media, linux-wireless,
	linux-scsi, linux-eata, devel, linux-arch

On Mon, Jun 23, 2014 at 06:41:28AM -0700, Joe Perches wrote:
> Adding the helper reduces object code size as well as overall
> source size line count.
> 
> It's also consistent with all the various zalloc mechanisms
> in the kernel.
> 
> Done with a simple cocci script and some typing.
> 
> Joe Perches (22):

>   ipw2100: Use pci_zalloc_consistent
>   mwl8k: Use pci_zalloc_consistent
>   rtl818x: Use pci_zalloc_consistent
>   rtlwifi: Use pci_zalloc_consistent

Sure, fine by me.

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-25 19:27   ` Andrew Morton
@ 2014-06-25 21:51     ` Joe Perches
  2014-06-25 21:57       ` Andrew Morton
  2014-06-25 22:31       ` David Miller
  0 siblings, 2 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-25 21:51 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, Arnd Bergmann, linux-arch

On Wed, 2014-06-25 at 12:27 -0700, Andrew Morton wrote:
> On Mon, 23 Jun 2014 06:41:29 -0700 Joe Perches <joe@perches.com> wrote:
> 
> > Add this helper for consistency with pci_zalloc_coherent
> > and the ability to remove unnecessary memset(,0,) uses.
> 
> While we're being anal..  I'm not a big fan of the patch titles.  Worst
> is "amd: Use pci_zalloc_consistent".  "amd" is quite a poor identifier
> - it's only when you get in and look at the diff that you realise it's
> an ethernet driver.

Yeah, those "amd:" prefixes should really have been "pcnet32:"

> People sometimes address this by using
> 
> 	"drivers: net: ethernet: amd: use pci_zalloc_consistent"
> 
> which strikes me as utterly perverse.  We already have a nice way of
> representing the hierarchy and that's using '/'.

I used to do that until several people complained.
Now I don't.  btw:  Documentation/SubmittingPatches says:

15) The canonical patch format

The canonical patch subject line is:

    Subject: [PATCH 001/123] subsystem: summary phrase


> So when the irritation gets too high and when I can be bothered I'll
> rewrite things like that to
> 
> 	"drivers/net/ethernet/amd: use pci_zalloc_consistent"
> 
> which strikes me as being blindingly obvious, but apparently I'm in a
> small minority :(
> 
> > --- a/include/asm-generic/pci-dma-compat.h
> > +++ b/include/asm-generic/pci-dma-compat.h
> > @@ -19,6 +19,14 @@ pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
> >  	return dma_alloc_coherent(hwdev == NULL ? NULL : &hwdev->dev, size, dma_handle, GFP_ATOMIC);
> >  }
> >  
> > +static inline void *
> > +pci_zalloc_consistent(struct pci_dev *hwdev, size_t size,
> > +		      dma_addr_t *dma_handle)
> > +{
> > +	return dma_zalloc_coherent(hwdev == NULL ? NULL : &hwdev->dev,
> > +				   size, dma_handle, GFP_ATOMIC);
> > +}
> > +
> 
> We'd get a smaller kernel by uninlining this.  It is hardly
> performance-sensitive.  Uninlining would presumably use more stack,
> but GFP_ATOMIC won't use a ton of stack anyway.

True.  Maybe via a follow-on patch.

Another option would be to remove pci_[z]alloc_consistent
and just use dma_alloc_coherent instead.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-25 21:51     ` Joe Perches
@ 2014-06-25 21:57       ` Andrew Morton
  2014-06-25 22:14         ` Joe Perches
  2014-06-25 22:31       ` David Miller
  1 sibling, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2014-06-25 21:57 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, Arnd Bergmann, linux-arch

On Wed, 25 Jun 2014 14:51:51 -0700 Joe Perches <joe@perches.com> wrote:

> > People sometimes address this by using
> > 
> > 	"drivers: net: ethernet: amd: use pci_zalloc_consistent"
> > 
> > which strikes me as utterly perverse.  We already have a nice way of
> > representing the hierarchy and that's using '/'.
> 
> I used to do that until several people complained.

Slap 'em.

I don't know where this thing is coming from - I'd suspected there must
be some misguided doc somewhere but I don't know where it is.  Some
vast conspiracy against common sense.

> Now I don't.  btw:  Documentation/SubmittingPatches says:
> 
> 15) The canonical patch format
> 
> The canonical patch subject line is:
> 
>     Subject: [PATCH 001/123] subsystem: summary phrase

Yes, that doesn't go into how to identify the subsystem.

I don't want to be overly proscriptive here, but it's a bit maddening
when you're skimming patch titles and cannot work out which part of the
kernel is being patched :(

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-25 21:57       ` Andrew Morton
@ 2014-06-25 22:14         ` Joe Perches
  0 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2014-06-25 22:14 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, Arnd Bergmann, linux-arch

On Wed, 2014-06-25 at 14:57 -0700, Andrew Morton wrote:
> On Wed, 25 Jun 2014 14:51:51 -0700 Joe Perches <joe@perches.com> wrote:
> 
> > > People sometimes address this by using
> > > 
> > > 	"drivers: net: ethernet: amd: use pci_zalloc_consistent"
> > > 
> > > which strikes me as utterly perverse.  We already have a nice way of
> > > representing the hierarchy and that's using '/'.
> > 
> > I used to do that until several people complained.
> 
> Slap 'em.

And get booked for assault?  No thanks.
My mild-mannered persona thinks that a poor plan.

My actual feeling is I don't much care.

btw:  https://lkml.org/lkml/2010/11/15/387

I also proposed a scheme where the prescriptive
patch subject pedants could put some pattern
into MAINTAINERS so that these subject lines
could be more automatically generated.

https://lkml.org/lkml/2010/11/16/245

> I don't know where this thing is coming from - I'd suspected there must
> be some misguided doc somewhere but I don't know where it is.  Some
> vast conspiracy against common sense.

> I don't want to be overly proscriptive here, but it's a bit maddening
> when you're skimming patch titles and cannot work out which part of the
> kernel is being patched :(

Yup.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper
  2014-06-25 21:51     ` Joe Perches
  2014-06-25 21:57       ` Andrew Morton
@ 2014-06-25 22:31       ` David Miller
  1 sibling, 0 replies; 21+ messages in thread
From: David Miller @ 2014-06-25 22:31 UTC (permalink / raw)
  To: joe; +Cc: akpm, linux-kernel, arnd, linux-arch

From: Joe Perches <joe@perches.com>
Date: Wed, 25 Jun 2014 14:51:51 -0700

> On Wed, 2014-06-25 at 12:27 -0700, Andrew Morton wrote:
>> On Mon, 23 Jun 2014 06:41:29 -0700 Joe Perches <joe@perches.com> wrote:
>> 
>> > Add this helper for consistency with pci_zalloc_coherent
>> > and the ability to remove unnecessary memset(,0,) uses.
>> 
>> While we're being anal..  I'm not a big fan of the patch titles.  Worst
>> is "amd: Use pci_zalloc_consistent".  "amd" is quite a poor identifier
>> - it's only when you get in and look at the diff that you realise it's
>> an ethernet driver.
> 
> Yeah, those "amd:" prefixes should really have been "pcnet32:"
> 
>> People sometimes address this by using
>> 
>> 	"drivers: net: ethernet: amd: use pci_zalloc_consistent"
>> 
>> which strikes me as utterly perverse.  We already have a nice way of
>> representing the hierarchy and that's using '/'.
> 
> I used to do that until several people complained.
> Now I don't.  btw:  Documentation/SubmittingPatches says:
> 
> 15) The canonical patch format
> 
> The canonical patch subject line is:
> 
>     Subject: [PATCH 001/123] subsystem: summary phrase

+1

We've been representing subsystems using colon prefixes for ages, and
I can't seen the value of starting to use something inconsistent with
a decade of precedence.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2014-06-25 22:31 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-23 13:41 [PATCH 00/22] Add and use pci_zalloc_consistent Joe Perches
2014-06-23 13:41 ` Joe Perches
2014-06-23 13:41 ` [PATCH 01/22] pci-dma-compat: Add pci_zalloc_consistent helper Joe Perches
2014-06-23 13:41   ` Joe Perches
2014-06-23 13:53   ` Arnd Bergmann
2014-06-23 13:53     ` Arnd Bergmann
2014-06-23 14:03     ` Joe Perches
2014-06-25 19:27   ` Andrew Morton
2014-06-25 21:51     ` Joe Perches
2014-06-25 21:57       ` Andrew Morton
2014-06-25 22:14         ` Joe Perches
2014-06-25 22:31       ` David Miller
2014-06-23 17:25 ` [PATCH 00/22] Add and use pci_zalloc_consistent Luis R. Rodriguez
2014-06-23 17:25   ` Luis R. Rodriguez
2014-06-23 19:13   ` Joe Perches
2014-06-23 19:13     ` Joe Perches
2014-06-23 23:27     ` Julian Calaby
2014-06-24 11:32       ` Johannes Berg
     [not found] ` <cover.1403530604.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
2014-06-23 21:49   ` David Miller
2014-06-23 21:49     ` David Miller
2014-06-25 19:41 ` John W. Linville

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).