* [PATCH] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster @ 2018-01-26 15:05 Nikolay Borisov 2018-01-30 14:32 ` [RESEND PATCH] " Nikolay Borisov 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Borisov @ 2018-01-26 15:05 UTC (permalink / raw) To: linux-btrfs; +Cc: Nikolay Borisov Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Josef Bacik <jbacik@fb.com> --- Based on off-list feadback from Josef I'm also clearing the EXTENT_BOUNDARY flag so we don't leak it. Otherwise it's okay. He said to add his Reviewed-by as long as the extent clearing bit is adjusted. fs/btrfs/relocation.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index f0c3f00e97cb..8b2a31cef5cf 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -3268,12 +3268,25 @@ static int relocate_file_extent_cluster(struct inode *inode, nr++; } - btrfs_set_extent_delalloc(inode, page_start, page_end, 0, NULL, - 0); + ret = btrfs_set_extent_delalloc(inode, page_start, page_end, 0, + NULL, 0); + if (ret) { + unlock_page(page); + put_page(page); + btrfs_delalloc_release_metadata(BTRFS_I(inode), + PAGE_SIZE); + btrfs_delalloc_release_extents(BTRFS_I(inode), + PAGE_SIZE); + + clear_extent_bits(&BTRFS_I(inode)->io_tree, + page_start, page_end, + EXTENT_LOCKED | EXTENT_BOUNDARY); + goto out; + + } set_page_dirty(page); - unlock_extent(&BTRFS_I(inode)->io_tree, - page_start, page_end); + unlock_extent(&BTRFS_I(inode)->io_tree, page_start, page_end); unlock_page(page); put_page(page); -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RESEND PATCH] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster 2018-01-26 15:05 [PATCH] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster Nikolay Borisov @ 2018-01-30 14:32 ` Nikolay Borisov 2018-01-31 10:49 ` Filipe Manana 2018-01-31 15:14 ` [PATCH v2] " Nikolay Borisov 0 siblings, 2 replies; 8+ messages in thread From: Nikolay Borisov @ 2018-01-30 14:32 UTC (permalink / raw) To: dsterba; +Cc: linux-btrfs, Nikolay Borisov Essentially duplicate the error handling from the above block which handles the !PageUptodate(page) case and additionally clear EXTENT_BOUNDARY. Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Josef Bacik <jbacik@fb.com> --- Put description of the intended changes. fs/btrfs/relocation.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index f0c3f00e97cb..8b2a31cef5cf 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -3268,12 +3268,25 @@ static int relocate_file_extent_cluster(struct inode *inode, nr++; } - btrfs_set_extent_delalloc(inode, page_start, page_end, 0, NULL, - 0); + ret = btrfs_set_extent_delalloc(inode, page_start, page_end, 0, + NULL, 0); + if (ret) { + unlock_page(page); + put_page(page); + btrfs_delalloc_release_metadata(BTRFS_I(inode), + PAGE_SIZE); + btrfs_delalloc_release_extents(BTRFS_I(inode), + PAGE_SIZE); + + clear_extent_bits(&BTRFS_I(inode)->io_tree, + page_start, page_end, + EXTENT_LOCKED | EXTENT_BOUNDARY); + goto out; + + } set_page_dirty(page); - unlock_extent(&BTRFS_I(inode)->io_tree, - page_start, page_end); + unlock_extent(&BTRFS_I(inode)->io_tree, page_start, page_end); unlock_page(page); put_page(page); -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster 2018-01-30 14:32 ` [RESEND PATCH] " Nikolay Borisov @ 2018-01-31 10:49 ` Filipe Manana 2018-01-31 10:53 ` Nikolay Borisov 2018-01-31 15:14 ` [PATCH v2] " Nikolay Borisov 1 sibling, 1 reply; 8+ messages in thread From: Filipe Manana @ 2018-01-31 10:49 UTC (permalink / raw) To: Nikolay Borisov; +Cc: dsterba, linux-btrfs On Tue, Jan 30, 2018 at 2:32 PM, Nikolay Borisov <nborisov@suse.com> wrote: > Essentially duplicate the error handling from the above block which > handles the !PageUptodate(page) case and additionally clear > EXTENT_BOUNDARY. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > Reviewed-by: Josef Bacik <jbacik@fb.com> > --- > > Put description of the intended changes. > > fs/btrfs/relocation.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index f0c3f00e97cb..8b2a31cef5cf 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -3268,12 +3268,25 @@ static int relocate_file_extent_cluster(struct inode *inode, > nr++; > } > > - btrfs_set_extent_delalloc(inode, page_start, page_end, 0, NULL, > - 0); > + ret = btrfs_set_extent_delalloc(inode, page_start, page_end, 0, > + NULL, 0); > + if (ret) { > + unlock_page(page); > + put_page(page); > + btrfs_delalloc_release_metadata(BTRFS_I(inode), > + PAGE_SIZE); > + btrfs_delalloc_release_extents(BTRFS_I(inode), > + PAGE_SIZE); > + > + clear_extent_bits(&BTRFS_I(inode)->io_tree, > + page_start, page_end, > + EXTENT_LOCKED | EXTENT_BOUNDARY); > + goto out; > + > + } > set_page_dirty(page); > > - unlock_extent(&BTRFS_I(inode)->io_tree, > - page_start, page_end); > + unlock_extent(&BTRFS_I(inode)->io_tree, page_start, page_end); This is an unrelated change. Please don't mix pure white space/indentation changes with functional changes. > unlock_page(page); > put_page(page); > > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.” ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster 2018-01-31 10:49 ` Filipe Manana @ 2018-01-31 10:53 ` Nikolay Borisov 2018-01-31 10:57 ` Filipe Manana 2018-01-31 14:50 ` David Sterba 0 siblings, 2 replies; 8+ messages in thread From: Nikolay Borisov @ 2018-01-31 10:53 UTC (permalink / raw) To: fdmanana; +Cc: dsterba, linux-btrfs On 31.01.2018 12:49, Filipe Manana wrote: > On Tue, Jan 30, 2018 at 2:32 PM, Nikolay Borisov <nborisov@suse.com> wrote: >> Essentially duplicate the error handling from the above block which >> handles the !PageUptodate(page) case and additionally clear >> EXTENT_BOUNDARY. >> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >> Reviewed-by: Josef Bacik <jbacik@fb.com> >> --- >> >> Put description of the intended changes. >> >> fs/btrfs/relocation.c | 21 +++++++++++++++++---- >> 1 file changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c >> index f0c3f00e97cb..8b2a31cef5cf 100644 >> --- a/fs/btrfs/relocation.c >> +++ b/fs/btrfs/relocation.c >> @@ -3268,12 +3268,25 @@ static int relocate_file_extent_cluster(struct inode *inode, >> nr++; >> } >> >> - btrfs_set_extent_delalloc(inode, page_start, page_end, 0, NULL, >> - 0); >> + ret = btrfs_set_extent_delalloc(inode, page_start, page_end, 0, >> + NULL, 0); >> + if (ret) { >> + unlock_page(page); >> + put_page(page); >> + btrfs_delalloc_release_metadata(BTRFS_I(inode), >> + PAGE_SIZE); >> + btrfs_delalloc_release_extents(BTRFS_I(inode), >> + PAGE_SIZE); >> + >> + clear_extent_bits(&BTRFS_I(inode)->io_tree, >> + page_start, page_end, >> + EXTENT_LOCKED | EXTENT_BOUNDARY); >> + goto out; >> + >> + } >> set_page_dirty(page); >> >> - unlock_extent(&BTRFS_I(inode)->io_tree, >> - page_start, page_end); >> + unlock_extent(&BTRFS_I(inode)->io_tree, page_start, page_end); > > This is an unrelated change. Please don't mix pure white > space/indentation changes with functional changes. David seems rather adamant in not accepting pure whitespace/indention changes on their own so I don't see a way to actually improve the code base in that regard unless i slip them up when modifying nearby code. There are a couple of space with trailing whitespace which I constantly select out from my commits. Given that you have now also expressed objection to such cleanups, how should they eventually be fixed? > >> unlock_page(page); >> put_page(page); >> >> -- >> 2.7.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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] 8+ messages in thread
* Re: [RESEND PATCH] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster 2018-01-31 10:53 ` Nikolay Borisov @ 2018-01-31 10:57 ` Filipe Manana 2018-01-31 14:50 ` David Sterba 1 sibling, 0 replies; 8+ messages in thread From: Filipe Manana @ 2018-01-31 10:57 UTC (permalink / raw) To: Nikolay Borisov; +Cc: dsterba, linux-btrfs On Wed, Jan 31, 2018 at 10:53 AM, Nikolay Borisov <nborisov@suse.com> wrote: > > > On 31.01.2018 12:49, Filipe Manana wrote: >> On Tue, Jan 30, 2018 at 2:32 PM, Nikolay Borisov <nborisov@suse.com> wrote: >>> Essentially duplicate the error handling from the above block which >>> handles the !PageUptodate(page) case and additionally clear >>> EXTENT_BOUNDARY. >>> >>> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >>> Reviewed-by: Josef Bacik <jbacik@fb.com> >>> --- >>> >>> Put description of the intended changes. >>> >>> fs/btrfs/relocation.c | 21 +++++++++++++++++---- >>> 1 file changed, 17 insertions(+), 4 deletions(-) >>> >>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c >>> index f0c3f00e97cb..8b2a31cef5cf 100644 >>> --- a/fs/btrfs/relocation.c >>> +++ b/fs/btrfs/relocation.c >>> @@ -3268,12 +3268,25 @@ static int relocate_file_extent_cluster(struct inode *inode, >>> nr++; >>> } >>> >>> - btrfs_set_extent_delalloc(inode, page_start, page_end, 0, NULL, >>> - 0); >>> + ret = btrfs_set_extent_delalloc(inode, page_start, page_end, 0, >>> + NULL, 0); >>> + if (ret) { >>> + unlock_page(page); >>> + put_page(page); >>> + btrfs_delalloc_release_metadata(BTRFS_I(inode), >>> + PAGE_SIZE); >>> + btrfs_delalloc_release_extents(BTRFS_I(inode), >>> + PAGE_SIZE); >>> + >>> + clear_extent_bits(&BTRFS_I(inode)->io_tree, >>> + page_start, page_end, >>> + EXTENT_LOCKED | EXTENT_BOUNDARY); >>> + goto out; >>> + >>> + } >>> set_page_dirty(page); >>> >>> - unlock_extent(&BTRFS_I(inode)->io_tree, >>> - page_start, page_end); >>> + unlock_extent(&BTRFS_I(inode)->io_tree, page_start, page_end); >> >> This is an unrelated change. Please don't mix pure white >> space/indentation changes with functional changes. > > David seems rather adamant in not accepting pure whitespace/indention Well, for every project I worked on, either open source or not this was always frowned upon. And it's quite obvious why... > changes on their own so I don't see a way to actually improve the code > base in that regard unless i slip them up when modifying nearby code. > There are a couple of space with trailing whitespace which I constantly > select out from my commits. > > Given that you have now also expressed objection to such cleanups, how > should they eventually be fixed? You "fix" them by doing a functional change affecting that code. If you know it's frowned upon, please don't keep sneaking this sort of changes, > >> >>> unlock_page(page); >>> put_page(page); >>> >>> -- >>> 2.7.4 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.” ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster 2018-01-31 10:53 ` Nikolay Borisov 2018-01-31 10:57 ` Filipe Manana @ 2018-01-31 14:50 ` David Sterba 1 sibling, 0 replies; 8+ messages in thread From: David Sterba @ 2018-01-31 14:50 UTC (permalink / raw) To: Nikolay Borisov; +Cc: fdmanana, dsterba, linux-btrfs On Wed, Jan 31, 2018 at 12:53:38PM +0200, Nikolay Borisov wrote: > > This is an unrelated change. Please don't mix pure white > > space/indentation changes with functional changes. > > David seems rather adamant in not accepting pure whitespace/indention > changes on their own so I don't see a way to actually improve the code > base in that regard unless i slip them up when modifying nearby code. If it's an unrelated change, than it's wrong to slip it in just because it's near. It's still an unrelated change. Other than that, and I think I mentioned that in your previous attempts to add whitespace changes, it just pollutes the commit history. Looking for a commit that potentially broke some code and finding a whitespace change just makes anybody grumpy. Maintaner should know and not let such changes in. I've been on both sides, and based on this experience I will not let in such changes. The review process in the mailinglist is there to catch that and point out, though pointing out just whitespace is kind of not welcome, unless the real review is also done. If there are minor things to fix, I do that at commit time, which means I edit majority of all patches or changelogs. In some cases I will let the patch author know so I don't have to fix that over and over again. (But it never lasts.) > There are a couple of space with trailing whitespace which I constantly > select out from my commits. > > Given that you have now also expressed objection to such cleanups, how > should they eventually be fixed? They will be fixed once the code on those lines gets changed. Which may not anytime soon. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster 2018-01-30 14:32 ` [RESEND PATCH] " Nikolay Borisov 2018-01-31 10:49 ` Filipe Manana @ 2018-01-31 15:14 ` Nikolay Borisov 2018-02-06 16:11 ` David Sterba 1 sibling, 1 reply; 8+ messages in thread From: Nikolay Borisov @ 2018-01-31 15:14 UTC (permalink / raw) To: dsterba; +Cc: linux-btrfs, Nikolay Borisov Essentially duplicate the error handling from the above block which handles the !PageUptodate(page) case and additionally clear EXTENT_BOUNDARY. Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Josef Bacik <jbacik@fb.com> --- V2: * Remove unrelated whitespace fix fs/btrfs/relocation.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index f0c3f00e97cb..cd2298d185dd 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -3268,8 +3268,22 @@ static int relocate_file_extent_cluster(struct inode *inode, nr++; } - btrfs_set_extent_delalloc(inode, page_start, page_end, 0, NULL, - 0); + ret = btrfs_set_extent_delalloc(inode, page_start, page_end, 0, + NULL, 0); + if (ret) { + unlock_page(page); + put_page(page); + btrfs_delalloc_release_metadata(BTRFS_I(inode), + PAGE_SIZE); + btrfs_delalloc_release_extents(BTRFS_I(inode), + PAGE_SIZE); + + clear_extent_bits(&BTRFS_I(inode)->io_tree, + page_start, page_end, + EXTENT_LOCKED | EXTENT_BOUNDARY); + goto out; + + } set_page_dirty(page); unlock_extent(&BTRFS_I(inode)->io_tree, -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster 2018-01-31 15:14 ` [PATCH v2] " Nikolay Borisov @ 2018-02-06 16:11 ` David Sterba 0 siblings, 0 replies; 8+ messages in thread From: David Sterba @ 2018-02-06 16:11 UTC (permalink / raw) To: Nikolay Borisov; +Cc: dsterba, linux-btrfs On Wed, Jan 31, 2018 at 05:14:02PM +0200, Nikolay Borisov wrote: > Essentially duplicate the error handling from the above block which > handles the !PageUptodate(page) case and additionally clear > EXTENT_BOUNDARY. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > Reviewed-by: Josef Bacik <jbacik@fb.com> Added to next, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-02-06 16:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-26 15:05 [PATCH] btrfs: Handle btrfs_set_extent_delalloc failure in relocate_file_extent_cluster Nikolay Borisov 2018-01-30 14:32 ` [RESEND PATCH] " Nikolay Borisov 2018-01-31 10:49 ` Filipe Manana 2018-01-31 10:53 ` Nikolay Borisov 2018-01-31 10:57 ` Filipe Manana 2018-01-31 14:50 ` David Sterba 2018-01-31 15:14 ` [PATCH v2] " Nikolay Borisov 2018-02-06 16:11 ` David Sterba
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.