Date: Fri, 16 Aug 2019 07:32:15 -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: <CANCZdfo8vm3hZxFFe2wvoLKSOp8K38kC%2BKHCVNhv-_Ct3LD0VQ@mail.gmail.com> In-Reply-To: <20190816081406.GM2738@kib.kiev.ua> References: <8bbee981-4f95-22eb-d9ec-00267c8e111d@FreeBSD.org> <20190816081406.GM2738@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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? > 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?CANCZdfo8vm3hZxFFe2wvoLKSOp8K38kC%2BKHCVNhv-_Ct3LD0VQ>