Hi. I noticed that when non-64bit variable is given as a second argument to atomic_add_64() function, the result is invalid. I found few places where such situation occurs. I wonder how this got unnoticed with ztest, which fails on me within a few seconds (after I started to use Solaris atomic operations) on assertions. Maybe this only doesn''t work when compiled with gcc? Not sure, but most of the time 64bit variables are used properly. Anyway, the patch is here: http://people.freebsd.org/~pjd/opensolaris/10.patch -- Pawel Jakub Dawidek http://www.wheel.pl pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20070218/d0445b11/attachment.bin>
On Sunday 18 February 2007 01:41, Pawel Jakub Dawidek wrote:> I found few places where such situation occurs. I wonder how this got > unnoticed with ztest, which fails on me within a few seconds (after I > started to use Solaris atomic operations) on assertions. Maybe this > only doesn''t work when compiled with gcc? Not sure, but most of the time > 64bit variables are used properly.Hi Pawel, I''ve been using the Solaris assembly code for the atomic operation since the beginning, but lately zfs-fuse has been failing with assertions in that exact same piece of code of your patch: lib/libzpool/build-kernel/arc.c:736: arc_change_state: Assertion `new_state->size + to_delta >= new_state->lsize (0x1194000 >= 0x11a8000)` failed. Were you hitting this bug? Unfortunately your patch didn''t fix this :( I''m having trouble figuring out this problem.. Thanks.
On Mon, Feb 19, 2007 at 05:15:08AM +0000, Ricardo Correia wrote:> On Sunday 18 February 2007 01:41, Pawel Jakub Dawidek wrote: > > I found few places where such situation occurs. I wonder how this got > > unnoticed with ztest, which fails on me within a few seconds (after I > > started to use Solaris atomic operations) on assertions. Maybe this > > only doesn''t work when compiled with gcc? Not sure, but most of the time > > 64bit variables are used properly. > > Hi Pawel, > > I''ve been using the Solaris assembly code for the atomic operation since the > beginning, but lately zfs-fuse has been failing with assertions in that exact > same piece of code of your patch: > > lib/libzpool/build-kernel/arc.c:736: arc_change_state: Assertion > `new_state->size + to_delta >= new_state->lsize (0x1194000 >= 0x11a8000)` > failed. > > Were you hitting this bug?I was hitting one of those assertions, not sure which exactly.> Unfortunately your patch didn''t fix this :(Your numbers looks sane, so you most probably have race somewhere else. I got impossible values in my assertions.> I''m having trouble figuring out this problem..My first guess was that there are many assertions following atomic operations, like the one you''re having problem with: atomic_add_64(&new_state->arcs_lsize, to_delta); ASSERT3U(new_state->arcs_size + to_delta, >=, new_state->arcs_lsize); I thought that there could a race between modify and check, but those assertions are race-free AFAICT. -- Pawel Jakub Dawidek http://www.wheel.pl pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20070219/900f6949/attachment.bin>
On Feb 17, 2007, at 5:41 PM, Pawel Jakub Dawidek wrote:> Hi. > > I noticed that when non-64bit variable is given as a second > argument to > atomic_add_64() function, the result is invalid. > > I found few places where such situation occurs. I wonder how this got > unnoticed with ztest, which fails on me within a few seconds (after I > started to use Solaris atomic operations) on assertions. Maybe this > only doesn''t work when compiled with gcc? Not sure, but most of the > time > 64bit variables are used properly. > > Anyway, the patch is here: > > http://people.freebsd.org/~pjd/opensolaris/10.patchthanks for the code diffs Pawel, i filed the following to cover this: 6526874 ARC is using ints instead of uint64s in some places eric