On Wed 30-11-11 08:37:16, Wu Fengguang wrote:
> (snip)
> > > @@ -676,6 +677,20 @@ ondemand_readahead(struct address_space
> > > }
> > >
> > > /*
> > > + * backwards reading
> > > + */
> > > + if (offset < ra->start && offset + req_size >= ra->start) {
> > > + ra->pattern = RA_PATTERN_BACKWARDS;
> > > + ra->size = get_next_ra_size(ra, max);
> > > + max = ra->start;
> > > + if (ra->size > max)
> > > + ra->size = max;
> > > + ra->async_size = 0;
> > > + ra->start -= ra->size;
> > IMHO much more obvious way to write this is:
> > ra->size = get_next_ra_size(ra, max);
> > if (ra->size > ra->start) {
> > ra->size = ra->start;
> > ra->start = 0;
> > } else
> > ra->start -= ra->size;
>
> Good idea! Here is the updated code:
>
> /*
> * backwards reading
> */
> if (offset < ra->start && offset + req_size >= ra->start) {
> ra->pattern = RA_PATTERN_BACKWARDS;
> ra->size = get_next_ra_size(ra, max);
> if (ra->size > ra->start) {
> /*
> * ra->start may be concurrently set to some huge
> * value, the min() at least avoids submitting huge IO
> * in this race condition
> */
> ra->size = min(ra->start, max);
> ra->start = 0;
> } else
> ra->start -= ra->size;
> ra->async_size = 0;
> goto readit;
> }
Looks good. You can add:
Acked-by: Jan Kara <jack@xxxxxxx>
to the patch.
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>