search

dolohow / uksm

Ultra Kernel Samepage Merging
241 stars 35 forks source link

Merging uksm upstream? #41

Open Harvie opened 5 years ago

Harvie commented 5 years ago

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

dolohow commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

dolohow commented 5 years ago

@Szpadel Thanks for kind words :)

dolohow commented 5 years ago

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

demfloro commented 5 years ago

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 commented 5 years ago

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

dolohow commented 5 years ago

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 commented 4 years ago

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 commented 3 years ago

Any progress on this?

Evengard commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

@dolohow

SharkWipf commented 2 years ago

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 commented 2 years ago

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.