Date: Wed, 22 Jun 2011 09:42:06 -0600 From: Warner Losh <imp@bsdimp.com> To: Attilio Rao <attilio@freebsd.org> Cc: src-committers@freebsd.org, Alan Cox <alc@rice.edu>, Alan Cox <alc@freebsd.org>, svn-src-all@freebsd.org, "Bjoern A. Zeeb" <bz@freebsd.org>, Bruce Evans <brde@optusnet.com.au>, svn-src-head@freebsd.org Subject: Re: svn commit: r223307 - head/sys/vm Message-ID: <82E2B828-97C9-4C35-A619-ACDB5C40E99B@bsdimp.com> In-Reply-To: <BANLkTinZofib5HSjOfeb9qx5QXuUzGonCw@mail.gmail.com> References: <201106191913.p5JJDOqJ006272@svn.freebsd.org> <BBC34F79-FFA7-4A05-83B3-DE17E0AB14D0@FreeBSD.org> <20110622063258.D2275@besplex.bde.org> <BANLkTi=7WnYPQRwE4Hi472DuJz91d1sK=g@mail.gmail.com> <4E0128FF.6020804@rice.edu> <BEDB4F71-8151-4799-9272-8CE79CDA6C17@bsdimp.com> <BANLkTinZofib5HSjOfeb9qx5QXuUzGonCw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jun 22, 2011, at 3:26 AM, Attilio Rao wrote: > 2011/6/22 Warner Losh <imp@bsdimp.com>: >>=20 >> On Jun 21, 2011, at 5:27 PM, Alan Cox wrote: >>=20 >> On 06/21/2011 16:09, Attilio Rao wrote: >>=20 >> 2011/6/21 Bruce Evans<brde@optusnet.com.au>: >>=20 >> On Tue, 21 Jun 2011, Bjoern A. Zeeb wrote: >>=20 >> On Jun 19, 2011, at 7:13 PM, Alan Cox wrote: >>=20 >> Hi Alan, >>=20 >> Author: alc >>=20 >> Date: Sun Jun 19 19:13:24 2011 >>=20 >> New Revision: 223307 >>=20 >> URL: http://svn.freebsd.org/changeset/base/223307 >>=20 >> Log: >>=20 >> Precisely document the synchronization rules for the page's dirty = field. >>=20 >> (Saying that the lock on the object that the page belongs to must be >>=20 >> held >>=20 >> only represents one aspect of the rules.) >>=20 >> Eliminate the use of the page queues lock for atomically performing >>=20 >> read- >>=20 >> modify-write operations on the dirty field when the underlying >>=20 >> architecture >>=20 >> supports atomic operations on char and short types. >>=20 >> Document the fact that 32KB pages aren't really supported. >>=20 >> contrary to the tinderbox I'd like to point out that all mips kernels >>=20 >> built by universe are broken with a SVN HEAD from earlier today. = Could you >>=20 >> please check and see if you can fix it? The errors I get are: >>=20 >> vm_page.o: In function `vm_page_clear_dirty': >>=20 >> /sys/vm/vm_page.c:(.text+0x18d0): undefined reference to = `atomic_clear_8' >>=20 >> /sys/vm/vm_page.c:(.text+0x18d0): relocation truncated to fit: = R_MIPS_26 >>=20 >> against `atomic_clear_8' >>=20 >> vm_page.o: In function `vm_page_set_validclean': >>=20 >> /sys/vm/vm_page.c:(.text+0x38f0): undefined reference to = `atomic_clear_8' >>=20 >> /sys/vm/vm_page.c:(.text+0x38f0): relocation truncated to fit: = R_MIPS_26 >>=20 >> against `atomic_clear_8' >>=20 >> Atomic types shorter than int cannot be used in MI code, since they = might >>=20 >> not exist. Apparently they don't exist on mips. jake@ fixed all = their >>=20 >> old uses for sparc4 in ~Y2K. >>=20 >> I'm sure they do, they exist in support.S though and may not have the >>=20 >> _8 form (they may just have the _char version). I may look at the = code >>=20 >> again to be sure. >>=20 >>=20 >> It appears that while mips/include/atomic.h declares the existence of >> atomic_clear_8, mips/mips/support.S doesn't implement it. In other = words, >> only support for int's and short's is currently implemented, not = char's: >>=20 >> # grep atomic_clear mips/mips/support.S >> * atomic_clear_32(u_int32_t *a, u_int32_t b) >> LEAF(atomic_clear_32) >> END(atomic_clear_32) >> * atomic_clear_16(u_int16_t *a, u_int16_t b) >> LEAF(atomic_clear_16) >> END(atomic_clear_16) >>=20 >> The current crop of atomic*16 and atomic*8 functions have the = restriction >> that the address must be 32-bit aligned (and it forces this by = aligning to >> 32-bits silently and then operates on the low 8 or 16 bits in that = word!) >> I'm guessing that this is likely just wrong. Comments? >> Warner >=20 > That is wrong, of course, and my personal opinion is that one should > not implement atomic operations if they cannot be done efficiently > (example: if you need to disable interrupts or similar expensive > operation just to assure atomicity of operation, just don't support > it) as long as not having _8/_char is perfectly fine. I think it can be efficient, for some reasonable definition of = efficient. The masking and shifting operations aren't that onerous to = write and the instructions cycles would be dwarfed by the ll/sc pair, = which are required to implement atomic. The issue is that the code today is clearly wrong and needs to be fixed. Warner=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?82E2B828-97C9-4C35-A619-ACDB5C40E99B>