user-mode-linux-devel

Re: [uml-devel] [PATCH 2/2] Bulk IO Transaction support part 1

[Richard W. <ri...@no...>]

Am 10.01.2016 um 17:36 schrieb Anton Ivanov:
> On 10/01/16 16:00, Richard Weinberger wrote:
>> On Mon, Dec 21, 2015 at 8:04 PM, Anton Ivanov
>> <ant...@ko...> wrote:
>>> Hi list, hi Richard,
>>>
>>> This rather primitive patchset pushes disk IO by ~ 15%.
>>>
>>> dd to /dev/null on a host memory cached 16G sparse disk image with
>>> ubuntu on it goes from 512MB/s on my machine to 580MB/s.
>>>
>>> Part 2 will be ring buffers and read/write poll loop in the io_thread as
>>> well as elimination of "partial read/write" workarounds. While it does
>>> not push disk benchmarks a lot it makes UML more responsive.
>>>
>>> Part 3 will be merging adjacent IO transactions (by file location) via
>>> vector IO. This pushes trhoughput even further by quite a bit.
>>>
>>> While I can try to submit all of that in one go, I'd rather submit it in
>>> chunks so it is easier to review even if this will result in patch churn
>>> (the whole do_safe_read malarkey will bite the bullet once part 2 is out).
>> Is this already stable enough for this merge window?
>> At least checkpatch.pl dislikes part 1. :(
>>
> You can leave this one out. I will be working further on this to squeeze
> ~ 10% more and introduce vector IO.

Ok!

Thanks,
//richard

Re: [uml-devel] [PATCH v5 0/2] um: Protect memory mapped file

[Richard W. <ri...@no...>]

Am 22.12.2015 um 22:15 schrieb Mickaël Salaün:
> This series protect the memory mapped file. This apply on v4.4-rc6.
> 
> Changes since v4:
> * fix spelling [1/2]
> 
> Changes since v3:
> * add Tristan Schmelcher's ack
> 
> Changes since v2; addressed Tristan Schmelcher's comment:
> * remove the whole fchmod call [1/2]
> 
> Changes since v1; addressed Richard Weinberger's comments:
> * add attacker model to the patch description [1/2]
> * remove errno reset [2/2]
> 
> Regards,
>  Mickaël
> 
> Mickaël Salaün (2):
>   um: Do not set unsecure permission for temporary file
>   um: Use race-free temporary file creation

Applied!

Thanks,
//richard

Re: [uml-devel] [PATCH] um: link with -lpthread

[Richard W. <ri...@no...>]

Am 31.12.2015 um 17:06 schrieb Vegard Nossum:
> Similarly to commit fb1770aa78a43530940d0c2dd161e77bc705bdac, with gcc 5
> on Ubuntu and CONFIG_STATIC_LINK=y I was seeing these linker errors:
> 
> /usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new':
> (.text+0xcd): undefined reference to `pthread_once'
> /usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new':
> (.text+0x126): undefined reference to `pthread_attr_init'
> /usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new':
> (.text+0x168): undefined reference to `pthread_attr_setdetachstate'
> [...]
> 
> Obviously we also need -lpthread for librt.a.
> 
> Signed-off-by: Vegard Nossum <veg...@or...>

Applied!

Thanks,
//richard

Re: [uml-devel] [PATCH 2/2] Bulk IO Transaction support part 1

[Anton I. <ant...@ko...>]

On 10/01/16 16:00, Richard Weinberger wrote:
> On Mon, Dec 21, 2015 at 8:04 PM, Anton Ivanov
> <ant...@ko...> wrote:
>> Hi list, hi Richard,
>>
>> This rather primitive patchset pushes disk IO by ~ 15%.
>>
>> dd to /dev/null on a host memory cached 16G sparse disk image with
>> ubuntu on it goes from 512MB/s on my machine to 580MB/s.
>>
>> Part 2 will be ring buffers and read/write poll loop in the io_thread as
>> well as elimination of "partial read/write" workarounds. While it does
>> not push disk benchmarks a lot it makes UML more responsive.
>>
>> Part 3 will be merging adjacent IO transactions (by file location) via
>> vector IO. This pushes trhoughput even further by quite a bit.
>>
>> While I can try to submit all of that in one go, I'd rather submit it in
>> chunks so it is easier to review even if this will result in patch churn
>> (the whole do_safe_read malarkey will bite the bullet once part 2 is out).
> Is this already stable enough for this merge window?
> At least checkpatch.pl dislikes part 1. :(
>
You can leave this one out. I will be working further on this to squeeze
~ 10% more and introduce vector IO.

A.

Re: [uml-devel] [PATCH 2/2] Bulk IO Transaction support part 1

[Richard W. <ric...@gm...>]

On Mon, Dec 21, 2015 at 8:04 PM, Anton Ivanov
<ant...@ko...> wrote:
> Hi list, hi Richard,
>
> This rather primitive patchset pushes disk IO by ~ 15%.
>
> dd to /dev/null on a host memory cached 16G sparse disk image with
> ubuntu on it goes from 512MB/s on my machine to 580MB/s.
>
> Part 2 will be ring buffers and read/write poll loop in the io_thread as
> well as elimination of "partial read/write" workarounds. While it does
> not push disk benchmarks a lot it makes UML more responsive.
>
> Part 3 will be merging adjacent IO transactions (by file location) via
> vector IO. This pushes trhoughput even further by quite a bit.
>
> While I can try to submit all of that in one go, I'd rather submit it in
> chunks so it is easier to review even if this will result in patch churn
> (the whole do_safe_read malarkey will bite the bullet once part 2 is out).

Is this already stable enough for this merge window?
At least checkpatch.pl dislikes part 1. :(

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH 1/2] Update UBD to use pread/pwrite family of functions

[Richard W. <ric...@gm...>]

On Mon, Dec 21, 2015 at 7:54 PM, Anton Ivanov <ai...@br...> wrote:
> This decreases the number of syscalls per read/write by half.
>
> Signed-off-by: Anton Ivanov <ai...@br...>

Applied!

-- 
Thanks,
//richard

Re: [uml-devel] IRQ handler reentrancy

[Richard W. <ric...@gm...>]

On Mon, Dec 21, 2015 at 12:55 PM, Anton Ivanov
<ant...@ko...> wrote:
> Patch in your queue, current code looks OK.
>
> I will try to assemble the full ubd acceleration sequence of patches
> this afternoon to submit that as well.

Applied. Let's see how much will explode. :-)

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH] uml: flush stdout before forking

[Richard W. <ric...@gm...>]

On Fri, Dec 18, 2015 at 9:28 PM, Vegard Nossum <veg...@or...> wrote:
> I was seeing some really weird behaviour where piping UML's output
> somewhere would cause output to get duplicated:
>
>   $ ./vmlinux | head -n 40
>   Checking that ptrace can change system call numbers...Core dump limits :
>           soft - 0
>           hard - NONE
>   OK
>   Checking syscall emulation patch for ptrace...Core dump limits :
>           soft - 0
>           hard - NONE
>   OK
>   Checking advanced syscall emulation patch for ptrace...Core dump limits :
>           soft - 0
>           hard - NONE
>   OK
>   Core dump limits :
>           soft - 0
>           hard - NONE
>
> This is because these tests do a fork() which duplicates the non-empty
> stdout buffer, then glibc flushes the duplicated buffer as each child
> exits.
>
> A simple workaround is to flush before forking.
>
> Signed-off-by: Vegard Nossum <veg...@or...>

Applied and queued for stable!

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH] um: Fix get_signal() usage

[Richard W. <ri...@no...>]

Am 09.01.2016 um 04:51 schrieb Al Viro:
> On Wed, Nov 18, 2015 at 09:51:43AM +0100, Richard Weinberger wrote:
>> If get_signal() returns us a signal to post
>> we must not call it again, otherwise the already
>> posted signal will be overridden.
>> Before commit a610d6e672d this was the case as we stopped
>> the while after a successful handle_signal().
> 
> Old behaviour had been wrong.  If you have several pending signals,
> more than one sigframe should be built, as if the second, etc. had
> been delivered right on the entry into the handler.
> 
> Stopping after the first one is obviously wrong - consider the case
> when attempt to deliver it has raised SIGSEGV.

You are right. Thanks for pointing this out.
Will revert.

Thanks,
//richard

Re: [uml-devel] [PATCH] um: Fix get_signal() usage

[Al V. <vi...@Ze...>]

On Sat, Jan 09, 2016 at 03:51:25AM +0000, Al Viro wrote:
> On Wed, Nov 18, 2015 at 09:51:43AM +0100, Richard Weinberger wrote:
> > If get_signal() returns us a signal to post
> > we must not call it again, otherwise the already
> > posted signal will be overridden.
> > Before commit a610d6e672d this was the case as we stopped
> > the while after a successful handle_signal().
> 
> Old behaviour had been wrong.  If you have several pending signals,
> more than one sigframe should be built, as if the second, etc. had
> been delivered right on the entry into the handler.
> 
> Stopping after the first one is obviously wrong - consider the case
> when attempt to deliver it has raised SIGSEGV.

Note that subsequent signals do *not* override the register values set by
the first one - copy_sc_to_user() will store them in the corresponding
sigframes, so that after you reach the end of the handler for your second
signal {rt_,}sigreturn() will land you in the beginning of the first one.

Check how native x86 behaves - set a couple of handlers, block them with
sigprocmask(), raise both signals and unblock them.  Then have the
handlers examine and dump their struct ucontext (pointed to by the third
argument of handler).  You'll see that the first one to be executed will
have uc->uc_mcontext.ip pointing to the other handler and uc->uc_mcontext.ax
containing the other signal number.

Re: [uml-devel] [PATCH] um: Fix get_signal() usage

[Al V. <vi...@Ze...>]

On Wed, Nov 18, 2015 at 09:51:43AM +0100, Richard Weinberger wrote:
> If get_signal() returns us a signal to post
> we must not call it again, otherwise the already
> posted signal will be overridden.
> Before commit a610d6e672d this was the case as we stopped
> the while after a successful handle_signal().

Old behaviour had been wrong.  If you have several pending signals,
more than one sigframe should be built, as if the second, etc. had
been delivered right on the entry into the handler.

Stopping after the first one is obviously wrong - consider the case
when attempt to deliver it has raised SIGSEGV.

Re: [uml-devel] [PATCH] [RFC] um: define and use HAVE_LIBPCAP/HAVE_LIBVDEPLUG

[Vegard N. <veg...@or...>]

On 01/02/2016 12:43 PM, Richard Weinberger wrote:
> Am 02.01.2016 um 03:31 schrieb Vegard Nossum:
>> If you don't have libpcap or libvdeplug installed, you will get build
>> failures when compiling certain files:
>>
>> arch/um/drivers/vde_user.c:8:24: fatal error: libvdeplug.h: No such file or directory
>>   #include <libvdeplug.h>
>>
>> arch/um/drivers/pcap_user.c:7:18: fatal error: pcap.h: No such file or directory
>>   #include <pcap.h>
>>
>> This patch adds a basic pre-build check and defines the kconfig variables
>> HAVE_LIBPCAP and HAVE_LIBVDEPLUG depending on the result.
>>
>> There is a basic disadvantage to this scheme, namely that the user may
>> never see the options that rely on these libraries if they are not
>> installed. As a trade-off, we add a brand new option, MISSING_LIBRARIES
>> (defaulting to 'y'), which allows those options to be visible (and
>> selectable) anyway.
>>
>> [Note: I find this useful personally as I ran into the above build
>>   failures when playing around with UML -- so take this more as a
>>   suggestion on how things MAY be done better than a real patch.]
>
> Why can't you build with UML_NET_VDE=n and UML_NET_PCAP=n
> or install the missing libs?
> To me the patch reads like a lazy approach to make allyesconfig somehow build. :-)
>
> If one selects VDE or PCAP (either manually or via allyesconfig) and the libs are
> missing the build has to fail. Everything else will introduce nasty side effects
> like having different builds with the same config and packagers will cry.

Yeah, fair enough.


Vegard

Re: [uml-devel] [PATCH] [RFC] um: define and use HAVE_LIBPCAP/HAVE_LIBVDEPLUG

[Richard W. <ri...@no...>]

Am 02.01.2016 um 03:31 schrieb Vegard Nossum:
> If you don't have libpcap or libvdeplug installed, you will get build
> failures when compiling certain files:
> 
> arch/um/drivers/vde_user.c:8:24: fatal error: libvdeplug.h: No such file or directory
>  #include <libvdeplug.h>
> 
> arch/um/drivers/pcap_user.c:7:18: fatal error: pcap.h: No such file or directory
>  #include <pcap.h>
> 
> This patch adds a basic pre-build check and defines the kconfig variables
> HAVE_LIBPCAP and HAVE_LIBVDEPLUG depending on the result.
> 
> There is a basic disadvantage to this scheme, namely that the user may
> never see the options that rely on these libraries if they are not
> installed. As a trade-off, we add a brand new option, MISSING_LIBRARIES
> (defaulting to 'y'), which allows those options to be visible (and
> selectable) anyway.
> 
> [Note: I find this useful personally as I ran into the above build
>  failures when playing around with UML -- so take this more as a
>  suggestion on how things MAY be done better than a real patch.]

Why can't you build with UML_NET_VDE=n and UML_NET_PCAP=n
or install the missing libs?
To me the patch reads like a lazy approach to make allyesconfig somehow build. :-)

If one selects VDE or PCAP (either manually or via allyesconfig) and the libs are
missing the build has to fail. Everything else will introduce nasty side effects
like having different builds with the same config and packagers will cry.

Thanks,
//richard

Re: [uml-devel] [PATCH] [RFC] um: define and use HAVE_LIBPCAP/HAVE_LIBVDEPLUG

[Thomas M. <th...@m3...>]

Hi Vegard,

I like this idea. 

With kind regards
Thomas

> Am 02.01.2016 um 03:31 schrieb Vegard Nossum <veg...@or...>:
> 
> If you don't have libpcap or libvdeplug installed, you will get build
> failures when compiling certain files:
> 
> arch/um/drivers/vde_user.c:8:24: fatal error: libvdeplug.h: No such file or directory
> #include <libvdeplug.h>
> 
> arch/um/drivers/pcap_user.c:7:18: fatal error: pcap.h: No such file or directory
> #include <pcap.h>
> 
> This patch adds a basic pre-build check and defines the kconfig variables
> HAVE_LIBPCAP and HAVE_LIBVDEPLUG depending on the result.
> 
> There is a basic disadvantage to this scheme, namely that the user may
> never see the options that rely on these libraries if they are not
> installed. As a trade-off, we add a brand new option, MISSING_LIBRARIES
> (defaulting to 'y'), which allows those options to be visible (and
> selectable) anyway.
> 
> [Note: I find this useful personally as I ran into the above build
> failures when playing around with UML -- so take this more as a
> suggestion on how things MAY be done better than a real patch.]
> 
> Signed-off-by: Vegard Nossum <veg...@or...>
> ---
> arch/um/Kconfig.common | 18 ++++++++++++++++++
> arch/um/Kconfig.net    |  2 ++
> arch/um/Kconfig.um     | 13 +++++++++++++
> arch/um/Makefile       |  5 +++++
> 4 files changed, 38 insertions(+)
> 
> diff --git a/arch/um/Kconfig.common b/arch/um/Kconfig.common
> index d195a87..35ce3a1 100644
> --- a/arch/um/Kconfig.common
> +++ b/arch/um/Kconfig.common
> @@ -59,3 +59,21 @@ config HZ
> config SUBARCH
>    string
>    option env="SUBARCH"
> +
> +# Host libraries; these are defined by arch/um/Makefile
> +
> +config ENV_HAVE_LIBPCAP
> +    string
> +    option env="ENV_HAVE_LIBPCAP"
> +
> +config HAVE_LIBPCAP
> +    bool
> +    default y if ENV_HAVE_LIBPCAP="1"
> +
> +config ENV_HAVE_LIBVDEPLUG
> +    string
> +    option env="ENV_HAVE_LIBVDEPLUG"
> +
> +config HAVE_LIBVDEPLUG
> +    bool
> +    default y if ENV_HAVE_LIBVDEPLUG="1"
> diff --git a/arch/um/Kconfig.net b/arch/um/Kconfig.net
> index 820a56f..dfa7849 100644
> --- a/arch/um/Kconfig.net
> +++ b/arch/um/Kconfig.net
> @@ -111,6 +111,7 @@ config UML_NET_DAEMON
> config UML_NET_VDE
>    bool "VDE transport"
>    depends on UML_NET
> +    depends on HAVE_LIBVDEPLUG || MISSING_LIBRARIES
>    help
>    This User-Mode Linux network transport allows one or more running
>    UMLs on a single host to communicate with each other and also
> @@ -158,6 +159,7 @@ config UML_NET_MCAST
> config UML_NET_PCAP
>    bool "pcap transport"
>    depends on UML_NET
> +    depends on HAVE_LIBPCAP || MISSING_LIBRARIES
>    help
>    The pcap transport makes a pcap packet stream on the host look
>    like an ethernet device inside UML.  This is useful for making
> diff --git a/arch/um/Kconfig.um b/arch/um/Kconfig.um
> index 28a9885..15944d9 100644
> --- a/arch/um/Kconfig.um
> +++ b/arch/um/Kconfig.um
> @@ -9,6 +9,19 @@ config STATIC_LINK
>      Additionally, this option enables using higher memory spaces (up to
>      2.75G) for UML.
> 
> +config MISSING_LIBRARIES
> +    bool "Prompt for options which require missing host libraries"
> +    default y
> +    help
> +      Certain options rely on host libraries to compile.
> +
> +      If you say N here, then these options will only be available if
> +      the libraries they require have been detected to be present.
> +
> +      If you say Y, then the options will be available anyway, but
> +      beware that attempting to build the kernel will most likely
> +      result in an error.
> +
> source "mm/Kconfig"
> 
> config LD_SCRIPT_STATIC
> diff --git a/arch/um/Makefile b/arch/um/Makefile
> index e3abe6f..a8320e6 100644
> --- a/arch/um/Makefile
> +++ b/arch/um/Makefile
> @@ -166,3 +166,8 @@ include/generated/user_constants.h: $(HOST_DIR)/um/user-offsets.s
>    $(call filechk,gen-asm-offsets)
> 
> export SUBARCH USER_CFLAGS CFLAGS_NO_HARDENING OS DEV_NULL_PATH
> +
> +# Host libraries
> +has_header = $(shell echo | $(CC) -include $(1) -xc -c - >/dev/null 2>&1 && echo 1)
> +export ENV_HAVE_LIBPCAP := $(call has_header,pcap.h)
> +export ENV_HAVE_LIBVDEPLUG := $(call has_header,libvdeplug.h)
> -- 
> 1.9.1
> 
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> User-mode-linux-devel mailing list
> Use...@li...
> https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] [PATCH] [RFC] um: define and use HAVE_LIBPCAP/HAVE_LIBVDEPLUG

[Vegard N. <veg...@or...>]

If you don't have libpcap or libvdeplug installed, you will get build
failures when compiling certain files:

arch/um/drivers/vde_user.c:8:24: fatal error: libvdeplug.h: No such file or directory
 #include <libvdeplug.h>

arch/um/drivers/pcap_user.c:7:18: fatal error: pcap.h: No such file or directory
 #include <pcap.h>

This patch adds a basic pre-build check and defines the kconfig variables
HAVE_LIBPCAP and HAVE_LIBVDEPLUG depending on the result.

There is a basic disadvantage to this scheme, namely that the user may
never see the options that rely on these libraries if they are not
installed. As a trade-off, we add a brand new option, MISSING_LIBRARIES
(defaulting to 'y'), which allows those options to be visible (and
selectable) anyway.

[Note: I find this useful personally as I ran into the above build
 failures when playing around with UML -- so take this more as a
 suggestion on how things MAY be done better than a real patch.]

Signed-off-by: Vegard Nossum <veg...@or...>
---
 arch/um/Kconfig.common | 18 ++++++++++++++++++
 arch/um/Kconfig.net    |  2 ++
 arch/um/Kconfig.um     | 13 +++++++++++++
 arch/um/Makefile       |  5 +++++
 4 files changed, 38 insertions(+)

diff --git a/arch/um/Kconfig.common b/arch/um/Kconfig.common
index d195a87..35ce3a1 100644
--- a/arch/um/Kconfig.common
+++ b/arch/um/Kconfig.common
@@ -59,3 +59,21 @@ config HZ
 config SUBARCH
 	string
 	option env="SUBARCH"
+
+# Host libraries; these are defined by arch/um/Makefile
+
+config ENV_HAVE_LIBPCAP
+	string
+	option env="ENV_HAVE_LIBPCAP"
+
+config HAVE_LIBPCAP
+	bool
+	default y if ENV_HAVE_LIBPCAP="1"
+
+config ENV_HAVE_LIBVDEPLUG
+	string
+	option env="ENV_HAVE_LIBVDEPLUG"
+
+config HAVE_LIBVDEPLUG
+	bool
+	default y if ENV_HAVE_LIBVDEPLUG="1"
diff --git a/arch/um/Kconfig.net b/arch/um/Kconfig.net
index 820a56f..dfa7849 100644
--- a/arch/um/Kconfig.net
+++ b/arch/um/Kconfig.net
@@ -111,6 +111,7 @@ config UML_NET_DAEMON
 config UML_NET_VDE
 	bool "VDE transport"
 	depends on UML_NET
+	depends on HAVE_LIBVDEPLUG || MISSING_LIBRARIES
 	help
 	This User-Mode Linux network transport allows one or more running
 	UMLs on a single host to communicate with each other and also
@@ -158,6 +159,7 @@ config UML_NET_MCAST
 config UML_NET_PCAP
 	bool "pcap transport"
 	depends on UML_NET
+	depends on HAVE_LIBPCAP || MISSING_LIBRARIES
 	help
 	The pcap transport makes a pcap packet stream on the host look
 	like an ethernet device inside UML.  This is useful for making
diff --git a/arch/um/Kconfig.um b/arch/um/Kconfig.um
index 28a9885..15944d9 100644
--- a/arch/um/Kconfig.um
+++ b/arch/um/Kconfig.um
@@ -9,6 +9,19 @@ config STATIC_LINK
 	  Additionally, this option enables using higher memory spaces (up to
 	  2.75G) for UML.
 
+config MISSING_LIBRARIES
+	bool "Prompt for options which require missing host libraries"
+	default y
+	help
+	  Certain options rely on host libraries to compile.
+
+	  If you say N here, then these options will only be available if
+	  the libraries they require have been detected to be present.
+
+	  If you say Y, then the options will be available anyway, but
+	  beware that attempting to build the kernel will most likely
+	  result in an error.
+
 source "mm/Kconfig"
 
 config LD_SCRIPT_STATIC
diff --git a/arch/um/Makefile b/arch/um/Makefile
index e3abe6f..a8320e6 100644
--- a/arch/um/Makefile
+++ b/arch/um/Makefile
@@ -166,3 +166,8 @@ include/generated/user_constants.h: $(HOST_DIR)/um/user-offsets.s
 	$(call filechk,gen-asm-offsets)
 
 export SUBARCH USER_CFLAGS CFLAGS_NO_HARDENING OS DEV_NULL_PATH
+
+# Host libraries
+has_header = $(shell echo | $(CC) -include $(1) -xc -c - >/dev/null 2>&1 && echo 1)
+export ENV_HAVE_LIBPCAP := $(call has_header,pcap.h)
+export ENV_HAVE_LIBVDEPLUG := $(call has_header,libvdeplug.h)
-- 
1.9.1

Re: [uml-devel] [PATCH] um: link with -lpthread

[Thomas M. <th...@m3...>]

Am 31.12.2015 5:06 nachm. schrieb Vegard Nossum <veg...@or...>:
>
> Similarly to commit fb1770aa78a43530940d0c2dd161e77bc705bdac, with gcc 5 
> on Ubuntu and CONFIG_STATIC_LINK=y I was seeing these linker errors: 

Hi,

Oops, yes this patch looks good to me. Honestly I did never test the static link case. Sorry.

With kind regards
Thomas

>
> /usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new': 
> (.text+0xcd): undefined reference to `pthread_once' 
> /usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new': 
> (.text+0x126): undefined reference to `pthread_attr_init' 
> /usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new': 
> (.text+0x168): undefined reference to `pthread_attr_setdetachstate' 
> [...] 
>
> Obviously we also need -lpthread for librt.a. 
>
> Signed-off-by: Vegard Nossum <veg...@or...> 
> --- 
> scripts/link-vmlinux.sh | 2 +- 
> 1 file changed, 1 insertion(+), 1 deletion(-) 
>
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh 
> index dacf71a..ba6c34e 100755 
> --- a/scripts/link-vmlinux.sh 
> +++ b/scripts/link-vmlinux.sh 
> @@ -62,7 +62,7 @@ vmlinux_link() 
> -Wl,--start-group                                    \ 
> ${KBUILD_VMLINUX_MAIN}                      \ 
> -Wl,--end-group                                      \ 
> - -lutil -lrt ${1} 
> + -lutil -lrt -lpthread ${1} 
> rm -f linux 
> fi 
> } 
> -- 
> 1.9.1 
>
>
> ------------------------------------------------------------------------------ 
> _______________________________________________ 
> User-mode-linux-devel mailing list 
> Use...@li... 
> https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel 

Re: [uml-devel] [PATCH] um: link with -lpthread

[Richard W. <ri...@no...>]

Am 31.12.2015 um 17:21 schrieb Thomas Meyer:
> Am 31.12.2015 5:06 nachm. schrieb Vegard Nossum <veg...@or...>:
>>
>> Similarly to commit fb1770aa78a43530940d0c2dd161e77bc705bdac, with gcc 5 
>> on Ubuntu and CONFIG_STATIC_LINK=y I was seeing these linker errors: 
> 
> Hi,
> 
> Oops, yes this patch looks good to me. Honestly I did never test the static link case. Sorry.

Thanks for fixing this.
I'm back from being MIA and start collecting patches soon.

Thanks,
//richard

[uml-devel] [PATCH] um: link with -lpthread

[Vegard N. <veg...@or...>]

Similarly to commit fb1770aa78a43530940d0c2dd161e77bc705bdac, with gcc 5
on Ubuntu and CONFIG_STATIC_LINK=y I was seeing these linker errors:

/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new':
(.text+0xcd): undefined reference to `pthread_once'
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new':
(.text+0x126): undefined reference to `pthread_attr_init'
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `__timer_create_new':
(.text+0x168): undefined reference to `pthread_attr_setdetachstate'
[...]

Obviously we also need -lpthread for librt.a.

Signed-off-by: Vegard Nossum <veg...@or...>
---
 scripts/link-vmlinux.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index dacf71a..ba6c34e 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -62,7 +62,7 @@ vmlinux_link()
 			-Wl,--start-group                                    \
 				 ${KBUILD_VMLINUX_MAIN}                      \
 			-Wl,--end-group                                      \
-			-lutil -lrt ${1}
+			-lutil -lrt -lpthread ${1}
 		rm -f linux
 	fi
 }
-- 
1.9.1

Re: [uml-devel] [PATCH v2] um: Fix pointer cast

[Richard W. <ri...@no...>]

Am 23.12.2015 um 14:17 schrieb Mickaël Salaün:
> Fix a pointer cast typo introduced in v4.4-rc5 especially visible for
> the i386 subarchitecture where it results in a kernel crash.
> 
> Fixes: 8090bfd2bb9a ("um: Fix fpstate handling")
> Signed-off-by: Mickaël Salaün <mi...@di...>
> Cc: Jeff Dike <jd...@ad...>
> Cc: Richard Weinberger <ri...@no...>
> Cc: Linus Torvalds <tor...@li...>
> Cc: Al Viro <vi...@Ze...>
> ---
>  arch/x86/um/signal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/um/signal.c b/arch/x86/um/signal.c
> index e5f854ce2d72..14fcd01ed992 100644
> --- a/arch/x86/um/signal.c
> +++ b/arch/x86/um/signal.c
> @@ -470,7 +470,7 @@ long sys_sigreturn(void)
>  	struct sigcontext __user *sc = &frame->sc;
>  	int sig_size = (_NSIG_WORDS - 1) * sizeof(unsigned long);
>  
> -	if (copy_from_user(&set.sig[0], (void *)sc->oldmask, sizeof(set.sig[0])) ||
> +	if (copy_from_user(&set.sig[0], &sc->oldmask, sizeof(set.sig[0])) ||
>  	    copy_from_user(&set.sig[1], frame->extramask, sig_size))
>  		goto segfault;

FYI Linus already merged your patch. :-)
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=de3793796f78e293cc0873744a75588c99ed2fdd

Thanks,
//richard

Re: [uml-devel] [PATCH] um: Fix pointer cast

[Al V. <vi...@Ze...>]

On Tue, Dec 22, 2015 at 09:44:01PM +0100, Mickaël Salaün wrote:
> Fix a pointer cast typo introduced in v4.4-rc5 especially visible for
> the i386 subarchitecture where it results in a kernel crash.

Why the hell bother casting it at all?  _Any_ pointer will quietly convert
to void *, no typecasts needed.  The second argument of copy_from_user
is const void __user *; sc is struct sigcontext __user *sc, so
&sb->oldmask is either __u32 __user * or __u64 __user *, for 32bit and
64bit builds resp.  Either is assignment-compatible with
const void __user *.

Basically, cast is telling the typechecking logics "sod off, I know better".
And here it's not needed at all.  Moreover, the bug you are fixing here is
precisely that this code did *not* know better - if not for that cast,
compiler would've immediately pointed to the problem.

> -	if (copy_from_user(&set.sig[0], (void *)sc->oldmask, sizeof(set.sig[0])) ||
> +	if (copy_from_user(&set.sig[0], (void *)&sc->oldmask, sizeof(set.sig[0])) ||

Please, remove the cast completely.  Simply pass it &sc->oldmask and be
done with that.

Re: [uml-devel] [PATCH] um: Fix build error and kconfig for i386

[Richard W. <ri...@no...>]

Am 22.12.2015 um 22:44 schrieb Mickaël Salaün:
> Fix build error by selecting COREDUMP when X86_32 is selected:
> 
> arch/x86/um/built-in.o: In function `elf_core_write_extra_phdrs':
> (.text+0x3e62): undefined reference to `dump_emit'
> arch/x86/um/built-in.o: In function `elf_core_write_extra_data':
> (.text+0x3eef): undefined reference to `dump_emit'
> 
> Signed-off-by: Mickaël Salaün <mi...@di...>
> Cc: Jeff Dike <jd...@ad...>
> Cc: Richard Weinberger <ri...@no...>
> ---
>  arch/x86/um/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/um/Kconfig b/arch/x86/um/Kconfig
> index ed56a1c4ae73..bb7cd8b38043 100644
> --- a/arch/x86/um/Kconfig
> +++ b/arch/x86/um/Kconfig
> @@ -26,6 +26,7 @@ config X86_32
>  	select CLONE_BACKWARDS
>  	select OLD_SIGSUSPEND3
>  	select OLD_SIGACTION
> +	select COREDUMP


Do you know since when this is broken?

Thanks,
//richard

Re: [uml-devel] [PATCH] um: Fix pointer cast

[Richard W. <ri...@no...>]

Am 22.12.2015 um 21:44 schrieb Mickaël Salaün:
> Fix a pointer cast typo introduced in v4.4-rc5 especially visible for
> the i386 subarchitecture where it results in a kernel crash.
> 
> Fixes: 8090bfd2bb9a ("um: Fix fpstate handling")
> 
> Signed-off-by: Mickaël Salaün <mi...@di...>
> Cc: Jeff Dike <jd...@ad...>
> Cc: Richard Weinberger <ri...@no...>
> Cc: Linus Torvalds <tor...@li...>
> ---
>  arch/x86/um/signal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/um/signal.c b/arch/x86/um/signal.c
> index e5f854ce2d72..cbb541b80358 100644
> --- a/arch/x86/um/signal.c
> +++ b/arch/x86/um/signal.c
> @@ -470,7 +470,7 @@ long sys_sigreturn(void)
>  	struct sigcontext __user *sc = &frame->sc;
>  	int sig_size = (_NSIG_WORDS - 1) * sizeof(unsigned long);
>  
> -	if (copy_from_user(&set.sig[0], (void *)sc->oldmask, sizeof(set.sig[0])) ||
> +	if (copy_from_user(&set.sig[0], (void *)&sc->oldmask, sizeof(set.sig[0])) ||
>  	    copy_from_user(&set.sig[1], frame->extramask, sig_size))
>  		goto segfault;

o_O, thanks for catching this!

Thanks,
//richard

Re: [uml-devel] [PATCH 2/2] Bulk IO Transaction support part 1

[Anton I. <ant...@ko...>]

Hi list, hi Richard,

This rather primitive patchset pushes disk IO by ~ 15%.

dd to /dev/null on a host memory cached 16G sparse disk image with 
ubuntu on it goes from 512MB/s on my machine to 580MB/s.

Part 2 will be ring buffers and read/write poll loop in the io_thread as 
well as elimination of "partial read/write" workarounds. While it does 
not push disk benchmarks a lot it makes UML more responsive.

Part 3 will be merging adjacent IO transactions (by file location) via 
vector IO. This pushes trhoughput even further by quite a bit.

While I can try to submit all of that in one go, I'd rather submit it in 
chunks so it is easier to review even if this will result in patch churn 
(the whole do_safe_read malarkey will bite the bullet once part 2 is out).

A.

On 21/12/15 18:54, Anton Ivanov wrote:
> This patch adds support for merging notifications on the ubd
> notification file descriptor. Multiple transactions are processed
> at a time resulting in 10-15% virtual disk speed improvement.
>
> The mechanics are rather primitive - no ring buffers, primitive
> guaranteed flush and guaranteed to-record-size read.
>
> Signed-off-by: Anton Ivanov <ai...@br...>
> ---
>   arch/um/drivers/ubd.h       |   2 +
>   arch/um/drivers/ubd_kern.c  | 156 ++++++++++++++++++++++++++++++++++++--------
>   arch/um/drivers/ubd_user.c  |  19 +++++-
>   arch/um/include/shared/os.h |   1 +
>   arch/um/os-Linux/file.c     |  12 ++++
>   5 files changed, 161 insertions(+), 29 deletions(-)
>
> diff --git a/arch/um/drivers/ubd.h b/arch/um/drivers/ubd.h
> index 3b48cd2..2c5e6fd 100644
> --- a/arch/um/drivers/ubd.h
> +++ b/arch/um/drivers/ubd.h
> @@ -10,6 +10,8 @@
>   extern int start_io_thread(unsigned long sp, int *fds_out);
>   extern int io_thread(void *arg);
>   extern int kernel_fd;
> +extern void setup_ubd_pollfd(void * fds, int fd);
> +extern int size_of_pollfd(void);
>   
>   #endif
>   
> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
> index 39ba207..9f30c50 100644
> --- a/arch/um/drivers/ubd_kern.c
> +++ b/arch/um/drivers/ubd_kern.c
> @@ -58,6 +58,12 @@ struct io_thread_req {
>   	int error;
>   };
>   
> +static void * kernel_pfd = NULL;
> +
> +struct io_thread_req **kernel_reqs;
> +
> +struct io_thread_req **helper_reqs;
> +
>   static inline int ubd_test_bit(__u64 bit, unsigned char *data)
>   {
>   	__u64 n;
> @@ -442,6 +448,32 @@ static void do_ubd_request(struct request_queue * q);
>   static int thread_fd = -1;
>   static LIST_HEAD(restart);
>   
> +
> +static int do_safe_read(int fd, void * buffer, int max_size, int record_size){
> +	unsigned char * buf;
> +	int n, size;
> +
> +	size = 0;
> +	buf = (unsigned char *) buffer;
> +
> +	return os_read_file(fd, buf + size, max_size - size);
> +
> +	do {
> +		n = os_read_file(fd, buf, max_size - size);
> +		if ((size == 0) && (n == -EAGAIN))
> +			return n;
> +		if (n > 0)
> +			size = size + n;
> +		if ((n < 0) && (n != -EAGAIN))
> +			return n;
> +		if (size == max_size)
> +			break;
> +	} while ((size % record_size) != 0);
> +
> +	return size;
> +}
> +
> +
>   /* XXX - move this inside ubd_intr. */
>   /* Called without dev->lock held, and only in interrupt context. */
>   static void ubd_handler(void)
> @@ -450,21 +482,37 @@ static void ubd_handler(void)
>   	struct ubd *ubd;
>   	struct list_head *list, *next_ele;
>   	unsigned long flags;
> -	int n;
> +	int n, rcount, i;
>   
>   	while(1){
> -		n = os_read_file(thread_fd, &req,
> -				 sizeof(struct io_thread_req *));
> -		if(n != sizeof(req)){
> -			if(n == -EAGAIN)
> -				break;
> -			printk(KERN_ERR "spurious interrupt in ubd_handler, "
> -			       "err = %d\n", -n);
> -			return;
> +		n = do_safe_read(thread_fd, helper_reqs,
> +				 sizeof(struct io_thread_req *) * MAX_SG,
> +				 sizeof(struct io_thread_req *)
> +				 );
> +
> +		if(n == -EAGAIN){
> +			break;
> +		}
> +		if(n < 0){
> +			printk("io_thread - read failed, fd = %d, "
> +			       "err = %d\n", thread_fd, -n);
> +		}
> +
> +		if(n % sizeof(struct io_thread_req *) != 0){
> +			printk("kernel_ubd_io - invalid read, fd = %d, "
> +			       "read = %d\n", thread_fd, n);
> +			continue;
>   		}
>   
> -		blk_end_request(req->req, 0, req->length);
> -		kfree(req);
> +		rcount = n / sizeof(struct io_thread_req *);
> +		
> +		for (i = 0; i < rcount; i++) {
> +
> +			req = * (helper_reqs + i);
> +			blk_end_request(req->req, 0, req->length);
> +			kfree(req);
> +
> +		}
>   	}
>   	reactivate_fd(thread_fd, UBD_IRQ);
>   
> @@ -1080,6 +1128,26 @@ late_initcall(ubd_init);
>   static int __init ubd_driver_init(void){
>   	unsigned long stack;
>   	int err;
> +	
> +	kernel_reqs = kmalloc(MAX_SG * sizeof(struct io_thread_req *), GFP_KERNEL);
> +	if (kernel_reqs == NULL) {
> +		printk("Failed to allocate memory for req buffer\n");
> +		return 0;
> +	}
> +
> +
> +	kernel_pfd = kmalloc(size_of_pollfd(), GFP_KERNEL);
> +	if (kernel_pfd == NULL) {
> +		printk("Failed to allocate memory for pollfd\n");
> +		return 0;
> +	}
> +
> +	helper_reqs = kmalloc(MAX_SG * sizeof(struct io_thread_req *), GFP_KERNEL);
> +	if (helper_reqs == NULL) {
> +		printk("Failed to allocate memory for req buffer\n");
> +		return 0;
> +	}
> +
>   
>   	/* Set by CONFIG_BLK_DEV_UBD_SYNC or ubd=sync.*/
>   	if(global_openflags.s){
> @@ -1458,30 +1526,62 @@ static int io_count = 0;
>   int io_thread(void *arg)
>   {
>   	struct io_thread_req *req;
> -	int n;
> +	unsigned char * buffer;
> +	int n, rcount, i;
>   
>   	os_fix_helper_signals();
>   
> +	printk("Starting UBD helper thread\n");
> +
>   	while(1){
> -		n = os_read_file(kernel_fd, &req,
> -				 sizeof(struct io_thread_req *));
> -		if(n != sizeof(struct io_thread_req *)){
> -			if(n < 0)
> -				printk("io_thread - read failed, fd = %d, "
> -				       "err = %d\n", kernel_fd, -n);
> -			else {
> -				printk("io_thread - short read, fd = %d, "
> -				       "length = %d\n", kernel_fd, n);
> -			}
> +		setup_ubd_pollfd(kernel_pfd, kernel_fd);
> +		os_poll(kernel_pfd, 1, -1);
> +		n = do_safe_read(kernel_fd, kernel_reqs,
> +				 sizeof(struct io_thread_req *) * MAX_SG,
> +				 sizeof(struct io_thread_req *) * MAX_SG
> +				 );
> +		if(n == -EAGAIN){
>   			continue;
>   		}
> -		io_count++;
> -		do_io(req);
> -		n = os_write_file(kernel_fd, &req,
> -				  sizeof(struct io_thread_req *));
> -		if(n != sizeof(struct io_thread_req *))
> +		if(n < 0){
> +			printk("io_thread - read failed, fd = %d, "
> +			       "err = %d\n", kernel_fd, -n);
> +		}
> +
> +		if(n % sizeof(struct io_thread_req *) != 0){
> +			printk("io_thread - invalid read, fd = %d, "
> +			       "read = %d\n", kernel_fd, n);
> +			continue;
> +		}
> +
> +		rcount = n / sizeof(struct io_thread_req *);
> +		
> +		for (i = 0; i < rcount; i++) {
> +
> +			io_count++;
> +
> +			req = * (kernel_reqs + i);
> +
> +			do_io(req);
> +
> +		}
> +
> +		buffer = (unsigned char *) kernel_reqs;
> +
> +		while (n > 0) {
> +			i = os_write_file(kernel_fd, buffer, n);
> +			if(i >= 0){
> +				buffer = buffer + i;
> +				n = n - i;
> +			} else {
> +				if(i != -EAGAIN)
> +					break;
> +			}
> +		}
> +
> +		if(n > 0)
>   			printk("io_thread - write failed, fd = %d, err = %d\n",
> -			       kernel_fd, -n);
> +			       kernel_fd, -i);
>   	}
>   
>   	return 0;
> diff --git a/arch/um/drivers/ubd_user.c b/arch/um/drivers/ubd_user.c
> index e376f9b..3abec4f 100644
> --- a/arch/um/drivers/ubd_user.c
> +++ b/arch/um/drivers/ubd_user.c
> @@ -17,10 +17,22 @@
>   #include <sys/param.h>
>   #include <endian.h>
>   #include <byteswap.h>
> +#include <poll.h>
>   
>   #include "ubd.h"
>   #include <os.h>
>   
> +void setup_ubd_pollfd(void * fds, int fd) {
> +	struct pollfd * pfds = (struct pollfd *) fds;
> +	pfds->fd = fd;
> +	pfds->events = POLLIN | POLLPRI;
> +	pfds->revents = 0;
> +}
> +
> +int size_of_pollfd(void) {
> +	return sizeof(struct pollfd);
> +}
> +
>   int start_io_thread(unsigned long sp, int *fd_out)
>   {
>   	int pid, fds[2], err;
> @@ -36,10 +48,15 @@ int start_io_thread(unsigned long sp, int *fd_out)
>   
>   	err = os_set_fd_block(*fd_out, 0);
>   	if (err) {
> -		printk("start_io_thread - failed to set nonblocking I/O.\n");
> +		printk("start_io_thread - failed to set nonblocking I/O - kernel_fd.\n");
>   		goto out_close;
>   	}
>   
> +	err = os_set_fd_block(kernel_fd, 0);
> +	if (err) {
> +		printk("start_io_thread - failed to set nonblocking I/O - user_fd.\n");
> +		goto out_close;
> +	}
>   	pid = clone(io_thread, (void *) sp, CLONE_FILES | CLONE_VM, NULL);
>   	if(pid < 0){
>   		err = -errno;
> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
> index 7a04ddd..0b75efa 100644
> --- a/arch/um/include/shared/os.h
> +++ b/arch/um/include/shared/os.h
> @@ -149,6 +149,7 @@ extern int os_file_size(const char *file, unsigned long long *size_out);
>   extern int os_pread_file(int fd, void *buf, int len, unsigned long long offset);
>   extern int os_pwrite_file(int fd, const void *buf, int count, unsigned long long offset);
>   extern int os_file_modtime(const char *file, unsigned long *modtime);
> +extern int os_poll(void *fds, unsigned int nfds, int timeout);
>   extern int os_pipe(int *fd, int stream, int close_on_exec);
>   extern int os_set_fd_async(int fd);
>   extern int os_clear_fd_async(int fd);
> diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c
> index 2db18cb..726b1e1 100644
> --- a/arch/um/os-Linux/file.c
> +++ b/arch/um/os-Linux/file.c
> @@ -14,6 +14,7 @@
>   #include <sys/stat.h>
>   #include <sys/un.h>
>   #include <sys/types.h>
> +#include <poll.h>
>   #include <os.h>
>   
>   static void copy_stat(struct uml_stat *dst, const struct stat64 *src)
> @@ -355,6 +356,17 @@ int os_file_modtime(const char *file, unsigned long *modtime)
>   	return 0;
>   }
>   
> +int os_poll(void *fds, unsigned int nfds, int timeout)
> +{
> +	int n;
> +
> +	CATCH_EINTR(n = poll((struct pollfd *) fds, nfds, timeout));
> +	if (n < 0)
> +		return -errno;
> +
> +	return n;
> +}
> +
>   int os_set_exec_close(int fd)
>   {
>   	int err;

[uml-devel] [PATCH 1/2] Update UBD to use pread/pwrite family of functions

[Anton I. <ai...@br...>]

This decreases the number of syscalls per read/write by half.

Signed-off-by: Anton Ivanov <ai...@br...>
---
 arch/um/drivers/ubd_kern.c  | 27 +++++----------------------
 arch/um/include/shared/os.h |  2 ++
 arch/um/os-Linux/file.c     | 19 +++++++++++++++++++
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index e8ab93c..39ba207 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -535,11 +535,7 @@ static int read_cow_bitmap(int fd, void *buf, int offset, int len)
 {
 	int err;
 
-	err = os_seek_file(fd, offset);
-	if (err < 0)
-		return err;
-
-	err = os_read_file(fd, buf, len);
+	err = os_pread_file(fd, buf, len, offset);
 	if (err < 0)
 		return err;
 
@@ -1377,14 +1373,8 @@ static int update_bitmap(struct io_thread_req *req)
 	if(req->cow_offset == -1)
 		return 0;
 
-	n = os_seek_file(req->fds[1], req->cow_offset);
-	if(n < 0){
-		printk("do_io - bitmap lseek failed : err = %d\n", -n);
-		return 1;
-	}
-
-	n = os_write_file(req->fds[1], &req->bitmap_words,
-			  sizeof(req->bitmap_words));
+	n = os_pwrite_file(req->fds[1], &req->bitmap_words,
+			  sizeof(req->bitmap_words), req->cow_offset);
 	if(n != sizeof(req->bitmap_words)){
 		printk("do_io - bitmap update failed, err = %d fd = %d\n", -n,
 		       req->fds[1]);
@@ -1399,7 +1389,6 @@ static void do_io(struct io_thread_req *req)
 	char *buf;
 	unsigned long len;
 	int n, nsectors, start, end, bit;
-	int err;
 	__u64 off;
 
 	if (req->op == UBD_FLUSH) {
@@ -1428,18 +1417,12 @@ static void do_io(struct io_thread_req *req)
 		len = (end - start) * req->sectorsize;
 		buf = &req->buffer[start * req->sectorsize];
 
-		err = os_seek_file(req->fds[bit], off);
-		if(err < 0){
-			printk("do_io - lseek failed : err = %d\n", -err);
-			req->error = 1;
-			return;
-		}
 		if(req->op == UBD_READ){
 			n = 0;
 			do {
 				buf = &buf[n];
 				len -= n;
-				n = os_read_file(req->fds[bit], buf, len);
+				n = os_pread_file(req->fds[bit], buf, len, off);
 				if (n < 0) {
 					printk("do_io - read failed, err = %d "
 					       "fd = %d\n", -n, req->fds[bit]);
@@ -1449,7 +1432,7 @@ static void do_io(struct io_thread_req *req)
 			} while((n < len) && (n != 0));
 			if (n < len) memset(&buf[n], 0, len - n);
 		} else {
-			n = os_write_file(req->fds[bit], buf, len);
+			n = os_pwrite_file(req->fds[bit], buf, len, off);
 			if(n != len){
 				printk("do_io - write failed err = %d "
 				       "fd = %d\n", -n, req->fds[bit]);
diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index 868e6c3..7a04ddd 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -146,6 +146,8 @@ extern int os_read_file(int fd, void *buf, int len);
 extern int os_write_file(int fd, const void *buf, int count);
 extern int os_sync_file(int fd);
 extern int os_file_size(const char *file, unsigned long long *size_out);
+extern int os_pread_file(int fd, void *buf, int len, unsigned long long offset);
+extern int os_pwrite_file(int fd, const void *buf, int count, unsigned long long offset);
 extern int os_file_modtime(const char *file, unsigned long *modtime);
 extern int os_pipe(int *fd, int stream, int close_on_exec);
 extern int os_set_fd_async(int fd);
diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c
index 26e0164..2db18cb 100644
--- a/arch/um/os-Linux/file.c
+++ b/arch/um/os-Linux/file.c
@@ -264,6 +264,15 @@ int os_read_file(int fd, void *buf, int len)
 	return n;
 }
 
+int os_pread_file(int fd, void *buf, int len, unsigned long long offset)
+{
+	int n = pread(fd, buf, len, offset);
+
+	if (n < 0)
+		return -errno;
+	return n;
+}
+
 int os_write_file(int fd, const void *buf, int len)
 {
 	int n = write(fd, (void *) buf, len);
@@ -282,6 +291,16 @@ int os_sync_file(int fd)
 	return n;
 }
 
+int os_pwrite_file(int fd, const void *buf, int len, unsigned long long offset)
+{
+	int n = pwrite(fd, (void *) buf, len, offset);
+
+	if (n < 0)
+		return -errno;
+	return n;
+}
+
+
 int os_file_size(const char *file, unsigned long long *size_out)
 {
 	struct uml_stat buf;
-- 
2.1.4

[uml-devel] [PATCH 2/2] Bulk IO Transaction support part 1

[Anton I. <ai...@br...>]

This patch adds support for merging notifications on the ubd
notification file descriptor. Multiple transactions are processed
at a time resulting in 10-15% virtual disk speed improvement.

The mechanics are rather primitive - no ring buffers, primitive
guaranteed flush and guaranteed to-record-size read.

Signed-off-by: Anton Ivanov <ai...@br...>
---
 arch/um/drivers/ubd.h       |   2 +
 arch/um/drivers/ubd_kern.c  | 156 ++++++++++++++++++++++++++++++++++++--------
 arch/um/drivers/ubd_user.c  |  19 +++++-
 arch/um/include/shared/os.h |   1 +
 arch/um/os-Linux/file.c     |  12 ++++
 5 files changed, 161 insertions(+), 29 deletions(-)

diff --git a/arch/um/drivers/ubd.h b/arch/um/drivers/ubd.h
index 3b48cd2..2c5e6fd 100644
--- a/arch/um/drivers/ubd.h
+++ b/arch/um/drivers/ubd.h
@@ -10,6 +10,8 @@
 extern int start_io_thread(unsigned long sp, int *fds_out);
 extern int io_thread(void *arg);
 extern int kernel_fd;
+extern void setup_ubd_pollfd(void * fds, int fd);
+extern int size_of_pollfd(void);
 
 #endif
 
diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 39ba207..9f30c50 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -58,6 +58,12 @@ struct io_thread_req {
 	int error;
 };
 
+static void * kernel_pfd = NULL;
+
+struct io_thread_req **kernel_reqs;
+
+struct io_thread_req **helper_reqs;
+
 static inline int ubd_test_bit(__u64 bit, unsigned char *data)
 {
 	__u64 n;
@@ -442,6 +448,32 @@ static void do_ubd_request(struct request_queue * q);
 static int thread_fd = -1;
 static LIST_HEAD(restart);
 
+
+static int do_safe_read(int fd, void * buffer, int max_size, int record_size){
+	unsigned char * buf;
+	int n, size;
+
+	size = 0;
+	buf = (unsigned char *) buffer;
+
+	return os_read_file(fd, buf + size, max_size - size);
+
+	do {
+		n = os_read_file(fd, buf, max_size - size);
+		if ((size == 0) && (n == -EAGAIN))
+			return n;
+		if (n > 0)
+			size = size + n;
+		if ((n < 0) && (n != -EAGAIN))
+			return n;
+		if (size == max_size)
+			break;
+	} while ((size % record_size) != 0);
+
+	return size;
+}
+
+
 /* XXX - move this inside ubd_intr. */
 /* Called without dev->lock held, and only in interrupt context. */
 static void ubd_handler(void)
@@ -450,21 +482,37 @@ static void ubd_handler(void)
 	struct ubd *ubd;
 	struct list_head *list, *next_ele;
 	unsigned long flags;
-	int n;
+	int n, rcount, i;
 
 	while(1){
-		n = os_read_file(thread_fd, &req,
-				 sizeof(struct io_thread_req *));
-		if(n != sizeof(req)){
-			if(n == -EAGAIN)
-				break;
-			printk(KERN_ERR "spurious interrupt in ubd_handler, "
-			       "err = %d\n", -n);
-			return;
+		n = do_safe_read(thread_fd, helper_reqs,
+				 sizeof(struct io_thread_req *) * MAX_SG, 
+				 sizeof(struct io_thread_req *)
+				 );
+
+		if(n == -EAGAIN){
+			break;
+		}
+		if(n < 0){
+			printk("io_thread - read failed, fd = %d, "
+			       "err = %d\n", thread_fd, -n);
+		} 
+
+		if(n % sizeof(struct io_thread_req *) != 0){
+			printk("kernel_ubd_io - invalid read, fd = %d, "
+			       "read = %d\n", thread_fd, n);
+			continue;
 		}
 
-		blk_end_request(req->req, 0, req->length);
-		kfree(req);
+		rcount = n / sizeof(struct io_thread_req *);
+		
+		for (i = 0; i < rcount; i++) {
+
+			req = * (helper_reqs + i);
+			blk_end_request(req->req, 0, req->length);
+			kfree(req);
+
+		}
 	}
 	reactivate_fd(thread_fd, UBD_IRQ);
 
@@ -1080,6 +1128,26 @@ late_initcall(ubd_init);
 static int __init ubd_driver_init(void){
 	unsigned long stack;
 	int err;
+	
+	kernel_reqs = kmalloc(MAX_SG * sizeof(struct io_thread_req *), GFP_KERNEL);
+	if (kernel_reqs == NULL) {
+		printk("Failed to allocate memory for req buffer\n");
+		return 0;
+	}
+
+
+	kernel_pfd = kmalloc(size_of_pollfd(), GFP_KERNEL);
+	if (kernel_pfd == NULL) {
+		printk("Failed to allocate memory for pollfd\n");
+		return 0;
+	}
+
+	helper_reqs = kmalloc(MAX_SG * sizeof(struct io_thread_req *), GFP_KERNEL);
+	if (helper_reqs == NULL) {
+		printk("Failed to allocate memory for req buffer\n");
+		return 0;
+	}
+
 
 	/* Set by CONFIG_BLK_DEV_UBD_SYNC or ubd=sync.*/
 	if(global_openflags.s){
@@ -1458,30 +1526,62 @@ static int io_count = 0;
 int io_thread(void *arg)
 {
 	struct io_thread_req *req;
-	int n;
+	unsigned char * buffer;
+	int n, rcount, i;
 
 	os_fix_helper_signals();
 
+	printk("Starting UBD helper thread\n");
+
 	while(1){
-		n = os_read_file(kernel_fd, &req,
-				 sizeof(struct io_thread_req *));
-		if(n != sizeof(struct io_thread_req *)){
-			if(n < 0)
-				printk("io_thread - read failed, fd = %d, "
-				       "err = %d\n", kernel_fd, -n);
-			else {
-				printk("io_thread - short read, fd = %d, "
-				       "length = %d\n", kernel_fd, n);
-			}
+		setup_ubd_pollfd(kernel_pfd, kernel_fd);
+		os_poll(kernel_pfd, 1, -1);
+		n = do_safe_read(kernel_fd, kernel_reqs,
+				 sizeof(struct io_thread_req *) * MAX_SG,
+				 sizeof(struct io_thread_req *) * MAX_SG
+				 );
+		if(n == -EAGAIN){
 			continue;
 		}
-		io_count++;
-		do_io(req);
-		n = os_write_file(kernel_fd, &req,
-				  sizeof(struct io_thread_req *));
-		if(n != sizeof(struct io_thread_req *))
+		if(n < 0){
+			printk("io_thread - read failed, fd = %d, "
+			       "err = %d\n", kernel_fd, -n);
+		} 
+
+		if(n % sizeof(struct io_thread_req *) != 0){
+			printk("io_thread - invalid read, fd = %d, "
+			       "read = %d\n", kernel_fd, n);
+			continue;
+		}
+
+		rcount = n / sizeof(struct io_thread_req *);
+		
+		for (i = 0; i < rcount; i++) {
+
+			io_count++;
+
+			req = * (kernel_reqs + i);
+
+			do_io(req);
+
+		}
+
+		buffer = (unsigned char *) kernel_reqs;
+
+		while (n > 0) {
+			i = os_write_file(kernel_fd, buffer, n);
+			if(i >= 0){
+				buffer = buffer + i;
+				n = n - i;
+			} else {
+				if(i != -EAGAIN)
+					break;
+			}
+		} 
+
+		if(n > 0)
 			printk("io_thread - write failed, fd = %d, err = %d\n",
-			       kernel_fd, -n);
+			       kernel_fd, -i);
 	}
 
 	return 0;
diff --git a/arch/um/drivers/ubd_user.c b/arch/um/drivers/ubd_user.c
index e376f9b..3abec4f 100644
--- a/arch/um/drivers/ubd_user.c
+++ b/arch/um/drivers/ubd_user.c
@@ -17,10 +17,22 @@
 #include <sys/param.h>
 #include <endian.h>
 #include <byteswap.h>
+#include <poll.h>
 
 #include "ubd.h"
 #include <os.h>
 
+void setup_ubd_pollfd(void * fds, int fd) {
+	struct pollfd * pfds = (struct pollfd *) fds;
+	pfds->fd = fd;
+	pfds->events = POLLIN | POLLPRI;
+	pfds->revents = 0;
+}
+
+int size_of_pollfd(void) {
+	return sizeof(struct pollfd);
+}
+
 int start_io_thread(unsigned long sp, int *fd_out)
 {
 	int pid, fds[2], err;
@@ -36,10 +48,15 @@ int start_io_thread(unsigned long sp, int *fd_out)
 
 	err = os_set_fd_block(*fd_out, 0);
 	if (err) {
-		printk("start_io_thread - failed to set nonblocking I/O.\n");
+		printk("start_io_thread - failed to set nonblocking I/O - kernel_fd.\n");
 		goto out_close;
 	}
 
+	err = os_set_fd_block(kernel_fd, 0);
+	if (err) {
+		printk("start_io_thread - failed to set nonblocking I/O - user_fd.\n");
+		goto out_close;
+	}
 	pid = clone(io_thread, (void *) sp, CLONE_FILES | CLONE_VM, NULL);
 	if(pid < 0){
 		err = -errno;
diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index 7a04ddd..0b75efa 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -149,6 +149,7 @@ extern int os_file_size(const char *file, unsigned long long *size_out);
 extern int os_pread_file(int fd, void *buf, int len, unsigned long long offset);
 extern int os_pwrite_file(int fd, const void *buf, int count, unsigned long long offset);
 extern int os_file_modtime(const char *file, unsigned long *modtime);
+extern int os_poll(void *fds, unsigned int nfds, int timeout);
 extern int os_pipe(int *fd, int stream, int close_on_exec);
 extern int os_set_fd_async(int fd);
 extern int os_clear_fd_async(int fd);
diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c
index 2db18cb..726b1e1 100644
--- a/arch/um/os-Linux/file.c
+++ b/arch/um/os-Linux/file.c
@@ -14,6 +14,7 @@
 #include <sys/stat.h>
 #include <sys/un.h>
 #include <sys/types.h>
+#include <poll.h>
 #include <os.h>
 
 static void copy_stat(struct uml_stat *dst, const struct stat64 *src)
@@ -355,6 +356,17 @@ int os_file_modtime(const char *file, unsigned long *modtime)
 	return 0;
 }
 
+int os_poll(void *fds, unsigned int nfds, int timeout)
+{
+	int n;
+
+	CATCH_EINTR(n = poll((struct pollfd *) fds, nfds, timeout));
+	if (n < 0)
+		return -errno;
+
+	return n;
+}
+
 int os_set_exec_close(int fd)
 {
 	int err;
-- 
2.1.4