EPICS Controls Argonne National Laboratory

Experimental Physics and
Industrial Control System

2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  <20192020  2021  2022  2023  2024  Index 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  <20192020  2021  2022  2023  2024 
<== Date ==> <== Thread ==>

Subject: Re: pvPutLog?
From: Michael Davidsaver via Core-talk <[email protected]>
To: "Zimoch Dirk (PSI)" <[email protected]>, "[email protected]" <[email protected]>
Cc: "[email protected]" <[email protected]>
Date: Tue, 3 Sep 2019 19:45:01 -0700
On 9/3/19 1:01 AM, Zimoch Dirk (PSI) via Core-talk wrote:
> On Mon, 2019-09-02 at 08:57 -0700, Michael Davidsaver wrote:
>> The main concern I see is locking and callbacks.  Current calls to asTrap*() happen
>> without holding the record lock.  The asTrap* functions can't be moved entirely into
>> dbChannelPut() without changing this.
>>
>> This concern is also why I can't trivially add asTrap*() calls to QSRV.  The PVA to DBR
>> data type conversion is currently happening under the record lock.  (Not required,
>> just convenient)
>>
>> If this were to change then some answer/plan would need to be made to address the
>> possibility of deadlocks and slowdown.  Maybe it is enough to say that asTrap handlers
>> should never be locking records or blocking?  
>>
>> Also, I hadn't noticed asTrapWritePvt::lock before.  So many singleton locks...
> 
> In the context of caPutLog that means to read the value of the record without a lock.
> That may lead to inconsistent data, in particular in the case of arrays, but for some architectures maybe even for doubles.
> Extended to PVA, structures may be read inconsistently too.
> However caPutLog does not read the full arrays anyway.

https://github.com/epics-modules/caPutLog/blob/b544f92c6efe1df98506b567f3f7e5137965e2a4/caPutLogApp/caPutLogAs.c#L104

dbGetField() is used, which of course calls dbScanLock().
This is done twice, for the "before" and "after" values.
So 'pmessage->data' is not used.

I guess I can just pass data=NULL or a dummy value.

This seems like a rather significant design failure for the dbTrap*() API
that the first/primary use case has to do this extra work.

> The original purpose of asTrap, deciding if writing is allowed, would probably not need to read the record at all.

We need to distinguish between asCheckPut() and asTrapWriteBeforeWithData()/asTrapWriteAfterWrite().
The former is used in access control decisions, the later two are called
for permitted writes to (maybe) log.

QSRV now calls asCheckPut(), which is low impact (just tests an integer).
It does not call asTrap*() yet.

In looking at your iocLogClientFixes changes I notice that the send() on
the log socket is (and has been) called during asTrapWriteAfterWrite().
So a flood of log messages can hold up processing until the log server
has consumed them all.

For some reason (hopeful nativity?) I thought this was happening on a worker thread.


>>
>>
>> On 9/2/19 4:39 AM, Johnson, Andrew N. wrote:
>>> I don’t think that would be trivial even for just the Trap Write calls, the dbChannel layer doesn’t have access to the asClientPVT context pointer. For the other interfaces dbChannel doesn’t know anything about the client that is requesting access. I suspect the current arrangement may be the simplest, but I haven’t looked at it too closely.
>>>
>>> - Andrew
>>>
>>>
>>>> On Sep 2, 2019, at 9:08 AM, Zimoch Dirk (PSI) via Core-talk <[email protected]> wrote:
>>>>
>>>> On Mon, 2019-09-02 at 07:00 +0000, Zimoch Dirk (PSI) via Core-talk wrote:
>>>>> Shouldn't PVA be implemented one level lower in dbChannel instead of in each network protocol?
>>>>
>>>> Sorry typo, I meant: Shouldn't access security be implemented in dbChannel?
>>>> Dirk
>>
>>


Replies:
Re: pvPutLog? Zimoch Dirk (PSI) via Core-talk
Re: pvPutLog? Johnson, Andrew N. via Core-talk
References:
pvPutLog? Timo Korhonen via Core-talk
Re: pvPutLog? Zimoch Dirk (PSI) via Core-talk
Re: pvPutLog? Michael Davidsaver via Core-talk
Re: pvPutLog? Zimoch Dirk (PSI) via Core-talk
Re: pvPutLog? Michael Davidsaver via Core-talk
Re: pvPutLog? Timo Korhonen via Core-talk
Re: pvPutLog? Michael Davidsaver via Core-talk
Re: pvPutLog? Zimoch Dirk (PSI) via Core-talk
Re: pvPutLog? Zimoch Dirk (PSI) via Core-talk
Re: pvPutLog? Johnson, Andrew N. via Core-talk
Re: pvPutLog? Michael Davidsaver via Core-talk
Re: pvPutLog? Zimoch Dirk (PSI) via Core-talk

Navigate by Date:
Prev: [Bug 1841634] Re: CP link triggers lost when record is async mdavidsaver via Core-talk
Next: Re: [Merge] ~dirk.zimoch/epics-base:iocLogClientFixes into epics-base:7.0 mdavidsaver via Core-talk
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  <20192020  2021  2022  2023  2024 
Navigate by Thread:
Prev: Re: pvPutLog? Zimoch Dirk (PSI) via Core-talk
Next: Re: pvPutLog? Zimoch Dirk (PSI) via Core-talk
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  <20192020  2021  2022  2023  2024 
ANJ, 06 Sep 2019 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· Search · EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·