On Thu, Oct 19, 2017 at 06:48:58AM +1100, Balbir Singh wrote:
> On Wed, 18 Oct 2017 12:40:43 +0530
> Anshuman Khandual <khandual@xxxxxxxxxxxxxxxxxx> wrote:
>
> > On 10/18/2017 12:01 PM, Balbir Singh wrote:
> > > With HMM, we can have either public or private zone
> > > device pages. With private zone device pages, they should
> > > show up as swapped entities. For public zone device pages
> >
> > Might be missing something here but why they should show up
> > as swapped entities ? Could you please elaborate.
> >
>
> For migrated entries, my use case is to
>
> 1. malloc()/mmap() memory
> 2. call migrate_vma()
> 3. Look at smaps
>
> It's probably not clear in the changelog.
My only worry is about API, is smaps consider as userspace API ?
My fear here is that maybe we will want to report device memory
differently in the future and have different category of device
memory. Even thought right now i can only think of wanting to
differentiate between public and private device memory but right
now as you pointed out this is reported as swap out.
Otherwise patches looks good and you got:
Reviewed-by: Jérôme Glisse <jglisse@xxxxxxxxxx>
>
> > > the smaps output can be confusing and incomplete.
> > >
> > > This patch adds a new attribute to just smaps to show
> > > device memory usage.
> >
> > If we are any way adding a new entry here then why not one
> > more for private device memory pages as well. Just being
> > curious.
> >
>
> Well, how do you define visibility of device private memory?
> Device private is either seen as swapped out or when migrated
> back is visible as a part of the mm. Am I missing anything?
>
> > >
> > > Signed-off-by: Balbir Singh <bsingharora@xxxxxxxxx>
> > > ---
> > > fs/proc/task_mmu.c | 17 +++++++++++++++--
> > > 1 file changed, 15 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > > index 9f1e2b2b5f5a..b7f32f42ee93 100644
> > > --- a/fs/proc/task_mmu.c
> > > +++ b/fs/proc/task_mmu.c
> > > @@ -451,6 +451,7 @@ struct mem_size_stats {
> > > unsigned long shared_hugetlb;
> > > unsigned long private_hugetlb;
> > > unsigned long first_vma_start;
> > > + unsigned long device_memory;
> > > u64 pss;
> > > u64 pss_locked;
> > > u64 swap_pss;
> > > @@ -463,12 +464,22 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page,
> > > int i, nr = compound ? 1 << compound_order(page) : 1;
> > > unsigned long size = nr * PAGE_SIZE;
> > >
> > > + /*
> > > + * We don't want to process public zone device pages further
> > > + * than just showing how much device memory we have
> > > + */
> > > + if (is_zone_device_page(page)) {
> >
> > Should not this contain both public and private device pages.
> >
>
> This page is received from _vm_normal_page(.., true), I don't
> think device private pages show up here.
>
> > > + mss->device_memory += size;
> > > + return;
> > > + }
> > > +
> > > if (PageAnon(page)) {
> > > mss->anonymous += size;
> > > if (!PageSwapBacked(page) && !dirty && !PageDirty(page))
> > > mss->lazyfree += size;
> > > }
> > >
> > > +
> >
> > Stray new line.
> >
>
> I can remove it
>
> > > mss->resident += size;
> > > /* Accumulate the size in pages that have been accessed. */
> > > if (young || page_is_young(page) || PageReferenced(page))
> > > @@ -833,7 +844,8 @@ static int show_smap(struct seq_file *m, void *v, int is_pid)
> > > "Private_Hugetlb: %7lu kB\n"
> > > "Swap: %8lu kB\n"
> > > "SwapPss: %8lu kB\n"
> > > - "Locked: %8lu kB\n",
> > > + "Locked: %8lu kB\n"
> >
> > Stray changed line.
>
> ?? The line has changed
>
>
> Thanks for the review!
>
> Balbir Singh.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
