From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:06:15
Hello all,
First off, sorry for the wide reaching To and Cc, but this patch series
touches the core kernel and also reaches across subsystems a bit. If
some of the people who read this can provide review feedback, I would
very much appreciate it.
This series introduces new AIO functionality to make use of kernel
threads (by way of queue_work()) to implement additional asynchronous
operations. The work came about as the result of various tuning done to
the kernel for my employer (Solace Systems) that we ship in our
products.
First off, the benefits: using kernel threads to implement AIO
functionality has a significant benefit in our application. Compared to
a user space thread pool based AIO implementation, we see roughly a 25%
performance improvement in our application by using this new kernel
based AIO functionality. This comes about as a consequence of fewer
context switches, fewer transitions to/from userspace, and the ability
to make certain optimizations in the kernel that are otherwise
impossible in userspace (ie the new readahead functionality).
Now the downsides: when using queue_work(), code executes in the context
of a different task that the submitter of the operation. This means
that there are significant security concerns if there are any bugs in
the code that sets up the appropriate security credentials and related
context in struct task. There may well be DoS bugs in this
implementation which have yet to be discovered.
Given the benefits, I am of the opinion that this patch series is a
useful addition to the kernel. Since this code will be experimental for
some period of time as the interactions with other subsystems are
reviewed and tested, I have implemented a config option to allow for
this code to be compiled out and a sysctl (fs.aio-auto-threads) that
must be explicitly set to 1 before this new functionality is available
to userspace. Hopefully this is enough to address the security concerns
during the growing pains and allow other developers to safely explore
the new functionality.
Caveats: the existing O_DIRECT AIO code path is currently bypassed when
the new thread helpers are enabled. I plan to do additional work in
this area, but the fact that the dio code can block under certain
conditions is not acceptable to the applications I am working on, as it
leads to starvation of other requests the system is processing. That
said, this is what's ready today, and I hope that people can provide
feedback to help drive further improvements.
I will be posting further documentation and test cases later this week
for people to experiment with, but for those looking for a few test
programs to exercise the new functionality, there is a collection of
code at git://git.kvack.org/aio-testprogs.git/ . Getting the code
cleaned up from the internal implementation to something that is in
reasonable condition for submission ended up taking longer than
expected. Thankfully, this kernel cycle lines up with some internal QA
work, so there should be additional testing taking place over the next
couple of months.
Also, the libaio test harness has some bugs that the new functionality
revealed. A version with fixes for those tests can be fetched from
git://git.kvack.org/~bcrl/libaio.git/ . Wrappers for the new IOCB_CMD
types should be posted there by the end of the day.
Some notes on the new functionality: all operations are cancellable
providing the kernel subsystem involved aborts operations when delivered
a SIGKILL. This ensures that async operations on pipe and sockets are
cancelled when the process that issued the operations exits. A couple
of the test programs exercise this functionality on pipes.
Signal handling is slightly impacted by this AIO functionality.
Specifically, the first patch in the series introduces a new helper,
io_send_sig() that delivers a signal intended for the performer of an io
operation. This is used to deliver signals like SIGXFS and SIGPIPE. It
is a straightforward replacement of send_sig(SIGXXX, current, 0) to
io_send_sig(SIGXXX).
As always, comments, bug reports and feedback are appreciated.
Developers looking for a git pull can find one at
git://git.kvack.org/aio-next.git/ . Cheers!
-ben
Benjamin LaHaise (13):
signals: distinguish signals sent due to i/o via io_send_sig()
aio: add aio_get_mm() helper
aio: for async operations, make the iter argument persistent
signals: add and use aio_get_task() to direct signals sent via
io_send_sig()
fs: make do_loop_readv_writev() non-static
aio: add queue_work() based threaded aio support
aio: enabled thread based async fsync
aio: add support for aio poll via aio thread helper
aio: add support for async openat()
aio: add async unlinkat functionality
mm: enable __do_page_cache_readahead() to include present pages
aio: add support for aio readahead
aio: add support for aio renameat operation
drivers/gpu/drm/drm_lock.c | 2 +-
drivers/gpu/drm/ttm/ttm_lock.c | 6 +-
fs/aio.c | 727 ++++++++++++++++++++++++++++++++++++++---
fs/attr.c | 2 +-
fs/binfmt_flat.c | 2 +-
fs/fuse/dev.c | 2 +-
fs/internal.h | 6 +
fs/namei.c | 2 +-
fs/pipe.c | 4 +-
fs/read_write.c | 5 +-
fs/splice.c | 8 +-
include/linux/aio.h | 9 +
include/linux/fs.h | 3 +
include/linux/sched.h | 6 +
include/uapi/linux/aio_abi.h | 15 +-
init/Kconfig | 13 +
kernel/auditsc.c | 6 +-
kernel/signal.c | 20 ++
kernel/sysctl.c | 9 +
mm/filemap.c | 6 +-
mm/internal.h | 4 +-
mm/readahead.c | 13 +-
net/atm/common.c | 4 +-
net/ax25/af_ax25.c | 2 +-
net/caif/caif_socket.c | 2 +-
net/core/stream.c | 2 +-
net/decnet/af_decnet.c | 2 +-
net/irda/af_irda.c | 4 +-
net/netrom/af_netrom.c | 2 +-
net/rose/af_rose.c | 2 +-
net/sctp/socket.c | 2 +-
net/unix/af_unix.c | 4 +-
net/x25/af_x25.c | 2 +-
33 files changed, 817 insertions(+), 81 deletions(-)
--
2.5.0
--
"Thought is the essence of where you are now."
--
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>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:06:32
In preparation for thread based aio support, make the callers of
send_sig() that are sending a signal as a direct consequence of a
read or write operation (typically for SIGPIPE or SIGXFS) use a
separate helper of io_send_sig(). This will make it possible for
the thread based aio operations to direct these signals to the
process that actually submitted the aio request.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
drivers/gpu/drm/drm_lock.c | 2 +-
drivers/gpu/drm/ttm/ttm_lock.c | 6 +++---
fs/attr.c | 2 +-
fs/binfmt_flat.c | 2 +-
fs/fuse/dev.c | 2 +-
fs/pipe.c | 4 ++--
fs/splice.c | 8 ++++----
include/linux/sched.h | 1 +
kernel/auditsc.c | 6 +++---
kernel/signal.c | 14 ++++++++++++++
mm/filemap.c | 6 ++++--
net/atm/common.c | 4 ++--
net/ax25/af_ax25.c | 2 +-
net/caif/caif_socket.c | 2 +-
net/core/stream.c | 2 +-
net/decnet/af_decnet.c | 2 +-
net/irda/af_irda.c | 4 ++--
net/netrom/af_netrom.c | 2 +-
net/rose/af_rose.c | 2 +-
net/sctp/socket.c | 2 +-
net/unix/af_unix.c | 4 ++--
net/x25/af_x25.c | 2 +-
22 files changed, 49 insertions(+), 32 deletions(-)
@@ -83,7 +83,7 @@ int drm_legacy_lock(struct drm_device *dev, void *data,__set_current_state(TASK_INTERRUPTIBLE);if(!master->lock.hw_lock){/* Device has been unregistered */-send_sig(SIGTERM,current,0);+io_send_sig(SIGTERM);ret=-EINTR;break;}
@@ -1422,6 +1422,20 @@ int send_sig_info(int sig, struct siginfo *info, struct task_struct *p)returndo_send_sig_info(sig,info,p,false);}+/* io_send_sig: send a signal caused by an i/o operation+*+*Usethishelperwhenasignalisbeingsenttothetaskthatisresponsible+*foraerinitiatedoperation.Mostcommonlythisisusedtosendsignals+*likeSIGPIPEorSIGXFSthataretheresultofattemptingareadorwrite+*operation.Thisisusedbyaiotodirectasignaltothecorrecttaskin+*thecaseofasyncoperations.+*/+intio_send_sig(intsig)+{+returnsend_sig(sig,current,0);+}+EXPORT_SYMBOL(io_send_sig);+#define __si_special(priv) \((priv)?SEND_SIG_PRIV:SEND_SIG_NOINFO)
@@ -182,7 +182,7 @@ int sk_stream_error(struct sock *sk, int flags, int err)if(err==-EPIPE)err=sock_error(sk)?:-EPIPE;if(err==-EPIPE&&!(flags&MSG_NOSIGNAL))-send_sig(SIGPIPE,current,0);+io_send_sig(SIGPIPE);returnerr;}EXPORT_SYMBOL(sk_stream_error);
@@ -1556,7 +1556,7 @@ static int sctp_error(struct sock *sk, int flags, int err)if(err==-EPIPE)err=sock_error(sk)?:-EPIPE;if(err==-EPIPE&&!(flags&MSG_NOSIGNAL))-send_sig(SIGPIPE,current,0);+io_send_sig(SIGPIPE);returnerr;}
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:06:42
For various async operations, it is necessary to have a way of finding
the address space to use for accessing user memory. Add the helper
struct mm_struct *aio_get_mm(struct kiocb *) to address this use-case.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/aio.c | 15 +++++++++++++++
include/linux/aio.h | 2 ++
2 files changed, 17 insertions(+)
@@ -24,6 +25,7 @@ static inline long do_io_submit(aio_context_t ctx_id, long nr,boolcompat){return0;}staticinlinevoidkiocb_set_cancel_fn(structkiocb*req,kiocb_cancel_fn*cancel){}+staticinlinestructmm_struct*aio_get_mm(structkiocb*req){returnNULL;}#endif /* CONFIG_AIO *//* for sysctl: */
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:06:50
When implementing async read/write operations, the complexity of having
to duplicate the iter argument before passing to another thread leads to
duplicate code. There is no reason async operations issued by the aio
core need to be placed on the stack when an aio_kiocb is allocated for
each operation, so put the iter and iovec into aio_kiocb instead of on
the stack.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/aio.c | 41 +++++++++++++++++++++--------------------
1 file changed, 21 insertions(+), 20 deletions(-)
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:07:00
When a signal is triggered due to an i/o, io_send_sig() needs to deliver
the signal to the task issuing the i/o. Prepare for thread based aios
by annotating task_struct with a struct kiocb pointer that enables
io_sed_sig() to direct these signals to the submitter of the aio.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/aio.c | 16 ++++++++++++++++
include/linux/aio.h | 3 +++
include/linux/sched.h | 5 +++++
kernel/signal.c | 8 +++++++-
4 files changed, 31 insertions(+), 1 deletion(-)
@@ -18,6 +18,7 @@ extern long do_io_submit(aio_context_t ctx_id, long nr,structiocb__user*__user*iocbpp,boolcompat);voidkiocb_set_cancel_fn(structkiocb*req,kiocb_cancel_fn*cancel);structmm_struct*aio_get_mm(structkiocb*req);+structtask_struct*aio_get_task(structkiocb*req);#elsestaticinlinevoidexit_aio(structmm_struct*mm){}staticinlinelongdo_io_submit(aio_context_tctx_id,longnr,
@@ -26,6 +27,8 @@ static inline long do_io_submit(aio_context_t ctx_id, long nr,staticinlinevoidkiocb_set_cancel_fn(structkiocb*req,kiocb_cancel_fn*cancel){}staticinlinestructmm_struct*aio_get_mm(structkiocb*req){returnNULL;}+staticinlinestructtask_struct*aio_get_task(structkiocb*req)+{returncurrent;}#endif /* CONFIG_AIO *//* for sysctl: */
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:07:06
The threaded aio helper code needs to be able to call
do_loop_readv_writev() to perform i/o to file_operations that do not have
read_iter or write_iter methods. Make the prototype for
do_loop_readv_writev() non-static and move it into fs/internal.h
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/internal.h | 6 ++++++
fs/read_write.c | 5 +----
2 files changed, 7 insertions(+), 4 deletions(-)
@@ -668,7 +665,7 @@ static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter,}/* Do it by hand, with file-ops */-staticssize_tdo_loop_readv_writev(structfile*filp,structiov_iter*iter,+ssize_tdo_loop_readv_writev(structfile*filp,structiov_iter*iter,loff_t*ppos,io_fn_tfn){ssize_tret=0;
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:07:14
Add support for performing asynchronous reads and writes via kernel
threads by way of the queue_work() functionality. This enables fully
asynchronous and cancellable reads and writes for any file or device in
the kernel. Cancellation is implemented by sending a SIGKILL to the
kernel thread executing the async operation. So long as the read or
write operation can be interrupted by signals, the AIO request can be
cancelled.
This functionality is currently disabled by default until the DoS
implications of having user controlled kernel thread creation are fully
understood. When compiled into the kernel, this functionality can be
enabled by setting the fs.aio-auto-threads sysctl to 1. It is expected
that the feature will be enabled by default in a future kernel version.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/aio.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/aio.h | 4 +
include/linux/fs.h | 2 +
init/Kconfig | 13 +++
kernel/sysctl.c | 9 ++
5 files changed, 264 insertions(+)
@@ -194,12 +198,21 @@ struct aio_kiocb {/* Fields used for threaded aio helper. */structtask_struct*ki_submit_task;+#if IS_ENABLED(CONFIG_AIO_THREAD)+structtask_struct*ki_cancel_task;+unsignedlongki_rlimit_fsize;+aio_thread_work_fn_tki_work_fn;+structwork_structki_work;+#endif};/*------ sysctl variables----*/staticDEFINE_SPINLOCK(aio_nr_lock);unsignedlongaio_nr;/* current system wide number of aio requests */unsignedlongaio_max_nr=0x10000;/* system wide maximum number of aio requests */+#if IS_ENABLED(CONFIG_AIO_THREAD)+unsignedlongaio_auto_threads;/* Currently disabled by default */+#endif/*----end sysctl variables---*/staticstructkmem_cache*kiocb_cachep;
@@ -528,6 +550,8 @@ static int aio_setup_ring(struct kioctx *ctx)ring->head=ring->tail=0;ring->magic=AIO_RING_MAGIC;ring->compat_features=AIO_RING_COMPAT_FEATURES;+if(aio_may_use_threads())+ring->compat_features|=AIO_RING_COMPAT_THREADED;ring->incompat_features=AIO_RING_INCOMPAT_FEATURES;ring->header_length=sizeof(structaio_ring);kunmap_atomic(ring);
@@ -1436,6 +1460,202 @@ static int aio_setup_vectored_rw(int rw, char __user *buf, size_t len,len,UIO_FASTIOV,iovec,iter);}+#if IS_ENABLED(CONFIG_AIO_THREAD)+/* aio_thread_queue_iocb_cancel_early:+*Earlystagecancellationhelperfunctionforthreadedaios.This+*isusedpriortotheiocbbeingassignedtoaworkerthread.+*/+staticintaio_thread_queue_iocb_cancel_early(structkiocb*iocb)+{+return0;+}++/* aio_thread_queue_iocb_cancel:+*Latestagecancellationmethodforthreadedaios.Onceaniocbis+*assignedtoaworkerthread,weuseafatalsignaltointerruptan+*in-progressoperation.+*/+staticintaio_thread_queue_iocb_cancel(structkiocb*kiocb)+{+structaio_kiocb*iocb=container_of(kiocb,structaio_kiocb,common);++if(iocb->ki_cancel_task){+force_sig(SIGKILL,iocb->ki_cancel_task);+return0;+}+return-EAGAIN;+}++/* aio_thread_fn:+*Entrypointforworkertoperformthreadedaio.Handlesissues+*arisingduetocancellationusingsignals.+*/+staticvoidaio_thread_fn(structwork_struct*work)+{+structaio_kiocb*iocb=container_of(work,structaio_kiocb,ki_work);+kiocb_cancel_fn*old_cancel;+longret;++iocb->ki_cancel_task=current;+current->kiocb=&iocb->common;/* For io_send_sig(). */+WARN_ON(atomic_read(¤t->signal->sigcnt)!=1);++/* Check for early stage cancellation and switch to late stage+*cancellationifithasnotalreadyoccurred.+*/+old_cancel=cmpxchg(&iocb->ki_cancel,+aio_thread_queue_iocb_cancel_early,+aio_thread_queue_iocb_cancel);+if(old_cancel!=KIOCB_CANCELLED)+ret=iocb->ki_work_fn(iocb);+else+ret=-EINTR;++current->kiocb=NULL;+if(unlikely(ret==-ERESTARTSYS||ret==-ERESTARTNOINTR||+ret==-ERESTARTNOHAND||ret==-ERESTART_RESTARTBLOCK))+ret=-EINTR;++/* Completion serializes cancellation by taking ctx_lock, so+*aio_complete()willnotreturnuntilafterforce_sig()in+*aio_thread_queue_iocb_cancel().Thisshouldensurethat+*thesignalispendingbeforebeingflushedinthisthread.+*/+aio_complete(&iocb->common,ret,0);+if(fatal_signal_pending(current))+flush_signals(current);+}++#define AIO_THREAD_NEED_TASK 0x0001 /* Need aio_kiocb->ki_submit_task */++/* aio_thread_queue_iocb+*Queuesanaio_kiocbfordispatchtoaworkerthread.Preparesthe+*aio_kiocbforcancellation.Thecallermustprovideafunctionto+*executetheoperationinwork_fn.Theflagsmaybeprovidedasan+*oredsetAIO_THREAD_xxx.+*/+staticssize_taio_thread_queue_iocb(structaio_kiocb*iocb,+aio_thread_work_fn_twork_fn,+unsignedflags)+{+INIT_WORK(&iocb->ki_work,aio_thread_fn);+iocb->ki_work_fn=work_fn;+if(flags&AIO_THREAD_NEED_TASK){+iocb->ki_submit_task=current;+get_task_struct(iocb->ki_submit_task);+}++/* Cancellation needs to be always available for operations performed+*usinghelperthreads.Priortotheiocbbeingassignedtoaworker+*thread,weneedtorecordthatacancellationhasoccurred.We+*candothisbyhavingaminimalhelperfunctionthatisrecordedin+*ki_cancel.+*/+kiocb_set_cancel_fn(&iocb->common,aio_thread_queue_iocb_cancel_early);+queue_work(system_long_wq,&iocb->ki_work);+return-EIOCBQUEUED;+}++staticlongaio_thread_op_read_iter(structaio_kiocb*iocb)+{+structfile*filp;+longret;++use_mm(iocb->ki_ctx->mm);+filp=iocb->common.ki_filp;++if(filp->f_op->read_iter){+structkiocbsync_kiocb;++init_sync_kiocb(&sync_kiocb,filp);+sync_kiocb.ki_pos=iocb->common.ki_pos;+ret=filp->f_op->read_iter(&sync_kiocb,&iocb->ki_iter);+}elseif(filp->f_op->read)+ret=do_loop_readv_writev(filp,&iocb->ki_iter,+&iocb->common.ki_pos,+filp->f_op->read);+else+ret=-EINVAL;+unuse_mm(iocb->ki_ctx->mm);+returnret;+}++ssize_tgeneric_async_read_iter_non_direct(structkiocb*iocb,+structiov_iter*iter)+{+if((iocb->ki_flags&IOCB_DIRECT)||+(iocb->ki_complete!=aio_complete))+returniocb->ki_filp->f_op->read_iter(iocb,iter);+returngeneric_async_read_iter(iocb,iter);+}+EXPORT_SYMBOL(generic_async_read_iter_non_direct);++ssize_tgeneric_async_read_iter(structkiocb*iocb,structiov_iter*iter)+{+structaio_kiocb*req;++req=container_of(iocb,structaio_kiocb,common);+if(iter!=&req->ki_iter)+return-EINVAL;++returnaio_thread_queue_iocb(req,aio_thread_op_read_iter,+AIO_THREAD_NEED_TASK);+}+EXPORT_SYMBOL(generic_async_read_iter);++staticlongaio_thread_op_write_iter(structaio_kiocb*iocb)+{+u64saved_rlim_fsize;+structfile*filp;+longret;++use_mm(iocb->ki_ctx->mm);+filp=iocb->common.ki_filp;+saved_rlim_fsize=rlimit(RLIMIT_FSIZE);+current->signal->rlim[RLIMIT_FSIZE].rlim_cur=iocb->ki_rlimit_fsize;++if(filp->f_op->write_iter){+structkiocbsync_kiocb;++init_sync_kiocb(&sync_kiocb,filp);+sync_kiocb.ki_pos=iocb->common.ki_pos;+ret=filp->f_op->write_iter(&sync_kiocb,&iocb->ki_iter);+}elseif(filp->f_op->write)+ret=do_loop_readv_writev(filp,&iocb->ki_iter,+&iocb->common.ki_pos,+(io_fn_t)filp->f_op->write);+else+ret=-EINVAL;+current->signal->rlim[RLIMIT_FSIZE].rlim_cur=saved_rlim_fsize;+unuse_mm(iocb->ki_ctx->mm);+returnret;+}++ssize_tgeneric_async_write_iter_non_direct(structkiocb*iocb,+structiov_iter*iter)+{+if((iocb->ki_flags&IOCB_DIRECT)||+(iocb->ki_complete!=aio_complete))+returniocb->ki_filp->f_op->write_iter(iocb,iter);+returngeneric_async_write_iter(iocb,iter);+}+EXPORT_SYMBOL(generic_async_write_iter_non_direct);++ssize_tgeneric_async_write_iter(structkiocb*iocb,structiov_iter*iter)+{+structaio_kiocb*req;++req=container_of(iocb,structaio_kiocb,common);+if(iter!=&req->ki_iter)+return-EINVAL;+req->ki_rlimit_fsize=rlimit(RLIMIT_FSIZE);++returnaio_thread_queue_iocb(req,aio_thread_op_write_iter,+AIO_THREAD_NEED_TASK);+}+EXPORT_SYMBOL(generic_async_write_iter);+#endif /* IS_ENABLED(CONFIG_AIO_THREAD) */+/**aio_run_iocb:*Performstheinitialchecksandiosubmission.
@@ -19,6 +19,9 @@ extern long do_io_submit(aio_context_t ctx_id, long nr,voidkiocb_set_cancel_fn(structkiocb*req,kiocb_cancel_fn*cancel);structmm_struct*aio_get_mm(structkiocb*req);structtask_struct*aio_get_task(structkiocb*req);+structiov_iter;+ssize_tgeneric_async_read_iter(structkiocb*iocb,structiov_iter*iter);+ssize_tgeneric_async_write_iter(structkiocb*iocb,structiov_iter*iter);#elsestaticinlinevoidexit_aio(structmm_struct*mm){}staticinlinelongdo_io_submit(aio_context_tctx_id,longnr,
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:07:31
Applications that require a unified event loop occasionally have a need
to interface with libraries or other code that require notification on a
file descriptor becoming ready for read or write via poll. Add support
for the aio poll operation to enable these use-cases by way of the
thread based aio helpers.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/aio.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/aio_abi.h | 2 +-
2 files changed, 47 insertions(+), 1 deletion(-)
@@ -39,8 +39,8 @@ enum {IOCB_CMD_FDSYNC=3,/* These two are experimental.*IOCB_CMD_PREADX=4,-*IOCB_CMD_POLL=5,*/+IOCB_CMD_POLL=5,IOCB_CMD_NOOP=6,IOCB_CMD_PREADV=7,IOCB_CMD_PWRITEV=8,
--
2.5.0
--
"Thought is the essence of where you are now."
--
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>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:07:38
Another blocking operation used by applications that want aio
functionality is that of opening files that are not resident in memory.
Using the thread based aio helper, add support for IOCB_CMD_OPENAT.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/aio.c | 120 +++++++++++++++++++++++++++++++++++++------
include/uapi/linux/aio_abi.h | 2 +
2 files changed, 107 insertions(+), 15 deletions(-)
@@ -1496,6 +1502,9 @@ static int aio_thread_queue_iocb_cancel(struct kiocb *kiocb)staticvoidaio_thread_fn(structwork_struct*work){structaio_kiocb*iocb=container_of(work,structaio_kiocb,ki_work);+structfiles_struct*old_files=current->files;+conststructcred*old_cred=current_cred();+structfs_struct*old_fs=current->fs;kiocb_cancel_fn*old_cancel;longret;
@@ -1503,6 +1512,13 @@ static void aio_thread_fn(struct work_struct *work)current->kiocb=&iocb->common;/* For io_send_sig(). */WARN_ON(atomic_read(¤t->signal->sigcnt)!=1);+if(iocb->ki_fs)+current->fs=iocb->ki_fs;+if(iocb->ki_files)+current->files=iocb->ki_files;+if(iocb->ki_cred)+current->cred=iocb->ki_cred;+/* Check for early stage cancellation and switch to late stage*cancellationifithasnotalreadyoccurred.*/
@@ -1530,6 +1559,9 @@ static void aio_thread_fn(struct work_struct *work)}#define AIO_THREAD_NEED_TASK 0x0001 /* Need aio_kiocb->ki_submit_task */+#define AIO_THREAD_NEED_FS 0x0002 /* Need aio_kiocb->ki_fs */+#define AIO_THREAD_NEED_FILES 0x0004 /* Need aio_kiocb->ki_files */+#define AIO_THREAD_NEED_CRED 0x0008 /* Need aio_kiocb->ki_cred *//* aio_thread_queue_iocb*Queuesanaio_kiocbfordispatchtoaworkerthread.Preparesthe
@@ -1547,6 +1579,20 @@ static ssize_t aio_thread_queue_iocb(struct aio_kiocb *iocb,iocb->ki_submit_task=current;get_task_struct(iocb->ki_submit_task);}+if(flags&AIO_THREAD_NEED_FS){+structfs_struct*fs=current->fs;++iocb->ki_fs=fs;+spin_lock(&fs->lock);+fs->users++;+spin_unlock(&fs->lock);+}+if(flags&AIO_THREAD_NEED_FILES){+iocb->ki_files=current->files;+atomic_inc(&iocb->ki_files->count);+}+if(flags&AIO_THREAD_NEED_CRED)+iocb->ki_cred=get_current_cred();/* Cancellation needs to be always available for operations performed*usinghelperthreads.Priortotheiocbbeingassignedtoaworker
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:07:52
For the upcoming AIO readahead operation it is necessary to know that
all the pages in a readahead request have had reads issued for them or
that the read was satisfied from cache. Add a parameter to
__do_page_cache_readahead() to instruct it to count these pages in the
return value.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
mm/internal.h | 4 ++--
mm/readahead.c | 13 +++++++++----
2 files changed, 11 insertions(+), 6 deletions(-)
@@ -151,12 +151,13 @@ out:*/int__do_page_cache_readahead(structaddress_space*mapping,structfile*filp,pgoff_toffset,unsignedlongnr_to_read,-unsignedlonglookahead_size)+unsignedlonglookahead_size,intreport_present){structinode*inode=mapping->host;structpage*page;unsignedlongend_index;/* The last page we want to read */LIST_HEAD(page_pool);+intpresent=0;intpage_idx;intret=0;loff_tisize=i_size_read(inode);
--
2.5.0
--
"Thought is the essence of where you are now."
--
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>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:07:58
Introduce an asynchronous operation to populate the page cache with
pages at a given offset and length. This operation is conceptually
similar to performing an asynchronous read except that it does not
actually copy the data from the page cache into userspace, rather it
performs readahead and notifies userspace when all pages have been read.
The motivation for this came about as a result of investigation into a
performace degradation when reading from disk. In the case of a heavily
loaded system, the copy_to_user() performed for an asynchronous read was
temporally quite distant from when the data was actually used. By only
reading the data into the kernel's page cache, the cache pollution
caused by copying the data into userspace is avoided, and overall system
performance is improved.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/aio.c | 141 +++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/aio_abi.h | 1 +
2 files changed, 142 insertions(+)
@@ -238,6 +239,8 @@ long aio_do_openat(int fd, const char *filename, int flags, int mode);longaio_do_unlinkat(intfd,constchar*filename,intflags,intmode);longaio_foo_at(structaio_kiocb*req,do_foo_at_tdo_foo_at);+longaio_readahead(structaio_kiocb*iocb,unsignedlonglen);+static__always_inlineboolaio_may_use_threads(void){#if IS_ENABLED(CONFIG_AIO_THREAD)
@@ -1812,6 +1815,137 @@ long aio_foo_at(struct aio_kiocb *req, do_foo_at_t do_foo_at)AIO_THREAD_NEED_FILES|AIO_THREAD_NEED_CRED);}++staticintaio_ra_filler(void*data,structpage*page)+{+structfile*file=data;++returnfile->f_mapping->a_ops->readpage(file,page);+}++staticlongaio_ra_wait_on_pages(structfile*file,pgoff_tstart,+unsignedlongnr)+{+structaddress_space*mapping=file->f_mapping;+unsignedlongi;++/* Wait on pages starting at the end to holdfully avoid too many+*wakeups.+*/+for(i=nr;i-->0;){+pgoff_tindex=start+i;+structpage*page;++/* First do the quick check to see if the page is present and+*uptodate.+*/+rcu_read_lock();+page=radix_tree_lookup(&mapping->page_tree,index);+rcu_read_unlock();++if(page&&!radix_tree_exceptional_entry(page)&&+PageUptodate(page)){+continue;+}++page=read_cache_page(mapping,index,aio_ra_filler,file);+if(IS_ERR(page))+returnPTR_ERR(page);+page_cache_release(page);+}+return0;+}++staticlongaio_thread_op_readahead(structaio_kiocb*iocb)+{+pgoff_tstart,end,nr,offset;+longret=0;++start=iocb->common.ki_pos>>PAGE_CACHE_SHIFT;+end=(iocb->common.ki_pos+iocb->ki_data-1)>>PAGE_CACHE_SHIFT;+nr=end-start+1;++for(offset=0;offset<nr;){+pgoff_tchunk=nr-offset;+unsignedlongmax_chunk=(2*1024*1024)/PAGE_CACHE_SIZE;++if(chunk>max_chunk)+chunk=max_chunk;++ret=__do_page_cache_readahead(iocb->common.ki_filp->f_mapping,+iocb->common.ki_filp,+start+offset,chunk,0,1);+if(ret<=0)+break;+offset+=ret;+}++if(!offset&&ret<0)+returnret;++if(offset>0){+ret=aio_ra_wait_on_pages(iocb->common.ki_filp,start,offset);+if(ret<0)+returnret;+}++if(offset==nr)+returniocb->ki_data;+if(offset>0)+return((start+offset)<<PAGE_CACHE_SHIFT)-+iocb->common.ki_pos;+return0;+}++longaio_readahead(structaio_kiocb*iocb,unsignedlonglen)+{+structaddress_space*mapping=iocb->common.ki_filp->f_mapping;+pgoff_tindex,end;+loff_tepos,isize;+intdo_io=0;++if(!mapping||!mapping->a_ops)+return-EBADF;+if(!mapping->a_ops->readpage&&!mapping->a_ops->readpages)+return-EBADF;+if(!len)+return0;++epos=iocb->common.ki_pos+len;+if(epos<0)+return-EINVAL;+isize=i_size_read(mapping->host);+if(isize<epos){+epos=isize-iocb->common.ki_pos;+if(epos<=0)+return0;+if((unsignedlong)epos!=epos)+return-EINVAL;+len=epos;+}++index=iocb->common.ki_pos>>PAGE_CACHE_SHIFT;+end=(iocb->common.ki_pos+len-1)>>PAGE_CACHE_SHIFT;+iocb->ki_data=len;+if(end<index)+return-EINVAL;++do{+structpage*page;++rcu_read_lock();+page=radix_tree_lookup(&mapping->page_tree,index);+rcu_read_unlock();++if(!page||radix_tree_exceptional_entry(page)||+!PageUptodate(page))+do_io=1;+}while(!do_io&&(index++<end));++if(do_io)+returnaio_thread_queue_iocb(iocb,aio_thread_op_readahead,0);+returnlen;+}#endif /* IS_ENABLED(CONFIG_AIO_THREAD) *//*
@@ -1922,6 +2056,13 @@ rw_common:ret=aio_foo_at(req,aio_do_unlinkat);break;+caseIOCB_CMD_READAHEAD:+if(user_iocb->aio_buf)+return-EINVAL;+if(aio_may_use_threads())+ret=aio_readahead(req,user_iocb->aio_nbytes);+break;+default:pr_debug("EINVAL: no operation provided\n");return-EINVAL;
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-11 22:08:05
Add support for an aio renameat operation that implements an
asynchronous renameat2().
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
fs/aio.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/aio_abi.h | 9 +++++++
2 files changed, 72 insertions(+)
@@ -240,6 +240,7 @@ long aio_do_unlinkat(int fd, const char *filename, int flags, int mode);longaio_foo_at(structaio_kiocb*req,do_foo_at_tdo_foo_at);longaio_readahead(structaio_kiocb*iocb,unsignedlonglen);+longaio_renameat(structaio_kiocb*iocb,structiocb*user_iocb);static__always_inlineboolaio_may_use_threads(void){
@@ -1946,6 +1947,63 @@ long aio_readahead(struct aio_kiocb *iocb, unsigned long len)returnaio_thread_queue_iocb(iocb,aio_thread_op_readahead,0);returnlen;}++staticlongaio_thread_op_renameat(structaio_kiocb*iocb)+{+constvoid*__useruser_info=(void*__user)iocb->common.private;+structrenameat_infoinfo;+constchar*__userold;+constchar*__usernew;+intolddir,newdir;+unsignedflags;+longret;++use_mm(aio_get_mm(&iocb->common));+if(unlikely(copy_from_user(&info,user_info,sizeof(info)))){+ret=-EFAULT;+gotodone;+}++old=(constchar*__user)(unsignedlong)info.oldpath;+new=(constchar*__user)(unsignedlong)info.newpath;+olddir=info.olddirfd;+newdir=info.newdirfd;+flags=info.flags;++if(((unsignedlong)old!=info.oldpath)||+((unsignedlong)new!=info.newpath)||+(olddir!=info.olddirfd)||+(newdir!=info.newdirfd)||+(flags!=info.flags))+ret=-EINVAL;+else+ret=sys_renameat2(olddir,old,newdir,new,flags);+done:+unuse_mm(aio_get_mm(&iocb->common));+returnret;+}++longaio_renameat(structaio_kiocb*iocb,structiocb*user_iocb)+{+constvoid*__useruser_info;++if(user_iocb->aio_nbytes!=sizeof(structrenameat_info))+return-EINVAL;+if(user_iocb->aio_offset)+return-EINVAL;++user_info=(constvoid*__user)user_iocb->aio_buf;+if(unlikely(!access_ok(VERIFY_READ,user_info,+sizeof(structrenameat_info))))+return-EFAULT;++iocb->common.private=(void*)user_info;+returnaio_thread_queue_iocb(iocb,aio_thread_op_renameat,+AIO_THREAD_NEED_TASK|+AIO_THREAD_NEED_FS|+AIO_THREAD_NEED_FILES|+AIO_THREAD_NEED_CRED);+}#endif /* IS_ENABLED(CONFIG_AIO_THREAD) *//*
@@ -2063,6 +2121,11 @@ rw_common:ret=aio_readahead(req,user_iocb->aio_nbytes);break;+caseIOCB_CMD_RENAMEAT:+if(aio_may_use_threads())+ret=aio_renameat(req,user_iocb);+break;+default:pr_debug("EINVAL: no operation provided\n");return-EINVAL;
--
2.5.0
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
On Mon, Jan 11, 2016 at 2:07 PM, Benjamin LaHaise [off-list ref] wrote:
Another blocking operation used by applications that want aio
functionality is that of opening files that are not resident in memory.
Using the thread based aio helper, add support for IOCB_CMD_OPENAT.
So I think this is ridiculously ugly.
AIO is a horrible ad-hoc design, with the main excuse being "other,
less gifted people, made that design, and we are implementing it for
compatibility because database people - who seldom have any shred of
taste - actually use it".
But AIO was always really really ugly.
Now you introduce the notion of doing almost arbitrary system calls
asynchronously in threads, but then you use that ass-backwards nasty
interface to do so.
Why?
If you want to do arbitrary asynchronous system calls, just *do* it.
But do _that_, not "let's extend this horrible interface in arbitrary
random ways one special system call at a time".
In other words, why is the interface not simply: "do arbitrary system
call X with arguments A, B, C, D asynchronously using a kernel
thread".
That's something that a lot of people might use. In fact, if they can
avoid the nasty AIO interface, maybe they'll even use it for things
like read() and write().
So I really think it would be a nice thing to allow some kind of
arbitrary "queue up asynchronous system call" model.
But I do not think the AIO model should be the model used for that,
even if I think there might be some shared infrastructure.
So I would seriously suggest:
- how about we add a true "asynchronous system call" interface
- make it be a list of system calls with a futex completion for each
list entry, so that you can easily wait for the end result that way.
- maybe (and this is where it gets really iffy) you could even pass
in the result of one system call to the next, so that you can do
things like
fd = openat(..)
ret = read(fd, ..)
asynchronously and then just wait for the read() to complete.
and let us *not* tie this to the aio interface.
In fact, if we do it well, we can go the other way, and try to
implement the nasty AIO interface on top of the generic "just do
things asynchronously".
And I actually think many of your kernel thread parts are good for a
generic implementation. That whole "AIO_THREAD_NEED_CRED" etc logic
all makes sense, although I do suspect you could just make it
unconditional. The cost of a few atomics shouldn't be excessive when
we're talking "use a thread to do op X".
What do you think? Do you think it might be possible to aim for a
generic "do system call asynchronously" model instead?
I'm adding Ingo the to cc, because I think Ingo had a "run this list
of system calls" patch at one point - in order to avoid system call
overhead. I don't think that was very interesting (because system call
overhead is seldom all that noticeable for any interesting system
calls), but with the "let's do the list asynchronously" addition it
might be much more intriguing. Ingo, do I remember correctly that it
was you? I might be confused about who wrote that patch, and I can't
find it now.
Linus
--
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>
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-12 01:11:28
On Mon, Jan 11, 2016 at 05:07:23PM -0500, Benjamin LaHaise wrote:
Enable a fully asynchronous fsync and fdatasync operations in aio using
the aio thread queuing mechanism.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
Insufficient. Needs the range to be passed through and call
vfs_fsync_range(), as I implemented here:
https://lkml.org/lkml/2015/10/28/878
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
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>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-12 01:17:39
On Mon, Jan 11, 2016 at 04:22:28PM -0800, Linus Torvalds wrote:
On Mon, Jan 11, 2016 at 2:07 PM, Benjamin LaHaise [off-list ref] wrote:
quoted
Another blocking operation used by applications that want aio
functionality is that of opening files that are not resident in memory.
Using the thread based aio helper, add support for IOCB_CMD_OPENAT.
So I think this is ridiculously ugly.
AIO is a horrible ad-hoc design, with the main excuse being "other,
less gifted people, made that design, and we are implementing it for
compatibility because database people - who seldom have any shred of
taste - actually use it".
But AIO was always really really ugly.
Now you introduce the notion of doing almost arbitrary system calls
asynchronously in threads, but then you use that ass-backwards nasty
interface to do so.
Why?
Understood, but there are some reasons behind this. The core aio submit
mechanism is modeled after the lio_listio() call in posix. While the
cost of performing syscalls has decreased substantially over the last 10
years, the cost of context switches has not. Some AIO operations really
want to do part of the work in the context of the original submitter for
the work. That was/is a critical piece of the async readahead
functionality in this series -- without being able to do a quick return
to the caller when all the cached data is allready resident in the
kernel, there is a significant performance degradation in my tests. For
other operations which are going to do blocking i/o anyways, the cost of
the context switch often becomes noise.
The async readahead also fills a fills a hole in the proposed extensions
to preadv()/pwritev() -- they need some way to trigger and know when a
readahead operation has completed. One needs a completion queue of some
sort to figure out which operation has completed in a reasonable
efficient manner. The futex doesn't really have the ability to do this.
Thread dispatching is another problem the applications I work on
encounter, and AIO helps in this particular area because a thread that
is running hot can simply check the AIO event ring buffer for new events
in its main event loop. Userspace fundamentally *cannot* do a good job of
dispatching work to threads. The code I've see other developers come up
with ends up doing things like epoll() in one thread followed by
dispatching the receieved events to different threads. This ends up
making multiple expensive syscalls (since locking and cross CPU bouncing
is required) when the kernel could just direct things to the right
thread in the first place.
There are a lot of requirements bringing additional complexity that start
to surface once you look at how some of these applications are actually
written.
If you want to do arbitrary asynchronous system calls, just *do* it.
But do _that_, not "let's extend this horrible interface in arbitrary
random ways one special system call at a time".
In other words, why is the interface not simply: "do arbitrary system
call X with arguments A, B, C, D asynchronously using a kernel
thread".
We've had a few proposals to do this, none of which have really managed
to tackle all the problems that arose. If we go down this path, we will
end up needing a table of what syscalls can actually be performed
asynchronously, and flags indicating what bits of context those syscalls
require. This does end up looking a bit like how AIO does things
depending on how hard you squint.
I'm not opposed to reworking how AIO dispatches things. If we're willing
to relax some constraints (like the hard enforced limits on the number
of AIOs in flight), things can be substantially simplified. Again,
worries about things like memory usage today are vastly different than
they were back in the early '00s, so the decisions that make sense now
will certainly change the design.
Cancellation is also a concern. Cancellation is not something that can
be sacrificed. Without some mechanism to cancel operations that are in
flight, there is no way for a process to cleanly exit. This patch
series nicely proves that signals work very well for cancellation, and
fit in with a lot of the code we already have. This implies we would
need to treat threads doing async operations differently from normal
threads. What happens with the pid namespace?
That's something that a lot of people might use. In fact, if they can
avoid the nasty AIO interface, maybe they'll even use it for things
like read() and write().
So I really think it would be a nice thing to allow some kind of
arbitrary "queue up asynchronous system call" model.
But I do not think the AIO model should be the model used for that,
even if I think there might be some shared infrastructure.
So I would seriously suggest:
- how about we add a true "asynchronous system call" interface
- make it be a list of system calls with a futex completion for each
list entry, so that you can easily wait for the end result that way.
- maybe (and this is where it gets really iffy) you could even pass
in the result of one system call to the next, so that you can do
things like
fd = openat(..)
ret = read(fd, ..)
asynchronously and then just wait for the read() to complete.
and let us *not* tie this to the aio interface.
In fact, if we do it well, we can go the other way, and try to
implement the nasty AIO interface on top of the generic "just do
things asynchronously".
And I actually think many of your kernel thread parts are good for a
generic implementation. That whole "AIO_THREAD_NEED_CRED" etc logic
all makes sense, although I do suspect you could just make it
unconditional. The cost of a few atomics shouldn't be excessive when
we're talking "use a thread to do op X".
What do you think? Do you think it might be possible to aim for a
generic "do system call asynchronously" model instead?
Maybe it's not too bad to do -- the syscall() primitive is reasonably
well defined and is supported across architectures, but we're going to
need new wrappers for *every* syscall supported. Odds are the work will
have to be done incrementally to weed out which syscalls are safe and
which are not, but there is certainly no reason we can't reuse syscall
numbers and the same argument layout.
Chaining things becomes messy. There are some cases where that works,
but at least on the applications I've worked on, there tends to be a
fair amount of logic that needs to be run before you can figure out what
and where the next operation is. The canonical example I can think of
is the case where one is retreiving data from disk. The first operation
is a read into some table to find out where data is located, the next
operation is a search (binary search in the case I'm thinking of) in the
data that was just read to figure out which record actually contains the
data the app cares about, followed by a read to actually fetch the data
the user actually requires.
And it gets more complicated: different disk i/os need to be issued with
different priorities (something that was not included in what I just
posted today, but is work I plan to propose for merging in the future).
In some cases the priority is known beforehand, but in other cases it
needs to be adjusted dynamically depending on information fetched (users
don't like it if huge i/os completely starve their smaller i/os for
significant amounts of time).
I'm adding Ingo the to cc, because I think Ingo had a "run this list
of system calls" patch at one point - in order to avoid system call
overhead. I don't think that was very interesting (because system call
overhead is seldom all that noticeable for any interesting system
calls), but with the "let's do the list asynchronously" addition it
might be much more intriguing. Ingo, do I remember correctly that it
was you? I might be confused about who wrote that patch, and I can't
find it now.
I'd certainly be interested in hearing more ideas concerning
requirements.
Sorry for the giant wall of text... Nothing is simple! =-)
-ben
Linus
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
On Mon, Jan 11, 2016 at 5:11 PM, Dave Chinner [off-list ref] wrote:
Insufficient. Needs the range to be passed through and call
vfs_fsync_range(), as I implemented here:
And I think that's insufficient *also*.
What you actually want is "sync_file_range()", with the full set of arguments.
Yes, really. Sometimes you want to start the writeback, sometimes you
want to wait for it. Sometimes you want both.
For example, if you are doing your own manual write-behind logic, it
is not sufficient for "wait for data". What you want is "start IO on
new data" followed by "wait for old data to have been written out".
I think this only strengthens my "stop with the idiotic
special-case-AIO magic already" argument. If we want something more
generic than the usual aio, then we should go all in. Not "let's make
more limited special cases".
Linus
--
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>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-12 01:30:18
On Tue, Jan 12, 2016 at 12:11:28PM +1100, Dave Chinner wrote:
On Mon, Jan 11, 2016 at 05:07:23PM -0500, Benjamin LaHaise wrote:
quoted
Enable a fully asynchronous fsync and fdatasync operations in aio using
the aio thread queuing mechanism.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
Insufficient. Needs the range to be passed through and call
vfs_fsync_range(), as I implemented here:
Please at least Cc the aio list in the future on aio patches, as I do
not have the time to read linux-kernel these days unless prodded to do
so...
-ben
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Chris Mason <hidden> Date: 2016-01-12 01:45:09
On Mon, Jan 11, 2016 at 04:22:28PM -0800, Linus Torvalds wrote:
On Mon, Jan 11, 2016 at 2:07 PM, Benjamin LaHaise [off-list ref] wrote:
quoted
Another blocking operation used by applications that want aio
functionality is that of opening files that are not resident in memory.
Using the thread based aio helper, add support for IOCB_CMD_OPENAT.
So I think this is ridiculously ugly.
AIO is a horrible ad-hoc design, with the main excuse being "other,
less gifted people, made that design, and we are implementing it for
compatibility because database people - who seldom have any shred of
taste - actually use it".
But AIO was always really really ugly.
Now you introduce the notion of doing almost arbitrary system calls
asynchronously in threads, but then you use that ass-backwards nasty
interface to do so.
[ ... ]
I'm adding Ingo the to cc, because I think Ingo had a "run this list
of system calls" patch at one point - in order to avoid system call
overhead. I don't think that was very interesting (because system call
overhead is seldom all that noticeable for any interesting system
calls), but with the "let's do the list asynchronously" addition it
might be much more intriguing. Ingo, do I remember correctly that it
was you? I might be confused about who wrote that patch, and I can't
find it now.
Zach Brown and Ingo traded a bunch of ideas. There were chicklets and
syslets? A little search, it looks like acall was a slightly different
iteration, but the patches didn't make it off oss.oracle.com:
https://lwn.net/Articles/316806/
-chris
--
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>
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-12 02:25:48
On Mon, Jan 11, 2016 at 05:20:42PM -0800, Linus Torvalds wrote:
On Mon, Jan 11, 2016 at 5:11 PM, Dave Chinner [off-list ref] wrote:
quoted
Insufficient. Needs the range to be passed through and call
vfs_fsync_range(), as I implemented here:
And I think that's insufficient *also*.
What you actually want is "sync_file_range()", with the full set of arguments.
That's a different interface. the aio fsync interface has been
exposed to userspace for years, we just haven't implemented it in
the kernel. That's a major difference to everything else being
proposed in this patch set, especially this one.
FYI sync_file_range() is definitely not a fsync/fdatasync
replacement as it does not guarantee data durability in any way.
i.e. you can call sync_file_range, have it wait for data to be
written, return to userspace, then lose power and lose the data that
sync_file_range said it wrote. That's because sync_file_range()
does not:
a) write the metadata needed to reference the data to disk;
and
b) flush volatile storage caches after data and metadata is
written.
Hence sync_file_range is useless to applications that need to
guarantee data durability. Not to mention that most AIO applications
use direct IO, and so have no use for fine grained control over page
cache writeback semantics. They only require a) and b) above, so
implementing the AIO fsync primitive is exactly what they want.
Yes, really. Sometimes you want to start the writeback, sometimes you
want to wait for it. Sometimes you want both.
Without durability guarantees such application level optimisations
are pretty much worthless.
I think this only strengthens my "stop with the idiotic
special-case-AIO magic already" argument. If we want something more
generic than the usual aio, then we should go all in. Not "let's make
more limited special cases".
No, I don't think this specific case does, because the AIO fsync
interface already exists....
Cheers,
Dave.
--
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
On Mon, Jan 11, 2016 at 6:25 PM, Dave Chinner [off-list ref] wrote:
That's a different interface.
So is openat. So is readahead.
My point is that this idiotic "let's expose special cases" must end.
It's broken. It inevitably only exposes a subset of what different
people would want.
Making "aio_read()" and friends a special interface had historical
reasons for it. But expanding willy-nilly on that model does not.
Linus
--
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>
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-12 03:37:08
On Mon, Jan 11, 2016 at 06:38:15PM -0800, Linus Torvalds wrote:
On Mon, Jan 11, 2016 at 6:25 PM, Dave Chinner [off-list ref] wrote:
quoted
That's a different interface.
So is openat. So is readahead.
My point is that this idiotic "let's expose special cases" must end.
It's broken. It inevitably only exposes a subset of what different
people would want.
Making "aio_read()" and friends a special interface had historical
reasons for it. But expanding willy-nilly on that model does not.
Yes, I heard you the first time, but you haven't acknowledged that
the aio fsync interface is indeed different because it already
exists. What's the problem with implementing an AIO call that we've
advertised as supported for many years now that people are asking us
to implement it?
As for a generic async syscall interface, why not just add
IOCB_CMD_SYSCALL that encodes the syscall number and parameters
into the iovec structure and let the existing aio subsystem handle
demultiplexing it and handing them off to threads/workqueues/etc?
That was we get contexts, events, signals, completions,
cancelations, etc from the existing infrastructure, and there's
really only a dispatch/collection layer that needs to be added?
If we then provide the userspace interface via the libaio library to
call the async syscalls with an AIO context handle, then there's
little more that needs to be done to support just about everything
as an async syscall...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
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>
On Mon, Jan 11, 2016 at 7:37 PM, Dave Chinner [off-list ref] wrote:
Yes, I heard you the first time, but you haven't acknowledged that
the aio fsync interface is indeed different because it already
exists. What's the problem with implementing an AIO call that we've
advertised as supported for many years now that people are asking us
to implement it?
Oh, I don't disagree with that. I think it should be exposed, my point
was that that too was not enough.
I don't see why you argue. You said "that's not enough". And I jjust
said that your expansion wasn't sufficient either, and that I think we
should strive to expand things even more.
And preferably not in some ad-hoc manner. Expand it to *everything* we can do.
As for a generic async syscall interface, why not just add
IOCB_CMD_SYSCALL that encodes the syscall number and parameters
into the iovec structure and let the existing aio subsystem handle
demultiplexing it and handing them off to threads/workqueues/etc?
That would likely be the simplest approach, yes.
There's a few arguments against it, though:
- doing the indirect system call thing does end up being
architecture-specific, so now you do need the AIO code to call into
some arch wrapper.
Not a huge deal, since the arch wrapper will be pretty simple (and
we can have a default one that just returns ENOSYS, so that we don't
have to synchronize all architectures)
- the aio interface really is horrible crap. Really really.
For example, the whole "send signal as a completion model" is so
f*cking broken that I really don't want to extend the aio interface
too much. I think it's unfixable.
So I really think we'd be *much* better off with a new interface
entirely - preferably one that allows the old aio interfaces to fall
out fairly naturally.
Ben mentioned lio_listio() as a reason for why he wanted to extend the
AIO interface, but I think it works the other way around: yes, we
should look at lio_listio(), but we should look at it mainly as a way
to ask ourselves: "can we implement a new aynchronous system call
submission model that would also make it possible to implement
lio_listio() as a user space wrapper around it".
For example, if we had an actual _good_ way to queue up things, you
could probably make that "struct sigevent" completion for lio_listio()
just be another asynchronous system call at the end of the list - a
system call that sends the completion signal. And the aiocb_list[]
itself? Maybe those could just be done as normal (individual) aio
calls (so that you end up having the aiocb that you can wait on with
aio_suspend() etc).
But then people who do *not* want the crazy aiocb, and do *not* want
some SIGIO or whatever, could just fire off asynchronous system calls
without that cruddy interface.
So my argument is really that I think it would be better to at least
look into maybe creating something less crapulent, and striving to
make it easy to make the old legacy interfaces be just wrappers around
a more capable model.
And hey, it may be that in the end nobody cares enough, and the right
thing (or at least the prudent thing) to do is to just pile the crap
on deeper and higher, and just add a single IOCB_CMD_SYSCALL
indirection entry.
So I'm not dismissing that as a solution - I just don't think it's a
particularly clean one.
It does have the advantage of likely being a fairly simple hack. But
it smells like a hack.
Linus
--
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>
On Mon, Jan 11, 2016 at 8:03 PM, Linus Torvalds
[off-list ref] wrote:
So my argument is really that I think it would be better to at least
look into maybe creating something less crapulent, and striving to
make it easy to make the old legacy interfaces be just wrappers around
a more capable model.
Hmm. Thinking more about this makes me worry about all the system call
versioning and extra work done by libc.
At least glibc has traditionally decided to munge and extend on kernel
system call interfaces, to the point where even fairly core data
structures (like "struct stat") may not always look the same to the
kernel as they do to user space.
So with that worry, I have to admit that maybe a limited interface -
rather than allowing arbitrary generic async system calls - might have
advantages. Less room for mismatches.
I'll have to think about this some more.
Linus
--
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>
What do you think? Do you think it might be possible to aim for a generic "do
system call asynchronously" model instead?
I'm adding Ingo the to cc, because I think Ingo had a "run this list of system
calls" patch at one point - in order to avoid system call overhead. I don't
think that was very interesting (because system call overhead is seldom all that
noticeable for any interesting system calls), but with the "let's do the list
asynchronously" addition it might be much more intriguing. Ingo, do I remember
correctly that it was you? I might be confused about who wrote that patch, and I
can't find it now.
Yeah, it was the whole 'syslets' and 'threadlets' stuff - I had both implemented
and prototyped into a 'list directory entries asynchronously' testcase.
Threadlets was pretty close to what you are suggesting now. Here's a very good (as
usual!) writeup from LWN:
https://lwn.net/Articles/223899/
Thanks,
Ingo
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-12 22:50:11
On Mon, Jan 11, 2016 at 08:48:23PM -0800, Linus Torvalds wrote:
On Mon, Jan 11, 2016 at 8:03 PM, Linus Torvalds
[off-list ref] wrote:
quoted
So my argument is really that I think it would be better to at least
look into maybe creating something less crapulent, and striving to
make it easy to make the old legacy interfaces be just wrappers around
a more capable model.
Hmm. Thinking more about this makes me worry about all the system call
versioning and extra work done by libc.
That is one of my worries, and one of the reasons an async getdents64()
or readdir() operation isn't in this batch -- there are a ton of ABI
issues glibc handles on some platforms.
At least glibc has traditionally decided to munge and extend on kernel
system call interfaces, to the point where even fairly core data
structures (like "struct stat") may not always look the same to the
kernel as they do to user space.
So with that worry, I have to admit that maybe a limited interface -
rather than allowing arbitrary generic async system calls - might have
advantages. Less room for mismatches.
I'll have to think about this some more.
Linus
I think some cleanups can be made on how and where the AIO operations
are implemented. A first stab is below (not very tested as of yet,
still have more work to do) that uses an array to dispatch AIO submits.
By using function pointers to dispatch the operations fairly early in
the process, the code that actually does the required verifications is
less spread out and much easier to follow instead of the giant select
cases.
Another possible improvement might be to move things like aio_fsync()
into sync.c with all the other relevant sync code. That would make much
more sense and make it much more obvious as to which subsystem
maintainers a given set of functionality really belongs. If that sounds
like an improvement, I can put some effort into that as well.
-ben
aio.c | 242 ++++++++++++++++++++++++++++++++----------------------------------
1 file changed, 118 insertions(+), 124 deletions(-)
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-01-12 22:59:23
On Jan 11, 2016 8:04 PM, "Linus Torvalds" [off-list ref] wrote:
On Mon, Jan 11, 2016 at 7:37 PM, Dave Chinner [off-list ref] wrote:
quoted
Yes, I heard you the first time, but you haven't acknowledged that
the aio fsync interface is indeed different because it already
exists. What's the problem with implementing an AIO call that we've
advertised as supported for many years now that people are asking us
to implement it?
Oh, I don't disagree with that. I think it should be exposed, my point
was that that too was not enough.
I don't see why you argue. You said "that's not enough". And I jjust
said that your expansion wasn't sufficient either, and that I think we
should strive to expand things even more.
And preferably not in some ad-hoc manner. Expand it to *everything* we can do.
quoted
As for a generic async syscall interface, why not just add
IOCB_CMD_SYSCALL that encodes the syscall number and parameters
into the iovec structure and let the existing aio subsystem handle
demultiplexing it and handing them off to threads/workqueues/etc?
That would likely be the simplest approach, yes.
There's a few arguments against it, though:
- doing the indirect system call thing does end up being
architecture-specific, so now you do need the AIO code to call into
some arch wrapper.
How many arches *can* do it? As of 4.4, x86_32 can, but x86_64 can't
yet. We'd also need a whitelist of acceptable indirect syscalls (e.g.
exit is bad). And we have to worry about things that depend on the mm
or creds.
It would be extra nice if we could avoid switch_mm for things that
don't need it (fsync) and only do it for things like read that do.
--Andy
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2016-01-14 09:19:56
On 12/01/2016 02:20, Linus Torvalds wrote:
On Mon, Jan 11, 2016 at 5:11 PM, Dave Chinner [off-list ref] wrote:
quoted
Insufficient. Needs the range to be passed through and call
vfs_fsync_range(), as I implemented here:
And I think that's insufficient *also*.
What you actually want is "sync_file_range()", with the full set of arguments.
Yes, really. Sometimes you want to start the writeback, sometimes you
want to wait for it. Sometimes you want both.
For example, if you are doing your own manual write-behind logic, it
is not sufficient for "wait for data". What you want is "start IO on
new data" followed by "wait for old data to have been written out".
I think this only strengthens my "stop with the idiotic
special-case-AIO magic already" argument. If we want something more
generic than the usual aio, then we should go all in. Not "let's make
more limited special cases".
The question is, do we really want something more generic than the usual
AIO?
Virt is one of the 10 (that's a binary number) users of AIO, and we
don't even use it by default because in most cases it's really a wash.
Let's compare AIO with a simple userspace thread pool.
AIO has the ability to submit and retrieve the results of multiple
operations at once. Thread pools do not have the ability to submit
multiple operations at a time (you could play games with FUTEX_WAKE, but
then all the threads in the pool would have cacheline bounces on the futex).
The syscall overhead on the critical path is comparable. For AIO it's
io_submit+io_getevents, for a thread pool it's FUTEX_WAKE plus invoking
the actual syscall. Again, the only difference for AIO is batching.
Unless userspace is submitting tens of thousands of operations per
second, which is pretty much the case only for read/write, there's no
real benefit in asynchronous system calls over a userspace thread pool.
That applies to openat, unlinkat, fadvise (for readahead). It also
applies to msync and fsync, etc. because if your workload is doing tons
of those you'd better buy yourself a disk with a battery-backed cache,
or an UPS, and remove the msync/fsync altogether.
So I'm really happy if we can move the thread creation overhead for such
a thread pool to the kernel. It keeps the benefits of batching, it uses
the optimized kernel workqueues, it doesn't incur the cost of pthreads,
it makes it easy to remove the cases where AIO is blocking, it makes it
easy to add support for !O_DIRECT. But everything else seems overkill.
Paolo
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-15 20:21:31
On Mon, Jan 11, 2016 at 08:48:23PM -0800, Linus Torvalds wrote:
On Mon, Jan 11, 2016 at 8:03 PM, Linus Torvalds
[off-list ref] wrote:
quoted
So my argument is really that I think it would be better to at least
look into maybe creating something less crapulent, and striving to
make it easy to make the old legacy interfaces be just wrappers around
a more capable model.
Hmm. Thinking more about this makes me worry about all the system call
versioning and extra work done by libc.
At least glibc has traditionally decided to munge and extend on kernel
system call interfaces, to the point where even fairly core data
structures (like "struct stat") may not always look the same to the
kernel as they do to user space.
So with that worry, I have to admit that maybe a limited interface -
rather than allowing arbitrary generic async system calls - might have
advantages. Less room for mismatches.
I'll have to think about this some more.
Any further thoughts on this after a few days worth of pondering?
-ben
On Fri, Jan 15, 2016 at 12:21 PM, Benjamin LaHaise [off-list ref] wrote:
quoted
I'll have to think about this some more.
Any further thoughts on this after a few days worth of pondering?
Sorry about the delay, with the merge window and me being sick for a
couple of days I didn't get around to this.
After thinking it over some more, I guess I'm ok with your approach.
The table-driven patch makes me a bit happier, and I guess not very
many people end up ever wanting to do async system calls anyway.
Are there other users outside of Solace? It would be good to get comments..
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
On Tue, Jan 19, 2016 at 07:59:35PM -0800, Linus Torvalds wrote:
After thinking it over some more, I guess I'm ok with your approach.
The table-driven patch makes me a bit happier, and I guess not very
many people end up ever wanting to do async system calls anyway.
Are there other users outside of Solace? It would be good to get comments..
For async I/O? We're using it inside Google, for networking and for
storage I/O's. We don't need async fsync/fdatasync, but we do need
very fast, low overhead I/O's. To that end, we have some patches to
batch block layer completion handling, which Kent tried upstreaming a
few years back but which everyone thought was too ugly to live.
(It *was* ugly, but we had access to some very fast storage devices
where it really mattered. With upcoming NVMe devices, that sort of
hardware should be available to more folks, so it's something that
I've been meaning to revisit from an upstreaming perspective,
especially if I can get my hands on some publically available hardware
for benchmarking purposes to demonstrate why it's useful, even if it
is ugly.)
The other thing which we have which is a bit more experimental is that
we've plumbed through the aio priority bits to the block layer, as
well as aio_cancel. The idea for the latter is if you are are
interested in low latency access to a clustered file system, where
sometimes a read request can get stuck behind other I/O requests if a
server has a long queue of requests to service. So the client for
which low latency is very important fires off the request to more than
one server, and as soon as it gets an answer it sends a "never mind"
message to the other server(s).
The code to do aio_cancellation in the block layer is fairly well
tested, and was in Kent's git trees, but never got formally pushed
upstream. The code to push the cancellation request all the way to
the HDD (for those hard disks / storage devices that support I/O
cancellation) is even more experimental, and needs a lot of cleanup
before it could be sent for review (it was done by someone who isn't
used to upstream coding standards).
The reason why we haven't tried to pushed more of these changes
upsream has been lack of resources, and the fact that the AIO code
*is* ugly, which means extensions tend to make the code at the very
least, more complex. Especially since some of the folks working on
it, such as Kent, were really worried about performance at all costs,
and Kerningham's "it's twice as hard to debug code as to write it"
comment really applies here. And since very few people outside of
Google seem to use AIO, and even fewer seem eager to review or work on
AIO, and our team is quite small for the work we need to do, it just
hasn't risen to the top of the priority list.
Still, it's fair to say that if you are using Google Hangouts, or
Google Mail, or Google Docs, AIO is most definitely getting used to
process your queries.
As far as comments, aside from the "we really care about performance",
and "the code is scary complex and barely on the edge of being
maintainable", the other comment I'd make is libaio is pretty awful,
and so as a result a number (most?) of our AIO users have elected to
use the raw system call interfaces and are *not* using the libaio
abstractions --- which, as near as I can tell, don't really buy you
much anyway. (Do we really need to keep code that provides backwards
compatibility with kernels over 10+ years old at this point?)
Cheers,
- Ted
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-20 19:59:57
On Tue, Jan 19, 2016 at 07:59:35PM -0800, Linus Torvalds wrote:
On Fri, Jan 15, 2016 at 12:21 PM, Benjamin LaHaise [off-list ref] wrote:
quoted
quoted
I'll have to think about this some more.
Any further thoughts on this after a few days worth of pondering?
Sorry about the delay, with the merge window and me being sick for a
couple of days I didn't get around to this.
After thinking it over some more, I guess I'm ok with your approach.
The table-driven patch makes me a bit happier, and I guess not very
many people end up ever wanting to do async system calls anyway.
Are there other users outside of Solace? It would be good to get comments..
I know of quite a few storage/db products that use AIO. The most
recent high profile project that have been reporting issues with AIO
on XFS is http://www.scylladb.com/. That project is architected
around non-blocking AIO for scalability reasons...
Cheers,
Dave.
--
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
On Wed, Jan 20, 2016 at 11:59 AM, Dave Chinner [off-list ref] wrote:
quoted
Are there other users outside of Solace? It would be good to get comments..
I know of quite a few storage/db products that use AIO. The most
recent high profile project that have been reporting issues with AIO
on XFS is http://www.scylladb.com/. That project is architected
around non-blocking AIO for scalability reasons...
I was more wondering about the new interfaces, making sure that the
feature set actually matches what people want to do..
That said, I also agree that it would be interesting to hear what the
performance impact is for existing performance-sensitive users. Could
we make that "aio_may_use_threads()" case be unconditional, making
things simpler?
Linus
--
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>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-20 20:44:49
On Wed, Jan 20, 2016 at 12:29:32PM -0800, Linus Torvalds wrote:
On Wed, Jan 20, 2016 at 11:59 AM, Dave Chinner [off-list ref] wrote:
quoted
quoted
Are there other users outside of Solace? It would be good to get comments..
I know of quite a few storage/db products that use AIO. The most
recent high profile project that have been reporting issues with AIO
on XFS is http://www.scylladb.com/. That project is architected
around non-blocking AIO for scalability reasons...
I was more wondering about the new interfaces, making sure that the
feature set actually matches what people want to do..
I suspect this will be an ongoing learning exercise as people start to use
the new functionality and find gaps in terms of what is needed. Certainly
there is a bunch of stuff we need to add to cover the cases where disk i/o
is required. getdents() is one example, but the ABI issues we have with it
are somewhat more complicated given the history associated with that
interface.
That said, I also agree that it would be interesting to hear what the
performance impact is for existing performance-sensitive users. Could
we make that "aio_may_use_threads()" case be unconditional, making
things simpler?
Making it unconditional is a goal, but some work is required before that
can be the case. The O_DIRECT issue is one such matter -- it requires some
changes to the filesystems to ensure that they adhere to the non-blocking
nature of the new interface (ie taking i_mutex is a Bad Thing that users
really do not want to be exposed to; if taking it blocks, the code should
punt to a helper thread). Additional auditing of some of the read/write
implementations is also required, which will likely need some minor changes
in things like sysfs and other weird functionality we have. Having the
flag reflects that while the functionality is useful, not all of the bugs
have been worked out yet.
What's the desired approach to merge these changes? Does it make sense
to merge what is ready now and prepare the next round of changes for 4.6?
Or is it more important to grow things to a more complete state before
merging?
Regards,
-ben
Linus
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-20 21:45:46
On Wed, Jan 20, 2016 at 03:44:49PM -0500, Benjamin LaHaise wrote:
On Wed, Jan 20, 2016 at 12:29:32PM -0800, Linus Torvalds wrote:
quoted
On Wed, Jan 20, 2016 at 11:59 AM, Dave Chinner [off-list ref] wrote:
quoted
quoted
Are there other users outside of Solace? It would be good to get comments..
I know of quite a few storage/db products that use AIO. The most
recent high profile project that have been reporting issues with AIO
on XFS is http://www.scylladb.com/. That project is architected
around non-blocking AIO for scalability reasons...
I was more wondering about the new interfaces, making sure that the
feature set actually matches what people want to do..
I suspect this will be an ongoing learning exercise as people start to use
the new functionality and find gaps in terms of what is needed. Certainly
there is a bunch of stuff we need to add to cover the cases where disk i/o
is required. getdents() is one example, but the ABI issues we have with it
are somewhat more complicated given the history associated with that
interface.
quoted
That said, I also agree that it would be interesting to hear what the
performance impact is for existing performance-sensitive users. Could
we make that "aio_may_use_threads()" case be unconditional, making
things simpler?
Making it unconditional is a goal, but some work is required before that
can be the case. The O_DIRECT issue is one such matter -- it requires some
changes to the filesystems to ensure that they adhere to the non-blocking
nature of the new interface (ie taking i_mutex is a Bad Thing that users
really do not want to be exposed to; if taking it blocks, the code should
punt to a helper thread).
Filesystems *must take locks* in the IO path. We have to serialise
against truncate and other operations at some point in the IO path
(e.g. block mapping vs concurrent allocation and/or removal), and
that can only be done sanely with sleeping locks. There is no way
of knowing in advance if we are going to block, and so either we
always use threads for IO submission or we accept that occasionally
the AIO submission will block.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-20 21:56:30
On Thu, Jan 21, 2016 at 08:45:46AM +1100, Dave Chinner wrote:
Filesystems *must take locks* in the IO path. We have to serialise
against truncate and other operations at some point in the IO path
(e.g. block mapping vs concurrent allocation and/or removal), and
that can only be done sanely with sleeping locks. There is no way
of knowing in advance if we are going to block, and so either we
always use threads for IO submission or we accept that occasionally
the AIO submission will block.
I never said we don't take locks. Still, we can be more intelligent
about when and where we do so. With the nonblocking pread() and pwrite()
changes being proposed elsewhere, we can do the part of the I/O that
doesn't block in the submitter, which is a huge win when possible.
As it stands today, *every* buffered write takes i_mutex immediately
on entering ->write(). That one issue alone accounts for a nearly 10x
performance difference between an O_SYNC write and an O_DIRECT write,
and using O_SYNC writes is a legitimate use-case for users who want
caching of data by the kernel (duplicating that functionality is a huge
amount of work for an application, plus if you want the cache to be
persistent between runs of an app, you have to get the kernel to do it).
-ben
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-20 21:57:03
On Wed, Jan 20, 2016 at 12:29:32PM -0800, Linus Torvalds wrote:
On Wed, Jan 20, 2016 at 11:59 AM, Dave Chinner [off-list ref] wrote:
quoted
quoted
Are there other users outside of Solace? It would be good to get comments..
I know of quite a few storage/db products that use AIO. The most
recent high profile project that have been reporting issues with AIO
on XFS is http://www.scylladb.com/. That project is architected
around non-blocking AIO for scalability reasons...
I was more wondering about the new interfaces, making sure that the
feature set actually matches what people want to do..
Well, they have mentioned that openat() can block, as will the first
operation after open that requires reading the file extent map from
disk. There are some ways of hacking around this (e.g. running
FIEMAP with a zero extent count or ext4's special extent prefetch
ioctl in a separate thread to prefetch the extent list into memory
before IO is required) so I suspect we may actually need some
interfaces that don't current exist at all....
That said, I also agree that it would be interesting to hear what the
performance impact is for existing performance-sensitive users. Could
we make that "aio_may_use_threads()" case be unconditional, making
things simpler?
That would make things a lot simpler in the kernel and AIO
submission a lot more predictable/deterministic for userspace. I'd
suggest that, at minimum, it should be the default behaviour...
Cheers,
Dave.
--
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
On Jan 20, 2016 1:46 PM, "Dave Chinner" [off-list ref] wrote:
quoted
quoted
That said, I also agree that it would be interesting to hear what the
performance impact is for existing performance-sensitive users. Could
we make that "aio_may_use_threads()" case be unconditional, making
things simpler?
Making it unconditional is a goal, but some work is required before that
can be the case. The O_DIRECT issue is one such matter -- it requires
some
quoted
changes to the filesystems to ensure that they adhere to the
non-blocking
quoted
nature of the new interface (ie taking i_mutex is a Bad Thing that users
really do not want to be exposed to; if taking it blocks, the code
should
quoted
punt to a helper thread).
Filesystems *must take locks* in the IO path.
I agree.
I also would prefer to make the aio code have as little interaction and
magic flags with the filesystem code as humanly possible.
I wonder if we could make the rough rule be that the only synchronous case
the aio code ever has is more or less entirely in the generic vfs caches?
IOW, could we possibly aim to make the rule be that if we call down to the
filesystem layer, we do that within a thread?
We could do things like that for the name loopkup for openat() too, where
we could handle the successful RCU loopkup synchronously, but then if we
fall out of RCU mode we'd do the thread.
Linus
From: Andres Freund <hidden> Date: 2016-01-22 15:31:55
On 2016-01-12 12:11:28 +1100, Dave Chinner wrote:
On Mon, Jan 11, 2016 at 05:07:23PM -0500, Benjamin LaHaise wrote:
quoted
Enable a fully asynchronous fsync and fdatasync operations in aio using
the aio thread queuing mechanism.
Signed-off-by: Benjamin LaHaise <redacted>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
FWIW, I finally started to play around with this (or more precisely
https://lkml.org/lkml/2015/10/29/517). There were some prerequisite
changes in postgres required, to actually be able to benefit, delaying
things. First results are good, increasing OLTP throughput
considerably.
It'd also be rather helpful to be able to do
sync_file_range(SYNC_FILE_RANGE_WRITE) asynchronously, i.e. flush
without an implied barrier. Currently this blocks very frequently, even
if there's actually IO bandwidth available.
Regards,
Andres
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Andres Freund <hidden> Date: 2016-01-22 15:41:08
On 2016-01-19 19:59:35 -0800, Linus Torvalds wrote:
Are there other users outside of Solace? It would be good to get comments..
PostgreSQL is a potential user of async fdatasync, fsync,
sync_file_range and potentially readahead, write, read. First tests with
Dave's async fsync/fsync_range are positive, so are the results with a
self-hacked async sync_file_range (although I'm kinda thinking that it
shouldn't really require to be used asynchronously).
I rather doubt openat, unlink et al are going to be interesting for
*us*, the requires structural changes would be too bit. But obviously
that doesn't mean anything for others.
Andres
--
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>
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-23 04:24:49
On Wed, Jan 20, 2016 at 04:56:30PM -0500, Benjamin LaHaise wrote:
On Thu, Jan 21, 2016 at 08:45:46AM +1100, Dave Chinner wrote:
quoted
Filesystems *must take locks* in the IO path. We have to serialise
against truncate and other operations at some point in the IO path
(e.g. block mapping vs concurrent allocation and/or removal), and
that can only be done sanely with sleeping locks. There is no way
of knowing in advance if we are going to block, and so either we
always use threads for IO submission or we accept that occasionally
the AIO submission will block.
I never said we don't take locks. Still, we can be more intelligent
about when and where we do so. With the nonblocking pread() and pwrite()
changes being proposed elsewhere, we can do the part of the I/O that
doesn't block in the submitter, which is a huge win when possible.
As it stands today, *every* buffered write takes i_mutex immediately
on entering ->write(). That one issue alone accounts for a nearly 10x
performance difference between an O_SYNC write and an O_DIRECT write,
Yes, that locking is for correct behaviour, not for performance
reasons. The i_mutex is providing the required semantics for POSIX
write(2) functionality - writes must serialise against other reads
and writes so that they are completed atomically w.r.t. other IO.
i.e. writes to the same offset must not interleave, not should reads
be able to see partial data from a write in progress.
Direct IO does not conform to POSIX concurrency standards, so we
don't have to serialise concurrent IO against each other.
and using O_SYNC writes is a legitimate use-case for users who want
caching of data by the kernel (duplicating that functionality is a huge
amount of work for an application, plus if you want the cache to be
persistent between runs of an app, you have to get the kernel to do it).
Yes, but you take what you get given. Buffered IO sucks in many ways;
this is just one of them.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-23 04:39:22
On Wed, Jan 20, 2016 at 03:07:26PM -0800, Linus Torvalds wrote:
On Jan 20, 2016 1:46 PM, "Dave Chinner" [off-list ref] wrote:
quoted
quoted
quoted
That said, I also agree that it would be interesting to hear what the
performance impact is for existing performance-sensitive users. Could
we make that "aio_may_use_threads()" case be unconditional, making
things simpler?
Making it unconditional is a goal, but some work is required before that
can be the case. The O_DIRECT issue is one such matter -- it requires
some
quoted
quoted
changes to the filesystems to ensure that they adhere to the
non-blocking
quoted
quoted
nature of the new interface (ie taking i_mutex is a Bad Thing that users
really do not want to be exposed to; if taking it blocks, the code
should
quoted
quoted
punt to a helper thread).
Filesystems *must take locks* in the IO path.
I agree.
I also would prefer to make the aio code have as little interaction and
magic flags with the filesystem code as humanly possible.
I wonder if we could make the rough rule be that the only synchronous case
the aio code ever has is more or less entirely in the generic vfs caches?
IOW, could we possibly aim to make the rule be that if we call down to the
filesystem layer, we do that within a thread?
We have to go through the filesystem layer locking even on page
cache hits, and even if we get into the page cache copy-in/copy-out
code we can still get stuck on things like page locks and page
faults. Even if hte pages are cached, we can still get caught on
deeper filesystem locks for block mapping. e.g. read from a hole,
get zeros back, page cache is populated. Write data into range,
fetch page, realise it's unmapped, need to do block/delayed
allocation which requires filesystem locks and potentially
transactions and IO....
We could do things like that for the name loopkup for openat() too, where
we could handle the successful RCU loopkup synchronously, but then if we
fall out of RCU mode we'd do the thread.
We'd have to do quite a bit of work to unwind back out to the AIO
layer before we can dispatch the open operation again in a thread,
wouldn't we?
So I'm not convinced that conditional thread dispatch makes sense. I
think the simplest thing to do is make all AIO use threads/
workqueues by default, and if the application is smart enough to
only do things that minimise blocking they can turn off the threaded
dispatch and get the same behaviour they get now.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-01-23 04:50:24
On Sat, Jan 23, 2016 at 03:24:49PM +1100, Dave Chinner wrote:
On Wed, Jan 20, 2016 at 04:56:30PM -0500, Benjamin LaHaise wrote:
quoted
On Thu, Jan 21, 2016 at 08:45:46AM +1100, Dave Chinner wrote:
quoted
Filesystems *must take locks* in the IO path. We have to serialise
against truncate and other operations at some point in the IO path
(e.g. block mapping vs concurrent allocation and/or removal), and
that can only be done sanely with sleeping locks. There is no way
of knowing in advance if we are going to block, and so either we
always use threads for IO submission or we accept that occasionally
the AIO submission will block.
I never said we don't take locks. Still, we can be more intelligent
about when and where we do so. With the nonblocking pread() and pwrite()
changes being proposed elsewhere, we can do the part of the I/O that
doesn't block in the submitter, which is a huge win when possible.
As it stands today, *every* buffered write takes i_mutex immediately
on entering ->write(). That one issue alone accounts for a nearly 10x
performance difference between an O_SYNC write and an O_DIRECT write,
Yes, that locking is for correct behaviour, not for performance
reasons. The i_mutex is providing the required semantics for POSIX
write(2) functionality - writes must serialise against other reads
and writes so that they are completed atomically w.r.t. other IO.
i.e. writes to the same offset must not interleave, not should reads
be able to see partial data from a write in progress.
No, the locks are not *required* for POSIX semantics, they are a legacy
of how Linux filesystem code has been implemented and how we ensure the
necessary internal consistency needed inside our filesystems is
provided. There are other ways to achieve the required semantics that
do not involve a single giant lock for the entire file/inode. And no, I
am not saying that doing this is simple or easy to do.
-ben
Direct IO does not conform to POSIX concurrency standards, so we
don't have to serialise concurrent IO against each other.
quoted
and using O_SYNC writes is a legitimate use-case for users who want
caching of data by the kernel (duplicating that functionality is a huge
amount of work for an application, plus if you want the cache to be
persistent between runs of an app, you have to get the kernel to do it).
Yes, but you take what you get given. Buffered IO sucks in many ways;
this is just one of them.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
"Thought is the essence of where you are now."
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
From: Dave Chinner <david@fromorbit.com> Date: 2016-01-23 22:22:57
On Fri, Jan 22, 2016 at 11:50:24PM -0500, Benjamin LaHaise wrote:
On Sat, Jan 23, 2016 at 03:24:49PM +1100, Dave Chinner wrote:
quoted
On Wed, Jan 20, 2016 at 04:56:30PM -0500, Benjamin LaHaise wrote:
quoted
On Thu, Jan 21, 2016 at 08:45:46AM +1100, Dave Chinner wrote:
quoted
Filesystems *must take locks* in the IO path. We have to serialise
against truncate and other operations at some point in the IO path
(e.g. block mapping vs concurrent allocation and/or removal), and
that can only be done sanely with sleeping locks. There is no way
of knowing in advance if we are going to block, and so either we
always use threads for IO submission or we accept that occasionally
the AIO submission will block.
I never said we don't take locks. Still, we can be more intelligent
about when and where we do so. With the nonblocking pread() and pwrite()
changes being proposed elsewhere, we can do the part of the I/O that
doesn't block in the submitter, which is a huge win when possible.
As it stands today, *every* buffered write takes i_mutex immediately
on entering ->write(). That one issue alone accounts for a nearly 10x
performance difference between an O_SYNC write and an O_DIRECT write,
Yes, that locking is for correct behaviour, not for performance
reasons. The i_mutex is providing the required semantics for POSIX
write(2) functionality - writes must serialise against other reads
and writes so that they are completed atomically w.r.t. other IO.
i.e. writes to the same offset must not interleave, not should reads
be able to see partial data from a write in progress.
No, the locks are not *required* for POSIX semantics, they are a legacy
of how Linux filesystem code has been implemented and how we ensure the
necessary internal consistency needed inside our filesystems is
provided.
That may be the case, but I really don't see how you can provide
such required functionality without some kind of exclusion barrier
in place. No matter how you implement that exclusion, it can be seen
effectively as a lock.
Even if the filesystem doesn't use the i_mutex for exclusion to the
page cache, it has to use some kind of lock as that IO still needs
to be serialised against any truncate, hole punch or other extent
manipulation that is currently in progress on the inode...
There are other ways to achieve the required semantics that
do not involve a single giant lock for the entire file/inode.
Most performant filesystems don't have a "single giant lock"
anymore. The problem is that the VFS expects the i_mutex to be held
for certain operations in the IO path and the VFS lock order
heirarchy makes it impossible to do anything but "get i_mutex
first". That's the problem that needs to be solved - the VFS
enforces the "one giant lock" model, even when underlying
filesystems do not require it.
i.e. we could quite happily remove the i_mutex completely from the XFS
buffered IO path without breaking anything, but we can't because
that results in the VFS throwing warnings that we don't hold the
i_mutex (e.g like when removing the SUID bits on write). So there's
lots of VFS functionality that needs to be turned on it's head
before the i_mutex can be removed from the IO path.
And no, I
am not saying that doing this is simple or easy to do.
Sure. That's always been the problem. Even when a split IO/metadata
locking strategy like what XFS uses (and other modern filesystems
are moving to internally) is suggested as a model for solving
these problems, the usual response instant dismissal with
"no way, that's unworkable" and so nothing ever changes...
Cheers,
Dave.
--
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
From: Benjamin LaHaise <bcrl@kvack.org> Date: 2016-03-14 17:17:37
On Sat, Jan 23, 2016 at 03:39:22PM +1100, Dave Chinner wrote:
On Wed, Jan 20, 2016 at 03:07:26PM -0800, Linus Torvalds wrote:
...
quoted
We could do things like that for the name loopkup for openat() too, where
we could handle the successful RCU loopkup synchronously, but then if we
fall out of RCU mode we'd do the thread.
We'd have to do quite a bit of work to unwind back out to the AIO
layer before we can dispatch the open operation again in a thread,
wouldn't we?
I had some time last week to make an aio openat do what it can in
submit context. The results are an improvement: when openat is handled
in submit context it completes in about half the time it takes compared
to the round trip via the work queue, and it's not terribly much code
either.
-ben
--
"Thought is the essence of where you are now."
fs/aio.c | 122 +++++++++++++++++++++++++++++++++++++++++---------
fs/internal.h | 1
fs/namei.c | 16 ++++--
fs/open.c | 2
include/linux/namei.h | 1
5 files changed, 117 insertions(+), 25 deletions(-)
commit 5d3d80fcf99287decc4774af01967cebbb0242fd
Author: Benjamin LaHaise [off-list ref]
Date: Thu Mar 10 17:15:07 2016 -0500
aio: add support for in-submit openat
Using the LOOKUP_RCU infrastructure added for open(), implement such
functionality to enable in io_submit() openat() that does a non-blocking
file open operation. This avoids the overhead of punting to another
kernel thread to complete the open operation when the files and data are
already in the dcache. This helps cut simple aio openat() from ~60-90K
cycles to ~24-45K cycles on my test system.
Signed-off-by: Benjamin LaHaise [off-list ref]
@@ -1546,6 +1553,18 @@ static void aio_thread_fn(struct work_struct *work)ret==-ERESTARTNOHAND||ret==-ERESTART_RESTARTBLOCK))ret=-EINTR;+/* Completion serializes cancellation by taking ctx_lock, so+*aio_complete()willnotreturnuntilafterforce_sig()in+*aio_thread_queue_iocb_cancel().Thisshouldensurethat+*thesignalispendingbeforebeingflushedinthisthread.+*/+aio_complete(&iocb->common,ret,0);+if(fatal_signal_pending(current))+flush_signals(current);++/* Clean up state after aio_complete() since ki_destruct may still+*needtoaccessthem.+*/if(iocb->ki_cred){current->cred=old_cred;put_cred(iocb->ki_cred);
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>
On Mon, Mar 14, 2016 at 10:17 AM, Benjamin LaHaise [off-list ref] wrote:
I had some time last week to make an aio openat do what it can in
submit context. The results are an improvement: when openat is handled
in submit context it completes in about half the time it takes compared
to the round trip via the work queue, and it's not terribly much code
either.
This looks good to me, and I do suspect that any of these aio paths
should strive to have a synchronous vs threaded model. I think that
makes the whole thing much more interesting from a performance
standpoint.
I still think the aio interface is really nasty, but this together
with the table-based approach you posted earlier does make me a _lot_
happier about the implementation.It just looks way less hacky, and now
it ends up exposing a rather more clever implementation too.
Linus
--
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>
From: Al Viro <viro@ZenIV.linux.org.uk> Date: 2016-03-20 01:26:10
On Sat, Mar 19, 2016 at 06:20:24PM -0700, Linus Torvalds wrote:
On Mon, Mar 14, 2016 at 10:17 AM, Benjamin LaHaise [off-list ref] wrote:
quoted
I had some time last week to make an aio openat do what it can in
submit context. The results are an improvement: when openat is handled
in submit context it completes in about half the time it takes compared
to the round trip via the work queue, and it's not terribly much code
either.
This looks good to me, and I do suspect that any of these aio paths
should strive to have a synchronous vs threaded model. I think that
makes the whole thing much more interesting from a performance
standpoint.
Umm... You do realize that LOOKUP_RCU in flags does *NOT* guarantee that
it won't block, right? At the very least one would need to refuse to
fall back on non-RCU mode without a full restart. Furthermore, vfs_open()
itself can easily block.
So this new LOOKUP flag makes no sense, and it's in the just about _the_
worst place possible for adding special cases with ill-defined semantics -
do_last() is already far too convoluted and needs untangling, not adding
half-assed kludges.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
On Sat, Mar 19, 2016 at 6:26 PM, Al Viro [off-list ref] wrote:
Umm... You do realize that LOOKUP_RCU in flags does *NOT* guarantee that
it won't block, right? At the very least one would need to refuse to
fall back on non-RCU mode without a full restart.
It actually does seem to do that, although in an admittedly rather
questionable way.
I think it should use path_openat() rather than do_filp_open(), but
passing in LOOKUP_RCU to do_filp_open() actually does work: it just
means that the retry after ECHILD/ESTALE will just do it *again* with
LOOKUP_RCU. It won't fall back to non-rcu mode, it just won't or in
the LOOKUP_RCU flag that is already set.
So I agree that it should be cleaned up, but the basic model seems
fine. I'm sure you're right about do_last() not necessarily being the
best place either. But that doesn't really change that the approach
seems *much* better than the old unconditional "do in a work queue".
Also, the whole "no guarantees of never blocking" is a specious argument.
Just copying the iocb from user space can block. Copying the pathname
likewise (or copying the iovec in the case of reads and writes). So
the aio interface at no point is "guaranteed to never block". Blocking
will happen. You can block on allocating the "struct file", or on
extending the filp table.
In the end it's about _performance_, and if the performance is better
with very unlikely blocking synchronous calls, then that's the right
thing to do.
Linus
From: Al Viro <viro@ZenIV.linux.org.uk> Date: 2016-03-20 01:55:11
On Sat, Mar 19, 2016 at 06:45:19PM -0700, Linus Torvalds wrote:
It actually does seem to do that, although in an admittedly rather
questionable way.
I think it should use path_openat() rather than do_filp_open(), but
passing in LOOKUP_RCU to do_filp_open() actually does work: it just
means that the retry after ECHILD/ESTALE will just do it *again* with
LOOKUP_RCU. It won't fall back to non-rcu mode, it just won't or in
the LOOKUP_RCU flag that is already set.
What would make unlazy_walk() fail? And if it succeeds, you are not
in RCU mode anymore *without* restarting from scratch...
--
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>
On Sat, Mar 19, 2016 at 6:55 PM, Al Viro [off-list ref] wrote:
What would make unlazy_walk() fail? And if it succeeds, you are not
in RCU mode anymore *without* restarting from scratch...
I don't see your point.
You don't want to be in RCU mode any more. You want to either succeed
or fail with ECHILD/ESTALE. Then, in the failure case, you go to the
thread.
What I meant by restarting was the restart that do_filp_open() does,
and there it just restarts with "op->lookup_flags", which has
RCU_LOOKUP still set, so it would just try to do the RCU lookup again.
But I actually notice now that Ben actually disabled that restart if
LOOKUP_RCU was set, so that ends up not even happening.
Anyway, I'm not saying it's polished and pretty. I think the changes
to do_filp_open() are a bit silly, and the code should just use
path_openat() directly. Possibly using a new helper (ie perhaps just
introduce a "rcu_filp_openat()" thing). But from a design perspective,
I think this all looks fine.
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>