Messages in this thread | | | Subject | Re: [PATCH v9 2/3] mm: add a field to store names for private anonymous memory | From | Rasmus Villemoes <> | Date | Fri, 1 Oct 2021 09:01:23 +0200 |
| |
On 03/09/2021 01.18, Suren Baghdasaryan wrote: > From: Colin Cross <ccross@google.com> > > > changes in v9 > - Changed max anon vma name length from 64 to 256 (as in the original patch) > because I found one case of the name length being 139 bytes. If anyone is > curious, here it is: > dalvik-/data/dalvik-cache/arm64/apex@com.android.permission@priv-app@GooglePermissionController@GooglePermissionController.apk@classes.art
I'm not sure that's a very convincing argument. We don't add code arbitrarily just because some userspace code running on some custom kernel (ab)uses something in that kernel. Surely that user can come up with a name that doesn't contain GooglePermissionController twice.
The argument for using strings and not just a 128 bit uuid was that it should (also) be human readable, and 250-byte strings are not that. Also, there's no natural law forcing this to be some power-of-two, and in fact the implementation means that it's actually somewhat harmful (give it a 256 char name, and we'll do a 260 byte alloc, which becomes a 512 byte alloc). So just make the limit 80, the kernel's definition of a sane line length. As for the allowed chars, it can be relaxed later if convincing arguments can be made.
> +/* mmap_lock should be read-locked */ > +static inline bool is_same_vma_anon_name(struct vm_area_struct *vma, > + const char *name) > +{ > + const char *vma_name = vma_anon_name(vma); > + > + if (likely(!vma_name)) > + return name == NULL; > + > + return name && !strcmp(name, vma_name);
It's probably preferable to spell this
/* either both NULL, or pointers to same refcounted string */ if (vma_name == name) return true;
return name && vma_name && !strcmp(name, vma_name);
so you have one less conditional in the common case.
Rasmus
| |