* Re: [PATCH 1/4] aio: remove the needless registration of ring file's private_data [not found] <1405996804-8262-1-git-send-email-guz.fnst@cn.fujitsu.com> @ 2014-07-22 13:53 ` Jeff Moyer [not found] ` <1405996804-8262-2-git-send-email-guz.fnst@cn.fujitsu.com> ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Jeff Moyer @ 2014-07-22 13:53 UTC (permalink / raw) To: Gu Zheng; +Cc: bcrl, axboe, akpm, linux-aio, linux-kernel Gu Zheng <guz.fnst@cn.fujitsu.com> writes: > Remove the registration of ring file's private_data, we do not use > it. > > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> Reviewed-by: Jeff Moyer <jmoyer@redhat.com> > --- > fs/aio.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/fs/aio.c b/fs/aio.c > index 955947e..ad35876 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -192,7 +192,6 @@ static struct file *aio_private_file(struct kioctx *ctx, loff_t nr_pages) > } > > file->f_flags = O_RDWR; > - file->private_data = ctx; > return file; > } ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1405996804-8262-2-git-send-email-guz.fnst@cn.fujitsu.com>]
* Re: [PATCH 2/4] aio: use the macro rather than the inline magic number [not found] ` <1405996804-8262-2-git-send-email-guz.fnst@cn.fujitsu.com> @ 2014-07-22 13:54 ` Jeff Moyer 2014-07-25 3:13 ` Ming Lei 2014-07-22 14:11 ` Benjamin LaHaise 1 sibling, 1 reply; 11+ messages in thread From: Jeff Moyer @ 2014-07-22 13:54 UTC (permalink / raw) To: Gu Zheng; +Cc: bcrl, axboe, akpm, linux-aio, linux-kernel Gu Zheng <guz.fnst@cn.fujitsu.com> writes: > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> Reviewed-by: Jeff Moyer <jmoyer@redhat.com> > --- > fs/aio.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/fs/aio.c b/fs/aio.c > index ad35876..1dc6158 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -201,7 +201,7 @@ static struct dentry *aio_mount(struct file_system_type *fs_type, > static const struct dentry_operations ops = { > .d_dname = simple_dname, > }; > - return mount_pseudo(fs_type, "aio:", NULL, &ops, 0xa10a10a1); > + return mount_pseudo(fs_type, "aio:", NULL, &ops, AIO_RING_MAGIC); > } > > /* aio_setup ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] aio: use the macro rather than the inline magic number 2014-07-22 13:54 ` [PATCH 2/4] aio: use the macro rather than the inline magic number Jeff Moyer @ 2014-07-25 3:13 ` Ming Lei 2014-07-25 5:54 ` Gu Zheng 0 siblings, 1 reply; 11+ messages in thread From: Ming Lei @ 2014-07-25 3:13 UTC (permalink / raw) To: Jeff Moyer Cc: Gu Zheng, Benjamin LaHaise, Jens Axboe, Andrew Morton, open list:AIO, Linux Kernel Mailing List On Tue, Jul 22, 2014 at 9:54 PM, Jeff Moyer <jmoyer@redhat.com> wrote: > Gu Zheng <guz.fnst@cn.fujitsu.com> writes: > >> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> > > Reviewed-by: Jeff Moyer <jmoyer@redhat.com> > >> --- >> fs/aio.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/fs/aio.c b/fs/aio.c >> index ad35876..1dc6158 100644 >> --- a/fs/aio.c >> +++ b/fs/aio.c >> @@ -201,7 +201,7 @@ static struct dentry *aio_mount(struct file_system_type *fs_type, >> static const struct dentry_operations ops = { >> .d_dname = simple_dname, >> }; >> - return mount_pseudo(fs_type, "aio:", NULL, &ops, 0xa10a10a1); >> + return mount_pseudo(fs_type, "aio:", NULL, &ops, AIO_RING_MAGIC); IMO, it might be better to use a new macro like AIO_FS_MAGIC with same number here because the same magic number 0xa10a10a1 serves for two purposes. Thanks, ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] aio: use the macro rather than the inline magic number 2014-07-25 3:13 ` Ming Lei @ 2014-07-25 5:54 ` Gu Zheng 0 siblings, 0 replies; 11+ messages in thread From: Gu Zheng @ 2014-07-25 5:54 UTC (permalink / raw) To: Ming Lei Cc: Jeff Moyer, Benjamin LaHaise, Jens Axboe, Andrew Morton, AIO, Linux Kernel Mailing List, Hi Lei, On 07/25/2014 11:13 AM, Ming Lei wrote: > On Tue, Jul 22, 2014 at 9:54 PM, Jeff Moyer <jmoyer@redhat.com> wrote: >> Gu Zheng <guz.fnst@cn.fujitsu.com> writes: >> >>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> >> >> Reviewed-by: Jeff Moyer <jmoyer@redhat.com> >> >>> --- >>> fs/aio.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/fs/aio.c b/fs/aio.c >>> index ad35876..1dc6158 100644 >>> --- a/fs/aio.c >>> +++ b/fs/aio.c >>> @@ -201,7 +201,7 @@ static struct dentry *aio_mount(struct file_system_type *fs_type, >>> static const struct dentry_operations ops = { >>> .d_dname = simple_dname, >>> }; >>> - return mount_pseudo(fs_type, "aio:", NULL, &ops, 0xa10a10a1); >>> + return mount_pseudo(fs_type, "aio:", NULL, &ops, AIO_RING_MAGIC); > > IMO, it might be better to use a new macro like AIO_FS_MAGIC > with same number here because the same magic number > 0xa10a10a1 serves for two purposes. Sounds reasonable, but I prefer a more common name like AIO_MAGIC to serve two if we need.:) Thanks, Gu > > Thanks, > . > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] aio: use the macro rather than the inline magic number [not found] ` <1405996804-8262-2-git-send-email-guz.fnst@cn.fujitsu.com> 2014-07-22 13:54 ` [PATCH 2/4] aio: use the macro rather than the inline magic number Jeff Moyer @ 2014-07-22 14:11 ` Benjamin LaHaise 1 sibling, 0 replies; 11+ messages in thread From: Benjamin LaHaise @ 2014-07-22 14:11 UTC (permalink / raw) To: Gu Zheng; +Cc: axboe, akpm, linux-aio, linux-kernel On Tue, Jul 22, 2014 at 10:40:02AM +0800, Gu Zheng wrote: > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> You're missing a commit message here. The patch is otherwise fine. -ben > --- > fs/aio.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/fs/aio.c b/fs/aio.c > index ad35876..1dc6158 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -201,7 +201,7 @@ static struct dentry *aio_mount(struct file_system_type *fs_type, > static const struct dentry_operations ops = { > .d_dname = simple_dname, > }; > - return mount_pseudo(fs_type, "aio:", NULL, &ops, 0xa10a10a1); > + return mount_pseudo(fs_type, "aio:", NULL, &ops, AIO_RING_MAGIC); > } > > /* aio_setup > -- > 1.7.7 -- "Thought is the essence of where you are now." ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1405996804-8262-3-git-send-email-guz.fnst@cn.fujitsu.com>]
* Re: [PATCH 3/4] aio: fix some comments [not found] ` <1405996804-8262-3-git-send-email-guz.fnst@cn.fujitsu.com> @ 2014-07-22 13:54 ` Jeff Moyer 2014-07-22 14:12 ` Benjamin LaHaise 1 sibling, 0 replies; 11+ messages in thread From: Jeff Moyer @ 2014-07-22 13:54 UTC (permalink / raw) To: Gu Zheng; +Cc: bcrl, axboe, akpm, linux-aio, linux-kernel Gu Zheng <guz.fnst@cn.fujitsu.com> writes: > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> Reviewed-by: Jeff Moyer <jmoyer@redhat.com> > --- > fs/aio.c | 7 +++---- > 1 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/fs/aio.c b/fs/aio.c > index 1dc6158..0cd0479 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -1037,7 +1037,7 @@ void aio_complete(struct kiocb *iocb, long res, long res2) > } > EXPORT_SYMBOL(aio_complete); > > -/* aio_read_events > +/* aio_read_events_ring > * Pull an event off of the ioctx's event ring. Returns the number of > * events fetched > */ > @@ -1289,9 +1289,8 @@ static ssize_t aio_setup_single_vector(struct kiocb *kiocb, > } > > /* > - * aio_setup_iocb: > - * Performs the initial checks and aio retry method > - * setup for the kiocb at the time of io submission. > + * aio_run_iocb: > + * Performs the initial checks and io submission. > */ > static ssize_t aio_run_iocb(struct kiocb *req, unsigned opcode, > char __user *buf, bool compat) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] aio: fix some comments [not found] ` <1405996804-8262-3-git-send-email-guz.fnst@cn.fujitsu.com> 2014-07-22 13:54 ` [PATCH 3/4] aio: fix some comments Jeff Moyer @ 2014-07-22 14:12 ` Benjamin LaHaise 2014-07-23 3:58 ` Gu Zheng 1 sibling, 1 reply; 11+ messages in thread From: Benjamin LaHaise @ 2014-07-22 14:12 UTC (permalink / raw) To: Gu Zheng; +Cc: axboe, akpm, linux-aio, linux-kernel On Tue, Jul 22, 2014 at 10:40:03AM +0800, Gu Zheng wrote: > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> Again, you're missing a commit message here. Please resubmit with a commit message. -ben > --- > fs/aio.c | 7 +++---- > 1 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/fs/aio.c b/fs/aio.c > index 1dc6158..0cd0479 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -1037,7 +1037,7 @@ void aio_complete(struct kiocb *iocb, long res, long res2) > } > EXPORT_SYMBOL(aio_complete); > > -/* aio_read_events > +/* aio_read_events_ring > * Pull an event off of the ioctx's event ring. Returns the number of > * events fetched > */ > @@ -1289,9 +1289,8 @@ static ssize_t aio_setup_single_vector(struct kiocb *kiocb, > } > > /* > - * aio_setup_iocb: > - * Performs the initial checks and aio retry method > - * setup for the kiocb at the time of io submission. > + * aio_run_iocb: > + * Performs the initial checks and io submission. > */ > static ssize_t aio_run_iocb(struct kiocb *req, unsigned opcode, > char __user *buf, bool compat) > -- > 1.7.7 -- "Thought is the essence of where you are now." ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] aio: fix some comments 2014-07-22 14:12 ` Benjamin LaHaise @ 2014-07-23 3:58 ` Gu Zheng 0 siblings, 0 replies; 11+ messages in thread From: Gu Zheng @ 2014-07-23 3:58 UTC (permalink / raw) To: Benjamin LaHaise; +Cc: axboe, akpm, linux-aio, linux-kernel On 07/22/2014 10:12 PM, Benjamin LaHaise wrote: > On Tue, Jul 22, 2014 at 10:40:03AM +0800, Gu Zheng wrote: >> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> > > Again, you're missing a commit message here. Please resubmit with a commit > message. Got it, I'll resend it later. Thanks, Gu > > -ben > >> --- >> fs/aio.c | 7 +++---- >> 1 files changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/fs/aio.c b/fs/aio.c >> index 1dc6158..0cd0479 100644 >> --- a/fs/aio.c >> +++ b/fs/aio.c >> @@ -1037,7 +1037,7 @@ void aio_complete(struct kiocb *iocb, long res, long res2) >> } >> EXPORT_SYMBOL(aio_complete); >> >> -/* aio_read_events >> +/* aio_read_events_ring >> * Pull an event off of the ioctx's event ring. Returns the number of >> * events fetched >> */ >> @@ -1289,9 +1289,8 @@ static ssize_t aio_setup_single_vector(struct kiocb *kiocb, >> } >> >> /* >> - * aio_setup_iocb: >> - * Performs the initial checks and aio retry method >> - * setup for the kiocb at the time of io submission. >> + * aio_run_iocb: >> + * Performs the initial checks and io submission. >> */ >> static ssize_t aio_run_iocb(struct kiocb *req, unsigned opcode, >> char __user *buf, bool compat) >> -- >> 1.7.7 > ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1405996804-8262-4-git-send-email-guz.fnst@cn.fujitsu.com>]
* Re: [PATCH 4/4] aio: use iovec array rather than the single one [not found] ` <1405996804-8262-4-git-send-email-guz.fnst@cn.fujitsu.com> @ 2014-07-22 13:48 ` Jeff Moyer 2014-07-22 15:20 ` Jeff Moyer 1 sibling, 0 replies; 11+ messages in thread From: Jeff Moyer @ 2014-07-22 13:48 UTC (permalink / raw) To: Gu Zheng; +Cc: bcrl, axboe, akpm, linux-aio, linux-kernel Gu Zheng <guz.fnst@cn.fujitsu.com> writes: > use an iovec array rather than the single one, so that we can avoid > to alloc more iovecs buffer in small(< 8) PREADV/PWRITEV cases. It would be helpful to know what motivated this change and how you tested it. Thanks, Jeff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] aio: use iovec array rather than the single one [not found] ` <1405996804-8262-4-git-send-email-guz.fnst@cn.fujitsu.com> 2014-07-22 13:48 ` [PATCH 4/4] aio: use iovec array rather than the single one Jeff Moyer @ 2014-07-22 15:20 ` Jeff Moyer 2014-07-23 4:05 ` Gu Zheng 1 sibling, 1 reply; 11+ messages in thread From: Jeff Moyer @ 2014-07-22 15:20 UTC (permalink / raw) To: Gu Zheng; +Cc: bcrl, axboe, akpm, linux-aio, linux-kernel Gu Zheng <guz.fnst@cn.fujitsu.com> writes: > use an iovec array rather than the single one, so that we can avoid > to alloc more iovecs buffer in small(< 8) PREADV/PWRITEV cases. I did some basic functional testing of this change and the change in patch 1/4. That testing included using aio-stress to drive queue depths of 7, 8 and 9, and verify that it didn't fall over. I also ran xfstests './check -g aio', and libaio's 'make partcheck'. The change looks good to me, and passed testing, so: Reviewed-by: Jeff Moyer <jmoyer@redhat.com> However, I still would like some comment on the reasoning behind it, and whether there is some measurable performance advantage for some workload. Additionally, it would be nice if that comment made its way into the commit message. Cheers, Jeff > > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> > --- > fs/aio.c | 10 +++++----- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/fs/aio.c b/fs/aio.c > index 0cd0479..ef21efe 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -1260,12 +1260,12 @@ static ssize_t aio_setup_vectored_rw(struct kiocb *kiocb, > if (compat) > ret = compat_rw_copy_check_uvector(rw, > (struct compat_iovec __user *)buf, > - *nr_segs, 1, *iovec, iovec); > + *nr_segs, UIO_FASTIOV, *iovec, iovec); > else > #endif > ret = rw_copy_check_uvector(rw, > (struct iovec __user *)buf, > - *nr_segs, 1, *iovec, iovec); > + *nr_segs, UIO_FASTIOV, *iovec, iovec); > if (ret < 0) > return ret; > > @@ -1302,7 +1302,7 @@ static ssize_t aio_run_iocb(struct kiocb *req, unsigned opcode, > fmode_t mode; > aio_rw_op *rw_op; > rw_iter_op *iter_op; > - struct iovec inline_vec, *iovec = &inline_vec; > + struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs; > struct iov_iter iter; > > switch (opcode) { > @@ -1337,7 +1337,7 @@ rw_common: > if (!ret) > ret = rw_verify_area(rw, file, &req->ki_pos, req->ki_nbytes); > if (ret < 0) { > - if (iovec != &inline_vec) > + if (iovec != inline_vecs) > kfree(iovec); > return ret; > } > @@ -1384,7 +1384,7 @@ rw_common: > return -EINVAL; > } > > - if (iovec != &inline_vec) > + if (iovec != inline_vecs) > kfree(iovec); > > if (ret != -EIOCBQUEUED) { ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] aio: use iovec array rather than the single one 2014-07-22 15:20 ` Jeff Moyer @ 2014-07-23 4:05 ` Gu Zheng 0 siblings, 0 replies; 11+ messages in thread From: Gu Zheng @ 2014-07-23 4:05 UTC (permalink / raw) To: Jeff Moyer; +Cc: bcrl, axboe, akpm, linux-aio, linux-kernel Hi Jeff, On 07/22/2014 11:20 PM, Jeff Moyer wrote: > Gu Zheng <guz.fnst@cn.fujitsu.com> writes: > >> use an iovec array rather than the single one, so that we can avoid >> to alloc more iovecs buffer in small(< 8) PREADV/PWRITEV cases. > > I did some basic functional testing of this change and the change in > patch 1/4. That testing included using aio-stress to drive queue depths > of 7, 8 and 9, and verify that it didn't fall over. I also ran xfstests > './check -g aio', and libaio's 'make partcheck'. > > The change looks good to me, and passed testing, so: > > Reviewed-by: Jeff Moyer <jmoyer@redhat.com> Thanks for your review and test. > > However, I still would like some comment on the reasoning behind it, and > whether there is some measurable performance advantage for some > workload. Additionally, it would be nice if that comment made its way > into the commit message. I'll add more useful info, and send it out later. Thanks, Gu > > Cheers, > Jeff > >> >> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> >> --- >> fs/aio.c | 10 +++++----- >> 1 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/fs/aio.c b/fs/aio.c >> index 0cd0479..ef21efe 100644 >> --- a/fs/aio.c >> +++ b/fs/aio.c >> @@ -1260,12 +1260,12 @@ static ssize_t aio_setup_vectored_rw(struct kiocb *kiocb, >> if (compat) >> ret = compat_rw_copy_check_uvector(rw, >> (struct compat_iovec __user *)buf, >> - *nr_segs, 1, *iovec, iovec); >> + *nr_segs, UIO_FASTIOV, *iovec, iovec); >> else >> #endif >> ret = rw_copy_check_uvector(rw, >> (struct iovec __user *)buf, >> - *nr_segs, 1, *iovec, iovec); >> + *nr_segs, UIO_FASTIOV, *iovec, iovec); >> if (ret < 0) >> return ret; >> >> @@ -1302,7 +1302,7 @@ static ssize_t aio_run_iocb(struct kiocb *req, unsigned opcode, >> fmode_t mode; >> aio_rw_op *rw_op; >> rw_iter_op *iter_op; >> - struct iovec inline_vec, *iovec = &inline_vec; >> + struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs; >> struct iov_iter iter; >> >> switch (opcode) { >> @@ -1337,7 +1337,7 @@ rw_common: >> if (!ret) >> ret = rw_verify_area(rw, file, &req->ki_pos, req->ki_nbytes); >> if (ret < 0) { >> - if (iovec != &inline_vec) >> + if (iovec != inline_vecs) >> kfree(iovec); >> return ret; >> } >> @@ -1384,7 +1384,7 @@ rw_common: >> return -EINVAL; >> } >> >> - if (iovec != &inline_vec) >> + if (iovec != inline_vecs) >> kfree(iovec); >> >> if (ret != -EIOCBQUEUED) { > . > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-07-25 6:06 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1405996804-8262-1-git-send-email-guz.fnst@cn.fujitsu.com> 2014-07-22 13:53 ` [PATCH 1/4] aio: remove the needless registration of ring file's private_data Jeff Moyer [not found] ` <1405996804-8262-2-git-send-email-guz.fnst@cn.fujitsu.com> 2014-07-22 13:54 ` [PATCH 2/4] aio: use the macro rather than the inline magic number Jeff Moyer 2014-07-25 3:13 ` Ming Lei 2014-07-25 5:54 ` Gu Zheng 2014-07-22 14:11 ` Benjamin LaHaise [not found] ` <1405996804-8262-3-git-send-email-guz.fnst@cn.fujitsu.com> 2014-07-22 13:54 ` [PATCH 3/4] aio: fix some comments Jeff Moyer 2014-07-22 14:12 ` Benjamin LaHaise 2014-07-23 3:58 ` Gu Zheng [not found] ` <1405996804-8262-4-git-send-email-guz.fnst@cn.fujitsu.com> 2014-07-22 13:48 ` [PATCH 4/4] aio: use iovec array rather than the single one Jeff Moyer 2014-07-22 15:20 ` Jeff Moyer 2014-07-23 4:05 ` Gu Zheng
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.