Re: [RFC v3 PATCH 3/5] mm: refactor do_munmap() to extract the common part

2018-07-02 Thread Yang Shi




On 7/2/18 10:58 AM, Michal Hocko wrote:

On Mon 02-07-18 09:59:06, Yang Shi wrote:


On 7/2/18 6:42 AM, Michal Hocko wrote:

On Sat 30-06-18 06:39:43, Yang Shi wrote:

Introduces two new helper functions:
* munmap_addr_sanity()
* munmap_lookup_vma()

They will be used by do_munmap() and the new do_munmap with zapping
large mapping early in the later patch.

There is no functional change, just code refactor.

There are whitespace changes which make the code much harder to review
than necessary.

+static inline bool munmap_addr_sanity(unsigned long start, size_t len)
   {
-   unsigned long end;
-   struct vm_area_struct *vma, *prev, *last;
+   if ((offset_in_page(start)) || start > TASK_SIZE || len > TASK_SIZE - 
start)
+   return false;
-   if ((offset_in_page(start)) || start > TASK_SIZE || len > 
TASK_SIZE-start)
-   return -EINVAL;

e.g. here.

Oh, yes. I did some coding style cleanup too.

If you want to do some coding style cleanups make them a separate patch.
The resulting diff would be much easier to review.


Sure, thanks







Re: [RFC v3 PATCH 3/5] mm: refactor do_munmap() to extract the common part

2018-07-02 Thread Michal Hocko
On Mon 02-07-18 09:59:06, Yang Shi wrote:
> 
> 
> On 7/2/18 6:42 AM, Michal Hocko wrote:
> > On Sat 30-06-18 06:39:43, Yang Shi wrote:
> > > Introduces two new helper functions:
> > >* munmap_addr_sanity()
> > >* munmap_lookup_vma()
> > > 
> > > They will be used by do_munmap() and the new do_munmap with zapping
> > > large mapping early in the later patch.
> > > 
> > > There is no functional change, just code refactor.
> > There are whitespace changes which make the code much harder to review
> > than necessary.
> > > +static inline bool munmap_addr_sanity(unsigned long start, size_t len)
> > >   {
> > > - unsigned long end;
> > > - struct vm_area_struct *vma, *prev, *last;
> > > + if ((offset_in_page(start)) || start > TASK_SIZE || len > TASK_SIZE - 
> > > start)
> > > + return false;
> > > - if ((offset_in_page(start)) || start > TASK_SIZE || len > 
> > > TASK_SIZE-start)
> > > - return -EINVAL;
> > e.g. here.
> 
> Oh, yes. I did some coding style cleanup too.

If you want to do some coding style cleanups make them a separate patch.
The resulting diff would be much easier to review.

-- 
Michal Hocko
SUSE Labs


Re: [RFC v3 PATCH 3/5] mm: refactor do_munmap() to extract the common part

2018-07-02 Thread Yang Shi




On 7/2/18 6:42 AM, Michal Hocko wrote:

On Sat 30-06-18 06:39:43, Yang Shi wrote:

Introduces two new helper functions:
   * munmap_addr_sanity()
   * munmap_lookup_vma()

They will be used by do_munmap() and the new do_munmap with zapping
large mapping early in the later patch.

There is no functional change, just code refactor.

There are whitespace changes which make the code much harder to review
than necessary.

+static inline bool munmap_addr_sanity(unsigned long start, size_t len)
  {
-   unsigned long end;
-   struct vm_area_struct *vma, *prev, *last;
+   if ((offset_in_page(start)) || start > TASK_SIZE || len > TASK_SIZE - 
start)
+   return false;
  
-	if ((offset_in_page(start)) || start > TASK_SIZE || len > TASK_SIZE-start)

-   return -EINVAL;

e.g. here.


Oh, yes. I did some coding style cleanup too.




Re: [RFC v3 PATCH 3/5] mm: refactor do_munmap() to extract the common part

2018-07-02 Thread Michal Hocko
On Sat 30-06-18 06:39:43, Yang Shi wrote:
> Introduces two new helper functions:
>   * munmap_addr_sanity()
>   * munmap_lookup_vma()
> 
> They will be used by do_munmap() and the new do_munmap with zapping
> large mapping early in the later patch.
> 
> There is no functional change, just code refactor.

There are whitespace changes which make the code much harder to review
than necessary.
> +static inline bool munmap_addr_sanity(unsigned long start, size_t len)
>  {
> - unsigned long end;
> - struct vm_area_struct *vma, *prev, *last;
> + if ((offset_in_page(start)) || start > TASK_SIZE || len > TASK_SIZE - 
> start)
> + return false;
>  
> - if ((offset_in_page(start)) || start > TASK_SIZE || len > 
> TASK_SIZE-start)
> - return -EINVAL;

e.g. here.
-- 
Michal Hocko
SUSE Labs