lkml.org 
[lkml]   [2024]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 1/2] x86/resctrl: Pass domain to target CPU
    From
    Hi Tony,

    On 2/22/2024 10:50 AM, Tony Luck wrote:
    > reset_all_ctrls() and resctrl_arch_update_domains() use on_each_cpu_mask()
    > to call rdt_ctrl_update() on potentially one CPU from each domain.
    >
    > But this means rdt_ctrl_update() needs to figure out which domain to apply
    > changes to. Doing so requires a search of all domains in a resource,
    > which can only be done safely if cpus_lock is held. Both callers do
    > hold this lock, but there isn't a way for a function called on another
    > CPU via IPI to verify this.
    >
    > Commit c0d848fcb09d ("x86/resctrl: Remove lockdep annotation that triggers
    > false positive") removed the incorrect assertions.
    >
    > Adding the target domain to the msr_param structure, and calling
    > for each domain separately using smp_call_function_single() means
    > that rdt_ctrl_update() doesn't need to search for the domain. Thus
    > get_domain_from_cpu() can safely assert that the cpus_lock is held since
    > the remaining callers do not use IPI.

    Please stick to the imperative tone. Something like (please feel free to
    improve):

    Add the target domain to the msr_param structure and call
    rdt_ctrl_update() for each domain separately using
    smp_call_function_single(). This means that rdt_ctrl_update()
    doesn't need to search for the domain and get_domain_from_cpu()
    can safely assert that the cpus_lock is held since
    the remaining callers do not use IPI.


    ..

    > @@ -463,6 +457,8 @@ static int domain_setup_ctrlval(struct rdt_resource *r, struct rdt_domain *d)
    > hw_dom->ctrl_val = dc;
    > setup_default_ctrlval(r, dc);
    >
    > + m.res = r;

    This belongs in the next patch.

    > + m.dom = d;
    > m.low = 0;
    > m.high = hw_res->num_closid;
    > hw_res->msr_update(d, &m, r);

    The rest looks good to me and I think it is a good improvement.
    Thank you very much.

    Reinette


    \
     
     \ /
      Last update: 2024-05-27 15:25    [W:4.138 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site