[PATCH] thinko in mm/filemap.c (242p1)

[PATCH] thinko in mm/filemap.c (242p1)

[Rasmus Andersen]

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?

--- linux/mm/filemap.c.org      Tue Feb  6 13:00:03 2001
+++ linux/mm/filemap.c  Tue Feb  6 13:00:53 2001
@@ -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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] thinko in mm/filemap.c (242p1)

[Marcelo Tosatti]

On Tue, 6 Feb 2001, Rasmus Andersen wrote:

> On Tue, Feb 06, 2001 at 10:51:05AM -0200, Rik van Riel wrote:
> > On Tue, 6 Feb 2001, Rasmus Andersen wrote:
> [...]
> > 
> > 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).
> 
> 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.

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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] thinko in mm/filemap.c (242p1)

[Rik van Riel]

On Tue, 6 Feb 2001, Rasmus Andersen wrote:

> 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?

> -       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,

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

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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] thinko in mm/filemap.c (242p1)

[Rasmus Andersen]

On Tue, Feb 06, 2001 at 10:51:05AM -0200, Rik van Riel wrote:
> On Tue, 6 Feb 2001, Rasmus Andersen wrote:
[...]
> 
> 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).

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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] thinko in mm/filemap.c (242p1)

[Rik van Riel]

On Tue, 6 Feb 2001, Rasmus Andersen wrote:

> 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.

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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/