[Cc Jann] On Fri 01-07-22 08:43:23, cgel.zte@xxxxxxxxx wrote: > From: xu xin <xu.xin16@xxxxxxxxxx> > > The benefits of doing this are obvious because using madvise in user code > is the only current way to enable KSM, which is inconvenient for those > compiled app without marking MERGEABLE wanting to enable KSM. I would rephrase: " KSM functionality is currently available only to processes which are using MADV_MERGEABLE directly. This is limiting because there are usecases which will benefit from enabling KSM on a remote process. One example would be an application which cannot be modified (e.g. because it is only distributed as a binary). MORE EXAMPLES WOULD BE REALLY BENEFICIAL. " > Since we already have the syscall of process_madvise(), then reusing the > interface to allow external KSM hints is more acceptable [1]. > > Although this patch was released by Oleksandr Natalenko, but it was > unfortunately terminated without any conclusions because there was debate > on whether it should use signal_pending() to check the target task besides > the task of current() when calling unmerge_ksm_pages of other task [2]. I am not sure this is particularly interesting. I do not remember details of that discussion but checking signal_pending on a different task is rarely the right thing to do. In this case the check is meant to allow bailing out from the operation so that the caller could be terminated for example. > I think it's unneeded to check the target task. For example, when we set > the klob /sys/kernel/mm/ksm/run from 1 to 2, > unmerge_and_remove_all_rmap_items() doesn't use signal_pending() to check > all other target tasks either. > > I hope this patch can get attention again. One thing that the changelog is missing and it is quite important IMHO is the permission model. As we have discussed in previous incarnations of the remote KSM functionality that KSM has some security implications. It would be really great to refer to that in the changelog for the future reference (http://lkml.kernel.org/r/CAG48ez0riS60zcA9CC9rUDV=kLS0326Rr23OKv1_RHaTkOOj7A@xxxxxxxxxxxxxx) So this implementation requires PTRACE_MODE_READ_FSCREDS and CAP_SYS_NICE so the remote process would need to be allowed to introspect the address space. This is the same constrain applied to the remote momory reclaim. Is this sufficient? I would say yes because to some degree KSM mergning can have very similar effect to memory reclaim from the side channel POV. But it should be really documented in the changelog so that it is clear that this has been a deliberate decision and thought through. Other than that this looks like the most reasonable approach to me. > [1] https://lore.kernel.org/lkml/YoOrdh85+AqJH8w1@xxxxxxxxxxxxxx/ > [2] https://lore.kernel.org/lkml/2a66abd8-4103-f11b-06d1-07762667eee6@xxxxxxx/ > > Signed-off-by: Oleksandr Natalenko <oleksandr@xxxxxxxxxx> > Signed-off-by: xu xin <xu.xin16@xxxxxxxxxx> > --- > mm/madvise.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/madvise.c b/mm/madvise.c > index 851fa4e134bc..df915531ad9f 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -1173,6 +1173,10 @@ process_madvise_behavior_valid(int behavior) > case MADV_COLD: > case MADV_PAGEOUT: > case MADV_WILLNEED: > +#ifdef CONFIG_KSM > + case MADV_MERGEABLE: > + case MADV_UNMERGEABLE: > +#endif > return true; > default: > return false; > -- > 2.25.1 -- Michal Hocko SUSE Labs
