Re: [PATCH v3 2/3] block: verify data when endio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bob,

On Fri, Mar 29, 2019 at 10:25 PM Bob Liu <bob.liu@xxxxxxxxxx> wrote:
>
> Call verify callback same as bio integrity.
> If verify fail, drivers like MD will try other mirrors until get a correct
> one or return failure after all mirrors are tried.
> The MD driver already works like this, so no extra changed.
>
> Todo:
> - union with "struct bio_integrity_payload *bi_integrity" to save bio space.
>
> Signed-off-by: Bob Liu <bob.liu@xxxxxxxxxx>
> ---
>  block/bio-integrity.c     | 45 +++++++++++++++++++++++++++++++++++++++
>  block/bio.c               |  3 +++
>  block/blk-core.c          |  4 ++++
>  block/blk.h               |  8 +++++++
>  block/bounce.c            |  1 +
>  drivers/md/raid1.c        |  1 +
>  drivers/md/raid5-ppl.c    |  1 +
>  include/linux/blk_types.h |  5 +++++
>  8 files changed, 68 insertions(+)
>
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index 1b633a3526d4..90a47ad31dbf 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -372,6 +372,51 @@ bool __bio_integrity_endio(struct bio *bio)
>         return true;
>  }
>
> +/**
> + * bio_verify_fn - Verify I/O completion worker
> + * @work:      Work struct stored in bio to be verified
> + *
> + * Description: This workqueue function is called to complete a READ
> + * request.  The function call verifier callack that fs pass down
> + * and then calls the original bio end_io function.
> + */
> +static void bio_verify_fn(struct work_struct *work)
> +{
> +       struct bio *bio =
> +               container_of(work, struct bio, bi_work);
> +
> +       bio->bi_status = bio->bi_verifier(bio);
> +       /* Clear flag if verify succeed to avoid verifing
> +        * it unnecessary by parent bio
> +        */
> +       if (!bio->bi_status)
> +               bio->bi_opf &= ~REQ_VERIFY;
> +       bio_endio(bio);
> +}

1) why is bio->bi_verifier run from workqueue context instead of being
called directly
from bio_endio()?

2) the followings part of bio_endio(bio) may be run twice, will this
way work correctly?

        if (!bio_remaining_done(bio))
                return;
        if (!bio_integrity_endio(bio))
                return;

> +
> +/**
> + * __bio_verify_endio - Verify I/O completion function
> + * @bio:       Protected bio
> + *
> + * Description: Completion for verify I/O
> + *
> + * Normally I/O completion is done in interrupt context.  However,
> + * verifying I/O is a time-consuming task which must be run
> + * in process context. This function postpones completion
> + * accordingly.
> + */
> +bool __bio_verify_endio(struct bio *bio)
> +{
> +       if (bio_op(bio) == REQ_OP_READ && !bio->bi_status &&
> +           (bio->bi_opf & REQ_VERIFY) && bio->bi_verifier) {
> +               INIT_WORK(&bio->bi_work, bio_verify_fn);
> +               queue_work(kintegrityd_wq, &bio->bi_work);
> +               return false;
> +       }
> +
> +       return true;
> +}
> +
>  /**
>   * bio_integrity_advance - Advance integrity vector
>   * @bio:       bio whose integrity vector to update
> diff --git a/block/bio.c b/block/bio.c
> index 4db1008309ed..8928806acda6 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -608,6 +608,7 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src)
>         bio->bi_write_hint = bio_src->bi_write_hint;
>         bio->bi_iter = bio_src->bi_iter;
>         bio->bi_io_vec = bio_src->bi_io_vec;
> +       bio->bi_verifier = bio_src->bi_verifier;

->bi_verifier is cloned along the IO stack, is this .bi_verifier
capable of covering
the cloned bio which may be just a part of original bio? such as, bio split.

Given in the 3rd patch, .bi_verifier is implemented in fs, I guess it should
only work for the bio submitted from fs because the verifier uses fs's
knowledge.

If yes, looks not necessary to clone .bi_verifier here. Otherwise, could you
explain a bit how ->bi_verifier can work for the cloned bio in the stack?


Thanks,
Ming Lei



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux