Re: [PATCH] 9p: set page uptodate when required in write_end()
On Wed, Oct 11, 2017 at 12:27:43PM +, Levin, Alexander (Sasha Levin) wrote: > >> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c > >> index adaf6f6..e1cbdfd 100644 > >> --- a/fs/9p/vfs_addr.c > >> +++ b/fs/9p/vfs_addr.c > >> @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct > >> address_space *mapping, > >> > >>p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); > >> > >> - if (unlikely(copied < len && !PageUptodate(page))) { > >> - copied = 0; > >> - goto out; > >> + if (!PageUptodate(page)) { > >> + if (unlikely(copied < len)) { > >> + copied = 0; > >> + goto out; > >> + } else if (len == PAGE_SIZE) { > >> + SetPageUptodate(page); > >> + } Umm... I'm not sure I like it in that form. Look: to get here we need to successfully pass through ->write_begin(). Which means that we either have returned with PageUptodate(page) or had hit len == PAGE_SIZE && !PageUptodate(page). Case of non-uptodate with len < PAGE_SIZE is handled by forcing readpage and repeating the entire thing. So the only way to get !PageUptodate(page) in ->write_end() is to have had len == PAGE_SIZE. It needs a comment, at the very least.
Re: [PATCH] 9p: set page uptodate when required in write_end()
On Wed, Oct 11, 2017 at 12:27:43PM +, Levin, Alexander (Sasha Levin) wrote: > >> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c > >> index adaf6f6..e1cbdfd 100644 > >> --- a/fs/9p/vfs_addr.c > >> +++ b/fs/9p/vfs_addr.c > >> @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct > >> address_space *mapping, > >> > >>p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); > >> > >> - if (unlikely(copied < len && !PageUptodate(page))) { > >> - copied = 0; > >> - goto out; > >> + if (!PageUptodate(page)) { > >> + if (unlikely(copied < len)) { > >> + copied = 0; > >> + goto out; > >> + } else if (len == PAGE_SIZE) { > >> + SetPageUptodate(page); > >> + } Umm... I'm not sure I like it in that form. Look: to get here we need to successfully pass through ->write_begin(). Which means that we either have returned with PageUptodate(page) or had hit len == PAGE_SIZE && !PageUptodate(page). Case of non-uptodate with len < PAGE_SIZE is handled by forcing readpage and repeating the entire thing. So the only way to get !PageUptodate(page) in ->write_end() is to have had len == PAGE_SIZE. It needs a comment, at the very least.
Re: [PATCH] 9p: set page uptodate when required in write_end()
On Wed, Oct 11, 2017 at 5:27 AM, Levin, Alexander (Sasha Levin)wrote: > > Can one of you please pick this one directly? > > This has been on LKML since April, but the 9p maintainers seem to be MIA? Applied. Thanks, Linus
Re: [PATCH] 9p: set page uptodate when required in write_end()
On Wed, Oct 11, 2017 at 5:27 AM, Levin, Alexander (Sasha Levin) wrote: > > Can one of you please pick this one directly? > > This has been on LKML since April, but the 9p maintainers seem to be MIA? Applied. Thanks, Linus
Re: [PATCH] 9p: set page uptodate when required in write_end()
Linus, Al, Can one of you please pick this one directly? This has been on LKML since April, but the 9p maintainers seem to be MIA? On Thu, Jun 01, 2017 at 10:31:09PM -0400, Sasha Levin wrote: >Ping? > >On Mon, Apr 10, 2017 at 02:46:57PM -0400, Sasha Levin wrote: >> From: Alexander Levin>> >> Commit 77469c3f570 prevented setting the page as uptodate when we wrote the >> right amount of data, fix that. >> >> Fixes: 77469c3f570 ("9p: saner ->write_end() on failing copy into >> non-uptodate page") >> Reviewed-by: Jan Kara >> Signed-off-by: Alexander Levin >> --- >> fs/9p/vfs_addr.c | 10 +++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c >> index adaf6f6..e1cbdfd 100644 >> --- a/fs/9p/vfs_addr.c >> +++ b/fs/9p/vfs_addr.c >> @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct >> address_space *mapping, >> >> p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); >> >> -if (unlikely(copied < len && !PageUptodate(page))) { >> -copied = 0; >> -goto out; >> +if (!PageUptodate(page)) { >> +if (unlikely(copied < len)) { >> +copied = 0; >> +goto out; >> +} else if (len == PAGE_SIZE) { >> +SetPageUptodate(page); >> +} >> } >> /* >> * No need to use i_size_read() here, the i_size >> -- >> 2.7.4 > >-- > >Thanks, >Sasha -- Thanks, Sasha
Re: [PATCH] 9p: set page uptodate when required in write_end()
Linus, Al, Can one of you please pick this one directly? This has been on LKML since April, but the 9p maintainers seem to be MIA? On Thu, Jun 01, 2017 at 10:31:09PM -0400, Sasha Levin wrote: >Ping? > >On Mon, Apr 10, 2017 at 02:46:57PM -0400, Sasha Levin wrote: >> From: Alexander Levin >> >> Commit 77469c3f570 prevented setting the page as uptodate when we wrote the >> right amount of data, fix that. >> >> Fixes: 77469c3f570 ("9p: saner ->write_end() on failing copy into >> non-uptodate page") >> Reviewed-by: Jan Kara >> Signed-off-by: Alexander Levin >> --- >> fs/9p/vfs_addr.c | 10 +++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c >> index adaf6f6..e1cbdfd 100644 >> --- a/fs/9p/vfs_addr.c >> +++ b/fs/9p/vfs_addr.c >> @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct >> address_space *mapping, >> >> p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); >> >> -if (unlikely(copied < len && !PageUptodate(page))) { >> -copied = 0; >> -goto out; >> +if (!PageUptodate(page)) { >> +if (unlikely(copied < len)) { >> +copied = 0; >> +goto out; >> +} else if (len == PAGE_SIZE) { >> +SetPageUptodate(page); >> +} >> } >> /* >> * No need to use i_size_read() here, the i_size >> -- >> 2.7.4 > >-- > >Thanks, >Sasha -- Thanks, Sasha
Re: [PATCH] 9p: set page uptodate when required in write_end()
Ping? On Mon, Apr 10, 2017 at 02:46:57PM -0400, Sasha Levin wrote: > From: Alexander Levin> > Commit 77469c3f570 prevented setting the page as uptodate when we wrote the > right amount of data, fix that. > > Fixes: 77469c3f570 ("9p: saner ->write_end() on failing copy into > non-uptodate page") > Reviewed-by: Jan Kara > Signed-off-by: Alexander Levin > --- > fs/9p/vfs_addr.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c > index adaf6f6..e1cbdfd 100644 > --- a/fs/9p/vfs_addr.c > +++ b/fs/9p/vfs_addr.c > @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct > address_space *mapping, > > p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); > > - if (unlikely(copied < len && !PageUptodate(page))) { > - copied = 0; > - goto out; > + if (!PageUptodate(page)) { > + if (unlikely(copied < len)) { > + copied = 0; > + goto out; > + } else if (len == PAGE_SIZE) { > + SetPageUptodate(page); > + } > } > /* >* No need to use i_size_read() here, the i_size > -- > 2.7.4 -- Thanks, Sasha
Re: [PATCH] 9p: set page uptodate when required in write_end()
Ping? On Mon, Apr 10, 2017 at 02:46:57PM -0400, Sasha Levin wrote: > From: Alexander Levin > > Commit 77469c3f570 prevented setting the page as uptodate when we wrote the > right amount of data, fix that. > > Fixes: 77469c3f570 ("9p: saner ->write_end() on failing copy into > non-uptodate page") > Reviewed-by: Jan Kara > Signed-off-by: Alexander Levin > --- > fs/9p/vfs_addr.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c > index adaf6f6..e1cbdfd 100644 > --- a/fs/9p/vfs_addr.c > +++ b/fs/9p/vfs_addr.c > @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct > address_space *mapping, > > p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); > > - if (unlikely(copied < len && !PageUptodate(page))) { > - copied = 0; > - goto out; > + if (!PageUptodate(page)) { > + if (unlikely(copied < len)) { > + copied = 0; > + goto out; > + } else if (len == PAGE_SIZE) { > + SetPageUptodate(page); > + } > } > /* >* No need to use i_size_read() here, the i_size > -- > 2.7.4 -- Thanks, Sasha
[PATCH] 9p: set page uptodate when required in write_end()
From: Alexander LevinCommit 77469c3f570 prevented setting the page as uptodate when we wrote the right amount of data, fix that. Fixes: 77469c3f570 ("9p: saner ->write_end() on failing copy into non-uptodate page") Reviewed-by: Jan Kara Signed-off-by: Alexander Levin --- fs/9p/vfs_addr.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c index adaf6f6..e1cbdfd 100644 --- a/fs/9p/vfs_addr.c +++ b/fs/9p/vfs_addr.c @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct address_space *mapping, p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); - if (unlikely(copied < len && !PageUptodate(page))) { - copied = 0; - goto out; + if (!PageUptodate(page)) { + if (unlikely(copied < len)) { + copied = 0; + goto out; + } else if (len == PAGE_SIZE) { + SetPageUptodate(page); + } } /* * No need to use i_size_read() here, the i_size -- 2.7.4
[PATCH] 9p: set page uptodate when required in write_end()
From: Alexander Levin Commit 77469c3f570 prevented setting the page as uptodate when we wrote the right amount of data, fix that. Fixes: 77469c3f570 ("9p: saner ->write_end() on failing copy into non-uptodate page") Reviewed-by: Jan Kara Signed-off-by: Alexander Levin --- fs/9p/vfs_addr.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c index adaf6f6..e1cbdfd 100644 --- a/fs/9p/vfs_addr.c +++ b/fs/9p/vfs_addr.c @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct address_space *mapping, p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); - if (unlikely(copied < len && !PageUptodate(page))) { - copied = 0; - goto out; + if (!PageUptodate(page)) { + if (unlikely(copied < len)) { + copied = 0; + goto out; + } else if (len == PAGE_SIZE) { + SetPageUptodate(page); + } } /* * No need to use i_size_read() here, the i_size -- 2.7.4