Re: [PATCH 5/6] nvme: factor out a nvme_ns_is_readonly helper

[Joel Granados <j.granados@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 3047 bytes --]

On Thu, Jul 21, 2022 at 08:03:19AM +0200, Christoph Hellwig wrote:
> Add a little helper to check if a namespace should be marked read-only
> that uses a new is_readonly flag in the nvme_ns_info structure.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Javier González <javier.gonz@samsung.com>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>  drivers/nvme/host/core.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 6169737c233b2..271c0de483a9b 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -36,6 +36,7 @@ struct nvme_ns_info {
>  	u32 nsid;
>  	__le32 anagrpid;
>  	bool is_shared;
> +	bool is_readonly;
>  	bool is_ready;
>  };
>  
> @@ -1447,6 +1448,7 @@ static int nvme_ns_info_from_identify(struct nvme_ctrl *ctrl,
>  		return ret;
>  	info->anagrpid = id->anagrpid;
>  	info->is_shared = id->nmic & NVME_NS_NMIC_SHARED;
> +	info->is_readonly = id->nsattr & NVME_NS_ATTR_RO;
>  	info->is_ready = true;
>  	if (ctrl->quirks & NVME_QUIRK_BOGUS_NID) {
>  		dev_info(ctrl->device,
> @@ -1482,6 +1484,7 @@ static int nvme_ns_info_from_id_cs_indep(struct nvme_ctrl *ctrl,
>  	if (!ret) {
>  		info->anagrpid = id->anagrpid;
>  		info->is_shared = id->nmic & NVME_NS_NMIC_SHARED;
> +		info->is_readonly = id->nsattr & NVME_NS_ATTR_RO;
>  		info->is_ready = id->nstat & NVME_NSTAT_NRDY;
>  	}
>  	kfree(id);
> @@ -1909,6 +1912,11 @@ static void nvme_update_disk_info(struct gendisk *disk,
>  					   ns->ctrl->max_zeroes_sectors);
>  }
>  
> +static bool nvme_ns_is_readonly(struct nvme_ns *ns, struct nvme_ns_info *info)
> +{
> +	return info->is_readonly || test_bit(NVME_NS_FORCE_RO, &ns->flags);
> +}
> +
>  static inline bool nvme_first_scan(struct gendisk *disk)
>  {
>  	/* nvme_alloc_ns() scans the disk prior to adding it */
> @@ -1974,8 +1982,7 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
>  		}
>  	}
>  
> -	set_disk_ro(ns->disk, (id->nsattr & NVME_NS_ATTR_RO) ||
> -		test_bit(NVME_NS_FORCE_RO, &ns->flags));
> +	set_disk_ro(ns->disk, nvme_ns_is_readonly(ns, info));
>  	set_bit(NVME_NS_READY, &ns->flags);
>  	blk_mq_unfreeze_queue(ns->disk->queue);
>  
> @@ -1988,9 +1995,7 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
>  	if (nvme_ns_head_multipath(ns->head)) {
>  		blk_mq_freeze_queue(ns->head->disk->queue);
>  		nvme_update_disk_info(ns->head->disk, ns, id);
> -		set_disk_ro(ns->head->disk,
> -			    (id->nsattr & NVME_NS_ATTR_RO) ||
> -				    test_bit(NVME_NS_FORCE_RO, &ns->flags));
> +		set_disk_ro(ns->head->disk, nvme_ns_is_readonly(ns, info));
>  		nvme_mpath_revalidate_paths(ns);
>  		blk_stack_limits(&ns->head->disk->queue->limits,
>  				 &ns->queue->limits, 0);
> -- 
> 2.30.2
> 

LGTM
Reviewed-by: Joel Granados <j.granados@samsung.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]