| Message ID | 20230224174502.321490-14-michael.christie@oracle.com (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [v4,01/18] block: Add PR callouts for read keys and reservation | expand |
Hi Mike,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next axboe-block/for-next linus/master v6.2 next-20230224]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mike-Christie/block-Rename-BLK_STS_NEXUS-to-BLK_STS_RESV_CONFLICT/20230225-024505
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link: https://lore.kernel.org/r/20230224174502.321490-14-michael.christie%40oracle.com
patch subject: [PATCH v4 13/18] nvme: Add pr_ops read_reservation support
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230225/202302250448.cEVYdC1I-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f66174eef73e332bdca3a158541875a4c2e617d1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mike-Christie/block-Rename-BLK_STS_NEXUS-to-BLK_STS_RESV_CONFLICT/20230225-024505
git checkout f66174eef73e332bdca3a158541875a4c2e617d1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/nvme/host/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302250448.cEVYdC1I-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/nvme/host/pr.c: In function 'block_pr_type_from_nvme':
>> drivers/nvme/host/pr.c:43:24: warning: implicit conversion from 'enum nvme_pr_type' to 'enum pr_type' [-Wenum-conversion]
43 | return NVME_PR_EXCLUSIVE_ACCESS_ALL_REGS;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +43 drivers/nvme/host/pr.c
28
29 static enum pr_type block_pr_type_from_nvme(enum nvme_pr_type type)
30 {
31 switch (type) {
32 case NVME_PR_WRITE_EXCLUSIVE:
33 return PR_WRITE_EXCLUSIVE;
34 case NVME_PR_EXCLUSIVE_ACCESS:
35 return PR_EXCLUSIVE_ACCESS;
36 case NVME_PR_WRITE_EXCLUSIVE_REG_ONLY:
37 return PR_WRITE_EXCLUSIVE_REG_ONLY;
38 case NVME_PR_EXCLUSIVE_ACCESS_REG_ONLY:
39 return PR_EXCLUSIVE_ACCESS_REG_ONLY;
40 case NVME_PR_WRITE_EXCLUSIVE_ALL_REGS:
41 return PR_WRITE_EXCLUSIVE_ALL_REGS;
42 case NVME_PR_EXCLUSIVE_ACCESS_ALL_REGS:
> 43 return NVME_PR_EXCLUSIVE_ACCESS_ALL_REGS;
44 }
45
46 return 0;
47 }
48
On 2/24/2023 9:44 AM, Mike Christie wrote: > This patch adds support for the pr_ops read_reservation callout by > calling the NVMe Reservation Report helper. It then parses that info to > detect if there is a reservation and if there is then convert the > returned info to a pr_ops pr_held_reservation struct. > > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck
On 2/24/23 3:04 PM, kernel test robot wrote: > > drivers/nvme/host/pr.c: In function 'block_pr_type_from_nvme': >>> drivers/nvme/host/pr.c:43:24: warning: implicit conversion from 'enum nvme_pr_type' to 'enum pr_type' [-Wenum-conversion] > 43 | return NVME_PR_EXCLUSIVE_ACCESS_ALL_REGS; > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Should have been PR_EXCLUSIVE_ACCESS_ALL_REGS. Will fix.
Same comment on struct_size() as two patches earlier, but otherwise this looks good to me.
diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c index 66086369dbce..cadf61dc60c3 100644 --- a/drivers/nvme/host/pr.c +++ b/drivers/nvme/host/pr.c @@ -26,6 +26,26 @@ static enum nvme_pr_type nvme_pr_type_from_blk(enum pr_type type) return 0; } +static enum pr_type block_pr_type_from_nvme(enum nvme_pr_type type) +{ + switch (type) { + case NVME_PR_WRITE_EXCLUSIVE: + return PR_WRITE_EXCLUSIVE; + case NVME_PR_EXCLUSIVE_ACCESS: + return PR_EXCLUSIVE_ACCESS; + case NVME_PR_WRITE_EXCLUSIVE_REG_ONLY: + return PR_WRITE_EXCLUSIVE_REG_ONLY; + case NVME_PR_EXCLUSIVE_ACCESS_REG_ONLY: + return PR_EXCLUSIVE_ACCESS_REG_ONLY; + case NVME_PR_WRITE_EXCLUSIVE_ALL_REGS: + return PR_WRITE_EXCLUSIVE_ALL_REGS; + case NVME_PR_EXCLUSIVE_ACCESS_ALL_REGS: + return NVME_PR_EXCLUSIVE_ACCESS_ALL_REGS; + } + + return 0; +} + static int nvme_send_ns_head_pr_command(struct block_device *bdev, struct nvme_command *c, void *data, unsigned int data_len) { @@ -215,6 +235,66 @@ static int nvme_pr_read_keys(struct block_device *bdev, return ret; } +static int nvme_pr_read_reservation(struct block_device *bdev, + struct pr_held_reservation *resv) +{ + struct nvme_reservation_status tmp_rs, *rs; + int ret, i, num_regs; + u32 rs_len; + bool eds; + +get_num_regs: + /* + * Get the number of registrations so we know how big to allocate + * the response buffer. + */ + ret = nvme_pr_resv_report(bdev, &tmp_rs, sizeof(tmp_rs), &eds); + if (ret) + return ret; + + num_regs = get_unaligned_le16(&tmp_rs.regctl); + if (!num_regs) { + resv->generation = le32_to_cpu(tmp_rs.gen); + return 0; + } + + rs_len = sizeof(*rs) + + num_regs * sizeof(struct nvme_registered_ctrl_ext); + rs = kzalloc(rs_len, GFP_KERNEL); + if (!rs) + return -ENOMEM; + + ret = nvme_pr_resv_report(bdev, rs, rs_len, &eds); + if (ret) + goto free_rs; + + if (num_regs != get_unaligned_le16(&rs->regctl)) { + kfree(rs); + goto get_num_regs; + } + + resv->generation = le32_to_cpu(rs->gen); + resv->type = block_pr_type_from_nvme(rs->rtype); + + for (i = 0; i < num_regs; i++) { + if (eds) { + if (rs->regctl_eds[i].rcsts) { + resv->key = le64_to_cpu(rs->regctl_eds[i].rkey); + break; + } + } else { + if (rs->regctl_ds[i].rcsts) { + resv->key = le64_to_cpu(rs->regctl_ds[i].rkey); + break; + } + } + } + +free_rs: + kfree(rs); + return ret; +} + const struct pr_ops nvme_pr_ops = { .pr_register = nvme_pr_register, .pr_reserve = nvme_pr_reserve, @@ -222,4 +302,5 @@ const struct pr_ops nvme_pr_ops = { .pr_preempt = nvme_pr_preempt, .pr_clear = nvme_pr_clear, .pr_read_keys = nvme_pr_read_keys, + .pr_read_reservation = nvme_pr_read_reservation, };
This patch adds support for the pr_ops read_reservation callout by calling the NVMe Reservation Report helper. It then parses that info to detect if there is a reservation and if there is then convert the returned info to a pr_ops pr_held_reservation struct. Signed-off-by: Mike Christie <michael.christie@oracle.com> --- drivers/nvme/host/pr.c | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+)