Date: Mon, 7 Jan 2013 04:02:01 -0500 From: Kurt Lidl <lidl@pix.net> To: Marius Strobl <marius@alchemy.franken.de> Cc: mav@freebsd.org, freebsd-sparc64@freebsd.org Subject: Re: smartmontools panics 9.1-RELEASE on sunfire 240 Message-ID: <20130107090200.GA53424@pix.net> In-Reply-To: <20130106031245.GC26039@alchemy.franken.de> References: <20130104051914.GA22613@pix.net> <20130104235336.GB37999@alchemy.franken.de> <20130105013224.GA31361@pix.net> <20130105015242.GB26039@alchemy.franken.de> <20130106021923.GE1410@funkthat.com> <20130106031245.GC26039@alchemy.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jan 06, 2013 at 04:12:45AM +0100, Marius Strobl wrote:
> On Sat, Jan 05, 2013 at 06:19:23PM -0800, John-Mark Gurney wrote:
> > Marius Strobl wrote this message on Sat, Jan 05, 2013 at 02:52 +0100:
> > > On Fri, Jan 04, 2013 at 08:32:24PM -0500, Kurt Lidl wrote:
> > > > On Sat, Jan 05, 2013 at 12:53:36AM +0100, Marius Strobl wrote:
> > > > > Uhm, probably an userland buffer which isn't even 16-bit aligned.
> > > > > If that's the cause, the attached patch hopefully should at least
> > > > > prevent the panic. If it does, smartmontools still need to be fixed
> > > > > though.
> > > >
> > > > You patch prevents the panic from happening.
> > > > When I try to start smartd now, it reports:
> > > >
> > > > root@host-98: /usr/local/etc/rc.d/smartd onestart
> > > > Starting smartd.
> > > > smartd: cam_send_ccb: Invalid argument
> > > > /usr/local/etc/rc.d/smartd: WARNING: failed to start smartd
> > > >
> > > > I had updated the kernel on the machine to 9-STABLE, and
> > > > verified that without this patch, the crash still happened with
> > > > a 9-STABLE kernel, in addition to 9.1-RELEASE kernel.
> > > >
> > > > My kernel now identifies itself as:
> > > > FreeBSD 9.1-STABLE (GENERIC) #1 r245044:245048M: Fri Jan 4 20:19:50 EST 2013
> > > >
> > > > -Kurt
> > > >
> > > > > Index: cam_periph.c
> > > > > ===================================================================
> > > > > --- cam_periph.c (revision 245046)
> > > > > +++ cam_periph.c (working copy)
> > > > > @@ -744,6 +744,9 @@ cam_periph_mapmem(union ccb *ccb, struct cam_perip
> > > > > if ((ccb->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_NONE)
> > > > > return(0);
> > > > >
> > > > > + if ((uintptr_t)ccb->ataio.data_ptr % sizeof(uint16_t) != 0)
> > > > > + return (EINVAL);
> > > > > +
> > > > > data_ptrs[0] = &ccb->ataio.data_ptr;
> > > > > lengths[0] = ccb->ataio.dxfer_len;
> > > > > dirs[0] = ccb->ccb_h.flags & CAM_DIR_MASK;
> > > >
> > >
> > > Alexander, are you okay with this approach or do you have a better
> > > idea how to handle this? In any case, it doesn't seem to make sense
> > > to teach the kernel how to cope with arbitrarily aligned buffers for
> > > ATA.
> >
> > Shouldn't we make it dependant on the __NO_STRICT_ALIGNMENT define so
> > that it won't immediately break other arches?
> >
>
> No, not doing so tremendously helps ensuring that the software is
> properly written (apart from compact flash, ATA devices really
> only support 16-bit and 32-bit accesses) and judging the history
> of the patches in the smartmontools port it apparently already
> has to care about proper alignment for SCSI anyway. It would also
> not be the first time the smartmontools port is blown out of the
> water :)
The following one-line change to smartd.cpp allows it at least start
on my sparc64 machine. (Tested on a machine with a completely
stock 9.1-RELEASE kernel.)
--- smartd.cpp.orig 2012-09-15 11:50:29.000000000 -0400
+++ smartd.cpp 2013-01-07 02:15:22.828853514 -0500
@@ -1910,7 +1910,7 @@
static int ATADeviceScan(dev_config & cfg, dev_state & state, ata_device * atadev)
{
int supported=0;
- struct ata_identify_device drive;
+ struct ata_identify_device drive __attribute__ ((aligned (4)));
const char *name = cfg.name.c_str();
int retid;
-Kurt
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130107090200.GA53424>