Date: Fri, 16 Aug 2019 07:48:45 -0600 From: Warner Losh <imp@bsdimp.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Andriy Gapon <avg@freebsd.org>, "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org> Subject: Re: confused/concerned about ZFS / 64-bit atomics / 32-bit platforms Message-ID: <CANCZdfr-PVGyg6KKWYerWcS3vHUBimkOXJ4Y%2BOWbkn6sZPvncg@mail.gmail.com> In-Reply-To: <CANCZdfo8vm3hZxFFe2wvoLKSOp8K38kC%2BKHCVNhv-_Ct3LD0VQ@mail.gmail.com> References: <8bbee981-4f95-22eb-d9ec-00267c8e111d@FreeBSD.org> <20190816081406.GM2738@kib.kiev.ua> <CANCZdfo8vm3hZxFFe2wvoLKSOp8K38kC%2BKHCVNhv-_Ct3LD0VQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[[ sorry to followup, but had a followup thought ]] On Fri, Aug 16, 2019 at 7:32 AM Warner Losh <imp@bsdimp.com> wrote: > > > On Fri, Aug 16, 2019 at 2:14 AM Konstantin Belousov <kostikbel@gmail.com> > wrote: > >> On Fri, Aug 16, 2019 at 10:23:01AM +0300, Andriy Gapon wrote: >> > >> > I somewhat confused with respect to what guarantees C provides with >> > respect to accessing 64-bit objects on 32-bit platforms, if any. >> > I am also concerned about the use of 64-bit atomic values in ZFS given >> > that FreeBSD seems to support ZFS on 32-bit platforms (powerpc, i386, >> ...). >> > >> > My concerns stems from a failed import of a ZFS change from illumos. >> > That change has this pattern: >> > >> > volatile uint64_t *p; >> > uint64_t x, y; >> > ... >> > x = *p; >> > ... >> > atomic_foo_64(p, y); >> > >> > Specifically, I am concerned that there can be a torn read in x=*p >> > assignment on 32-bit platforms even if they provide a native >> > implementation of atomic_foo_64(). I am even more concerned about >> > platforms where atomic_foo_64() is not available and we need to emulate >> > it in opensolaris_atomic.c with the help from atomic_mtx. >> > >> > In more general terms, I am concerned about plain reads of 64-bit >> > variables that are manipulated atomically elsewhere, on 32-bit >> platforms. >> > Is my concern justified? >> Yes, your concerns are justified. Plain reads of 64bit variables on 32bit >> arches are not atomic. Also we do not provide e.g. atomic_load_64(9) on >> 32bit machines. >> > > Should we? I mean for those that support it? > Or is the practice pervasive enough that we can't hope to catch them all and we should just mark ZFS broken on all 32-bit platforms because even though it compiles, the torn-write and other cases are too hard to detect and too pervasive to support? > On some architectures, there might be specific tricks to ensure >> atomicity of load and store for 64bit vars, e.g. on i386 (actually >> Pentium and newer) with use of of the CMPXCHG8 instruction, using it in >> dumb mode. ARM seems to provide LDREXD/STREXD. But AFAIK 32 ppc does not >> have any means to do it. >> > > My take: On those platforms that can do 64-bit atomics can support this > software, those that can't don't get support for this software. > > i386 and armv[67] are still big enough players that supporting ZFS on them > is desirable (though not mandatory). Since they have the functions, we > should enable building there and fix bugs that are identified. If there's > too many, or they are too elusive, we might want to disable by default > selectively. armv6, if it differs from armv7, is likely OK to mark ZFS as > broken there given how little power / memory the PiB and Pi0 have as well > as a lack of a good way to connect mass storage (just USB which exacerbates > the power situation on systems where power is already fragile). > > PowerPC 32-bit really isn't setup for ZFS either. The systems tend to have > less memory and don't tend to have a lot of disk. While it's nice to run > ZFS in a single disk setup, UFS is adequate for those situations. Don't > build file servers with FreeBSD 13 on this platform. We should mark it as > broken here. We already do this with several drivers. > > mips can do 64-bit atomics on 32-bit platforms, but it's ugly via > disabling interrupts (at least I think I committed those).... But even > fewer people have 32-bit mips boxes that want ZFS too... We don't build ZFS > on mips today, IIRC, so this is kinda moot. > > If ZFS needs this (now or in the future), and there's some atomics that > are possible, but not yet implemented, on these 32-bit platforms, we > should mark ZFS broken on those platforms and let their supporters fill in > the gaps (though it would be nice to ask for help before doing this if its > i386 or armv[67]). > > Warner > > >> > >> > Note that I saw the above access pattern only in the code that is not >> > imported yet. I only suspect that that pattern might already be present >> > in the current ZFS/FreeBSD code given that it uses 64-bit variables and >> > atomics a lot. >> > I am not sure if there is a quick way to check that. Maybe >> > devel/coccinelle could be used for that. >> > >> > -- >> > Andriy Gapon >> > _______________________________________________ >> > freebsd-hackers@freebsd.org mailing list >> > https://lists.freebsd.org/mailman/listinfo/freebsd-hackers >> > To unsubscribe, send any mail to " >> freebsd-hackers-unsubscribe@freebsd.org" >> _______________________________________________ >> freebsd-hackers@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers >> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org >> " >> >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfr-PVGyg6KKWYerWcS3vHUBimkOXJ4Y%2BOWbkn6sZPvncg>