Re: [PATCH] mm/vmscan: change generic_file_write() comment to do_sync_write()

Re: [PATCH] mm/vmscan: change generic_file_write() comment to do_sync_write()

[Vincent Li]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1804 bytes --]

On Wed, 26 Aug 2009, Nikita Danilov wrote:

> Vincent Li <macli@brc.ubc.ca> writes:
> 
> > On Wed, 26 Aug 2009, Christoph Hellwig wrote:
> >
> >> On Tue, Aug 25, 2009 at 03:18:08PM -0700, Vincent Li wrote:
> >> > Commit 543ade1fc9 (Streamline generic_file_* interfaces and filemap cleanups)
> >> > removed generic_file_write() in filemap. For consistency, change the comment in
> >> > vmscan pageout() to do_sync_write().
> >>
> >> I think the right replacement would be __generic_file_aio_write.  But
> >
> > There is no __generic_file_aio_write, but __generic_file_aio_write_nolock,
> > generic_file_aio_write and generic_file_aio_write_nolock.
> >
> > I read the commit 543ade1fc9, it seems it replaced all .write = generic_file_write to
> > .write = do_sync_write. I thought they are the same.
> >
> >> from a quick glance over the code don't have the slightest idea what it
> >> is referring to.
> >
> > I read the code  over and over again, still no clue about the comment :-(.
> 
> This comment is about (bdi == current->backing_dev_info) condition in may_write_to_queue(),
> checking against
> 
>     /* We can write back this queue in page reclaim */
>     current->backing_dev_info = mapping->backing_dev_info;
> 
> bit (that used to be?) in __generic_file_aio_write_nolock()
> 
> Thank you,
> Nikita.

Thank you for the explaintion!

	* If this process is currently in generic_file_write() against
	* this page's queue, we can perform writeback even if that
	* will block.

So my interpretation for the comment is that if the current process is 
already in __generic_file_aio_write against the page's queue, The page 
claim path code can still perfom writeback even if the __generic_file_aio_write 
will block. Am I right?


Vincent Li
Biomedical Research Center
University of British Columbia

Re: [PATCH] mm/vmscan: change generic_file_write() comment to do_sync_write()

[Nikita Danilov]

2009/8/26 Vincent Li <macli@brc.ubc.ca>

[...]

> Thank you for the explaintion!
>
>        * If this process is currently in generic_file_write() against
>        * this page's queue, we can perform writeback even if that
>        * will block.
>
> So my interpretation for the comment is that if the current process is
> already in __generic_file_aio_write against the page's queue, The page
> claim path code can still perfom writeback even if the __generic_file_aio_write
> will block. Am I right?

Yes. The idea is that a heavy writer causing a lot of page allocations
should wait for the queue to drain, thus throttling itself; while a
thread that triggeres direct reclaim occasionally (i.e., not from the
main write path) shouldn't be blocked on the transput incurred by
somebody else.

Nikita.

>
>
>
> Vincent Li
> Biomedical Research Center
> University of British Columbia

--
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-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] mm/vmscan: change generic_file_write() comment to do_sync_write()

[Vincent Li]

Commit 543ade1fc9 (Streamline generic_file_* interfaces and filemap cleanups)
removed generic_file_write() in filemap. For consistency, change the comment in
vmscan pageout() to do_sync_write().

Signed-off-by: Vincent Li <macli@brc.ubc.ca>
---
 mm/vmscan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 1219ceb..5e03c22 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -358,7 +358,7 @@ static pageout_t pageout(struct page *page, struct address_space *mapping,
 	 * stalls if we need to run get_block().  We could test
 	 * PagePrivate for that.
 	 *
-	 * If this process is currently in generic_file_write() against
+	 * If this process is currently in do_sync_write() against
 	 * this page's queue, we can perform writeback even if that
 	 * will block.
 	 *
-- 
1.6.0.4

--
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-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm/vmscan: change generic_file_write() comment to do_sync_write()

[Christoph Hellwig]

On Tue, Aug 25, 2009 at 03:18:08PM -0700, Vincent Li wrote:
> Commit 543ade1fc9 (Streamline generic_file_* interfaces and filemap cleanups)
> removed generic_file_write() in filemap. For consistency, change the comment in
> vmscan pageout() to do_sync_write().

I think the right replacement would be __generic_file_aio_write.  But
from a quick glance over the code don't have the slightest idea what it
is referring to.

--
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-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm/vmscan: change generic_file_write() comment to do_sync_write()

[Vincent Li]

On Wed, 26 Aug 2009, Christoph Hellwig wrote:

> On Tue, Aug 25, 2009 at 03:18:08PM -0700, Vincent Li wrote:
> > Commit 543ade1fc9 (Streamline generic_file_* interfaces and filemap cleanups)
> > removed generic_file_write() in filemap. For consistency, change the comment in
> > vmscan pageout() to do_sync_write().
> 
> I think the right replacement would be __generic_file_aio_write.  But

There is no __generic_file_aio_write, but __generic_file_aio_write_nolock, 
generic_file_aio_write and generic_file_aio_write_nolock. 

I read the commit 543ade1fc9, it seems it replaced all .write = generic_file_write to 
.write = do_sync_write. I thought they are the same.

> from a quick glance over the code don't have the slightest idea what it
> is referring to.

I read the code  over and over again, still no clue about the comment :-(. 

Vincent Li
Biomedical Research Center
University of British Columbia

--
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-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm/vmscan: change generic_file_write() comment to do_sync_write()

[Christoph Hellwig]

On Tue, Aug 25, 2009 at 03:45:44PM -0700, Vincent Li wrote:
> There is no __generic_file_aio_write, but __generic_file_aio_write_nolock, 
> generic_file_aio_write and generic_file_aio_write_nolock. 

Indeed right now there is, but it gets renamed to
__generic_file_aio_write in a patchset queued up ;-)

> 
> I read the commit 543ade1fc9, it seems it replaced all .write = generic_file_write to 
> .write = do_sync_write. I thought they are the same.

That's true.  But do_sync_write is just a wrapper waiting for the
.aio_write method, for which the generic_file_write equivalent is
generic_file_aio_writev which calls into the above fuction.

--
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-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>