Skip to contentSkip to conversation

aio: remove 'always false' check

PATCH In review Yuanhan Liu 2 participants 3 messages Jul 11, 2012 – Jul 13, 2012

Subsystems (2)
VFS Aio
lore
Share & tools
lore

Yuanhan Liu

author
#0 lore raw

PATCH 1/1 aio: remove 'always false' check

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 <[email protected]>
---
 fs/aio.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
fs/aio.c +0 -4
index 55c4c76..7248327 100644
@@ -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;
Signature · 1 line
1.7.7.6
#1 lore raw
Quoted 2 lines · 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().

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

Quoted 2 lines

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

author
#2 lore raw

Hi Jeff,

Quoted 5 lines · Jeff Moyer
Quoted 2 lines · 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().

Quoted 8 lines

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

Quoted 2 lines

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

Quoted 4 lines

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.