On Fri, Jun 09, 2023 at 07:09:00PM +0800, jim.tsai wrote:
> Export symbols for arch_vma_name and smap_gather_stats
> functions so that we can detect the memory leak issues.
> Besides, we can know which memory type is leaked, too.
This commit description doesn't give enough information. How does
exporting arch_vma_name() help with detecting memory leak issues?
You haven't included any users of these new exports, so the initial
reaction is going to be negative - please include the users of these
new symbols in your patch set.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Fri, 2023-06-09 at 12:20 +0100, Russell King (Oracle) wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On Fri, Jun 09, 2023 at 07:09:00PM +0800, jim.tsai wrote:
> > Export symbols for arch_vma_name and smap_gather_stats
> > functions so that we can detect the memory leak issues.
> > Besides, we can know which memory type is leaked, too.
>
> This commit description doesn't give enough information. How does
> exporting arch_vma_name() help with detecting memory leak issues?
>
> You haven't included any users of these new exports, so the initial
> reaction is going to be negative - please include the users of these
> new symbols in your patch set.
>
> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
arch_vma_name() is to get the heap infromation from a user process.
We use these two export functions from our kernel module to get a
specific user process's memory information and heap usage. Furthermore,
we can use such information to detect the memory leak issues.
The example code is as follows:
/*example code*/
void m_map_vma(struct vm_area_struct *vma, unsigned long cur_pss,
unsigned long *native_heap, unsigned long
*java_heap)
{
struct mm_struct *mm = vma->vm_mm;
const char *name = NULL;
...
name = arch_vma_name(vma);
if (!name) {
struct anon_vma_name *anon_name;
...
anon_name = m_anon_vma_name(vma);
if (anon_name) {
if (strstr(anon_name->name, "scudo"))
(*native_heap) += cur_pss;
else if (strstr(anon_name->name,
"libc_malloc"))
(*native_heap) += cur_pss;
else if (strstr(anon_name->name, "GWP-ASan"))
(*native_heap) += cur_pss;
else if (strstr(anon_name->name, "dalvik-alloc
space"))
(*java_heap) += cur_pss;
else if (strstr(anon_name->name, "dalvik-main
space"))
(*java_heap) += cur_pss;
...
}
}
}
void calculate_process_memory(struct task_struct *t)
{
struct mm_struct *mm = NULL;
struct vm_area_struct *vma = NULL;
struct mem_size_stats mss;
unsigned long pss, uss, rss, swap, cur_pss;
unsigned long java_heap = 0, native_heap = 0;
struct vma_iterator vmi;
get_task_struct(t);
mm = t->mm;
if (mm) {
memset(&mss, 0, sizeof(mss));
mmgrab(mm);
mmap_read_lock(mm);
vma_iter_init(&vmi, mm, 0);
for_each_vma(vmi, vma) {
cur_pss = (unsigned long)(mss.pss >>
PSS_SHIFT);
smap_gather_stats(vma, &mss, 0);
cur_pss =
((unsigned long)(mss.pss >> PSS_SHIFT))
- cur_pss;
cur_pss = cur_pss / 1024;
m_map_vma(vma, cur_pss, &native_heap,
&java_heap);
}
mmap_read_unlock(mm);
mmdrop(mm);
}
put_task_struct(t);
...
}
Thanks
Jim
On Fri, Jun 09, 2023 at 04:09:01PM +0000, Wei-chin Tsai (蔡維晉) wrote:
> > You haven't included any users of these new exports, so the initial
> > reaction is going to be negative - please include the users of these
> > new symbols in your patch set.
> We use these two export functions from our kernel module to get a
> specific user process's memory information and heap usage. Furthermore,
> we can use such information to detect the memory leak issues.
>
> The example code is as follows:
No. You need to be submitting the code that will use the symbol *at the
same time* as the patch to export the symbol. No example code showing
how it could be used. Because if the user isn't compelling, the patch
to export the symbol won't be applied either.
On Sat, 2023-06-10 at 01:21 +0100, Matthew Wilcox wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On Fri, Jun 09, 2023 at 04:09:01PM +0000, Wei-chin Tsai (蔡維晉) wrote:
> > > You haven't included any users of these new exports, so the
> initial
> > > reaction is going to be negative - please include the users of
> these
> > > new symbols in your patch set.
> > We use these two export functions from our kernel module to get a
> > specific user process's memory information and heap usage.
> Furthermore,
> > we can use such information to detect the memory leak issues.
> >
> > The example code is as follows:
>
> No. You need to be submitting the code that will use the symbol *at
> the
> same time* as the patch to export the symbol. No example code
> showing
> how it could be used. Because if the user isn't compelling, the
> patch
> to export the symbol won't be applied either.
Hi Matthew,
Got it. The following attached patch file
"v1-0001-memory-export-symbols-for-process-memory-related-.patch" is
the patch including the users of these new symbols. Thanks.
Regards,
Jim
On 12/06/2023 16:21, Wei-chin Tsai (蔡維晉) wrote:
> On Sat, 2023-06-10 at 01:21 +0100, Matthew Wilcox wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> On Fri, Jun 09, 2023 at 04:09:01PM +0000, Wei-chin Tsai (蔡維晉) wrote:
>> > > You haven't included any users of these new exports, so the
>> initial
>> > > reaction is going to be negative - please include the users of
>> these
>> > > new symbols in your patch set.
>> > We use these two export functions from our kernel module to get a
>> > specific user process's memory information and heap usage.
>> Furthermore,
>> > we can use such information to detect the memory leak issues.
>> >
>> > The example code is as follows:
>>
>> No. You need to be submitting the code that will use the symbol *at
>> the
>> same time* as the patch to export the symbol. No example code
>> showing
>> how it could be used. Because if the user isn't compelling, the
>> patch
>> to export the symbol won't be applied either.
>
> Hi Matthew,
>
> Got it. The following attached patch file
> "v1-0001-memory-export-symbols-for-process-memory-related-.patch" is
> the patch including the users of these new symbols. Thanks.
Please send both patches as a single series, then we can start review process. I
had a very quick look on the attached patch and it's missing a good commit
message describing what the drivers is for and why you need it.
Regards,
Matthias
>
> Regards,
>
> Jim
>
> ************* MEDIATEK Confidentiality Notice ********************
> The information contained in this e-mail message (including any
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be
> conveyed only to the designated recipient(s). Any use, dissemination,
> distribution, printing, retaining or copying of this e-mail (including its
> attachments) by unintended recipient(s) is strictly prohibited and may
> be unlawful. If you are not an intended recipient of this e-mail, or believe
> that you have received this e-mail in error, please notify the sender
> immediately (by replying to this e-mail), delete any and all copies of
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!