thinko in mm/filemap.c (242p1)

Hi.

The following patch fixes what I think is a cut'n'paste slipup in mm/filemap.c::generic_buffer_fdatasync. It applies against 242p1 and 241-ac3. Comments?

@@ -397,7 +397,7 @@
        retval |= do_buffer_fdatasync(&inode->i_mapping->locked_pages, start_idx

, end_idx, writeout_one_page);

        /* now wait for locked buffers on pages from both clean and dirty lists 

*/
- retval |= do_buffer_fdatasync(&inode->i_mapping->dirty_pages, start_idx,
end_idx, writeout_one_page);
+ retval |= do_buffer_fdatasync(&inode->i_mapping->dirty_pages, start_idx,

 end_idx, waitfor_one_page);
        retval |= do_buffer_fdatasync(&inode->i_mapping->clean_pages, start_idx,
 end_idx, waitfor_one_page);
        retval |= do_buffer_fdatasync(&inode->i_mapping->locked_pages, start_idx

, end_idx, waitfor_one_page);

Regards,
Rasmus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

I guess the writeout_one_page schedules the dirty pages for IO and puts them on the list of locked pages. The last call then waits on those same pages until they've been flushed to disk.

Your change would wait on the pages but never submit them for IO (again, a guess, I haven't looked at the code in too much detail).

regards,

Rik
--
Linux MM bugzilla: http://linux-mm.org/bugzilla.shtml

Virtual memory is like a game you can't win; However, without VM there's truly nothing to lose...

		http://www.surriel.com/

http://www.conectiva.com/ http://distro.conectiva.com/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

[...]

The total function (generic_buffer_fdatasync) (with my patch):

        /* writeout dirty buffers on pages from both clean and dirty lists */
        retval = do_buffer_fdatasync(&inode->i_mapping->dirty_pages, start_idx, 
end_idx, writeout_one_page);
        retval |= do_buffer_fdatasync(&inode->i_mapping->clean_pages, start_idx,
 end_idx, writeout_one_page);
        retval |= do_buffer_fdatasync(&inode->i_mapping->locked_pages, start_idx

, end_idx, writeout_one_page);

        /* now wait for locked buffers on pages from both clean and dirty lists 
*/
        retval |= do_buffer_fdatasync(&inode->i_mapping->dirty_pages, start_idx,
 end_idx, waitfor_one_page);
        retval |= do_buffer_fdatasync(&inode->i_mapping->clean_pages, start_idx,
 end_idx, waitfor_one_page);
        retval |= do_buffer_fdatasync(&inode->i_mapping->locked_pages, start_idx

, end_idx, waitfor_one_page);

So we start the writeout in the three first lines and wait for them in the last three. Without my patch we write dirty_pages out again in the second run.

Regards,
Rasmus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Pages written in the first run will be moved to the locked list.

No big deal, but its not a critical bug right now.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

OK, I guess it makes sense then ... ;)

Rik
--
Linux MM bugzilla: http://linux-mm.org/bugzilla.shtml

Virtual memory is like a game you can't win; However, without VM there's truly nothing to lose...

		http://www.surriel.com/

http://www.conectiva.com/ http://distro.conectiva.com/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/