On Wed, Nov 25, 2020 at 06:34:48PM +0300, Anastasia Lubennikova wrote:
> I took a look at the patch. It looks fine and I see, that it contains fixes
> for the latest questions in the thread.
>
> I think we should provide a test module for this feature, that will serve as
> both test and example of the use.
Yep. That's missing. The trick here is usually to be able to come up
with something minimalist still useful enough for two reasons:
- This can be used as a template.
- This makes sure that the API work.
As the patch stands, nothing in this architecture is tested.
> This is a feature for extension developers, so I don't know where we should
> document it. At the very least we can improve comments. For example,
> describe the fact that custom signals are handled after receiving SIGUSR1.
As you say, this is for extension developers, so this should be
documented in the headers defining those APIs. FWIW, I am -1 with the
patch as proposed. First, it has zero documentation. Second, it uses
a hardcoded custom number of signals of 64 instead of a flexible
design. In most cases, 64 is a waste. In some cases, 64 may not be
enough. IMO, a design based on the registration of a custom
procsignal done at shmem init time would be much more flexible. You
need one registration API and something to get an ID associated to the
custom entry, and that's roughly what Teodor tells upthread.
This needs more work, so I am marking it as returned with feedback.
--
Michael
