On Thu, Jan 13, 2011 at 18:43, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Jan 13, 2011 at 11:08 AM, Magnus Hagander <magnus@hagander.net> wrote:
>> On Wed, Jan 12, 2011 at 03:03, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Tue, Jan 11, 2011 at 7:24 AM, Magnus Hagander <magnus@hagander.net> wrote:
>>>>> No, do this at top
>>>>>
>>>>> if (walsnd->state == state)
>>>>> return;
>>>>>
>>>>> Keep spinlocks when actually setting it.
>>>
>>> I think this is safe...
>>>
>>>> Aha. Thanks for the pointers, pfa a new version.
>>>
>>> ...but I think you also need to take the spinlock when reading the value.
>>
>> Even when it can only ever be set by one process (the owning
>> walsender), and the variable is atomic (as it should be, since it's a
>> single enum/int)?
>
> The fact that it can only be modified by one process makes it safe for
> *that process* to read it without taking the lock, but another process
> that wants to read it still needs the lock, I believe - otherwise you
> might get a slightly stale value. That's probably not a *huge* deal
> in this case, but I think it'd be better to get it right because
> people tend to copy these sorts of things elsewhere, and it'd be bad
> if it got copied into some place more critical.
ok, thanks for the pointers - fix applied.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
