Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
On Tue, Sep 23, 2014 at 01:56:48AM -0400, Tejun Heo wrote: On Tue, Sep 23, 2014 at 07:55:54AM +0200, Christoph Hellwig wrote: Jens, can we simply get these commits reverted from now if there's no better fix? I'd hate to have this boot stall in the first kernel with blk-mq support for scsi. Patches going out right now. [PATCHSET percpu/for-3.18] percpu_ref: implement switch_to_atomic/percpu() looks way to big for 3.17, and the regression was introduced in the 3.17 merge window. I'm not sure what was broken before, but it defintively survived a lot of testing. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
On Tue, Sep 23, 2014 at 02:01:41AM -0400, Tejun Heo wrote: On Tue, Sep 23, 2014 at 07:59:24AM +0200, Christoph Hellwig wrote: [PATCHSET percpu/for-3.18] percpu_ref: implement switch_to_atomic/percpu() looks way to big for 3.17, and the regression was introduced in the 3.17 merge window. I'm not sure what was broken before, but it defintively survived a lot of testing. Do we even care about fixing it for 3.17? scsi-mq isn't enabled by default even for 3.18. The open-coded percpu ref thing was subtly broken there. It'd be difficult to trigger but I'm fairly sure it'd crap out in the wild once in a blue moon. Hmmm... also, this is actually an issue with scsi that block layer is working around. For other drivers (virtio), the latency during shutdown should be completely fine. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
On Tue, Sep 23, 2014 at 02:01:41AM -0400, Tejun Heo wrote: On Tue, Sep 23, 2014 at 07:59:24AM +0200, Christoph Hellwig wrote: [PATCHSET percpu/for-3.18] percpu_ref: implement switch_to_atomic/percpu() looks way to big for 3.17, and the regression was introduced in the 3.17 merge window. I'm not sure what was broken before, but it defintively survived a lot of testing. Do we even care about fixing it for 3.17? scsi-mq isn't enabled by default even for 3.18. The open-coded percpu ref thing was subtly broken there. It'd be difficult to trigger but I'm fairly sure it'd crap out in the wild once in a blue moon. It's compiled in by default, and people are extremly eager to test it. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
On Tue, Sep 23, 2014 at 08:09:06AM +0200, Christoph Hellwig wrote: On Tue, Sep 23, 2014 at 02:01:41AM -0400, Tejun Heo wrote: On Tue, Sep 23, 2014 at 07:59:24AM +0200, Christoph Hellwig wrote: [PATCHSET percpu/for-3.18] percpu_ref: implement switch_to_atomic/percpu() looks way to big for 3.17, and the regression was introduced in the 3.17 merge window. I'm not sure what was broken before, but it defintively survived a lot of testing. Do we even care about fixing it for 3.17? scsi-mq isn't enabled by default even for 3.18. The open-coded percpu ref thing was subtly broken there. It'd be difficult to trigger but I'm fairly sure it'd crap out in the wild once in a blue moon. It's compiled in by default, and people are extremly eager to test it. Ugh, I don't know. It's not like we have a very good baseline we can go back to and reverting it for -stable and then redoing it seems kinda excessive for a yet experimental feature. Jens? -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: boot stall regression due to blk-mq: use percpu_ref for mq usage count
-Original Message- From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- ow...@vger.kernel.org] On Behalf Of Tejun Heo Sent: Tuesday, 23 September, 2014 1:12 AM To: Christoph Hellwig Cc: Jens Axboe; linux-ker...@vger.kernel.org; linux-scsi@vger.kernel.org Subject: Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count On Tue, Sep 23, 2014 at 08:09:06AM +0200, Christoph Hellwig wrote: On Tue, Sep 23, 2014 at 02:01:41AM -0400, Tejun Heo wrote: On Tue, Sep 23, 2014 at 07:59:24AM +0200, Christoph Hellwig wrote: [PATCHSET percpu/for-3.18] percpu_ref: implement switch_to_atomic/percpu() looks way to big for 3.17, and the regression was introduced in the 3.17 merge window. I'm not sure what was broken before, but it defintively survived a lot of testing. Do we even care about fixing it for 3.17? scsi-mq isn't enabled by default even for 3.18. The open-coded percpu ref thing was subtly broken there. It'd be difficult to trigger but I'm fairly sure it'd crap out in the wild once in a blue moon. It's compiled in by default, and people are extremly eager to test it. Ugh, I don't know. It's not like we have a very good baseline we can go back to and reverting it for -stable and then redoing it seems kinda excessive for a yet experimental feature. Jens? scsi_mod.use_blk_mq is not listed in 3.17rc6 Documentation/kernel- parameters.txt or Documentation/scsi/scsi-parameters.txt (which does admit This document may not be entirely up to date and comprehensive.). Perhaps a description with an experimental warning could be slipped into scsi-parameters.txt in 3.17, with plans to remove that warning in 3.18. --- Rob ElliottHP Server Storage -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
On 09/23/2014 12:11 AM, Tejun Heo wrote: On Tue, Sep 23, 2014 at 08:09:06AM +0200, Christoph Hellwig wrote: On Tue, Sep 23, 2014 at 02:01:41AM -0400, Tejun Heo wrote: On Tue, Sep 23, 2014 at 07:59:24AM +0200, Christoph Hellwig wrote: [PATCHSET percpu/for-3.18] percpu_ref: implement switch_to_atomic/percpu() looks way to big for 3.17, and the regression was introduced in the 3.17 merge window. I'm not sure what was broken before, but it defintively survived a lot of testing. Do we even care about fixing it for 3.17? scsi-mq isn't enabled by default even for 3.18. The open-coded percpu ref thing was subtly broken there. It'd be difficult to trigger but I'm fairly sure it'd crap out in the wild once in a blue moon. It's compiled in by default, and people are extremly eager to test it. Ugh, I don't know. It's not like we have a very good baseline we can go back to and reverting it for -stable and then redoing it seems kinda excessive for a yet experimental feature. Jens? It's not just scsi-mq, there are active users of blk-mq in the current tree - like virtio_blk, mtip32xx. None of those are affected by the RCU slowdown due to these changes, so it's not a big deal to them. But it is a big deal if we can't tell people to test scsi-mq in 3.17, that was the entire point of having it there but not default to on. So yeah, this really should be fixed for 3.17. I'm not aware of any reports on the existing enter count breaking things for them. So while it may not be perfect, reverting the percpu ref count changes for 3.17 may be the best option that we have. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
Jens, can we simply get these commits reverted from now if there's no better fix? I'd hate to have this boot stall in the first kernel with blk-mq support for scsi. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
On Tue, Sep 23, 2014 at 07:55:54AM +0200, Christoph Hellwig wrote: Jens, can we simply get these commits reverted from now if there's no better fix? I'd hate to have this boot stall in the first kernel with blk-mq support for scsi. Patches going out right now. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
On Tue, Sep 23, 2014 at 01:56:48AM -0400, Tejun Heo wrote: On Tue, Sep 23, 2014 at 07:55:54AM +0200, Christoph Hellwig wrote: Jens, can we simply get these commits reverted from now if there's no better fix? I'd hate to have this boot stall in the first kernel with blk-mq support for scsi. Patches going out right now. And the original implementation was broken, so... -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: boot stall regression due to blk-mq: use percpu_ref for mq usage count
On 09/19/2014 05:38 AM, Christoph Hellwig wrote: Hi Jens, hi Tejun, I've seen multi-second boot stalls in one of my KVM setups during the initial scsi scan: [0.949892] scsi host0: Virtio SCSI HBA [1.007864] scsi 0:0:0:0: Direct-Access QEMU QEMU HARDDISK1.1. PQ: 0 ANSI: 5 [1.021299] scsi 0:0:1:0: Direct-Access QEMU QEMU HARDDISK1.1. PQ: 0 ANSI: 5 [1.520356] tsc: Refined TSC clocksource calibration: 2491.910 MHz stall [ 16.186549] sd 0:0:0:0: Attached scsi generic sg0 type 0 [ 16.190478] sd 0:0:1:0: Attached scsi generic sg1 type 0 [ 16.194099] osd: LOADED open-osd 0.2.1 [ 16.203202] sd 0:0:0:0: [sda] 31457280 512-byte logical blocks: (16.1 GB/15.0 GiB) [ 16.208478] sd 0:0:0:0: [sda] Write Protect is off [ 16.211439] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 16.218771] sd 0:0:1:0: [sdb] 31457280 512-byte logical blocks: (16.1 GB/15.0 GiB) [ 16.223264] sd 0:0:1:0: [sdb] Write Protect is off [ 16.225682] sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA I've tracked this down to blk-mq: use percpu_ref for mq usage count in a rather painful way as that one introduced enough other regressions to mess up bisect. If I revert the following commits: dd840087086f3b93ac20f7472b4fca59aff7b79f cddd5d17642cc6881352732693c2ae6930e9ce65 add703fda981b9719d37f371498b9f129acbd997 which are the above mentioned commit and two fixes to it the problem goes away. Thanks for bisecting this, I ran into something that I think is the same issue about a week ago. Tejun, any ideas before I dig into this one? -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
