[PATCH v3] mm: Change return type to vm_fault_t

[PATCH v3] mm: Change return type to vm_fault_t

[Souptick Joarder]

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(-)

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;
-- 
1.9.1

Re: [PATCH v3] mm: Change return type to vm_fault_t

[Joe Perches]

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);

Re: [PATCH v3] mm: Change return type to vm_fault_t

[Souptick Joarder]

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 ?

Re: [PATCH v3] mm: Change return type to vm_fault_t

[Souptick Joarder]

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.

Re: [PATCH v3] mm: Change return type to vm_fault_t

[Matthew Wilcox]

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.

Re: [PATCH v3] mm: Change return type to vm_fault_t

[Souptick Joarder]

>> 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 ?

Re: [PATCH v3] mm: Change return type to vm_fault_t

[Dan Williams]

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.

Re: [PATCH v3] mm: Change return type to vm_fault_t

[Joe Perches]

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