On Mon, 2018-01-15 at 20:17 +0800, Ming Lei wrote:
> On Sun, Jan 14, 2018 at 06:40:40PM -0500, Laurence Oberman wrote:
> > On Thu, 2018-01-04 at 14:32 -0800, Vinson Lee wrote:
> > > Hi.
> > > 
> > > HP ProLiant DL360p Gen8 with Smart Array P420i boots to the login
> > > prompt and hangs with Linux 4.13 or later. I cannot log in on
> > > console
> > > or SSH into the machine. Linux 4.12 and older boot fine.
> > > 
> > > 
> > 
> > ...
> > 
> > ...
> > 
> > This issue bit me for for two straight days.
> > I was testing Mike Snitzers combined tree and this commit crept
> > into
> > the latest combined tree.
> > 
> > commit 84676c1f21e8ff54befe985f4f14dc1edc10046b
> > Author: Christoph Hellwig <h...@lst.de>
> > Date:   Fri Jan 12 10:53:05 2018 +0800
> > 
> >     genirq/affinity: assign vectors to all possible CPUs
> >    
> >     Currently we assign managed interrupt vectors to all present
> > CPUs.  This
> >     works fine for systems were we only online/offline CPUs.  But
> > in
> > case of
> >     systems that support physical CPU hotplug (or the virtualized
> > version of
> >     it) this means the additional CPUs covered for in the ACPI
> > tables
> > or on
> >     the command line are not catered for.  To fix this we'd either
> > need
> > to
> >     introduce new hotplug CPU states just for this case, or we can
> > start
> >     assining vectors to possible but not present CPUs.
> >    
> >     Reported-by: Christian Borntraeger <borntrae...@de.ibm.com>
> >     Tested-by: Christian Borntraeger <borntrae...@de.ibm.com>
> >     Tested-by: Stefan Haberland <s...@linux.vnet.ibm.com>
> >     Fixes: 4b855ad37194 ("blk-mq: Create hctx for each present
> > CPU")
> >     Cc: linux-kernel@vger.kernel.org
> >     Cc: Thomas Gleixner <t...@linutronix.de>
> >     Signed-off-by: Christoph Hellwig <h...@lst.de>
> >     Signed-off-by: Jens Axboe <ax...@kernel.dk>
> > 
> > Reason I never thought about this being my reason for the latest
> > hang
> > is I have used Linus' tree all the way to 4.15-rc7 with no issues.
> > 
> > Vinson reporting it against 4.13 or later was not making sense
> > because
> > I had not seen the hang until this weekend.
> > 
> > I checked  and its in Linus's tree but its not an issue in the
> > generic
> > 4.15-rc7 for me.
> 
> Hi Laurence,
> 
> Wrt. your issue, I have investigated a bit and found that it is
> because
> one irq vector may be assigned to all offline CPUs, and it may not be
> same with Vinson's.
> 
> And the following patch can address your issue, I may prepare a
> formal
> version if no one objects this approach.
> 
> Thomas, Christoph, could you take a look this patch?
> 
> ---
>  kernel/irq/affinity.c | 69 +++++++++++++++++++++++++++++++++++----
> ------------
>  1 file changed, 47 insertions(+), 22 deletions(-)
> 
> diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c
> index a37a3b4b6342..dfc1f6a9c488 100644
> --- a/kernel/irq/affinity.c
> +++ b/kernel/irq/affinity.c
> @@ -94,6 +94,39 @@ static int get_nodes_in_cpumask(cpumask_var_t
> *node_to_possible_cpumask,
>       return nodes;
>  }
>  
> +/*
> + * Spread the affinity of @nmsk into @nr_vecs irq vectors, and the
> + * result is stored to @start_irqmsk.
> + */
> +static int irq_vecs_spread_affinity(struct cpumask *irqmsk,
> +                                 int max_irqmsks,
> +                                 struct cpumask *nmsk,
> +                                 int max_ncpus)
> +{
> +     int v, ncpus;
> +     int vecs_to_assign, extra_vecs;
> +
> +     /* Calculate the number of cpus per vector */
> +     ncpus = cpumask_weight(nmsk);
> +     vecs_to_assign = min(max_ncpus, ncpus);
> +
> +     /* Account for rounding errors */
> +     extra_vecs = ncpus - vecs_to_assign * (ncpus /
> vecs_to_assign);
> +
> +     for (v = 0; v < min(max_irqmsks, vecs_to_assign); v++) {
> +             int cpus_per_vec = ncpus / vecs_to_assign;
> +
> +             /* Account for extra vectors to compensate rounding
> errors */
> +             if (extra_vecs) {
> +                     cpus_per_vec++;
> +                     --extra_vecs;
> +             }
> +             irq_spread_init_one(irqmsk + v, nmsk, cpus_per_vec);
> +     }
> +
> +     return v;
> +}
> +
>  /**
>   * irq_create_affinity_masks - Create affinity masks for multiqueue
> spreading
>   * @nvecs:   The total number of vectors
> @@ -104,7 +137,7 @@ static int get_nodes_in_cpumask(cpumask_var_t
> *node_to_possible_cpumask,
>  struct cpumask *
>  irq_create_affinity_masks(int nvecs, const struct irq_affinity
> *affd)
>  {
> -     int n, nodes, cpus_per_vec, extra_vecs, curvec;
> +     int n, nodes, curvec;
>       int affv = nvecs - affd->pre_vectors - affd->post_vectors;
>       int last_affv = affv + affd->pre_vectors;
>       nodemask_t nodemsk = NODE_MASK_NONE;
> @@ -154,33 +187,25 @@ irq_create_affinity_masks(int nvecs, const
> struct irq_affinity *affd)
>       }
>  
>       for_each_node_mask(n, nodemsk) {
> -             int ncpus, v, vecs_to_assign, vecs_per_node;
> +             int vecs_per_node;
>  
>               /* Spread the vectors per node */
>               vecs_per_node = (affv - (curvec - affd-
> >pre_vectors)) / nodes;
>  
> -             /* Get the cpus on this node which are in the mask
> */
> -             cpumask_and(nmsk, cpu_possible_mask,
> node_to_possible_cpumask[n]);
>  
> -             /* Calculate the number of cpus per vector */
> -             ncpus = cpumask_weight(nmsk);
> -             vecs_to_assign = min(vecs_per_node, ncpus);
> -
> -             /* Account for rounding errors */
> -             extra_vecs = ncpus - vecs_to_assign * (ncpus /
> vecs_to_assign);
> -
> -             for (v = 0; curvec < last_affv && v <
> vecs_to_assign;
> -                  curvec++, v++) {
> -                     cpus_per_vec = ncpus / vecs_to_assign;
> -
> -                     /* Account for extra vectors to compensate
> rounding errors */
> -                     if (extra_vecs) {
> -                             cpus_per_vec++;
> -                             --extra_vecs;
> -                     }
> -                     irq_spread_init_one(masks + curvec, nmsk,
> cpus_per_vec);
> -             }
> +             /* spread non-online possible cpus */
> +             cpumask_andnot(nmsk, node_to_possible_cpumask[n],
> cpu_online_mask);
> +             irq_vecs_spread_affinity(&masks[curvec], last_affv -
> curvec,
> +                                      nmsk, vecs_per_node);
>  
> +             /*
> +              * spread online possible cpus to make sure each
> vector
> +              * can get one online cpu to handle
> +              */
> +             cpumask_and(nmsk, node_to_possible_cpumask[n],
> cpu_online_mask);
> +             curvec += irq_vecs_spread_affinity(&masks[curvec],
> +                                                last_affv -
> curvec,
> +                                                nmsk,
> vecs_per_node);
>               if (curvec >= last_affv)
>                       break;
>               --nodes;
> -- 
> 2.9.5
> 
> 

Hello Ming

I will test the patch. I did not spend a lot of time seeing if this
weekends stalls were an exact match to Vinson, I just knew pulling that
patch resolved it.
Perhaps this explains why I was not seeing this on generic 4.15-rc7.

Thanks
Laurence

Reply via email to