From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: "rtdm/nrtsig: move inband work description off the stack" References: <2098c9e4-4fb1-ef49-2c2d-d2c6573499b5@siemens.com> <87lf83cj7k.fsf@xenomai.org> <14ad027b-f326-5ea8-9cce-cb4b82984a23@siemens.com> <87y2c3b092.fsf@xenomai.org> <3bf57e72-9992-0ab4-a956-d6f6197bfe24@siemens.com> <87sg2bat1l.fsf@xenomai.org> From: Jan Kiszka Message-ID: Date: Tue, 25 May 2021 14:56:44 +0200 MIME-Version: 1.0 In-Reply-To: <87sg2bat1l.fsf@xenomai.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit List-Id: Discussions about the Xenomai project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Philippe Gerum Cc: Xenomai On 25.05.21 14:37, Philippe Gerum wrote: > > Jan Kiszka writes: > >> On 25.05.21 12:01, Philippe Gerum wrote: >>> >>> Jan Kiszka writes: >>> >>>> On 25.05.21 10:46, Philippe Gerum wrote: >>>>> >>>>> Jan Kiszka writes: >>>>> >>>>>> Hi Philippe, >>>>>> >>>>>> [1] makes rtdm_schedule_nrt_work a rather heavyweight service, compared >>>>>> to what it was (and even over I-pipe). xnmalloc is nothing you would >>>>>> expect from a "send a signal" service, specifically from >>>>>> rtdm_nrtsig_pend which does not even make use of the sending extra payload. >>>>>> >>>>>> Can we do better? Also for xnthread_signal (fd and udd usecases are not >>>>>> time-sensitive anyway). >>>>>> >>>>> >>>>> Nope, I cannot see any significantly better option which would still >>>>> allow a common implementation we can map on top of Dovetail / I-pipe. >>>>> >>>> >>>> E.g. pre-allocate the work object for data-free signals and use that - >>>> or not send it when it is in use, thus pending. >>>> >>> >>> Ok, let's recap: >>> >>> - rtdm_nrtsig_pend(): does not allocate anything, merely uses a >>> user-provided memory block, containing a request header. Current >>> callers should either already care for not resending a request >>> uselessly because it is pending (e.g. [1]), or their internal logic >>> should prevent that (e.g. [2]). This interface always convey user data >>> by reference. >> >> Ah, I fantasized that rtdm_nrtsig_pend would call rtdm_schedule_nrt_work >> to do the work, it's the other way around. False alarm here. >> >>> >>> - rtdm_schedule_nrt_work(): does allocate a nrtsig_t struct in order to >>> convey a request block we cannot squeeze into a work_struct, since the >>> latter is in itself an anchor type the user may embed into its own >>> private argument block. I'm unsure ensuring that >>> rtdm_schedule_nrt_work() calls do not pile up would be worth it, as >>> this call is not supposed to be used frenetically on some hot path in >>> the first place. But if we'd want to do that, then we would have to >>> change the signature of rtdm_schedule_nrt_work(), so that we gain a >>> persistent context data we could use for determining whether a nrt >>> work request is in flight. We could not probe the work_struct for that >>> purpose, that would be racy. >>> >>> Do you see any way to have a smarter rtdm_schedule_nrt_work() without >>> changing its signature? >> >> There is no rule that prevents changing the signature if that is needed. >> However, the kernel is fine without allocation as well, using a very >> similar signature. >> > > schedule_work() and rtdm_schedule_nrt_work() have a major difference: > the latter has to pass on the request from the oob to the in-band > context. This is what bugs us and creates the requirement for additional > mechanism and data. > >> I do not yet understand way we need that indirection via the rtdm_nrtsig >> on Dovetail. I thought we can trigger work directly from the oob domain >> there, can't we? How do you handle such a case in evl so far? >> > > Dovetail can trigger in-band work from oob via the generic irq_work() > service, we don't need the extra code involved in the I-pipe for the > same purpose. The gist of the matter is about having the same > implementation for rtdm_schedule_nrt_work() which works in both Dovetail > and I-pipe contexts: the way we do that is by using rtdm_nrtsig_pend() > which already abstracts the base mechanism for relaying oob -> in-band > signals. > > On the other hand, EVL has the evl_call_inband[_from]() service, which > combines the irq_work and work_struct anchors we need into a single > evl_work type, which in turn can be embedded into a user request > block. This is what rtdm_schedule_nrt_work() does not expose, so it has > to build one internally by combining a dynamic allocation and the > user-provided work_struct. > Then enhance the rtdm service to enable such a combination - and be even more ready for the switch to Xenomai 4, I would say. We can either add a new service and deprecate the old one (while implementing it as you suggest) or just break the interface to get the (presumably) few users quickly converted. >> And the third case remains xnthread_signal, btw. >> > > xnthread_signal() is used to trigger SIGDEBUG/SIGSHADOW/SIGTERM signals > for a thread, a seldom event. Optimizing there would be overkill, unless > the application behaves wrongly in the first place. Some caller are under nklock, thus it makes potentially lengthy critical sections now a bit longer. Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux