| Message ID | 20180512061712.GA26660@jordon-HP-15-Notebook-PC (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 2018-05-12 at 11:47 +0530, Souptick Joarder wrote: > Use new return type vm_fault_t for fault handler > in struct vm_operations_struct. For now, this is > just documenting that the function returns a > VM_FAULT value rather than an errno. Once all > instances are converted, vm_fault_t will become > a distinct type. trivia: > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h [] > @@ -627,7 +627,7 @@ struct vm_special_mapping { > * If non-NULL, then this is called to resolve page faults > * on the special mapping. If used, .pages is not checked. > */ > - int (*fault)(const struct vm_special_mapping *sm, > + vm_fault_t (*fault)(const struct vm_special_mapping *sm, > struct vm_area_struct *vma, > struct vm_fault *vmf); It'd be nicer to realign the 2nd and 3rd arguments on the subsequent lines. vm_fault_t (*fault)(const struct vm_special_mapping *sm, struct vm_area_struct *vma, struct vm_fault *vmf);
On Sat, May 12, 2018 at 11:50 AM, Joe Perches <joe@perches.com> wrote: > On Sat, 2018-05-12 at 11:47 +0530, Souptick Joarder wrote: >> Use new return type vm_fault_t for fault handler >> in struct vm_operations_struct. For now, this is >> just documenting that the function returns a >> VM_FAULT value rather than an errno. Once all >> instances are converted, vm_fault_t will become >> a distinct type. > > trivia: > >> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > [] >> @@ -627,7 +627,7 @@ struct vm_special_mapping { >> * If non-NULL, then this is called to resolve page faults >> * on the special mapping. If used, .pages is not checked. >> */ >> - int (*fault)(const struct vm_special_mapping *sm, >> + vm_fault_t (*fault)(const struct vm_special_mapping *sm, >> struct vm_area_struct *vma, >> struct vm_fault *vmf); > > > It'd be nicer to realign the 2nd and 3rd arguments > on the subsequent lines. > > vm_fault_t (*fault)(const struct vm_special_mapping *sm, > struct vm_area_struct *vma, > struct vm_fault *vmf); > Just now posted v3. Do you want me to send v4 again with realignment ?
On Sat, May 12, 2018 at 11:55 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote: > On Sat, May 12, 2018 at 11:50 AM, Joe Perches <joe@perches.com> wrote: >> On Sat, 2018-05-12 at 11:47 +0530, Souptick Joarder wrote: >>> Use new return type vm_fault_t for fault handler >>> in struct vm_operations_struct. For now, this is >>> just documenting that the function returns a >>> VM_FAULT value rather than an errno. Once all >>> instances are converted, vm_fault_t will become >>> a distinct type. >> >> trivia: >> >>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h >> [] >>> @@ -627,7 +627,7 @@ struct vm_special_mapping { >>> * If non-NULL, then this is called to resolve page faults >>> * on the special mapping. If used, .pages is not checked. >>> */ >>> - int (*fault)(const struct vm_special_mapping *sm, >>> + vm_fault_t (*fault)(const struct vm_special_mapping *sm, >>> struct vm_area_struct *vma, >>> struct vm_fault *vmf); >> >> >> It'd be nicer to realign the 2nd and 3rd arguments >> on the subsequent lines. >> >> vm_fault_t (*fault)(const struct vm_special_mapping *sm, >> struct vm_area_struct *vma, >> struct vm_fault *vmf); >> > > Just now posted v3. Do you want me to send v4 again with > realignment ? Sorry, please ignore this mail.
On Fri, May 11, 2018 at 11:20:29PM -0700, Joe Perches wrote: > It'd be nicer to realign the 2nd and 3rd arguments > on the subsequent lines. > > vm_fault_t (*fault)(const struct vm_special_mapping *sm, > struct vm_area_struct *vma, > struct vm_fault *vmf); > It'd be nicer if people didn't try to line up arguments at all and just indented by an extra two tabs when they had to break a logical line due to the 80-column limit.
>> It'd be nicer to realign the 2nd and 3rd arguments >> on the subsequent lines. >> >> vm_fault_t (*fault)(const struct vm_special_mapping *sm, >> struct vm_area_struct *vma, >> struct vm_fault *vmf); >> > > It'd be nicer if people didn't try to line up arguments at all and > just indented by an extra two tabs when they had to break a logical > line due to the 80-column limit. Matthew, there are two different opinions. Which one to take ?
On Sat, May 12, 2018 at 12:14 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote: >>> It'd be nicer to realign the 2nd and 3rd arguments >>> on the subsequent lines. > >>> >>> vm_fault_t (*fault)(const struct vm_special_mapping *sm, >>> struct vm_area_struct *vma, >>> struct vm_fault *vmf); >>> >> > >> It'd be nicer if people didn't try to line up arguments at all and >> just indented by an extra two tabs when they had to break a logical >> line due to the 80-column limit. > > Matthew, there are two different opinions. Which one to take ? Unfortunately this is one of those "maintainer's choice" preferences that drives new contributors crazy. Just go with the two tabs like Matthew said and be done.
On Sat, 2018-05-12 at 19:51 -0700, Dan Williams wrote: > On Sat, May 12, 2018 at 12:14 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote: > > > > It'd be nicer to realign the 2nd and 3rd arguments > > > > on the subsequent lines. > > > > > > > > vm_fault_t (*fault)(const struct vm_special_mapping *sm, > > > > struct vm_area_struct *vma, > > > > struct vm_fault *vmf); > > > > > > > It'd be nicer if people didn't try to line up arguments at all and > > > just indented by an extra two tabs when they had to break a logical > > > line due to the 80-column limit. > > > > Matthew, there are two different opinions. Which one to take ? > > Unfortunately this is one of those "maintainer's choice" preferences > that drives new contributors crazy. Just go with the two tabs like > Matthew said and be done. The only reason I mentioned it was the old function name was aligned that way with arguments aligned to the open parenthesis. Renaming the function should keep the same alignment style and not just rename the function. - int (*fault)(const struct vm_special_mapping *sm, + vm_fault_t (*fault)(const struct vm_special_mapping *sm, struct vm_area_struct *vma, struct vm_fault *vmf); Here the previous indent was 2 tabs, 5 spaces
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 2161234..11acfdb 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -627,7 +627,7 @@ struct vm_special_mapping { * If non-NULL, then this is called to resolve page faults * on the special mapping. If used, .pages is not checked. */ - int (*fault)(const struct vm_special_mapping *sm, + vm_fault_t (*fault)(const struct vm_special_mapping *sm, struct vm_area_struct *vma, struct vm_fault *vmf); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 2186791..7e00bd3 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -3159,7 +3159,7 @@ static unsigned long hugetlb_vm_op_pagesize(struct vm_area_struct *vma) * hugegpage VMA. do_page_fault() is supposed to trap this, so BUG is we get * this far. */ -static int hugetlb_vm_op_fault(struct vm_fault *vmf) +static vm_fault_t hugetlb_vm_op_fault(struct vm_fault *vmf) { BUG(); return 0; diff --git a/mm/mmap.c b/mm/mmap.c index 188f195..bdd4ba9a 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -3228,7 +3228,7 @@ void vm_stat_account(struct mm_struct *mm, vm_flags_t flags, long npages) mm->data_vm += npages; } -static int special_mapping_fault(struct vm_fault *vmf); +static vm_fault_t special_mapping_fault(struct vm_fault *vmf); /* * Having a close hook prevents vma merging regardless of flags. @@ -3267,7 +3267,7 @@ static int special_mapping_mremap(struct vm_area_struct *new_vma) .fault = special_mapping_fault, }; -static int special_mapping_fault(struct vm_fault *vmf) +static vm_fault_t special_mapping_fault(struct vm_fault *vmf) { struct vm_area_struct *vma = vmf->vma; pgoff_t pgoff;
Use new return type vm_fault_t for fault handler in struct vm_operations_struct. For now, this is just documenting that the function returns a VM_FAULT value rather than an errno. Once all instances are converted, vm_fault_t will become a distinct type. commit 1c8f422059ae ("mm: change return type to vm_fault_t") Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com> --- v2: updated the change log v3: added <linux/mm_types.h> changes into the same patch include/linux/mm_types.h | 2 +- mm/hugetlb.c | 2 +- mm/mmap.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)