* [PATCH 0/2] Cleanup invalidate page
@ 2013-08-09 16:59 Milosz Tanski
2013-08-09 16:59 ` [PATCH 1/2] ceph: Remove bogus check in invalidatepage Milosz Tanski
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Milosz Tanski @ 2013-08-09 16:59 UTC (permalink / raw)
To: ceph-devel; +Cc: linux-fsdevel, sage, zheng.z.yan
Currently ceph_invalidatepage has is overly eger with it's checks which are
moot. The second change cleans up the case where offset is non zero.
Please pull the from:
https://bitbucket.org/adfin/linux-fs.git wip-invalidatepage
This simple patchset came from the changes I made while working on fscache
support for cephfs. Per Sage's request I split this up into smaller bites for
testing and review.
Milosz Tanski (2):
ceph: Remove bogus check in invalidatepage
ceph: cleanup the logic in ceph_invalidatepage
fs/ceph/addr.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
--
1.8.1.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] ceph: Remove bogus check in invalidatepage 2013-08-09 16:59 [PATCH 0/2] Cleanup invalidate page Milosz Tanski @ 2013-08-09 16:59 ` Milosz Tanski 2013-08-09 16:59 ` [PATCH 2/2] ceph: cleanup the logic in ceph_invalidatepage Milosz Tanski 2013-08-09 17:44 ` [PATCH 0/2] Cleanup invalidate page Sage Weil 2 siblings, 0 replies; 7+ messages in thread From: Milosz Tanski @ 2013-08-09 16:59 UTC (permalink / raw) To: ceph-devel; +Cc: linux-fsdevel, sage, zheng.z.yan The early bug checks are moot because the VMA layer ensures those things. 1. It will not call invalidatepage unless PagePrivate (or PagePrivate2) are set 2. It will not call invalidatepage without taking a PageLock first. 3. Guantrees that the inode page is mapped. Signed-off-by: Milosz Tanski <milosz@adfin.com> --- fs/ceph/addr.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index afb2fc2..f1d6c60 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -149,10 +149,6 @@ static void ceph_invalidatepage(struct page *page, unsigned long offset) struct ceph_inode_info *ci; struct ceph_snap_context *snapc = page_snap_context(page); - BUG_ON(!PageLocked(page)); - BUG_ON(!PagePrivate(page)); - BUG_ON(!page->mapping); - inode = page->mapping->host; /* -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] ceph: cleanup the logic in ceph_invalidatepage 2013-08-09 16:59 [PATCH 0/2] Cleanup invalidate page Milosz Tanski 2013-08-09 16:59 ` [PATCH 1/2] ceph: Remove bogus check in invalidatepage Milosz Tanski @ 2013-08-09 16:59 ` Milosz Tanski 2013-08-09 17:44 ` [PATCH 0/2] Cleanup invalidate page Sage Weil 2 siblings, 0 replies; 7+ messages in thread From: Milosz Tanski @ 2013-08-09 16:59 UTC (permalink / raw) To: ceph-devel; +Cc: linux-fsdevel, sage, zheng.z.yan The invalidatepage code bails if it encounters a non-zero page offset. The current logic that does is non-obvious with multiple if statements. This should be logically and functionally equivalent. Signed-off-by: Milosz Tanski <milosz@adfin.com> --- fs/ceph/addr.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index f1d6c60..341f998 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -150,6 +150,13 @@ static void ceph_invalidatepage(struct page *page, unsigned long offset) struct ceph_snap_context *snapc = page_snap_context(page); inode = page->mapping->host; + ci = ceph_inode(inode); + + if (offset != 0) { + dout("%p invalidatepage %p idx %lu partial dirty page\n", + inode, page, page->index); + return; + } /* * We can get non-dirty pages here due to races between @@ -159,21 +166,15 @@ static void ceph_invalidatepage(struct page *page, unsigned long offset) if (!PageDirty(page)) pr_err("%p invalidatepage %p page not dirty\n", inode, page); - if (offset == 0) - ClearPageChecked(page); + ClearPageChecked(page); - ci = ceph_inode(inode); - if (offset == 0) { - dout("%p invalidatepage %p idx %lu full dirty page %lu\n", - inode, page, page->index, offset); - ceph_put_wrbuffer_cap_refs(ci, 1, snapc); - ceph_put_snap_context(snapc); - page->private = 0; - ClearPagePrivate(page); - } else { - dout("%p invalidatepage %p idx %lu partial dirty page\n", - inode, page, page->index); - } + dout("%p invalidatepage %p idx %lu full dirty page %lu\n", + inode, page, page->index, offset); + + ceph_put_wrbuffer_cap_refs(ci, 1, snapc); + ceph_put_snap_context(snapc); + page->private = 0; + ClearPagePrivate(page); } /* just a sanity check */ -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Cleanup invalidate page 2013-08-09 16:59 [PATCH 0/2] Cleanup invalidate page Milosz Tanski 2013-08-09 16:59 ` [PATCH 1/2] ceph: Remove bogus check in invalidatepage Milosz Tanski 2013-08-09 16:59 ` [PATCH 2/2] ceph: cleanup the logic in ceph_invalidatepage Milosz Tanski @ 2013-08-09 17:44 ` Sage Weil 2013-08-09 19:43 ` Milosz Tanski 2 siblings, 1 reply; 7+ messages in thread From: Sage Weil @ 2013-08-09 17:44 UTC (permalink / raw) To: Milosz Tanski; +Cc: ceph-devel, linux-fsdevel, zheng.z.yan Hi Milosz, I pulled both these into the testing branch. Thanks! sage On Fri, 9 Aug 2013, Milosz Tanski wrote: > Currently ceph_invalidatepage has is overly eger with it's checks which are > moot. The second change cleans up the case where offset is non zero. > > Please pull the from: > https://bitbucket.org/adfin/linux-fs.git wip-invalidatepage > > This simple patchset came from the changes I made while working on fscache > support for cephfs. Per Sage's request I split this up into smaller bites for > testing and review. > > Milosz Tanski (2): > ceph: Remove bogus check in invalidatepage > ceph: cleanup the logic in ceph_invalidatepage > > fs/ceph/addr.c | 33 +++++++++++++++------------------ > 1 file changed, 15 insertions(+), 18 deletions(-) > > -- > 1.8.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Cleanup invalidate page 2013-08-09 17:44 ` [PATCH 0/2] Cleanup invalidate page Sage Weil @ 2013-08-09 19:43 ` Milosz Tanski 2013-08-09 19:44 ` Sage Weil 0 siblings, 1 reply; 7+ messages in thread From: Milosz Tanski @ 2013-08-09 19:43 UTC (permalink / raw) To: Sage Weil; +Cc: ceph-devel, linux-fsdevel, Yan, Zheng Sage, Great. Is there some automated testing system that looks for regressions in cephfs that I can be watching for? - Milosz On Fri, Aug 9, 2013 at 1:44 PM, Sage Weil <sage@inktank.com> wrote: > Hi Milosz, > > I pulled both these into the testing branch. Thanks! > > sage > > On Fri, 9 Aug 2013, Milosz Tanski wrote: > >> Currently ceph_invalidatepage has is overly eger with it's checks which are >> moot. The second change cleans up the case where offset is non zero. >> >> Please pull the from: >> https://bitbucket.org/adfin/linux-fs.git wip-invalidatepage >> >> This simple patchset came from the changes I made while working on fscache >> support for cephfs. Per Sage's request I split this up into smaller bites for >> testing and review. >> >> Milosz Tanski (2): >> ceph: Remove bogus check in invalidatepage >> ceph: cleanup the logic in ceph_invalidatepage >> >> fs/ceph/addr.c | 33 +++++++++++++++------------------ >> 1 file changed, 15 insertions(+), 18 deletions(-) >> >> -- >> 1.8.1.2 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Cleanup invalidate page 2013-08-09 19:43 ` Milosz Tanski @ 2013-08-09 19:44 ` Sage Weil 2013-08-09 19:51 ` Milosz Tanski 0 siblings, 1 reply; 7+ messages in thread From: Sage Weil @ 2013-08-09 19:44 UTC (permalink / raw) To: Milosz Tanski; +Cc: ceph-devel, linux-fsdevel, Yan, Zheng On Fri, 9 Aug 2013, Milosz Tanski wrote: > Sage, > > Great. Is there some automated testing system that looks for > regressions in cephfs that I can be watching for? Yep, you can join the ceph-qa@ceph.com email list and watch for the kcephfs suite results (see http://ceph.com/resources/mailing-list-irc/). BTW I made a few tweaks to the second patch due to a conflict (added handling for the length arg to invalidatepage). Thanks! sage > > - Milosz > > On Fri, Aug 9, 2013 at 1:44 PM, Sage Weil <sage@inktank.com> wrote: > > Hi Milosz, > > > > I pulled both these into the testing branch. Thanks! > > > > sage > > > > On Fri, 9 Aug 2013, Milosz Tanski wrote: > > > >> Currently ceph_invalidatepage has is overly eger with it's checks which are > >> moot. The second change cleans up the case where offset is non zero. > >> > >> Please pull the from: > >> https://bitbucket.org/adfin/linux-fs.git wip-invalidatepage > >> > >> This simple patchset came from the changes I made while working on fscache > >> support for cephfs. Per Sage's request I split this up into smaller bites for > >> testing and review. > >> > >> Milosz Tanski (2): > >> ceph: Remove bogus check in invalidatepage > >> ceph: cleanup the logic in ceph_invalidatepage > >> > >> fs/ceph/addr.c | 33 +++++++++++++++------------------ > >> 1 file changed, 15 insertions(+), 18 deletions(-) > >> > >> -- > >> 1.8.1.2 > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> > >> > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Cleanup invalidate page 2013-08-09 19:44 ` Sage Weil @ 2013-08-09 19:51 ` Milosz Tanski 0 siblings, 0 replies; 7+ messages in thread From: Milosz Tanski @ 2013-08-09 19:51 UTC (permalink / raw) To: Sage Weil; +Cc: ceph-devel, linux-fsdevel, Yan, Zheng On Fri, Aug 9, 2013 at 3:44 PM, Sage Weil <sage@inktank.com> wrote: > On Fri, 9 Aug 2013, Milosz Tanski wrote: >> Sage, >> >> Great. Is there some automated testing system that looks for >> regressions in cephfs that I can be watching for? > > Yep, you can join the ceph-qa@ceph.com email list and watch for the > kcephfs suite results (see http://ceph.com/resources/mailing-list-irc/). I'll brace my self for the spam. > > BTW I made a few tweaks to the second patch due to a conflict (added > handling for the length arg to invalidatepage). Thanks and sorry for the oversight. Best, -Milosz > > Thanks! > sage > >> >> - Milosz >> >> On Fri, Aug 9, 2013 at 1:44 PM, Sage Weil <sage@inktank.com> wrote: >> > Hi Milosz, >> > >> > I pulled both these into the testing branch. Thanks! >> > >> > sage >> > >> > On Fri, 9 Aug 2013, Milosz Tanski wrote: >> > >> >> Currently ceph_invalidatepage has is overly eger with it's checks which are >> >> moot. The second change cleans up the case where offset is non zero. >> >> >> >> Please pull the from: >> >> https://bitbucket.org/adfin/linux-fs.git wip-invalidatepage >> >> >> >> This simple patchset came from the changes I made while working on fscache >> >> support for cephfs. Per Sage's request I split this up into smaller bites for >> >> testing and review. >> >> >> >> Milosz Tanski (2): >> >> ceph: Remove bogus check in invalidatepage >> >> ceph: cleanup the logic in ceph_invalidatepage >> >> >> >> fs/ceph/addr.c | 33 +++++++++++++++------------------ >> >> 1 file changed, 15 insertions(+), 18 deletions(-) >> >> >> >> -- >> >> 1.8.1.2 >> >> >> >> -- >> >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in >> >> the body of a message to majordomo@vger.kernel.org >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-08-09 19:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-08-09 16:59 [PATCH 0/2] Cleanup invalidate page Milosz Tanski 2013-08-09 16:59 ` [PATCH 1/2] ceph: Remove bogus check in invalidatepage Milosz Tanski 2013-08-09 16:59 ` [PATCH 2/2] ceph: cleanup the logic in ceph_invalidatepage Milosz Tanski 2013-08-09 17:44 ` [PATCH 0/2] Cleanup invalidate page Sage Weil 2013-08-09 19:43 ` Milosz Tanski 2013-08-09 19:44 ` Sage Weil 2013-08-09 19:51 ` Milosz Tanski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.