Re: [RFC PATCH] RFC: Sketch of dm event notification

Simon Glass
2021-12-07T15:08:07Z

Hi Tahahiro,

On Wed, 24 Nov 2021 at 19:57, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Hi Simon,
>
> On Wed, Nov 24, 2021 at 05:12:45PM -0700, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Tue, 16 Nov 2021 at 20:03, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > On Tue, Nov 16, 2021 at 07:48:59PM -0700, Simon Glass wrote:
> > > > Hi Takahiro,
> > > >
> > > > On Tue, 16 Nov 2021 at 19:31, AKASHI Takahiro
> > > > <takahiro.akashi@linaro.org> wrote:
> > > > >
> > > > > Simon,
> > > > >
> > > > > On Mon, Nov 01, 2021 at 03:41:50PM +0900, AKASHI Takahiro wrote:
> > > > > > Hello Simon,
> > > > > >
> > > > > > On Sun, Oct 31, 2021 at 09:07:01PM -0600, Simon Glass wrote:
> > > > > > > This is a patch for Takahiro to look at, just for illustration. Not ready
> > > > > > > for review.
> > > > > >
> > > > > > Thank you for posting the draft.
> > > > > > At a first glance, it looks good and I don't see any specific issue
> > > > > > with your implementation.
> > > > >
> > > > > I said OK and functionally it should work well, but I now have
> > > > > some concerns:
> > > > > 1) In my current implementation, I use post_probe/pre_remove hooks
> > > > > of BLK device to invoke efi callback functions. In this sense,
> > > > > event(POST_PROBE/ PRE_REMOVE) seems to be a duplicated feature
> > > > > in some way.
> > > >
> > > > We should not be calling EFI functions from a BLK devince, unless it
> > > > is an EFI block device, i.e. we should provide hooks for EFI or any
> > > > other interested party to use.
> > >
> > > Even for now, part_init() is called in post_probe() (and
> > > blk_create_partitions(), which is to create another type of
> > > uclass device, in my patch as well).
> > > So I'm not sure what functions can and should be called
> > > and what not in those hooks.
> > >
> > > Do you want to convert part_init() to an event subscriber
> > > along with my efi hooks?
> >
> > You could do. From my reading of the code it makes sense to do that.
>
> Ok, but I will put this change on hold for now.
>
> One of problems with the proposed framework of event notification
> is that we cannot deterministically specify the order of callback
> functions when we want to add callbacks for a number of subsystems
> to a particular event.
> For instance, we have callbacks for efi_disk creations as well as
> partitions (part_init() & blk_create_partitions()) at POST_PROBE of
> BLK devices, they should (logically) be called in this order but
> currently no definite guarantee.
> If we utilise a callback to create efi_disk's for "partition" devices
> as well, this situation can get more complicated.
> # Why? If the order of invocations is the other way around, efi_disks
> # for partitions might be created before an efi_disk for a whole disk.

Yes I was wondering if that would cause problems.

We also need to think about discoverability. It needs to be *very
easy* to see a list of callbacks for an event a priori and we don't
want debugging to be a pain.

I'll take a look at adding priority and some commands to list the callbacks.

Regards,
Simon