[PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[Davidlohr Bueso]

Hi,

The series is really straightforward and based on Peter's work that
introduces[1] the atomic_fetch_$op machinery. Only patch 1 implements
the actual atomic_fetch_{inc,dec} calls based on atomic_fetch_{add,sub}.

The rest of the patches really update callers that are doing the ugly
<op> - N workaround to compute the previous state before the variable's
modification.

Applies on today's -tip.

Thanks!

[1] https://lkml.org/lkml/2016/5/31/327

Davidlohr Bueso (12):
  locking/atomic: Introduce inc/dec calls for FETCH-OP flavors
  net/neighbour: Employ atomic_fetch_inc()
  PM,devfreq: Employ atomic_fetch_inc()
  EDAC: Employ atomic_fetch_inc()
  tty/serial:  Employ atomic_fetch_inc()
  HID,wacom: Employ atomic_fetch_inc()
  drivers/media: Employ atomic_fetch_inc()
  infiniband: Employ atomic_fetch_inc()
  drivers/hv: Employ atomic_fetch_inc()
  s390/scm_block: Employ atomic_fetch_inc()
  scsi: Employ atomic_fetch_inc()
  dma-buf/fence: Employ atomic_fetch_add

 drivers/devfreq/devfreq-event.c          |   2 +-
 drivers/dma-buf/fence.c                  |   2 +-
 drivers/edac/edac_device.c               |   2 +-
 drivers/edac/edac_pci.c                  |   2 +-
 drivers/hid/wacom_sys.c                  |   2 +-
 drivers/hv/channel.c                     |   3 +-
 drivers/infiniband/hw/cxgb4/device.c     |   3 +-
 drivers/infiniband/hw/hfi1/verbs.c       |   3 +-
 drivers/media/pci/cobalt/cobalt-driver.c |   2 +-
 drivers/media/pci/cx18/cx18-driver.c     |   2 +-
 drivers/media/v4l2-core/v4l2-device.c    |   2 +-
 drivers/s390/block/scm_blk.c             |   2 +-
 drivers/scsi/fcoe/fcoe_sysfs.c           |   4 +-
 drivers/scsi/scsi_lib.c                  |   6 +-
 drivers/tty/serial/ioc4_serial.c         |   2 +-
 drivers/tty/serial/msm_serial.c          |   2 +-
 include/linux/atomic.h                   | 104 +++++++++++++++++++++++++++++++
 net/core/neighbour.c                     |   2 +-
 18 files changed, 124 insertions(+), 23 deletions(-)

--
2.6.6

[PATCH 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

[Davidlohr Bueso]

With the inclusion of atomic FETCH-OP variants, many places in the
kernel can make use of atomic_fetch_$op() to avoid the hacky callers
that need to compute the value/state _before_ the operation. Peter
laid out the machinery but we are still missing the simpler dec,inc
calls (which future patches will make use of).

This patch only deals with the generic code, as at least right now
no arch actually implement them -- which is similar to what the
OP-RETURN primitives currently do.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 include/linux/atomic.h | 104 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 12d910d61b83..3017dd7ee654 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -188,6 +188,34 @@
 #endif
 #endif /* atomic_fetch_add_relaxed */
 
+/* atomic_fetch_inc_relaxed */
+#ifndef atomic_fetch_inc_relaxed
+#define atomic_fetch_inc_relaxed	atomic_fetch_inc
+#define atomic_fetch_inc_acquire	atomic_fetch_inc
+#define atomic_fetch_inc_release	atomic_fetch_inc
+
+#else /* atomic_fetch_inc_relaxed */
+
+#ifndef atomic_fetch_inc_acquire
+#define atomic_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_inc_release
+#define atomic_fetch_inc_release(...)					\
+	__atomic_op_release(atomic_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_add
+#define atomic_fetch_add(...)						\
+	__atomic_op_fence(atomic_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_inc_relaxed */
+
+#ifndef atomic_fetch_inc
+#define atomic_fetch_inc(v)  (atomic_fetch_add(1, v))
+#endif
+
 /* atomic_fetch_sub_relaxed */
 #ifndef atomic_fetch_sub_relaxed
 #define atomic_fetch_sub_relaxed	atomic_fetch_sub
@@ -212,6 +240,34 @@
 #endif
 #endif /* atomic_fetch_sub_relaxed */
 
+/* atomic_fetch_dec_relaxed */
+#ifndef atomic_fetch_dec_relaxed
+#define atomic_fetch_dec_relaxed	atomic_fetch_dec
+#define atomic_fetch_dec_acquire	atomic_fetch_dec
+#define atomic_fetch_dec_release	atomic_fetch_dec
+
+#else /* atomic_fetch_dec_relaxed */
+
+#ifndef atomic_fetch_dec_acquire
+#define atomic_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_dec_release
+#define atomic_fetch_dec_release(...)					\
+	__atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(...)						\
+	__atomic_op_fence(atomic_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_dec_relaxed */
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(v)  (atomic_fetch_sub(1, v))
+#endif
+
 /* atomic_fetch_or_relaxed */
 #ifndef atomic_fetch_or_relaxed
 #define atomic_fetch_or_relaxed	atomic_fetch_or
@@ -697,6 +753,30 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_add_relaxed */
 
+/* atomic64_fetch_inc_relaxed */
+#ifndef atomic64_fetch_inc_relaxed
+#define atomic64_fetch_inc_relaxed	atomic64_fetch_inc
+#define atomic64_fetch_inc_acquire	atomic64_fetch_inc
+#define atomic64_fetch_inc_release	atomic64_fetch_inc
+
+#else /* atomic64_fetch_inc_relaxed */
+
+#ifndef atomic64_fetch_inc_acquire
+#define atomic64_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_inc_release
+#define atomic64_fetch_inc_release(...)					\
+	__atomic_op_release(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_inc
+#define atomic64_fetch_inc(...)						\
+	__atomic_op_fence(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_inc_relaxed */
+
 /* atomic64_fetch_sub_relaxed */
 #ifndef atomic64_fetch_sub_relaxed
 #define atomic64_fetch_sub_relaxed	atomic64_fetch_sub
@@ -721,6 +801,30 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_sub_relaxed */
 
+/* atomic64_fetch_dec_relaxed */
+#ifndef atomic64_fetch_dec_relaxed
+#define atomic64_fetch_dec_relaxed	atomic64_fetch_dec
+#define atomic64_fetch_dec_acquire	atomic64_fetch_dec
+#define atomic64_fetch_dec_release	atomic64_fetch_dec
+
+#else /* atomic64_fetch_dec_relaxed */
+
+#ifndef atomic64_fetch_dec_acquire
+#define atomic64_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_dec_release
+#define atomic64_fetch_dec_release(...)					\
+	__atomic_op_release(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_dec
+#define atomic64_fetch_dec(...)						\
+	__atomic_op_fence(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_dec_relaxed */
+
 /* atomic64_fetch_or_relaxed */
 #ifndef atomic64_fetch_or_relaxed
 #define atomic64_fetch_or_relaxed	atomic64_fetch_or
-- 
2.6.6

[PATCH 02/12] net/neighbour: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 net/core/neighbour.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 29dd8cc22bbf..60e981a8735e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -269,7 +269,7 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl, struct net_device
 	unsigned long now = jiffies;
 	int entries;
 
-	entries = atomic_inc_return(&tbl->entries) - 1;
+	entries = atomic_fetch_inc(&tbl->entries);
 	if (entries >= tbl->gc_thresh3 ||
 	    (entries >= tbl->gc_thresh2 &&
 	     time_after(now, tbl->last_flush + 5 * HZ))) {
-- 
2.6.6

[PATCH 03/12] PM,devfreq: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: Chanwoo Choi <cw00.choi@samsung.com>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/devfreq/devfreq-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
index 39b048eda2ce..45b02d3d7e7d 100644
--- a/drivers/devfreq/devfreq-event.c
+++ b/drivers/devfreq/devfreq-event.c
@@ -329,7 +329,7 @@ struct devfreq_event_dev *devfreq_event_add_edev(struct device *dev,
 	edev->dev.class = devfreq_event_class;
 	edev->dev.release = devfreq_event_release_edev;
 
-	dev_set_name(&edev->dev, "event.%d", atomic_inc_return(&event_no) - 1);
+	dev_set_name(&edev->dev, "event.%d", atomic_fetch_inc(&event_no));
 	ret = device_register(&edev->dev);
 	if (ret < 0) {
 		put_device(&edev->dev);
-- 
2.6.6

[PATCH 04/12] EDAC: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: Doug Thompson <dougthompson@xmission.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/edac/edac_device.c | 2 +-
 drivers/edac/edac_pci.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index a97900333e2d..00a6ea02ac0d 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -470,7 +470,7 @@ int edac_device_alloc_index(void)
 {
 	static atomic_t device_indexes = ATOMIC_INIT(0);
 
-	return atomic_inc_return(&device_indexes) - 1;
+	return atomic_fetch_inc(&device_indexes);
 }
 EXPORT_SYMBOL_GPL(edac_device_alloc_index);
 
diff --git a/drivers/edac/edac_pci.c b/drivers/edac/edac_pci.c
index 8f2f2899a7a2..c6d8f5df4156 100644
--- a/drivers/edac/edac_pci.c
+++ b/drivers/edac/edac_pci.c
@@ -224,7 +224,7 @@ static void edac_pci_workq_function(struct work_struct *work_req)
  */
 int edac_pci_alloc_index(void)
 {
-	return atomic_inc_return(&pci_indexes) - 1;
+	return atomic_fetch_inc(&pci_indexes);
 }
 EXPORT_SYMBOL_GPL(edac_pci_alloc_index);
 
-- 
2.6.6

[PATCH 05/12] tty/serial: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Pat Gefre <pfg@sgi.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/tty/serial/ioc4_serial.c | 2 +-
 drivers/tty/serial/msm_serial.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/ioc4_serial.c b/drivers/tty/serial/ioc4_serial.c
index e5c42fef69d2..5982ae159d56 100644
--- a/drivers/tty/serial/ioc4_serial.c
+++ b/drivers/tty/serial/ioc4_serial.c
@@ -974,7 +974,7 @@ intr_connect(struct ioc4_soft *soft, int type,
 	BUG_ON(!((type == IOC4_SIO_INTR_TYPE)
 	       || (type == IOC4_OTHER_INTR_TYPE)));
 
-	i = atomic_inc_return(&soft-> is_intr_type[type].is_num_intrs) - 1;
+	i = atomic_fetch_inc(&soft-> is_intr_type[type].is_num_intrs);
 	BUG_ON(!(i < MAX_IOC4_INTR_ENTS || (printk("i %d\n", i), 0)));
 
 	/* Save off the lower level interrupt handler */
diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b7d80bd57db9..a6ae591e90d3 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -1585,7 +1585,7 @@ static int msm_serial_probe(struct platform_device *pdev)
 		line = pdev->id;
 
 	if (line < 0)
-		line = atomic_inc_return(&msm_uart_next_id) - 1;
+		line = atomic_fetch_inc(&msm_uart_next_id);
 
 	if (unlikely(line < 0 || line >= UART_NR))
 		return -ENXIO;
-- 
2.6.6

[PATCH 06/12] HID,wacom: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: Jiri Kosina <jikos@kernel.org>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/hid/wacom_sys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 499cc8213cfe..88bde7687edd 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1088,7 +1088,7 @@ static int wacom_initialize_battery(struct wacom *wacom)
 	if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
 		struct power_supply_desc *bat_desc = &wacom->battery_desc;
 		struct power_supply_desc *ac_desc = &wacom->ac_desc;
-		n = atomic_inc_return(&battery_no) - 1;
+		n = atomic_fetch_inc(&battery_no);
 
 		bat_desc->properties = wacom_battery_props;
 		bat_desc->num_properties = ARRAY_SIZE(wacom_battery_props);
-- 
2.6.6

[PATCH 07/12] drivers/media: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: linux-media@vger.kernel.org
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/media/pci/cobalt/cobalt-driver.c | 2 +-
 drivers/media/pci/cx18/cx18-driver.c     | 2 +-
 drivers/media/v4l2-core/v4l2-device.c    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/cobalt/cobalt-driver.c b/drivers/media/pci/cobalt/cobalt-driver.c
index 8d6f04fc8013..70dfcb0c6a72 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.c
+++ b/drivers/media/pci/cobalt/cobalt-driver.c
@@ -683,7 +683,7 @@ static int cobalt_probe(struct pci_dev *pci_dev,
 	int i;
 
 	/* FIXME - module parameter arrays constrain max instances */
-	i = atomic_inc_return(&cobalt_instance) - 1;
+	i = atomic_fetch_inc(&cobalt_instance);
 
 	cobalt = kzalloc(sizeof(struct cobalt), GFP_ATOMIC);
 	if (cobalt == NULL)
diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
index 260e462d91b4..5cb3408c6859 100644
--- a/drivers/media/pci/cx18/cx18-driver.c
+++ b/drivers/media/pci/cx18/cx18-driver.c
@@ -908,7 +908,7 @@ static int cx18_probe(struct pci_dev *pci_dev,
 	struct cx18 *cx;
 
 	/* FIXME - module parameter arrays constrain max instances */
-	i = atomic_inc_return(&cx18_instance) - 1;
+	i = atomic_fetch_inc(&cx18_instance);
 	if (i >= CX18_MAX_CARDS) {
 		printk(KERN_ERR "cx18: cannot manage card %d, driver has a "
 		       "limit of 0 - %d\n", i, CX18_MAX_CARDS - 1);
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 06fa5f1b2cff..1bc5b68c2724 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -76,7 +76,7 @@ EXPORT_SYMBOL_GPL(v4l2_device_put);
 int v4l2_device_set_name(struct v4l2_device *v4l2_dev, const char *basename,
 						atomic_t *instance)
 {
-	int num = atomic_inc_return(instance) - 1;
+	int num = atomic_fetch_inc(instance);
 	int len = strlen(basename);
 
 	if (basename[len - 1] >= '0' && basename[len - 1] <= '9')
-- 
2.6.6

[PATCH 08/12] infiniband: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: Doug Ledford <dledford@redhat.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/infiniband/hw/cxgb4/device.c | 3 +--
 drivers/infiniband/hw/hfi1/verbs.c   | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index ae2e8b23d2dd..a32959fa2c82 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -111,8 +111,7 @@ void c4iw_log_wr_stats(struct t4_wq *wq, struct t4_cqe *cqe)
 	if (!wq->rdev->wr_log)
 		return;
 
-	idx = (atomic_inc_return(&wq->rdev->wr_log_idx) - 1) &
-		(wq->rdev->wr_log_size - 1);
+	idx = atomic_fetch_inc(&wq->rdev->wr_log_idx) & (wq->rdev->wr_log_size - 1);
 	le.poll_sge_ts = cxgb4_read_sge_timestamp(wq->rdev->lldi.ports[0]);
 	getnstimeofday(&le.poll_host_ts);
 	le.valid = 1;
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index 849c4b9399d4..9ba311b53085 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -261,8 +261,7 @@ static void wss_advance_clean_counter(void)
 		 * wss.clean_entry.  The table size, wss.num_entries,
 		 * is always a power-of-2.
 		 */
-		entry = (atomic_inc_return(&wss.clean_entry) - 1)
-			& (wss.num_entries - 1);
+		entry = atomic_fetch_inc(&wss.clean_entry) & (wss.num_entries - 1);
 
 		/* clear the entry and count the bits */
 		bits = xchg(&wss.entries[entry], 0);
-- 
2.6.6

[PATCH 09/12] drivers/hv: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/hv/channel.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 56dd261f7142..6f2421a62947 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -397,8 +397,7 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 	unsigned long flags;
 	int ret = 0;
 
-	next_gpadl_handle =
-		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
+	next_gpadl_handle = atomic_fetch_inc(&vmbus_connection.next_gpadl_handle);
 
 	ret = create_gpadl_header(kbuffer, size, &msginfo, &msgcount);
 	if (ret)
-- 
2.6.6

[PATCH 10/12] s390/scm_block: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/s390/block/scm_blk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/block/scm_blk.c b/drivers/s390/block/scm_blk.c
index e6f54d3b8969..eef68990474a 100644
--- a/drivers/s390/block/scm_blk.c
+++ b/drivers/s390/block/scm_blk.c
@@ -475,7 +475,7 @@ int scm_blk_dev_setup(struct scm_blk_dev *bdev, struct scm_device *scmdev)
 	int len, ret = -ENOMEM;
 	unsigned int devindex, nr_max_blk;
 
-	devindex = atomic_inc_return(&nr_devices) - 1;
+	devindex = atomic_fetch_inc(&nr_devices);
 	/* scma..scmz + scmaa..scmzz */
 	if (devindex > 701) {
 		ret = -ENODEV;
-- 
2.6.6

[PATCH 11/12] scsi: Employ atomic_fetch_inc()

[Davidlohr Bueso]

Now that we have fetch_inc() we can stop using inc_return() - 1.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/scsi/fcoe/fcoe_sysfs.c | 4 ++--
 drivers/scsi/scsi_lib.c        | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index 045c4e11ee54..3d6833fbdea7 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -685,7 +685,7 @@ struct fcoe_ctlr_device *fcoe_ctlr_device_add(struct device *parent,
 	if (!ctlr)
 		goto out;
 
-	ctlr->id = atomic_inc_return(&ctlr_num) - 1;
+	ctlr->id = atomic_fetch_inc(&ctlr_num);
 	ctlr->f = f;
 	ctlr->mode = FIP_CONN_TYPE_FABRIC;
 	INIT_LIST_HEAD(&ctlr->fcfs);
@@ -902,7 +902,7 @@ struct fcoe_fcf_device *fcoe_fcf_device_add(struct fcoe_ctlr_device *ctlr,
 	fcf->dev.parent = &ctlr->dev;
 	fcf->dev.bus = &fcoe_bus_type;
 	fcf->dev.type = &fcoe_fcf_device_type;
-	fcf->id = atomic_inc_return(&fcf_num) - 1;
+	fcf->id = atomic_fetch_inc(&fcf_num);
 	fcf->state = FCOE_FCF_STATE_UNKNOWN;
 
 	fcf->dev_loss_tmo = ctlr->fcf_dev_loss_tmo;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c71344aebdbb..113ec916210d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1322,7 +1322,7 @@ static inline int scsi_dev_queue_ready(struct request_queue *q,
 {
 	unsigned int busy;
 
-	busy = atomic_inc_return(&sdev->device_busy) - 1;
+	busy = atomic_fetch_inc(&sdev->device_busy);
 	if (atomic_read(&sdev->device_blocked)) {
 		if (busy)
 			goto out_dec;
@@ -1375,7 +1375,7 @@ static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
 	if (starget->can_queue <= 0)
 		return 1;
 
-	busy = atomic_inc_return(&starget->target_busy) - 1;
+	busy = atomic_fetch_inc(&starget->target_busy);
 	if (atomic_read(&starget->target_blocked) > 0) {
 		if (busy)
 			goto starved;
@@ -1419,7 +1419,7 @@ static inline int scsi_host_queue_ready(struct request_queue *q,
 	if (scsi_host_in_recovery(shost))
 		return 0;
 
-	busy = atomic_inc_return(&shost->host_busy) - 1;
+	busy = atomic_fetch_inc(&shost->host_busy);
 	if (atomic_read(&shost->host_blocked) > 0) {
 		if (busy)
 			goto starved;
-- 
2.6.6

[PATCH 12/12] dma-buf/fence: Employ atomic_fetch_add

[Davidlohr Bueso]

Now that we have fetch_add() we can stop using add_return() - num.

These are very similar to the existing OP-RETURN primitives we already
have, except they return the value of the atomic variable _before_
modification.

Cc: Sumit Semwal <sumit.semwal@linaro.org>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 drivers/dma-buf/fence.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
index 7b05dbe9b296..a58da5da112c 100644
--- a/drivers/dma-buf/fence.c
+++ b/drivers/dma-buf/fence.c
@@ -47,7 +47,7 @@ static atomic_t fence_context_counter = ATOMIC_INIT(0);
 unsigned fence_context_alloc(unsigned num)
 {
 	BUG_ON(!num);
-	return atomic_add_return(num, &fence_context_counter) - num;
+	return atomic_fetch_add(num, &fence_context_counter);
 }
 EXPORT_SYMBOL(fence_context_alloc);
 
-- 
2.6.6

Re: [PATCH 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

[Peter Zijlstra]

On Mon, Jun 20, 2016 at 01:05:53PM -0700, Davidlohr Bueso wrote:
> +#ifndef atomic_fetch_inc
> +#define atomic_fetch_inc(v)  (atomic_fetch_add(1, v))
> +#endif

> +#ifndef atomic_fetch_dec
> +#define atomic_fetch_dec(v)  (atomic_fetch_sub(1, v))
> +#endif

You're missing these for atomic64

[PATCH v2 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

[Davidlohr Bueso]

With the inclusion of atomic FETCH-OP variants, many places in the
kernel can make use of atomic_fetch_$op() to avoid the hacky callers
that need to compute the value/state _before_ the operation. Peter
laid out the machinery but we are still missing the simpler dec,inc
calls (which future patches will make use of).

This patch only deals with the generic code, as at least right now
no arch actually implement them -- which is similar to what the
OP-RETURN primitives currently do.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---

Changes from v1: added atomic64 flavors, only define atomic_fetch_inc/dec once,
                 the fence logic will come through the add/sub calls instead.

 include/linux/atomic.h | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 12d910d61b83..c848d89bc9a0 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -188,6 +188,29 @@
 #endif
 #endif /* atomic_fetch_add_relaxed */
 
+#ifndef atomic_fetch_inc
+#define atomic_fetch_inc(v)  (atomic_fetch_add(1, v))
+#endif
+
+/* atomic_fetch_inc_relaxed */
+#ifndef atomic_fetch_inc_relaxed
+#define atomic_fetch_inc_relaxed	atomic_fetch_inc
+#define atomic_fetch_inc_acquire	atomic_fetch_inc
+#define atomic_fetch_inc_release	atomic_fetch_inc
+
+#else /* atomic_fetch_inc_relaxed */
+
+#ifndef atomic_fetch_inc_acquire
+#define atomic_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_inc_release
+#define atomic_fetch_inc_release(...)					\
+	__atomic_op_release(atomic_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_inc_relaxed */
+
 /* atomic_fetch_sub_relaxed */
 #ifndef atomic_fetch_sub_relaxed
 #define atomic_fetch_sub_relaxed	atomic_fetch_sub
@@ -212,6 +235,33 @@
 #endif
 #endif /* atomic_fetch_sub_relaxed */
 
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(v)  (atomic_fetch_sub(1, v))
+#endif
+
+/* atomic_fetch_dec_relaxed */
+#ifndef atomic_fetch_dec_relaxed
+#define atomic_fetch_dec_relaxed	atomic_fetch_dec
+#define atomic_fetch_dec_acquire	atomic_fetch_dec
+#define atomic_fetch_dec_release	atomic_fetch_dec
+
+#else /* atomic_fetch_dec_relaxed */
+
+#ifndef atomic_fetch_dec_acquire
+#define atomic_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_dec_release
+#define atomic_fetch_dec_release(...)					\
+	__atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_dec_relaxed */
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(v)  (atomic_fetch_sub(1, v))
+#endif
+
 /* atomic_fetch_or_relaxed */
 #ifndef atomic_fetch_or_relaxed
 #define atomic_fetch_or_relaxed	atomic_fetch_or
@@ -697,6 +747,29 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_add_relaxed */
 
+#ifndef atomic64_fetch_inc
+#define atomic64_fetch_inc(v)  (atomic64_fetch_add(1, v))
+#endif
+
+/* atomic64_fetch_inc_relaxed */
+#ifndef atomic64_fetch_inc_relaxed
+#define atomic64_fetch_inc_relaxed	atomic64_fetch_inc
+#define atomic64_fetch_inc_acquire	atomic64_fetch_inc
+#define atomic64_fetch_inc_release	atomic64_fetch_inc
+
+#else /* atomic64_fetch_inc_relaxed */
+
+#ifndef atomic64_fetch_inc_acquire
+#define atomic64_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_inc_release
+#define atomic64_fetch_inc_release(...)					\
+	__atomic_op_release(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_inc_relaxed */
+
 /* atomic64_fetch_sub_relaxed */
 #ifndef atomic64_fetch_sub_relaxed
 #define atomic64_fetch_sub_relaxed	atomic64_fetch_sub
@@ -721,6 +794,29 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_sub_relaxed */
 
+#ifndef atomic64_fetch_dec
+#define atomic64_fetch_dec(v)  (atomic64_fetch_sub(1, v))
+#endif
+
+/* atomic64_fetch_dec_relaxed */
+#ifndef atomic64_fetch_dec_relaxed
+#define atomic64_fetch_dec_relaxed	atomic64_fetch_dec
+#define atomic64_fetch_dec_acquire	atomic64_fetch_dec
+#define atomic64_fetch_dec_release	atomic64_fetch_dec
+
+#else /* atomic64_fetch_dec_relaxed */
+
+#ifndef atomic64_fetch_dec_acquire
+#define atomic64_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_dec_release
+#define atomic64_fetch_dec_release(...)					\
+	__atomic_op_release(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_dec_relaxed */
+
 /* atomic64_fetch_or_relaxed */
 #ifndef atomic64_fetch_or_relaxed
 #define atomic64_fetch_or_relaxed	atomic64_fetch_or
-- 
2.6.6

Re: [PATCH 04/12] EDAC: Employ atomic_fetch_inc()

[Borislav Petkov]

On Mon, Jun 20, 2016 at 01:05:56PM -0700, Davidlohr Bueso wrote:
> Now that we have fetch_inc() we can stop using inc_return() - 1.
> 
> These are very similar to the existing OP-RETURN primitives we already
> have, except they return the value of the atomic variable _before_
> modification.
> 
> Cc: Doug Thompson <dougthompson@xmission.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> ---
>  drivers/edac/edac_device.c | 2 +-
>  drivers/edac/edac_pci.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v2 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

[Davidlohr Bueso]

Sorry, I was also missing the _long variants. While at it I added a missing
ATOMIC_LONG_FETCH_INC_DEC_OP undef.

--------------8<-----------------------------
From: Davidlohr Bueso <dave@stgolabs.net>
Subject: [PATCH -v3 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

With the inclusion of atomic FETCH-OP variants, many places in the
kernel can make use of atomic_fetch_$op() to avoid the hacky callers
that need to compute the value/state _before_ the operation. Peter
laid out the machinery but we are still missing the simpler dec,inc
calls (which future patches will make use of).

This patch only deals with the generic code, as at least right now
no arch actually implement them -- which is similar to what the
OP-RETURN primitives currently do.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 include/asm-generic/atomic-long.h | 22 +++++++++
 include/linux/atomic.h            | 96 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+)

diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
index 2d0d3cf791ab..288cc9e96395 100644
--- a/include/asm-generic/atomic-long.h
+++ b/include/asm-generic/atomic-long.h
@@ -146,6 +146,28 @@ ATOMIC_LONG_FETCH_OP(xor, _relaxed)
 ATOMIC_LONG_FETCH_OP(xor, _acquire)
 ATOMIC_LONG_FETCH_OP(xor, _release)

+#undef ATOMIC_LONG_FETCH_OP
+
+#define ATOMIC_LONG_FETCH_INC_DEC_OP(op, mo)					\
+static inline long							\
+atomic_long_fetch_##op##mo(atomic_long_t *l)				\
+{									\
+	ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l;		\
+									\
+	return (long)ATOMIC_LONG_PFX(_fetch_##op##mo)(v);		\
+}
+
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc,)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _relaxed)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _acquire)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _release)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec,)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _relaxed)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _acquire)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _release)
+
+#undef ATOMIC_LONG_FETCH_INC_DEC_OP
+
 #define ATOMIC_LONG_OP(op)						\
 static __always_inline void						\
 atomic_long_##op(long i, atomic_long_t *l)				\
diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 12d910d61b83..c848d89bc9a0 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -188,6 +188,29 @@
 #endif
 #endif /* atomic_fetch_add_relaxed */

+#ifndef atomic_fetch_inc
+#define atomic_fetch_inc(v)  (atomic_fetch_add(1, v))
+#endif
+
+/* atomic_fetch_inc_relaxed */
+#ifndef atomic_fetch_inc_relaxed
+#define atomic_fetch_inc_relaxed	atomic_fetch_inc
+#define atomic_fetch_inc_acquire	atomic_fetch_inc
+#define atomic_fetch_inc_release	atomic_fetch_inc
+
+#else /* atomic_fetch_inc_relaxed */
+
+#ifndef atomic_fetch_inc_acquire
+#define atomic_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_inc_release
+#define atomic_fetch_inc_release(...)					\
+	__atomic_op_release(atomic_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_inc_relaxed */
+
 /* atomic_fetch_sub_relaxed */
 #ifndef atomic_fetch_sub_relaxed
 #define atomic_fetch_sub_relaxed	atomic_fetch_sub
@@ -212,6 +235,33 @@
 #endif
 #endif /* atomic_fetch_sub_relaxed */

+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(v)  (atomic_fetch_sub(1, v))
+#endif
+
+/* atomic_fetch_dec_relaxed */
+#ifndef atomic_fetch_dec_relaxed
+#define atomic_fetch_dec_relaxed	atomic_fetch_dec
+#define atomic_fetch_dec_acquire	atomic_fetch_dec
+#define atomic_fetch_dec_release	atomic_fetch_dec
+
+#else /* atomic_fetch_dec_relaxed */
+
+#ifndef atomic_fetch_dec_acquire
+#define atomic_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_dec_release
+#define atomic_fetch_dec_release(...)					\
+	__atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_dec_relaxed */
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(v)  (atomic_fetch_sub(1, v))
+#endif
+
 /* atomic_fetch_or_relaxed */
 #ifndef atomic_fetch_or_relaxed
 #define atomic_fetch_or_relaxed	atomic_fetch_or
@@ -697,6 +747,29 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_add_relaxed */

+#ifndef atomic64_fetch_inc
+#define atomic64_fetch_inc(v)  (atomic64_fetch_add(1, v))
+#endif
+
+/* atomic64_fetch_inc_relaxed */
+#ifndef atomic64_fetch_inc_relaxed
+#define atomic64_fetch_inc_relaxed	atomic64_fetch_inc
+#define atomic64_fetch_inc_acquire	atomic64_fetch_inc
+#define atomic64_fetch_inc_release	atomic64_fetch_inc
+
+#else /* atomic64_fetch_inc_relaxed */
+
+#ifndef atomic64_fetch_inc_acquire
+#define atomic64_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_inc_release
+#define atomic64_fetch_inc_release(...)					\
+	__atomic_op_release(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_inc_relaxed */
+
 /* atomic64_fetch_sub_relaxed */
 #ifndef atomic64_fetch_sub_relaxed
 #define atomic64_fetch_sub_relaxed	atomic64_fetch_sub
@@ -721,6 +794,29 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_sub_relaxed */

+#ifndef atomic64_fetch_dec
+#define atomic64_fetch_dec(v)  (atomic64_fetch_sub(1, v))
+#endif
+
+/* atomic64_fetch_dec_relaxed */
+#ifndef atomic64_fetch_dec_relaxed
+#define atomic64_fetch_dec_relaxed	atomic64_fetch_dec
+#define atomic64_fetch_dec_acquire	atomic64_fetch_dec
+#define atomic64_fetch_dec_release	atomic64_fetch_dec
+
+#else /* atomic64_fetch_dec_relaxed */
+
+#ifndef atomic64_fetch_dec_acquire
+#define atomic64_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_dec_release
+#define atomic64_fetch_dec_release(...)					\
+	__atomic_op_release(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_dec_relaxed */
+
 /* atomic64_fetch_or_relaxed */
 #ifndef atomic64_fetch_or_relaxed
 #define atomic64_fetch_or_relaxed	atomic64_fetch_or
--
2.6.6

Re: [PATCH 06/12] HID,wacom: Employ atomic_fetch_inc()

[Jiri Kosina]

On Mon, 20 Jun 2016, Davidlohr Bueso wrote:

> Now that we have fetch_inc() we can stop using inc_return() - 1.
> 
> These are very similar to the existing OP-RETURN primitives we already
> have, except they return the value of the atomic variable _before_
> modification.
> 
> Cc: Jiri Kosina <jikos@kernel.org>
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>

Acked-by: Jiri Kosina <jkosina@suse.cz>

> ---
>  drivers/hid/wacom_sys.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 499cc8213cfe..88bde7687edd 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -1088,7 +1088,7 @@ static int wacom_initialize_battery(struct wacom *wacom)
>  	if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
>  		struct power_supply_desc *bat_desc = &wacom->battery_desc;
>  		struct power_supply_desc *ac_desc = &wacom->ac_desc;
> -		n = atomic_inc_return(&battery_no) - 1;
> +		n = atomic_fetch_inc(&battery_no);
>  
>  		bat_desc->properties = wacom_battery_props;
>  		bat_desc->num_properties = ARRAY_SIZE(wacom_battery_props);
> -- 
> 2.6.6
> 

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

[Peter Zijlstra]

On Tue, Jun 21, 2016 at 09:36:11AM -0700, Davidlohr Bueso wrote:
> Sorry, I was also missing the _long variants. While at it I added a missing
> ATOMIC_LONG_FETCH_INC_DEC_OP undef.
> 
> --------------8<-----------------------------
> From: Davidlohr Bueso <dave@stgolabs.net>
> Subject: [PATCH -v3 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors
> 
> With the inclusion of atomic FETCH-OP variants, many places in the
> kernel can make use of atomic_fetch_$op() to avoid the hacky callers
> that need to compute the value/state _before_ the operation. Peter
> laid out the machinery but we are still missing the simpler dec,inc
> calls (which future patches will make use of).
> 
> This patch only deals with the generic code, as at least right now
> no arch actually implement them -- which is similar to what the
> OP-RETURN primitives currently do.


Would something like so make sense?

---
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -188,15 +188,18 @@
 #endif
 #endif /* atomic_fetch_add_relaxed */
 
-#ifndef atomic_fetch_inc
-#define atomic_fetch_inc(v)  (atomic_fetch_add(1, v))
-#endif
-
 /* atomic_fetch_inc_relaxed */
 #ifndef atomic_fetch_inc_relaxed
+
+#ifndef atomic_fetch_inc
+#define atomic_fetch_inc_relaxed(v)	atomic_fetch_add_relaxed(1, (v))
+#define atomic_fetch_inc_acquire(v)	atomic_fetch_add_acquire(1, (v))
+#define atomic_fetch_inc_release(v)	atomic_fetch_add_release(1, (v))
+#else /* atomic_fetch_inc */
 #define atomic_fetch_inc_relaxed	atomic_fetch_inc
 #define atomic_fetch_inc_acquire	atomic_fetch_inc
 #define atomic_fetch_inc_release	atomic_fetch_inc
+#endif /* atomic_fetch_inc */
 
 #else /* atomic_fetch_inc_relaxed */
 
@@ -235,15 +238,18 @@
 #endif
 #endif /* atomic_fetch_sub_relaxed */
 
-#ifndef atomic_fetch_dec
-#define atomic_fetch_dec(v)  (atomic_fetch_sub(1, v))
-#endif
-
 /* atomic_fetch_dec_relaxed */
 #ifndef atomic_fetch_dec_relaxed
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec_relaxed(v)	atomic_fetch_sub_relaxed(1, (v))
+#define atomic_fetch_dec_acquire(v)	atomic_fetch_sub_acquire(1, (v))
+#define atomic_fetch_dec_release(v)	atomic_fetch_sub_release(1, (v))
+#else /* atomic_fetch_dec */
 #define atomic_fetch_dec_relaxed	atomic_fetch_dec
 #define atomic_fetch_dec_acquire	atomic_fetch_dec
 #define atomic_fetch_dec_release	atomic_fetch_dec
+#endif /* atomic_fetch_dec */
 
 #else /* atomic_fetch_dec_relaxed */
 
@@ -753,9 +759,16 @@ static inline int atomic_dec_if_positive
 
 /* atomic64_fetch_inc_relaxed */
 #ifndef atomic64_fetch_inc_relaxed
+
+#ifndef atomic64_fetch_inc
+#define atomic64_fetch_inc_relaxed(v)	atomic64_fetch_add_relaxed(1, (v))
+#define atomic64_fetch_inc_acquire(v)	atomic64_fetch_add_acquire(1, (v))
+#define atomic64_fetch_inc_release(v)	atomic64_fetch_add_release(1, (v))
+#else /* atomic64_fetch_inc */
 #define atomic64_fetch_inc_relaxed	atomic64_fetch_inc
 #define atomic64_fetch_inc_acquire	atomic64_fetch_inc
 #define atomic64_fetch_inc_release	atomic64_fetch_inc
+#endif /* atomic64_fetch_inc */
 
 #else /* atomic64_fetch_inc_relaxed */
 
@@ -794,15 +807,18 @@ static inline int atomic_dec_if_positive
 #endif
 #endif /* atomic64_fetch_sub_relaxed */
 
-#ifndef atomic64_fetch_dec
-#define atomic64_fetch_dec(v)  (atomic64_fetch_sub(1, v))
-#endif
-
 /* atomic64_fetch_dec_relaxed */
 #ifndef atomic64_fetch_dec_relaxed
+
+#ifndef atomic64_fetch_dec
+#define atomic64_fetch_dec_relaxed(v)	atomic64_fetch_sub_relaxed(1, (v))
+#define atomic64_fetch_dec_acquire(v)	atomic64_fetch_sub_acquire(1, (v))
+#define atomic64_fetch_dec_release(v)	atomic64_fetch_sub_release(1, (v))
+#else /* atomic64_fetch_dec */
 #define atomic64_fetch_dec_relaxed	atomic64_fetch_dec
 #define atomic64_fetch_dec_acquire	atomic64_fetch_dec
 #define atomic64_fetch_dec_release	atomic64_fetch_dec
+#endif /* atomic64_fetch_dec */
 
 #else /* atomic64_fetch_dec_relaxed */
 

Re: [PATCH v2 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

[Davidlohr Bueso]

On Thu, 23 Jun 2016, Peter Zijlstra wrote:

>Would something like so make sense?
>
>---
>--- a/include/linux/atomic.h
>+++ b/include/linux/atomic.h
>@@ -188,15 +188,18 @@
> #endif
> #endif /* atomic_fetch_add_relaxed */
>
>-#ifndef atomic_fetch_inc
>-#define atomic_fetch_inc(v)  (atomic_fetch_add(1, v))
>-#endif

Hmm perhaps I'm a bit lost in all this CPP maze, but where do you in fact
define atomic_fetch_inc() generically here? gcc cannot see it either.

Another option could be to add the above for atomic_fetch_inc inside the
ifndef atomic_fetch_release. Ie:

>-
> /* atomic_fetch_inc_relaxed */
> #ifndef atomic_fetch_inc_relaxed
>+
>+#ifndef atomic_fetch_inc

#define atomic_fetch_inc(v)  (atomic_fetch_add(1, v))

>+#define atomic_fetch_inc_relaxed(v)	atomic_fetch_add_relaxed(1, (v))
>+#define atomic_fetch_inc_acquire(v)	atomic_fetch_add_acquire(1, (v))
>+#define atomic_fetch_inc_release(v)	atomic_fetch_add_release(1, (v))

I was under the impression that if the archs don't define their own calls,
then we always default to fully ordered. Which is why I based all this ifdefery
on what is currently done with the other atomic_fetch_$ops.

>+#else /* atomic_fetch_inc */
> #define atomic_fetch_inc_relaxed	atomic_fetch_inc
> #define atomic_fetch_inc_acquire	atomic_fetch_inc
> #define atomic_fetch_inc_release	atomic_fetch_inc
>+#endif /* atomic_fetch_inc */
>
> #else /* atomic_fetch_inc_relaxed */

Thanks,
Davidlohr

Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[James Bottomley]

On Mon, 2016-06-20 at 13:05 -0700, Davidlohr Bueso wrote:
> Hi,
> 
> The series is really straightforward and based on Peter's work that
> introduces[1] the atomic_fetch_$op machinery. Only patch 1 implements
> the actual atomic_fetch_{inc,dec} calls based on 
> atomic_fetch_{add,sub}.

Could I just ask why?  atomic_inc_return(x) - 1 seems a reasonable
thing to do to me.  Is it because on architectures where atomics are
implemented in asm, it costs us one more CPU instruction to do the
extra decrement which gcc can't optimise?   If that's it, I'm not sure
the added complexity justifies the cycle savings.

James

Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[Davidlohr Bueso]

On Fri, 24 Jun 2016, James Bottomley wrote:

>On Mon, 2016-06-20 at 13:05 -0700, Davidlohr Bueso wrote:
>> Hi,
>>
>> The series is really straightforward and based on Peter's work that
>> introduces[1] the atomic_fetch_$op machinery. Only patch 1 implements
>> the actual atomic_fetch_{inc,dec} calls based on
>> atomic_fetch_{add,sub}.
>
>Could I just ask why?  atomic_inc_return(x) - 1 seems a reasonable
>thing to do to me.

For one restoring the old state like that can be racy and looses the notion
of atomicity. The new family of atomic_fetch_$ops also better express the
purpose of the call imo. Finally, the added machinery (considering it came
from fetch_op() NOHZ needs), was mainly suggested by Linus (although yes, we
don't have users for all the calls): https://lkml.org/lkml/2016/3/15/352.

Thanks,
Davidlohr

Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[James Bottomley]

On Fri, 2016-06-24 at 10:30 -0700, Davidlohr Bueso wrote:
> On Fri, 24 Jun 2016, James Bottomley wrote:
> 
> > On Mon, 2016-06-20 at 13:05 -0700, Davidlohr Bueso wrote:
> > > Hi,
> > > 
> > > The series is really straightforward and based on Peter's work 
> > > that introduces[1] the atomic_fetch_$op machinery. Only patch 1
> > > implements the actual atomic_fetch_{inc,dec} calls based on
> > > atomic_fetch_{add,sub}.
> > 
> > Could I just ask why?  atomic_inc_return(x) - 1 seems a reasonable
> > thing to do to me.
> 
> For one restoring the old state like that can be racy and looses the 
> notion of atomicity.

I don't understand this argument: any return of an atomic value is
inherently racy because the atomic source may have changed by the time
you use the returned value.  It's no more or less racy to my mind to
return the original value and increment than to return the incremented
value and subtract one.

>  The new family of atomic_fetch_$ops also better express the purpose
> of the call imo.

So this is probably the core of my objection: adding APIs simply
because we can.  A good reason to add things like this is because it's
a common pattern people get wrong, because we can optimize it nicely on
an architecture, or some other good reason.  Absent a good reason it
doesn't seem like a good API addition because trying to keep up with
all the API variants when you want to use atomics adds to the burden of
the programmer.

>  Finally, the added machinery (considering it came from fetch_op()
> NOHZ needs), was mainly suggested by Linus (although yes, we don't
> have users for all the calls): 
> https://lkml.org/lkml/2016/3/15/352.

I'd really rather you told me *you* believed it was a good idea (and
why) ...

James

RE: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[KY Srinivasan]

> -----Original Message-----
> From: Davidlohr Bueso [mailto:dave@stgolabs.net]
> Sent: Friday, June 24, 2016 10:30 AM
> To: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: peterz@infradead.org; mingo@kernel.org; davem@davemloft.net;
> cw00.choi@samsung.com; dougthompson@xmission.com; bp@alien8.de;
> mchehab@osg.samsung.com; gregkh@linuxfoundation.org; pfg@sgi.com;
> jikos@kernel.org; hans.verkuil@cisco.com; awalls@md.metrocast.net;
> dledford@redhat.com; sean.hefty@intel.com; KY Srinivasan
> <kys@microsoft.com>; heiko.carstens@de.ibm.com;
> sumit.semwal@linaro.org; schwidefsky@de.ibm.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for
> FETCH-OP flavors
> 
> On Fri, 24 Jun 2016, James Bottomley wrote:
> 
> >On Mon, 2016-06-20 at 13:05 -0700, Davidlohr Bueso wrote:
> >> Hi,
> >>
> >> The series is really straightforward and based on Peter's work that
> >> introduces[1] the atomic_fetch_$op machinery. Only patch 1 implements
> >> the actual atomic_fetch_{inc,dec} calls based on
> >> atomic_fetch_{add,sub}.
> >
> >Could I just ask why?  atomic_inc_return(x) - 1 seems a reasonable
> >thing to do to me.
> 
> For one restoring the old state like that can be racy and looses the notion of
> atomicity. The new family of atomic_fetch_$ops also better express the
How so? Can you expand on the racy part. The subtraction is done on a local copy of
the value.

K. Y

> purpose of the call imo. Finally, the added machinery (considering it came from
> fetch_op() NOHZ needs), was mainly suggested by Linus (although yes, we
> don't have users for all the calls):
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2flkml.org%
> 2flkml%2f2016%2f3%2f15%2f352&data=01%7c01%7ckys%40microsoft.com%
> 7c5c7cfad67568440f6e2108d39c5546e0%7c72f988bf86f141af91ab2d7cd011
> db47%7c1&sdata=uZrdmvDCuTp%2bMNHAXzMPT68w%2bVGtvH2V99nUEBr6
> 1ro%3d.
> 
> Thanks,
> Davidlohr

Re: [PATCH v2 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

[Peter Zijlstra]

On Fri, Jun 24, 2016 at 09:34:30AM -0700, Davidlohr Bueso wrote:
> On Thu, 23 Jun 2016, Peter Zijlstra wrote:
> 
> >-
> >/* atomic_fetch_inc_relaxed */
> >#ifndef atomic_fetch_inc_relaxed
> >+
> >+#ifndef atomic_fetch_inc
> 
> #define atomic_fetch_inc(v)  (atomic_fetch_add(1, v))

Ah yes.

> >+#define atomic_fetch_inc_relaxed(v)	atomic_fetch_add_relaxed(1, (v))
> >+#define atomic_fetch_inc_acquire(v)	atomic_fetch_add_acquire(1, (v))
> >+#define atomic_fetch_inc_release(v)	atomic_fetch_add_release(1, (v))
> 
> I was under the impression that if the archs don't define their own calls,
> then we always default to fully ordered. Which is why I based all this ifdefery
> on what is currently done with the other atomic_fetch_$ops.

Right, but this allows an arch to only define atomic_fetch_add_relaxed()
and have the various forms of fetch_inc() instantiated as well.

With your earlier version they would all fall back to the fully
sequential variant of fetch_add().

Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[Peter Zijlstra]

On Fri, Jun 24, 2016 at 09:46:05AM -0700, James Bottomley wrote:
> On Mon, 2016-06-20 at 13:05 -0700, Davidlohr Bueso wrote:
> > Hi,
> > 
> > The series is really straightforward and based on Peter's work that
> > introduces[1] the atomic_fetch_$op machinery. Only patch 1 implements
> > the actual atomic_fetch_{inc,dec} calls based on 
> > atomic_fetch_{add,sub}.
> 
> Could I just ask why?  atomic_inc_return(x) - 1 seems a reasonable
> thing to do to me.  Is it because on architectures where atomics are
> implemented in asm, it costs us one more CPU instruction to do the
> extra decrement which gcc can't optimise?   If that's it, I'm not sure
> the added complexity justifies the cycle savings.

That boat has sailed, fetch_$op is implemented (in asm mostly) for _all_
architectures already.

All Davidlohr does here is add fetch_{inc,dec}(v) -> fetch_{add,sub}(1,
v) macros because he's lazy.

In any case, fetch_$op is the natural form of atomics that return a
value; Linux has historically chosen the 'wrong' form. The fetch_$op,
test-and-modify, load-store whatever is what hardware typically does
natively and is what works for irreversible operations.

Sure, for reversible operations (add/sub) what you say can (and is)
done, and then we hope the compiler knows that x-x == 0 (and it
typically does). As you say, that's slightly sub-optimal for archs where
the compiler cannot see into the atomic (typically LL/SC archs).

But add/sub were _2_ lines extra after I did all the groundwork for
fetch_{or,and,xor}. So we might as well save those few extra add/dec
cycles. Some of them are in fairly hot paths.

Lastly; and the weakest argument; fetch_$op is what C11 has, probably
because the above reasons.

Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[Davidlohr Bueso]

On Fri, 24 Jun 2016, KY Srinivasan wrote:

>How so? Can you expand on the racy part. The subtraction is done on a local copy of
>the value.

Yeah, you're right. I took a look at the generated code and I was unnecessarily worried
about the window between the return value and the subtraction.

Thanks,
Davidlohr

Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

[Davidlohr Bueso]

On Fri, 24 Jun 2016, James Bottomley wrote:

>On Fri, 2016-06-24 at 10:30 -0700, Davidlohr Bueso wrote:
>> On Fri, 24 Jun 2016, James Bottomley wrote:
>>
>> > On Mon, 2016-06-20 at 13:05 -0700, Davidlohr Bueso wrote:
>> > > Hi,
>> > >
>> > > The series is really straightforward and based on Peter's work
>> > > that introduces[1] the atomic_fetch_$op machinery. Only patch 1
>> > > implements the actual atomic_fetch_{inc,dec} calls based on
>> > > atomic_fetch_{add,sub}.
>> >
>> > Could I just ask why?  atomic_inc_return(x) - 1 seems a reasonable
>> > thing to do to me.
>>
>> For one restoring the old state like that can be racy and looses the
>> notion of atomicity.
>
>I don't understand this argument: any return of an atomic value is
>inherently racy because the atomic source may have changed by the time
>you use the returned value.  It's no more or less racy to my mind to
>return the original value and increment than to return the incremented
>value and subtract one.

I was looking at the n + xadd() vs xadd(). But yeah, same applies to
checking the return value of any cas operation.

>>  The new family of atomic_fetch_$ops also better express the purpose
>> of the call imo.
>
>So this is probably the core of my objection: adding APIs simply
>because we can.  A good reason to add things like this is because it's
>a common pattern people get wrong, because we can optimize it nicely on
>an architecture, or some other good reason.  Absent a good reason it
>doesn't seem like a good API addition because trying to keep up with
>all the API variants when you want to use atomics adds to the burden of
>the programmer.

That all makes sense, but again the 'race' was my main concern. Although
saving the add cycles in the rwsem (loop) path is always welcome. The rest,
and including this patchset, is merely for completeness. 

Thanks,
Davidlohr

[PATCH -v4 01/12] locking/atomic: Introduce inc/dec calls for FETCH-OP flavors

[Davidlohr Bueso]

With the inclusion of atomic FETCH-OP variants, many places in the
kernel can make use of atomic_fetch_$op() to avoid the callers that
need to compute the value/state _before_ the operation. Peter laid
out the machinery but we are still missing the simpler dec,inc calls
(which future patches will make use of).

This patch only deals with the generic code, as at least right now
no arch actually implement them -- which is similar to what the
OP-RETURN primitives currently do.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---

 include/asm-generic/atomic-long.h |  22 +++++++
 include/linux/atomic.h            | 128 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+)

diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
index 2d0d3cf791ab..288cc9e96395 100644
--- a/include/asm-generic/atomic-long.h
+++ b/include/asm-generic/atomic-long.h
@@ -146,6 +146,28 @@ ATOMIC_LONG_FETCH_OP(xor, _relaxed)
 ATOMIC_LONG_FETCH_OP(xor, _acquire)
 ATOMIC_LONG_FETCH_OP(xor, _release)
 
+#undef ATOMIC_LONG_FETCH_OP
+
+#define ATOMIC_LONG_FETCH_INC_DEC_OP(op, mo)					\
+static inline long							\
+atomic_long_fetch_##op##mo(atomic_long_t *l)				\
+{									\
+	ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l;		\
+									\
+	return (long)ATOMIC_LONG_PFX(_fetch_##op##mo)(v);		\
+}
+
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc,)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _relaxed)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _acquire)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _release)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec,)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _relaxed)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _acquire)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _release)
+
+#undef ATOMIC_LONG_FETCH_INC_DEC_OP
+
 #define ATOMIC_LONG_OP(op)						\
 static __always_inline void						\
 atomic_long_##op(long i, atomic_long_t *l)				\
diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 12d910d61b83..e71835bf60a9 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -188,6 +188,38 @@
 #endif
 #endif /* atomic_fetch_add_relaxed */
 
+/* atomic_fetch_inc_relaxed */
+#ifndef atomic_fetch_inc_relaxed
+
+#ifndef atomic_fetch_inc
+#define atomic_fetch_inc(v)	        atomic_fetch_add(1, (v))
+#define atomic_fetch_inc_relaxed(v)	atomic_fetch_add_relaxed(1, (v))
+#define atomic_fetch_inc_acquire(v)	atomic_fetch_add_acquire(1, (v))
+#define atomic_fetch_inc_release(v)	atomic_fetch_add_release(1, (v))
+#else /* atomic_fetch_inc */
+#define atomic_fetch_inc_relaxed	atomic_fetch_inc
+#define atomic_fetch_inc_acquire	atomic_fetch_inc
+#define atomic_fetch_inc_release	atomic_fetch_inc
+#endif /* atomic_fetch_inc */
+
+#else /* atomic_fetch_inc_relaxed */
+
+#ifndef atomic_fetch_inc_acquire
+#define atomic_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_inc_release
+#define atomic_fetch_inc_release(...)					\
+	__atomic_op_release(atomic_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_inc
+#define atomic_fetch_inc(...)						\
+	__atomic_op_fence(atomic_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_inc_relaxed */
+
 /* atomic_fetch_sub_relaxed */
 #ifndef atomic_fetch_sub_relaxed
 #define atomic_fetch_sub_relaxed	atomic_fetch_sub
@@ -212,6 +244,38 @@
 #endif
 #endif /* atomic_fetch_sub_relaxed */
 
+/* atomic_fetch_dec_relaxed */
+#ifndef atomic_fetch_dec_relaxed
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(v)	        atomic_fetch_sub(1, (v))
+#define atomic_fetch_dec_relaxed(v)	atomic_fetch_sub_relaxed(1, (v))
+#define atomic_fetch_dec_acquire(v)	atomic_fetch_sub_acquire(1, (v))
+#define atomic_fetch_dec_release(v)	atomic_fetch_sub_release(1, (v))
+#else /* atomic_fetch_dec */
+#define atomic_fetch_dec_relaxed	atomic_fetch_dec
+#define atomic_fetch_dec_acquire	atomic_fetch_dec
+#define atomic_fetch_dec_release	atomic_fetch_dec
+#endif /* atomic_fetch_dec */
+
+#else /* atomic_fetch_dec_relaxed */
+
+#ifndef atomic_fetch_dec_acquire
+#define atomic_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_dec_release
+#define atomic_fetch_dec_release(...)					\
+	__atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(...)						\
+	__atomic_op_fence(atomic_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_dec_relaxed */
+
 /* atomic_fetch_or_relaxed */
 #ifndef atomic_fetch_or_relaxed
 #define atomic_fetch_or_relaxed	atomic_fetch_or
@@ -697,6 +761,38 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_add_relaxed */
 
+/* atomic64_fetch_inc_relaxed */
+#ifndef atomic64_fetch_inc_relaxed
+
+#ifndef atomic64_fetch_inc
+#define atomic64_fetch_inc(v)		atomic64_fetch_add(1, (v))
+#define atomic64_fetch_inc_relaxed(v)	atomic64_fetch_add_relaxed(1, (v))
+#define atomic64_fetch_inc_acquire(v)	atomic64_fetch_add_acquire(1, (v))
+#define atomic64_fetch_inc_release(v)	atomic64_fetch_add_release(1, (v))
+#else /* atomic64_fetch_inc */
+#define atomic64_fetch_inc_relaxed	atomic64_fetch_inc
+#define atomic64_fetch_inc_acquire	atomic64_fetch_inc
+#define atomic64_fetch_inc_release	atomic64_fetch_inc
+#endif /* atomic64_fetch_inc */
+
+#else /* atomic64_fetch_inc_relaxed */
+
+#ifndef atomic64_fetch_inc_acquire
+#define atomic64_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_inc_release
+#define atomic64_fetch_inc_release(...)					\
+	__atomic_op_release(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_inc
+#define atomic64_fetch_inc(...)						\
+	__atomic_op_fence(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_inc_relaxed */
+
 /* atomic64_fetch_sub_relaxed */
 #ifndef atomic64_fetch_sub_relaxed
 #define atomic64_fetch_sub_relaxed	atomic64_fetch_sub
@@ -721,6 +817,38 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_sub_relaxed */
 
+/* atomic64_fetch_dec_relaxed */
+#ifndef atomic64_fetch_dec_relaxed
+
+#ifndef atomic64_fetch_dec
+#define atomic64_fetch_dec(v)		atomic64_fetch_sub(1, (v))
+#define atomic64_fetch_dec_relaxed(v)	atomic64_fetch_sub_relaxed(1, (v))
+#define atomic64_fetch_dec_acquire(v)	atomic64_fetch_sub_acquire(1, (v))
+#define atomic64_fetch_dec_release(v)	atomic64_fetch_sub_release(1, (v))
+#else /* atomic64_fetch_dec */
+#define atomic64_fetch_dec_relaxed	atomic64_fetch_dec
+#define atomic64_fetch_dec_acquire	atomic64_fetch_dec
+#define atomic64_fetch_dec_release	atomic64_fetch_dec
+#endif /* atomic64_fetch_dec */
+
+#else /* atomic64_fetch_dec_relaxed */
+
+#ifndef atomic64_fetch_dec_acquire
+#define atomic64_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_dec_release
+#define atomic64_fetch_dec_release(...)					\
+	__atomic_op_release(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_dec
+#define atomic64_fetch_dec(...)						\
+	__atomic_op_fence(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_dec_relaxed */
+
 /* atomic64_fetch_or_relaxed */
 #ifndef atomic64_fetch_or_relaxed
 #define atomic64_fetch_or_relaxed	atomic64_fetch_or
-- 
2.6.6

Re: [PATCH 03/12] PM,devfreq: Employ atomic_fetch_inc()

[Chanwoo Choi]

Hi Davidlohr,


Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Thanks,
Chanwoo Choi


2016-06-21 5:05 GMT+09:00 Davidlohr Bueso <dave@stgolabs.net>:
> Now that we have fetch_inc() we can stop using inc_return() - 1.
>
> These are very similar to the existing OP-RETURN primitives we already
> have, except they return the value of the atomic variable _before_
> modification.
>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> ---
>  drivers/devfreq/devfreq-event.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
> index 39b048eda2ce..45b02d3d7e7d 100644
> --- a/drivers/devfreq/devfreq-event.c
> +++ b/drivers/devfreq/devfreq-event.c
> @@ -329,7 +329,7 @@ struct devfreq_event_dev *devfreq_event_add_edev(struct device *dev,
>         edev->dev.class = devfreq_event_class;
>         edev->dev.release = devfreq_event_release_edev;
>
> -       dev_set_name(&edev->dev, "event.%d", atomic_inc_return(&event_no) - 1);
> +       dev_set_name(&edev->dev, "event.%d", atomic_fetch_inc(&event_no));
>         ret = device_register(&edev->dev);
>         if (ret < 0) {
>                 put_device(&edev->dev);
> --
> 2.6.6
>

[tip:locking/core] locking/atomic: Introduce inc/dec variants for the atomic_fetch_$op() API

[tip-bot for Davidlohr Bueso]

Commit-ID:  f06628638cf6e75f179742b6c1b35076965b9fdd
Gitweb:     http://git.kernel.org/tip/f06628638cf6e75f179742b6c1b35076965b9fdd
Author:     Davidlohr Bueso <dave@stgolabs.net>
AuthorDate: Tue, 28 Jun 2016 14:56:51 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 7 Jul 2016 09:16:20 +0200

locking/atomic: Introduce inc/dec variants for the atomic_fetch_$op() API

With the inclusion of atomic FETCH-OP variants, many places in the
kernel can make use of atomic_fetch_$op() to avoid the callers that
need to compute the value/state _before_ the operation.

Peter Zijlstra laid out the machinery but we are still missing the
simpler dec,inc() calls (which future patches will make use of).

This patch only deals with the generic code, as at least right now
no arch actually implement them -- which is similar to what the
OP-RETURN primitives currently do.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: James.Bottomley@HansenPartnership.com
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: awalls@md.metrocast.net
Cc: bp@alien8.de
Cc: cw00.choi@samsung.com
Cc: davem@davemloft.net
Cc: dledford@redhat.com
Cc: dougthompson@xmission.com
Cc: gregkh@linuxfoundation.org
Cc: hans.verkuil@cisco.com
Cc: heiko.carstens@de.ibm.com
Cc: jikos@kernel.org
Cc: kys@microsoft.com
Cc: mchehab@osg.samsung.com
Cc: pfg@sgi.com
Cc: schwidefsky@de.ibm.com
Cc: sean.hefty@intel.com
Cc: sumit.semwal@linaro.org
Link: http://lkml.kernel.org/r/20160628215651.GA20048@linux-80c1.suse
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/asm-generic/atomic-long.h |  22 +++++++
 include/linux/atomic.h            | 128 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+)

diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
index 2d0d3cf..288cc9e 100644
--- a/include/asm-generic/atomic-long.h
+++ b/include/asm-generic/atomic-long.h
@@ -146,6 +146,28 @@ ATOMIC_LONG_FETCH_OP(xor, _relaxed)
 ATOMIC_LONG_FETCH_OP(xor, _acquire)
 ATOMIC_LONG_FETCH_OP(xor, _release)
 
+#undef ATOMIC_LONG_FETCH_OP
+
+#define ATOMIC_LONG_FETCH_INC_DEC_OP(op, mo)					\
+static inline long							\
+atomic_long_fetch_##op##mo(atomic_long_t *l)				\
+{									\
+	ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l;		\
+									\
+	return (long)ATOMIC_LONG_PFX(_fetch_##op##mo)(v);		\
+}
+
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc,)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _relaxed)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _acquire)
+ATOMIC_LONG_FETCH_INC_DEC_OP(inc, _release)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec,)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _relaxed)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _acquire)
+ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _release)
+
+#undef ATOMIC_LONG_FETCH_INC_DEC_OP
+
 #define ATOMIC_LONG_OP(op)						\
 static __always_inline void						\
 atomic_long_##op(long i, atomic_long_t *l)				\
diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 12d910d..e71835b 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -188,6 +188,38 @@
 #endif
 #endif /* atomic_fetch_add_relaxed */
 
+/* atomic_fetch_inc_relaxed */
+#ifndef atomic_fetch_inc_relaxed
+
+#ifndef atomic_fetch_inc
+#define atomic_fetch_inc(v)	        atomic_fetch_add(1, (v))
+#define atomic_fetch_inc_relaxed(v)	atomic_fetch_add_relaxed(1, (v))
+#define atomic_fetch_inc_acquire(v)	atomic_fetch_add_acquire(1, (v))
+#define atomic_fetch_inc_release(v)	atomic_fetch_add_release(1, (v))
+#else /* atomic_fetch_inc */
+#define atomic_fetch_inc_relaxed	atomic_fetch_inc
+#define atomic_fetch_inc_acquire	atomic_fetch_inc
+#define atomic_fetch_inc_release	atomic_fetch_inc
+#endif /* atomic_fetch_inc */
+
+#else /* atomic_fetch_inc_relaxed */
+
+#ifndef atomic_fetch_inc_acquire
+#define atomic_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_inc_release
+#define atomic_fetch_inc_release(...)					\
+	__atomic_op_release(atomic_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_inc
+#define atomic_fetch_inc(...)						\
+	__atomic_op_fence(atomic_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_inc_relaxed */
+
 /* atomic_fetch_sub_relaxed */
 #ifndef atomic_fetch_sub_relaxed
 #define atomic_fetch_sub_relaxed	atomic_fetch_sub
@@ -212,6 +244,38 @@
 #endif
 #endif /* atomic_fetch_sub_relaxed */
 
+/* atomic_fetch_dec_relaxed */
+#ifndef atomic_fetch_dec_relaxed
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(v)	        atomic_fetch_sub(1, (v))
+#define atomic_fetch_dec_relaxed(v)	atomic_fetch_sub_relaxed(1, (v))
+#define atomic_fetch_dec_acquire(v)	atomic_fetch_sub_acquire(1, (v))
+#define atomic_fetch_dec_release(v)	atomic_fetch_sub_release(1, (v))
+#else /* atomic_fetch_dec */
+#define atomic_fetch_dec_relaxed	atomic_fetch_dec
+#define atomic_fetch_dec_acquire	atomic_fetch_dec
+#define atomic_fetch_dec_release	atomic_fetch_dec
+#endif /* atomic_fetch_dec */
+
+#else /* atomic_fetch_dec_relaxed */
+
+#ifndef atomic_fetch_dec_acquire
+#define atomic_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_dec_release
+#define atomic_fetch_dec_release(...)					\
+	__atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic_fetch_dec
+#define atomic_fetch_dec(...)						\
+	__atomic_op_fence(atomic_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic_fetch_dec_relaxed */
+
 /* atomic_fetch_or_relaxed */
 #ifndef atomic_fetch_or_relaxed
 #define atomic_fetch_or_relaxed	atomic_fetch_or
@@ -697,6 +761,38 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_add_relaxed */
 
+/* atomic64_fetch_inc_relaxed */
+#ifndef atomic64_fetch_inc_relaxed
+
+#ifndef atomic64_fetch_inc
+#define atomic64_fetch_inc(v)		atomic64_fetch_add(1, (v))
+#define atomic64_fetch_inc_relaxed(v)	atomic64_fetch_add_relaxed(1, (v))
+#define atomic64_fetch_inc_acquire(v)	atomic64_fetch_add_acquire(1, (v))
+#define atomic64_fetch_inc_release(v)	atomic64_fetch_add_release(1, (v))
+#else /* atomic64_fetch_inc */
+#define atomic64_fetch_inc_relaxed	atomic64_fetch_inc
+#define atomic64_fetch_inc_acquire	atomic64_fetch_inc
+#define atomic64_fetch_inc_release	atomic64_fetch_inc
+#endif /* atomic64_fetch_inc */
+
+#else /* atomic64_fetch_inc_relaxed */
+
+#ifndef atomic64_fetch_inc_acquire
+#define atomic64_fetch_inc_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_inc_release
+#define atomic64_fetch_inc_release(...)					\
+	__atomic_op_release(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_inc
+#define atomic64_fetch_inc(...)						\
+	__atomic_op_fence(atomic64_fetch_inc, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_inc_relaxed */
+
 /* atomic64_fetch_sub_relaxed */
 #ifndef atomic64_fetch_sub_relaxed
 #define atomic64_fetch_sub_relaxed	atomic64_fetch_sub
@@ -721,6 +817,38 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 #endif
 #endif /* atomic64_fetch_sub_relaxed */
 
+/* atomic64_fetch_dec_relaxed */
+#ifndef atomic64_fetch_dec_relaxed
+
+#ifndef atomic64_fetch_dec
+#define atomic64_fetch_dec(v)		atomic64_fetch_sub(1, (v))
+#define atomic64_fetch_dec_relaxed(v)	atomic64_fetch_sub_relaxed(1, (v))
+#define atomic64_fetch_dec_acquire(v)	atomic64_fetch_sub_acquire(1, (v))
+#define atomic64_fetch_dec_release(v)	atomic64_fetch_sub_release(1, (v))
+#else /* atomic64_fetch_dec */
+#define atomic64_fetch_dec_relaxed	atomic64_fetch_dec
+#define atomic64_fetch_dec_acquire	atomic64_fetch_dec
+#define atomic64_fetch_dec_release	atomic64_fetch_dec
+#endif /* atomic64_fetch_dec */
+
+#else /* atomic64_fetch_dec_relaxed */
+
+#ifndef atomic64_fetch_dec_acquire
+#define atomic64_fetch_dec_acquire(...)					\
+	__atomic_op_acquire(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_dec_release
+#define atomic64_fetch_dec_release(...)					\
+	__atomic_op_release(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+
+#ifndef atomic64_fetch_dec
+#define atomic64_fetch_dec(...)						\
+	__atomic_op_fence(atomic64_fetch_dec, __VA_ARGS__)
+#endif
+#endif /* atomic64_fetch_dec_relaxed */
+
 /* atomic64_fetch_or_relaxed */
 #ifndef atomic64_fetch_or_relaxed
 #define atomic64_fetch_or_relaxed	atomic64_fetch_or

Re: [PATCH 07/12] drivers/media: Employ atomic_fetch_inc()

[Mauro Carvalho Chehab]

Em Mon, 20 Jun 2016 13:05:59 -0700
Davidlohr Bueso <dave@stgolabs.net> escreveu:

> Now that we have fetch_inc() we can stop using inc_return() - 1.
> 
> These are very similar to the existing OP-RETURN primitives we already
> have, except they return the value of the atomic variable _before_
> modification.
> 
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Andy Walls <awalls@md.metrocast.net>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>

I suspect that you'll be applying this together with the other patches
in this series. So:

Acked-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

> ---
>  drivers/media/pci/cobalt/cobalt-driver.c | 2 +-
>  drivers/media/pci/cx18/cx18-driver.c     | 2 +-
>  drivers/media/v4l2-core/v4l2-device.c    | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/pci/cobalt/cobalt-driver.c b/drivers/media/pci/cobalt/cobalt-driver.c
> index 8d6f04fc8013..70dfcb0c6a72 100644
> --- a/drivers/media/pci/cobalt/cobalt-driver.c
> +++ b/drivers/media/pci/cobalt/cobalt-driver.c
> @@ -683,7 +683,7 @@ static int cobalt_probe(struct pci_dev *pci_dev,
>  	int i;
>  
>  	/* FIXME - module parameter arrays constrain max instances */
> -	i = atomic_inc_return(&cobalt_instance) - 1;
> +	i = atomic_fetch_inc(&cobalt_instance);
>  
>  	cobalt = kzalloc(sizeof(struct cobalt), GFP_ATOMIC);
>  	if (cobalt == NULL)
> diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
> index 260e462d91b4..5cb3408c6859 100644
> --- a/drivers/media/pci/cx18/cx18-driver.c
> +++ b/drivers/media/pci/cx18/cx18-driver.c
> @@ -908,7 +908,7 @@ static int cx18_probe(struct pci_dev *pci_dev,
>  	struct cx18 *cx;
>  
>  	/* FIXME - module parameter arrays constrain max instances */
> -	i = atomic_inc_return(&cx18_instance) - 1;
> +	i = atomic_fetch_inc(&cx18_instance);
>  	if (i >= CX18_MAX_CARDS) {
>  		printk(KERN_ERR "cx18: cannot manage card %d, driver has a "
>  		       "limit of 0 - %d\n", i, CX18_MAX_CARDS - 1);
> diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
> index 06fa5f1b2cff..1bc5b68c2724 100644
> --- a/drivers/media/v4l2-core/v4l2-device.c
> +++ b/drivers/media/v4l2-core/v4l2-device.c
> @@ -76,7 +76,7 @@ EXPORT_SYMBOL_GPL(v4l2_device_put);
>  int v4l2_device_set_name(struct v4l2_device *v4l2_dev, const char *basename,
>  						atomic_t *instance)
>  {
> -	int num = atomic_inc_return(instance) - 1;
> +	int num = atomic_fetch_inc(instance);
>  	int len = strlen(basename);
>  
>  	if (basename[len - 1] >= '0' && basename[len - 1] <= '9')



Thanks,
Mauro