generic_file_splice_read() issues

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Miklos Szeredi
Date: Wednesday, June 25, 2008 - 6:54 am

| 	error = 0;
| 	while (spd.nr_pages < nr_pages) {
| 		/*
| 		 * Page could be there, find_get_pages_contig() breaks on
| 		 * the first hole.
| 		 */
| 		page = find_get_page(mapping, index);
| 		if (!page) {
| 			/*
| 			 * page didn't exist, allocate one.
| 			 */
| 			page = page_cache_alloc_cold(mapping);
| 			if (!page)

error = -ENOMEM?

| 				break;
| 
| 			error = add_to_page_cache_lru(page, mapping, index,
| 						mapping_gfp_mask(mapping));
| 			if (unlikely(error)) {
| 				page_cache_release(page);
| 				if (error == -EEXIST)

error = 0?  It may not matter, but leaving error as EEXIST is
confusing at best (and coupled with the above missing ENOMEM could
result in really weird errors for splice() ;).

| 					continue;
| 				break;
| 			}
| 			/*
| 			 * add_to_page_cache() locks the page, unlock it
| 			 * to avoid convoluting the logic below even more.
| 			 */
| 			unlock_page(page);
| 		}
| 
| 		pages[spd.nr_pages++] = page;
| 		index++;
| 	}

...

| 
| 			/*
| 			 * need to read in the page
| 			 */
| 			error = mapping->a_ops->readpage(in, page);
| 			if (unlikely(error)) {
| 				/*
| 				 * We really should re-lookup the page here,
| 				 * but it complicates things a lot. Instead
| 				 * lets just do what we already stored, and
| 				 * we'll get it the next time we are called.
| 				 */
| 				if (error == AOP_TRUNCATED_PAGE)
| 					error = 0;

This may also cause similar issues as the invalidatation race.  I'd
think it would be better not to be sloppy here.

| 
| 				break;
| 			}
| 		}
| fill_it:

Miklos
--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
generic_file_splice_read() issues, Miklos Szeredi, (Wed Jun 25, 6:54 am)
Re: generic_file_splice_read() issues, Jens Axboe, (Wed Jun 25, 7:10 am)
Re: generic_file_splice_read() issues, Miklos Szeredi, (Wed Jun 25, 7:26 am)