Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 11 Aug 2020 14:31:26 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        Emmanuel Vadot <manu@bidouilliste.com>
Cc:        Alan Cox <alc@rice.edu>, Emmanuel Vadot <manu@freebsd.org>, src-committers@freebsd.org,  svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r363842 - head/sys/compat/linuxkpi/common/include/linux
Message-ID:  <CAGudoHHJ7mj3td4jD2SNABANoAsSwzKrOM18WE-zhoV0sNWbiQ@mail.gmail.com>
In-Reply-To: <20200804202541.0b6b3eb6ce26522ee12a796c@bidouilliste.com>
References:  <202008041525.074FPNLh043749@repo.freebsd.org> <1c90911b-d5f6-f8b1-8f4d-97a88fa028bd@rice.edu> <20200804202541.0b6b3eb6ce26522ee12a796c@bidouilliste.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi. This remains unfixed.

On 8/4/20, Emmanuel Vadot <manu@bidouilliste.com> wrote:
> On Tue, 4 Aug 2020 13:11:02 -0500
> Alan Cox <alc@rice.edu> wrote:
>
>>
>> On 8/4/20 10:25 AM, Emmanuel Vadot wrote:
>> > Author: manu
>> > Date: Tue Aug  4 15:25:22 2020
>> > New Revision: 363842
>> > URL: https://svnweb.freebsd.org/changeset/base/363842
>> >
>> > Log:
>> >    linuxkpi: Add clear_bit_unlock
>> >
>> >    This calls clear_bit and adds a memory barrier.
>> >
>> >    Sponsored by: The FreeBSD Foundation
>> >
>> >    Reviewed by:	hselasky
>> >    MFC after:	1 week
>> >    Differential Revision:	https://reviews.freebsd.org/D25943
>> >
>> > Modified:
>> >    head/sys/compat/linuxkpi/common/include/linux/bitops.h
>> >
>> > Modified: head/sys/compat/linuxkpi/common/include/linux/bitops.h
>> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
>> > --- head/sys/compat/linuxkpi/common/include/linux/bitops.h	Tue Aug  4
>> > 15:00:02 2020	(r363841)
>> > +++ head/sys/compat/linuxkpi/common/include/linux/bitops.h	Tue Aug  4
>> > 15:25:22 2020	(r363842)
>> > @@ -275,6 +275,13 @@ find_next_zero_bit(const unsigned long *addr,
>> > unsigned
>> >   #define	test_bit(i, a)							\
>> >       !!(READ_ONCE(((volatile const unsigned long *)(a))[BIT_WORD(i)])=
 &
>> > BIT_MASK(i))
>> >
>> > +static inline void
>> > +clear_bit_unlock(long bit, volatile unsigned long *var)
>> > +{
>> > +	clear_bit(bit, var);
>> > +	wmb();
>>
>>
>> For an unlock operation, the memory barrier should come before the
>> clear_bit() call, not after.=C2=A0 See, for example, the alpha implement=
ation
>> in Linux.=C2=A0 Also, the correct "spelling" for this memory barrier in
>> FreeBSD would be atomic_thread_fence_rel(). See, for example, the
>> comment at the top of sys/amd64/include/atomic.h.
>
>  Ah yes, thanks. I probably got lost looking for the linux implem but
> that does make sense, I'll fix that probably tomorow.
>
>  Thanks.
>
>>
>> > +}
>> > +
>> >   static inline int
>> >   test_and_clear_bit(long bit, volatile unsigned long *var)
>> >   {
>
>
> --
> Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
>


--=20
Mateusz Guzik <mjguzik gmail.com>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHHJ7mj3td4jD2SNABANoAsSwzKrOM18WE-zhoV0sNWbiQ>