Re: [PATCH v7 0/1] Implement the NVMe reservation feature

[Guixin Liu <kanie@linux.alibaba.com>]

Hi guys,

     I will send a v8 patch to add me as the new file's maintainer.

Best Regards,

Guixin Liu

在 2024/2/1 10:32, Guixin Liu 写道:
> Hi guys,
>      I've implemented the NVMe reservation feature. Please review it, all
> comments are welcome as usual.
>
> Changes from v6 to v7:
> - Handle "reservation notification mask" feature command to mask reservation
> log.
>
> - Add all the registrants that need to be freed to a temporary list fist,
> and then after calling synchronize_rcu(), release all the registrants on the
> temporary list.
>
> - Fix the resv log page is random when there is no resv log page.
>
> - Change nvmet_is_host_still_connected() to nvmet_is_host_connected().
>
> - Remove nvmet_pr_set_rtype_and_holder() and change nvmet_pr_create_new_resv()
> to nvmet_pr_create_new_reservation().
>
> - Change nvmet_pr_find_registrant_by_hostid() to nvmet_pr_find_registrant().
>
> - Change nvmet_pr_send_resv_released() to nvmet_pr_resv_released().
>
> - Change __nvmet_pr_unregister_one() to nvmet_pr_unregister_one().
>
> - In nvmet_pr_unreg_by_prkey(), nvmet_pr_unreg_by_prkey_except_hostid() and
> nvmet_pr_unreg_except_hostid(), first do unregistering and then do event sending.
>
>
> Changes from v5 to v6:
> - Use synchronize_rcu() and kfree() to free registrant instead of kfree_rcu().
>
> - Remove nvmet_pr_register_check_rkey(), put the check into pr_lock warp.
> And refactor the nvmet_pr_register().
>
> - Add the print fmt to the head.
>
> - Add lockdep_is_held(&pr->pr_lock) condition to list_for_each_entry_rcu.
>
> - Fix the bug in nvmet_pr_update_reg_attr(), when the change_attr hook
> return fail, we should not replace the holder.
>
> Changes from v4 to v5:
>
> - Use rculist macros to handle registration_list instead of list macros
> regardless of in mutex lock or not.
>
> - Use goto statement instead of return in nvmet_is_host_still_connected
> and __nvmet_pr_unregister_one.
>
> - Add lockdep_assert_held and rcu_read_lock_held assert to many functions,
> if it's necessary.
>
> - Add a comment to nvmet_execute_get_log_page_resv to explain how lost_count
> works.
>
> - In nvmet_pr_clear, we should set holder to NULL first, I fixed this.
>
> - Unify nvmet_pr_update_holder_rtype and __nvmet_pr_do_replace to
> nvmet_pr_update_reg_attr.
>
> - Fix wrong nr_pages in nvmet_execute_get_log_page_resv.
>
> - Fix the deadlock issue of nvmet_pr_exit_ns, put it out of the subsys lock.
>
>
> Changes from v3 to v4:
> - Use kfifo to handle resv log page instead of list, and also limit the
> resv log queue to 64.
>
> - Change the function calling alignment style to:
>      nvmet_pr_send_event_by_hostid(pr, hostid,
>              NVME_PR_LOG_RESERVATOPM_PREEMPTED);
>
> - Put kmalloc out of rcu_read_lock in nvmet_execute_pr_report().
>
> - Remove the goto in __nvmet_pr_unregister_one().
>
> - Change generation to atomic_t, and remove nvmet_pr_inc_generation().
>
> - In addtion, the number2 patch "nvmet: unify aer type enum" is not
> relate with this patch, so I will send it separately.
>
>
> Changes from v2 to v3:
> - Use rcu instead of rwlock to make IO path run faster, and put the rtype
> into the struct nvmet_pr_registrant.
>
> - Limit the resv_log_list to 128.
>
> - Change generation to atomic64.
>
> - Put register rkey check to a warpper.
>
> - Change nr_avl_pages to nr_pages.
>
> - Use NVME_SC_SUCCESS instead of 0.
>
> - Change kmalloc param to let it not sleep in mutex lock.
>
>
> Changes from v1 to v2:
> - Implement the reservation notification report, includes registration
> preempted, reservation released and reservation preempted.
>    And also handle the reservation log page available event and send get
> reservation log page command to clear log page at host.
>
> - Put the reservation check access after validate opcode. And remove
> opcodes which nvmet not implement yet check.
>    Now there is no admin opcode nvmet implemented needs reservation check,
> so I dont add reservation check to admin command path.
>    Next we need to do reservation check includes the situation of nsid is
> 0xffffffff at each admin command path, if it is needed.
>
> - Add reservation commands support in nvmet_get_cmd_effects_nvm().
>
> - From Chaitanya, change the local variable tree style to make it cleaner,
> and add some comments about NVMe spec.
>    And also change others advice from chaitanya.
>
> - Put the nvmet_pr_check_cmd_access and nvmet_parse_pr_cmd into reservation
> enable check warp.
>
> - Remove kmem_cache instead to use kmalloc and kfree.
>
> - Change others advice from Sagi.
>
> - Add a blktest test case, this patch will be sent before these series of
> patches.
>
> Guixin Liu (1):
>    nvmet: support reservation feature
>
>   drivers/nvme/target/Makefile    |    2 +-
>   drivers/nvme/target/admin-cmd.c |   20 +-
>   drivers/nvme/target/configfs.c  |   27 +
>   drivers/nvme/target/core.c      |   51 +-
>   drivers/nvme/target/nvmet.h     |   36 ++
>   drivers/nvme/target/pr.c        | 1041 +++++++++++++++++++++++++++++++
>   include/linux/nvme.h            |   54 ++
>   7 files changed, 1224 insertions(+), 7 deletions(-)
>   create mode 100644 drivers/nvme/target/pr.c
>