Xen devel - May 2012 - xen3.3.x on modern kernel / fs/aio.c / SUSE patches.xen/xen3-auto-common.diff

I am busy trying to forward-port xen3.3.x onto an Ubuntu Precise kernel.
My route of attack is to apply all the patches.xen/ files that were
in the SUSE 3.3.2 kernel release.

I appear to have been reasonably successful with this bar one change
I don''t understand.

In fs/aio.c, patches.xen/xen3-autocommon.diff has the following patch hunk
listed at the end of this email. The first hunk applies fine. The later
hunk does not. This is because the ioctx handling appears to have changed.
I am not sure how this works or what it is trying to do.

The Ubuntu version had:
        ioctx = ioctx_alloc(nr_events);
        ret = PTR_ERR(ioctx);
        if (!IS_ERR(ioctx)) {
                ret = put_user(ioctx->user_id, ctxp);
                if (!ret) {
                        put_ioctx(ioctx);
                        return 0;
                }
                io_destroy(ioctx);
        }

out:
        return ret;

Note the put_ioctx, ioctx_alloc, and lack of get_ioctx.

I /think/ the right solution here is as follows. Any ideas?

        ioctx = ioctx_alloc(nr_events);
        ret = PTR_ERR(ioctx);
        if (!IS_ERR(ioctx)) {
                ret = put_user(ioctx->user_id, ctxp);
#ifdef CONFIG_EPOLL
                if (make_fd && ret >= 0)
                        ret = make_aio_fd(ioctx);
#endif
                if (!ret) {
                        put_ioctx(ioctx);
                        return 0;
                }
                io_destroy(ioctx);
        }

out:
        return ret;

git tree at
 http://git.alex.org.uk/linux-flexiantxendom0.git/tree/flexiantxendom0

my version of that patch to aio.c here:
 http://git.alex.org.uk/linux-flexiantxendom0.git/blobdiff/237bae5c53e25adcbc
945a23759186e09c4cb50e..82c4e3c170517972d9369d42564303170432acef:/fs/aio.c

for anyone interested (I haven''t done the config flavour stuff yet).

-- 
Alex Bligh

--- head-2012-01-06.orig/fs/aio.c       2012-01-06 10:21:23.000000000 +0100
+++ head-2012-01-06/fs/aio.c    2011-11-16 17:01:54.000000000 +0100
...snip...
@@ -1307,18 +1392,30 @@ static void io_destroy(struct kioctx *io
  *     resources are available.  May fail with -EFAULT if an invalid
  *     pointer is passed for ctxp.  Will fail with -ENOSYS if not
  *     implemented.
+ *
+ *     To request a selectable fd, the user context has to be initialized
+ *     to 1, instead of 0, and the return value is the fd.
+ *     This keeps the system call compatible, since a non-zero value
+ *     was not allowed so far.
  */
 SYSCALL_DEFINE2(io_setup, unsigned, nr_events, aio_context_t __user *, 
ctxp)
 {
        struct kioctx *ioctx = NULL;
        unsigned long ctx;
        long ret;
+       int make_fd = 0;

        ret = get_user(ctx, ctxp);
        if (unlikely(ret))
                goto out;

        ret = -EINVAL;
+#ifdef CONFIG_EPOLL
+       if (ctx == 1) {
+               make_fd = 1;
+               ctx = 0;
+       }
+#endif
        if (unlikely(ctx || nr_events == 0)) {
                pr_debug("EINVAL: io_setup: ctx %lu nr_events %u\n",
                         ctx, nr_events);
@@ -1329,8 +1426,12 @@ SYSCALL_DEFINE2(io_setup, unsigned, nr_e
        ret = PTR_ERR(ioctx);
        if (!IS_ERR(ioctx)) {
                ret = put_user(ioctx->user_id, ctxp);
-               if (!ret)
-                       return 0;
+#ifdef CONFIG_EPOLL
+               if (make_fd && ret >= 0)
+                       ret = make_aio_fd(ioctx);
+#endif
+               if (ret >= 0)
+                       return ret;

                get_ioctx(ioctx); /* io_destroy() expects us to hold a ref 
*/
                io_destroy(ioctx);

>>> On 17.05.12 at 23:05, Alex Bligh <alex@alex.org.uk> wrote:
> I am busy trying to forward-port xen3.3.x onto an Ubuntu Precise kernel.
> My route of attack is to apply all the patches.xen/ files that were
> in the SUSE 3.3.2 kernel release.
> 
> I appear to have been reasonably successful with this bar one change
> I don''t understand.
> 
> In fs/aio.c, patches.xen/xen3-autocommon.diff has the following patch hunk
> listed at the end of this email. The first hunk applies fine. The later
> hunk does not. This is because the ioctx handling appears to have changed.
> I am not sure how this works or what it is trying to do.
> 
> The Ubuntu version had:
>         ioctx = ioctx_alloc(nr_events);
>         ret = PTR_ERR(ioctx);
>         if (!IS_ERR(ioctx)) {
>                 ret = put_user(ioctx->user_id, ctxp);
>                 if (!ret) {
>                         put_ioctx(ioctx);
>                         return 0;
>                 }
>                 io_destroy(ioctx);
>         }
> 
> out:
>         return ret;
> 
> Note the put_ioctx, ioctx_alloc, and lack of get_ioctx.
> 
> I /think/ the right solution here is as follows. Any ideas?
> 
>         ioctx = ioctx_alloc(nr_events);
>         ret = PTR_ERR(ioctx);
>         if (!IS_ERR(ioctx)) {
>                 ret = put_user(ioctx->user_id, ctxp);
> #ifdef CONFIG_EPOLL
>                 if (make_fd && ret >= 0)
>                         ret = make_aio_fd(ioctx);
> #endif
>                 if (!ret) {
>                         put_ioctx(ioctx);
>                         return 0;
>                 }
>                 io_destroy(ioctx);
>         }
> 
> out:
>         return ret;
Our code (3.4-rc6) looks like this

	if (!IS_ERR(ioctx)) {
		ret = put_user(ioctx->user_id, ctxp);
#ifdef CONFIG_EPOLL
		if (make_fd && !ret)
			ret = make_aio_fd(ioctx);
#endif
		if (ret < 0)
			io_destroy(ioctx);
		put_ioctx(ioctx);
	}

The patch apply failure and other confusion are presumably due
to a backport of a first attempt to address the issue someone
spotted in the AIO code - I would assume/hope that the full
3.4 solution got/will get backported to a later 3.3.x.

Jan