Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Merging uksm upstream? #41

Open
Harvie opened this issue Mar 31, 2019 · 16 comments
Open

Merging uksm upstream? #41

Harvie opened this issue Mar 31, 2019 · 16 comments

Comments

@Harvie
Copy link

Harvie commented Mar 31, 2019

Hello,
can we expect this to be merged upstream? are there some reasons why this might not be included?

@dolohow
Copy link
Owner

dolohow commented Apr 1, 2019

I have no plans. I think it would be a lot of work and this concept introduce security risks. That would open kernel for new vulnerabilities.

@Harvie
Copy link
Author

Harvie commented Apr 1, 2019

I think it would be a lot of work

But it would be work that leads somewhere. Just maintaining these patches outside of kernel tree is also lot of work, but it's only useful in the short run. All similar projects that maintain kernel patches outside upstream eventualy get tired and cease to maintain their codebase...

That would open kernel for new vulnerabilities.

Well. That might be true. I don't fully understand how this side-chanel atack vector on samepage merging is supposed to work. But there are two concerns:

1.) Kernel already contains KSM, which probably suffers from the same problem (eg. attacking one Qemu VM from another) and nobody seems to complain...

2.) I think this is OK as long as it's disabled by default and users are warned about the risks they can face if they're going to enable it.

@dolohow
Copy link
Owner

dolohow commented Apr 25, 2019

I'll try to do that, but

  1. I am not the initial creator of those patches.
  2. I have limited time to adjust the code to fit upstream needs.
    and I'll probably fail with this effort, but at least I'll try

@Szpadel
Copy link
Contributor

Szpadel commented Apr 25, 2019

If you put upstream feedback here, then we (community) could help with some issues together

@dolohow
Copy link
Owner

dolohow commented Apr 25, 2019

@Szpadel Thanks for kind words :)

@dolohow
Copy link
Owner

dolohow commented Sep 25, 2019

I will post those patches in the next merge window as they are and let's see what developers will say.

@demfloro
Copy link

We know that current patch is unacceptable for merge from previous attempt. We should send the patch as is at any time since during merge window there will be no time to review it.

@dolohow
Copy link
Owner

dolohow commented Sep 26, 2019

So there was an attempt, let me check...

@dolohow
Copy link
Owner

dolohow commented Sep 30, 2019

Looks like a bit of a work... i will try to do some coding, but can't promise anything since I am very short with time :(

@Szpadel
Copy link
Contributor

Szpadel commented Nov 15, 2019

5.4 got some fixes to zram race conditions, maybe those issues are resolved now

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7daefe4231e57381d92c2e2ad905a899c28e402

@soredake
Copy link

soredake commented Aug 3, 2021

Any progress on this?

@Harvie Harvie changed the title Merging ukms upstream? Merging uksm upstream? Aug 3, 2021
@Evengard
Copy link

I would love to see that integrated into the main kernel, it would benefit so many systems. Is there any status updates on that?

@Harvie
Copy link
Author

Harvie commented Sep 24, 2021

On a side note, there are threads for this on Proxmox website as well:
https://forum.proxmox.com/threads/can-you-please-add-uksm-into-kernel.32778/
https://bugzilla.proxmox.com/show_bug.cgi?id=3637

@soredake
Copy link

@dolohow

@SharkWipf
Copy link

The problem with merging this upstream is that there's no-one who really understands the code to maintain it. IIRC, @dolohow stepped up to update the old patches, but did not write them, nor has a complete understanding of how the code works.
While updating the code manually would not be necessary anymore, without anyone capable of fixing bugs or making improvements, merging upstream would just leave it unmaintained in-kernel, with all the issues that come with that.
Not only would this be difficult to get approved to merge, its value would also be debatable.
Without a maintainer stepping up with understanding of the code I don't see this getting merged upstream.

@AGenchev
Copy link

AGenchev commented Dec 9, 2021

Thank you guys for working on this. I'm grateful to the original authors as well. The kernel coding is a bit of black art for me.... yet this code works for me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants