LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Zach Brown <zach.brown@oracle.com>
To: "Sébastien Dugué" <sebastien.dugue@bull.net>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	linux-aio <linux-aio@kvack.org>, Andrew Morton <akpm@osdl.org>,
	Suparna Bhattacharya <suparna@in.ibm.com>,
	Christoph Hellwig <hch@infradead.org>,
	Badari Pulavarty <pbadari@us.ibm.com>,
	Ulrich Drepper <drepper@redhat.com>,
	Jean Pierre Dion <jean-pierre.dion@bull.net>
Subject: Re: [PATCH -mm 1/5][AIO] - Rework compat_sys_io_submit
Date: Wed, 29 Nov 2006 16:47:47 -0800	[thread overview]
Message-ID: <8BA392C6-FCCB-40BD-9CCF-3EF56C3491BD@oracle.com> (raw)
In-Reply-To: <20061129113212.1e614a61@frecb000686>

On Nov 29, 2006, at 2:32 AM, Sébastien Dugué wrote:

>                  compat_sys_io_submit() cleanup
>
>
>   Cleanup compat_sys_io_submit by duplicating some of the native  
> syscall
> logic in the compat layer and directly calling io_submit_one() instead
> of fooling the syscall into thinking it is called from a native 64-bit
> caller.
>
>   This is needed for the completion notification patch to avoid having
> to rewrite each iocb on the caller stack for sys_io_submit() to  
> find the
> sigevents.

You could explicitly mention that this eliminates:

  - the overhead of copying nr pointers on the userspace caller's stack

  - the arbitrary PAGE_SIZE/(sizeof(void *)) limit on the number of  
iocbs that can be submitted

Those alone make this worth merging.

> +	if (unlikely(!access_ok(VERIFY_READ, iocb, (nr * sizeof(u32)))))
> +		return -EFAULT;

I'm glad you got that right :)  I no doubt would have initially  
hoisted these little checks into a shared helper function and missed  
that detail of getting the size of the access_ok() right in the  
compat case.

> +	put_ioctx(ctx);
> +
> +	return i? i: ret;

sys_io_getevents() reads:

         put_ioctx(ctx);
         return i ? i : ret;

So while this compat_sys_io_submit() logic seems fine and I would be  
comfortable with it landing as-is, I'd also appreciate it if we  
didn't introduce differences between the two functions when it seems  
just as easy to make them the same.  (That chunk is just one  
example.  There's whitespace, missing unlikely()s, etc).

- z

  reply	other threads:[~2006-11-30  0:47 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-29 10:24 [PATCH -mm 0/5][AIO] - AIO completion signal notification v3 Sébastien Dugué
2006-11-29 10:32 ` [PATCH -mm 1/5][AIO] - Rework compat_sys_io_submit Sébastien Dugué
2006-11-30  0:47   ` Zach Brown [this message]
2006-11-30  9:57     ` Sébastien Dugué
2006-11-30 17:27       ` Zach Brown
2006-11-29 10:32 ` [PATCH -mm 2/5][AIO] - fix aio.h includes Sébastien Dugué
2006-11-29 10:32 ` [PATCH -mm 3/5][AIO] - export good_sigevent() Sébastien Dugué
2006-11-29 10:38   ` Christoph Hellwig
2006-11-29 10:46     ` Sébastien Dugué
2006-11-29 14:54   ` Christoph Hellwig
2006-11-29 16:10     ` Sébastien Dugué
2006-12-04 17:13   ` Bharata B Rao
2006-12-05  8:30     ` Sébastien Dugué
2006-11-29 10:33 ` [PATCH -mm 4/5][AIO] - AIO completion signal notification Sébastien Dugué
2006-11-29 10:51   ` Christoph Hellwig
2006-11-29 13:08     ` Sébastien Dugué
2006-11-29 13:50       ` Christoph Hellwig
2006-11-29 14:18         ` Sébastien Dugué
2006-11-29 11:33   ` Jakub Jelinek
2006-11-29 13:25     ` Sébastien Dugué
2006-11-29 10:33 ` [PATCH -mm 5/5][AIO] - Listio support Sébastien Dugué
2006-11-30  8:25   ` Suparna Bhattacharya
2006-11-30 10:04     ` Sébastien Dugué
2006-11-30 15:38 [PATCH -mm 0/5][AIO] - AIO completion signal notification v4 Sébastien Dugué
2006-11-30 15:49 ` [PATCH -mm 1/5][AIO] - Rework compat_sys_io_submit Sébastien Dugué
     [not found] <20070117104601.36b2ab18@frecb000686>
2007-01-17  9:48 ` Sébastien Dugué

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8BA392C6-FCCB-40BD-9CCF-3EF56C3491BD@oracle.com \
    --to=zach.brown@oracle.com \
    --cc=akpm@osdl.org \
    --cc=drepper@redhat.com \
    --cc=hch@infradead.org \
    --cc=jean-pierre.dion@bull.net \
    --cc=linux-aio@kvack.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbadari@us.ibm.com \
    --cc=sebastien.dugue@bull.net \
    --cc=suparna@in.ibm.com \
    --subject='Re: [PATCH -mm 1/5][AIO] - Rework compat_sys_io_submit' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).