On 11/19/2019 9:17 AM, James Smart wrote:
On 11/18/2019 12:21 PM, Hannes Reinecke wrote:
On 11/18/19 8:28 PM, James Smart wrote:
On 11/18/2019 4:30 AM, Hannes Reinecke wrote:
The lpfc driver allocates a cpu_map based on the number of possible
cpus during startup. If a CPU hotplug occurs the number of CPUs
might change, causing an out-of-bounds access when trying to lookup
the hardware index for a given CPU.
Suggested-by: Daniel Wagner <daniel.wagner@xxxxxxxx>
Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
---
drivers/scsi/lpfc/lpfc_scsi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/lpfc/lpfc_scsi.c
b/drivers/scsi/lpfc/lpfc_scsi.c
index ba26df90a36a..2380452a8efd 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -642,7 +642,8 @@ lpfc_get_scsi_buf_s4(struct lpfc_hba *phba,
struct lpfc_nodelist *ndlp,
int tag;
struct fcp_cmd_rsp_buf *tmp = NULL;
- cpu = raw_smp_processor_id();
+ cpu = min_t(u32, raw_smp_processor_id(),
+ phba->sli4_hba.num_possible_cpu);
if (cmnd && phba->cfg_fcp_io_sched == LPFC_FCP_SCHED_BY_HDWQ) {
tag = blk_mq_unique_tag(cmnd->request);
idx = blk_mq_unique_tag_to_hwq(tag);
This should be unnecessary with the lpfc 12.6.0.1 and 12.6.0.2
patches that tie into cpu onling/offlining and cpu hot add.
I am curious, how if a cpu is hot removed - how num_possible
dynamically changes (to a lower value ?) such that a thread can be
running on a cpu that returns a higher cpu number than num_possible ?
Actually, the behaviour of num_possible_cpus() under cpu hotplug
seems to be implementation-specific.
One might want to set it to the max value at all times, which has the
drawback that you'd need to scale per-cpu values with that number,
leading to a massive memory overhead as only very few installation
will ever reach that number.
Others might want to set to the max value at the current
configuration, implying that it might change under CPU hotplug.
Which seems to be the case for PowerPC, which was the case which
triggered the issue at hand.
But maybe it was a plain bug in the CPU hotplug implementation; one
should be asking BenH for details here.
Cheers,
Hannes
That sure seems to be at odds with this comment from
include/linux/cpumask.h:
* The cpu_possible_mask is fixed at boot time, as the set of CPU id's
* that it is possible might ever be plugged in at anytime during the
* life of that system boot. The cpu_present_mask is dynamic(*),
* representing which CPUs are currently plugged in. And
* cpu_online_mask is the dynamic subset of cpu_present_mask,
* indicating those CPUs available for scheduling.
and
#define num_possible_cpus() cpumask_weight(cpu_possible_mask)
-- james
Although... num_possible_cpus() shouldn't change, but I guess a cpu id
can be greater than the possible cpu count.
But there is this as well - also from include/linux/cpumask.h:
* If HOTPLUG is enabled, then cpu_possible_mask is forced to have
* all NR_CPUS bits set, otherwise it is just the set of CPUs that
* ACPI reports present at boot.
-- james
