On Mar 29, 2019, at 8:40 AM, Bob Liu <bob.liu@oracle.com> wrote:
> 
> On 3/29/19 10:28 PM, Jens Axboe wrote:
>> On 3/29/19 8:23 AM, Bob Liu wrote:
>>> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
>>> index d66bf5f32610..e9f25f162138 100644
>>> --- a/include/linux/blk_types.h
>>> +++ b/include/linux/blk_types.h
>>> @@ -18,6 +18,7 @@ struct block_device;
>>> struct io_context;
>>> struct cgroup_subsys_state;
>>> typedef void (bio_end_io_t) (struct bio *);
>>> +typedef int (bio_verifier_t) (struct bio *);
>>> 
>>> /*
>>>  * Block error status values.  See block/blk-core:blk_errors for the details.
>>> @@ -187,6 +188,8 @@ struct bio {
>>> 		struct bio_integrity_payload *bi_integrity; /* data integrity */
>>> #endif
>>> 	};
>>> +	bio_verifier_t		*bi_verifier;	/* verify callback when endio */
>>> +	struct work_struct      bi_work;	/* I/O completion */
>>> 
>>> 	unsigned short		bi_vcnt;	/* how many bio_vec's */
>> 
>> I told you this for the initial posting, and the objection still stands.
>> Adding 40 bytes to struct bio is a no-go.
> 
> Yes, I know that.
> Already list in Todo:
> - union with "struct bio_integrity_payload *bi_integrity" to save bio space.
> 
> This is only a rfc patch, because not sure any other objection of this passdown-verify solution..
> Will consider more about the bio space if there is agreement of the passdown-verify way.

It is best if "rfc patch" is labelled with "RFC" in the subject when there are known
TODO issues with the patch, to avoid this kind of confusion... :-)


I worked on a similar patch that adds generate and verify callback functions to struct
bio_integrity_payload rather than directly in the bio, to avoid increasing bio size.

This was implemented by registering the callback functions via bio_integrity_prep_fn()
and stored them in bio_integrity_payload before the bio_submit() was called, rather
than passing them down the call stack as would be needed here.

This would allow having a per-bio callback without the changing the whole callchain.
Instead of XFS calling the new bio_submit_verify(), it calls bio_integrity_prep_fn(),
which doesn't seem materially different in terms of code complexity (i.e. there will
still need to be some changes to XFS but it doesn't affect other code).

I've attached the patch below.  It was based on an older kernel and I've just done a
quick port to the latest kernel (untested), so it is entirely possible that there is
some breakage but it shows the general approach.

Cheers, Andreas