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

[Sagi Grimberg <sagig@dev.mellanox.co.il>]

On 4/15/2015 5:33 PM, Sagi Grimberg wrote:
> On 4/15/2015 5:16 PM, Akinobu Mita wrote:
>> 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.
>>

Actually this is a bug. Why didn't the initiator allocate integrity
meta-data for WRITE_SAME? Looking at the code it looks like it should.

Martin?

>> 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.
>>
>
> I don't think this is sufficient. With this we actually write
> unprotected data for WRITE_SAME (i.e. write data blocks but not storing
> the corresponding PI information). When this data will be read back you
> will see PI errors (you currently don't see those because your backend
> drive contains escape values I assume).
>
> I'd say the correct fix is to calc PI for the block and "write_same"
> it...
>
> Sagi.