| Message ID | 20220913041707.197334-4-ZiyangZhang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ublk_drv: add USER_RECOVERY support | expand |
On Tue, Sep 13, 2022 at 12:17:03PM +0800, ZiyangZhang wrote: > Define some macros for recovery feature. Especially define a new state: > UBLK_S_DEV_QUIESCED which implies that ublk_device is quiesced > and is ready for recovery. This state can be observed by userspace. > > UBLK_F_USER_RECOVERY implies that: > (1) ublk_drv enables recovery feature. It won't let monitor_work to > automatically abort rqs and release the device. > (2) With a dying ubq_daemon, ublk_drv ends(aborts) rqs issued to > userspace(ublksrv) before crash. > (3) With a dying ubq_daemon, in task work and ublk_queue_rq(), > ublk_drv requeues rqs. > > UBLK_F_USER_RECOVERY_REISSUE implies that: > (1) everything UBLK_F_USER_RECOVERY implies except > (2) With a dying ubq_daemon, ublk_drv requeues rqs issued to > userspace(ublksrv) before crash. > > UBLK_F_USER_RECOVERY_REISSUE is designed for backends which: > (1) tolerates double-writes because ublk_drv may issue the same rq > twice. > (2) does not let frontend users get I/O error. such as read-only FS > and VM backend. > > Signed-off-by: ZiyangZhang <ZiyangZhang@linux.alibaba.com> > --- > drivers/block/ublk_drv.c | 45 ++++++++++++++++++++++++++++++++++- > include/uapi/linux/ublk_cmd.h | 7 ++++++ > 2 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c > index 0c6db0978ed0..23337bd7c105 100644 > --- a/drivers/block/ublk_drv.c > +++ b/drivers/block/ublk_drv.c > @@ -49,7 +49,9 @@ > /* All UBLK_F_* have to be included into UBLK_F_ALL */ > #define UBLK_F_ALL (UBLK_F_SUPPORT_ZERO_COPY \ > | UBLK_F_URING_CMD_COMP_IN_TASK \ > - | UBLK_F_NEED_GET_DATA) > + | UBLK_F_NEED_GET_DATA \ > + | UBLK_F_USER_RECOVERY \ > + | UBLK_F_USER_RECOVERY_REISSUE) > > /* All UBLK_PARAM_TYPE_* should be included here */ > #define UBLK_PARAM_TYPE_ALL (UBLK_PARAM_TYPE_BASIC | UBLK_PARAM_TYPE_DISCARD) > @@ -323,6 +325,47 @@ static inline int ublk_queue_cmd_buf_size(struct ublk_device *ub, int q_id) > PAGE_SIZE); > } > > +static inline bool ublk_queue_can_use_recovery( > + struct ublk_queue *ubq) > +{ > + if (ubq->flags & UBLK_F_USER_RECOVERY) > + return true; > + return false; > +} > + > +static inline void ublk_disable_recovery(struct ublk_device *ub) > +{ > + int i; > + > + for (i = 0; i < ub->dev_info.nr_hw_queues; i++) { > + struct ublk_queue *ubq = ublk_get_queue(ub, i); > + > + ubq->flags &= ~UBLK_F_USER_RECOVERY; > + } > +} Flags is supposed to not changed, especially ublk_disable_recovery isn't necessary with my suggestion in the following link: https://lore.kernel.org/linux-block/YylEjEply6y+bs0B@T590/T/#u > + > +static inline bool ublk_can_use_recovery(struct ublk_device *ub) > +{ > + int i; > + > + for (i = 0; i < ub->dev_info.nr_hw_queues; i++) { > + struct ublk_queue *ubq = ublk_get_queue(ub, i); > + > + if (!ublk_queue_can_use_recovery(ubq)) > + return false; > + } > + return true; > +} The above is too tricky, why can't check ub->dev_info & UBLK_F_USER_RECOVERY directly? > + > +static inline bool ublk_queue_can_use_recovery_reissue( > + struct ublk_queue *ubq) > +{ > + if (ublk_queue_can_use_recovery(ubq) && > + (ubq->flags & UBLK_F_USER_RECOVERY_REISSUE)) > + return true; > + return false; > +} > + > static void ublk_free_disk(struct gendisk *disk) > { > struct ublk_device *ub = disk->private_data; > diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h > index 677edaab2b66..87204c39f1ee 100644 > --- a/include/uapi/linux/ublk_cmd.h > +++ b/include/uapi/linux/ublk_cmd.h > @@ -17,6 +17,8 @@ > #define UBLK_CMD_STOP_DEV 0x07 > #define UBLK_CMD_SET_PARAMS 0x08 > #define UBLK_CMD_GET_PARAMS 0x09 > +#define UBLK_CMD_START_USER_RECOVERY 0x10 > +#define UBLK_CMD_END_USER_RECOVERY 0x11 > > /* > * IO commands, issued by ublk server, and handled by ublk driver. > @@ -74,9 +76,14 @@ > */ > #define UBLK_F_NEED_GET_DATA (1UL << 2) > > +#define UBLK_F_USER_RECOVERY (1UL << 3) > + > +#define UBLK_F_USER_RECOVERY_REISSUE (1UL << 4) The above are two features. I'd suggest to add UBLK_F_USER_RECOVERY and its implementation first, then add one delta patch for supporting the new feature of UBLK_F_USER_RECOVERY_REISSUE. Not only it is more helpful for reviewing, but also easier to understand the two's difference. thanks, Ming
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index 0c6db0978ed0..23337bd7c105 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -49,7 +49,9 @@ /* All UBLK_F_* have to be included into UBLK_F_ALL */ #define UBLK_F_ALL (UBLK_F_SUPPORT_ZERO_COPY \ | UBLK_F_URING_CMD_COMP_IN_TASK \ - | UBLK_F_NEED_GET_DATA) + | UBLK_F_NEED_GET_DATA \ + | UBLK_F_USER_RECOVERY \ + | UBLK_F_USER_RECOVERY_REISSUE) /* All UBLK_PARAM_TYPE_* should be included here */ #define UBLK_PARAM_TYPE_ALL (UBLK_PARAM_TYPE_BASIC | UBLK_PARAM_TYPE_DISCARD) @@ -323,6 +325,47 @@ static inline int ublk_queue_cmd_buf_size(struct ublk_device *ub, int q_id) PAGE_SIZE); } +static inline bool ublk_queue_can_use_recovery( + struct ublk_queue *ubq) +{ + if (ubq->flags & UBLK_F_USER_RECOVERY) + return true; + return false; +} + +static inline void ublk_disable_recovery(struct ublk_device *ub) +{ + int i; + + for (i = 0; i < ub->dev_info.nr_hw_queues; i++) { + struct ublk_queue *ubq = ublk_get_queue(ub, i); + + ubq->flags &= ~UBLK_F_USER_RECOVERY; + } +} + +static inline bool ublk_can_use_recovery(struct ublk_device *ub) +{ + int i; + + for (i = 0; i < ub->dev_info.nr_hw_queues; i++) { + struct ublk_queue *ubq = ublk_get_queue(ub, i); + + if (!ublk_queue_can_use_recovery(ubq)) + return false; + } + return true; +} + +static inline bool ublk_queue_can_use_recovery_reissue( + struct ublk_queue *ubq) +{ + if (ublk_queue_can_use_recovery(ubq) && + (ubq->flags & UBLK_F_USER_RECOVERY_REISSUE)) + return true; + return false; +} + static void ublk_free_disk(struct gendisk *disk) { struct ublk_device *ub = disk->private_data; diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h index 677edaab2b66..87204c39f1ee 100644 --- a/include/uapi/linux/ublk_cmd.h +++ b/include/uapi/linux/ublk_cmd.h @@ -17,6 +17,8 @@ #define UBLK_CMD_STOP_DEV 0x07 #define UBLK_CMD_SET_PARAMS 0x08 #define UBLK_CMD_GET_PARAMS 0x09 +#define UBLK_CMD_START_USER_RECOVERY 0x10 +#define UBLK_CMD_END_USER_RECOVERY 0x11 /* * IO commands, issued by ublk server, and handled by ublk driver. @@ -74,9 +76,14 @@ */ #define UBLK_F_NEED_GET_DATA (1UL << 2) +#define UBLK_F_USER_RECOVERY (1UL << 3) + +#define UBLK_F_USER_RECOVERY_REISSUE (1UL << 4) + /* device state */ #define UBLK_S_DEV_DEAD 0 #define UBLK_S_DEV_LIVE 1 +#define UBLK_S_DEV_QUIESCED 2 /* shipped via sqe->cmd of io_uring command */ struct ublksrv_ctrl_cmd {
Define some macros for recovery feature. Especially define a new state: UBLK_S_DEV_QUIESCED which implies that ublk_device is quiesced and is ready for recovery. This state can be observed by userspace. UBLK_F_USER_RECOVERY implies that: (1) ublk_drv enables recovery feature. It won't let monitor_work to automatically abort rqs and release the device. (2) With a dying ubq_daemon, ublk_drv ends(aborts) rqs issued to userspace(ublksrv) before crash. (3) With a dying ubq_daemon, in task work and ublk_queue_rq(), ublk_drv requeues rqs. UBLK_F_USER_RECOVERY_REISSUE implies that: (1) everything UBLK_F_USER_RECOVERY implies except (2) With a dying ubq_daemon, ublk_drv requeues rqs issued to userspace(ublksrv) before crash. UBLK_F_USER_RECOVERY_REISSUE is designed for backends which: (1) tolerates double-writes because ublk_drv may issue the same rq twice. (2) does not let frontend users get I/O error. such as read-only FS and VM backend. Signed-off-by: ZiyangZhang <ZiyangZhang@linux.alibaba.com> --- drivers/block/ublk_drv.c | 45 ++++++++++++++++++++++++++++++++++- include/uapi/linux/ublk_cmd.h | 7 ++++++ 2 files changed, 51 insertions(+), 1 deletion(-)