Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1420296 > unrolled thread

[PATCH] aio: Cleanup unnecessary test for nr_pages

Started byMinfei Huang <mnghuan@gmail.com>
First post2016-06-12 18:40 +0200
Last post2016-06-22 08:10 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] aio: Cleanup unnecessary test for nr_pages Minfei Huang <mnghuan@gmail.com> - 2016-06-12 18:40 +0200
    Re: [PATCH] aio: Cleanup unnecessary test for nr_pages Minfei Huang <mnghuan@gmail.com> - 2016-06-20 16:10 +0200
      Re: [PATCH] aio: Cleanup unnecessary test for nr_pages Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-20 16:30 +0200
        Re: [PATCH] aio: Cleanup unnecessary test for nr_pages Minfei Huang <mnghuan@gmail.com> - 2016-06-22 08:10 +0200

#1420296 — [PATCH] aio: Cleanup unnecessary test for nr_pages

FromMinfei Huang <mnghuan@gmail.com>
Date2016-06-12 18:40 +0200
Subject[PATCH] aio: Cleanup unnecessary test for nr_pages
Message-ID<rJgAx-2ym-7@gated-at.bofh.it>
The variable nr_pages is always more than 1, because the size of
structure aio_ring is bigger than 0. So remove unnecessary test for
nr_page.

Signed-off-by: Minfei Huang <mnghuan@gmail.com>
---
 fs/aio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index fb8e45b..ec05137 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -450,8 +450,6 @@ static int aio_setup_ring(struct kioctx *ctx)
 	size += sizeof(struct io_event) * nr_events;
 
 	nr_pages = PFN_UP(size);
-	if (nr_pages < 0)
-		return -EINVAL;
 
 	file = aio_private_file(ctx, nr_pages);
 	if (IS_ERR(file)) {
-- 
2.7.4 (Apple Git-66)

[toc] | [next] | [standalone]


#1426636

FromMinfei Huang <mnghuan@gmail.com>
Date2016-06-20 16:10 +0200
Message-ID<rM83L-OW-5@gated-at.bofh.it>
In reply to#1420296
Ping. Any comment is appreciate.

Thanks
Minfei

On 06/13/16 at 12:33P, Minfei Huang wrote:
> The variable nr_pages is always more than 1, because the size of
> structure aio_ring is bigger than 0. So remove unnecessary test for
> nr_page.
> 
> Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> ---
>  fs/aio.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index fb8e45b..ec05137 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -450,8 +450,6 @@ static int aio_setup_ring(struct kioctx *ctx)
>  	size += sizeof(struct io_event) * nr_events;
>  
>  	nr_pages = PFN_UP(size);
> -	if (nr_pages < 0)
> -		return -EINVAL;
>  
>  	file = aio_private_file(ctx, nr_pages);
>  	if (IS_ERR(file)) {
> -- 
> 2.7.4 (Apple Git-66)
> 

[toc] | [prev] | [next] | [standalone]


#1426650

FromAl Viro <viro@ZenIV.linux.org.uk>
Date2016-06-20 16:30 +0200
Message-ID<rM8n8-Wx-13@gated-at.bofh.it>
In reply to#1426636
On Mon, Jun 20, 2016 at 10:05:45PM +0800, Minfei Huang wrote:
> Ping. Any comment is appreciate.
> 
> Thanks
> Minfei
> 
> On 06/13/16 at 12:33P, Minfei Huang wrote:
> > The variable nr_pages is always more than 1, because the size of
> > structure aio_ring is bigger than 0. So remove unnecessary test for
> > nr_page.

What this test really checks is that the value we'd put into nr_pages
(PFN_UP(size)) is not greater than 2^31.  Whether it's redundant or not
is a separate question - it very well might be, due to the code in
ioctx_alloc() that caps nr_events, but that needs a proof.  In any case,
the reasons you are offering in commit message are wrong - it's about
size being too _large_, not too small.

[toc] | [prev] | [next] | [standalone]


#1428442

FromMinfei Huang <mnghuan@gmail.com>
Date2016-06-22 08:10 +0200
Message-ID<rMJwm-88Y-17@gated-at.bofh.it>
In reply to#1426650
On 06/20/16 at 03:17P, Al Viro wrote:
> On Mon, Jun 20, 2016 at 10:05:45PM +0800, Minfei Huang wrote:
> > Ping. Any comment is appreciate.
> > 
> > Thanks
> > Minfei
> > 
> > On 06/13/16 at 12:33P, Minfei Huang wrote:
> > > The variable nr_pages is always more than 1, because the size of
> > > structure aio_ring is bigger than 0. So remove unnecessary test for
> > > nr_page.
> 
> What this test really checks is that the value we'd put into nr_pages
> (PFN_UP(size)) is not greater than 2^31.  Whether it's redundant or not
> is a separate question - it very well might be, due to the code in
> ioctx_alloc() that caps nr_events, but that needs a proof.  In any case,
> the reasons you are offering in commit message are wrong - it's about
> size being too _large_, not too small.

Hmm. But there is a following test in function ioctx_alloc which is used
to limit the max of nr_events.

 713         /* Prevent overflows */
 714         if (nr_events > (0x10000000U / sizeof(struct io_event))) {
 715                 pr_debug("ENOMEM: nr_events too high\n");
 716                 return ERR_PTR(-EINVAL);
 717         }

So according to this test, we can make sure that nr_pages(PFN_UP(size))
cann't be greater than 2^31, and max to 2^28.

Thanks
Minfei

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web