[PATCH] aio: remove 'always false' check

[PATCH] aio: remove 'always false' check

[Yuanhan Liu]

The only case that nr_pages will be < 0 is that nr_pages overflowed due
to nr_events is too big. But it is exclued at ioctx_alloc().

Thus, nr_pages will always > 0. So remove the dead 'if (nr_pages < 0)'
code block.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 fs/aio.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index 55c4c76..7248327 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -117,10 +117,6 @@ static int aio_setup_ring(struct kioctx *ctx)
 	size = sizeof(struct aio_ring);
 	size += sizeof(struct io_event) * nr_events;
 	nr_pages = (size + PAGE_SIZE-1) >> PAGE_SHIFT;
-
-	if (nr_pages < 0)
-		return -EINVAL;
-
 	nr_events = (PAGE_SIZE * nr_pages - sizeof(struct aio_ring)) / sizeof(struct io_event);
 
 	info->nr = 0;
-- 
1.7.7.6

Re: [PATCH] aio: remove 'always false' check

[Jeff Moyer]

Yuanhan Liu <yuanhan.liu@linux.intel.com> writes:

> The only case that nr_pages will be < 0 is that nr_pages overflowed due
> to nr_events is too big. But it is exclued at ioctx_alloc().

Right, the number of events is limited to what will fit in 256MB, for
some odd reason.  Maybe Ben can comment on that.

> Thus, nr_pages will always > 0. So remove the dead 'if (nr_pages < 0)'
> code block.

Sure, but I don't see any harm in keeping this check.  We do add another
page to the size, so retesting for overflow is good practice, and isn't
going to kill performance.

So, I'm going to NAK this.

Cheers,
Jeff

Re: [PATCH] aio: remove 'always false' check

[Yuanhan Liu]

Hi Jeff,

On Wed, Jul 11, 2012 at 10:17:26AM -0400, Jeff Moyer wrote:
> Yuanhan Liu <yuanhan.liu@linux.intel.com> writes:
> 
> > The only case that nr_pages will be < 0 is that nr_pages overflowed due
> > to nr_events is too big. But it is exclued at ioctx_alloc().
> 
> Right, the number of events is limited to what will fit in 256MB, for
> some odd reason.  Maybe Ben can comment on that.
> 
> > Thus, nr_pages will always > 0. So remove the dead 'if (nr_pages < 0)'
> > code block.
> 
> Sure, but I don't see any harm in keeping this check.  We do add another
> page to the size,

That would change nothing :)

> so retesting for overflow is good practice, and isn't
> going to kill performance.
> 
> So, I'm going to NAK this.

I'm fine with this NAK.  And, thanks for your comments.