[PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[Ahmed S. Darwish]

Hi,

Since patch #7 and #8 from the series:

   [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
   https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de

are now pending on the lockdep/x86 IRQ state tracking patch series:

   [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
   https://lkml.kernel.org/r/20200529212728.795169701@infradead.org

   [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
   https://lkml.kernel.org/r/20200529213550.683440625@infradead.org

This is a repost only of the seqcount_t call sites bugfixes that were on
top of the seqlock patch series.

These fixes are independent, and can thus be merged on their own. I'm
reposting them now so they can at least hit -rc2 or -rc3.

Changelog-v2:

  - patch #1: Add a missing up_read() on netdev_get_name() error path
              exit. Thanks to Dan/kbuild-bot report.

  - patch #4: new patch, invalid preemptible context found by the new
              lockdep checks added in the seqlock series + kbuild-bot.

Thanks,

8<--------------

Ahmed S. Darwish (6):
  net: core: device_rename: Use rwsem instead of a seqcount
  net: phy: fixed_phy: Remove unused seqcount
  u64_stats: Document writer non-preemptibility requirement
  net: mdiobus: Disable preemption upon u64_stats update
  block: nr_sects_write(): Disable preemption on seqcount write
  dma-buf: Remove custom seqcount lockdep class key

 block/blk.h                    |  2 ++
 drivers/dma-buf/dma-resv.c     |  9 +------
 drivers/net/phy/fixed_phy.c    | 26 ++++++++------------
 drivers/net/phy/mdio_bus.c     |  2 ++
 include/linux/dma-resv.h       |  2 --
 include/linux/u64_stats_sync.h | 43 ++++++++++++++++++----------------
 net/core/dev.c                 | 40 ++++++++++++++-----------------
 7 files changed, 56 insertions(+), 68 deletions(-)

base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
--
2.20.1

[PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[Ahmed S. Darwish]

Hi,

Since patch #7 and #8 from the series:

   [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
   https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de

are now pending on the lockdep/x86 IRQ state tracking patch series:

   [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
   https://lkml.kernel.org/r/20200529212728.795169701@infradead.org

   [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
   https://lkml.kernel.org/r/20200529213550.683440625@infradead.org

This is a repost only of the seqcount_t call sites bugfixes that were on
top of the seqlock patch series.

These fixes are independent, and can thus be merged on their own. I'm
reposting them now so they can at least hit -rc2 or -rc3.

Changelog-v2:

  - patch #1: Add a missing up_read() on netdev_get_name() error path
              exit. Thanks to Dan/kbuild-bot report.

  - patch #4: new patch, invalid preemptible context found by the new
              lockdep checks added in the seqlock series + kbuild-bot.

Thanks,

8<--------------

Ahmed S. Darwish (6):
  net: core: device_rename: Use rwsem instead of a seqcount
  net: phy: fixed_phy: Remove unused seqcount
  u64_stats: Document writer non-preemptibility requirement
  net: mdiobus: Disable preemption upon u64_stats update
  block: nr_sects_write(): Disable preemption on seqcount write
  dma-buf: Remove custom seqcount lockdep class key

 block/blk.h                    |  2 ++
 drivers/dma-buf/dma-resv.c     |  9 +------
 drivers/net/phy/fixed_phy.c    | 26 ++++++++------------
 drivers/net/phy/mdio_bus.c     |  2 ++
 include/linux/dma-resv.h       |  2 --
 include/linux/u64_stats_sync.h | 43 ++++++++++++++++++----------------
 net/core/dev.c                 | 40 ++++++++++++++-----------------
 7 files changed, 56 insertions(+), 68 deletions(-)

base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/6] net: core: device_rename: Use rwsem instead of a seqcount

[Ahmed S. Darwish]

Sequence counters write paths are critical sections that must never be
preempted, and blocking, even for CONFIG_PREEMPTION=n, is not allowed.

Commit 5dbe7c178d3f ("net: fix kernel deadlock with interface rename and
netdev name retrieval.") handled a deadlock, observed with
CONFIG_PREEMPTION=n, where the devnet_rename seqcount read side was
infinitely spinning: it got scheduled after the seqcount write side
blocked inside its own critical section.

To fix that deadlock, among other issues, the commit added a
cond_resched() inside the read side section. While this will get the
non-preemptible kernel eventually unstuck, the seqcount reader is fully
exhausting its slice just spinning -- until TIF_NEED_RESCHED is set.

The fix is also still broken: if the seqcount reader belongs to a
real-time scheduling policy, it can spin forever and the kernel will
livelock.

Disabling preemption over the seqcount write side critical section will
not work: inside it are a number of GFP_KERNEL allocations and mutex
locking through the drivers/base/ :: device_rename() call chain.

From all the above, replace the seqcount with a rwsem.

Fixes: 5dbe7c178d3f (net: fix kernel deadlock with interface rename and netdev name retrieval.)
Fixes: 30e6c9fa93cf (net: devnet_rename_seq should be a seqcount)
Fixes: c91f6df2db49 (sockopt: Change getsockopt() of SO_BINDTODEVICE to return an interface name)
Cc: <stable@vger.kernel.org>
Reported-by: kbuild test robot <lkp@intel.com> [ v1 missing up_read() on error exit ]
Reported-by: Dan Carpenter <dan.carpenter@oracle.com> [ v1 missing up_read() on error exit ]
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 net/core/dev.c | 40 ++++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 2d8aceee4284..93a279ab4e97 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -79,6 +79,7 @@
 #include <linux/sched.h>
 #include <linux/sched/mm.h>
 #include <linux/mutex.h>
+#include <linux/rwsem.h>
 #include <linux/string.h>
 #include <linux/mm.h>
 #include <linux/socket.h>
@@ -194,7 +195,7 @@ static DEFINE_SPINLOCK(napi_hash_lock);
 static unsigned int napi_gen_id = NR_CPUS;
 static DEFINE_READ_MOSTLY_HASHTABLE(napi_hash, 8);
 
-static seqcount_t devnet_rename_seq;
+static DECLARE_RWSEM(devnet_rename_sem);
 
 static inline void dev_base_seq_inc(struct net *net)
 {
@@ -930,33 +931,28 @@ EXPORT_SYMBOL(dev_get_by_napi_id);
  *	@net: network namespace
  *	@name: a pointer to the buffer where the name will be stored.
  *	@ifindex: the ifindex of the interface to get the name from.
- *
- *	The use of raw_seqcount_begin() and cond_resched() before
- *	retrying is required as we want to give the writers a chance
- *	to complete when CONFIG_PREEMPTION is not set.
  */
 int netdev_get_name(struct net *net, char *name, int ifindex)
 {
 	struct net_device *dev;
-	unsigned int seq;
+	int ret;
 
-retry:
-	seq = raw_seqcount_begin(&devnet_rename_seq);
+	down_read(&devnet_rename_sem);
 	rcu_read_lock();
+
 	dev = dev_get_by_index_rcu(net, ifindex);
 	if (!dev) {
-		rcu_read_unlock();
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out;
 	}
 
 	strcpy(name, dev->name);
-	rcu_read_unlock();
-	if (read_seqcount_retry(&devnet_rename_seq, seq)) {
-		cond_resched();
-		goto retry;
-	}
 
-	return 0;
+	ret = 0;
+out:
+	rcu_read_unlock();
+	up_read(&devnet_rename_sem);
+	return ret;
 }
 
 /**
@@ -1228,10 +1224,10 @@ int dev_change_name(struct net_device *dev, const char *newname)
 	    likely(!(dev->priv_flags & IFF_LIVE_RENAME_OK)))
 		return -EBUSY;
 
-	write_seqcount_begin(&devnet_rename_seq);
+	down_write(&devnet_rename_sem);
 
 	if (strncmp(newname, dev->name, IFNAMSIZ) == 0) {
-		write_seqcount_end(&devnet_rename_seq);
+		up_write(&devnet_rename_sem);
 		return 0;
 	}
 
@@ -1239,7 +1235,7 @@ int dev_change_name(struct net_device *dev, const char *newname)
 
 	err = dev_get_valid_name(net, dev, newname);
 	if (err < 0) {
-		write_seqcount_end(&devnet_rename_seq);
+		up_write(&devnet_rename_sem);
 		return err;
 	}
 
@@ -1254,11 +1250,11 @@ int dev_change_name(struct net_device *dev, const char *newname)
 	if (ret) {
 		memcpy(dev->name, oldname, IFNAMSIZ);
 		dev->name_assign_type = old_assign_type;
-		write_seqcount_end(&devnet_rename_seq);
+		up_write(&devnet_rename_sem);
 		return ret;
 	}
 
-	write_seqcount_end(&devnet_rename_seq);
+	up_write(&devnet_rename_sem);
 
 	netdev_adjacent_rename_links(dev, oldname);
 
@@ -1279,7 +1275,7 @@ int dev_change_name(struct net_device *dev, const char *newname)
 		/* err >= 0 after dev_alloc_name() or stores the first errno */
 		if (err >= 0) {
 			err = ret;
-			write_seqcount_begin(&devnet_rename_seq);
+			down_write(&devnet_rename_sem);
 			memcpy(dev->name, oldname, IFNAMSIZ);
 			memcpy(oldname, newname, IFNAMSIZ);
 			dev->name_assign_type = old_assign_type;
-- 
2.20.1

[PATCH v2 2/6] net: phy: fixed_phy: Remove unused seqcount

[Ahmed S. Darwish]

Commit bf7afb29d545 ("phy: improve safety of fixed-phy MII register
reading") protected the fixed PHY status with a sequence counter.

Two years later, commit d2b977939b18 ("net: phy: fixed-phy: remove
fixed_phy_update_state()") removed the sequence counter's write side
critical section -- neutralizing its read side retry loop.

Remove the unused seqcount.

Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/phy/fixed_phy.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 4a3d34f40cb9..c4641b1704d6 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -19,7 +19,6 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/gpio/consumer.h>
-#include <linux/seqlock.h>
 #include <linux/idr.h>
 #include <linux/netdevice.h>
 #include <linux/linkmode.h>
@@ -34,7 +33,6 @@ struct fixed_mdio_bus {
 struct fixed_phy {
 	int addr;
 	struct phy_device *phydev;
-	seqcount_t seqcount;
 	struct fixed_phy_status status;
 	bool no_carrier;
 	int (*link_update)(struct net_device *, struct fixed_phy_status *);
@@ -80,19 +78,17 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
 	list_for_each_entry(fp, &fmb->phys, node) {
 		if (fp->addr == phy_addr) {
 			struct fixed_phy_status state;
-			int s;
 
-			do {
-				s = read_seqcount_begin(&fp->seqcount);
-				fp->status.link = !fp->no_carrier;
-				/* Issue callback if user registered it. */
-				if (fp->link_update)
-					fp->link_update(fp->phydev->attached_dev,
-							&fp->status);
-				/* Check the GPIO for change in status */
-				fixed_phy_update(fp);
-				state = fp->status;
-			} while (read_seqcount_retry(&fp->seqcount, s));
+			fp->status.link = !fp->no_carrier;
+
+			/* Issue callback if user registered it. */
+			if (fp->link_update)
+				fp->link_update(fp->phydev->attached_dev,
+						&fp->status);
+
+			/* Check the GPIO for change in status */
+			fixed_phy_update(fp);
+			state = fp->status;
 
 			return swphy_read_reg(reg_num, &state);
 		}
@@ -150,8 +146,6 @@ static int fixed_phy_add_gpiod(unsigned int irq, int phy_addr,
 	if (!fp)
 		return -ENOMEM;
 
-	seqcount_init(&fp->seqcount);
-
 	if (irq != PHY_POLL)
 		fmb->mii_bus->irq[phy_addr] = irq;
 
-- 
2.20.1

[PATCH v2 3/6] u64_stats: Document writer non-preemptibility requirement

[Ahmed S. Darwish]

The u64_stats mechanism uses sequence counters to protect against 64-bit
values tearing on 32-bit architectures. Updating such statistics is a
sequence counter write side critical section.

Preemption must be disabled before entering this seqcount write critical
section.  Failing to do so, the seqcount read side can preempt the write
side section and spin for the entire scheduler tick.  If that reader
belongs to a real-time scheduling class, it can spin forever and the
kernel will livelock.

Document this statistics update side non-preemptibility requirement.

Reword the introductory paragraph to highlight u64_stats raison d'être:
64-bit values tearing protection on 32-bit architectures. Divide
documentation on a basis of internal design vs. usage constraints.

Reword the u64_stats header file top comment to always mention "Reader"
or "Writer" at the start of each bullet point, making it easier to
follow which side each point is actually for.

Clarify the statement "whole thing is a NOOP on 64bit arches or UP
kernels".  For 32-bit UP kernels, preemption is always disabled for the
statistics read side section.

Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/u64_stats_sync.h | 43 ++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/include/linux/u64_stats_sync.h b/include/linux/u64_stats_sync.h
index 9de5c10293f5..c6abb79501b3 100644
--- a/include/linux/u64_stats_sync.h
+++ b/include/linux/u64_stats_sync.h
@@ -3,33 +3,36 @@
 #define _LINUX_U64_STATS_SYNC_H
 
 /*
- * To properly implement 64bits network statistics on 32bit and 64bit hosts,
- * we provide a synchronization point, that is a noop on 64bit or UP kernels.
+ * Protect against 64-bit values tearing on 32-bit architectures. This is
+ * typically used for statistics read/update in different subsystems.
  *
  * Key points :
- * 1) Use a seqcount on SMP 32bits, with low overhead.
- * 2) Whole thing is a noop on 64bit arches or UP kernels.
- * 3) Write side must ensure mutual exclusion or one seqcount update could
+ *
+ * -  Use a seqcount on 32-bit SMP, only disable preemption for 32-bit UP.
+ * -  The whole thing is a no-op on 64-bit architectures.
+ *
+ * Usage constraints:
+ *
+ * 1) Write side must ensure mutual exclusion, or one seqcount update could
  *    be lost, thus blocking readers forever.
- *    If this synchronization point is not a mutex, but a spinlock or
- *    spinlock_bh() or disable_bh() :
- * 3.1) Write side should not sleep.
- * 3.2) Write side should not allow preemption.
- * 3.3) If applicable, interrupts should be disabled.
+ *
+ * 2) Write side must disable preemption, or a seqcount reader can preempt the
+ *    writer and also spin forever.
+ *
+ * 3) Write side must use the _irqsave() variant if other writers, or a reader,
+ *    can be invoked from an IRQ context.
  *
  * 4) If reader fetches several counters, there is no guarantee the whole values
- *    are consistent (remember point 1) : this is a noop on 64bit arches anyway)
+ *    are consistent w.r.t. each other (remember point #2: seqcounts are not
+ *    used for 64bit architectures).
  *
- * 5) readers are allowed to sleep or be preempted/interrupted : They perform
- *    pure reads. But if they have to fetch many values, it's better to not allow
- *    preemptions/interruptions to avoid many retries.
+ * 5) Readers are allowed to sleep or be preempted/interrupted: they perform
+ *    pure reads.
  *
- * 6) If counter might be written by an interrupt, readers should block interrupts.
- *    (On UP, there is no seqcount_t protection, a reader allowing interrupts could
- *     read partial values)
- *
- * 7) For irq and softirq uses, readers can use u64_stats_fetch_begin_irq() and
- *    u64_stats_fetch_retry_irq() helpers
+ * 6) Readers must use both u64_stats_fetch_{begin,retry}_irq() if the stats
+ *    might be updated from a hardirq or softirq context (remember point #1:
+ *    seqcounts are not used for UP kernels). 32-bit UP stat readers could read
+ *    corrupted 64-bit values otherwise.
  *
  * Usage :
  *
-- 
2.20.1

[PATCH v2 4/6] net: mdiobus: Disable preemption upon u64_stats update

[Ahmed S. Darwish]

The u64_stats mechanism uses sequence counters to protect against 64-bit
values tearing on 32-bit architectures. Updating u64_stats is thus a
sequence counter write side critical section where preemption must be
disabled.

For mdiobus_stats_acct(), disable preemption upon the u64_stats update.
It is called from process context through mdiobus_read() and
mdiobus_write().

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/phy/mdio_bus.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 7a4eb3f2cb74..a1a4dee2a033 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -757,6 +757,7 @@ EXPORT_SYMBOL(mdiobus_scan);
 
 static void mdiobus_stats_acct(struct mdio_bus_stats *stats, bool op, int ret)
 {
+	preempt_disable();
 	u64_stats_update_begin(&stats->syncp);
 
 	u64_stats_inc(&stats->transfers);
@@ -771,6 +772,7 @@ static void mdiobus_stats_acct(struct mdio_bus_stats *stats, bool op, int ret)
 		u64_stats_inc(&stats->writes);
 out:
 	u64_stats_update_end(&stats->syncp);
+	preempt_enable();
 }
 
 /**
-- 
2.20.1

[PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write

[Ahmed S. Darwish]

For optimized block readers not holding a mutex, the "number of sectors"
64-bit value is protected from tearing on 32-bit architectures by a
sequence counter.

Disable preemption before entering that sequence counter's write side
critical section. Otherwise, the read side can preempt the write side
section and spin for the entire scheduler tick. If the reader belongs to
a real-time scheduling class, it can spin forever and the kernel will
livelock.

Fixes: c83f6bf98dc1 ("block: add partition resize function to blkpg ioctl")
Cc: <stable@vger.kernel.org>
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 block/blk.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/blk.h b/block/blk.h
index 0a94ec68af32..151f86932547 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -470,9 +470,11 @@ static inline sector_t part_nr_sects_read(struct hd_struct *part)
 static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
 {
 #if BITS_PER_LONG==32 && defined(CONFIG_SMP)
+	preempt_disable();
 	write_seqcount_begin(&part->nr_sects_seq);
 	part->nr_sects = size;
 	write_seqcount_end(&part->nr_sects_seq);
+	preempt_enable();
 #elif BITS_PER_LONG==32 && defined(CONFIG_PREEMPTION)
 	preempt_disable();
 	part->nr_sects = size;
-- 
2.20.1

[PATCH v2 6/6] dma-buf: Remove custom seqcount lockdep class key

[Ahmed S. Darwish]

Commit 3c3b177a9369 ("reservation: add support for read-only access
using rcu") introduced a sequence counter to manage updates to
reservations. Back then, the reservation object initializer
reservation_object_init() was always inlined.

Having the sequence counter initialization inlined meant that each of
the call sites would have a different lockdep class key, which would've
broken lockdep's deadlock detection. The aforementioned commit thus
introduced, and exported, a custom seqcount lockdep class key and name.

The commit 8735f16803f00 ("dma-buf: cleanup reservation_object_init...")
transformed the reservation object initializer to a normal non-inlined C
function. seqcount_init(), which automatically defines the seqcount
lockdep class key and must be called non-inlined, can now be safely used.

Remove the seqcount custom lockdep class key, name, and export. Use
seqcount_init() inside the dma reservation object initializer.

Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/dma-buf/dma-resv.c | 9 +--------
 include/linux/dma-resv.h   | 2 --
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 4264e64788c4..590ce7ad60a0 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -50,12 +50,6 @@
 DEFINE_WD_CLASS(reservation_ww_class);
 EXPORT_SYMBOL(reservation_ww_class);
 
-struct lock_class_key reservation_seqcount_class;
-EXPORT_SYMBOL(reservation_seqcount_class);
-
-const char reservation_seqcount_string[] = "reservation_seqcount";
-EXPORT_SYMBOL(reservation_seqcount_string);
-
 /**
  * dma_resv_list_alloc - allocate fence list
  * @shared_max: number of fences we need space for
@@ -134,9 +128,8 @@ subsys_initcall(dma_resv_lockdep);
 void dma_resv_init(struct dma_resv *obj)
 {
 	ww_mutex_init(&obj->lock, &reservation_ww_class);
+	seqcount_init(&obj->seq);
 
-	__seqcount_init(&obj->seq, reservation_seqcount_string,
-			&reservation_seqcount_class);
 	RCU_INIT_POINTER(obj->fence, NULL);
 	RCU_INIT_POINTER(obj->fence_excl, NULL);
 }
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
index ee50d10f052b..a6538ae7d93f 100644
--- a/include/linux/dma-resv.h
+++ b/include/linux/dma-resv.h
@@ -46,8 +46,6 @@
 #include <linux/rcupdate.h>
 
 extern struct ww_class reservation_ww_class;
-extern struct lock_class_key reservation_seqcount_class;
-extern const char reservation_seqcount_string[];
 
 /**
  * struct dma_resv_list - a list of shared fences
-- 
2.20.1

[PATCH v2 6/6] dma-buf: Remove custom seqcount lockdep class key

[Ahmed S. Darwish]

Commit 3c3b177a9369 ("reservation: add support for read-only access
using rcu") introduced a sequence counter to manage updates to
reservations. Back then, the reservation object initializer
reservation_object_init() was always inlined.

Having the sequence counter initialization inlined meant that each of
the call sites would have a different lockdep class key, which would've
broken lockdep's deadlock detection. The aforementioned commit thus
introduced, and exported, a custom seqcount lockdep class key and name.

The commit 8735f16803f00 ("dma-buf: cleanup reservation_object_init...")
transformed the reservation object initializer to a normal non-inlined C
function. seqcount_init(), which automatically defines the seqcount
lockdep class key and must be called non-inlined, can now be safely used.

Remove the seqcount custom lockdep class key, name, and export. Use
seqcount_init() inside the dma reservation object initializer.

Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/dma-buf/dma-resv.c | 9 +--------
 include/linux/dma-resv.h   | 2 --
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 4264e64788c4..590ce7ad60a0 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -50,12 +50,6 @@
 DEFINE_WD_CLASS(reservation_ww_class);
 EXPORT_SYMBOL(reservation_ww_class);
 
-struct lock_class_key reservation_seqcount_class;
-EXPORT_SYMBOL(reservation_seqcount_class);
-
-const char reservation_seqcount_string[] = "reservation_seqcount";
-EXPORT_SYMBOL(reservation_seqcount_string);
-
 /**
  * dma_resv_list_alloc - allocate fence list
  * @shared_max: number of fences we need space for
@@ -134,9 +128,8 @@ subsys_initcall(dma_resv_lockdep);
 void dma_resv_init(struct dma_resv *obj)
 {
 	ww_mutex_init(&obj->lock, &reservation_ww_class);
+	seqcount_init(&obj->seq);
 
-	__seqcount_init(&obj->seq, reservation_seqcount_string,
-			&reservation_seqcount_class);
 	RCU_INIT_POINTER(obj->fence, NULL);
 	RCU_INIT_POINTER(obj->fence_excl, NULL);
 }
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
index ee50d10f052b..a6538ae7d93f 100644
--- a/include/linux/dma-resv.h
+++ b/include/linux/dma-resv.h
@@ -46,8 +46,6 @@
 #include <linux/rcupdate.h>
 
 extern struct ww_class reservation_ww_class;
-extern struct lock_class_key reservation_seqcount_class;
-extern const char reservation_seqcount_string[];
 
 /**
  * struct dma_resv_list - a list of shared fences
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/6] net: phy: fixed_phy: Remove unused seqcount

[Andrew Lunn]

On Wed, Jun 03, 2020 at 04:49:45PM +0200, Ahmed S. Darwish wrote:
> Commit bf7afb29d545 ("phy: improve safety of fixed-phy MII register
> reading") protected the fixed PHY status with a sequence counter.
> 
> Two years later, commit d2b977939b18 ("net: phy: fixed-phy: remove
> fixed_phy_update_state()") removed the sequence counter's write side
> critical section -- neutralizing its read side retry loop.
> 
> Remove the unused seqcount.
> 
> Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[Daniel Vetter]

On Wed, Jun 03, 2020 at 04:49:43PM +0200, Ahmed S. Darwish wrote:
> Hi,
> 
> Since patch #7 and #8 from the series:
> 
>    [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
>    https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
> 
> are now pending on the lockdep/x86 IRQ state tracking patch series:
> 
>    [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
>    https://lkml.kernel.org/r/20200529212728.795169701@infradead.org
> 
>    [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
>    https://lkml.kernel.org/r/20200529213550.683440625@infradead.org
> 
> This is a repost only of the seqcount_t call sites bugfixes that were on
> top of the seqlock patch series.
> 
> These fixes are independent, and can thus be merged on their own. I'm
> reposting them now so they can at least hit -rc2 or -rc3.

I'm confused on what I should do with patch 6 here for dma-buf. Looks like
just a good cleanup/prep work, so I'd queue it for linux-next and 5.9, but
sounds like you want this in earlier. Do you need this in 5.8-rc for some
work meant for 5.9? Will this go in through some topic branch directly?
Should I apply it?

Patch itself lgtm, I'm just confused what I should do with it.
-Daniel

> 
> Changelog-v2:
> 
>   - patch #1: Add a missing up_read() on netdev_get_name() error path
>               exit. Thanks to Dan/kbuild-bot report.
> 
>   - patch #4: new patch, invalid preemptible context found by the new
>               lockdep checks added in the seqlock series + kbuild-bot.
> 
> Thanks,
> 
> 8<--------------
> 
> Ahmed S. Darwish (6):
>   net: core: device_rename: Use rwsem instead of a seqcount
>   net: phy: fixed_phy: Remove unused seqcount
>   u64_stats: Document writer non-preemptibility requirement
>   net: mdiobus: Disable preemption upon u64_stats update
>   block: nr_sects_write(): Disable preemption on seqcount write
>   dma-buf: Remove custom seqcount lockdep class key
> 
>  block/blk.h                    |  2 ++
>  drivers/dma-buf/dma-resv.c     |  9 +------
>  drivers/net/phy/fixed_phy.c    | 26 ++++++++------------
>  drivers/net/phy/mdio_bus.c     |  2 ++
>  include/linux/dma-resv.h       |  2 --
>  include/linux/u64_stats_sync.h | 43 ++++++++++++++++++----------------
>  net/core/dev.c                 | 40 ++++++++++++++-----------------
>  7 files changed, 56 insertions(+), 68 deletions(-)
> 
> base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
> --
> 2.20.1

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[Daniel Vetter]

On Wed, Jun 03, 2020 at 04:49:43PM +0200, Ahmed S. Darwish wrote:
> Hi,
> 
> Since patch #7 and #8 from the series:
> 
>    [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
>    https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
> 
> are now pending on the lockdep/x86 IRQ state tracking patch series:
> 
>    [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
>    https://lkml.kernel.org/r/20200529212728.795169701@infradead.org
> 
>    [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
>    https://lkml.kernel.org/r/20200529213550.683440625@infradead.org
> 
> This is a repost only of the seqcount_t call sites bugfixes that were on
> top of the seqlock patch series.
> 
> These fixes are independent, and can thus be merged on their own. I'm
> reposting them now so they can at least hit -rc2 or -rc3.

I'm confused on what I should do with patch 6 here for dma-buf. Looks like
just a good cleanup/prep work, so I'd queue it for linux-next and 5.9, but
sounds like you want this in earlier. Do you need this in 5.8-rc for some
work meant for 5.9? Will this go in through some topic branch directly?
Should I apply it?

Patch itself lgtm, I'm just confused what I should do with it.
-Daniel

> 
> Changelog-v2:
> 
>   - patch #1: Add a missing up_read() on netdev_get_name() error path
>               exit. Thanks to Dan/kbuild-bot report.
> 
>   - patch #4: new patch, invalid preemptible context found by the new
>               lockdep checks added in the seqlock series + kbuild-bot.
> 
> Thanks,
> 
> 8<--------------
> 
> Ahmed S. Darwish (6):
>   net: core: device_rename: Use rwsem instead of a seqcount
>   net: phy: fixed_phy: Remove unused seqcount
>   u64_stats: Document writer non-preemptibility requirement
>   net: mdiobus: Disable preemption upon u64_stats update
>   block: nr_sects_write(): Disable preemption on seqcount write
>   dma-buf: Remove custom seqcount lockdep class key
> 
>  block/blk.h                    |  2 ++
>  drivers/dma-buf/dma-resv.c     |  9 +------
>  drivers/net/phy/fixed_phy.c    | 26 ++++++++------------
>  drivers/net/phy/mdio_bus.c     |  2 ++
>  include/linux/dma-resv.h       |  2 --
>  include/linux/u64_stats_sync.h | 43 ++++++++++++++++++----------------
>  net/core/dev.c                 | 40 ++++++++++++++-----------------
>  7 files changed, 56 insertions(+), 68 deletions(-)
> 
> base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
> --
> 2.20.1

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[Ahmed S. Darwish]

On Thu, Jun 04, 2020 at 09:28:41AM +0200, Daniel Vetter wrote:
> On Wed, Jun 03, 2020 at 04:49:43PM +0200, Ahmed S. Darwish wrote:
> > Hi,
> >
> > Since patch #7 and #8 from the series:
> >
> >    [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
> >    https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
> >
> > are now pending on the lockdep/x86 IRQ state tracking patch series:
> >
> >    [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
> >    https://lkml.kernel.org/r/20200529212728.795169701@infradead.org
> >
> >    [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
> >    https://lkml.kernel.org/r/20200529213550.683440625@infradead.org
> >
> > This is a repost only of the seqcount_t call sites bugfixes that were on
> > top of the seqlock patch series.
> >
> > These fixes are independent, and can thus be merged on their own. I'm
> > reposting them now so they can at least hit -rc2 or -rc3.
>
> I'm confused on what I should do with patch 6 here for dma-buf. Looks like
> just a good cleanup/prep work, so I'd queue it for linux-next and 5.9, but
> sounds like you want this in earlier. Do you need this in 5.8-rc for some
> work meant for 5.9? Will this go in through some topic branch directly?
> Should I apply it?
>
> Patch itself lgtm, I'm just confused what I should do with it.
>

My apologies for the confusion. The cover letter is indeed misleading
w.r.t. the dma-buf patch.  It isn't a bugfix, so it shouldn't hit -rc.

Since without this patch compiling the seqcount series will fail, it
will be best to merge it through tip instead.

So all I need for now is a reviewed-by tag :) I will forwoard it to the
tip tree afterwards.

Thanks,

--
Ahmed S. Darwish
Linutronix GmbH

Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[Ahmed S. Darwish]

On Thu, Jun 04, 2020 at 09:28:41AM +0200, Daniel Vetter wrote:
> On Wed, Jun 03, 2020 at 04:49:43PM +0200, Ahmed S. Darwish wrote:
> > Hi,
> >
> > Since patch #7 and #8 from the series:
> >
> >    [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
> >    https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
> >
> > are now pending on the lockdep/x86 IRQ state tracking patch series:
> >
> >    [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
> >    https://lkml.kernel.org/r/20200529212728.795169701@infradead.org
> >
> >    [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
> >    https://lkml.kernel.org/r/20200529213550.683440625@infradead.org
> >
> > This is a repost only of the seqcount_t call sites bugfixes that were on
> > top of the seqlock patch series.
> >
> > These fixes are independent, and can thus be merged on their own. I'm
> > reposting them now so they can at least hit -rc2 or -rc3.
>
> I'm confused on what I should do with patch 6 here for dma-buf. Looks like
> just a good cleanup/prep work, so I'd queue it for linux-next and 5.9, but
> sounds like you want this in earlier. Do you need this in 5.8-rc for some
> work meant for 5.9? Will this go in through some topic branch directly?
> Should I apply it?
>
> Patch itself lgtm, I'm just confused what I should do with it.
>

My apologies for the confusion. The cover letter is indeed misleading
w.r.t. the dma-buf patch.  It isn't a bugfix, so it shouldn't hit -rc.

Since without this patch compiling the seqcount series will fail, it
will be best to merge it through tip instead.

So all I need for now is a reviewed-by tag :) I will forwoard it to the
tip tree afterwards.

Thanks,

--
Ahmed S. Darwish
Linutronix GmbH
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 6/6] dma-buf: Remove custom seqcount lockdep class key

[Daniel Vetter]

On Wed, Jun 03, 2020 at 04:49:49PM +0200, Ahmed S. Darwish wrote:
> Commit 3c3b177a9369 ("reservation: add support for read-only access
> using rcu") introduced a sequence counter to manage updates to
> reservations. Back then, the reservation object initializer
> reservation_object_init() was always inlined.
> 
> Having the sequence counter initialization inlined meant that each of
> the call sites would have a different lockdep class key, which would've
> broken lockdep's deadlock detection. The aforementioned commit thus
> introduced, and exported, a custom seqcount lockdep class key and name.
> 
> The commit 8735f16803f00 ("dma-buf: cleanup reservation_object_init...")
> transformed the reservation object initializer to a normal non-inlined C
> function. seqcount_init(), which automatically defines the seqcount
> lockdep class key and must be called non-inlined, can now be safely used.
> 
> Remove the seqcount custom lockdep class key, name, and export. Use
> seqcount_init() inside the dma reservation object initializer.
> 
> Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Patch lgtm, and Ahmed says plan is that this should land through -tip
since it's part of a larger series, so

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

for merging through whatever tree/branch fits bets. I don't expect
conflicts here, nothing in-flight touching this. I expect this will show
up in 5.9-rc1 or so.

Cheers, Daniel

> ---
>  drivers/dma-buf/dma-resv.c | 9 +--------
>  include/linux/dma-resv.h   | 2 --
>  2 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index 4264e64788c4..590ce7ad60a0 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -50,12 +50,6 @@
>  DEFINE_WD_CLASS(reservation_ww_class);
>  EXPORT_SYMBOL(reservation_ww_class);
>  
> -struct lock_class_key reservation_seqcount_class;
> -EXPORT_SYMBOL(reservation_seqcount_class);
> -
> -const char reservation_seqcount_string[] = "reservation_seqcount";
> -EXPORT_SYMBOL(reservation_seqcount_string);
> -
>  /**
>   * dma_resv_list_alloc - allocate fence list
>   * @shared_max: number of fences we need space for
> @@ -134,9 +128,8 @@ subsys_initcall(dma_resv_lockdep);
>  void dma_resv_init(struct dma_resv *obj)
>  {
>  	ww_mutex_init(&obj->lock, &reservation_ww_class);
> +	seqcount_init(&obj->seq);
>  
> -	__seqcount_init(&obj->seq, reservation_seqcount_string,
> -			&reservation_seqcount_class);
>  	RCU_INIT_POINTER(obj->fence, NULL);
>  	RCU_INIT_POINTER(obj->fence_excl, NULL);
>  }
> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
> index ee50d10f052b..a6538ae7d93f 100644
> --- a/include/linux/dma-resv.h
> +++ b/include/linux/dma-resv.h
> @@ -46,8 +46,6 @@
>  #include <linux/rcupdate.h>
>  
>  extern struct ww_class reservation_ww_class;
> -extern struct lock_class_key reservation_seqcount_class;
> -extern const char reservation_seqcount_string[];
>  
>  /**
>   * struct dma_resv_list - a list of shared fences
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 6/6] dma-buf: Remove custom seqcount lockdep class key

[Daniel Vetter]

On Wed, Jun 03, 2020 at 04:49:49PM +0200, Ahmed S. Darwish wrote:
> Commit 3c3b177a9369 ("reservation: add support for read-only access
> using rcu") introduced a sequence counter to manage updates to
> reservations. Back then, the reservation object initializer
> reservation_object_init() was always inlined.
> 
> Having the sequence counter initialization inlined meant that each of
> the call sites would have a different lockdep class key, which would've
> broken lockdep's deadlock detection. The aforementioned commit thus
> introduced, and exported, a custom seqcount lockdep class key and name.
> 
> The commit 8735f16803f00 ("dma-buf: cleanup reservation_object_init...")
> transformed the reservation object initializer to a normal non-inlined C
> function. seqcount_init(), which automatically defines the seqcount
> lockdep class key and must be called non-inlined, can now be safely used.
> 
> Remove the seqcount custom lockdep class key, name, and export. Use
> seqcount_init() inside the dma reservation object initializer.
> 
> Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Patch lgtm, and Ahmed says plan is that this should land through -tip
since it's part of a larger series, so

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

for merging through whatever tree/branch fits bets. I don't expect
conflicts here, nothing in-flight touching this. I expect this will show
up in 5.9-rc1 or so.

Cheers, Daniel

> ---
>  drivers/dma-buf/dma-resv.c | 9 +--------
>  include/linux/dma-resv.h   | 2 --
>  2 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index 4264e64788c4..590ce7ad60a0 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -50,12 +50,6 @@
>  DEFINE_WD_CLASS(reservation_ww_class);
>  EXPORT_SYMBOL(reservation_ww_class);
>  
> -struct lock_class_key reservation_seqcount_class;
> -EXPORT_SYMBOL(reservation_seqcount_class);
> -
> -const char reservation_seqcount_string[] = "reservation_seqcount";
> -EXPORT_SYMBOL(reservation_seqcount_string);
> -
>  /**
>   * dma_resv_list_alloc - allocate fence list
>   * @shared_max: number of fences we need space for
> @@ -134,9 +128,8 @@ subsys_initcall(dma_resv_lockdep);
>  void dma_resv_init(struct dma_resv *obj)
>  {
>  	ww_mutex_init(&obj->lock, &reservation_ww_class);
> +	seqcount_init(&obj->seq);
>  
> -	__seqcount_init(&obj->seq, reservation_seqcount_string,
> -			&reservation_seqcount_class);
>  	RCU_INIT_POINTER(obj->fence, NULL);
>  	RCU_INIT_POINTER(obj->fence_excl, NULL);
>  }
> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
> index ee50d10f052b..a6538ae7d93f 100644
> --- a/include/linux/dma-resv.h
> +++ b/include/linux/dma-resv.h
> @@ -46,8 +46,6 @@
>  #include <linux/rcupdate.h>
>  
>  extern struct ww_class reservation_ww_class;
> -extern struct lock_class_key reservation_seqcount_class;
> -extern const char reservation_seqcount_string[];
>  
>  /**
>   * struct dma_resv_list - a list of shared fences
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[David Miller]

From: "Ahmed S. Darwish" <a.darwish@linutronix.de>
Date: Wed,  3 Jun 2020 16:49:43 +0200

> Since patch #7 and #8 from the series:
> 
>    [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
>    https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
> 
> are now pending on the lockdep/x86 IRQ state tracking patch series:
> 
>    [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
>    https://lkml.kernel.org/r/20200529212728.795169701@infradead.org
> 
>    [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
>    https://lkml.kernel.org/r/20200529213550.683440625@infradead.org
> 
> This is a repost only of the seqcount_t call sites bugfixes that were on
> top of the seqlock patch series.
> 
> These fixes are independent, and can thus be merged on their own. I'm
> reposting them now so they can at least hit -rc2 or -rc3.
> 
> Changelog-v2:
> 
>   - patch #1: Add a missing up_read() on netdev_get_name() error path
>               exit. Thanks to Dan/kbuild-bot report.
> 
>   - patch #4: new patch, invalid preemptible context found by the new
>               lockdep checks added in the seqlock series + kbuild-bot.

I'll apply patches 1-4 to the net tree.

Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes

[David Miller]

From: "Ahmed S. Darwish" <a.darwish@linutronix.de>
Date: Wed,  3 Jun 2020 16:49:43 +0200

> Since patch #7 and #8 from the series:
> 
>    [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
>    https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
> 
> are now pending on the lockdep/x86 IRQ state tracking patch series:
> 
>    [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
>    https://lkml.kernel.org/r/20200529212728.795169701@infradead.org
> 
>    [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
>    https://lkml.kernel.org/r/20200529213550.683440625@infradead.org
> 
> This is a repost only of the seqcount_t call sites bugfixes that were on
> top of the seqlock patch series.
> 
> These fixes are independent, and can thus be merged on their own. I'm
> reposting them now so they can at least hit -rc2 or -rc3.
> 
> Changelog-v2:
> 
>   - patch #1: Add a missing up_read() on netdev_get_name() error path
>               exit. Thanks to Dan/kbuild-bot report.
> 
>   - patch #4: new patch, invalid preemptible context found by the new
>               lockdep checks added in the seqlock series + kbuild-bot.

I'll apply patches 1-4 to the net tree.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write

[Jens Axboe]

On 6/3/20 8:49 AM, Ahmed S. Darwish wrote:
> For optimized block readers not holding a mutex, the "number of sectors"
> 64-bit value is protected from tearing on 32-bit architectures by a
> sequence counter.
> 
> Disable preemption before entering that sequence counter's write side
> critical section. Otherwise, the read side can preempt the write side
> section and spin for the entire scheduler tick. If the reader belongs to
> a real-time scheduling class, it can spin forever and the kernel will
> livelock.

Applied, thanks.

-- 
Jens Axboe

Re: [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write

[Sasha Levin]

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: c83f6bf98dc1 ("block: add partition resize function to blkpg ioctl").

The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182, v4.9.225, v4.4.225.

v5.6.15: Failed to apply! Possible dependencies:
    1a9fba3a77a5 ("block: unexport read_dev_sector and put_dev_sector")
    2b8bd423614c ("block/diskstats: more accurate approximation of io_ticks for slow disks")
    387048bf67ee ("block: merge partition-generic.c and check.c")
    3ad5cee5cd00 ("block: move sysfs methods shared by disks and partitions to genhd.c")
    581e26004a09 ("block: move block layer internals out of include/linux/genhd.h")
    74cc979c3c7f ("block: cleanup how md_autodetect_dev is called")
    f17c21c1ecb8 ("block: remove alloc_part_info and free_part_info")
    ffa9ed647aa4 ("block: remove warn_no_part")

v5.4.43: Failed to apply! Possible dependencies:
    387048bf67ee ("block: merge partition-generic.c and check.c")
    3ad5cee5cd00 ("block: move sysfs methods shared by disks and partitions to genhd.c")
    581e26004a09 ("block: move block layer internals out of include/linux/genhd.h")
    5eac3eb30c9a ("block: Remove partition support for zoned block devices")
    6c1b1da58f8c ("block: add zone open, close and finish operations")
    74cc979c3c7f ("block: cleanup how md_autodetect_dev is called")
    b68663186577 ("block: add iostat counters for flush requests")
    c7a1d926dc40 ("block: Simplify REQ_OP_ZONE_RESET_ALL handling")
    ceeb373aa6b9 ("block: Simplify report zones execution")
    f902b0260002 ("block: refactor rescan_partitions")

v4.19.125: Failed to apply! Possible dependencies:
    2268c0feb0ff ("blkcg: introduce common blkg association logic")
    27e6fa996c53 ("blkcg: fix ref count issue with bio_blkcg using task_css")
    43b729bfe9cf ("block: move integrity_req_gap_{back,front}_merge to blk.h")
    49f4c2dc2b50 ("blkcg: update blkg_lookup_create to do locking")
    581e26004a09 ("block: move block layer internals out of include/linux/genhd.h")
    5bf9a1f3b4ef ("blkcg: consolidate bio_issue_init to be a part of core")
    6f70fb66182b ("blkcg: remove bio_disassociate_task()")
    a7b39b4e961c ("blkcg: always associate a bio with a blkg")
    b5f2954d30c7 ("blkcg: revert blkcg cleanups series")
    bdc2491708c4 ("blkcg: associate writeback bios with a blkg")
    beea9da07d8a ("blkcg: convert blkg_lookup_create() to find closest blkg")
    c839e7a03f92 ("blkcg: remove bio->bi_css and instead use bio->bi_blkg")
    d459d853c2ed ("blkcg: reassociate bios when make_request() is called recursively")
    ece841abbed2 ("block: fix memleak of bio integrity data")

v4.14.182: Failed to apply! Possible dependencies:
    055f6e18e08f ("block: Make q_usage_counter also track legacy requests")
    1b6d65a0bfb5 ("block: Introduce BLK_MQ_REQ_PREEMPT")
    2268c0feb0ff ("blkcg: introduce common blkg association logic")
    359f642700f2 ("block: move bio_integrity_{intervals,bytes} into blkdev.h")
    37f9579f4c31 ("blk-mq: Avoid that submitting a bio concurrently with device removal triggers a crash")
    3a0a529971ec ("block, scsi: Make SCSI quiesce and resume work reliably")
    43b729bfe9cf ("block: move integrity_req_gap_{back,front}_merge to blk.h")
    581e26004a09 ("block: move block layer internals out of include/linux/genhd.h")
    6a15674d1e90 ("block: Introduce blk_get_request_flags()")
    6f70fb66182b ("blkcg: remove bio_disassociate_task()")
    9a95e4ef7095 ("block, nvme: Introduce blk_mq_req_flags_t")
    c9254f2ddb19 ("block: Add the QUEUE_FLAG_PREEMPT_ONLY request queue flag")
    d459d853c2ed ("blkcg: reassociate bios when make_request() is called recursively")
    ece841abbed2 ("block: fix memleak of bio integrity data")
    f421e1d9ade4 ("block: provide a direct_make_request helper")

v4.9.225: Failed to apply! Possible dependencies:
    297e3d854784 ("blk-throttle: make throtl_slice tunable")
    43b729bfe9cf ("block: move integrity_req_gap_{back,front}_merge to blk.h")
    4e4cbee93d56 ("block: switch bios to blk_status_t")
    581e26004a09 ("block: move block layer internals out of include/linux/genhd.h")
    7c20f11680a4 ("bio-integrity: stop abusing bi_end_io")
    87760e5eef35 ("block: hook up writeback throttling")
    9e234eeafbe1 ("blk-throttle: add a simple idle detection")
    cf43e6be865a ("block: add scalable completion tracking of requests")
    e806402130c9 ("block: split out request-only flags into a new namespace")
    fbbaf700e7b1 ("block: trace completion of all bios.")

v4.4.225: Failed to apply! Possible dependencies:
    005411ea7ee7 ("doc: update block/queue-sysfs.txt entries")
    27489a3c827b ("blk-mq: turn hctx->run_work into a regular work struct")
    297e3d854784 ("blk-throttle: make throtl_slice tunable")
    38f8baae8905 ("block: factor out chained bio completion")
    43b729bfe9cf ("block: move integrity_req_gap_{back,front}_merge to blk.h")
    4e4cbee93d56 ("block: switch bios to blk_status_t")
    511cbce2ff8b ("irq_poll: make blk-iopoll available outside the block layer")
    581e26004a09 ("block: move block layer internals out of include/linux/genhd.h")
    7c20f11680a4 ("bio-integrity: stop abusing bi_end_io")
    87760e5eef35 ("block: hook up writeback throttling")
    8d354f133e86 ("blk-mq: improve layout of blk_mq_hw_ctx")
    9467f85960a3 ("blk-mq/cpu-notif: Convert to new hotplug state machine")
    9e234eeafbe1 ("blk-throttle: add a simple idle detection")
    af3e3a5259e3 ("block: don't unecessarily clobber bi_error for chained bios")
    ba8c6967b739 ("block: cleanup bio_endio")
    cf43e6be865a ("block: add scalable completion tracking of requests")
    e57690fe009b ("blk-mq: don't overwrite rq->mq_ctx")
    e6a40b096e28 ("block: prepare request creation/destruction code to use REQ_OPs")
    e806402130c9 ("block: split out request-only flags into a new namespace")
    fbbaf700e7b1 ("block: trace completion of all bios.")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

Re: [PATCH v2 1/6] net: core: device_rename: Use rwsem instead of a seqcount

[Sasha Levin]

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 5dbe7c178d3f ("net: fix kernel deadlock with interface rename and netdev name retrieval.").

The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182, v4.9.225, v4.4.225.

v5.6.15: Build OK!
v5.4.43: Failed to apply! Possible dependencies:
    2da2b32fd934 ("sched/rt, net: Use CONFIG_PREEMPTION.patch")

v4.19.125: Failed to apply! Possible dependencies:
    2da2b32fd934 ("sched/rt, net: Use CONFIG_PREEMPTION.patch")

v4.14.182: Failed to apply! Possible dependencies:
    2da2b32fd934 ("sched/rt, net: Use CONFIG_PREEMPTION.patch")

v4.9.225: Failed to apply! Possible dependencies:
    2da2b32fd934 ("sched/rt, net: Use CONFIG_PREEMPTION.patch")

v4.4.225: Failed to apply! Possible dependencies:
    2da2b32fd934 ("sched/rt, net: Use CONFIG_PREEMPTION.patch")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha