Re: [PATCH] smaps: Fix the abnormal memory statistics obtained through /proc/pid/smaps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27.07.23 22:30, Peter Xu wrote:
On Thu, Jul 27, 2023 at 09:17:45PM +0200, David Hildenbrand wrote:
On 27.07.23 20:59, Peter Xu wrote:
On Thu, Jul 27, 2023 at 07:27:02PM +0200, David Hildenbrand wrote:

This was wrong from the very start. If we're not in GUP, we shouldn't call
GUP functions.

My understanding is !GET && !PIN is also called gup.. otherwise we don't
need GET and it can just be always implied.

That's not the point. The point is that _arbitrary_ code shouldn't call into
GUP internal helper functions, where they bypass, for example, any sanity
checks.

What's the sanity checks that you're referring to?


For example in follow_page()

if (vma_is_secretmem(vma))
	return NULL;

if (WARN_ON_ONCE(foll_flags & FOLL_PIN))
	return NULL;


Maybe you can elaborate why you think we should *not* be using
vm_normal_page_pmd() and instead some arbitrary GUP internal helper? I don't
get it.

Because the old code was written like that?

And it's not 2014 anymore. Nowadays we do have the right helper in place.

[...]

FOLL_NUMA naming was nowadays wrong to begin with (not to mention, confusing
a we learned). There are other reasons why we have PROT_NONE -- mprotect(),
for example.

It doesn't really violate with the name, IMHO - protnone can be either numa
hint or PROT_NONE for real. As long as we return NULL for a FOLL_NUMA
request we're achieving the goal we want - we guarantee a NUMA balancing to
trigger with when FOLL_NUMA provided.  It doesn't need to guarantee
anything else, afaiu.  The final check relies in vma_is_accessible() in the
fault paths anyway.  So I don't blame the old name that much.

IMHO, the name FOLL_NUMA made sense when it still was called pte_numa.



We could have a flag that goes the other way around: FOLL_IGNORE_PROTNONE
... which surprisingly then ends up being exactly what FOLL_FORCE means
without FOLL_WRITE, and what this patch does.

Does that make sense to you?



The very least is if with above we should really document FOLL_FORCE - we
should mention NUMA effects.  But that's ... really confusing. Thinking
about that I personally prefer a revival of FOLL_NUMA, then smaps issue all
go away.

smaps needs to be changed in any case IMHO. And I'm absolutely not in favor
of revicing FOLL_NUMA.

As stated above, to me FOLL_NUMA is all fine and clear.  If you think
having a flag back for protnone is worthwhile no matter as-is (FOLL_NUMA)
or with reverted meaning, then that sounds all fine to me. Maybe the old
name at least makes old developers know what's that.

I don't have a strong opinion on names though; mostly never had.

I'll avoid new FOLL_ flags first and post my proposal. If many people are unhappy with that approach, we can revert the commit and call it a day.

Thanks!

--
Cheers,

David / dhildenb




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux