From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [5.0-rc5 regression] "scsi: kill off the legacy IO path" causes 5 minute delay during boot on Sun Blade 2500 Date: Mon, 11 Feb 2019 18:13:18 -0800 Message-ID: <1549937598.2857.8.camel@HansenPartnership.com> References: <1549736341.2971.7.camel@HansenPartnership.com> <1549813472.4142.3.camel@HansenPartnership.com> <3380ed8e-ae02-96f2-142b-7cce09459df8@kernel.dk> <1549815924.4142.8.camel@HansenPartnership.com> <0e6e5d67-d305-dd00-2e42-e2299166c8b2@kernel.dk> <1549898730.2831.6.camel@HansenPartnership.com> <44bb4374-0b7c-733b-a53e-92d2f03f2f49@kernel.dk> <1549899773.2831.12.camel@HansenPartnership.com> <1a00da0e-cb8e-30ea-8d17-120f97242b2f@kernel.dk> <1549902521.2831.23.camel@HansenPartnership.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Jens Axboe , Mikael Pettersson Cc: Linux SPARC Kernel Mailing List , linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-scsi List-Id: linux-scsi@vger.kernel.org On Mon, 2019-02-11 at 09:31 -0700, Jens Axboe wrote: > On 2/11/19 9:28 AM, James Bottomley wrote: > > On Mon, 2019-02-11 at 08:46 -0700, Jens Axboe wrote: > > > On 2/11/19 8:42 AM, James Bottomley wrote: > > > > On Mon, 2019-02-11 at 08:28 -0700, Jens Axboe wrote: > > > > > On 2/11/19 8:25 AM, James Bottomley wrote: > > > > > > On Sun, 2019-02-10 at 09:35 -0700, Jens Axboe wrote: > > > > > > > On 2/10/19 9:25 AM, James Bottomley wrote: > > > > [...] > > > > > > > > That check wasn't changed by the code removal. > > > > > > > > > > > > > > As I said above, for sd. This isn't true for non-disks. > > > > > > > > > > > > Yes, but the behaviour above doesn't change across a switch > > > > > > to MQ, so I don't quite understand how it bisects back to > > > > > > that change. If we're not gathering entropy for the device > > > > > > now, we wouldn't have been before the switch, so the > > > > > > entropy characteristics shouldn't have changed. > > > > > > > > > > But it does, as I also wrote in that first email. The legacy > > > > > queue flags had QUEUE_FLAG_ADD_RANDOM set by default, the MQ > > > > > ones do not. Hence any non-sd device would previously ALWAYS > > > > > have ADD_RANDOM set, now none of them do. Also see the patch > > > > > I sent. > > > > > > > > So your theory is that the disk in question never gets to the > > > > rotational check? because the check will clear the flag if > > > > it's non-rotational and set it if it's not, so the default > > > > state of the flag shouldn't matter. > > > > > > No, my point is about non-disks, devices that aren't driven by > > > sd. The behavior for sd hasn't changed, as it sets/clears it > > > unconditionally. > > > > I agree, but I don't think any of them were significant entropy > > contributors before: things like nvme have always been outside of > > this and sr and st don't really contribute much to the seek load > > during boot because they're probed but not used by the boot > > sequence, so I can't see how they would cause this behaviour. I > > suppose it could be target probing, but even that seems unlikely > > because it should be dwarfed by the number of root disk reads > > during boot. > > > > For the rng to take an additional 5 minutes to initialize, we must > > have lost a significant entropy source somewhere. > > I agree it's not a significant amount of entropy, but even just one > bit could mean a long stall if that put us over the edge of just not > having enough for whatever is blocking on /dev/random. Mikael's boot > did have a CDROM, it's not impossible that the handful of commands we > end up doing to that device would have contributed enough entropy to > get the boot done without stalling for minutes. > > One way to know for sure, and that's if Mikael tests the patch. I think I've got the root cause. I have one system in my test bed exhibiting this behaviour. It turns out the disk in it has no characteristics VPD page. The 0xB1 VPD was a SBC-3 addition, so that's not surprising. However, the characteristics check bails before setting the flags, so it takes the default flag which has flipped. We can either fix this by setting the QUEUE_FLAG_ADD_RANDOM if there's no 0xB1 page or by setting the default as Jens proposed. James --- diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d0a980915801..1f3a1474042e 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2961,6 +2961,10 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp) buffer = kmalloc(vpd_len, GFP_KERNEL); + /* set to rotational in case no device characteristic page exists */ + blk_queue_flag_clear(QUEUE_FLAG_NONROT, q); + blk_queue_flag_set(QUEUE_FLAG_ADD_RANDOM, q); + if (!buffer || /* Block Device Characteristics VPD */ scsi_get_vpd_page(sdkp->device, 0xb1, buffer, vpd_len)) @@ -2971,9 +2975,6 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp) if (rot == 1) { blk_queue_flag_set(QUEUE_FLAG_NONROT, q); blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, q); - } else { - blk_queue_flag_clear(QUEUE_FLAG_NONROT, q); - blk_queue_flag_set(QUEUE_FLAG_ADD_RANDOM, q); } if (sdkp->device->type == TYPE_ZBC) {