aio: Make it possible to remap aio ring
Subsystems (4)
PATCH 1/1 aio: Make it possible to remap aio ring
Hi,
There are actually two issues this patch addresses. Let me start with the one I tried to solve in the beginning.
So, in the checkpoint-restore project (criu) we try to dump tasks' state and restore one back exactly as it was. One of the tasks' state bits is rings set up with io_setup() call. There's (almost) no problems in dumping them, there's a problem restoring them -- if I dump a task with aio ring originally mapped at address A, I want to restore one back at exactly the same address A. Unfortunately, the io_setup() does not allow for that -- it mmaps the ring at whatever place mm finds appropriate (it calls do_mmap_pgoff() with zero address and without the MAP_FIXED flag).
To make restore possible I'm going to mremap() the freshly created ring into the address A (under which it was seen before dump). The problem is that the ring's virtual address is passed back to the user-space as the context ID and this ID is then used as search key by all the other io_foo() calls. Reworking this ID to be just some integer doesn't seem to work, as this value is already used by libaio as a pointer using which this library accesses memory for aio meta-data.
So, to make restore work we need to make sure that
a) ring is mapped at desired virtual address
b) kioctx->user_id matches this value
Having said that, the patch makes mremap() on aio region update the kioctx's user_id and mmap_base values.
Here appears the 2nd issue I mentioned in the beginning of this mail. If (regardless of the C/R dances I do) someone creates an io context with io_setup(), then mremap()-s the ring and then destroys the context, the kill_ioctx() routine will call munmap() on wrong (old) address. This will result in a) aio ring remaining in memory and b) some other vma get unexpectedly unmapped.
What do you think?
--- fs/aio.c | 25 +++++++++++++++++++++++++ include/linux/fs.h | 1 + mm/mremap.c | 3 ++- 3 files changed, 28 insertions(+), 1 deletion(-)
fs/aio.c +25 -0
@@ -273,12 +273,37 @@ static void aio_free_ring(struct kioctx *ctx)static int aio_ring_mmap (struct file *file,struct vm_area_struct *vma) { + vma->vm_flags |=VM_DONTEXPAND ; vma->vm_ops = &generic_file_vm_ops;return 0 ; } +static void aio_ring_remap (struct file *file,struct vm_area_struct *vma) +{ +struct mm_struct *mm = vma->vm_mm; +struct kioctx_table *table; +int i; + +spin_lock (&mm->ioctx_lock); +rcu_read_lock (); + table =rcu_dereference (mm->ioctx_table); +for (i =0 ; i < table->nr; i++) { +struct kioctx *ctx; + + ctx = table->table[i]; +if (ctx && ctx->aio_ring_file == file) { + ctx->user_id = ctx->mmap_base = vma->vm_start; +break ; + } + } + +rcu_read_unlock (); +spin_unlock (&mm->ioctx_lock); +} +static const struct file_operations aio_ring_fops = { .mmap = aio_ring_mmap, + .mremap = aio_ring_remap, };static int aio_set_page_dirty (struct page *page)
include/linux/fs.h +1 -0
@@ -1467,6 +1467,7 @@ struct file_operations {long (*unlocked_ioctl) (struct file *,unsigned int ,unsigned long );long (*compat_ioctl) (struct file *,unsigned int ,unsigned long );int (*mmap) (struct file *,struct vm_area_struct *); +void (*mremap)(struct file *,struct vm_area_struct *);int (*open) (struct inode *,struct file *);int (*flush) (struct file *,fl_owner_t id);int (*release) (struct inode *,struct file *);
mm/mremap.c +2 -1
@@ -287,7 +287,8 @@ static unsigned long move_vma(struct vm_area_struct *vma, old_len = new_len; old_addr = new_addr; new_addr = -ENOMEM ; - } + }else if (vma->vm_file && vma->vm_file->f_op->mremap) + vma->vm_file->f_op->mremap (vma->vm_file, new_vma);/* Conceal VM_ACCOUNT so old reservation is not undone */ if (vm_flags &VM_ACCOUNT ) {
Signature · 7 lines
1.8.4.2 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 40 lines · Pavel Emelyanov
Hi,
There are actually two issues this patch addresses. Let me start with the one I tried to solve in the beginning.
So, in the checkpoint-restore project (criu) we try to dump tasks' state and restore one back exactly as it was. One of the tasks' state bits is rings set up with
io_setup()call. There's (almost) no problems in dumping them, there's a problem restoring them -- if I dump a task with aio ring originally mapped at address A, I want to restore one back at exactly the same address A. Unfortunately, theio_setup()does not allow for that -- it mmaps the ring at whatever place mm finds appropriate (it callsdo_mmap_pgoff()with zero address and without theMAP_FIXEDflag).To make restore possible I'm going to
mremap()the freshly created ring into the address A (under which it was seen before dump). The problem is that the ring's virtual address is passed back to the user-space as the context ID and this ID is then used as search key by all the otherio_foo()calls. Reworking this ID to be just some integer doesn't seem to work, as this value is already used by libaio as a pointer using which this library accesses memory for aio meta-data.So, to make restore work we need to make sure that
a) ring is mapped at desired virtual address
b)kioctx->user_idmatches this valueHaving said that, the patch makes
mremap()on aio region update the kioctx'suser_idandmmap_basevalues.Here appears the 2nd issue I mentioned in the beginning of this mail. If (regardless of the C/R dances I do) someone creates an io context with
io_setup(), then mremap()-s the ring and then destroys the context, thekill_ioctx()routine will callmunmap()on wrong (old) address. This will result in a) aio ring remaining in memory and b) some other vma get unexpectedly unmapped.
Quoted 1 line
What do you think?
Look reasonable.
Feel free to add Acked-by:Dmitry Monakhov <[email protected]>
Quoted 84 lines
Signed-off-by Pavel Emelyanov <[email protected]>--- fs/aio.c | 25 +++++++++++++++++++++++++ include/linux/fs.h | 1 + mm/mremap.c | 3 ++- 3 files changed, 28 insertions(+), 1 deletion(-)fs/aio.c +25
-0@@ -273,12 +273,37 @@ static void aio_free_ring(struct kioctx *ctx)static int aio_ring_mmap (struct file *file,struct vm_area_struct *vma) { + vma->vm_flags |=VM_DONTEXPAND ; vma->vm_ops = &generic_file_vm_ops;return 0 ; } +static void aio_ring_remap (struct file *file,struct vm_area_struct *vma) +{ +struct mm_struct *mm = vma->vm_mm; +struct kioctx_table *table; +int i; + +spin_lock (&mm->ioctx_lock); +rcu_read_lock (); + table =rcu_dereference (mm->ioctx_table); +for (i =0 ; i < table->nr; i++) { +struct kioctx *ctx; + + ctx = table->table[i]; +if (ctx && ctx->aio_ring_file == file) { + ctx->user_id = ctx->mmap_base = vma->vm_start; +break ; + } + } + +rcu_read_unlock (); +spin_unlock (&mm->ioctx_lock); +} +static const struct file_operations aio_ring_fops = { .mmap = aio_ring_mmap, + .mremap = aio_ring_remap, };static int aio_set_page_dirty (struct page *page)include/linux/fs.h +1
-0@@ -1467,6 +1467,7 @@ struct file_operations {long (*unlocked_ioctl) (struct file *,unsigned int ,unsigned long );long (*compat_ioctl) (struct file *,unsigned int ,unsigned long );int (*mmap) (struct file *,struct vm_area_struct *); +void (*mremap)(struct file *,struct vm_area_struct *);int (*open) (struct inode *,struct file *);int (*flush) (struct file *,fl_owner_t id);int (*release) (struct inode *,struct file *);mm/mremap.c +2
-1@@ -287,7 +287,8 @@ static unsigned long move_vma(struct vm_area_struct *vma, old_len = new_len; old_addr = new_addr; new_addr = -ENOMEM ; - } + }else if (vma->vm_file && vma->vm_file->f_op->mremap) + vma->vm_file->f_op->mremap (vma->vm_file, new_vma);/* Conceal VM_ACCOUNT so old reservation is not undone */ if (vm_flags &VM_ACCOUNT ) {Signature · 7 lines
1.8.4.2 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Benjamin LaHaise
maintainerQuoted 41 lines · Dmitry Monakhov
Quoted 40 lines · Pavel Emelyanov
Hi,
There are actually two issues this patch addresses. Let me start with the one I tried to solve in the beginning.
So, in the checkpoint-restore project (criu) we try to dump tasks' state and restore one back exactly as it was. One of the tasks' state bits is rings set up with
io_setup()call. There's (almost) no problems in dumping them, there's a problem restoring them -- if I dump a task with aio ring originally mapped at address A, I want to restore one back at exactly the same address A. Unfortunately, theio_setup()does not allow for that -- it mmaps the ring at whatever place mm finds appropriate (it callsdo_mmap_pgoff()with zero address and without theMAP_FIXEDflag).To make restore possible I'm going to
mremap()the freshly created ring into the address A (under which it was seen before dump). The problem is that the ring's virtual address is passed back to the user-space as the context ID and this ID is then used as search key by all the otherio_foo()calls. Reworking this ID to be just some integer doesn't seem to work, as this value is already used by libaio as a pointer using which this library accesses memory for aio meta-data.So, to make restore work we need to make sure that
a) ring is mapped at desired virtual address
b)kioctx->user_idmatches this valueHaving said that, the patch makes
mremap()on aio region update the kioctx'suser_idandmmap_basevalues.Here appears the 2nd issue I mentioned in the beginning of this mail. If (regardless of the C/R dances I do) someone creates an io context with
io_setup(), then mremap()-s the ring and then destroys the context, thekill_ioctx()routine will callmunmap()on wrong (old) address. This will result in a) aio ring remaining in memory and b) some other vma get unexpectedly unmapped.
Quoted 5 lines
Quoted 1 line
What do you think?
Look reasonable.
Feel free to add Acked-by:Dmitry Monakhov <[email protected]>Quoted 2 lines
Signed-off-by Pavel Emelyanov <[email protected]>
I've had a look over this patch, and it seems okay to me. The interaction with page migration looks safe, as well as with io_destroy(). I've applied this to my aio-next tree at git://git.kvack.org/~bcrl/aio-next.git . If mm folks have any concerns, please let me know.
-ben
Quoted 82 lines
Quoted 82 lines
--- fs/aio.c | 25 +++++++++++++++++++++++++ include/linux/fs.h | 1 + mm/mremap.c | 3 ++- 3 files changed, 28 insertions(+), 1 deletion(-)fs/aio.c +25
-0@@ -273,12 +273,37 @@ static void aio_free_ring(struct kioctx *ctx)static int aio_ring_mmap (struct file *file,struct vm_area_struct *vma) { + vma->vm_flags |=VM_DONTEXPAND ; vma->vm_ops = &generic_file_vm_ops;return 0 ; } +static void aio_ring_remap (struct file *file,struct vm_area_struct *vma) +{ +struct mm_struct *mm = vma->vm_mm; +struct kioctx_table *table; +int i; + +spin_lock (&mm->ioctx_lock); +rcu_read_lock (); + table =rcu_dereference (mm->ioctx_table); +for (i =0 ; i < table->nr; i++) { +struct kioctx *ctx; + + ctx = table->table[i]; +if (ctx && ctx->aio_ring_file == file) { + ctx->user_id = ctx->mmap_base = vma->vm_start; +break ; + } + } + +rcu_read_unlock (); +spin_unlock (&mm->ioctx_lock); +} +static const struct file_operations aio_ring_fops = { .mmap = aio_ring_mmap, + .mremap = aio_ring_remap, };static int aio_set_page_dirty (struct page *page)include/linux/fs.h +1
-0@@ -1467,6 +1467,7 @@ struct file_operations {long (*unlocked_ioctl) (struct file *,unsigned int ,unsigned long );long (*compat_ioctl) (struct file *,unsigned int ,unsigned long );int (*mmap) (struct file *,struct vm_area_struct *); +void (*mremap)(struct file *,struct vm_area_struct *);int (*open) (struct inode *,struct file *);int (*flush) (struct file *,fl_owner_t id);int (*release) (struct inode *,struct file *);mm/mremap.c +2
-1@@ -287,7 +287,8 @@ static unsigned long move_vma(struct vm_area_struct *vma, old_len = new_len; old_addr = new_addr; new_addr = -ENOMEM ; - } + }else if (vma->vm_file && vma->vm_file->f_op->mremap) + vma->vm_file->f_op->mremap (vma->vm_file, new_vma);/* Conceal VM_ACCOUNT so old reservation is not undone */ if (vm_flags &VM_ACCOUNT ) {Signature · 7 lines
1.8.4.2 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Signature · 7 lines
"Thought is the essence of where you are now." -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
To make restore possible I'm going to
mremap()the freshly created ring into the address A (under which it was seen before dump).What do you think?
Look reasonable.
Feel free to add Acked-by:Dmitry Monakhov <[email protected]>Quoted 1 line
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Signed-off-by Pavel Emelyanov <[email protected]>I've had a look over this patch, and it seems okay to me. The interaction with page migration looks safe, as well as with
io_destroy(). I've applied this to my aio-next tree atgit://git.kvack.org/~bcrl/aio-next.git. If mm folks have any concerns, please let me know.
I can chime in with generic support: the C/R folks have complained about the implicit context mapping in the past. I'm all for letting them explicitly re-establish it, though I didn't actually look at the patch.
- z
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>