From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sagi Grimberg Subject: Re: [RFC] Simlify dif_verify routines and fixup fileio protection information code. Date: Wed, 15 Apr 2015 18:08:06 +0300 Message-ID: <552E7ED6.4080304@dev.mellanox.co.il> References: <1428945575-30839-1-git-send-email-sagig@mellanox.com> <552D4C7B.9000402@dev.mellanox.co.il> <552E384F.3030203@dev.mellanox.co.il> <552E76D1.7040204@dev.mellanox.co.il> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <552E76D1.7040204@dev.mellanox.co.il> Sender: target-devel-owner@vger.kernel.org To: Akinobu Mita Cc: "Nicholas A. Bellinger" , target-devel@vger.kernel.org, "linux-scsi@vger.kernel.org" , "Martin K. Petersen" , Christoph Hellwig List-Id: linux-scsi@vger.kernel.org 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 : >>> 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.