lkml.org 
[lkml]   [2018]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()
    Date
    > > +struct workqueue_struct *efi_rts_wq;
    > > +
    > > static bool disable_runtime;
    > > static int __init setup_noefi(char *arg) { @@ -329,6 +331,19 @@
    > > static int __init efisubsys_init(void)
    > > return 0;
    > >
    > > /*
    > > + * Since we process only one efi_runtime_service() at a time, an
    > > + * ordered workqueue (which creates only one execution context)
    > > + * should suffice all our needs.
    > > + */
    > > + efi_rts_wq = alloc_ordered_workqueue("efi_rts_workqueue", 0);
    >
    > efi_rts_wq or efi_rts_workqueue?
    >
    > > + if (!efi_rts_wq) {
    > > + pr_err("Failed to create efi_rts_workqueue, EFI runtime services "
    >
    > Same here.

    Sure! I will make it consistent with "efi_rts_wq". Just tried to be more verbose
    with names :)

    [...]

    > > +#define efi_queue_work(_rts, _arg1, _arg2, _arg3, _arg4, _arg5) \
    > > +({ \
    > > + struct efi_runtime_work efi_rts_work; \
    > > + \
    > > + INIT_WORK_ONSTACK(&efi_rts_work.work, efi_call_rts); \
    > > + efi_rts_work.func = _rts; \
    > > + efi_rts_work.arg1 = _arg1; \
    > > + efi_rts_work.arg2 = _arg2; \
    > > + efi_rts_work.arg3 = _arg3; \
    > > + efi_rts_work.arg4 = _arg4; \
    > > + efi_rts_work.arg5 = _arg5; \
    > > + /* \
    > > + * queue_work() returns 0 if work was already on queue, \
    > > + * _ideally_ this should never happen. \
    > > + */ \
    > > + if (queue_work(efi_rts_wq, &efi_rts_work.work)) \
    > > + flush_work(&efi_rts_work.work); \
    > > + else \
    > > + BUG(); \
    >
    > Thanks for the change! One remark, I would just do:

    Sorry! but I am planning to remove BUG(). Looks like it could defeat the purpose
    of patch. Please see Boris comments on the other thread.

    [...]

    > > +/*
    > > + * efi_runtime_work: Details of EFI Runtime Service work
    > > + * @func: EFI Runtime Service function identifier
    > > + * @arg<1-5>: EFI Runtime Service function arguments
    > > + * @status: Status of executing EFI Runtime Service
    > > + */
    > > +struct efi_runtime_work {
    > > + u8 func;
    > > + void *arg1;
    > > + void *arg2;
    > > + void *arg3;
    > > + void *arg4;
    > > + void *arg5;
    > > + efi_status_t status;
    > > + struct work_struct work;
    > > +};
    >
    > Why is efi_runtime_work in the .h at all?
    >

    Thanks for the catch. I will move it to runtime-wrappers.c file and will make it
    static too. It isn't being used in any other place.

    > Please CC me for the next version! :-)

    Sure! Sorry for that. I should have done in V2.

    Regards,
    Sai
    \
     
     \ /
      Last update: 2018-03-08 05:23    [W:3.462 / U:0.736 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site