Re: [PATCH 3/8] Uprobe: Rename map_info to uprobe_map_info
On 03/15/2018 10:14 PM, Steven Rostedt wrote: > On Tue, 13 Mar 2018 18:25:58 +0530 > Ravi Bangoriawrote: >> -static inline struct map_info *free_map_info(struct map_info *info) >> +static inline struct uprobe_map_info * >> +uprobe_free_map_info(struct uprobe_map_info *info) >> { >> -struct map_info *next = info->next; >> +struct uprobe_map_info *next = info->next; >> kfree(info); >> return next; >> } >> >> -static struct map_info * >> -build_map_info(struct address_space *mapping, loff_t offset, bool >> is_register) >> +static struct uprobe_map_info * >> +uprobe_build_map_info(struct address_space *mapping, loff_t offset, > Also, as these functions have side effects (like you need to perform a > mmput(info->mm), you need to add kerneldoc type comments to these > functions, explaining how to use them. > > When you upgrade a function from static to use cases outside the file, > it requires documenting that function for future users. Sure, will add a comment here. Thanks for the review, Ravi -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] Uprobe: Rename map_info to uprobe_map_info
On Tue, 13 Mar 2018 18:25:58 +0530 Ravi Bangoriawrote: > -static inline struct map_info *free_map_info(struct map_info *info) > +static inline struct uprobe_map_info * > +uprobe_free_map_info(struct uprobe_map_info *info) > { > - struct map_info *next = info->next; > + struct uprobe_map_info *next = info->next; > kfree(info); > return next; > } > > -static struct map_info * > -build_map_info(struct address_space *mapping, loff_t offset, bool > is_register) > +static struct uprobe_map_info * > +uprobe_build_map_info(struct address_space *mapping, loff_t offset, Also, as these functions have side effects (like you need to perform a mmput(info->mm), you need to add kerneldoc type comments to these functions, explaining how to use them. When you upgrade a function from static to use cases outside the file, it requires documenting that function for future users. -- Steve > + bool is_register) > { > unsigned long pgoff = offset >> PAGE_SHIFT; > struct vm_area_struct *vma; > - struct map_info *curr = NULL; > - struct map_info *prev = NULL; > - struct map_info *info; > + struct uprobe_map_info *curr = NULL; > + struct uprobe_map_info *prev = NULL; > + struct uprobe_map_info *info; > int more = 0; > > again: > @@ -729,7 +731,7 @@ static inline struct map_info *free_map_info(struct > map_info *info) >* Needs GFP_NOWAIT to avoid i_mmap_rwsem recursion > through >* reclaim. This is optimistic, no harm done if it > fails. >*/ > - prev = kmalloc(sizeof(struct map_info), > + prev = kmalloc(sizeof(struct uprobe_map_info), > GFP_NOWAIT | __GFP_NOMEMALLOC | > __GFP_NOWARN); > if (prev) > prev->next = NULL; > @@ -762,7 +764,7 @@ static inline struct map_info *free_map_info(struct > map_info *info) > } > > do { > - info = kmalloc(sizeof(struct map_info), GFP_KERNEL); > + info = kmalloc(sizeof(struct uprobe_map_info), GFP_KERNEL); > if (!info) { > curr = ERR_PTR(-ENOMEM); > goto out; > @@ -774,7 +776,7 @@ static inline struct map_info *free_map_info(struct > map_info *info) > goto again; > out: > while (prev) > - prev = free_map_info(prev); > + prev = uprobe_free_map_info(prev); > return curr; > } > > @@ -782,11 +784,11 @@ static inline struct map_info *free_map_info(struct > map_info *info) > register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) > { > bool is_register = !!new; > - struct map_info *info; > + struct uprobe_map_info *info; > int err = 0; > > percpu_down_write(_mmap_sem); > - info = build_map_info(uprobe->inode->i_mapping, > + info = uprobe_build_map_info(uprobe->inode->i_mapping, > uprobe->offset, is_register); > if (IS_ERR(info)) { > err = PTR_ERR(info); > @@ -825,7 +827,7 @@ static inline struct map_info *free_map_info(struct > map_info *info) > up_write(>mmap_sem); > free: > mmput(mm); > - info = free_map_info(info); > + info = uprobe_free_map_info(info); > } > out: > percpu_up_write(_mmap_sem); -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] Uprobe: Rename map_info to uprobe_map_info
On Tue, Mar 13, 2018 at 06:25:58PM +0530, Ravi Bangoria wrote: > map_info is very generic name, rename it to uprobe_map_info. > Renaming will help to export this structure outside of the > file. > > Also rename free_map_info() to uprobe_free_map_info() and > build_map_info() to uprobe_build_map_info(). > > No functionality changes. > > Signed-off-by: Ravi BangoriaSame coccinelle comments as previously. Reviewed-by: Jérôme Glisse > --- > kernel/events/uprobes.c | 32 +--- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 535fd39..081b88c1 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -695,27 +695,29 @@ static void delete_uprobe(struct uprobe *uprobe) > put_uprobe(uprobe); > } > > -struct map_info { > - struct map_info *next; > +struct uprobe_map_info { > + struct uprobe_map_info *next; > struct mm_struct *mm; > unsigned long vaddr; > }; > > -static inline struct map_info *free_map_info(struct map_info *info) > +static inline struct uprobe_map_info * > +uprobe_free_map_info(struct uprobe_map_info *info) > { > - struct map_info *next = info->next; > + struct uprobe_map_info *next = info->next; > kfree(info); > return next; > } > > -static struct map_info * > -build_map_info(struct address_space *mapping, loff_t offset, bool > is_register) > +static struct uprobe_map_info * > +uprobe_build_map_info(struct address_space *mapping, loff_t offset, > + bool is_register) > { > unsigned long pgoff = offset >> PAGE_SHIFT; > struct vm_area_struct *vma; > - struct map_info *curr = NULL; > - struct map_info *prev = NULL; > - struct map_info *info; > + struct uprobe_map_info *curr = NULL; > + struct uprobe_map_info *prev = NULL; > + struct uprobe_map_info *info; > int more = 0; > > again: > @@ -729,7 +731,7 @@ static inline struct map_info *free_map_info(struct > map_info *info) >* Needs GFP_NOWAIT to avoid i_mmap_rwsem recursion > through >* reclaim. This is optimistic, no harm done if it > fails. >*/ > - prev = kmalloc(sizeof(struct map_info), > + prev = kmalloc(sizeof(struct uprobe_map_info), > GFP_NOWAIT | __GFP_NOMEMALLOC | > __GFP_NOWARN); > if (prev) > prev->next = NULL; > @@ -762,7 +764,7 @@ static inline struct map_info *free_map_info(struct > map_info *info) > } > > do { > - info = kmalloc(sizeof(struct map_info), GFP_KERNEL); > + info = kmalloc(sizeof(struct uprobe_map_info), GFP_KERNEL); > if (!info) { > curr = ERR_PTR(-ENOMEM); > goto out; > @@ -774,7 +776,7 @@ static inline struct map_info *free_map_info(struct > map_info *info) > goto again; > out: > while (prev) > - prev = free_map_info(prev); > + prev = uprobe_free_map_info(prev); > return curr; > } > > @@ -782,11 +784,11 @@ static inline struct map_info *free_map_info(struct > map_info *info) > register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) > { > bool is_register = !!new; > - struct map_info *info; > + struct uprobe_map_info *info; > int err = 0; > > percpu_down_write(_mmap_sem); > - info = build_map_info(uprobe->inode->i_mapping, > + info = uprobe_build_map_info(uprobe->inode->i_mapping, > uprobe->offset, is_register); > if (IS_ERR(info)) { > err = PTR_ERR(info); > @@ -825,7 +827,7 @@ static inline struct map_info *free_map_info(struct > map_info *info) > up_write(>mmap_sem); > free: > mmput(mm); > - info = free_map_info(info); > + info = uprobe_free_map_info(info); > } > out: > percpu_up_write(_mmap_sem); > -- > 1.8.3.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] Uprobe: Rename map_info to uprobe_map_info
map_info is very generic name, rename it to uprobe_map_info. Renaming will help to export this structure outside of the file. Also rename free_map_info() to uprobe_free_map_info() and build_map_info() to uprobe_build_map_info(). No functionality changes. Signed-off-by: Ravi Bangoria--- kernel/events/uprobes.c | 32 +--- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 535fd39..081b88c1 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -695,27 +695,29 @@ static void delete_uprobe(struct uprobe *uprobe) put_uprobe(uprobe); } -struct map_info { - struct map_info *next; +struct uprobe_map_info { + struct uprobe_map_info *next; struct mm_struct *mm; unsigned long vaddr; }; -static inline struct map_info *free_map_info(struct map_info *info) +static inline struct uprobe_map_info * +uprobe_free_map_info(struct uprobe_map_info *info) { - struct map_info *next = info->next; + struct uprobe_map_info *next = info->next; kfree(info); return next; } -static struct map_info * -build_map_info(struct address_space *mapping, loff_t offset, bool is_register) +static struct uprobe_map_info * +uprobe_build_map_info(struct address_space *mapping, loff_t offset, + bool is_register) { unsigned long pgoff = offset >> PAGE_SHIFT; struct vm_area_struct *vma; - struct map_info *curr = NULL; - struct map_info *prev = NULL; - struct map_info *info; + struct uprobe_map_info *curr = NULL; + struct uprobe_map_info *prev = NULL; + struct uprobe_map_info *info; int more = 0; again: @@ -729,7 +731,7 @@ static inline struct map_info *free_map_info(struct map_info *info) * Needs GFP_NOWAIT to avoid i_mmap_rwsem recursion through * reclaim. This is optimistic, no harm done if it fails. */ - prev = kmalloc(sizeof(struct map_info), + prev = kmalloc(sizeof(struct uprobe_map_info), GFP_NOWAIT | __GFP_NOMEMALLOC | __GFP_NOWARN); if (prev) prev->next = NULL; @@ -762,7 +764,7 @@ static inline struct map_info *free_map_info(struct map_info *info) } do { - info = kmalloc(sizeof(struct map_info), GFP_KERNEL); + info = kmalloc(sizeof(struct uprobe_map_info), GFP_KERNEL); if (!info) { curr = ERR_PTR(-ENOMEM); goto out; @@ -774,7 +776,7 @@ static inline struct map_info *free_map_info(struct map_info *info) goto again; out: while (prev) - prev = free_map_info(prev); + prev = uprobe_free_map_info(prev); return curr; } @@ -782,11 +784,11 @@ static inline struct map_info *free_map_info(struct map_info *info) register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) { bool is_register = !!new; - struct map_info *info; + struct uprobe_map_info *info; int err = 0; percpu_down_write(_mmap_sem); - info = build_map_info(uprobe->inode->i_mapping, + info = uprobe_build_map_info(uprobe->inode->i_mapping, uprobe->offset, is_register); if (IS_ERR(info)) { err = PTR_ERR(info); @@ -825,7 +827,7 @@ static inline struct map_info *free_map_info(struct map_info *info) up_write(>mmap_sem); free: mmput(mm); - info = free_map_info(info); + info = uprobe_free_map_info(info); } out: percpu_up_write(_mmap_sem); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html