RE:(2) [PATCH v2 02/14] block: bio-integrity: modify bio_integrity_add_page()

From: Jinyoung CHOI
Date: Tue May 16 2023 - 08:54:35 EST


>Hi Jinyoung,
>
>can you work a bit on the commit log and especially the subject line?
>
>I'd word this as something like:
>
>"Subject: bio-integrity: create multi-page bvecs in bio_integrity_add_page()
>
>Allow bio_integrity_add_page to create multi-page bvecs, just like
>the bio payloads.  This simplifies adding larger payloads, and fixes
>support for non-tiny workloads with nvme, which stopped using scatterlist
>for metadata a while ago"
>

Hi. Christoph. I checked late.

OK. I will revise it like that.

>It should probably also mentioned somewhere that you did an audit to
>ensure all drivers and the core code is fine with these multi-page
>segments.  If it's not, this patch should only be added after that
>has been made the case.

Regardless of a single-page or a multi-page configuration,
block_rq_count_integrity_sg() and block_rq_map_integry_sg() check
the pages included in the bip to create a segment for sg.
So... there doesn't seem to be a problem yet.

First, the nvme device that supports fips was tested using the above
functions, and it was also checked with the dix of scsi_debug turned on.
I am also trying to test the sas device that supports sed. It seems that
a problem occurs while going through the laid controller (mpt3sas driver).
It works normally in DIF mode, but protection error occurs when DIX mode
is forcibly activated. Of course, the same is happening in the original code.
I will check additionally and be careful not to cause any problems.

>
>I think the extra arguments struct is a bit overcompliated, and mostly
>due to me making the existing code to weird things in the low-level
>helpers.  With the "rationalize the flow in bio_add_page and friends"
>series I just sent out, I think we can drop the previous patch and
>simplify this one down to:

I tried not to make a big change from the existing logic. So I added
struct unnecessarily. I thought it would be a problem if I added
conditions to the callers and made them call the common code.
Thank you for solving this part.
If your code merges, I will modify it accordingly and proceed.

Best Regards,
Jinyoung.