Re: [RFC] Simlify dif_verify routines and fixup fileio protection information code.

[Akinobu Mita <akinobu.mita@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

2015-04-15 19:07 GMT+09:00 Sagi Grimberg <sagig@dev.mellanox.co.il>:
> On 4/15/2015 2:52 AM, Akinobu Mita wrote:
>
>>>>
>>>> Looks good...
>>>>
>>>> I'll test with these patches and check if the problems I met
>>>> disappear.
>>>
>>>
>>>
>>> Thanks Akinobu,
>>>
>>> Waiting to hear your verdict before sending a formal patchset.
>>
>>
>> I hit a original bug in sbc_dif_verify() which is not introduced by
>> your patch set, though.
>
>
> What is this original bug?

I meant to say about the problem I fixed by fix-sbc_dif_verify.patch.

>>  Please consider to include attached patch.
>
>
> I'll include it. thanks.
>
>> I'm still seeing another problem and trying to find out a root cause,
>> but it seems like it's caused by other change in -next.
>>
>
> care to elaborate?

When mounting ext4 filesystem at the first time after mkfs, a lot of
WRITE_SAME commands are issued for lazy initialization or something.

By the commit 436f4a0a ("loopback: Add fabric_prot_type attribute
support"), When WRITE_SAME command with WRPROTECT=0 is executed,
sbc_dif_generate() is called but cmd->t_prot_sg is NULL as block layer
didn't allocate it for WRITE_SAME.

I could work around with the attached patch, as the WRITE_SAME command
will fail after all when protection info is enabled with FILEIO, we only need to
avoid null pointer dereference.  But I need to ask Nic about the right
way to fix.

For this patch set, please feel free to add:

Tested-by: Akinobu Mita <akinobu.mita@gmail.com>

[-- Attachment #2: fix-sbc_dif_generate.patch --]
[-- Type: text/x-patch, Size: 485 bytes --]

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 3d88c00..d8d6267 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -1183,6 +1183,9 @@ sbc_dif_generate(struct se_cmd *cmd)
 	void *daddr, *paddr;
 	int i, j, offset = 0;
 
+	if (!psg)
+		return;
+
 	for_each_sg(cmd->t_data_sg, dsg, cmd->t_data_nents, i) {
 		daddr = kmap_atomic(sg_page(dsg)) + dsg->offset;
 		paddr = kmap_atomic(sg_page(psg)) + psg->offset;