Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Feb 2020 13:14:42 +0800
From:      Li-Wen Hsu <lwhsu@freebsd.org>
To:        "O. Hartmann" <o.hartmann@walstatt.org>
Cc:        Scott Long <scottl@freebsd.org>, src-committers <src-committers@freebsd.org>,  svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r357710 - in head/sys: cam/ata cam/nvme cam/scsi ufs/ffs
Message-ID:  <CAKBkRUxuvzZdDPKfdQr5Ynr=ZW0cp8ORpKjEP5Mxj2pzkzBnBw@mail.gmail.com>
In-Reply-To: <20200210060145.6920e8f5@thor.intern.walstatt.dynvpn.de>
References:  <202002100023.01A0NKmY053556@repo.freebsd.org> <20200210060145.6920e8f5@thor.intern.walstatt.dynvpn.de>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Feb 10, 2020 at 1:02 PM O. Hartmann <o.hartmann@walstatt.org> wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Am Mon, 10 Feb 2020 00:23:20 +0000 (UTC)
> Scott Long <scottl@FreeBSD.org> schrieb:
>
> > Author: scottl
> > Date: Mon Feb 10 00:23:20 2020
> > New Revision: 357710
> > URL: https://svnweb.freebsd.org/changeset/base/357710
> >
> > Log:
> >   Add rudamentary support for UFS to probe whether a block device supports the
> >   BIO_SPEEDUP command.  Add complimentary support to the CAM periphs that
> >   support it.
> >
> > Modified:
> >   head/sys/cam/ata/ata_da.c
> >   head/sys/cam/nvme/nvme_da.c
> >   head/sys/cam/scsi/scsi_da.c
> >   head/sys/ufs/ffs/ffs_softdep.c
> >   head/sys/ufs/ffs/ffs_vfsops.c
> >
> > Modified: head/sys/cam/ata/ata_da.c
> > ==============================================================================
> > --- head/sys/cam/ata/ata_da.c Mon Feb 10 00:05:04 2020        (r357709)
> > +++ head/sys/cam/ata/ata_da.c Mon Feb 10 00:23:20 2020        (r357710)
> > @@ -50,6 +50,7 @@ __FBSDID("$FreeBSD$");
> >  #include <sys/proc.h>
> >  #include <sys/reboot.h>
> >  #include <sys/sbuf.h>
> > +#include <geom/geom.h>
> >  #include <geom/geom_disk.h>
> >  #endif /* _KERNEL */
> >
> > @@ -1565,6 +1566,10 @@ adagetattr(struct bio *bp)
> >  {
> >       int ret;
> >       struct cam_periph *periph;
> > +
> > +     /* TODO: tunable knob */
> > +     if (g_handleattr_int(bp, "GEOM::canspeedup", 1))
> > +             return (0);
> >
> >       periph = (struct cam_periph *)bp->bio_disk->d_drv1;
> >       cam_periph_lock(periph);
> >
> > Modified: head/sys/cam/nvme/nvme_da.c
> > ==============================================================================
> > --- head/sys/cam/nvme/nvme_da.c       Mon Feb 10 00:05:04 2020        (r357709)
> > +++ head/sys/cam/nvme/nvme_da.c       Mon Feb 10 00:23:20 2020        (r357710)
> > @@ -48,6 +48,7 @@ __FBSDID("$FreeBSD$");
> >  #include <sys/cons.h>
> >  #include <sys/proc.h>
> >  #include <sys/reboot.h>
> > +#include <geom/geom.h>
> >  #include <geom/geom_disk.h>
> >  #endif /* _KERNEL */
> >
> > @@ -699,6 +700,10 @@ ndagetattr(struct bio *bp)
> >  {
> >       int ret;
> >       struct cam_periph *periph;
> > +
> > +     /* TODO: tunable knob */
> > +     if (g_handleattr_int(bp, "GEOM::canspeedup", 1))
> > +             return (0);
> >
> >       periph = (struct cam_periph *)bp->bio_disk->d_drv1;
> >       cam_periph_lock(periph);
> >
> > Modified: head/sys/cam/scsi/scsi_da.c
> > ==============================================================================
> > --- head/sys/cam/scsi/scsi_da.c       Mon Feb 10 00:05:04 2020        (r357709)
> > +++ head/sys/cam/scsi/scsi_da.c       Mon Feb 10 00:23:20 2020        (r357710)
> > @@ -1942,6 +1942,10 @@ dagetattr(struct bio *bp)
> >       int ret;
> >       struct cam_periph *periph;
> >
> > +     /* TODO: tunable knob for this */
> > +     if (g_handleattr_int(bp, "GEOM::canspeedup", 1))
> > +             return (0);
> > +
> >       periph = (struct cam_periph *)bp->bio_disk->d_drv1;
> >       cam_periph_lock(periph);
> >       ret = xpt_getattr(bp->bio_data, bp->bio_length, bp->bio_attribute,
> >
> > Modified: head/sys/ufs/ffs/ffs_softdep.c
> > ==============================================================================
> > --- head/sys/ufs/ffs/ffs_softdep.c    Mon Feb 10 00:05:04 2020        (r357709)
> > +++ head/sys/ufs/ffs/ffs_softdep.c    Mon Feb 10 00:23:20 2020        (r357710)
> > @@ -1464,6 +1464,9 @@ softdep_send_speedup(struct ufsmount *ump, size_t shor
> >  {
> >       struct buf *bp;
> >
> > +     if ((ump->um_flags & UM_CANSPEEDUP) == 0)
> > +             return;
> > +
> >       bp = malloc(sizeof(*bp), M_TRIM, M_WAITOK | M_ZERO);
> >       bp->b_iocmd = BIO_SPEEDUP;
> >       bp->b_ioflags = flags;
> >
> > Modified: head/sys/ufs/ffs/ffs_vfsops.c
> > ==============================================================================
> > --- head/sys/ufs/ffs/ffs_vfsops.c     Mon Feb 10 00:05:04 2020        (r357709)
> > +++ head/sys/ufs/ffs/ffs_vfsops.c     Mon Feb 10 00:23:20 2020        (r357710)
> > @@ -794,7 +794,7 @@ ffs_mountfs(devvp, mp, td)
> >       struct ucred *cred;
> >       struct g_consumer *cp;
> >       struct mount *nmp;
> > -     int candelete;
> > +     int candelete, canspeedup;
> >       off_t loc;
> >
> >       fs = NULL;
> > @@ -1009,6 +1009,13 @@ ffs_mountfs(devvp, mp, td)
> >                       ump->um_trimhash = hashinit(MAXTRIMIO, M_TRIM,
> >                           &ump->um_trimlisthashsize);
> >               }
> > +     }
> > +
> > +     /* TODO: sysctl tunables, runtime modification */
> > +     len = sizeof(int);
> > +     if (g_io_getattr("GEOM::canspeedup", cp, &len, &canspeedup) == 0) {
> > +             if (canspeedup)
> > +                     ump->um_flags |= UM_CANSPEEDUP;
> >       }
> >
> >       ump->um_mountp = mp;
> > _______________________________________________
> > svn-src-head@freebsd.org mailing list
> > https://lists.freebsd.org/mailman/listinfo/svn-src-head
> > To unsubscribe, send any mail to "svn-src-head-unsubscribe@freebsd.org"
>
> It seems that this commit makes our systems to hang at the point, when trying to mount the
> root partition, last seen message on console is:
>
> [...]
> mountroot: waiting for device /dev/gpt/root...
>
>
> The root partitions on those systems resides on UFS2 formated SSDs (if this has any relevance).

The test in the CI seems also handing because of this. The full
backtrace is available at:

https://ci.freebsd.org/job/FreeBSD-head-i386-test/8396/console

The changes in the same build are r357709 and r357710. The previous is
arm only change so it seems less suspicious.

Please help checking this. Thanks.

Li-Wen



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAKBkRUxuvzZdDPKfdQr5Ynr=ZW0cp8ORpKjEP5Mxj2pzkzBnBw>