Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes
@ 2020-06-03 14:49 Ahmed S. Darwish
  2020-06-03 14:49 ` [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write Ahmed S. Darwish
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ahmed S. Darwish @ 2020-06-03 14:49 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Will Deacon
  Cc: Thomas Gleixner, Paul E. McKenney, Sebastian A. Siewior,
	Steven Rostedt, LKML, Ahmed S. Darwish, David S. Miller,
	Jakub Kicinski, Eric Dumazet, Jens Axboe, Vivek Goyal,
	linux-block, David Airlie, Daniel Vetter, Sumit Semwal,
	linux-media, dri-devel

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

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

* [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write
  2020-06-03 14:49 [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes Ahmed S. Darwish
@ 2020-06-03 14:49 ` Ahmed S. Darwish
  2020-06-05  3:22   ` Jens Axboe
  2020-06-04  7:28 ` [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes Daniel Vetter
  2020-06-04 22:50 ` David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Ahmed S. Darwish @ 2020-06-03 14:49 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Will Deacon
  Cc: Thomas Gleixner, Paul E. McKenney, Sebastian A. Siewior,
	Steven Rostedt, LKML, Ahmed S. Darwish, Jens Axboe, Vivek Goyal,
	linux-block

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


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

* Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes
  2020-06-03 14:49 [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes Ahmed S. Darwish
  2020-06-03 14:49 ` [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write Ahmed S. Darwish
@ 2020-06-04  7:28 ` Daniel Vetter
  2020-06-04  8:41   ` Ahmed S. Darwish
  2020-06-04 22:50 ` David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2020-06-04  7:28 UTC (permalink / raw)
  To: Ahmed S. Darwish
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Thomas Gleixner,
	Paul E. McKenney, Sebastian A. Siewior, Steven Rostedt, LKML,
	David S. Miller, Jakub Kicinski, Eric Dumazet, Jens Axboe,
	Vivek Goyal, linux-block, David Airlie, Daniel Vetter,
	Sumit Semwal, linux-media, dri-devel

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

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

* Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes
  2020-06-04  7:28 ` [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes Daniel Vetter
@ 2020-06-04  8:41   ` Ahmed S. Darwish
  0 siblings, 0 replies; 6+ messages in thread
From: Ahmed S. Darwish @ 2020-06-04  8:41 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Thomas Gleixner,
	Paul E. McKenney, Sebastian A. Siewior, Steven Rostedt, LKML,
	David S. Miller, Jakub Kicinski, Eric Dumazet, Jens Axboe,
	Vivek Goyal, linux-block, David Airlie, Sumit Semwal,
	linux-media, dri-devel

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

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

* Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes
  2020-06-03 14:49 [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes Ahmed S. Darwish
  2020-06-03 14:49 ` [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write Ahmed S. Darwish
  2020-06-04  7:28 ` [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes Daniel Vetter
@ 2020-06-04 22:50 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-06-04 22:50 UTC (permalink / raw)
  To: a.darwish
  Cc: peterz, mingo, will, tglx, paulmck, bigeasy, rostedt,
	linux-kernel, kuba, edumazet, axboe, vgoyal, linux-block,
	airlied, daniel, sumit.semwal, linux-media, dri-devel

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.

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

* Re: [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write
  2020-06-03 14:49 ` [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write Ahmed S. Darwish
@ 2020-06-05  3:22   ` Jens Axboe
  0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2020-06-05  3:22 UTC (permalink / raw)
  To: Ahmed S. Darwish, Peter Zijlstra, Ingo Molnar, Will Deacon
  Cc: Thomas Gleixner, Paul E. McKenney, Sebastian A. Siewior,
	Steven Rostedt, LKML, Vivek Goyal, linux-block

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


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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 14:49 [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes Ahmed S. Darwish
2020-06-03 14:49 ` [PATCH v2 5/6] block: nr_sects_write(): Disable preemption on seqcount write Ahmed S. Darwish
2020-06-05  3:22   ` Jens Axboe
2020-06-04  7:28 ` [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes Daniel Vetter
2020-06-04  8:41   ` Ahmed S. Darwish
2020-06-04 22:50 ` David Miller

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org
	public-inbox-index linux-block

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git