On 19-09-05 19:43:54, Max Gurtovoy wrote:
> Currently dif_prepare/dif_complete functions are called during the
Maybe nicer if:
s/dif_prepare\/dif_complete/t10_pi_prepare\/t10_pi_complete
> NVMe and SCSi layers command preparetion/completion, but their actual
> place should be the block layer since T10-PI is a general data integrity
> feature that is used by block storage protocols.
>
> Suggested-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Max Gurtovoy <maxg@xxxxxxxxxxxx>
> ---
>
> changes from v2:
> - remove local variable for protection_type
> - remove remapping between NVMe T10 definition to blk definition
> - added patches 2/3 and 3/3
> - remove pi_type from ns structure
>
> changes from v1:
> - seperate from nvme_cleanup command patches
> - introduce blk_integrity_interval_shift to avoid div in fast path
>
> ---
> block/blk-core.c | 5 +++++
> block/blk-mq.c | 4 ++++
> block/blk-settings.c | 1 +
> block/t10-pi.c | 16 ++++++----------
> drivers/nvme/host/core.c | 27 ++++++++++-----------------
> drivers/nvme/host/nvme.h | 1 -
> drivers/scsi/sd.c | 28 ++++++++++------------------
> drivers/scsi/sd.h | 1 -
> drivers/scsi/sd_dif.c | 2 +-
> include/linux/blkdev.h | 12 ++++++++++++
> include/linux/genhd.h | 1 +
> include/linux/t10-pi.h | 10 ++++------
> 12 files changed, 54 insertions(+), 54 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index d0cc6e1..eda33f9 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -34,6 +34,7 @@
> #include <linux/ratelimit.h>
> #include <linux/pm_runtime.h>
> #include <linux/blk-cgroup.h>
> +#include <linux/t10-pi.h>
> #include <linux/debugfs.h>
> #include <linux/bpf.h>
>
> @@ -1405,6 +1406,10 @@ bool blk_update_request(struct request *req, blk_status_t error,
> if (!req->bio)
> return false;
>
> + if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ &&
> + error == BLK_STS_OK)
> + t10_pi_complete(req, nr_bytes);
> +
> if (unlikely(error && !blk_rq_is_passthrough(req) &&
> !(req->rq_flags & RQF_QUIET)))
> print_req_error(req, error, __func__);
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 0835f4d..30ec078 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -30,6 +30,7 @@
> #include <trace/events/block.h>
>
> #include <linux/blk-mq.h>
> +#include <linux/t10-pi.h>
> #include "blk.h"
> #include "blk-mq.h"
> #include "blk-mq-debugfs.h"
> @@ -693,6 +694,9 @@ void blk_mq_start_request(struct request *rq)
> */
> rq->nr_phys_segments++;
> }
> +
> + if (blk_integrity_rq(rq) && req_op(rq) == REQ_OP_WRITE)
> + t10_pi_prepare(rq);
> }
> EXPORT_SYMBOL(blk_mq_start_request);
>
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 2c18312..8183ffc 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -330,6 +330,7 @@ void blk_queue_max_segment_size(struct request_queue *q, unsigned int max_size)
> void blk_queue_logical_block_size(struct request_queue *q, unsigned short size)
> {
> q->limits.logical_block_size = size;
> + q->limits.logical_block_shift = ilog2(size);
>
> if (q->limits.physical_block_size < size)
> q->limits.physical_block_size = size;
> diff --git a/block/t10-pi.c b/block/t10-pi.c
> index 0c00946..7d9a151 100644
> --- a/block/t10-pi.c
> +++ b/block/t10-pi.c
> @@ -171,7 +171,6 @@ static blk_status_t t10_pi_type3_verify_ip(struct blk_integrity_iter *iter)
> /**
> * t10_pi_prepare - prepare PI prior submitting request to device
> * @rq: request with PI that should be prepared
> - * @protection_type: PI type (Type 1/Type 2/Type 3)
> *
> * For Type 1/Type 2, the virtual start sector is the one that was
> * originally submitted by the block layer for the ref_tag usage. Due to
> @@ -181,13 +180,13 @@ static blk_status_t t10_pi_type3_verify_ip(struct blk_integrity_iter *iter)
> *
> * Type 3 does not have a reference tag so no remapping is required.
> */
> -void t10_pi_prepare(struct request *rq, u8 protection_type)
> +void t10_pi_prepare(struct request *rq)
> {
> const int tuple_sz = rq->q->integrity.tuple_size;
> u32 ref_tag = t10_pi_ref_tag(rq);
> struct bio *bio;
>
> - if (protection_type == T10_PI_TYPE3_PROTECTION)
> + if (rq->rq_disk->protection_type == T10_PI_TYPE3_PROTECTION)
> return;
What do you think about making rq->rq_disk->protection_type with a
helper? Would it be no need or more confusing if helper provided?
Thanks!
