lkml.org 
[lkml]   [2018]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
    Date


    > -----Original Message-----
    > From: Thomas Gleixner <tglx@linutronix.de>
    > Sent: Thursday, April 26, 2018 3:17 PM
    > To: KY Srinivasan <kys@microsoft.com>
    > Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
    > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
    > apw@canonical.com; jasowang@redhat.com; hpa@zytor.com; Stephen
    > Hemminger <sthemmin@microsoft.com>; Michael Kelley (EOSG)
    > <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
    > Subject: Re: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
    >
    > On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
    > >
    > > +struct ipi_arg_ex {
    > > + u32 vector;
    > > + u32 reserved;
    > > + struct hv_vpset vp_set;
    >
    > Please align that in tabular fashion for easy of reading
    >
    > u32 vector;
    > u32 reserved;
    > struct hv_vpset vp_set;
    >
    > > +};
    > > +
    > > static struct apic orig_apic;
    > >
    > > static u64 hv_apic_icr_read(void)
    > > @@ -97,6 +103,40 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
    > > * IPI implementation on Hyper-V.
    > > */
    > >
    > > +static int __send_ipi_mask_ex(const struct cpumask *mask, int vector)
    > > +{
    > > + int nr_bank = 0;
    > > + struct ipi_arg_ex **arg;
    > > + struct ipi_arg_ex *ipi_arg;
    > > + int ret = 1;
    > > + unsigned long flags;
    >
    > This is really horrible to read.
    >
    > struct ipi_arg_ex *ipi_arg;
    > struct ipi_arg_ex **arg;
    > unsigned long flags;
    > bool ret = false;
    > int nr_bank = 0;
    >
    > is really more conveniant for quick reading.
    >
    > So the other more limited function has a lot more sanity checks vs. vector
    > number and other things. Why are they not required here? Comment
    > please.

    Yes, I will add the comments. This function is called from the other function
    after all the sanity checks have been done and hence are not replicated here.
    >
    > > + local_irq_save(flags);
    > > + arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
    > > +
    > > + ipi_arg = *arg;
    > > + if (unlikely(!ipi_arg))
    > > + goto ipi_mask_ex_done;
    > > +
    > > + ipi_arg->vector = vector;
    > > + ipi_arg->reserved = 0;
    > > + ipi_arg->vp_set.valid_bank_mask = 0;
    > > +
    > > + if (!cpumask_equal(mask, cpu_present_mask)) {
    > > + ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K;
    > > + nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
    >
    > nr_bank really confused me. bank_nr is what you mean, not number of
    > banks,
    > right?
    It is the number of banks. The hypercall used here is a variable length
    hypercall.

    Regards,

    K. Y

    \
     
     \ /
      Last update: 2018-04-27 08:25    [W:5.718 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site