* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Michael S. Tsirkin @ 2012-07-17 21:34 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Zhi Yong Wu,
Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini, lf-virt,
Christoph Hellwig
In-Reply-To: <1342559842.18004.440.camel@haakon2.linux-iscsi.org>
On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote:
> On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote:
> > On Wed, Jul 11, 2012 at 09:15:00PM +0000, Nicholas A. Bellinger wrote:
> > > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > >
> > > Hi folks,
> > >
> > > The following is a RFC-v2 series of tcm_vhost target fabric driver code
> > > currently in-flight for-3.6 mainline code.
> > >
> > > After last week's developments along with the help of some new folks, the
> > > changelog v1 -> v2 so far looks like:
> > >
> > > *) Fix drivers/vhost/test.c to use VHOST_NET_FEATURES in patch #1 (Asias He)
> > > *) Fix tv_cmd completion -> release SGL memory leak (nab)
> > > *) Fix sparse warnings for static variable usage (Fengguang Wu)
> > > *) Fix sparse warnings for min() typing + printk format specs (Fengguang Wu)
> > > *) Convert to cmwq submission for I/O dispatch (nab + hch)
> > >
> > > Also following Paolo's request, a patch for hw/virtio-scsi.c that sets
> > > scsi_host->max_target=0 that removes the need for virtio-scsi LLD to hardcode
> > > VirtIOSCSIConfig->max_id=1 in order to function with tcm_vhost.
> > >
> > > Note this series has been pushed into target-pending.git/for-next-merge, and
> > > should be getting picked up for tomorrow's linux-next build.
> > >
> > > Please let us know if you have any concerns and/or additional review feedback.
> > >
> > > Thank you!
> >
> >
> > It still seems not 100% clear whether this driver will have major
> > userspace using it. And if not, it would be very hard to support a driver
> > when recent userspace does not use it in the end.
> >
>
> I'm happy to commit to working with QEMU + kvm-tool folks to get to a
> series that can (eventually) see vhost-scsi support merged into upstream
> userspace code.
>
> It took roughly 2 years to get the megasas HBA emulation from Dr. Hannes
> merged, but certainly vhost-scsi has alot less moving pieces and
> hopefully alot less controversial bits than the buffer -> SGL
> conversion.. The key word being here 'hopefully'.. ;)
>
> > I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING.
> > Then we don't commit to an ABI.
> > For this, you can add a separate Kconfig and source it from drivers/staging/Kconfig.
> > Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig.
> >
>
> So tcm_vhost has been marked as Experimental following virtio-scsi.
>
> Wrt to staging, I'd like to avoid mucking with staging because:
>
> *) The code has been posted for review
> *) The code has been converted to use the latest target-core primitives
> *) The code does not require cleanups between staging -> merge
> *) The code has been stable the last 7 days since RFC-v2 with heavy
staging is not just for code that needs cleanups.
It's for anything that does not guarantee ABI stability yet.
And I think it's a bit early to guarantee ABI stability - 7 days
is not all that long.
See for example Anthony's comments that raise exactly the ABI
issues.
> Also, tcm_vhost has been marked as Experimental following virtio-scsi.
> I'd much rather leave it at Experimental until we merge upstream
> userspace support. If userspace support never ends up materializing,
> I'm fine with dropping it all together.
Once it's in kernel you never know who will use this driver.
Experimental does not mean driver can be dropped, staging does.
> However at this point given that there is a 3x performance gap between
> virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block
> I/O, and we still need the latter to do proper SCSI CDB passthrough for
> non TYPE_DISK devices I'm hoping that we can agree on userspace bits
> once tcm_vhost is merged.
>
> --nab
I do think upstream kernel would help you nail userspace issues too
but at this point it looks like either staging meterial or 3.6 is too
early.
--
MST
^ permalink raw reply
* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Nicholas A. Bellinger @ 2012-07-17 21:50 UTC (permalink / raw)
To: Anthony Liguori
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin,
lf-virt, Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini,
Zhi Yong Wu, Christoph Hellwig
In-Reply-To: <5005B52E.20509@us.ibm.com>
On Tue, 2012-07-17 at 13:55 -0500, Anthony Liguori wrote:
> On 07/17/2012 10:05 AM, Michael S. Tsirkin wrote:
> > On Wed, Jul 11, 2012 at 09:15:00PM +0000, Nicholas A. Bellinger wrote:
<SNIP>
> >
> > It still seems not 100% clear whether this driver will have major
> > userspace using it. And if not, it would be very hard to support a driver
> > when recent userspace does not use it in the end.
>
> I don't think this is a good reason to exclude something from the kernel.
> However, there are good reasons why this doesn't make sense for something like
> QEMU--specifically because we have a large number of features in our block layer
> that tcm_vhost would bypass.
>
I can definitely appreciate your concern here as the QEMU maintainer.
> But perhaps it makes sense for something like native kvm tool. And if it did go
> into the kernel, we would certainly support it in QEMU.
>
...
> But I do think the kernel should carefully consider whether it wants to support
> an interface like this. This an extremely complicated ABI with a lot of subtle
> details around state and compatibility.
>
> Are you absolutely confident that you can support a userspace application that
> expects to get exactly the same response from all possible commands in 20 kernel
> versions from now? Virtualization requires absolutely precise compatibility in
> terms of bugs and features. This is probably not something the TCM stack has
> had to consider yet.
>
We most certainly have thought about long term userspace compatibility
with TCM. Our userspace code (that's now available in all major
distros) is completely forward-compatible with new fabric modules such
as tcm_vhost. No update required.
Also, by virtue of the fact that we are using configfs + rtslib (python
object library) on top, it's very easy to keep any type of compatibility
logic around in python code. With rtslib, we are able to hide configfs
ABI changes from higher level apps.
So far we've had a track record of 100% userspace ABI compatibility in
mainline since .38, and I don't intend to merge a patch that breaks this
any time soon. But if that ever happens, apps using rtslib are not
going to be effected.
> > I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING.
> > Then we don't commit to an ABI.
>
> I think this is a good idea. Even if it goes in, a really clear policy would be
> needed wrt the userspace ABI.
>
> While tcm_vhost is probably more useful than vhost_blk, it's a much more complex
> ABI to maintain.
>
As far as I am concerned, the kernel API (eg: configfs directory layout)
as it is now in sys/kernel/config/target/vhost/ is not going to change.
It's based on the same drivers/target/target_core_fabric_configfs.c
generic layout that we've had since .38.
The basic functional fabric layout in configfs is identical (with fabric
dependent WWPN naming of course) regardless of fabric driver, and by
virtue of being generic it means we can add things like fabric dependent
attributes + parameters in the future for existing fabrics without
breaking userspace.
So while I agree the ABI is more complex than vhost-blk, the logic in
target_core_fabric_configfs.c is a basic ABI fabric definition that we
are enforcing across all fabric modules in mainline for long term
compatibility.
--nab
^ permalink raw reply
* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Michael S. Tsirkin @ 2012-07-17 21:58 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Zhi Yong Wu,
Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini, lf-virt,
Christoph Hellwig
In-Reply-To: <1342559842.18004.440.camel@haakon2.linux-iscsi.org>
On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote:
> Wrt to staging, I'd like to avoid mucking with staging because:
>
> *) The code has been posted for review
> *) The code has been converted to use the latest target-core primitives
> *) The code does not require cleanups between staging -> merge
> *) The code has been stable the last 7 days since RFC-v2 with heavy
BTW I don't suggest putting code itself in staging. Just the config
flag to enable it. Once we are more or less sure multiple userspaces
are using this driver, we'll move the config hopefully already in 3.7.
What's the downside?
--
MST
^ permalink raw reply
* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Nicholas A. Bellinger @ 2012-07-17 22:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Zhi Yong Wu,
Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini, lf-virt,
Christoph Hellwig
In-Reply-To: <20120717213437.GC1868@redhat.com>
On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote:
> On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote:
> > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote:
> > > On Wed, Jul 11, 2012 at 09:15:00PM +0000, Nicholas A. Bellinger wrote:
> > > > From: Nicholas Bellinger <nab@linux-iscsi.org>
<SNIP>
> > I'm happy to commit to working with QEMU + kvm-tool folks to get to a
> > series that can (eventually) see vhost-scsi support merged into upstream
> > userspace code.
> >
> > It took roughly 2 years to get the megasas HBA emulation from Dr. Hannes
> > merged, but certainly vhost-scsi has alot less moving pieces and
> > hopefully alot less controversial bits than the buffer -> SGL
> > conversion.. The key word being here 'hopefully'.. ;)
> >
> > > I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING.
> > > Then we don't commit to an ABI.
> > > For this, you can add a separate Kconfig and source it from drivers/staging/Kconfig.
> > > Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig.
> > >
> >
> > So tcm_vhost has been marked as Experimental following virtio-scsi.
> >
> > Wrt to staging, I'd like to avoid mucking with staging because:
> >
> > *) The code has been posted for review
> > *) The code has been converted to use the latest target-core primitives
> > *) The code does not require cleanups between staging -> merge
> > *) The code has been stable the last 7 days since RFC-v2 with heavy
>
> staging is not just for code that needs cleanups.
> It's for anything that does not guarantee ABI stability yet.
> And I think it's a bit early to guarantee ABI stability - 7 days
> is not all that long.
>
I was talking about the new I/O path has been running for 7 days.
> See for example Anthony's comments that raise exactly the ABI
> issues.
>
As mentioned in the response to Anthony, we are using the same generic
fabric ABI in drivers/target/target_core_fabric_configfs.c since .38.
That part is not going to change, and it has not changed for any of the
other 7 target fabric modules we've merged into mainline since then.
> > Also, tcm_vhost has been marked as Experimental following virtio-scsi.
> > I'd much rather leave it at Experimental until we merge upstream
> > userspace support. If userspace support never ends up materializing,
> > I'm fine with dropping it all together.
>
> Once it's in kernel you never know who will use this driver.
> Experimental does not mean driver can be dropped, staging does.
>
Yes, that's the point of being in mainline. People using the code,
right..?
> > However at this point given that there is a 3x performance gap between
> > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block
> > I/O, and we still need the latter to do proper SCSI CDB passthrough for
> > non TYPE_DISK devices I'm hoping that we can agree on userspace bits
> > once tcm_vhost is merged.
> >
> > --nab
>
> I do think upstream kernel would help you nail userspace issues too
> but at this point it looks like either staging meterial or 3.6 is too
> early.
>
I think for-3.6 is just the right time for this kernel code. Seriously,
The basic ABI fabric layout for /sys/kernel/config/target/vhost/ is
going to be the same now for-3.6, the same for-3.7, and the same for .38
code.
I'd be happy to move tcm_vhost back to drivers/target/ for now, and we
move it to drivers/vhost/ once the userspace bits are worked out..?
Would that be a reasonable compromise to move forward..?
--nab
^ permalink raw reply
* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Nicholas A. Bellinger @ 2012-07-17 22:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Zhi Yong Wu,
Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini, lf-virt,
Christoph Hellwig
In-Reply-To: <20120717215848.GD1868@redhat.com>
On Wed, 2012-07-18 at 00:58 +0300, Michael S. Tsirkin wrote:
> On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote:
> > Wrt to staging, I'd like to avoid mucking with staging because:
> >
> > *) The code has been posted for review
> > *) The code has been converted to use the latest target-core primitives
> > *) The code does not require cleanups between staging -> merge
> > *) The code has been stable the last 7 days since RFC-v2 with heavy
>
> BTW I don't suggest putting code itself in staging. Just the config
> flag to enable it. Once we are more or less sure multiple userspaces
> are using this driver, we'll move the config hopefully already in 3.7.
> What's the downside?
>
Ahh, sorry I managed to miss that part.. ;)
If it's just a CONFIG_STAGING flag for a release or two until we work
out the userspace bits, I don't have an objection doing something like
that if it helps getting the code exposed to a large set of eyes in
mainline.
--nab
^ permalink raw reply
* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Michael S. Tsirkin @ 2012-07-17 22:18 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Zhi Yong Wu,
Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini, lf-virt,
Christoph Hellwig
In-Reply-To: <1342562528.18004.480.camel@haakon2.linux-iscsi.org>
On Tue, Jul 17, 2012 at 03:02:08PM -0700, Nicholas A. Bellinger wrote:
> On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote:
> > On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote:
> > > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote:
> > > > On Wed, Jul 11, 2012 at 09:15:00PM +0000, Nicholas A. Bellinger wrote:
> > > > > From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> <SNIP>
>
> > > I'm happy to commit to working with QEMU + kvm-tool folks to get to a
> > > series that can (eventually) see vhost-scsi support merged into upstream
> > > userspace code.
> > >
> > > It took roughly 2 years to get the megasas HBA emulation from Dr. Hannes
> > > merged, but certainly vhost-scsi has alot less moving pieces and
> > > hopefully alot less controversial bits than the buffer -> SGL
> > > conversion.. The key word being here 'hopefully'.. ;)
> > >
> > > > I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING.
> > > > Then we don't commit to an ABI.
> > > > For this, you can add a separate Kconfig and source it from drivers/staging/Kconfig.
> > > > Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig.
> > > >
> > >
> > > So tcm_vhost has been marked as Experimental following virtio-scsi.
> > >
> > > Wrt to staging, I'd like to avoid mucking with staging because:
> > >
> > > *) The code has been posted for review
> > > *) The code has been converted to use the latest target-core primitives
> > > *) The code does not require cleanups between staging -> merge
> > > *) The code has been stable the last 7 days since RFC-v2 with heavy
> >
> > staging is not just for code that needs cleanups.
> > It's for anything that does not guarantee ABI stability yet.
> > And I think it's a bit early to guarantee ABI stability - 7 days
> > is not all that long.
> >
>
> I was talking about the new I/O path has been running for 7 days.
>
> > See for example Anthony's comments that raise exactly the ABI
> > issues.
> >
>
> As mentioned in the response to Anthony, we are using the same generic
> fabric ABI in drivers/target/target_core_fabric_configfs.c since .38.
>
> That part is not going to change, and it has not changed for any of the
> other 7 target fabric modules we've merged into mainline since then.
>
> > > Also, tcm_vhost has been marked as Experimental following virtio-scsi.
> > > I'd much rather leave it at Experimental until we merge upstream
> > > userspace support. If userspace support never ends up materializing,
> > > I'm fine with dropping it all together.
> >
> > Once it's in kernel you never know who will use this driver.
> > Experimental does not mean driver can be dropped, staging does.
> >
>
> Yes, that's the point of being in mainline. People using the code,
> right..?
Exactly. I am just worried about in the end being no major users and we
are being stuck with a niche driver that as a result is very hard to test.
And the reason for the fear is the initial negative reaction from the
qemu side. And no if it's there we can't just drop it.
> > > However at this point given that there is a 3x performance gap between
> > > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block
> > > I/O, and we still need the latter to do proper SCSI CDB passthrough for
> > > non TYPE_DISK devices I'm hoping that we can agree on userspace bits
> > > once tcm_vhost is merged.
> > >
> > > --nab
> >
> > I do think upstream kernel would help you nail userspace issues too
> > but at this point it looks like either staging meterial or 3.6 is too
> > early.
> >
>
> I think for-3.6 is just the right time for this kernel code. Seriously,
> The basic ABI fabric layout for /sys/kernel/config/target/vhost/ is
> going to be the same now for-3.6, the same for-3.7, and the same for .38
> code.
>
> I'd be happy to move tcm_vhost back to drivers/target/ for now, and we
> move it to drivers/vhost/ once the userspace bits are worked out..?
>
> Would that be a reasonable compromise to move forward..?
>
> --nab
I don't see how it helps. The driver is either a guaranteed ABI or not.
I'd prefer not to have vhost users outside drivers/vhost/ since it is
harder for me to keep track of them.
What's the problem with staging proposal? It's just another hoop to jump
through to enable it?
--
MST
^ permalink raw reply
* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Nicholas A. Bellinger @ 2012-07-17 22:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Zhi Yong Wu,
Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini, lf-virt,
Christoph Hellwig
In-Reply-To: <20120717221814.GH1868@redhat.com>
On Wed, 2012-07-18 at 01:18 +0300, Michael S. Tsirkin wrote:
> On Tue, Jul 17, 2012 at 03:02:08PM -0700, Nicholas A. Bellinger wrote:
> > On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote:
> > > On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote:
> > > > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote:
> > > > > On Wed, Jul 11, 2012 at 09:15:00PM +0000, Nicholas A. Bellinger wrote:
> > > > > > From: Nicholas Bellinger <nab@linux-iscsi.org>
<SNIP>
> > As mentioned in the response to Anthony, we are using the same generic
> > fabric ABI in drivers/target/target_core_fabric_configfs.c since .38.
> >
> > That part is not going to change, and it has not changed for any of the
> > other 7 target fabric modules we've merged into mainline since then.
> >
> > > > Also, tcm_vhost has been marked as Experimental following virtio-scsi.
> > > > I'd much rather leave it at Experimental until we merge upstream
> > > > userspace support. If userspace support never ends up materializing,
> > > > I'm fine with dropping it all together.
> > >
> > > Once it's in kernel you never know who will use this driver.
> > > Experimental does not mean driver can be dropped, staging does.
> > >
> >
> > Yes, that's the point of being in mainline. People using the code,
> > right..?
>
> Exactly. I am just worried about in the end being no major users and we
> are being stuck with a niche driver that as a result is very hard to test.
> And the reason for the fear is the initial negative reaction from the
> qemu side. And no if it's there we can't just drop it.
>
That is certainly a reasonable concern..
> > > > However at this point given that there is a 3x performance gap between
> > > > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block
> > > > I/O, and we still need the latter to do proper SCSI CDB passthrough for
> > > > non TYPE_DISK devices I'm hoping that we can agree on userspace bits
> > > > once tcm_vhost is merged.
> > > >
> > > > --nab
> > >
> > > I do think upstream kernel would help you nail userspace issues too
> > > but at this point it looks like either staging meterial or 3.6 is too
> > > early.
> > >
> >
> > I think for-3.6 is just the right time for this kernel code. Seriously,
> > The basic ABI fabric layout for /sys/kernel/config/target/vhost/ is
> > going to be the same now for-3.6, the same for-3.7, and the same for .38
> > code.
> >
> > I'd be happy to move tcm_vhost back to drivers/target/ for now, and we
> > move it to drivers/vhost/ once the userspace bits are worked out..?
> >
> > Would that be a reasonable compromise to move forward..?
> >
> > --nab
>
> I don't see how it helps. The driver is either a guaranteed ABI or not.
> I'd prefer not to have vhost users outside drivers/vhost/ since it is
> harder for me to keep track of them.
>
> What's the problem with staging proposal? It's just another hoop to jump
> through to enable it?
>
Yeah, I'm OK with just adding a CONFIG_STAGING tag is a reasonable step
forward for-3.6.
Adding the following patch into target-pending/for-next-merge now:
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index ccbeb6f..2cd7135 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -11,7 +11,7 @@ config VHOST_NET
config TCM_VHOST
tristate "TCM_VHOST fabric module (EXPERIMENTAL)"
- depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m
+ depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && STAGING && m
default n
---help---
Say M here to enable the TCM_VHOST fabric module for use with virtio-scsi guests
^ permalink raw reply related
* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Michael S. Tsirkin @ 2012-07-17 23:11 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Zhi Yong Wu,
Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini, lf-virt,
Christoph Hellwig
In-Reply-To: <1342564640.18004.492.camel@haakon2.linux-iscsi.org>
On Tue, Jul 17, 2012 at 03:37:20PM -0700, Nicholas A. Bellinger wrote:
> On Wed, 2012-07-18 at 01:18 +0300, Michael S. Tsirkin wrote:
> > On Tue, Jul 17, 2012 at 03:02:08PM -0700, Nicholas A. Bellinger wrote:
> > > On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote:
> > > > On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote:
> > > > > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote:
> > > > > > On Wed, Jul 11, 2012 at 09:15:00PM +0000, Nicholas A. Bellinger wrote:
> > > > > > > From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> <SNIP>
>
> > > As mentioned in the response to Anthony, we are using the same generic
> > > fabric ABI in drivers/target/target_core_fabric_configfs.c since .38.
> > >
> > > That part is not going to change, and it has not changed for any of the
> > > other 7 target fabric modules we've merged into mainline since then.
> > >
> > > > > Also, tcm_vhost has been marked as Experimental following virtio-scsi.
> > > > > I'd much rather leave it at Experimental until we merge upstream
> > > > > userspace support. If userspace support never ends up materializing,
> > > > > I'm fine with dropping it all together.
> > > >
> > > > Once it's in kernel you never know who will use this driver.
> > > > Experimental does not mean driver can be dropped, staging does.
> > > >
> > >
> > > Yes, that's the point of being in mainline. People using the code,
> > > right..?
> >
> > Exactly. I am just worried about in the end being no major users and we
> > are being stuck with a niche driver that as a result is very hard to test.
> > And the reason for the fear is the initial negative reaction from the
> > qemu side. And no if it's there we can't just drop it.
> >
>
> That is certainly a reasonable concern..
>
> > > > > However at this point given that there is a 3x performance gap between
> > > > > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block
> > > > > I/O, and we still need the latter to do proper SCSI CDB passthrough for
> > > > > non TYPE_DISK devices I'm hoping that we can agree on userspace bits
> > > > > once tcm_vhost is merged.
> > > > >
> > > > > --nab
> > > >
> > > > I do think upstream kernel would help you nail userspace issues too
> > > > but at this point it looks like either staging meterial or 3.6 is too
> > > > early.
> > > >
> > >
> > > I think for-3.6 is just the right time for this kernel code. Seriously,
> > > The basic ABI fabric layout for /sys/kernel/config/target/vhost/ is
> > > going to be the same now for-3.6, the same for-3.7, and the same for .38
> > > code.
> > >
> > > I'd be happy to move tcm_vhost back to drivers/target/ for now, and we
> > > move it to drivers/vhost/ once the userspace bits are worked out..?
> > >
> > > Would that be a reasonable compromise to move forward..?
> > >
> > > --nab
> >
> > I don't see how it helps. The driver is either a guaranteed ABI or not.
> > I'd prefer not to have vhost users outside drivers/vhost/ since it is
> > harder for me to keep track of them.
> >
> > What's the problem with staging proposal? It's just another hoop to jump
> > through to enable it?
> >
>
> Yeah, I'm OK with just adding a CONFIG_STAGING tag is a reasonable step
> forward for-3.6.
>
> Adding the following patch into target-pending/for-next-merge now:
>
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index ccbeb6f..2cd7135 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -11,7 +11,7 @@ config VHOST_NET
>
> config TCM_VHOST
> tristate "TCM_VHOST fabric module (EXPERIMENTAL)"
> - depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m
> + depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && STAGING && m
> default n
> ---help---
> Say M here to enable the TCM_VHOST fabric module for use with virtio-scsi guests
>
>
Hmm that's not explicit enough, someone might enable CONFIG_STAGING for
some other reason and won't notice the dependency.
We need it to appear with other staging drivers in the menu,
so there needs to be a Kconfig that is included from
drivers/staging/Kconfig.
For example, we can create
drivers/vhost/staging/Kconfig or drivers/vhost/tcm/Kconfig and include
it from drivers/staging/Kconfig. nouveau did something like this for a
while, see f3c93cbde7eab38671ae085cb1027b08f5f36757.
No need to move the rest of the code.
--
MST
^ permalink raw reply
* Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Nicholas A. Bellinger @ 2012-07-18 0:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Greg KH, Zhi Yong Wu,
Anthony Liguori, target-devel, linux-scsi, Paolo Bonzini, lf-virt,
Christoph Hellwig
In-Reply-To: <20120717231113.GA3004@redhat.com>
On Wed, 2012-07-18 at 02:11 +0300, Michael S. Tsirkin wrote:
> On Tue, Jul 17, 2012 at 03:37:20PM -0700, Nicholas A. Bellinger wrote:
> > On Wed, 2012-07-18 at 01:18 +0300, Michael S. Tsirkin wrote:
> > > On Tue, Jul 17, 2012 at 03:02:08PM -0700, Nicholas A. Bellinger wrote:
> > > > On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote:
<SNIP>
> > >
> > > I don't see how it helps. The driver is either a guaranteed ABI or not.
> > > I'd prefer not to have vhost users outside drivers/vhost/ since it is
> > > harder for me to keep track of them.
> > >
> > > What's the problem with staging proposal? It's just another hoop to jump
> > > through to enable it?
> > >
> >
> > Yeah, I'm OK with just adding a CONFIG_STAGING tag is a reasonable step
> > forward for-3.6.
> >
> > Adding the following patch into target-pending/for-next-merge now:
> >
> > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> > index ccbeb6f..2cd7135 100644
> > --- a/drivers/vhost/Kconfig
> > +++ b/drivers/vhost/Kconfig
> > @@ -11,7 +11,7 @@ config VHOST_NET
> >
> > config TCM_VHOST
> > tristate "TCM_VHOST fabric module (EXPERIMENTAL)"
> > - depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m
> > + depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && STAGING && m
> > default n
> > ---help---
> > Say M here to enable the TCM_VHOST fabric module for use with virtio-scsi guests
> >
> >
>
> Hmm that's not explicit enough, someone might enable CONFIG_STAGING for
> some other reason and won't notice the dependency.
> We need it to appear with other staging drivers in the menu,
> so there needs to be a Kconfig that is included from
> drivers/staging/Kconfig.
>
Mmmmm, I am sensing a linux-next merge conflict with staging-next and/or
another for-next-merge rebase coming on..
> For example, we can create
> drivers/vhost/staging/Kconfig or drivers/vhost/tcm/Kconfig and include
> it from drivers/staging/Kconfig. nouveau did something like this for a
> while, see f3c93cbde7eab38671ae085cb1027b08f5f36757.
>
> No need to move the rest of the code.
>
OK, lets use drivers/vhost/tcm/Kconfig and I'll post a incremental patch
to make it appear under staging it shortly.
(CC'ing Greg-KH for good measure.)
^ permalink raw reply
* [RFC-v3 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Nicholas A. Bellinger @ 2012-07-18 0:59 UTC (permalink / raw)
To: target-devel
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, Zhi Yong Wu, Anthony Liguori, linux-scsi,
Paolo Bonzini, lf-virt, Christoph Hellwig
From: Nicholas Bellinger <nab@linux-iscsi.org>
Hi folks,
The following is the RFC-v3 series of tcm_vhost target fabric driver code
currently in-flight for-3.6 mainline code.
With the merge window opening soon, the tcm_vhost code has started seeing
time in linux-next. The v2 -> v3 changelog from the last week is currently
looking like:
*) Unlock on error in tcm_vhost_drop_nexus() (DanC)
*) Fix strlen() doesn't count the terminator (DanC)
*) Call kfree() on an error path (DanC)
*) Convert tcm_vhost_write_pending to use target_execute_cmd (hch + nab)
*) Fix another strlen() off by one in tcm_vhost_make_tport (DanC)
*) Add option under drivers/staging/Kconfig, and move to drivers/vhost/tcm/
as requested by MST (nab)
Thanks to Dan Carpenter for his smatch fixes this past round.
Also as requested by MST, the code has been moved to a seperate tcm/ subdirectory
under drivers/vhost/ so that it can be included under staging's config options
until we can settle on the necessary userspace bits for QEMU and kvm-tool.
The updated series will be going out shortly to target-pending/for-next-merge.
Please have another look and let us know if you have any concerned.
Thanks!
Nicholas Bellinger (2):
vhost: Add vhost_scsi specific defines
tcm_vhost: Initial merge for vhost level target fabric driver
Stefan Hajnoczi (2):
vhost: Separate vhost-net features from vhost features
vhost: make vhost work queue visible
drivers/staging/Kconfig | 2 +
drivers/vhost/Makefile | 2 +
drivers/vhost/net.c | 4 +-
drivers/vhost/tcm/Kconfig | 6 +
drivers/vhost/tcm/Makefile | 1 +
drivers/vhost/tcm/tcm_vhost.c | 1611 +++++++++++++++++++++++++++++++++++++++++
drivers/vhost/tcm/tcm_vhost.h | 74 ++
drivers/vhost/test.c | 4 +-
drivers/vhost/vhost.c | 5 +-
drivers/vhost/vhost.h | 6 +-
include/linux/vhost.h | 9 +
11 files changed, 1716 insertions(+), 8 deletions(-)
create mode 100644 drivers/vhost/tcm/Kconfig
create mode 100644 drivers/vhost/tcm/Makefile
create mode 100644 drivers/vhost/tcm/tcm_vhost.c
create mode 100644 drivers/vhost/tcm/tcm_vhost.h
--
1.7.2.5
^ permalink raw reply
* [RFC-v3 1/4] vhost: Separate vhost-net features from vhost features
From: Nicholas A. Bellinger @ 2012-07-18 0:59 UTC (permalink / raw)
To: target-devel
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, Zhi Yong Wu, Anthony Liguori, linux-scsi,
Paolo Bonzini, lf-virt, Christoph Hellwig
In-Reply-To: <1342573172-17421-1-git-send-email-nab@linux-iscsi.org>
From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
In order for other vhost devices to use the VHOST_FEATURES bits the
vhost-net specific bits need to be moved to their own VHOST_NET_FEATURES
constant.
(Asias: Update drivers/vhost/test.c to use VHOST_NET_FEATURES)
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Zhi Yong Wu <wuzhy@cn.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Asias He <asias@redhat.com>
Signed-off-by: Nicholas A. Bellinger <nab@risingtidesystems.com>
---
drivers/vhost/net.c | 4 ++--
drivers/vhost/test.c | 4 ++--
drivers/vhost/vhost.h | 3 ++-
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f82a739..072cbba 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -823,14 +823,14 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
return -EFAULT;
return vhost_net_set_backend(n, backend.index, backend.fd);
case VHOST_GET_FEATURES:
- features = VHOST_FEATURES;
+ features = VHOST_NET_FEATURES;
if (copy_to_user(featurep, &features, sizeof features))
return -EFAULT;
return 0;
case VHOST_SET_FEATURES:
if (copy_from_user(&features, featurep, sizeof features))
return -EFAULT;
- if (features & ~VHOST_FEATURES)
+ if (features & ~VHOST_NET_FEATURES)
return -EOPNOTSUPP;
return vhost_net_set_features(n, features);
case VHOST_RESET_OWNER:
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 3de00d9..91d6f06 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -261,14 +261,14 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
return -EFAULT;
return vhost_test_run(n, test);
case VHOST_GET_FEATURES:
- features = VHOST_FEATURES;
+ features = VHOST_NET_FEATURES;
if (copy_to_user(featurep, &features, sizeof features))
return -EFAULT;
return 0;
case VHOST_SET_FEATURES:
if (copy_from_user(&features, featurep, sizeof features))
return -EFAULT;
- if (features & ~VHOST_FEATURES)
+ if (features & ~VHOST_NET_FEATURES)
return -EOPNOTSUPP;
return vhost_test_set_features(n, features);
case VHOST_RESET_OWNER:
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8de1fd5..07b9763 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -201,7 +201,8 @@ enum {
VHOST_FEATURES = (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
(1ULL << VIRTIO_RING_F_EVENT_IDX) |
- (1ULL << VHOST_F_LOG_ALL) |
+ (1ULL << VHOST_F_LOG_ALL),
+ VHOST_NET_FEATURES = VHOST_FEATURES |
(1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
(1ULL << VIRTIO_NET_F_MRG_RXBUF),
};
--
1.7.2.5
^ permalink raw reply related
* [RFC-v3 2/4] vhost: make vhost work queue visible
From: Nicholas A. Bellinger @ 2012-07-18 0:59 UTC (permalink / raw)
To: target-devel
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, Zhi Yong Wu, Anthony Liguori, linux-scsi,
Paolo Bonzini, lf-virt, Christoph Hellwig
In-Reply-To: <1342573172-17421-1-git-send-email-nab@linux-iscsi.org>
From: Stefan Hajnoczi <stefanha@gmail.com>
The vhost work queue allows processing to be done in vhost worker thread
context, which uses the owner process mm. Access to the vring and guest
memory is typically only possible from vhost worker context so it is
useful to allow work to be queued directly by users.
Currently vhost_net only uses the poll wrappers which do not expose the
work queue functions. However, for tcm_vhost (vhost_scsi) it will be
necessary to queue custom work.
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Zhi Yong Wu <wuzhy@cn.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
drivers/vhost/vhost.c | 5 ++---
drivers/vhost/vhost.h | 3 +++
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 94dbd25..1aab08b 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -64,7 +64,7 @@ static int vhost_poll_wakeup(wait_queue_t *wait, unsigned mode, int sync,
return 0;
}
-static void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
+void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
{
INIT_LIST_HEAD(&work->node);
work->fn = fn;
@@ -137,8 +137,7 @@ void vhost_poll_flush(struct vhost_poll *poll)
vhost_work_flush(poll->dev, &poll->work);
}
-static inline void vhost_work_queue(struct vhost_dev *dev,
- struct vhost_work *work)
+void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work)
{
unsigned long flags;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 07b9763..1125af3 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -43,6 +43,9 @@ struct vhost_poll {
struct vhost_dev *dev;
};
+void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
+void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
+
void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
unsigned long mask, struct vhost_dev *dev);
void vhost_poll_start(struct vhost_poll *poll, struct file *file);
--
1.7.2.5
^ permalink raw reply related
* [RFC-v3 3/4] vhost: Add vhost_scsi specific defines
From: Nicholas A. Bellinger @ 2012-07-18 0:59 UTC (permalink / raw)
To: target-devel
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, Zhi Yong Wu, Anthony Liguori, linux-scsi,
Paolo Bonzini, lf-virt, Nicholas Bellinger, Christoph Hellwig
In-Reply-To: <1342573172-17421-1-git-send-email-nab@linux-iscsi.org>
From: Nicholas Bellinger <nab@risingtidesystems.com>
This patch adds the initial vhost_scsi_ioctl() callers for VHOST_SCSI_SET_ENDPOINT
and VHOST_SCSI_CLEAR_ENDPOINT respectively, and also adds struct vhost_vring_target
that is used by tcm_vhost code when locating target ports during qemu setup.
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Zhi Yong Wu <wuzhy@cn.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Signed-off-by: Nicholas A. Bellinger <nab@risingtidesystems.com>
---
include/linux/vhost.h | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/include/linux/vhost.h b/include/linux/vhost.h
index e847f1e..33b313b 100644
--- a/include/linux/vhost.h
+++ b/include/linux/vhost.h
@@ -24,7 +24,11 @@ struct vhost_vring_state {
struct vhost_vring_file {
unsigned int index;
int fd; /* Pass -1 to unbind from file. */
+};
+struct vhost_vring_target {
+ unsigned char vhost_wwpn[224];
+ unsigned short vhost_tpgt;
};
struct vhost_vring_addr {
@@ -121,6 +125,11 @@ struct vhost_memory {
* device. This can be used to stop the ring (e.g. for migration). */
#define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0x30, struct vhost_vring_file)
+/* VHOST_SCSI specific defines */
+
+#define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_vring_target)
+#define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_vring_target)
+
/* Feature bits */
/* Log all write descriptors. Can be changed while device is active. */
#define VHOST_F_LOG_ALL 26
--
1.7.2.5
^ permalink raw reply related
* [RFC-v3 4/4] tcm_vhost: Initial merge for vhost level target fabric driver
From: Nicholas A. Bellinger @ 2012-07-18 0:59 UTC (permalink / raw)
To: target-devel
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, Zhi Yong Wu, Anthony Liguori, linux-scsi,
Paolo Bonzini, lf-virt, Christoph Hellwig
In-Reply-To: <1342573172-17421-1-git-send-email-nab@linux-iscsi.org>
From: Nicholas Bellinger <nab@linux-iscsi.org>
This patch adds the initial code for tcm_vhost, a Vhost level TCM
fabric driver for virtio SCSI initiators into KVM guest.
This code is currently up and running on v3.5-rc2 host+guest along
with the virtio-scsi vdev->scan() patch to allow a proper
scsi_scan_host() to occur once the tcm_vhost nexus has been established
by the paravirtualized virtio-scsi client here:
virtio-scsi: Add vdrv->scan for post VIRTIO_CONFIG_S_DRIVER_OK LUN scanning
http://marc.info/?l=linux-scsi&m=134160609212542&w=2
Using tcm_vhost requires Zhi's -> Stefan's qemu vhost-scsi tree here:
https://github.com/wuzhy/qemu/tree/vhost-scsi
along with the recent QEMU patch to hw/virtio-scsi.c to set max_target=0
during vhost-scsi operation.
Changelog v2 -> v3:
Unlock on error in tcm_vhost_drop_nexus() (DanC)
Fix strlen() doesn't count the terminator (DanC)
Call kfree() on an error path (DanC)
Convert tcm_vhost_write_pending to use target_execute_cmd (hch + nab)
Fix another strlen() off by one in tcm_vhost_make_tport (DanC)
Add option under drivers/staging/Kconfig, and move to drivers/vhost/tcm/
as requested by MST (nab)
Changelog v1 -> v2:
Fix tv_cmd completion -> release SGL memory leak (nab)
Fix sparse warnings for static variable usage ((Fengguang Wu)
Fix sparse warnings for min() typing + printk format specs (Fengguang Wu)
Convert to cmwq submission for I/O dispatch (nab + hch)
Changelog v0 -> v1:
Merge into single source + header file, and move to drivers/vhost/
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Zhi Yong Wu <wuzhy@cn.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
drivers/staging/Kconfig | 2 +
drivers/vhost/Makefile | 2 +
drivers/vhost/tcm/Kconfig | 6 +
drivers/vhost/tcm/Makefile | 1 +
drivers/vhost/tcm/tcm_vhost.c | 1611 +++++++++++++++++++++++++++++++++++++++++
drivers/vhost/tcm/tcm_vhost.h | 74 ++
6 files changed, 1696 insertions(+), 0 deletions(-)
create mode 100644 drivers/vhost/tcm/Kconfig
create mode 100644 drivers/vhost/tcm/Makefile
create mode 100644 drivers/vhost/tcm/tcm_vhost.c
create mode 100644 drivers/vhost/tcm/tcm_vhost.h
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 05e33c7..8d1a627 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -132,4 +132,6 @@ source "drivers/staging/ipack/Kconfig"
source "drivers/staging/gdm72xx/Kconfig"
+source "drivers/vhost/tcm/Kconfig"
+
endif # STAGING
diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
index 72dd020..3408bea 100644
--- a/drivers/vhost/Makefile
+++ b/drivers/vhost/Makefile
@@ -1,2 +1,4 @@
obj-$(CONFIG_VHOST_NET) += vhost_net.o
vhost_net-y := vhost.o net.o
+
+obj-$(CONFIG_TCM_VHOST) += tcm/
diff --git a/drivers/vhost/tcm/Kconfig b/drivers/vhost/tcm/Kconfig
new file mode 100644
index 0000000..a9c6f76
--- /dev/null
+++ b/drivers/vhost/tcm/Kconfig
@@ -0,0 +1,6 @@
+config TCM_VHOST
+ tristate "TCM_VHOST fabric module (EXPERIMENTAL)"
+ depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m
+ default n
+ ---help---
+ Say M here to enable the TCM_VHOST fabric module for use with virtio-scsi guests
diff --git a/drivers/vhost/tcm/Makefile b/drivers/vhost/tcm/Makefile
new file mode 100644
index 0000000..54b0ea6
--- /dev/null
+++ b/drivers/vhost/tcm/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TCM_VHOST) += tcm_vhost.o
diff --git a/drivers/vhost/tcm/tcm_vhost.c b/drivers/vhost/tcm/tcm_vhost.c
new file mode 100644
index 0000000..0ee4046
--- /dev/null
+++ b/drivers/vhost/tcm/tcm_vhost.c
@@ -0,0 +1,1611 @@
+/*******************************************************************************
+ * Vhost kernel TCM fabric driver for virtio SCSI initiators
+ *
+ * (C) Copyright 2010-2012 RisingTide Systems LLC.
+ * (C) Copyright 2010-2012 IBM Corp.
+ *
+ * Licensed to the Linux Foundation under the General Public License (GPL) version 2.
+ *
+ * Authors: Nicholas A. Bellinger <nab@risingtidesystems.com>
+ * Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ ****************************************************************************/
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <generated/utsrelease.h>
+#include <linux/utsname.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/kthread.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <linux/configfs.h>
+#include <linux/ctype.h>
+#include <linux/compat.h>
+#include <linux/eventfd.h>
+#include <linux/vhost.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <asm/unaligned.h>
+#include <scsi/scsi.h>
+#include <scsi/scsi_tcq.h>
+#include <target/target_core_base.h>
+#include <target/target_core_fabric.h>
+#include <target/target_core_fabric_configfs.h>
+#include <target/target_core_configfs.h>
+#include <target/configfs_macros.h>
+#include <linux/vhost.h>
+#include <linux/virtio_net.h> /* TODO vhost.h currently depends on this */
+#include <linux/virtio_scsi.h>
+
+#include "../vhost.c"
+#include "../vhost.h"
+#include "tcm_vhost.h"
+
+struct vhost_scsi {
+ atomic_t vhost_ref_cnt;
+ struct tcm_vhost_tpg *vs_tpg;
+ struct vhost_dev dev;
+ struct vhost_virtqueue vqs[3];
+
+ struct vhost_work vs_completion_work; /* cmd completion work item */
+ struct list_head vs_completion_list; /* cmd completion queue */
+ spinlock_t vs_completion_lock; /* protects s_completion_list */
+};
+
+/* Local pointer to allocated TCM configfs fabric module */
+static struct target_fabric_configfs *tcm_vhost_fabric_configfs;
+
+static struct workqueue_struct *tcm_vhost_workqueue;
+
+/* Global spinlock to protect tcm_vhost TPG list for vhost IOCTL access */
+static DEFINE_MUTEX(tcm_vhost_mutex);
+static LIST_HEAD(tcm_vhost_list);
+
+static int tcm_vhost_check_true(struct se_portal_group *se_tpg)
+{
+ return 1;
+}
+
+static int tcm_vhost_check_false(struct se_portal_group *se_tpg)
+{
+ return 0;
+}
+
+static char *tcm_vhost_get_fabric_name(void)
+{
+ return "vhost";
+}
+
+static u8 tcm_vhost_get_fabric_proto_ident(struct se_portal_group *se_tpg)
+{
+ struct tcm_vhost_tpg *tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+ struct tcm_vhost_tport *tport = tpg->tport;
+
+ switch (tport->tport_proto_id) {
+ case SCSI_PROTOCOL_SAS:
+ return sas_get_fabric_proto_ident(se_tpg);
+ case SCSI_PROTOCOL_FCP:
+ return fc_get_fabric_proto_ident(se_tpg);
+ case SCSI_PROTOCOL_ISCSI:
+ return iscsi_get_fabric_proto_ident(se_tpg);
+ default:
+ pr_err("Unknown tport_proto_id: 0x%02x, using"
+ " SAS emulation\n", tport->tport_proto_id);
+ break;
+ }
+
+ return sas_get_fabric_proto_ident(se_tpg);
+}
+
+static char *tcm_vhost_get_fabric_wwn(struct se_portal_group *se_tpg)
+{
+ struct tcm_vhost_tpg *tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+ struct tcm_vhost_tport *tport = tpg->tport;
+
+ return &tport->tport_name[0];
+}
+
+static u16 tcm_vhost_get_tag(struct se_portal_group *se_tpg)
+{
+ struct tcm_vhost_tpg *tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+ return tpg->tport_tpgt;
+}
+
+static u32 tcm_vhost_get_default_depth(struct se_portal_group *se_tpg)
+{
+ return 1;
+}
+
+static u32 tcm_vhost_get_pr_transport_id(
+ struct se_portal_group *se_tpg,
+ struct se_node_acl *se_nacl,
+ struct t10_pr_registration *pr_reg,
+ int *format_code,
+ unsigned char *buf)
+{
+ struct tcm_vhost_tpg *tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+ struct tcm_vhost_tport *tport = tpg->tport;
+
+ switch (tport->tport_proto_id) {
+ case SCSI_PROTOCOL_SAS:
+ return sas_get_pr_transport_id(se_tpg, se_nacl, pr_reg,
+ format_code, buf);
+ case SCSI_PROTOCOL_FCP:
+ return fc_get_pr_transport_id(se_tpg, se_nacl, pr_reg,
+ format_code, buf);
+ case SCSI_PROTOCOL_ISCSI:
+ return iscsi_get_pr_transport_id(se_tpg, se_nacl, pr_reg,
+ format_code, buf);
+ default:
+ pr_err("Unknown tport_proto_id: 0x%02x, using"
+ " SAS emulation\n", tport->tport_proto_id);
+ break;
+ }
+
+ return sas_get_pr_transport_id(se_tpg, se_nacl, pr_reg,
+ format_code, buf);
+}
+
+static u32 tcm_vhost_get_pr_transport_id_len(
+ struct se_portal_group *se_tpg,
+ struct se_node_acl *se_nacl,
+ struct t10_pr_registration *pr_reg,
+ int *format_code)
+{
+ struct tcm_vhost_tpg *tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+ struct tcm_vhost_tport *tport = tpg->tport;
+
+ switch (tport->tport_proto_id) {
+ case SCSI_PROTOCOL_SAS:
+ return sas_get_pr_transport_id_len(se_tpg, se_nacl, pr_reg,
+ format_code);
+ case SCSI_PROTOCOL_FCP:
+ return fc_get_pr_transport_id_len(se_tpg, se_nacl, pr_reg,
+ format_code);
+ case SCSI_PROTOCOL_ISCSI:
+ return iscsi_get_pr_transport_id_len(se_tpg, se_nacl, pr_reg,
+ format_code);
+ default:
+ pr_err("Unknown tport_proto_id: 0x%02x, using"
+ " SAS emulation\n", tport->tport_proto_id);
+ break;
+ }
+
+ return sas_get_pr_transport_id_len(se_tpg, se_nacl, pr_reg,
+ format_code);
+}
+
+static char *tcm_vhost_parse_pr_out_transport_id(
+ struct se_portal_group *se_tpg,
+ const char *buf,
+ u32 *out_tid_len,
+ char **port_nexus_ptr)
+{
+ struct tcm_vhost_tpg *tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+ struct tcm_vhost_tport *tport = tpg->tport;
+
+ switch (tport->tport_proto_id) {
+ case SCSI_PROTOCOL_SAS:
+ return sas_parse_pr_out_transport_id(se_tpg, buf, out_tid_len,
+ port_nexus_ptr);
+ case SCSI_PROTOCOL_FCP:
+ return fc_parse_pr_out_transport_id(se_tpg, buf, out_tid_len,
+ port_nexus_ptr);
+ case SCSI_PROTOCOL_ISCSI:
+ return iscsi_parse_pr_out_transport_id(se_tpg, buf, out_tid_len,
+ port_nexus_ptr);
+ default:
+ pr_err("Unknown tport_proto_id: 0x%02x, using"
+ " SAS emulation\n", tport->tport_proto_id);
+ break;
+ }
+
+ return sas_parse_pr_out_transport_id(se_tpg, buf, out_tid_len,
+ port_nexus_ptr);
+}
+
+static struct se_node_acl *tcm_vhost_alloc_fabric_acl(struct se_portal_group *se_tpg)
+{
+ struct tcm_vhost_nacl *nacl;
+
+ nacl = kzalloc(sizeof(struct tcm_vhost_nacl), GFP_KERNEL);
+ if (!nacl) {
+ pr_err("Unable to alocate struct tcm_vhost_nacl\n");
+ return NULL;
+ }
+
+ return &nacl->se_node_acl;
+}
+
+static void tcm_vhost_release_fabric_acl(
+ struct se_portal_group *se_tpg,
+ struct se_node_acl *se_nacl)
+{
+ struct tcm_vhost_nacl *nacl = container_of(se_nacl,
+ struct tcm_vhost_nacl, se_node_acl);
+ kfree(nacl);
+}
+
+static u32 tcm_vhost_tpg_get_inst_index(struct se_portal_group *se_tpg)
+{
+ return 1;
+}
+
+static void tcm_vhost_release_cmd(struct se_cmd *se_cmd)
+{
+ return;
+}
+
+static int tcm_vhost_shutdown_session(struct se_session *se_sess)
+{
+ return 0;
+}
+
+static void tcm_vhost_close_session(struct se_session *se_sess)
+{
+ return;
+}
+
+static u32 tcm_vhost_sess_get_index(struct se_session *se_sess)
+{
+ return 0;
+}
+
+static int tcm_vhost_write_pending(struct se_cmd *se_cmd)
+{
+ /* Go ahead and process the write immediately */
+ target_execute_cmd(se_cmd);
+ return 0;
+}
+
+static int tcm_vhost_write_pending_status(struct se_cmd *se_cmd)
+{
+ return 0;
+}
+
+static void tcm_vhost_set_default_node_attrs(struct se_node_acl *nacl)
+{
+ return;
+}
+
+static u32 tcm_vhost_get_task_tag(struct se_cmd *se_cmd)
+{
+ return 0;
+}
+
+static int tcm_vhost_get_cmd_state(struct se_cmd *se_cmd)
+{
+ return 0;
+}
+
+static void vhost_scsi_complete_cmd(struct tcm_vhost_cmd *);
+
+static int tcm_vhost_queue_data_in(struct se_cmd *se_cmd)
+{
+ struct tcm_vhost_cmd *tv_cmd = container_of(se_cmd,
+ struct tcm_vhost_cmd, tvc_se_cmd);
+ vhost_scsi_complete_cmd(tv_cmd);
+ return 0;
+}
+
+static int tcm_vhost_queue_status(struct se_cmd *se_cmd)
+{
+ struct tcm_vhost_cmd *tv_cmd = container_of(se_cmd,
+ struct tcm_vhost_cmd, tvc_se_cmd);
+ vhost_scsi_complete_cmd(tv_cmd);
+ return 0;
+}
+
+static int tcm_vhost_queue_tm_rsp(struct se_cmd *se_cmd)
+{
+ return 0;
+}
+
+static u16 tcm_vhost_set_fabric_sense_len(struct se_cmd *se_cmd, u32 sense_length)
+{
+ return 0;
+}
+
+static u16 tcm_vhost_get_fabric_sense_len(void)
+{
+ return 0;
+}
+
+static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
+{
+ struct se_cmd *se_cmd = &tv_cmd->tvc_se_cmd;
+
+ /* TODO locking against target/backend threads? */
+ transport_generic_free_cmd(se_cmd, 1);
+
+ if (tv_cmd->tvc_sgl_count) {
+ u32 i;
+ for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
+ put_page(sg_page(&tv_cmd->tvc_sgl[i]));
+
+ kfree(tv_cmd->tvc_sgl);
+ }
+
+ kfree(tv_cmd);
+}
+
+/* Dequeue a command from the completion list */
+static struct tcm_vhost_cmd *vhost_scsi_get_cmd_from_completion(struct vhost_scsi *vs)
+{
+ struct tcm_vhost_cmd *tv_cmd = NULL;
+
+ spin_lock_bh(&vs->vs_completion_lock);
+ if (list_empty(&vs->vs_completion_list)) {
+ spin_unlock_bh(&vs->vs_completion_lock);
+ return NULL;
+ }
+
+ list_for_each_entry(tv_cmd, &vs->vs_completion_list,
+ tvc_completion_list) {
+ list_del(&tv_cmd->tvc_completion_list);
+ break;
+ }
+ spin_unlock_bh(&vs->vs_completion_lock);
+ return tv_cmd;
+}
+
+/* Fill in status and signal that we are done processing this command
+ *
+ * This is scheduled in the vhost work queue so we are called with the owner
+ * process mm and can access the vring.
+ */
+static void vhost_scsi_complete_cmd_work(struct vhost_work *work)
+{
+ struct vhost_scsi *vs = container_of(work, struct vhost_scsi,
+ vs_completion_work);
+ struct tcm_vhost_cmd *tv_cmd;
+
+ while ((tv_cmd = vhost_scsi_get_cmd_from_completion(vs)) != NULL) {
+ struct virtio_scsi_cmd_resp v_rsp;
+ struct se_cmd *se_cmd = &tv_cmd->tvc_se_cmd;
+ int ret;
+
+ pr_debug("%s tv_cmd %p resid %u status %#02x\n", __func__,
+ tv_cmd, se_cmd->residual_count, se_cmd->scsi_status);
+
+ memset(&v_rsp, 0, sizeof(v_rsp));
+ v_rsp.resid = se_cmd->residual_count;
+ /* TODO is status_qualifier field needed? */
+ v_rsp.status = se_cmd->scsi_status;
+ v_rsp.sense_len = se_cmd->scsi_sense_length;
+ memcpy(v_rsp.sense, tv_cmd->tvc_sense_buf,
+ v_rsp.sense_len);
+ ret = copy_to_user(tv_cmd->tvc_resp, &v_rsp, sizeof(v_rsp));
+ if (likely(ret == 0))
+ vhost_add_used(&vs->vqs[2], tv_cmd->tvc_vq_desc, 0);
+ else
+ pr_err("Faulted on virtio_scsi_cmd_resp\n");
+
+ vhost_scsi_free_cmd(tv_cmd);
+ }
+
+ vhost_signal(&vs->dev, &vs->vqs[2]);
+}
+
+static void vhost_scsi_complete_cmd(struct tcm_vhost_cmd *tv_cmd)
+{
+ struct vhost_scsi *vs = tv_cmd->tvc_vhost;
+
+ pr_debug("%s tv_cmd %p\n", __func__, tv_cmd);
+
+ spin_lock_bh(&vs->vs_completion_lock);
+ list_add_tail(&tv_cmd->tvc_completion_list, &vs->vs_completion_list);
+ spin_unlock_bh(&vs->vs_completion_lock);
+
+ vhost_work_queue(&vs->dev, &vs->vs_completion_work);
+}
+
+static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
+ struct tcm_vhost_tpg *tv_tpg,
+ struct virtio_scsi_cmd_req *v_req,
+ u32 exp_data_len,
+ int data_direction)
+{
+ struct tcm_vhost_cmd *tv_cmd;
+ struct tcm_vhost_nexus *tv_nexus;
+ struct se_portal_group *se_tpg = &tv_tpg->se_tpg;
+ struct se_session *se_sess;
+ struct se_cmd *se_cmd;
+ int sam_task_attr;
+
+ tv_nexus = tv_tpg->tpg_nexus;
+ if (!tv_nexus) {
+ pr_err("Unable to locate active struct tcm_vhost_nexus\n");
+ return ERR_PTR(-EIO);
+ }
+ se_sess = tv_nexus->tvn_se_sess;
+
+ tv_cmd = kzalloc(sizeof(struct tcm_vhost_cmd), GFP_ATOMIC);
+ if (!tv_cmd) {
+ pr_err("Unable to allocate struct tcm_vhost_cmd\n");
+ return ERR_PTR(-ENOMEM);
+ }
+ INIT_LIST_HEAD(&tv_cmd->tvc_completion_list);
+ tv_cmd->tvc_tag = v_req->tag;
+
+ se_cmd = &tv_cmd->tvc_se_cmd;
+ /*
+ * Locate the SAM Task Attr from virtio_scsi_cmd_req
+ */
+ sam_task_attr = v_req->task_attr;
+ /*
+ * Initialize struct se_cmd descriptor from target_core_mod infrastructure
+ */
+ transport_init_se_cmd(se_cmd, se_tpg->se_tpg_tfo, se_sess, exp_data_len,
+ data_direction, sam_task_attr,
+ &tv_cmd->tvc_sense_buf[0]);
+
+#if 0 /* FIXME: vhost_scsi_allocate_cmd() BIDI operation */
+ if (bidi)
+ se_cmd->se_cmd_flags |= SCF_BIDI;
+#endif
+ return tv_cmd;
+}
+
+/*
+ * Map a user memory range into a scatterlist
+ *
+ * Returns the number of scatterlist entries used or -errno on error.
+ */
+static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
+ unsigned int sgl_count,
+ void __user *ptr, size_t len, int write)
+{
+ struct scatterlist *sg = sgl;
+ unsigned int npages = 0;
+ int ret;
+
+ while (len > 0) {
+ struct page *page;
+ unsigned int offset = (uintptr_t)ptr & ~PAGE_MASK;
+ unsigned int nbytes = min_t(unsigned int,
+ PAGE_SIZE - offset, len);
+
+ if (npages == sgl_count) {
+ ret = -ENOBUFS;
+ goto err;
+ }
+
+ ret = get_user_pages_fast((unsigned long)ptr, 1, write, &page);
+ BUG_ON(ret == 0); /* we should either get our page or fail */
+ if (ret < 0)
+ goto err;
+
+ sg_set_page(sg, page, nbytes, offset);
+ ptr += nbytes;
+ len -= nbytes;
+ sg++;
+ npages++;
+ }
+ return npages;
+
+err:
+ /* Put pages that we hold */
+ for (sg = sgl; sg != &sgl[npages]; sg++)
+ put_page(sg_page(sg));
+ return ret;
+}
+
+static int vhost_scsi_map_iov_to_sgl(struct tcm_vhost_cmd *tv_cmd,
+ struct iovec *iov, unsigned int niov,
+ int write)
+{
+ int ret;
+ unsigned int i;
+ u32 sgl_count;
+ struct scatterlist *sg;
+
+ /*
+ * Find out how long sglist needs to be
+ */
+ sgl_count = 0;
+ for (i = 0; i < niov; i++) {
+ sgl_count += (((uintptr_t)iov[i].iov_base + iov[i].iov_len +
+ PAGE_SIZE - 1) >> PAGE_SHIFT) -
+ ((uintptr_t)iov[i].iov_base >> PAGE_SHIFT);
+ }
+ /* TODO overflow checking */
+
+ sg = kmalloc(sizeof(tv_cmd->tvc_sgl[0]) * sgl_count, GFP_ATOMIC);
+ if (!sg)
+ return -ENOMEM;
+ pr_debug("%s sg %p sgl_count %u is_err %ld\n", __func__,
+ sg, sgl_count, IS_ERR(sg));
+ sg_init_table(sg, sgl_count);
+
+ tv_cmd->tvc_sgl = sg;
+ tv_cmd->tvc_sgl_count = sgl_count;
+
+ pr_debug("Mapping %u iovecs for %u pages\n", niov, sgl_count);
+ for (i = 0; i < niov; i++) {
+ ret = vhost_scsi_map_to_sgl(sg, sgl_count, iov[i].iov_base,
+ iov[i].iov_len, write);
+ if (ret < 0) {
+ for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
+ put_page(sg_page(&tv_cmd->tvc_sgl[i]));
+ kfree(tv_cmd->tvc_sgl);
+ tv_cmd->tvc_sgl = NULL;
+ tv_cmd->tvc_sgl_count = 0;
+ return ret;
+ }
+
+ sg += ret;
+ sgl_count -= ret;
+ }
+ return 0;
+}
+
+static void tcm_vhost_submission_work(struct work_struct *work)
+{
+ struct tcm_vhost_cmd *tv_cmd =
+ container_of(work, struct tcm_vhost_cmd, work);
+ struct se_cmd *se_cmd = &tv_cmd->tvc_se_cmd;
+ struct scatterlist *sg_ptr, *sg_bidi_ptr = NULL;
+ int rc, sg_no_bidi = 0;
+ /*
+ * Locate the struct se_lun pointer based on v_req->lun, and
+ * attach it to struct se_cmd
+ */
+ rc = transport_lookup_cmd_lun(&tv_cmd->tvc_se_cmd, tv_cmd->tvc_lun);
+ if (rc < 0) {
+ pr_err("Failed to look up lun: %d\n", tv_cmd->tvc_lun);
+ transport_send_check_condition_and_sense(&tv_cmd->tvc_se_cmd,
+ tv_cmd->tvc_se_cmd.scsi_sense_reason, 0);
+ transport_generic_free_cmd(se_cmd, 0);
+ return;
+ }
+
+ rc = target_setup_cmd_from_cdb(se_cmd, tv_cmd->tvc_cdb);
+ if (rc == -ENOMEM) {
+ transport_send_check_condition_and_sense(se_cmd,
+ TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0);
+ transport_generic_free_cmd(se_cmd, 0);
+ return;
+ } else if (rc < 0) {
+ if (se_cmd->se_cmd_flags & SCF_SCSI_RESERVATION_CONFLICT)
+ tcm_vhost_queue_status(se_cmd);
+ else
+ transport_send_check_condition_and_sense(se_cmd,
+ se_cmd->scsi_sense_reason, 0);
+ transport_generic_free_cmd(se_cmd, 0);
+ return;
+ }
+
+ if (tv_cmd->tvc_sgl_count) {
+ sg_ptr = tv_cmd->tvc_sgl;
+ /*
+ * For BIDI commands, pass in the extra READ buffer
+ * to transport_generic_map_mem_to_cmd() below..
+ */
+/* FIXME: Fix BIDI operation in tcm_vhost_submission_work() */
+#if 0
+ if (se_cmd->se_cmd_flags & SCF_BIDI) {
+ sg_bidi_ptr = NULL;
+ sg_no_bidi = 0;
+ }
+#endif
+ } else {
+ sg_ptr = NULL;
+ }
+
+ rc = transport_generic_map_mem_to_cmd(se_cmd, sg_ptr,
+ tv_cmd->tvc_sgl_count, sg_bidi_ptr,
+ sg_no_bidi);
+ if (rc < 0) {
+ transport_send_check_condition_and_sense(se_cmd,
+ se_cmd->scsi_sense_reason, 0);
+ transport_generic_free_cmd(se_cmd, 0);
+ return;
+ }
+ transport_handle_cdb_direct(se_cmd);
+}
+
+static void vhost_scsi_handle_vq(struct vhost_scsi *vs)
+{
+ struct vhost_virtqueue *vq = &vs->vqs[2];
+ struct virtio_scsi_cmd_req v_req;
+ struct tcm_vhost_tpg *tv_tpg;
+ struct tcm_vhost_cmd *tv_cmd;
+ u32 exp_data_len, data_first, data_num, data_direction;
+ unsigned out, in, i;
+ int head, ret;
+
+ /* Must use ioctl VHOST_SCSI_SET_ENDPOINT */
+ tv_tpg = vs->vs_tpg;
+ if (unlikely(!tv_tpg)) {
+ pr_err("%s endpoint not set\n", __func__);
+ return;
+ }
+
+ mutex_lock(&vq->mutex);
+ vhost_disable_notify(&vs->dev, vq);
+
+ for (;;) {
+ head = vhost_get_vq_desc(&vs->dev, vq, vq->iov,
+ ARRAY_SIZE(vq->iov), &out, &in,
+ NULL, NULL);
+ pr_debug("vhost_get_vq_desc: head: %d, out: %u in: %u\n", head, out, in);
+ /* On error, stop handling until the next kick. */
+ if (unlikely(head < 0))
+ break;
+ /* Nothing new? Wait for eventfd to tell us they refilled. */
+ if (head == vq->num) {
+ if (unlikely(vhost_enable_notify(&vs->dev, vq))) {
+ vhost_disable_notify(&vs->dev, vq);
+ continue;
+ }
+ break;
+ }
+
+/* FIXME: BIDI operation */
+ if (out == 1 && in == 1) {
+ data_direction = DMA_NONE;
+ data_first = 0;
+ data_num = 0;
+ } else if (out == 1 && in > 1) {
+ data_direction = DMA_FROM_DEVICE;
+ data_first = out + 1;
+ data_num = in - 1;
+ } else if (out > 1 && in == 1) {
+ data_direction = DMA_TO_DEVICE;
+ data_first = 1;
+ data_num = out - 1;
+ } else {
+ pr_err("Invalid buffer layout out: %u in: %u\n", out, in);
+ break;
+ }
+
+ /*
+ * Check for a sane resp buffer so we can report errors to
+ * the guest.
+ */
+ if (unlikely(vq->iov[out].iov_len !=
+ sizeof(struct virtio_scsi_cmd_resp))) {
+ pr_err("Expecting virtio_scsi_cmd_resp, got %zu bytes\n",
+ vq->iov[out].iov_len);
+ break;
+ }
+
+ if (unlikely(vq->iov[0].iov_len != sizeof(v_req))) {
+ pr_err("Expecting virtio_scsi_cmd_req, got %zu bytes\n",
+ vq->iov[0].iov_len);
+ break;
+ }
+ pr_debug("Calling __copy_from_user: vq->iov[0].iov_base: %p, len: %zu\n",
+ vq->iov[0].iov_base, sizeof(v_req));
+ ret = __copy_from_user(&v_req, vq->iov[0].iov_base, sizeof(v_req));
+ if (unlikely(ret)) {
+ pr_err("Faulted on virtio_scsi_cmd_req\n");
+ break;
+ }
+
+ exp_data_len = 0;
+ for (i = 0; i < data_num; i++) {
+ exp_data_len += vq->iov[data_first + i].iov_len;
+ }
+
+ tv_cmd = vhost_scsi_allocate_cmd(tv_tpg, &v_req,
+ exp_data_len, data_direction);
+ if (IS_ERR(tv_cmd)) {
+ pr_err("vhost_scsi_allocate_cmd failed %ld\n", PTR_ERR(tv_cmd));
+ break;
+ }
+ pr_debug("Allocated tv_cmd: %p exp_data_len: %d, data_direction: %d\n",
+ tv_cmd, exp_data_len, data_direction);
+
+ tv_cmd->tvc_vhost = vs;
+
+ if (unlikely(vq->iov[out].iov_len !=
+ sizeof(struct virtio_scsi_cmd_resp))) {
+ pr_err("Expecting virtio_scsi_cmd_resp, "
+ " got %zu bytes, out: %d, in: %d\n", vq->iov[out].iov_len, out, in);
+ break;
+ }
+
+ tv_cmd->tvc_resp = vq->iov[out].iov_base;
+
+ /*
+ * Copy in the recieved CDB descriptor into tv_cmd->tvc_cdb
+ * that will be used by tcm_vhost_new_cmd_map() and down into
+ * target_setup_cmd_from_cdb()
+ */
+ memcpy(tv_cmd->tvc_cdb, v_req.cdb, TCM_VHOST_MAX_CDB_SIZE);
+ /*
+ * Check that the recieved CDB size does not exceeded our
+ * hardcoded max for tcm_vhost
+ */
+ /* TODO what if cdb was too small for varlen cdb header? */
+ if (unlikely(scsi_command_size(tv_cmd->tvc_cdb) > TCM_VHOST_MAX_CDB_SIZE)) {
+ pr_err("Received SCSI CDB with command_size: %d that exceeds"
+ " SCSI_MAX_VARLEN_CDB_SIZE: %d\n",
+ scsi_command_size(tv_cmd->tvc_cdb), TCM_VHOST_MAX_CDB_SIZE);
+ break; /* TODO */
+ }
+ tv_cmd->tvc_lun = ((v_req.lun[2] << 8) | v_req.lun[3]) & 0x3FFF;
+
+ pr_debug("vhost_scsi got command opcode: %#02x, lun: %d\n",
+ tv_cmd->tvc_cdb[0], tv_cmd->tvc_lun);
+
+ if (data_direction != DMA_NONE) {
+ ret = vhost_scsi_map_iov_to_sgl(tv_cmd, &vq->iov[data_first],
+ data_num, data_direction == DMA_TO_DEVICE);
+ if (unlikely(ret)) {
+ pr_err("Failed to map iov to sgl\n");
+ break; /* TODO */
+ }
+ }
+
+ /*
+ * Save the descriptor from vhost_get_vq_desc() to be used to
+ * complete the virtio-scsi request in TCM callback context via
+ * tcm_vhost_queue_data_in() and tcm_vhost_queue_status()
+ */
+ tv_cmd->tvc_vq_desc = head;
+ /*
+ * Dispatch tv_cmd descriptor for cmwq execution in process
+ * context provided by tcm_vhost_workqueue. This also ensures
+ * tv_cmd is executed on the same kworker CPU as this vhost
+ * thread to gain positive L2 cache locality effects..
+ */
+ INIT_WORK(&tv_cmd->work, tcm_vhost_submission_work);
+ queue_work(tcm_vhost_workqueue, &tv_cmd->work);
+ }
+
+ mutex_unlock(&vq->mutex);
+}
+
+static void vhost_scsi_ctl_handle_kick(struct vhost_work *work)
+{
+ pr_err("%s: The handling func for control queue.\n", __func__);
+}
+
+static void vhost_scsi_evt_handle_kick(struct vhost_work *work)
+{
+ pr_err("%s: The handling func for event queue.\n", __func__);
+}
+
+static void vhost_scsi_handle_kick(struct vhost_work *work)
+{
+ struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue,
+ poll.work);
+ struct vhost_scsi *vs = container_of(vq->dev, struct vhost_scsi, dev);
+
+ vhost_scsi_handle_vq(vs);
+}
+
+/*
+ * Called from vhost_scsi_ioctl() context to walk the list of available tcm_vhost_tpg
+ * with an active struct tcm_vhost_nexus
+ */
+static int vhost_scsi_set_endpoint(
+ struct vhost_scsi *vs,
+ struct vhost_vring_target *t)
+{
+ struct tcm_vhost_tport *tv_tport;
+ struct tcm_vhost_tpg *tv_tpg;
+ int index;
+
+ mutex_lock(&vs->dev.mutex);
+ /* Verify that ring has been setup correctly. */
+ for (index = 0; index < vs->dev.nvqs; ++index) {
+ /* Verify that ring has been setup correctly. */
+ if (!vhost_vq_access_ok(&vs->vqs[index])) {
+ mutex_unlock(&vs->dev.mutex);
+ return -EFAULT;
+ }
+ }
+
+ if (vs->vs_tpg) {
+ mutex_unlock(&vs->dev.mutex);
+ return -EEXIST;
+ }
+ mutex_unlock(&vs->dev.mutex);
+
+ mutex_lock(&tcm_vhost_mutex);
+ list_for_each_entry(tv_tpg, &tcm_vhost_list, tv_tpg_list) {
+ mutex_lock(&tv_tpg->tv_tpg_mutex);
+ if (!tv_tpg->tpg_nexus) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ continue;
+ }
+ if (atomic_read(&tv_tpg->tv_tpg_vhost_count)) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ continue;
+ }
+ tv_tport = tv_tpg->tport;
+
+ if (!strcmp(tv_tport->tport_name, t->vhost_wwpn) &&
+ (tv_tpg->tport_tpgt == t->vhost_tpgt)) {
+ atomic_inc(&tv_tpg->tv_tpg_vhost_count);
+ smp_mb__after_atomic_inc();
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ mutex_unlock(&tcm_vhost_mutex);
+
+ mutex_lock(&vs->dev.mutex);
+ vs->vs_tpg = tv_tpg;
+ atomic_inc(&vs->vhost_ref_cnt);
+ smp_mb__after_atomic_inc();
+ mutex_unlock(&vs->dev.mutex);
+ return 0;
+ }
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ }
+ mutex_unlock(&tcm_vhost_mutex);
+ return -EINVAL;
+}
+
+static int vhost_scsi_clear_endpoint(
+ struct vhost_scsi *vs,
+ struct vhost_vring_target *t)
+{
+ struct tcm_vhost_tport *tv_tport;
+ struct tcm_vhost_tpg *tv_tpg;
+ int index;
+
+ mutex_lock(&vs->dev.mutex);
+ /* Verify that ring has been setup correctly. */
+ for (index = 0; index < vs->dev.nvqs; ++index) {
+ if (!vhost_vq_access_ok(&vs->vqs[index])) {
+ mutex_unlock(&vs->dev.mutex);
+ return -EFAULT;
+ }
+ }
+
+ if (!vs->vs_tpg) {
+ mutex_unlock(&vs->dev.mutex);
+ return -ENODEV;
+ }
+ tv_tpg = vs->vs_tpg;
+ tv_tport = tv_tpg->tport;
+
+ if (strcmp(tv_tport->tport_name, t->vhost_wwpn) ||
+ (tv_tpg->tport_tpgt != t->vhost_tpgt)) {
+ mutex_unlock(&vs->dev.mutex);
+ pr_warn("tv_tport->tport_name: %s, tv_tpg->tport_tpgt: %hu"
+ " does not match t->vhost_wwpn: %s, t->vhost_tpgt: %hu\n",
+ tv_tport->tport_name, tv_tpg->tport_tpgt,
+ t->vhost_wwpn, t->vhost_tpgt);
+ return -EINVAL;
+ }
+ atomic_dec(&tv_tpg->tv_tpg_vhost_count);
+ vs->vs_tpg = NULL;
+ mutex_unlock(&vs->dev.mutex);
+
+ return 0;
+}
+
+static int vhost_scsi_open(struct inode *inode, struct file *f)
+{
+ struct vhost_scsi *s;
+ int r;
+
+ s = kzalloc(sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+
+ vhost_work_init(&s->vs_completion_work, vhost_scsi_complete_cmd_work);
+ INIT_LIST_HEAD(&s->vs_completion_list);
+ spin_lock_init(&s->vs_completion_lock);
+
+ s->vqs[0].handle_kick = vhost_scsi_ctl_handle_kick;
+ s->vqs[1].handle_kick = vhost_scsi_evt_handle_kick;
+ s->vqs[2].handle_kick = vhost_scsi_handle_kick;
+ r = vhost_dev_init(&s->dev, s->vqs, 3);
+ if (r < 0) {
+ kfree(s);
+ return r;
+ }
+
+ f->private_data = s;
+ return 0;
+}
+
+static int vhost_scsi_release(struct inode *inode, struct file *f)
+{
+ struct vhost_scsi *s = f->private_data;
+
+ if (s->vs_tpg && s->vs_tpg->tport) {
+ struct vhost_vring_target backend;
+ memcpy(backend.vhost_wwpn, s->vs_tpg->tport->tport_name, sizeof(backend.vhost_wwpn));
+ backend.vhost_tpgt = s->vs_tpg->tport_tpgt;
+ vhost_scsi_clear_endpoint(s, &backend);
+ }
+
+ vhost_dev_cleanup(&s->dev, false);
+ kfree(s);
+ return 0;
+}
+
+static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
+{
+ if (features & ~VHOST_FEATURES)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&vs->dev.mutex);
+ if ((features & (1 << VHOST_F_LOG_ALL)) &&
+ !vhost_log_access_ok(&vs->dev)) {
+ mutex_unlock(&vs->dev.mutex);
+ return -EFAULT;
+ }
+ vs->dev.acked_features = features;
+ /* TODO possibly smp_wmb() and flush vqs */
+ mutex_unlock(&vs->dev.mutex);
+ return 0;
+}
+
+static long vhost_scsi_ioctl(struct file *f, unsigned int ioctl,
+ unsigned long arg)
+{
+ struct vhost_scsi *vs = f->private_data;
+ struct vhost_vring_target backend;
+ void __user *argp = (void __user *)arg;
+ u64 __user *featurep = argp;
+ u64 features;
+ int r;
+
+ switch (ioctl) {
+ case VHOST_SCSI_SET_ENDPOINT:
+ if (copy_from_user(&backend, argp, sizeof backend))
+ return -EFAULT;
+
+ return vhost_scsi_set_endpoint(vs, &backend);
+ case VHOST_SCSI_CLEAR_ENDPOINT:
+ if (copy_from_user(&backend, argp, sizeof backend))
+ return -EFAULT;
+
+ return vhost_scsi_clear_endpoint(vs, &backend);
+ case VHOST_GET_FEATURES:
+ features = VHOST_FEATURES;
+ if (copy_to_user(featurep, &features, sizeof features))
+ return -EFAULT;
+ return 0;
+ case VHOST_SET_FEATURES:
+ if (copy_from_user(&features, featurep, sizeof features))
+ return -EFAULT;
+ return vhost_scsi_set_features(vs, features);
+ default:
+ mutex_lock(&vs->dev.mutex);
+ r = vhost_dev_ioctl(&vs->dev, ioctl, arg);
+ mutex_unlock(&vs->dev.mutex);
+ return r;
+ }
+}
+
+static const struct file_operations vhost_scsi_fops = {
+ .owner = THIS_MODULE,
+ .release = vhost_scsi_release,
+ .unlocked_ioctl = vhost_scsi_ioctl,
+ /* TODO compat ioctl? */
+ .open = vhost_scsi_open,
+ .llseek = noop_llseek,
+};
+
+static struct miscdevice vhost_scsi_misc = {
+ MISC_DYNAMIC_MINOR,
+ "vhost-scsi",
+ &vhost_scsi_fops,
+};
+
+static int __init vhost_scsi_register(void)
+{
+ return misc_register(&vhost_scsi_misc);
+}
+
+static int vhost_scsi_deregister(void)
+{
+ return misc_deregister(&vhost_scsi_misc);
+}
+
+static char *tcm_vhost_dump_proto_id(struct tcm_vhost_tport *tport)
+{
+ switch (tport->tport_proto_id) {
+ case SCSI_PROTOCOL_SAS:
+ return "SAS";
+ case SCSI_PROTOCOL_FCP:
+ return "FCP";
+ case SCSI_PROTOCOL_ISCSI:
+ return "iSCSI";
+ default:
+ break;
+ }
+
+ return "Unknown";
+}
+
+static int tcm_vhost_port_link(
+ struct se_portal_group *se_tpg,
+ struct se_lun *lun)
+{
+ struct tcm_vhost_tpg *tv_tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+
+ atomic_inc(&tv_tpg->tv_tpg_port_count);
+ smp_mb__after_atomic_inc();
+
+ return 0;
+}
+
+static void tcm_vhost_port_unlink(
+ struct se_portal_group *se_tpg,
+ struct se_lun *se_lun)
+{
+ struct tcm_vhost_tpg *tv_tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+
+ atomic_dec(&tv_tpg->tv_tpg_port_count);
+ smp_mb__after_atomic_dec();
+}
+
+static struct se_node_acl *tcm_vhost_make_nodeacl(
+ struct se_portal_group *se_tpg,
+ struct config_group *group,
+ const char *name)
+{
+ struct se_node_acl *se_nacl, *se_nacl_new;
+ struct tcm_vhost_nacl *nacl;
+ u64 wwpn = 0;
+ u32 nexus_depth;
+
+ /* tcm_vhost_parse_wwn(name, &wwpn, 1) < 0)
+ return ERR_PTR(-EINVAL); */
+ se_nacl_new = tcm_vhost_alloc_fabric_acl(se_tpg);
+ if (!se_nacl_new)
+ return ERR_PTR(-ENOMEM);
+//#warning FIXME: Hardcoded nexus depth in tcm_vhost_make_nodeacl()
+ nexus_depth = 1;
+ /*
+ * se_nacl_new may be released by core_tpg_add_initiator_node_acl()
+ * when converting a NodeACL from demo mode -> explict
+ */
+ se_nacl = core_tpg_add_initiator_node_acl(se_tpg, se_nacl_new,
+ name, nexus_depth);
+ if (IS_ERR(se_nacl)) {
+ tcm_vhost_release_fabric_acl(se_tpg, se_nacl_new);
+ return se_nacl;
+ }
+ /*
+ * Locate our struct tcm_vhost_nacl and set the FC Nport WWPN
+ */
+ nacl = container_of(se_nacl, struct tcm_vhost_nacl, se_node_acl);
+ nacl->iport_wwpn = wwpn;
+ /* tcm_vhost_format_wwn(&nacl->iport_name[0], TCM_VHOST_NAMELEN, wwpn); */
+
+ return se_nacl;
+}
+
+static void tcm_vhost_drop_nodeacl(struct se_node_acl *se_acl)
+{
+ struct tcm_vhost_nacl *nacl = container_of(se_acl,
+ struct tcm_vhost_nacl, se_node_acl);
+ core_tpg_del_initiator_node_acl(se_acl->se_tpg, se_acl, 1);
+ kfree(nacl);
+}
+
+static int tcm_vhost_make_nexus(
+ struct tcm_vhost_tpg *tv_tpg,
+ const char *name)
+{
+ struct se_portal_group *se_tpg;
+ struct tcm_vhost_nexus *tv_nexus;
+
+ mutex_lock(&tv_tpg->tv_tpg_mutex);
+ if (tv_tpg->tpg_nexus) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ pr_debug("tv_tpg->tpg_nexus already exists\n");
+ return -EEXIST;
+ }
+ se_tpg = &tv_tpg->se_tpg;
+
+ tv_nexus = kzalloc(sizeof(struct tcm_vhost_nexus), GFP_KERNEL);
+ if (!tv_nexus) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ pr_err("Unable to allocate struct tcm_vhost_nexus\n");
+ return -ENOMEM;
+ }
+ /*
+ * Initialize the struct se_session pointer
+ */
+ tv_nexus->tvn_se_sess = transport_init_session();
+ if (IS_ERR(tv_nexus->tvn_se_sess)) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ kfree(tv_nexus);
+ return -ENOMEM;
+ }
+ /*
+ * Since we are running in 'demo mode' this call with generate a
+ * struct se_node_acl for the tcm_vhost struct se_portal_group with
+ * the SCSI Initiator port name of the passed configfs group 'name'.
+ */
+ tv_nexus->tvn_se_sess->se_node_acl = core_tpg_check_initiator_node_acl(
+ se_tpg, (unsigned char *)name);
+ if (!tv_nexus->tvn_se_sess->se_node_acl) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ pr_debug("core_tpg_check_initiator_node_acl() failed"
+ " for %s\n", name);
+ transport_free_session(tv_nexus->tvn_se_sess);
+ kfree(tv_nexus);
+ return -ENOMEM;
+ }
+ /*
+ * Now register the TCM vHost virtual I_T Nexus as active with the
+ * call to __transport_register_session()
+ */
+ __transport_register_session(se_tpg, tv_nexus->tvn_se_sess->se_node_acl,
+ tv_nexus->tvn_se_sess, tv_nexus);
+ tv_tpg->tpg_nexus = tv_nexus;
+
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ return 0;
+}
+
+static int tcm_vhost_drop_nexus(
+ struct tcm_vhost_tpg *tpg)
+{
+ struct se_session *se_sess;
+ struct tcm_vhost_nexus *tv_nexus;
+
+ mutex_lock(&tpg->tv_tpg_mutex);
+ tv_nexus = tpg->tpg_nexus;
+ if (!tv_nexus) {
+ mutex_unlock(&tpg->tv_tpg_mutex);
+ return -ENODEV;
+ }
+
+ se_sess = tv_nexus->tvn_se_sess;
+ if (!se_sess) {
+ mutex_unlock(&tpg->tv_tpg_mutex);
+ return -ENODEV;
+ }
+
+ if (atomic_read(&tpg->tv_tpg_port_count)) {
+ mutex_unlock(&tpg->tv_tpg_mutex);
+ pr_err("Unable to remove TCM_vHost I_T Nexus with"
+ " active TPG port count: %d\n",
+ atomic_read(&tpg->tv_tpg_port_count));
+ return -EPERM;
+ }
+
+ if (atomic_read(&tpg->tv_tpg_vhost_count)) {
+ mutex_unlock(&tpg->tv_tpg_mutex);
+ pr_err("Unable to remove TCM_vHost I_T Nexus with"
+ " active TPG vhost count: %d\n",
+ atomic_read(&tpg->tv_tpg_vhost_count));
+ return -EPERM;
+ }
+
+ pr_debug("TCM_vHost_ConfigFS: Removing I_T Nexus to emulated"
+ " %s Initiator Port: %s\n", tcm_vhost_dump_proto_id(tpg->tport),
+ tv_nexus->tvn_se_sess->se_node_acl->initiatorname);
+ /*
+ * Release the SCSI I_T Nexus to the emulated vHost Target Port
+ */
+ transport_deregister_session(tv_nexus->tvn_se_sess);
+ tpg->tpg_nexus = NULL;
+ mutex_unlock(&tpg->tv_tpg_mutex);
+
+ kfree(tv_nexus);
+ return 0;
+}
+
+static ssize_t tcm_vhost_tpg_show_nexus(
+ struct se_portal_group *se_tpg,
+ char *page)
+{
+ struct tcm_vhost_tpg *tv_tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+ struct tcm_vhost_nexus *tv_nexus;
+ ssize_t ret;
+
+ mutex_lock(&tv_tpg->tv_tpg_mutex);
+ tv_nexus = tv_tpg->tpg_nexus;
+ if (!tv_nexus) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ return -ENODEV;
+ }
+ ret = snprintf(page, PAGE_SIZE, "%s\n",
+ tv_nexus->tvn_se_sess->se_node_acl->initiatorname);
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+
+ return ret;
+}
+
+static ssize_t tcm_vhost_tpg_store_nexus(
+ struct se_portal_group *se_tpg,
+ const char *page,
+ size_t count)
+{
+ struct tcm_vhost_tpg *tv_tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+ struct tcm_vhost_tport *tport_wwn = tv_tpg->tport;
+ unsigned char i_port[TCM_VHOST_NAMELEN], *ptr, *port_ptr;
+ int ret;
+ /*
+ * Shutdown the active I_T nexus if 'NULL' is passed..
+ */
+ if (!strncmp(page, "NULL", 4)) {
+ ret = tcm_vhost_drop_nexus(tv_tpg);
+ return (!ret) ? count : ret;
+ }
+ /*
+ * Otherwise make sure the passed virtual Initiator port WWN matches
+ * the fabric protocol_id set in tcm_vhost_make_tport(), and call
+ * tcm_vhost_make_nexus().
+ */
+ if (strlen(page) >= TCM_VHOST_NAMELEN) {
+ pr_err("Emulated NAA Sas Address: %s, exceeds"
+ " max: %d\n", page, TCM_VHOST_NAMELEN);
+ return -EINVAL;
+ }
+ snprintf(&i_port[0], TCM_VHOST_NAMELEN, "%s", page);
+
+ ptr = strstr(i_port, "naa.");
+ if (ptr) {
+ if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_SAS) {
+ pr_err("Passed SAS Initiator Port %s does not"
+ " match target port protoid: %s\n", i_port,
+ tcm_vhost_dump_proto_id(tport_wwn));
+ return -EINVAL;
+ }
+ port_ptr = &i_port[0];
+ goto check_newline;
+ }
+ ptr = strstr(i_port, "fc.");
+ if (ptr) {
+ if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_FCP) {
+ pr_err("Passed FCP Initiator Port %s does not"
+ " match target port protoid: %s\n", i_port,
+ tcm_vhost_dump_proto_id(tport_wwn));
+ return -EINVAL;
+ }
+ port_ptr = &i_port[3]; /* Skip over "fc." */
+ goto check_newline;
+ }
+ ptr = strstr(i_port, "iqn.");
+ if (ptr) {
+ if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_ISCSI) {
+ pr_err("Passed iSCSI Initiator Port %s does not"
+ " match target port protoid: %s\n", i_port,
+ tcm_vhost_dump_proto_id(tport_wwn));
+ return -EINVAL;
+ }
+ port_ptr = &i_port[0];
+ goto check_newline;
+ }
+ pr_err("Unable to locate prefix for emulated Initiator Port:"
+ " %s\n", i_port);
+ return -EINVAL;
+ /*
+ * Clear any trailing newline for the NAA WWN
+ */
+check_newline:
+ if (i_port[strlen(i_port)-1] == '\n')
+ i_port[strlen(i_port)-1] = '\0';
+
+ ret = tcm_vhost_make_nexus(tv_tpg, port_ptr);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+TF_TPG_BASE_ATTR(tcm_vhost, nexus, S_IRUGO | S_IWUSR);
+
+static struct configfs_attribute *tcm_vhost_tpg_attrs[] = {
+ &tcm_vhost_tpg_nexus.attr,
+ NULL,
+};
+
+static struct se_portal_group *tcm_vhost_make_tpg(
+ struct se_wwn *wwn,
+ struct config_group *group,
+ const char *name)
+{
+ struct tcm_vhost_tport*tport = container_of(wwn,
+ struct tcm_vhost_tport, tport_wwn);
+
+ struct tcm_vhost_tpg *tpg;
+ unsigned long tpgt;
+ int ret;
+
+ if (strstr(name, "tpgt_") != name)
+ return ERR_PTR(-EINVAL);
+ if (strict_strtoul(name + 5, 10, &tpgt) || tpgt > UINT_MAX)
+ return ERR_PTR(-EINVAL);
+
+ tpg = kzalloc(sizeof(struct tcm_vhost_tpg), GFP_KERNEL);
+ if (!tpg) {
+ pr_err("Unable to allocate struct tcm_vhost_tpg");
+ return ERR_PTR(-ENOMEM);
+ }
+ mutex_init(&tpg->tv_tpg_mutex);
+ INIT_LIST_HEAD(&tpg->tv_tpg_list);
+ tpg->tport = tport;
+ tpg->tport_tpgt = tpgt;
+
+ ret = core_tpg_register(&tcm_vhost_fabric_configfs->tf_ops, wwn,
+ &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL);
+ if (ret < 0) {
+ kfree(tpg);
+ return NULL;
+ }
+ mutex_lock(&tcm_vhost_mutex);
+ list_add_tail(&tpg->tv_tpg_list, &tcm_vhost_list);
+ mutex_unlock(&tcm_vhost_mutex);
+
+ return &tpg->se_tpg;
+}
+
+static void tcm_vhost_drop_tpg(struct se_portal_group *se_tpg)
+{
+ struct tcm_vhost_tpg *tpg = container_of(se_tpg,
+ struct tcm_vhost_tpg, se_tpg);
+
+ mutex_lock(&tcm_vhost_mutex);
+ list_del(&tpg->tv_tpg_list);
+ mutex_unlock(&tcm_vhost_mutex);
+ /*
+ * Release the virtual I_T Nexus for this vHost TPG
+ */
+ tcm_vhost_drop_nexus(tpg);
+ /*
+ * Deregister the se_tpg from TCM..
+ */
+ core_tpg_deregister(se_tpg);
+ kfree(tpg);
+}
+
+static struct se_wwn *tcm_vhost_make_tport(
+ struct target_fabric_configfs *tf,
+ struct config_group *group,
+ const char *name)
+{
+ struct tcm_vhost_tport *tport;
+ char *ptr;
+ u64 wwpn = 0;
+ int off = 0;
+
+ /* if (tcm_vhost_parse_wwn(name, &wwpn, 1) < 0)
+ return ERR_PTR(-EINVAL); */
+
+ tport = kzalloc(sizeof(struct tcm_vhost_tport), GFP_KERNEL);
+ if (!tport) {
+ pr_err("Unable to allocate struct tcm_vhost_tport");
+ return ERR_PTR(-ENOMEM);
+ }
+ tport->tport_wwpn = wwpn;
+ /* tcm_vhost_format_wwn(&tport->tport_name[0], TCM_VHOST__NAMELEN, wwpn); */
+ /*
+ * Determine the emulated Protocol Identifier and Target Port Name
+ * based on the incoming configfs directory name.
+ */
+ ptr = strstr(name, "naa.");
+ if (ptr) {
+ tport->tport_proto_id = SCSI_PROTOCOL_SAS;
+ goto check_len;
+ }
+ ptr = strstr(name, "fc.");
+ if (ptr) {
+ tport->tport_proto_id = SCSI_PROTOCOL_FCP;
+ off = 3; /* Skip over "fc." */
+ goto check_len;
+ }
+ ptr = strstr(name, "iqn.");
+ if (ptr) {
+ tport->tport_proto_id = SCSI_PROTOCOL_ISCSI;
+ goto check_len;
+ }
+
+ pr_err("Unable to locate prefix for emulated Target Port:"
+ " %s\n", name);
+ kfree(tport);
+ return ERR_PTR(-EINVAL);
+
+check_len:
+ if (strlen(name) >= TCM_VHOST_NAMELEN) {
+ pr_err("Emulated %s Address: %s, exceeds"
+ " max: %d\n", name, tcm_vhost_dump_proto_id(tport),
+ TCM_VHOST_NAMELEN);
+ kfree(tport);
+ return ERR_PTR(-EINVAL);
+ }
+ snprintf(&tport->tport_name[0], TCM_VHOST_NAMELEN, "%s", &name[off]);
+
+ pr_debug("TCM_VHost_ConfigFS: Allocated emulated Target"
+ " %s Address: %s\n", tcm_vhost_dump_proto_id(tport), name);
+
+ return &tport->tport_wwn;
+}
+
+static void tcm_vhost_drop_tport(struct se_wwn *wwn)
+{
+ struct tcm_vhost_tport *tport = container_of(wwn,
+ struct tcm_vhost_tport, tport_wwn);
+
+ pr_debug("TCM_VHost_ConfigFS: Deallocating emulated Target"
+ " %s Address: %s\n", tcm_vhost_dump_proto_id(tport),
+ tport->tport_name);;
+
+ kfree(tport);
+}
+
+static ssize_t tcm_vhost_wwn_show_attr_version(
+ struct target_fabric_configfs *tf,
+ char *page)
+{
+ return sprintf(page, "TCM_VHOST fabric module %s on %s/%s"
+ "on "UTS_RELEASE"\n", TCM_VHOST_VERSION, utsname()->sysname,
+ utsname()->machine);
+}
+
+TF_WWN_ATTR_RO(tcm_vhost, version);
+
+static struct configfs_attribute *tcm_vhost_wwn_attrs[] = {
+ &tcm_vhost_wwn_version.attr,
+ NULL,
+};
+
+static struct target_core_fabric_ops tcm_vhost_ops = {
+ .get_fabric_name = tcm_vhost_get_fabric_name,
+ .get_fabric_proto_ident = tcm_vhost_get_fabric_proto_ident,
+ .tpg_get_wwn = tcm_vhost_get_fabric_wwn,
+ .tpg_get_tag = tcm_vhost_get_tag,
+ .tpg_get_default_depth = tcm_vhost_get_default_depth,
+ .tpg_get_pr_transport_id = tcm_vhost_get_pr_transport_id,
+ .tpg_get_pr_transport_id_len = tcm_vhost_get_pr_transport_id_len,
+ .tpg_parse_pr_out_transport_id = tcm_vhost_parse_pr_out_transport_id,
+ .tpg_check_demo_mode = tcm_vhost_check_true,
+ .tpg_check_demo_mode_cache = tcm_vhost_check_true,
+ .tpg_check_demo_mode_write_protect = tcm_vhost_check_false,
+ .tpg_check_prod_mode_write_protect = tcm_vhost_check_false,
+ .tpg_alloc_fabric_acl = tcm_vhost_alloc_fabric_acl,
+ .tpg_release_fabric_acl = tcm_vhost_release_fabric_acl,
+ .tpg_get_inst_index = tcm_vhost_tpg_get_inst_index,
+ .release_cmd = tcm_vhost_release_cmd,
+ .shutdown_session = tcm_vhost_shutdown_session,
+ .close_session = tcm_vhost_close_session,
+ .sess_get_index = tcm_vhost_sess_get_index,
+ .sess_get_initiator_sid = NULL,
+ .write_pending = tcm_vhost_write_pending,
+ .write_pending_status = tcm_vhost_write_pending_status,
+ .set_default_node_attributes = tcm_vhost_set_default_node_attrs,
+ .get_task_tag = tcm_vhost_get_task_tag,
+ .get_cmd_state = tcm_vhost_get_cmd_state,
+ .queue_data_in = tcm_vhost_queue_data_in,
+ .queue_status = tcm_vhost_queue_status,
+ .queue_tm_rsp = tcm_vhost_queue_tm_rsp,
+ .get_fabric_sense_len = tcm_vhost_get_fabric_sense_len,
+ .set_fabric_sense_len = tcm_vhost_set_fabric_sense_len,
+ /*
+ * Setup function pointers for generic logic in target_core_fabric_configfs.c
+ */
+ .fabric_make_wwn = tcm_vhost_make_tport,
+ .fabric_drop_wwn = tcm_vhost_drop_tport,
+ .fabric_make_tpg = tcm_vhost_make_tpg,
+ .fabric_drop_tpg = tcm_vhost_drop_tpg,
+ .fabric_post_link = tcm_vhost_port_link,
+ .fabric_pre_unlink = tcm_vhost_port_unlink,
+ .fabric_make_np = NULL,
+ .fabric_drop_np = NULL,
+ .fabric_make_nodeacl = tcm_vhost_make_nodeacl,
+ .fabric_drop_nodeacl = tcm_vhost_drop_nodeacl,
+};
+
+static int tcm_vhost_register_configfs(void)
+{
+ struct target_fabric_configfs *fabric;
+ int ret;
+
+ pr_debug("TCM_VHOST fabric module %s on %s/%s"
+ " on "UTS_RELEASE"\n",TCM_VHOST_VERSION, utsname()->sysname,
+ utsname()->machine);
+ /*
+ * Register the top level struct config_item_type with TCM core
+ */
+ fabric = target_fabric_configfs_init(THIS_MODULE, "vhost");
+ if (IS_ERR(fabric)) {
+ pr_err("target_fabric_configfs_init() failed\n");
+ return PTR_ERR(fabric);
+ }
+ /*
+ * Setup fabric->tf_ops from our local tcm_vhost_ops
+ */
+ fabric->tf_ops = tcm_vhost_ops;
+ /*
+ * Setup default attribute lists for various fabric->tf_cit_tmpl
+ */
+ TF_CIT_TMPL(fabric)->tfc_wwn_cit.ct_attrs = tcm_vhost_wwn_attrs;
+ TF_CIT_TMPL(fabric)->tfc_tpg_base_cit.ct_attrs = tcm_vhost_tpg_attrs;
+ TF_CIT_TMPL(fabric)->tfc_tpg_attrib_cit.ct_attrs = NULL;
+ TF_CIT_TMPL(fabric)->tfc_tpg_param_cit.ct_attrs = NULL;
+ TF_CIT_TMPL(fabric)->tfc_tpg_np_base_cit.ct_attrs = NULL;
+ TF_CIT_TMPL(fabric)->tfc_tpg_nacl_base_cit.ct_attrs = NULL;
+ TF_CIT_TMPL(fabric)->tfc_tpg_nacl_attrib_cit.ct_attrs = NULL;
+ TF_CIT_TMPL(fabric)->tfc_tpg_nacl_auth_cit.ct_attrs = NULL;
+ TF_CIT_TMPL(fabric)->tfc_tpg_nacl_param_cit.ct_attrs = NULL;
+ /*
+ * Register the fabric for use within TCM
+ */
+ ret = target_fabric_configfs_register(fabric);
+ if (ret < 0) {
+ pr_err("target_fabric_configfs_register() failed"
+ " for TCM_VHOST\n");
+ return ret;
+ }
+ /*
+ * Setup our local pointer to *fabric
+ */
+ tcm_vhost_fabric_configfs = fabric;
+ pr_debug("TCM_VHOST[0] - Set fabric -> tcm_vhost_fabric_configfs\n");
+ return 0;
+};
+
+static void tcm_vhost_deregister_configfs(void)
+{
+ if (!tcm_vhost_fabric_configfs)
+ return;
+
+ target_fabric_configfs_deregister(tcm_vhost_fabric_configfs);
+ tcm_vhost_fabric_configfs = NULL;
+ pr_debug("TCM_VHOST[0] - Cleared tcm_vhost_fabric_configfs\n");
+};
+
+static int __init tcm_vhost_init(void)
+{
+ int ret = -ENOMEM;
+
+ tcm_vhost_workqueue = alloc_workqueue("tcm_vhost", 0, 0);
+ if (!tcm_vhost_workqueue)
+ goto out;
+
+ ret = vhost_scsi_register();
+ if (ret < 0)
+ goto out_destroy_workqueue;
+
+ ret = tcm_vhost_register_configfs();
+ if (ret < 0)
+ goto out_vhost_scsi_deregister;
+
+ return 0;
+
+out_vhost_scsi_deregister:
+ vhost_scsi_deregister();
+out_destroy_workqueue:
+ destroy_workqueue(tcm_vhost_workqueue);
+out:
+ return ret;
+};
+
+static void tcm_vhost_exit(void)
+{
+ tcm_vhost_deregister_configfs();
+ vhost_scsi_deregister();
+ destroy_workqueue(tcm_vhost_workqueue);
+};
+
+MODULE_DESCRIPTION("TCM_VHOST series fabric driver");
+MODULE_LICENSE("GPL");
+module_init(tcm_vhost_init);
+module_exit(tcm_vhost_exit);
diff --git a/drivers/vhost/tcm/tcm_vhost.h b/drivers/vhost/tcm/tcm_vhost.h
new file mode 100644
index 0000000..9d6cace
--- /dev/null
+++ b/drivers/vhost/tcm/tcm_vhost.h
@@ -0,0 +1,74 @@
+#define TCM_VHOST_VERSION "v0.1"
+#define TCM_VHOST_NAMELEN 256
+#define TCM_VHOST_MAX_CDB_SIZE 32
+
+struct tcm_vhost_cmd {
+ /* Descriptor from vhost_get_vq_desc() for virt_queue segment */
+ int tvc_vq_desc;
+ /* The Tag from include/linux/virtio_scsi.h:struct virtio_scsi_cmd_req */
+ u64 tvc_tag;
+ /* The number of scatterlists associated with this cmd */
+ u32 tvc_sgl_count;
+ /* Saved unpacked SCSI LUN for tcm_vhost_submission_work() */
+ u32 tvc_lun;
+ /* Pointer to the SGL formatted memory from virtio-scsi */
+ struct scatterlist *tvc_sgl;
+ /* Pointer to response */
+ struct virtio_scsi_cmd_resp __user *tvc_resp;
+ /* Pointer to vhost_scsi for our device */
+ struct vhost_scsi *tvc_vhost;
+ /* The TCM I/O descriptor that is accessed via container_of() */
+ struct se_cmd tvc_se_cmd;
+ /* work item used for cmwq dispatch to tcm_vhost_submission_work() */
+ struct work_struct work;
+ /* Copy of the incoming SCSI command descriptor block (CDB) */
+ unsigned char tvc_cdb[TCM_VHOST_MAX_CDB_SIZE];
+ /* Sense buffer that will be mapped into outgoing status */
+ unsigned char tvc_sense_buf[TRANSPORT_SENSE_BUFFER];
+ /* Completed commands list, serviced from vhost worker thread */
+ struct list_head tvc_completion_list;
+};
+
+struct tcm_vhost_nexus {
+ /* Pointer to TCM session for I_T Nexus */
+ struct se_session *tvn_se_sess;
+};
+
+struct tcm_vhost_nacl {
+ /* Binary World Wide unique Port Name for Vhost Initiator port */
+ u64 iport_wwpn;
+ /* ASCII formatted WWPN for Sas Initiator port */
+ char iport_name[TCM_VHOST_NAMELEN];
+ /* Returned by tcm_vhost_make_nodeacl() */
+ struct se_node_acl se_node_acl;
+};
+
+struct tcm_vhost_tpg {
+ /* Vhost port target portal group tag for TCM */
+ u16 tport_tpgt;
+ /* Used to track number of TPG Port/Lun Links wrt to explict I_T Nexus shutdown */
+ atomic_t tv_tpg_port_count;
+ /* Used for vhost_scsi device reference to tpg_nexus */
+ atomic_t tv_tpg_vhost_count;
+ /* list for tcm_vhost_list */
+ struct list_head tv_tpg_list;
+ /* Used to protect access for tpg_nexus */
+ struct mutex tv_tpg_mutex;
+ /* Pointer to the TCM VHost I_T Nexus for this TPG endpoint */
+ struct tcm_vhost_nexus *tpg_nexus;
+ /* Pointer back to tcm_vhost_tport */
+ struct tcm_vhost_tport *tport;
+ /* Returned by tcm_vhost_make_tpg() */
+ struct se_portal_group se_tpg;
+};
+
+struct tcm_vhost_tport {
+ /* SCSI protocol the tport is providing */
+ u8 tport_proto_id;
+ /* Binary World Wide unique Port Name for Vhost Target port */
+ u64 tport_wwpn;
+ /* ASCII formatted WWPN for Vhost Target port */
+ char tport_name[TCM_VHOST_NAMELEN];
+ /* Returned by tcm_vhost_make_tport() */
+ struct se_wwn tport_wwn;
+};
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH RESEND 5/5] vhost-blk: Add vhost-blk support
From: Asias He @ 2012-07-18 1:22 UTC (permalink / raw)
To: Jeff Moyer; +Cc: virtualization, linux-kernel, kvm, Michael S. Tsirkin
In-Reply-To: <x49mx2yyzi5.fsf@segfault.boston.devel.redhat.com>
On 07/18/2012 03:10 AM, Jeff Moyer wrote:
> Asias He <asias@redhat.com> writes:
>
>> vhost-blk is a in kernel virito-blk device accelerator.
>>
>> This patch is based on Liu Yuan's implementation with various
>> improvements and bug fixes. Notably, this patch makes guest notify and
>> host completion processing in parallel which gives about 60% performance
>> improvement compared to Liu Yuan's implementation.
>
> So, first off, some basic questions. Is it correct to assume that you
> tested this with buffered I/O (files opened *without* O_DIRECT)?
> I'm pretty sure that if you used O_DIRECT, you'd run into problems (which
> are solved by the patch set posted by Shaggy, based on Zach Brown's work
> of many moons ago). Note that, with buffered I/O, the submission path
> is NOT asynchronous. So, any speedups you've reported are extremely
> suspect. ;-)
I always used O_DIRECT to test this patchset. And I mostly used raw
block device as guest image. Is this the reason why I did not hit the
problem you mentioned. Btw, I do have run this patchset on image based
file. I still do not see problems like IO hangs.
> Next, did you look at Shaggy's patch set? I think it would be best to
> focus your efforts on testing *that*, and implementing your work on top
> of it.
I guess you mean this one:
http://marc.info/?l=linux-fsdevel&m=133312234313122
I did not notice that until James pointed that out.
I talked with Zach and Shaggy. Shaggy said he is still working on that
patch set and will send that patch out soon.
> Having said that, I did do some review of this patch, inlined below.
Thanks, Jeff!
>> +static int vhost_blk_setup(struct vhost_blk *blk)
>> +{
>> + struct kioctx *ctx;
>> +
>> + if (blk->ioctx)
>> + return 0;
>> +
>> + blk->ioevent_nr = blk->vq.num;
>> + ctx = ioctx_alloc(blk->ioevent_nr);
>> + if (IS_ERR(ctx)) {
>> + pr_err("Failed to ioctx_alloc");
>> + return PTR_ERR(ctx);
>> + }
>> + put_ioctx(ctx);
>> + blk->ioctx = ctx;
>> +
>> + blk->ioevent = kmalloc(sizeof(struct io_event) * blk->ioevent_nr,
>> + GFP_KERNEL);
>> + if (!blk->ioevent) {
>> + pr_err("Failed to allocate memory for io_events");
>> + return -ENOMEM;
>
> You've just leaked blk->ioctx.
Yes. Will fix.
>> + }
>> +
>> + blk->reqs = kmalloc(sizeof(struct vhost_blk_req) * blk->ioevent_nr,
>> + GFP_KERNEL);
>> + if (!blk->reqs) {
>> + pr_err("Failed to allocate memory for vhost_blk_req");
>> + return -ENOMEM;
>
> And here.
Yes. Will fix.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
> [snip]
>> +static int vhost_blk_io_submit(struct vhost_blk *blk, struct file *file,
>> + struct vhost_blk_req *req,
>> + struct iovec *iov, u64 nr_vecs, loff_t offset,
>> + int opcode)
>> +{
>> + struct kioctx *ioctx = blk->ioctx;
>> + mm_segment_t oldfs = get_fs();
>> + struct kiocb_batch batch;
>> + struct blk_plug plug;
>> + struct kiocb *iocb;
>> + int ret;
>> +
>> + if (!try_get_ioctx(ioctx)) {
>> + pr_info("Failed to get ioctx");
>> + return -EAGAIN;
>> + }
>
> Using try_get_ioctx directly gives me a slightly uneasy feeling. I
> understand that you don't need to do the lookup, but at least wrap it
> and check for ->dead.
OK.
>
>> +
>> + atomic_long_inc_not_zero(&file->f_count);
>> + eventfd_ctx_get(blk->ectx);
>> +
>> + /* TODO: batch to 1 is not good! */
>
> Agreed. You should setup the batching in vhost_blk_handle_guest_kick.
> The way you've written the code, the batching is not at all helpful.
Yes. that's why there is a TODO.
>> + kiocb_batch_init(&batch, 1);
>> + blk_start_plug(&plug);
>> +
>> + iocb = aio_get_req(ioctx, &batch);
>> + if (unlikely(!iocb)) {
>> + ret = -EAGAIN;
>> + goto out;
>> + }
>> +
>> + iocb->ki_filp = file;
>> + iocb->ki_pos = offset;
>> + iocb->ki_buf = (void *)iov;
>> + iocb->ki_left = nr_vecs;
>> + iocb->ki_nbytes = nr_vecs;
>> + iocb->ki_opcode = opcode;
>> + iocb->ki_obj.user = req;
>> + iocb->ki_eventfd = blk->ectx;
>> +
>> + set_fs(KERNEL_DS);
>> + ret = aio_setup_iocb(iocb, false);
>> + set_fs(oldfs);
>> + if (unlikely(ret))
>> + goto out_put_iocb;
>> +
>> + spin_lock_irq(&ioctx->ctx_lock);
>> + if (unlikely(ioctx->dead)) {
>> + spin_unlock_irq(&ioctx->ctx_lock);
>> + ret = -EINVAL;
>> + goto out_put_iocb;
>> + }
>> + aio_run_iocb(iocb);
>> + spin_unlock_irq(&ioctx->ctx_lock);
>> +
>> + aio_put_req(iocb);
>> +
>> + blk_finish_plug(&plug);
>> + kiocb_batch_free(ioctx, &batch);
>> + put_ioctx(ioctx);
>> +
>> + return ret;
>> +out_put_iocb:
>> + aio_put_req(iocb); /* Drop extra ref to req */
>> + aio_put_req(iocb); /* Drop I/O ref to req */
>> +out:
>> + put_ioctx(ioctx);
>> + return ret;
>> +}
>> +
>
> You've duplicated a lot of io_submit_one. I'd rather see that factored
> out than to have to maintain two copies.
Agree.
> Again, what I'd *really* like to see is you rebase on top of Shaggy's
> work.
Sure. Let's wait for Shaggy's new version.
--
Asias
^ permalink raw reply
* Re: [PATCH RESEND 0/5] Add vhost-blk support
From: Asias He @ 2012-07-18 2:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jens Axboe, linux-aio, target-devel, linux-scsi, kvm,
linux-kernel, lf-virt, James Bottomley, Anthony Liguori,
Jeff Moyer, Benjamin LaHaise, Alexander Viro, linux-fsdevel,
Paolo Bonzini, Zhi Yong Wu, Christoph Hellwig, Stefan Hajnoczi
In-Reply-To: <20120717150928.GB11587@redhat.com>
On 07/17/2012 11:09 PM, Michael S. Tsirkin wrote:
> On Fri, Jul 13, 2012 at 04:55:06PM +0800, Asias He wrote:
>>
>> Hi folks,
>>
>> [I am resending to fix the broken thread in the previous one.]
>>
>> This patchset adds vhost-blk support. vhost-blk is a in kernel virito-blk
>> device accelerator. Compared to userspace virtio-blk implementation, vhost-blk
>> gives about 5% to 15% performance improvement.
>
> Same thing as tcm_host comment:
>
> It seems not 100% clear whether this driver will have major
> userspace using it. And if not, it would be very hard to support a
> driver when recent userspace does not use it in the end.
>
> I think a good idea for 3.6 would be to make it depend on
> CONFIG_STAGING. Then we don't commit to an ABI. For this, you can add
> a separate Kconfig and source it from drivers/staging/Kconfig. Maybe it
> needs to be in a separate directory drivers/vhost/staging/Kconfig.
OK.
> I Cc'd the list of tcm_host in the hope that you can cooperate on this.
Sure.
--
Asias
^ permalink raw reply
* [Xen-devel] [PATCH -v2] xen: populate correct number of pages when across mem boundary
From: zhenzhong.duan @ 2012-07-18 2:48 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: jeremy, xen-devel, x86, Feng Jin, linux-kernel, virtualization,
mingo, David Vrabel, hpa, tglx
In-Reply-To: <20120717144540.GC12374@phenom.dumpdata.com>
When populate pages across a mem boundary at bootup, the page count
populated isn't correct. This is due to mem populated to non-mem
region and ignored.
Pfn range is also wrongly aligned when mem boundary isn't page aligned.
-v2: If xen_do_chunk fail(populate), abort this chunk and any others.
Suggested by David, thanks.
For a dom0 booted with dom_mem=3368952K(0xcd9ff000-4k) dmesg diff is:
[ 0.000000] Freeing 9e-100 pfn range: 98 pages freed
[ 0.000000] 1-1 mapping on 9e->100
[ 0.000000] 1-1 mapping on cd9ff->100000
[ 0.000000] Released 98 pages of unused memory
[ 0.000000] Set 206435 page(s) to 1-1 mapping
-[ 0.000000] Populating cd9fe-cda00 pfn range: 1 pages added
+[ 0.000000] Populating cd9fe-cd9ff pfn range: 1 pages added
+[ 0.000000] Populating 100000-100061 pfn range: 97 pages added
[ 0.000000] BIOS-provided physical RAM map:
[ 0.000000] Xen: 0000000000000000 - 000000000009e000 (usable)
[ 0.000000] Xen: 00000000000a0000 - 0000000000100000 (reserved)
[ 0.000000] Xen: 0000000000100000 - 00000000cd9ff000 (usable)
[ 0.000000] Xen: 00000000cd9ffc00 - 00000000cda53c00 (ACPI NVS)
...
[ 0.000000] Xen: 0000000100000000 - 0000000100061000 (usable)
[ 0.000000] Xen: 0000000100061000 - 000000012c000000 (unusable)
...
[ 0.000000] MEMBLOCK configuration:
...
-[ 0.000000] reserved[0x4] [0x000000cd9ff000-0x000000cd9ffbff], 0xc00 bytes
-[ 0.000000] reserved[0x5] [0x00000100000000-0x00000100060fff], 0x61000 bytes
Related xen memory layout:
(XEN) Xen-e820 RAM map:
(XEN) 0000000000000000 - 000000000009ec00 (usable)
(XEN) 00000000000f0000 - 0000000000100000 (reserved)
(XEN) 0000000000100000 - 00000000cd9ffc00 (usable)
Signed-off-by: Zhenzhong Duan<zhenzhong.duan@oracle.com>
---
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index a4790bf..ead8557 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -157,25 +157,24 @@ static unsigned long __init xen_populate_chunk(
unsigned long dest_pfn;
for (i = 0, entry = list; i< map_size; i++, entry++) {
- unsigned long credits = credits_left;
unsigned long s_pfn;
unsigned long e_pfn;
unsigned long pfns;
long capacity;
- if (credits<= 0)
+ if (credits_left<= 0)
break;
if (entry->type != E820_RAM)
continue;
- e_pfn = PFN_UP(entry->addr + entry->size);
+ e_pfn = PFN_DOWN(entry->addr + entry->size);
/* We only care about E820 after the xen_start_info->nr_pages */
if (e_pfn<= max_pfn)
continue;
- s_pfn = PFN_DOWN(entry->addr);
+ s_pfn = PFN_UP(entry->addr);
/* If the E820 falls within the nr_pages, we want to start
* at the nr_pages PFN.
* If that would mean going past the E820 entry, skip it
@@ -184,23 +183,19 @@ static unsigned long __init xen_populate_chunk(
capacity = e_pfn - max_pfn;
dest_pfn = max_pfn;
} else {
- /* last_pfn MUST be within E820_RAM regions */
- if (*last_pfn&& e_pfn>= *last_pfn)
- s_pfn = *last_pfn;
capacity = e_pfn - s_pfn;
dest_pfn = s_pfn;
}
- /* If we had filled this E820_RAM entry, go to the next one. */
- if (capacity<= 0)
- continue;
- if (credits> capacity)
- credits = capacity;
+ if (credits_left< capacity)
+ capacity = credits_left;
- pfns = xen_do_chunk(dest_pfn, dest_pfn + credits, false);
+ pfns = xen_do_chunk(dest_pfn, dest_pfn + capacity, false);
done += pfns;
- credits_left -= pfns;
*last_pfn = (dest_pfn + pfns);
+ if (pfns< capacity)
+ break;
+ credits_left -= pfns;
}
return done;
}
--
1.7.3
^ permalink raw reply related
* [Xen-devel] [PATCH-v2] xen: populate correct number of pages when across mem boundary
From: zhenzhong.duan @ 2012-07-18 3:08 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: jeremy, xen-devel, x86, Feng Jin, linux-kernel, virtualization,
mingo, David Vrabel, hpa, tglx
In-Reply-To: <20120717144540.GC12374@phenom.dumpdata.com>
From c40ea05842fec8f6caa053b2d58f54608ed0835f Mon Sep 17 00:00:00 2001
From: Zhenzhong Duan<zhenzhong.duan@oracle.com>
Date: Wed, 4 Jul 2012 14:08:10 +0800
Subject: [PATCH] xen: populate right count of pages when across mem boundary
When populate pages across a mem boundary at bootup, the page count
populated isn't correct. This is due to mem populated to non-mem
region and ignored.
Pfn range is also wrongly aligned when mem boundary isn't page aligned.
-v2: If xen_do_chunk fail(populate), abort this chunk and any others.
Suggested by David, thanks.
For a dom0 booted with dom_mem=3368952K(0xcd9ff000-4k) dmesg diff is:
[ 0.000000] Freeing 9e-100 pfn range: 98 pages freed
[ 0.000000] 1-1 mapping on 9e->100
[ 0.000000] 1-1 mapping on cd9ff->100000
[ 0.000000] Released 98 pages of unused memory
[ 0.000000] Set 206435 page(s) to 1-1 mapping
-[ 0.000000] Populating cd9fe-cda00 pfn range: 1 pages added
+[ 0.000000] Populating cd9fe-cd9ff pfn range: 1 pages added
+[ 0.000000] Populating 100000-100061 pfn range: 97 pages added
[ 0.000000] BIOS-provided physical RAM map:
[ 0.000000] Xen: 0000000000000000 - 000000000009e000 (usable)
[ 0.000000] Xen: 00000000000a0000 - 0000000000100000 (reserved)
[ 0.000000] Xen: 0000000000100000 - 00000000cd9ff000 (usable)
[ 0.000000] Xen: 00000000cd9ffc00 - 00000000cda53c00 (ACPI NVS)
...
[ 0.000000] Xen: 0000000100000000 - 0000000100061000 (usable)
[ 0.000000] Xen: 0000000100061000 - 000000012c000000 (unusable)
...
[ 0.000000] MEMBLOCK configuration:
...
-[ 0.000000] reserved[0x4] [0x000000cd9ff000-0x000000cd9ffbff], 0xc00 bytes
-[ 0.000000] reserved[0x5] [0x00000100000000-0x00000100060fff], 0x61000 bytes
Related xen memory layout:
(XEN) Xen-e820 RAM map:
(XEN) 0000000000000000 - 000000000009ec00 (usable)
(XEN) 00000000000f0000 - 0000000000100000 (reserved)
(XEN) 0000000000100000 - 00000000cd9ffc00 (usable)
Signed-off-by: Zhenzhong Duan<zhenzhong.duan@oracle.com>
---
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index a4790bf..ead8557 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -157,25 +157,24 @@ static unsigned long __init xen_populate_chunk(
unsigned long dest_pfn;
for (i = 0, entry = list; i< map_size; i++, entry++) {
- unsigned long credits = credits_left;
unsigned long s_pfn;
unsigned long e_pfn;
unsigned long pfns;
long capacity;
- if (credits<= 0)
+ if (credits_left<= 0)
break;
if (entry->type != E820_RAM)
continue;
- e_pfn = PFN_UP(entry->addr + entry->size);
+ e_pfn = PFN_DOWN(entry->addr + entry->size);
/* We only care about E820 after the xen_start_info->nr_pages */
if (e_pfn<= max_pfn)
continue;
- s_pfn = PFN_DOWN(entry->addr);
+ s_pfn = PFN_UP(entry->addr);
/* If the E820 falls within the nr_pages, we want to start
* at the nr_pages PFN.
* If that would mean going past the E820 entry, skip it
@@ -184,23 +183,19 @@ static unsigned long __init xen_populate_chunk(
capacity = e_pfn - max_pfn;
dest_pfn = max_pfn;
} else {
- /* last_pfn MUST be within E820_RAM regions */
- if (*last_pfn&& e_pfn>= *last_pfn)
- s_pfn = *last_pfn;
capacity = e_pfn - s_pfn;
dest_pfn = s_pfn;
}
- /* If we had filled this E820_RAM entry, go to the next one. */
- if (capacity<= 0)
- continue;
- if (credits> capacity)
- credits = capacity;
+ if (credits_left< capacity)
+ capacity = credits_left;
- pfns = xen_do_chunk(dest_pfn, dest_pfn + credits, false);
+ pfns = xen_do_chunk(dest_pfn, dest_pfn + capacity, false);
done += pfns;
- credits_left -= pfns;
*last_pfn = (dest_pfn + pfns);
+ if (pfns< capacity)
+ break;
+ credits_left -= pfns;
}
return done;
}
--
1.7.3
^ permalink raw reply related
* Re: [Xen-devel] [PATCH-v2] xen: populate correct number of pages when across mem boundary
From: zhenzhong.duan @ 2012-07-18 4:40 UTC (permalink / raw)
To: zhenzhong.duan
Cc: jeremy, xen-devel, Konrad Rzeszutek Wilk, x86, Feng Jin,
linux-kernel, virtualization, mingo, David Vrabel, hpa, tglx
In-Reply-To: <500628A1.3060609@oracle.com>
Sorry, pls ignore it. Tab still be translated to space.
于 2012-07-18 11:08, zhenzhong.duan 写道:
> From c40ea05842fec8f6caa053b2d58f54608ed0835f Mon Sep 17 00:00:00 2001
> From: Zhenzhong Duan<zhenzhong.duan@oracle.com>
> Date: Wed, 4 Jul 2012 14:08:10 +0800
> Subject: [PATCH] xen: populate right count of pages when across mem
> boundary
>
> When populate pages across a mem boundary at bootup, the page count
> populated isn't correct. This is due to mem populated to non-mem
> region and ignored.
>
> Pfn range is also wrongly aligned when mem boundary isn't page aligned.
>
> -v2: If xen_do_chunk fail(populate), abort this chunk and any others.
> Suggested by David, thanks.
>
> For a dom0 booted with dom_mem=3368952K(0xcd9ff000-4k) dmesg diff is:
> [ 0.000000] Freeing 9e-100 pfn range: 98 pages freed
> [ 0.000000] 1-1 mapping on 9e->100
> [ 0.000000] 1-1 mapping on cd9ff->100000
> [ 0.000000] Released 98 pages of unused memory
> [ 0.000000] Set 206435 page(s) to 1-1 mapping
> -[ 0.000000] Populating cd9fe-cda00 pfn range: 1 pages added
> +[ 0.000000] Populating cd9fe-cd9ff pfn range: 1 pages added
> +[ 0.000000] Populating 100000-100061 pfn range: 97 pages added
> [ 0.000000] BIOS-provided physical RAM map:
> [ 0.000000] Xen: 0000000000000000 - 000000000009e000 (usable)
> [ 0.000000] Xen: 00000000000a0000 - 0000000000100000 (reserved)
> [ 0.000000] Xen: 0000000000100000 - 00000000cd9ff000 (usable)
> [ 0.000000] Xen: 00000000cd9ffc00 - 00000000cda53c00 (ACPI NVS)
> ...
> [ 0.000000] Xen: 0000000100000000 - 0000000100061000 (usable)
> [ 0.000000] Xen: 0000000100061000 - 000000012c000000 (unusable)
> ...
> [ 0.000000] MEMBLOCK configuration:
> ...
> -[ 0.000000] reserved[0x4] [0x000000cd9ff000-0x000000cd9ffbff], 0xc00
> bytes
> -[ 0.000000] reserved[0x5] [0x00000100000000-0x00000100060fff],
> 0x61000 bytes
>
> Related xen memory layout:
> (XEN) Xen-e820 RAM map:
> (XEN) 0000000000000000 - 000000000009ec00 (usable)
> (XEN) 00000000000f0000 - 0000000000100000 (reserved)
> (XEN) 0000000000100000 - 00000000cd9ffc00 (usable)
>
> Signed-off-by: Zhenzhong Duan<zhenzhong.duan@oracle.com>
> ---
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index a4790bf..ead8557 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -157,25 +157,24 @@ static unsigned long __init xen_populate_chunk(
> unsigned long dest_pfn;
>
> for (i = 0, entry = list; i< map_size; i++, entry++) {
> - unsigned long credits = credits_left;
> unsigned long s_pfn;
> unsigned long e_pfn;
> unsigned long pfns;
> long capacity;
>
> - if (credits<= 0)
> + if (credits_left<= 0)
> break;
>
> if (entry->type != E820_RAM)
> continue;
>
> - e_pfn = PFN_UP(entry->addr + entry->size);
> + e_pfn = PFN_DOWN(entry->addr + entry->size);
>
> /* We only care about E820 after the xen_start_info->nr_pages */
> if (e_pfn<= max_pfn)
> continue;
>
> - s_pfn = PFN_DOWN(entry->addr);
> + s_pfn = PFN_UP(entry->addr);
> /* If the E820 falls within the nr_pages, we want to start
> * at the nr_pages PFN.
> * If that would mean going past the E820 entry, skip it
> @@ -184,23 +183,19 @@ static unsigned long __init xen_populate_chunk(
> capacity = e_pfn - max_pfn;
> dest_pfn = max_pfn;
> } else {
> - /* last_pfn MUST be within E820_RAM regions */
> - if (*last_pfn&& e_pfn>= *last_pfn)
> - s_pfn = *last_pfn;
> capacity = e_pfn - s_pfn;
> dest_pfn = s_pfn;
> }
> - /* If we had filled this E820_RAM entry, go to the next one. */
> - if (capacity<= 0)
> - continue;
>
> - if (credits> capacity)
> - credits = capacity;
> + if (credits_left< capacity)
> + capacity = credits_left;
>
> - pfns = xen_do_chunk(dest_pfn, dest_pfn + credits, false);
> + pfns = xen_do_chunk(dest_pfn, dest_pfn + capacity, false);
> done += pfns;
> - credits_left -= pfns;
> *last_pfn = (dest_pfn + pfns);
> + if (pfns< capacity)
> + break;
> + credits_left -= pfns;
> }
> return done;
> }
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [Xen-devel] [PATCH -v2] xen: populate correct number of pages when across mem boundary
From: zhenzhong.duan @ 2012-07-18 5:06 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, David Vrabel
Cc: jeremy, xen-devel, x86, Feng Jin, linux-kernel, virtualization,
mingo, hpa, tglx
When populate pages across a mem boundary at bootup, the page count
populated isn't correct. This is due to mem populated to non-mem
region and ignored.
Pfn range is also wrongly aligned when mem boundary isn't page aligned.
-v2: If xen_do_chunk fail(populate), abort this chunk and any others.
Suggested by David, thanks.
For a dom0 booted with dom_mem=3368952K(0xcd9ff000-4k) dmesg diff is:
[ 0.000000] Freeing 9e-100 pfn range: 98 pages freed
[ 0.000000] 1-1 mapping on 9e->100
[ 0.000000] 1-1 mapping on cd9ff->100000
[ 0.000000] Released 98 pages of unused memory
[ 0.000000] Set 206435 page(s) to 1-1 mapping
-[ 0.000000] Populating cd9fe-cda00 pfn range: 1 pages added
+[ 0.000000] Populating cd9fe-cd9ff pfn range: 1 pages added
+[ 0.000000] Populating 100000-100061 pfn range: 97 pages added
[ 0.000000] BIOS-provided physical RAM map:
[ 0.000000] Xen: 0000000000000000 - 000000000009e000 (usable)
[ 0.000000] Xen: 00000000000a0000 - 0000000000100000 (reserved)
[ 0.000000] Xen: 0000000000100000 - 00000000cd9ff000 (usable)
[ 0.000000] Xen: 00000000cd9ffc00 - 00000000cda53c00 (ACPI NVS)
...
[ 0.000000] Xen: 0000000100000000 - 0000000100061000 (usable)
[ 0.000000] Xen: 0000000100061000 - 000000012c000000 (unusable)
...
[ 0.000000] MEMBLOCK configuration:
...
-[ 0.000000] reserved[0x4] [0x000000cd9ff000-0x000000cd9ffbff], 0xc00 bytes
-[ 0.000000] reserved[0x5] [0x00000100000000-0x00000100060fff], 0x61000 bytes
Related xen memory layout:
(XEN) Xen-e820 RAM map:
(XEN) 0000000000000000 - 000000000009ec00 (usable)
(XEN) 00000000000f0000 - 0000000000100000 (reserved)
(XEN) 0000000000100000 - 00000000cd9ffc00 (usable)
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
---
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index a4790bf..ead8557 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -157,25 +157,24 @@ static unsigned long __init xen_populate_chunk(
unsigned long dest_pfn;
for (i = 0, entry = list; i < map_size; i++, entry++) {
- unsigned long credits = credits_left;
unsigned long s_pfn;
unsigned long e_pfn;
unsigned long pfns;
long capacity;
- if (credits <= 0)
+ if (credits_left <= 0)
break;
if (entry->type != E820_RAM)
continue;
- e_pfn = PFN_UP(entry->addr + entry->size);
+ e_pfn = PFN_DOWN(entry->addr + entry->size);
/* We only care about E820 after the xen_start_info->nr_pages */
if (e_pfn <= max_pfn)
continue;
- s_pfn = PFN_DOWN(entry->addr);
+ s_pfn = PFN_UP(entry->addr);
/* If the E820 falls within the nr_pages, we want to start
* at the nr_pages PFN.
* If that would mean going past the E820 entry, skip it
@@ -184,23 +183,19 @@ static unsigned long __init xen_populate_chunk(
capacity = e_pfn - max_pfn;
dest_pfn = max_pfn;
} else {
- /* last_pfn MUST be within E820_RAM regions */
- if (*last_pfn && e_pfn >= *last_pfn)
- s_pfn = *last_pfn;
capacity = e_pfn - s_pfn;
dest_pfn = s_pfn;
}
- /* If we had filled this E820_RAM entry, go to the next one. */
- if (capacity <= 0)
- continue;
- if (credits > capacity)
- credits = capacity;
+ if (credits_left < capacity)
+ capacity = credits_left;
- pfns = xen_do_chunk(dest_pfn, dest_pfn + credits, false);
+ pfns = xen_do_chunk(dest_pfn, dest_pfn + capacity, false);
done += pfns;
- credits_left -= pfns;
*last_pfn = (dest_pfn + pfns);
+ if (pfns < capacity)
+ break;
+ credits_left -= pfns;
}
return done;
}
--
1.7.3
^ permalink raw reply related
* Re: [PATCH v4 1/3] mm: introduce compaction and migration for virtio ballooned pages
From: Minchan Kim @ 2012-07-18 5:48 UTC (permalink / raw)
To: Rafael Aquini
Cc: Rik van Riel, Konrad Rzeszutek Wilk, Michael S. Tsirkin,
linux-kernel, virtualization, linux-mm, Andi Kleen, Andrew Morton,
Rafael Aquini
In-Reply-To: <49f828a9331c9b729fcf77226006921ec5bc52fa.1342485774.git.aquini@redhat.com>
Hi Rafael,
On Tue, Jul 17, 2012 at 01:50:41PM -0300, Rafael Aquini wrote:
> This patch introduces the helper functions as well as the necessary changes
> to teach compaction and migration bits how to cope with pages which are
> part of a guest memory balloon, in order to make them movable by memory
> compaction procedures.
>
> Signed-off-by: Rafael Aquini <aquini@redhat.com>
> ---
> include/linux/mm.h | 15 +++++++
> mm/compaction.c | 126 ++++++++++++++++++++++++++++++++++++++++++++--------
> mm/migrate.c | 30 ++++++++++++-
> 3 files changed, 151 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index b36d08c..3112198 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1629,5 +1629,20 @@ static inline unsigned int debug_guardpage_minorder(void) { return 0; }
> static inline bool page_is_guard(struct page *page) { return false; }
> #endif /* CONFIG_DEBUG_PAGEALLOC */
>
> +#if (defined(CONFIG_VIRTIO_BALLOON) || \
> + defined(CONFIG_VIRTIO_BALLOON_MODULE)) && defined(CONFIG_COMPACTION)
> +extern bool putback_balloon_page(struct page *);
> +extern struct address_space *balloon_mapping;
> +
> +static inline bool is_balloon_page(struct page *page)
> +{
> + return (page->mapping == balloon_mapping) ? true : false;
> +}
> +#else
> +static inline bool is_balloon_page(struct page *page) { return false; }
> +static inline bool isolate_balloon_page(struct page *page) { return false; }
> +static inline bool putback_balloon_page(struct page *page) { return false; }
> +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && COMPACTION */
> +
> #endif /* __KERNEL__ */
> #endif /* _LINUX_MM_H */
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 2f42d95..51eac0c 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -14,6 +14,7 @@
> #include <linux/backing-dev.h>
> #include <linux/sysctl.h>
> #include <linux/sysfs.h>
> +#include <linux/export.h>
> #include "internal.h"
>
> #if defined CONFIG_COMPACTION || defined CONFIG_CMA
> @@ -21,6 +22,85 @@
> #define CREATE_TRACE_POINTS
> #include <trace/events/compaction.h>
>
> +#if defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE)
> +/*
> + * Balloon pages special page->mapping.
> + * Users must properly allocate and initialize an instance of balloon_mapping,
> + * and set it as the page->mapping for balloon enlisted page instances.
> + * There is no need on utilizing struct address_space locking schemes for
> + * balloon_mapping as, once it gets initialized at balloon driver, it will
> + * remain just like a static reference that helps us on identifying a guest
> + * ballooned page by its mapping, as well as it will keep the 'a_ops' callback
> + * pointers to the functions that will execute the balloon page mobility tasks.
> + *
> + * address_space_operations necessary methods for ballooned pages:
> + * .migratepage - used to perform balloon's page migration (as is)
> + * .invalidatepage - used to isolate a page from balloon's page list
> + * .freepage - used to reinsert an isolated page to balloon's page list
> + */
> +struct address_space *balloon_mapping;
> +EXPORT_SYMBOL_GPL(balloon_mapping);
> +
> +static inline void __isolate_balloon_page(struct page *page)
> +{
> + page->mapping->a_ops->invalidatepage(page, 0);
> +}
> +
> +static inline void __putback_balloon_page(struct page *page)
> +{
> + page->mapping->a_ops->freepage(page);
> +}
> +
> +/* __isolate_lru_page() counterpart for a ballooned page */
> +static bool isolate_balloon_page(struct page *page)
> +{
> + if (WARN_ON(!is_balloon_page(page)))
> + return false;
I am not sure we need this because you alreay check it before calling
isolate_balloon_page. If you really need it, it would be better to
add likely in isolate_balloon_page, too.
> +
> + if (likely(get_page_unless_zero(page))) {
> + /*
> + * We can race against move_to_new_page() & __unmap_and_move().
> + * If we stumble across a locked balloon page and succeed on
> + * isolating it, the result tends to be disastrous.
> + */
> + if (likely(trylock_page(page))) {
Hmm, I can't understand your comment.
What does this lock protect? Could you elaborate it with code sequence?
> + /*
> + * A ballooned page, by default, has just one refcount.
> + * Prevent concurrent compaction threads from isolating
> + * an already isolated balloon page.
> + */
> + if (is_balloon_page(page) && (page_count(page) == 2)) {
> + __isolate_balloon_page(page);
> + unlock_page(page);
> + return true;
> + }
> + unlock_page(page);
> + }
> + /* Drop refcount taken for this already isolated page */
> + put_page(page);
> + }
> + return false;
> +}
> +
> +/* putback_lru_page() counterpart for a ballooned page */
> +bool putback_balloon_page(struct page *page)
> +{
> + if (WARN_ON(!is_balloon_page(page)))
> + return false;
You already check WARN_ON in putback_lru_pages so we don't need it in here.
And you can add likely in here, too.
> +
> + if (likely(trylock_page(page))) {
> + if (is_balloon_page(page)) {
> + __putback_balloon_page(page);
> + put_page(page);
> + unlock_page(page);
> + return true;
> + }
> + unlock_page(page);
> + }
> + return false;
> +}
> +#endif /* CONFIG_VIRTIO_BALLOON || CONFIG_VIRTIO_BALLOON_MODULE */
> +
> static unsigned long release_freepages(struct list_head *freelist)
> {
> struct page *page, *next;
> @@ -312,32 +392,40 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
> continue;
> }
>
> - if (!PageLRU(page))
> - continue;
> -
> /*
> - * PageLRU is set, and lru_lock excludes isolation,
> - * splitting and collapsing (collapsing has already
> - * happened if PageLRU is set).
> + * It is possible to migrate LRU pages and balloon pages.
> + * Skip any other type of page.
> */
> - if (PageTransHuge(page)) {
> - low_pfn += (1 << compound_order(page)) - 1;
> - continue;
> - }
> + if (PageLRU(page)) {
> + /*
> + * PageLRU is set, and lru_lock excludes isolation,
> + * splitting and collapsing (collapsing has already
> + * happened if PageLRU is set).
> + */
> + if (PageTransHuge(page)) {
> + low_pfn += (1 << compound_order(page)) - 1;
> + continue;
> + }
>
> - if (!cc->sync)
> - mode |= ISOLATE_ASYNC_MIGRATE;
> + if (!cc->sync)
> + mode |= ISOLATE_ASYNC_MIGRATE;
>
> - lruvec = mem_cgroup_page_lruvec(page, zone);
> + lruvec = mem_cgroup_page_lruvec(page, zone);
>
> - /* Try isolate the page */
> - if (__isolate_lru_page(page, mode) != 0)
> - continue;
> + /* Try isolate the page */
> + if (__isolate_lru_page(page, mode) != 0)
> + continue;
> +
> + VM_BUG_ON(PageTransCompound(page));
>
> - VM_BUG_ON(PageTransCompound(page));
> + /* Successfully isolated */
> + del_page_from_lru_list(page, lruvec, page_lru(page));
> + } else if (is_balloon_page(page)) {
unlikely?
> + if (!isolate_balloon_page(page))
> + continue;
> + } else
> + continue;
>
> - /* Successfully isolated */
> - del_page_from_lru_list(page, lruvec, page_lru(page));
> list_add(&page->lru, migratelist);
> cc->nr_migratepages++;
> nr_isolated++;
> diff --git a/mm/migrate.c b/mm/migrate.c
> index be26d5c..59c7bc5 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -78,7 +78,10 @@ void putback_lru_pages(struct list_head *l)
> list_del(&page->lru);
> dec_zone_page_state(page, NR_ISOLATED_ANON +
> page_is_file_cache(page));
> - putback_lru_page(page);
> + if (unlikely(is_balloon_page(page)))
> + WARN_ON(!putback_balloon_page(page));
> + else
> + putback_lru_page(page);
Hmm, I don't like this.
The function name says we putback *lru* pages, but balloon page isn't.
How about this?
diff --git a/mm/compaction.c b/mm/compaction.c
index aad0a16..b07cd67 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -298,6 +298,8 @@ static bool too_many_isolated(struct zone *zone)
* Apart from cc->migratepages and cc->nr_migratetypes this function
* does not modify any cc's fields, in particular it does not modify
* (or read for that matter) cc->migrate_pfn.
+ *
+ * For returning page, you should use putback_pages instead of putback_lru_pages
*/
unsigned long
isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
@@ -827,7 +829,7 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
/* Release LRU pages not migrated */
if (err) {
- putback_lru_pages(&cc->migratepages);
+ putback_pages(&cc->migratepages);
cc->nr_migratepages = 0;
if (err == -ENOMEM) {
ret = COMPACT_PARTIAL;
diff --git a/mm/migrate.c b/mm/migrate.c
index 9705e70..a96b840 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -86,6 +86,22 @@ void putback_lru_pages(struct list_head *l)
}
}
+ /* blah blah .... */
+void putback_pages(struct list_head *l)
+{
+ struct page *page;
+ struct page *page2;
+
+ list_for_each_entry_safe(page, page2, l, lru) {
+ list_del(&page->lru);
+ dec_zone_page_state(page, NR_ISOLATED_ANON +
+ page_is_file_cache(page));
+ if (unlikely(is_balloon_page(page)))
+ WARN_ON(!putback_balloon_page(page));
+ else
+ putback_lru_page(page);
+ }
+}
+
/*
* Restore a potential migration pte to a working pte entry
*/
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 32985dd..decb82a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5655,7 +5655,7 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)
0, false, MIGRATE_SYNC);
}
- putback_lru_pages(&cc.migratepages);
+ putback_pages(&cc.migratepages);
return ret > 0 ? 0 : ret;
}
> }
> }
>
> @@ -783,6 +786,17 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
> }
> }
>
> + if (is_balloon_page(page)) {
unlikely?
> + /*
> + * A ballooned page does not need any special attention from
> + * physical to virtual reverse mapping procedures.
> + * Skip any attempt to unmap PTEs or to remap swap cache,
> + * in order to avoid burning cycles at rmap level.
> + */
> + remap_swapcache = 0;
> + goto skip_unmap;
> + }
> +
> /*
> * Corner case handling:
> * 1. When a new swap-cache page is read into, it is added to the LRU
> @@ -852,6 +866,20 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
> goto out;
>
> rc = __unmap_and_move(page, newpage, force, offlining, mode);
> +
> + if (is_balloon_page(newpage)) {
unlikely?
> + /*
> + * A ballooned page has been migrated already. Now, it is the
> + * time to wrap-up counters, handle the old page back to Buddy
> + * and return.
> + */
> + list_del(&page->lru);
> + dec_zone_page_state(page, NR_ISOLATED_ANON +
> + page_is_file_cache(page));
> + put_page(page);
> + __free_page(page);
Why do you use __free_page instead of put_page?
> + return rc;
> + }
> out:
> if (rc != -EAGAIN) {
> /*
> --
> 1.7.10.4
The feeling I look at your code in detail is that it makes compaction/migration
code rather complicated because compaction/migration assumes source/target would
be LRU pages.
How often memory ballooning happens? Does it make sense to hook it in generic
functions if it's very rare?
Couldn't you implement it like huge page?
It doesn't make current code complicated and doesn't affect performance
In compaction,
#ifdef CONFIG_VIRTIO_BALLOON
static int compact_zone(struct zone *zone, struct compact_control *cc, bool balloon)
{
if (balloon) {
isolate_balloonpages
migrate_balloon_pages
unmap_and_move_balloon_page
putback_balloonpages
}
}
#endif
I'm not sure memory ballooning so it might be dumb question.
Can we trigger it once only when ballooning happens?
Thanks!
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Kind regards,
Minchan Kim
^ permalink raw reply related
* Re: [PATCH 0/5] Add vhost-blk support
From: Asias He @ 2012-07-18 8:12 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel, Paolo Bonzini
In-Reply-To: <CAJSP0QW0tPV3WASGOBRE4hL=8zzSFQ6T=bHbEwMAzmHR9wnwJg@mail.gmail.com>
On 07/17/2012 07:11 PM, Stefan Hajnoczi wrote:
> On Tue, Jul 17, 2012 at 10:21 AM, Asias He <asias@redhat.com> wrote:
>> On 07/17/2012 04:52 PM, Paolo Bonzini wrote:
>>>
>>> Il 17/07/2012 10:29, Asias He ha scritto:
>>>>
>>>> So, vhost-blk at least saves ~6 syscalls for us in each request.
>>>
>>>
>>> Are they really 6? If I/O is coalesced by a factor of 3, for example
>>> (i.e. each exit processes 3 requests), it's really 2 syscalls per request.
>>
>>
>> Well. I am counting the number of syscalls in one notify and response
>> process. Sure the IO can be coalesced.
>
> Linux AIO also supports batching in io_submit() and io_getevents().
> Depending on the request pattern in the vring when you process it, you
> should be able to do better than 1 set of syscalls per host I/O
> request.
>
> Are you taking advantage of that at the moment in your userspace benchmark?
OK. I know that batching in io_submit() and io_getevetns(). There was a
patch for kvm tool long time ago. Now, both vhost-blk and kvm tool are
not taking advantage of that atm. There are issues: e.g. How many number
of request we want to batch? Does this batching hurt latency?
--
Asias
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Asias He @ 2012-07-18 8:47 UTC (permalink / raw)
To: Paolo Bonzini
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel
In-Reply-To: <50056275.6000407@redhat.com>
On 07/17/2012 09:02 PM, Paolo Bonzini wrote:
> Il 17/07/2012 14:48, Michael S. Tsirkin ha scritto:
>> On Tue, Jul 17, 2012 at 01:03:39PM +0100, Stefan Hajnoczi wrote:
>>> On Tue, Jul 17, 2012 at 12:54 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>> Knowing the answer to that is important before anyone can say whether
>>>>> this approach is good or not.
>>>>>
>>>>> Stefan
>>>>
>>>> Why is it?
>>>
>>> Because there might be a fix to kvmtool which closes the gap. It
>>> would be embarassing if vhost-blk was pushed just because no one
>>> looked into what is actually going on.
>>
>> Embarrasing to whom? Is someone working on an optimization that
>> makes the work in question redundant, with posting just around
>> the corner? Then maybe the thing to do is just wait a bit.
>
> Of course there is work going on to make QEMU perform better. Not sure
> about lkvm.
Of course for lkvm also.
>>> And on the flipside, hard evidence of an overhead that cannot be
>>> resolved could be good reason to do more vhost devices in the future.
>>
>> How can one have hard evidence of an overhead that cannot be resolved?
>
> Since we do have two completely independent userspaces (lkvm and
> data-plane QEMU), you can build up some compelling evidence of an
> overhead that cannot be resolved in user space.
This does not build the hard evidence either. How can one prove that
userspace lkvm and data-plane QEMU can not be improved further? The same
for vhost-blk.
>>> Either way, it's useful to do this before going further.
>>
>> I think each work should be discussed on its own merits. Maybe
>> vhost-blk is just well written. So? What is your conclusion?
>
> If it's just that vhost-blk is written well, my conclusion is that lkvm
> people should look into improving their virtio-blk userspace. We take
> hints from each other all the time, for example virtio-scsi will have
> unlocked kick in 3.6.
>
> Why can't vhost-* just get into staging, and we call it a day?
OK. I'm fine with staging.
--
Asias
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Ronen Hod @ 2012-07-18 9:46 UTC (permalink / raw)
To: Asias He
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel, Paolo Bonzini
In-Reply-To: <50052E7E.6020100@redhat.com>
On 07/17/2012 12:21 PM, Asias He wrote:
> On 07/17/2012 04:52 PM, Paolo Bonzini wrote:
>> Il 17/07/2012 10:29, Asias He ha scritto:
>>> So, vhost-blk at least saves ~6 syscalls for us in each request.
>>
>> Are they really 6? If I/O is coalesced by a factor of 3, for example
>> (i.e. each exit processes 3 requests), it's really 2 syscalls per request.
>
> Well. I am counting the number of syscalls in one notify and response process. Sure the IO can be coalesced.
Note that Asias is using very fast "disks" (FusionIO & Ramdisk).
- This might affect the level of coalescing both ways, depending on the scenario and algorithm.
- This also means that the 5%-15% gain will probably be lower in real life.
Ronen.
>
>> Also, is there anything we can improve? Perhaps we can modify epoll and
>> ask it to clear the eventfd for us (would save 2 reads)? Or
>> io_getevents (would save 1)?
>>
>>> I guess you mean qemu here. Yes, in theory, qemu's block layer can be
>>> improved to achieve similar performance as vhost-blk or kvm tool's
>>> userspace virito-blk has. But I think it makes no sense to prevent one
>>> solution becase there is another in theory solution called: we can do
>>> similar in qemu.
>>
>> It depends. Like vhost-scsi, vhost-blk has the problem of a crippled
>> feature set: no support for block device formats, non-raw protocols,
>> etc. This makes it different from vhost-net.
>
> Data-plane qemu also has this cripppled feature set problem, no? Does user always choose to use block devices format like qcow2? What if they prefer raw image or raw block device?
>
>>
>> So it begs the question, is it going to be used in production, or just a
>> useful reference tool?
>
> This should be decided by user, I can not speak for them. What is wrong with adding one option for user which they can decide?
>
^ permalink raw reply
* Re: [PATCH RESEND 0/5] Add vhost-blk support
From: Stefan Hajnoczi @ 2012-07-18 11:42 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jens Axboe, linux-aio, linux-scsi, kvm, linux-kernel, lf-virt,
James Bottomley, Anthony Liguori, Jeff Moyer, target-devel,
Alexander Viro, Paolo Bonzini, Benjamin LaHaise, linux-fsdevel,
Zhi Yong Wu, Christoph Hellwig, Stefan Hajnoczi
In-Reply-To: <20120717150928.GB11587@redhat.com>
On Tue, Jul 17, 2012 at 4:09 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Jul 13, 2012 at 04:55:06PM +0800, Asias He wrote:
>>
>> Hi folks,
>>
>> [I am resending to fix the broken thread in the previous one.]
>>
>> This patchset adds vhost-blk support. vhost-blk is a in kernel virito-blk
>> device accelerator. Compared to userspace virtio-blk implementation, vhost-blk
>> gives about 5% to 15% performance improvement.
>
> Same thing as tcm_host comment:
>
> It seems not 100% clear whether this driver will have major
> userspace using it. And if not, it would be very hard to support a
> driver when recent userspace does not use it in the end.
>
> I think a good idea for 3.6 would be to make it depend on
> CONFIG_STAGING. Then we don't commit to an ABI. For this, you can add
> a separate Kconfig and source it from drivers/staging/Kconfig. Maybe it
> needs to be in a separate directory drivers/vhost/staging/Kconfig.
>
> I Cc'd the list of tcm_host in the hope that you can cooperate on this.
>
Adding it to staging allows more people to try it out, so that's a
good thing. If I get a moment to play with it I'll let you know the
results.
Stefan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox