Skip to contentSkip to conversation

Change soft-dirty interface?

DISCUSSION Minchan Kim 2 participants 11 messages Jun 13, 2013 – Jun 21, 2013 Memory management

lore
Share & tools
lore
#0 lore raw

Hi all,

Sorry for late interrupting to promote patchset to the mainline. I'd like to discuss our usecase so I'd like to change per-process interface with per-range interface.

Our usecase is following as,

A application allocates a big buffer(A) and makes backup buffer(B) for it and copy B from A.
Let's assume A consists of subranges (A-1, A-2, A-3, A-4).
As time goes by, application can modify anywhere of A.
In this example, let's assume A-1 and A-2 are modified.
When the time happen, we compare A-1 with B-1 to make
diff of the range(On every iteration, we don't need all range's diff by design) and do something with diff, then we'd like to remark only the A-1 with soft-dirty, NOT A's all range of the process to track the A-1's further difference in future while keeping dirty information (A-2, A-3, A-4) because we will make A-2's diff in next iteration.

We can't do it by existing interface.

So, I'd like to add [addr, len] argument with using proc

    echo 4 0x100000 0x3000 > /proc/self/clear_refs

It doesn't break anything but not sure everyone like the interface because recently I heard from akpm following comment.

        https://lkml.org/lkml/2013/5/21/529

Although per-process reclaim is another story with this, I feel he seems to hate doing something on proc interface with /proc/pid/maps like above range parameter.

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

Signature · 8 lines
Kind regards,
Minchan Kim

--
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>
#1 lore raw
Quoted 20 lines · Minchan Kim

Hi all,

Sorry for late interrupting to promote patchset to the mainline. I'd like to discuss our usecase so I'd like to change per-process interface with per-range interface.

Our usecase is following as,

A application allocates a big buffer(A) and makes backup buffer(B) for it and copy B from A.
Let's assume A consists of subranges (A-1, A-2, A-3, A-4).
As time goes by, application can modify anywhere of A.
In this example, let's assume A-1 and A-2 are modified.
When the time happen, we compare A-1 with B-1 to make
diff of the range(On every iteration, we don't need all range's diff by design) and do something with diff, then we'd like to remark only the A-1 with soft-dirty, NOT A's all range of the process to track the A-1's further difference in future while keeping dirty information (A-2, A-3, A-4) because we will make A-2's diff in next iteration.

Quoted 1 line

We can't do it by existing interface.

So you need to track changes not in the whole range, but in sub-ranges. OK.

Quoted 9 lines

So, I'd like to add [addr, len] argument with using proc

    echo 4 0x100000 0x3000 > /proc/self/clear_refs

It doesn't break anything but not sure everyone like the interface because recently I heard from akpm following comment.

        https://lkml.org/lkml/2013/5/21/529
Quoted 7 lines

Although per-process reclaim is another story with this, I feel he seems to hate doing something on proc interface with /proc/pid/maps like above range parameter.

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

This looks like existing sys_madvise() one.

Quoted 9 lines

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

This is OK for me, though there's another issue with this API I'd like to mention -- consider your app is doing these tricks with soft-dirty and at the same time CRIU tools live-migrate it using the soft-dirty bits to optimize the freeze time.

In that case soft-dirty bits would be in wrong state for both -- you app and CRIU, but with the proc API we could compare the ctime-s of the clear_refs file and find out, that someone spoiled the soft-dirty state from last time we messed with it and handle it somehow (copy all the memory in the worst case). Can we somehow handle this with your proposal?

Thanks,
Pavel

--
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>

#2 lore raw

Hello Pavel,

Quoted 21 lines · Pavel Emelyanov
Quoted 20 lines · Minchan Kim

Hi all,

Sorry for late interrupting to promote patchset to the mainline. I'd like to discuss our usecase so I'd like to change per-process interface with per-range interface.

Our usecase is following as,

A application allocates a big buffer(A) and makes backup buffer(B) for it and copy B from A.
Let's assume A consists of subranges (A-1, A-2, A-3, A-4).
As time goes by, application can modify anywhere of A.
In this example, let's assume A-1 and A-2 are modified.
When the time happen, we compare A-1 with B-1 to make
diff of the range(On every iteration, we don't need all range's diff by design) and do something with diff, then we'd like to remark only the A-1 with soft-dirty, NOT A's all range of the process to track the A-1's further difference in future while keeping dirty information (A-2, A-3, A-4) because we will make A-2's diff in next iteration.

Quoted 4 lines
Quoted 1 line

We can't do it by existing interface.

So you need to track changes not in the whole range, but in sub-ranges. OK.

Right.

Quoted 14 lines
Quoted 14 lines

So, I'd like to add [addr, len] argument with using proc

    echo 4 0x100000 0x3000 > /proc/self/clear_refs

It doesn't break anything but not sure everyone like the interface because recently I heard from akpm following comment.

        https://lkml.org/lkml/2013/5/21/529

Although per-process reclaim is another story with this, I feel he seems to hate doing something on proc interface with /proc/pid/maps like above range parameter.

Quoted 5 lines
Quoted 3 lines

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

This looks like existing sys_madvise() one.

Except pid part. It is added by your purpose, which external task can control any process.

Quoted 16 lines
Quoted 10 lines

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

This is OK for me, though there's another issue with this API I'd like to mention -- consider your app is doing these tricks with soft-dirty and at the same time CRIU tools live-migrate it using the soft-dirty bits to optimize the freeze time.

Quoted 5 lines

In that case soft-dirty bits would be in wrong state for both -- you app and CRIU, but with the proc API we could compare the ctime-s of the clear_refs file and find out, that someone spoiled the soft-dirty state from last time we messed with it and handle it somehow (copy all the memory in the worst case). Can we somehow handle this with your proposal?

Good point I didn't think over that.
A simple idea popped from my mind is we can use read/write lock so if pid is equal to calling process's one or pid is NULL, we use read side lock, which can allow marking soft-dirty several vmas with parallel. And pid is not equal to calling process's one, the API should try to hold write-side lock then, if it's fail, the API should return EAGAIN so that CRIU can progress other processes and retry it after a while. Of course, it would make live-lock so that sys_softdirty might need another argument like "int block".

Quoted 9 lines

Thanks,
Pavel

--
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 · 8 lines
Kind regards,
Minchan Kim

--
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>
#3 lore raw
Quoted 76 lines · Minchan Kim

Hello Pavel,

Quoted 21 lines · Pavel Emelyanov
Quoted 20 lines · Minchan Kim

Hi all,

Sorry for late interrupting to promote patchset to the mainline. I'd like to discuss our usecase so I'd like to change per-process interface with per-range interface.

Our usecase is following as,

A application allocates a big buffer(A) and makes backup buffer(B) for it and copy B from A.
Let's assume A consists of subranges (A-1, A-2, A-3, A-4).
As time goes by, application can modify anywhere of A.
In this example, let's assume A-1 and A-2 are modified.
When the time happen, we compare A-1 with B-1 to make
diff of the range(On every iteration, we don't need all range's diff by design) and do something with diff, then we'd like to remark only the A-1 with soft-dirty, NOT A's all range of the process to track the A-1's further difference in future while keeping dirty information (A-2, A-3, A-4) because we will make A-2's diff in next iteration.

Quoted 4 lines
Quoted 1 line

We can't do it by existing interface.

So you need to track changes not in the whole range, but in sub-ranges. OK.

Right.

Quoted 14 lines
Quoted 14 lines

So, I'd like to add [addr, len] argument with using proc

    echo 4 0x100000 0x3000 > /proc/self/clear_refs

It doesn't break anything but not sure everyone like the interface because recently I heard from akpm following comment.

        https://lkml.org/lkml/2013/5/21/529

Although per-process reclaim is another story with this, I feel he seems to hate doing something on proc interface with /proc/pid/maps like above range parameter.

Quoted 5 lines
Quoted 3 lines

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

This looks like existing sys_madvise() one.

Except pid part. It is added by your purpose, which external task can control any process.

Quoted 16 lines
Quoted 10 lines

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

This is OK for me, though there's another issue with this API I'd like to mention -- consider your app is doing these tricks with soft-dirty and at the same time CRIU tools live-migrate it using the soft-dirty bits to optimize the freeze time.

Quoted 5 lines

In that case soft-dirty bits would be in wrong state for both -- you app and CRIU, but with the proc API we could compare the ctime-s of the clear_refs file and find out, that someone spoiled the soft-dirty state from last time we messed with it and handle it somehow (copy all the memory in the worst case). Can we somehow handle this with your proposal?

Quoted 10 lines

Good point I didn't think over that.
A simple idea popped from my mind is we can use read/write lock so if pid is equal to calling process's one or pid is NULL, we use read side lock, which can allow marking soft-dirty several vmas with parallel. And pid is not equal to calling process's one, the API should try to hold write-side lock then, if it's fail, the API should return EAGAIN so that CRIU can progress other processes and retry it after a while. Of course, it would make live-lock so that sys_softdirty might need another argument like "int block".

And we need a flag to show SELF_SOFT_DIRTY or EXTERNAL_SOFT_DIRTY and the flag will be protected by above lock. It could prevent mixed case by self and external.

Signature · 8 lines
Kind regards,
Minchan Kim

--
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>
#4 lore raw
Quoted 88 lines · Minchan Kim
Quoted 76 lines · Minchan Kim

Hello Pavel,

Quoted 21 lines · Pavel Emelyanov
Quoted 20 lines · Minchan Kim

Hi all,

Sorry for late interrupting to promote patchset to the mainline. I'd like to discuss our usecase so I'd like to change per-process interface with per-range interface.

Our usecase is following as,

A application allocates a big buffer(A) and makes backup buffer(B) for it and copy B from A.
Let's assume A consists of subranges (A-1, A-2, A-3, A-4).
As time goes by, application can modify anywhere of A.
In this example, let's assume A-1 and A-2 are modified.
When the time happen, we compare A-1 with B-1 to make
diff of the range(On every iteration, we don't need all range's diff by design) and do something with diff, then we'd like to remark only the A-1 with soft-dirty, NOT A's all range of the process to track the A-1's further difference in future while keeping dirty information (A-2, A-3, A-4) because we will make A-2's diff in next iteration.

Quoted 4 lines
Quoted 1 line

We can't do it by existing interface.

So you need to track changes not in the whole range, but in sub-ranges. OK.

Right.

Quoted 14 lines
Quoted 14 lines

So, I'd like to add [addr, len] argument with using proc

    echo 4 0x100000 0x3000 > /proc/self/clear_refs

It doesn't break anything but not sure everyone like the interface because recently I heard from akpm following comment.

        https://lkml.org/lkml/2013/5/21/529

Although per-process reclaim is another story with this, I feel he seems to hate doing something on proc interface with /proc/pid/maps like above range parameter.

Quoted 5 lines
Quoted 3 lines

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

This looks like existing sys_madvise() one.

Except pid part. It is added by your purpose, which external task can control any process.

Quoted 16 lines
Quoted 10 lines

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

This is OK for me, though there's another issue with this API I'd like to mention -- consider your app is doing these tricks with soft-dirty and at the same time CRIU tools live-migrate it using the soft-dirty bits to optimize the freeze time.

Quoted 5 lines

In that case soft-dirty bits would be in wrong state for both -- you app and CRIU, but with the proc API we could compare the ctime-s of the clear_refs file and find out, that someone spoiled the soft-dirty state from last time we messed with it and handle it somehow (copy all the memory in the worst case). Can we somehow handle this with your proposal?

Quoted 10 lines

Good point I didn't think over that.
A simple idea popped from my mind is we can use read/write lock so if pid is equal to calling process's one or pid is NULL, we use read side lock, which can allow marking soft-dirty several vmas with parallel. And pid is not equal to calling process's one, the API should try to hold write-side lock then, if it's fail, the API should return EAGAIN so that CRIU can progress other processes and retry it after a while. Of course, it would make live-lock so that sys_softdirty might need another argument like "int block".

Quoted 3 lines

And we need a flag to show SELF_SOFT_DIRTY or EXTERNAL_SOFT_DIRTY and the flag will be protected by above lock. It could prevent mixed case by self and external.

I realized it's not enough. Another idea is here. The intenion is followin as,

self softdirty VS self softdirty -> NOT exclusive self softdirty VS external softdirty -> exclusive external softdirty VS external softdirty-> excluisve

struct softdirty token {
        u64 external;
        u64 internal;
};

int sys_set_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty *token);
int sys_get_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty token, char *vec);

SYSCALL(set_softdirty, ..., token)

{
        struct task_struct *tsk = task_from_pid(pid);
        mutex_lock(&mm->st_lock);
        if (tsk == current)
                tsk->mm->token.internal++; 
        else
                tsk->mm->token.external++;
        token->external = mm->token.external;
        token->internal = mm->token.internal;
        mutex_unlock(&mm->st_lock);
        ..
        ..

}

SYSCALL(get_softdirty, ..., token, ...)

{
        struct task_struct *tsk = task_from_pid(pid);
        mutex_lock(&mm->st_lock);
        if (tsk == current) {
                if (tsk->mm->token.external != token.external) {
                        mutex_unlock
                        return -EAGAIN;
                }
        } else {
                if (tsk->mm->token.external != token.external ||
                    tsk->mm->token.internal != token.internal) {
                        mutex_unlock;
                        return -EAGAIN;
                }
        }
        mutex_unlock(&mm->st_lock);
        ...
}
Quoted 10 lines
Signature · 9 lines
-- 
Kind regards,
Minchan Kim

--
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 · 8 lines
Kind regards,
Minchan Kim

--
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>
#5 lore raw
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

This looks like existing sys_madvise() one.

Except pid part. It is added by your purpose, which external task can control any process.

In CRIU we can work with pid-less syscalls just fine :) So extending regular madvise would work.

Quoted 37 lines
Quoted 33 lines
Quoted 22 lines
Quoted 16 lines
Quoted 10 lines

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

This is OK for me, though there's another issue with this API I'd like to mention -- consider your app is doing these tricks with soft-dirty and at the same time CRIU tools live-migrate it using the soft-dirty bits to optimize the freeze time.

Quoted 5 lines

In that case soft-dirty bits would be in wrong state for both -- you app and CRIU, but with the proc API we could compare the ctime-s of the clear_refs file and find out, that someone spoiled the soft-dirty state from last time we messed with it and handle it somehow (copy all the memory in the worst case). Can we somehow handle this with your proposal?

Quoted 10 lines

Good point I didn't think over that.
A simple idea popped from my mind is we can use read/write lock so if pid is equal to calling process's one or pid is NULL, we use read side lock, which can allow marking soft-dirty several vmas with parallel. And pid is not equal to calling process's one, the API should try to hold write-side lock then, if it's fail, the API should return EAGAIN so that CRIU can progress other processes and retry it after a while. Of course, it would make live-lock so that sys_softdirty might need another argument like "int block".

Quoted 3 lines

And we need a flag to show SELF_SOFT_DIRTY or EXTERNAL_SOFT_DIRTY and the flag will be protected by above lock. It could prevent mixed case by self and external.

Quoted 6 lines

I realized it's not enough. Another idea is here. The intenion is followin as,

self softdirty VS self softdirty -> NOT exclusive self softdirty VS external softdirty -> exclusive external softdirty VS external softdirty-> excluisve

I think it might work for us. However, I have two comments to the implementation, please see below.

Quoted 9 lines
struct softdirty token {
        u64 external;
        u64 internal;
};

int sys_set_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty *token);
int sys_get_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty token, char *vec);

Can you please show an example how to use these two, I don't quite get how can I do external soft-dirty tracking in atomic manner.

Quoted 18 lines

SYSCALL(set_softdirty, ..., token)

{
        struct task_struct *tsk = task_from_pid(pid);
        mutex_lock(&mm->st_lock);
        if (tsk == current)
                tsk->mm->token.internal++; 
        else
                tsk->mm->token.external++;
        token->external = mm->token.external;
        token->internal = mm->token.internal;
        mutex_unlock(&mm->st_lock);
        ..
        ..

}

SYSCALL(get_softdirty, ..., token, ...)

Quoted 16 lines
{
        struct task_struct *tsk = task_from_pid(pid);
        mutex_lock(&mm->st_lock);
        if (tsk == current) {
                if (tsk->mm->token.external != token.external) {
                        mutex_unlock
                        return -EAGAIN;
                }
        } else {
                if (tsk->mm->token.external != token.external ||
                    tsk->mm->token.internal != token.internal) {
                        mutex_unlock;
                        return -EAGAIN;
                }
        }
        mutex_unlock(&mm->st_lock);

Presumably the critical section should be longer, as if tokens match and we release the lock and proceed with working on pagemap, the concurrent call to set_softdirty can proceed and spoil the picture.

Quoted 17 lines
        ...
}
Quoted 10 lines
Signature · 9 lines
-- 
Kind regards,
Minchan Kim

--
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>

#6 lore raw

Hello Pavel,

Quoted 11 lines · Pavel Emelyanov
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

This looks like existing sys_madvise() one.

Except pid part. It is added by your purpose, which external task can control any process.

In CRIU we can work with pid-less syscalls just fine :) So extending regular madvise would work.

I didn't know that.
Just out of curiosity. How can CRIU control other tasks without pid?

Quoted 41 lines
Quoted 41 lines
Quoted 34 lines
Quoted 23 lines
Quoted 17 lines
Quoted 11 lines

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

This is OK for me, though there's another issue with this API I'd like to mention -- consider your app is doing these tricks with soft-dirty and at the same time CRIU tools live-migrate it using the soft-dirty bits to optimize the freeze time.

Quoted 5 lines

In that case soft-dirty bits would be in wrong state for both -- you app and CRIU, but with the proc API we could compare the ctime-s of the clear_refs file and find out, that someone spoiled the soft-dirty state from last time we messed with it and handle it somehow (copy all the memory in the worst case). Can we somehow handle this with your proposal?

Quoted 10 lines

Good point I didn't think over that.
A simple idea popped from my mind is we can use read/write lock so if pid is equal to calling process's one or pid is NULL, we use read side lock, which can allow marking soft-dirty several vmas with parallel. And pid is not equal to calling process's one, the API should try to hold write-side lock then, if it's fail, the API should return EAGAIN so that CRIU can progress other processes and retry it after a while. Of course, it would make live-lock so that sys_softdirty might need another argument like "int block".

Quoted 3 lines

And we need a flag to show SELF_SOFT_DIRTY or EXTERNAL_SOFT_DIRTY and the flag will be protected by above lock. It could prevent mixed case by self and external.

I realized it's not enough. Another idea is here. The intenion is followin as,

Quoted 14 lines
Quoted 3 lines

self softdirty VS self softdirty -> NOT exclusive self softdirty VS external softdirty -> exclusive external softdirty VS external softdirty-> excluisve

I think it might work for us. However, I have two comments to the implementation, please see below.

Quoted 7 lines
struct softdirty token {
        u64 external;
        u64 internal;
};

int sys_set_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty *token);

I should have mentioned that start and len are ignored if pid is not eqaul to caller's pid.

Quoted 5 lines
Quoted 2 lines

int sys_get_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty token, char *vec);

Can you please show an example how to use these two, I don't quite get how can I do external soft-dirty tracking in atomic manner.

Hmm, I don't know how CRIU works but ...

	while(1) {

		struct softdirty token;
		
		sys_set_softdirty(tracked_pid, 0, 0, &token);
		...
		...
		...
		if (!sys_get_softdirty(tacked_pid, 0, 0, token, NULL))
			break;
	}

Maybe do you have a concern about live-lock?

Quoted 36 lines
Quoted 19 lines

SYSCALL(set_softdirty, ..., token)

{
        struct task_struct *tsk = task_from_pid(pid);
        mutex_lock(&mm->st_lock);
        if (tsk == current)
                tsk->mm->token.internal++; 
        else
                tsk->mm->token.external++;
        token->external = mm->token.external;
        token->internal = mm->token.internal;
        mutex_unlock(&mm->st_lock);
        ..
        ..

}

SYSCALL(get_softdirty, ..., token, ...)

Quoted 16 lines
{
        struct task_struct *tsk = task_from_pid(pid);
        mutex_lock(&mm->st_lock);
        if (tsk == current) {
                if (tsk->mm->token.external != token.external) {
                        mutex_unlock
                        return -EAGAIN;
                }
        } else {
                if (tsk->mm->token.external != token.external ||
                    tsk->mm->token.internal != token.internal) {
                        mutex_unlock;
                        return -EAGAIN;
                }
        }
        mutex_unlock(&mm->st_lock);
Quoted 3 lines

Presumably the critical section should be longer, as if tokens match and we release the lock and proceed with working on pagemap, the concurrent call to set_softdirty can proceed and spoil the picture.

True.

Signature · 8 lines
Kind regards,
Minchan Kim

--
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>
#7 lore raw
Quoted 9 lines · Minchan Kim

Hello Pavel,

Quoted 6 lines · Pavel Emelyanov
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

This looks like existing sys_madvise() one.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

Except pid part. It is added by your purpose, which external task can control any process.

In CRIU we can work with pid-less syscalls just fine :) So extending regular madvise would work.

I didn't know that.
Just out of curiosity. How can CRIU control other tasks without pid?

We use the parasite-injection technique [1]. Briefly -- we put a code into other task's address space using ptrace() and /proc/PID/map_files/ and make this code run and do what we need. Thus we can call madvise() "on" another task.

[1] http://lwn.net/Articles/454304/

Quoted 53 lines
Quoted 53 lines
Quoted 38 lines
Quoted 34 lines
Quoted 23 lines
Quoted 17 lines
Quoted 11 lines

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

This is OK for me, though there's another issue with this API I'd like to mention -- consider your app is doing these tricks with soft-dirty and at the same time CRIU tools live-migrate it using the soft-dirty bits to optimize the freeze time.

Quoted 5 lines

In that case soft-dirty bits would be in wrong state for both -- you app and CRIU, but with the proc API we could compare the ctime-s of the clear_refs file and find out, that someone spoiled the soft-dirty state from last time we messed with it and handle it somehow (copy all the memory in the worst case). Can we somehow handle this with your proposal?

Quoted 10 lines

Good point I didn't think over that.
A simple idea popped from my mind is we can use read/write lock so if pid is equal to calling process's one or pid is NULL, we use read side lock, which can allow marking soft-dirty several vmas with parallel. And pid is not equal to calling process's one, the API should try to hold write-side lock then, if it's fail, the API should return EAGAIN so that CRIU can progress other processes and retry it after a while. Of course, it would make live-lock so that sys_softdirty might need another argument like "int block".

Quoted 3 lines

And we need a flag to show SELF_SOFT_DIRTY or EXTERNAL_SOFT_DIRTY and the flag will be protected by above lock. It could prevent mixed case by self and external.

Quoted 6 lines

I realized it's not enough. Another idea is here. The intenion is followin as,

self softdirty VS self softdirty -> NOT exclusive self softdirty VS external softdirty -> exclusive external softdirty VS external softdirty-> excluisve

I think it might work for us. However, I have two comments to the implementation, please see below.

Quoted 5 lines
struct softdirty token {
        u64 external;
        u64 internal;
};
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

int sys_set_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty *token);

I should have mentioned that start and len are ignored if pid is not eqaul to caller's pid.

OK

Quoted 20 lines
Quoted 5 lines
Quoted 2 lines

int sys_get_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty token, char *vec);

Can you please show an example how to use these two, I don't quite get how can I do external soft-dirty tracking in atomic manner.

Hmm, I don't know how CRIU works but ...

	while(1) {

		struct softdirty token;
		
		sys_set_softdirty(tracked_pid, 0, 0, &token);
		...
		...
		...
		if (!sys_get_softdirty(tacked_pid, 0, 0, token, NULL))
			break;
	}
Quoted 1 line

Maybe do you have a concern about live-lock?

No, I worry about potential races with which we or application can skip dirty page. Let me describe how CRIU uses existing soft-dirty implementation.

1. stop the task we want to work on
2. read the /proc/pid/pagemap file to find out which pages to read. Those with soft-dirty _cleared_ should be _skipped_
3. read task's memory at calculated bitmap
4. reset soft dirty bits on task
5. resume task execution

With the interface you propose the sequence presumably should look like

1. stop the task we want to work on
2. call set_softdirty + get_softdirty to get the soft-dirty bitmap and reset one. If it reports error, then the soft-dirty we did before is spoiled and all memory should be read (iow -- bitmap should be filled with 1-s)
3. read task's memory at calculated bitmap
4. resume task execution

Am I right with this? If yes, why do we need two calls, wouldn't it be better to merge them into one?

Thanks,
Pavel

--
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>

#8 lore raw

Hi Pavel,

Sorry for the delaying.
Maybe our timezone difference and my boys's interrupt.

Quoted 16 lines · Pavel Emelyanov
Quoted 15 lines · Minchan Kim

Hello Pavel,

Quoted 11 lines · Pavel Emelyanov
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

If it's not allowed, another approach should be new system call.

        int sys_softdirty(pid_t pid, void *addr, size_t len);

This looks like existing sys_madvise() one.

Except pid part. It is added by your purpose, which external task can control any process.

In CRIU we can work with pid-less syscalls just fine :) So extending regular madvise would work.

Quoted 7 lines
Quoted 2 lines

I didn't know that.
Just out of curiosity. How can CRIU control other tasks without pid?

We use the parasite-injection technique [1]. Briefly -- we put a code into other task's address space using ptrace() and /proc/PID/map_files/ and make this code run and do what we need. Thus we can call madvise() "on" another task.

Interesting.

Quoted 91 lines

[1] http://lwn.net/Articles/454304/

Quoted 53 lines
Quoted 53 lines
Quoted 38 lines
Quoted 34 lines
Quoted 23 lines
Quoted 17 lines
Quoted 11 lines

If we approach new system call, we don't need to maintain current proc interface and it would be very handy to get a information without pagemap (open/read/close) so we can add a parameter to get a dirty information easily.

int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)

What do you think about it?

This is OK for me, though there's another issue with this API I'd like to mention -- consider your app is doing these tricks with soft-dirty and at the same time CRIU tools live-migrate it using the soft-dirty bits to optimize the freeze time.

Quoted 5 lines

In that case soft-dirty bits would be in wrong state for both -- you app and CRIU, but with the proc API we could compare the ctime-s of the clear_refs file and find out, that someone spoiled the soft-dirty state from last time we messed with it and handle it somehow (copy all the memory in the worst case). Can we somehow handle this with your proposal?

Quoted 10 lines

Good point I didn't think over that.
A simple idea popped from my mind is we can use read/write lock so if pid is equal to calling process's one or pid is NULL, we use read side lock, which can allow marking soft-dirty several vmas with parallel. And pid is not equal to calling process's one, the API should try to hold write-side lock then, if it's fail, the API should return EAGAIN so that CRIU can progress other processes and retry it after a while. Of course, it would make live-lock so that sys_softdirty might need another argument like "int block".

Quoted 3 lines

And we need a flag to show SELF_SOFT_DIRTY or EXTERNAL_SOFT_DIRTY and the flag will be protected by above lock. It could prevent mixed case by self and external.

Quoted 6 lines

I realized it's not enough. Another idea is here. The intenion is followin as,

self softdirty VS self softdirty -> NOT exclusive self softdirty VS external softdirty -> exclusive external softdirty VS external softdirty-> excluisve

I think it might work for us. However, I have two comments to the implementation, please see below.

Quoted 5 lines
struct softdirty token {
        u64 external;
        u64 internal;
};
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

int sys_set_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty *token);

I should have mentioned that start and len are ignored if pid is not eqaul to caller's pid.

OK

Quoted 20 lines
Quoted 5 lines
Quoted 2 lines

int sys_get_softdirty(pid_t pid, unsigned long start, size_t len,
struct softdirty token, char *vec);

Can you please show an example how to use these two, I don't quite get how can I do external soft-dirty tracking in atomic manner.

Hmm, I don't know how CRIU works but ...

	while(1) {

		struct softdirty token;
		
		sys_set_softdirty(tracked_pid, 0, 0, &token);
		...
		...
		...
		if (!sys_get_softdirty(tacked_pid, 0, 0, token, NULL))
			break;
	}
Quoted 1 line

Maybe do you have a concern about live-lock?

No, I worry about potential races with which we or application can skip dirty page. Let me describe how CRIU uses existing soft-dirty implementation.

Quoted 6 lines

1. stop the task we want to work on
2. read the /proc/pid/pagemap file to find out which pages to read. Those with soft-dirty _cleared_ should be _skipped_
3. read task's memory at calculated bitmap
4. reset soft dirty bits on task
5. resume task execution

Let me try to parse as my term.

1. admin does "echo 4 > /proc/<target>/clear_refs"
2. admin stop the target
3. admin reads the /proc/<target>/pagemap and make bitmap with only soft-dirty marked pages so we can avoid unnecessary migration
4. admin reads target's dirtied pages via bitmap from 3
5. admin does "echo 4 > /proc/<target>/clear_refs" again to find future diry pages of the target.
6. admin resumes the target

Right?
If so, my interface is following as

1. admin does set_softdirty(target, 0, 0, &token); (set_softdirty clears all soft-dirty bit from target process's page table.
2. admin stop the target
3. admin reads the /proc/target/pagemap and make bitmap with only soft-dirty marked pages so we can avoid unnecessary migration.
4. admins does get_softdirty(target, 0, 0, token) to confirm someone else spoiled since 1
4-1. If it is reports error, then admins discard the bitmap got
from 3 and have to read all memory.
5. admin does set_softdirty(target, 0, 0, &token) again to find future dirty pages of the target
5. admin resumes the target.

Quoted 12 lines

With the interface you propose the sequence presumably should look like

1. stop the task we want to work on
2. call set_softdirty + get_softdirty to get the soft-dirty bitmap and reset one. If it reports error, then the soft-dirty we did before is spoiled and all memory should be read (iow -- bitmap should be filled with 1-s)
3. read task's memory at calculated bitmap
4. resume task execution

Quoted 1 line

Am I right with this? If yes, why do we need two calls, wouldn't it be better

I failed to parse your terms so I wrote scnario as my understanding so please see my above sequence and if you have a comment, please ask again.

Quoted 1 line

to merge them into one?

It's not hard part but I wanted to show my intention clearly. If we all agree on, let's think over interface again.

Thanks!

Quoted 3 lines

Thanks,
Pavel

Signature · 8 lines
Kind regards,
Minchan Kim

--
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>
#9 lore raw
Quoted 24 lines
Quoted 11 lines
Quoted 1 line

Maybe do you have a concern about live-lock?

No, I worry about potential races with which we or application can skip dirty page. Let me describe how CRIU uses existing soft-dirty implementation.

1. stop the task we want to work on
2. read the /proc/pid/pagemap file to find out which pages to read. Those with soft-dirty _cleared_ should be _skipped_
3. read task's memory at calculated bitmap
4. reset soft dirty bits on task
5. resume task execution

Let me try to parse as my term.

1. admin does "echo 4 > /proc/<target>/clear_refs"
2. admin stop the target
3. admin reads the /proc/<target>/pagemap and make bitmap with only soft-dirty marked pages so we can avoid unnecessary migration
4. admin reads target's dirtied pages via bitmap from 3
5. admin does "echo 4 > /proc/<target>/clear_refs" again to find future diry pages of the target.
6. admin resumes the target

Quoted 1 line

Right?

Almost, the step #1 looks excessive. We shouldn't clear the soft dirty _before_ stopping the target, otherwise we lose all the bits "collected" before it.

Quoted 16 lines

If so, my interface is following as

1. admin does set_softdirty(target, 0, 0, &token); (set_softdirty clears all soft-dirty bit from target process's page table.
2. admin stop the target
3. admin reads the /proc/target/pagemap and make bitmap with only soft-dirty marked pages so we can avoid unnecessary migration.
4. admins does get_softdirty(target, 0, 0, token) to confirm someone else spoiled since 1
4-1. If it is reports error, then admins discard the bitmap got
from 3 and have to read all memory.
5. admin does set_softdirty(target, 0, 0, &token) again to find future dirty pages of the target
5. admin resumes the target.

Same here -- if we skip step #1, then we can merge steps 4 and 5 into one system call. Can we?

Quoted 14 lines
Quoted 12 lines

With the interface you propose the sequence presumably should look like

1. stop the task we want to work on
2. call set_softdirty + get_softdirty to get the soft-dirty bitmap and reset one. If it reports error, then the soft-dirty we did before is spoiled and all memory should be read (iow -- bitmap should be filled with 1-s)
3. read task's memory at calculated bitmap
4. resume task execution

Quoted 1 line

Am I right with this? If yes, why do we need two calls, wouldn't it be better

Quoted 8 lines

I failed to parse your terms so I wrote scnario as my understanding so please see my above sequence and if you have a comment, please ask again.

Quoted 1 line

to merge them into one?

It's not hard part but I wanted to show my intention clearly. If we all agree on, let's think over interface again.

For me the interface with a single syscall looks OK. If nobody else objects, I think you can go on with the kernel patches :) Presumably you can even use the criu project sources and tests to check how memory changes tracking works with the new interface.

Quoted 1 line

Thanks!

Thanks,
Pavel

--
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>

#10 lore raw
Quoted 70 lines · Pavel Emelyanov
Quoted 24 lines
Quoted 11 lines
Quoted 1 line

Maybe do you have a concern about live-lock?

No, I worry about potential races with which we or application can skip dirty page. Let me describe how CRIU uses existing soft-dirty implementation.

1. stop the task we want to work on
2. read the /proc/pid/pagemap file to find out which pages to read. Those with soft-dirty _cleared_ should be _skipped_
3. read task's memory at calculated bitmap
4. reset soft dirty bits on task
5. resume task execution

Let me try to parse as my term.

1. admin does "echo 4 > /proc/<target>/clear_refs"
2. admin stop the target
3. admin reads the /proc/<target>/pagemap and make bitmap with only soft-dirty marked pages so we can avoid unnecessary migration
4. admin reads target's dirtied pages via bitmap from 3
5. admin does "echo 4 > /proc/<target>/clear_refs" again to find future diry pages of the target.
6. admin resumes the target

Quoted 1 line

Right?

Almost, the step #1 looks excessive. We shouldn't clear the soft dirty _before_ stopping the target, otherwise we lose all the bits "collected" before it.

Quoted 16 lines

If so, my interface is following as

1. admin does set_softdirty(target, 0, 0, &token); (set_softdirty clears all soft-dirty bit from target process's page table.
2. admin stop the target
3. admin reads the /proc/target/pagemap and make bitmap with only soft-dirty marked pages so we can avoid unnecessary migration.
4. admins does get_softdirty(target, 0, 0, token) to confirm someone else spoiled since 1
4-1. If it is reports error, then admins discard the bitmap got
from 3 and have to read all memory.
5. admin does set_softdirty(target, 0, 0, &token) again to find future dirty pages of the target
5. admin resumes the target.

Same here -- if we skip step #1, then we can merge steps 4 and 5 into one system call. Can we?

Quoted 20 lines
Quoted 12 lines

With the interface you propose the sequence presumably should look like

1. stop the task we want to work on
2. call set_softdirty + get_softdirty to get the soft-dirty bitmap and reset one. If it reports error, then the soft-dirty we did before is spoiled and all memory should be read (iow -- bitmap should be filled with 1-s)
3. read task's memory at calculated bitmap
4. resume task execution

Quoted 1 line

Am I right with this? If yes, why do we need two calls, wouldn't it be better

I failed to parse your terms so I wrote scnario as my understanding so please see my above sequence and if you have a comment, please ask again.

Quoted 1 line

to merge them into one?

Quoted 7 lines
Quoted 2 lines

It's not hard part but I wanted to show my intention clearly. If we all agree on, let's think over interface again.

For me the interface with a single syscall looks OK. If nobody else objects, I think you can go on with the kernel patches :) Presumably you can even use the criu project sources and tests to check how memory changes tracking works with the new interface.

Thanks for the good discussion!

Signature · 8 lines
Kind regards,
Minchan Kim

--
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>