Skip to content

Commit da78373

Browse files
author
Christoph Hellwig
committed
nvmet: move the call to nvmet_ns_changed out of nvmet_ns_revalidate
nvmet_ns_changed states via lockdep that the ns->subsys->lock must be held. The only caller of nvmet_ns_changed which does not acquire that lock is nvmet_ns_revalidate. nvmet_ns_revalidate has 3 callers, of which 2 do not acquire that lock: nvmet_execute_identify_cns_cs_ns and nvmet_execute_identify_ns. The other caller nvmet_ns_revalidate_size_store does acquire the lock. Move the call to nvmet_ns_changed from nvmet_ns_revalidate to the callers so that they can perform the correct locking as needed. This issue was found using a static type-based analyser and manually verified. Reported-by: Niels Dossche <dossche.niels@gmail.com> Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Sagi Grimberg <sagi@grimberg.me> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
1 parent 98152eb commit da78373

5 files changed

Lines changed: 15 additions & 7 deletions

File tree

drivers/nvme/target/admin-cmd.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,11 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
511511
goto done;
512512
}
513513

514-
nvmet_ns_revalidate(req->ns);
514+
if (nvmet_ns_revalidate(req->ns)) {
515+
mutex_lock(&req->ns->subsys->lock);
516+
nvmet_ns_changed(req->ns->subsys, req->ns->nsid);
517+
mutex_unlock(&req->ns->subsys->lock);
518+
}
515519

516520
/*
517521
* nuse = ncap = nsze isn't always true, but we have no way to find

drivers/nvme/target/configfs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,8 @@ static ssize_t nvmet_ns_revalidate_size_store(struct config_item *item,
586586
mutex_unlock(&ns->subsys->lock);
587587
return -EINVAL;
588588
}
589-
nvmet_ns_revalidate(ns);
589+
if (nvmet_ns_revalidate(ns))
590+
nvmet_ns_changed(ns->subsys, ns->nsid);
590591
mutex_unlock(&ns->subsys->lock);
591592
return count;
592593
}

drivers/nvme/target/core.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ static void nvmet_p2pmem_ns_add_p2p(struct nvmet_ctrl *ctrl,
531531
ns->nsid);
532532
}
533533

534-
void nvmet_ns_revalidate(struct nvmet_ns *ns)
534+
bool nvmet_ns_revalidate(struct nvmet_ns *ns)
535535
{
536536
loff_t oldsize = ns->size;
537537

@@ -540,8 +540,7 @@ void nvmet_ns_revalidate(struct nvmet_ns *ns)
540540
else
541541
nvmet_file_ns_revalidate(ns);
542542

543-
if (oldsize != ns->size)
544-
nvmet_ns_changed(ns->subsys, ns->nsid);
543+
return oldsize != ns->size;
545544
}
546545

547546
int nvmet_ns_enable(struct nvmet_ns *ns)

drivers/nvme/target/nvmet.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ u16 nvmet_file_flush(struct nvmet_req *req);
542542
void nvmet_ns_changed(struct nvmet_subsys *subsys, u32 nsid);
543543
void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns);
544544
void nvmet_file_ns_revalidate(struct nvmet_ns *ns);
545-
void nvmet_ns_revalidate(struct nvmet_ns *ns);
545+
bool nvmet_ns_revalidate(struct nvmet_ns *ns);
546546
u16 blk_to_nvme_status(struct nvmet_req *req, blk_status_t blk_sts);
547547

548548
bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);

drivers/nvme/target/zns.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,11 @@ void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
123123
goto done;
124124
}
125125

126-
nvmet_ns_revalidate(req->ns);
126+
if (nvmet_ns_revalidate(req->ns)) {
127+
mutex_lock(&req->ns->subsys->lock);
128+
nvmet_ns_changed(req->ns->subsys, req->ns->nsid);
129+
mutex_unlock(&req->ns->subsys->lock);
130+
}
127131
zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
128132
req->ns->blksize_shift;
129133
id_zns->lbafe[0].zsze = cpu_to_le64(zsze);

0 commit comments

Comments
 (0)