Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, Jun 10, 2014 at 02:20:28PM -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 13:56 -0700, Linus Torvalds wrote: On Tue, Jun 10, 2014 at 1:25 PM, Nicholas A. Bellinger n...@linux-iscsi.org wrote: That would work, or I can simply include a pointer to Stephen's patch in the target-pending PULL request after the vhost API changes are merged and Linus can apply himself.. Yes. That way I'll include it in the merge, and everything should just work. nod, sounds good. MST, please drop the target related patches from your tree, and go ahead and send your PULL request now. --nab ack. Doing it right now. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Mon, 2014-06-09 at 16:30 +0300, Michael S. Tsirkin wrote: On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab OK since this conflicts with my vhost locking patches, I have rebased this and parked at vhost-review branch in my tree. git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-review Nicholas could you please take a look at the patches and confirm this is OK ASAP? If yes I will pack it all up and send to Linus. From my experience, Linus prefers to fix this type of conflict on his own at merge time, instead of having subsystem maintainers rebase their for-next branches to include other's commits. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Sun, 2014-06-08 at 19:05 +0300, Michael S. Tsirkin wrote: On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab OK, finally went over this, looks good to me: Acked-by: Michael S. Tsirkin m...@redhat.com However, we really should stop making more changes before fixing ANY_LAYOUT in this driver. virtio 1.0 should be out soon and that makes ANY_LAYOUT a required feature. What else is required to complete the ANY_LAYOUT conversion..? --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
Il 10/06/2014 09:07, Nicholas A. Bellinger ha scritto: OK, finally went over this, looks good to me: Acked-by: Michael S. Tsirkin m...@redhat.com However, we really should stop making more changes before fixing ANY_LAYOUT in this driver. virtio 1.0 should be out soon and that makes ANY_LAYOUT a required feature. What else is required to complete the ANY_LAYOUT conversion..? Basically, you need to stop expecting the request and response headers to be in a separate iov, and also vhost-scsi should not expect pi to be in a separate iov than the main data. A layout that has everything in the same iov would be fine, and similarly for a layout that has the CDB in a separate iov than the rest of the header. Paolo -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, Jun 10, 2014 at 12:05:12AM -0700, Nicholas A. Bellinger wrote: On Mon, 2014-06-09 at 16:30 +0300, Michael S. Tsirkin wrote: On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab OK since this conflicts with my vhost locking patches, I have rebased this and parked at vhost-review branch in my tree. git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-review Nicholas could you please take a look at the patches and confirm this is OK ASAP? If yes I will pack it all up and send to Linus. From my experience, Linus prefers to fix this type of conflict on his own at merge time, instead of having subsystem maintainers rebase their for-next branches to include other's commits. Cross-device type changes is exactly what vhost tree is there to allow so I don't see a problem. What Linus does not want is same patch in two trees. So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. - I pick up vhost scsi PI patches on my tree and you drop them from yours. I prefer option 2 because it's cleaner wrt bisect. But if you see a problem, pls let me know. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. --nab Yes but this does mean people trying to bisect will hit build breakages, not nice. -- MST -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au signature.asc Description: PGP signature
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote: Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Replacing bit access with has_feature silently fixes the bug. So if we want to avoid bisect breakage target tree will have to be rebased. And if doing that anyway, I don't see any reason not to merge everything through the vhost tree, esp since I already put the patches there. Less work for everyone involved. -- MST -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
Hi Michael, On Tue, 10 Jun 2014 16:02:08 +0300 Michael S. Tsirkin m...@redhat.com wrote: I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Ah ha! Indeed that is a different kettle of fish. :-) -- Cheers, Stephen Rothwells...@canb.auug.org.au signature.asc Description: PGP signature
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote: Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Replacing bit access with has_feature silently fixes the bug. So if we want to avoid bisect breakage target tree will have to be rebased. And if doing that anyway, I don't see any reason not to merge everything through the vhost tree, esp since I already put the patches there. Less work for everyone involved. The problem is with Sagi's recent changes wrt to including T10 PI bytes into expected data transfer length in target-core, you'll end up introducing a different bug into your tree.. ;) Why don't I simply add Stephen's patch to use vhost_has_feature() in target-pending/for-next, and we just make sure that the vhost PULL request goes out after target-pending..? --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote: Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Replacing bit access with has_feature silently fixes the bug. So if we want to avoid bisect breakage target tree will have to be rebased. And if doing that anyway, I don't see any reason not to merge everything through the vhost tree, esp since I already put the patches there. Less work for everyone involved. The problem is with Sagi's recent changes wrt to including T10 PI bytes into expected data transfer length in target-core, you'll end up introducing a different bug into your tree.. ;) Why don't I simply add Stephen's patch to use vhost_has_feature() in target-pending/for-next, and we just make sure that the vhost PULL request goes out after target-pending..? --nab Because that depends on vhost API changes :) -- MST -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote: Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Replacing bit access with has_feature silently fixes the bug. So if we want to avoid bisect breakage target tree will have to be rebased. And if doing that anyway, I don't see any reason not to merge everything through the vhost tree, esp since I already put the patches there. Less work for everyone involved. The problem is with Sagi's recent changes wrt to including T10 PI bytes into expected data transfer length in target-core, you'll end up introducing a different bug into your tree.. ;) I thought you wanted to fix it after -rc1 anyway? Why don't I simply add Stephen's patch to use vhost_has_feature() in target-pending/for-next, and we just make sure that the vhost PULL request goes out after target-pending..? --nab I can drop the PI feature bit in rc1. You will apply Sagi's changes and then enable the feature in rc2? -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, 2014-06-10 at 21:45 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote: Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Replacing bit access with has_feature silently fixes the bug. So if we want to avoid bisect breakage target tree will have to be rebased. And if doing that anyway, I don't see any reason not to merge everything through the vhost tree, esp since I already put the patches there. Less work for everyone involved. The problem is with Sagi's recent changes wrt to including T10 PI bytes into expected data transfer length in target-core, you'll end up introducing a different bug into your tree.. ;) Why don't I simply add Stephen's patch to use vhost_has_feature() in target-pending/for-next, and we just make sure that the vhost PULL request goes out after target-pending..? --nab Because that depends on vhost API changes :) Ugh, right. In that case, let's see what Linus (CC'ed) prefers to do.. --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, 2014-06-10 at 22:35 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote: Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Replacing bit access with has_feature silently fixes the bug. So if we want to avoid bisect breakage target tree will have to be rebased. And if doing that anyway, I don't see any reason not to merge everything through the vhost tree, esp since I already put the patches there. Less work for everyone involved. The problem is with Sagi's recent changes wrt to including T10 PI bytes into expected data transfer length in target-core, you'll end up introducing a different bug into your tree.. ;) I thought you wanted to fix it after -rc1 anyway? I've merged Sagi's patches and fixed up the resulting vhost-scsi breakage in target-pending/for-next here: https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?h=for-nextid=4101fc0abffeef604b14a707a3f9c9e2a8e39085 Only the qla2xxx target DIF related breakage remains, because those patches are going through the scsi/for-next tree and will need to be fixed in -rc2. Why don't I simply add Stephen's patch to use vhost_has_feature() in target-pending/for-next, and we just make sure that the vhost PULL request goes out after target-pending..? --nab I can drop the PI feature bit in rc1. You will apply Sagi's changes and then enable the feature in rc2? That is not necessary. I'll just apply Stephen's patch to use vhost_has_feature(), and then we make sure that target-pending is merged before vhost to avoid the build breakage. I'll be sending out the target-pending PULL request tomorrow afternoon and will CC' you. --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, 2014-06-10 at 12:57 -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 21:45 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote: Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Replacing bit access with has_feature silently fixes the bug. So if we want to avoid bisect breakage target tree will have to be rebased. And if doing that anyway, I don't see any reason not to merge everything through the vhost tree, esp since I already put the patches there. Less work for everyone involved. The problem is with Sagi's recent changes wrt to including T10 PI bytes into expected data transfer length in target-core, you'll end up introducing a different bug into your tree.. ;) Why don't I simply add Stephen's patch to use vhost_has_feature() in target-pending/for-next, and we just make sure that the vhost PULL request goes out after target-pending..? --nab Because that depends on vhost API changes :) Ugh, right. In that case, let's see what Linus (CC'ed) prefers to do.. Build a branch with all the patches dependent on the new API based on top of the vhost tree. Then you send it to Linus after the vhost tree is merged (otherwise you end up being the person sending the vhost tree). That way there's no API breakage and we get a consistent always buildable tree. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, 2014-06-10 at 13:09 -0700, James Bottomley wrote: On Tue, 2014-06-10 at 12:57 -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 21:45 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote: On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote: On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote: Hi Michael, On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: So I see two options: - I go ahead with my changes and you with yours and let Linus resolve the conflict. This means bisect build will be broken since the breakage will likely not be noticed until after the merge. Well, since the resolution is known, the one who submits their tree later should tell Linus (as suggested by Nicholas). That is part of the point of the linux-next tree ... and therefore there would be no bisect problem. Stephen (CC'ed) has included a fix in today's linux-next for the merge conflict here: https://lkml.org/lkml/2014/6/10/3 Please confirm, as it will be a pointer to Linus within the target-pending/for-next PULL request. Yes but this does mean people trying to bisect will hit build breakages, not nice. Not necessarily. -- Cheers, Stephen Rothwells...@canb.auug.org.au I don't see how that's possible. Here's a point you might have missed. Nicholas's patch isn't just introducing a merge conflict. It is also buggy. Replacing bit access with has_feature silently fixes the bug. So if we want to avoid bisect breakage target tree will have to be rebased. And if doing that anyway, I don't see any reason not to merge everything through the vhost tree, esp since I already put the patches there. Less work for everyone involved. The problem is with Sagi's recent changes wrt to including T10 PI bytes into expected data transfer length in target-core, you'll end up introducing a different bug into your tree.. ;) Why don't I simply add Stephen's patch to use vhost_has_feature() in target-pending/for-next, and we just make sure that the vhost PULL request goes out after target-pending..? --nab Because that depends on vhost API changes :) Ugh, right. In that case, let's see what Linus (CC'ed) prefers to do.. Build a branch with all the patches dependent on the new API based on top of the vhost tree. Then you send it to Linus after the vhost tree is merged (otherwise you end up being the person sending the vhost tree). That way there's no API breakage and we get a consistent always buildable tree. That would work, or I can simply include a pointer to Stephen's patch in the target-pending PULL request after the vhost API changes are merged and Linus can apply himself.. Linus, which do you prefer..? --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, Jun 10, 2014 at 1:25 PM, Nicholas A. Bellinger n...@linux-iscsi.org wrote: That would work, or I can simply include a pointer to Stephen's patch in the target-pending PULL request after the vhost API changes are merged and Linus can apply himself.. Yes. That way I'll include it in the merge, and everything should just work. Linus -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Tue, 2014-06-10 at 13:56 -0700, Linus Torvalds wrote: On Tue, Jun 10, 2014 at 1:25 PM, Nicholas A. Bellinger n...@linux-iscsi.org wrote: That would work, or I can simply include a pointer to Stephen's patch in the target-pending PULL request after the vhost API changes are merged and Linus can apply himself.. Yes. That way I'll include it in the merge, and everything should just work. nod, sounds good. MST, please drop the target related patches from your tree, and go ahead and send your PULL request now. --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
Il 08/06/2014 18:05, Michael S. Tsirkin ha scritto: OK, finally went over this, looks good to me: Acked-by: Michael S. Tsirkin m...@redhat.com However, we really should stop making more changes before fixing ANY_LAYOUT in this driver. virtio 1.0 should be out soon and that makes ANY_LAYOUT a required feature. Agreed. virtio-blk btw is getting ANY_LAYOUT support in QEMU 2.1, and virtio-scsi will follow suit. Paolo -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab OK since this conflicts with my vhost locking patches, I have rebased this and parked at vhost-review branch in my tree. git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-review Nicholas could you please take a look at the patches and confirm this is OK ASAP? If yes I will pack it all up and send to Linus. Nicholas Bellinger (6): virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits vhost/scsi: Move sanity check into vhost_scsi_map_iov_to_sgl vhost/scsi: Add preallocation of protection SGLs vhost/scsi: Add T10 PI IOV - SGL memory mapping logic vhost/scsi: Enable T10 PI IOV - SGL memory mapping virtio-scsi: Enable DIF/DIX modes in SCSI host LLD drivers/scsi/virtio_scsi.c | 86 +--- drivers/vhost/scsi.c| 305 +-- include/linux/virtio_scsi.h | 15 ++- 3 files changed, 292 insertions(+), 114 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab OK, finally went over this, looks good to me: Acked-by: Michael S. Tsirkin m...@redhat.com However, we really should stop making more changes before fixing ANY_LAYOUT in this driver. virtio 1.0 should be out soon and that makes ANY_LAYOUT a required feature. Nicholas Bellinger (6): virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits vhost/scsi: Move sanity check into vhost_scsi_map_iov_to_sgl vhost/scsi: Add preallocation of protection SGLs vhost/scsi: Add T10 PI IOV - SGL memory mapping logic vhost/scsi: Enable T10 PI IOV - SGL memory mapping virtio-scsi: Enable DIF/DIX modes in SCSI host LLD drivers/scsi/virtio_scsi.c | 86 +--- drivers/vhost/scsi.c| 305 +-- include/linux/virtio_scsi.h | 15 ++- 3 files changed, 292 insertions(+), 114 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab Acked-by: Michael S. Tsirkin m...@redhat.com Nicholas Bellinger (6): virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits vhost/scsi: Move sanity check into vhost_scsi_map_iov_to_sgl vhost/scsi: Add preallocation of protection SGLs vhost/scsi: Add T10 PI IOV - SGL memory mapping logic vhost/scsi: Enable T10 PI IOV - SGL memory mapping virtio-scsi: Enable DIF/DIX modes in SCSI host LLD drivers/scsi/virtio_scsi.c | 86 +--- drivers/vhost/scsi.c| 305 +-- include/linux/virtio_scsi.h | 15 ++- 3 files changed, 292 insertions(+), 114 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
Il 22/05/2014 04:26, Nicholas A. Bellinger ha scritto: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab Nicholas Bellinger (6): virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits vhost/scsi: Move sanity check into vhost_scsi_map_iov_to_sgl vhost/scsi: Add preallocation of protection SGLs vhost/scsi: Add T10 PI IOV - SGL memory mapping logic vhost/scsi: Enable T10 PI IOV - SGL memory mapping virtio-scsi: Enable DIF/DIX modes in SCSI host LLD drivers/scsi/virtio_scsi.c | 86 +--- drivers/vhost/scsi.c| 305 +-- include/linux/virtio_scsi.h | 15 ++- 3 files changed, 292 insertions(+), 114 deletions(-) Looks good, thanks! Paolo -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab Nicholas Bellinger (6): virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits vhost/scsi: Move sanity check into vhost_scsi_map_iov_to_sgl vhost/scsi: Add preallocation of protection SGLs vhost/scsi: Add T10 PI IOV - SGL memory mapping logic vhost/scsi: Enable T10 PI IOV - SGL memory mapping virtio-scsi: Enable DIF/DIX modes in SCSI host LLD drivers/scsi/virtio_scsi.c | 86 +--- drivers/vhost/scsi.c| 305 +-- include/linux/virtio_scsi.h | 15 ++- 3 files changed, 292 insertions(+), 114 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
