On 2020-08-12 3:19 p.m., Keith Busch wrote:
> On Wed, Aug 12, 2020 at 03:01:19PM -0600, Logan Gunthorpe wrote:
>> @@ -2971,15 +2971,16 @@ int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid,
>> u8 log_page, u8 lsp, u8 csi,
>> static struct nvme_cel *nvme_find_cel(struct nvme_ctrl *ctrl, u8 csi)
>> {
>> struct nvme_cel *cel, *ret = NULL;
>> + unsigned long flags;
>>
>> - spin_lock(&ctrl->lock);
>> + spin_lock_irqsave(&ctrl->lock, flags);
>> list_for_each_entry(cel, &ctrl->cels, entry) {
>> if (cel->csi == csi) {
>> ret = cel;
>> break;
>> }
>> }
>> - spin_unlock(&ctrl->lock);
>> + spin_unlock_irqrestore(&ctrl->lock, flags);
>>
>> return ret;
>> }
>> @@ -2988,6 +2989,7 @@ static int nvme_get_effects_log(struct nvme_ctrl
>> *ctrl, u8 csi,
>> struct nvme_effects_log **log)
>> {
>> struct nvme_cel *cel = nvme_find_cel(ctrl, csi);
>> + unsigned long flags;
>> int ret;
>>
>> if (cel)
>> @@ -3006,9 +3008,9 @@ static int nvme_get_effects_log(struct nvme_ctrl
>> *ctrl, u8 csi,
>>
>> cel->csi = csi;
>>
>> - spin_lock(&ctrl->lock);
>> + spin_lock_irqsave(&ctrl->lock, flags);
>> list_add_tail(&cel->entry, &ctrl->cels);
>> - spin_unlock(&ctrl->lock);
>> + spin_unlock_irqrestore(&ctrl->lock, flags);
>> out:
>> *log = &cel->log;
>> return 0;
>>
>
> Neither of these are ever called from an interrupt disabled context,
> correct? If so, you can just use spin_lock_irq() without saving the
> current irq state.
Yes, I believe that is correct. I'll send a v2 shortly.
Thanks,
Logan