| Message ID | 20170727185255.5122-1-fdmanana@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/27/2017 02:52 PM, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > If the range being cleared was not marked for defrag and we are not > about to clear the range from the defrag status, we don't need to > lock and unlock the inode. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Thanks Filipe, looks like it goes all the way back to: commit 47059d930f0e002ff851beea87d738146804726d Author: Wang Shilong <wangsl.fnst@cn.fujitsu.com> Date: Thu Jul 3 18:22:07 2014 +0800 Btrfs: make defragment work with nodatacow option I can't see how the inode lock is required here. Reviewed-by: Chris Mason <clm@fb.com> -chris > --- > fs/btrfs/inode.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index eb495e956d53..51c45c0a8553 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -1797,10 +1797,11 @@ static void btrfs_clear_bit_hook(void *private_data, > u64 len = state->end + 1 - state->start; > u32 num_extents = count_max_extents(len); > > - spin_lock(&inode->lock); > - if ((state->state & EXTENT_DEFRAG) && (*bits & EXTENT_DEFRAG)) > + if ((state->state & EXTENT_DEFRAG) && (*bits & EXTENT_DEFRAG)) { > + spin_lock(&inode->lock); > inode->defrag_bytes -= len; > - spin_unlock(&inode->lock); > + spin_unlock(&inode->lock); > + } > > /* > * set_bit and clear bit hooks normally require _irqsave/restore > -- 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
On Thu, Aug 3, 2017 at 11:00 PM, Chris Mason <clm@fb.com> wrote: > > > On 07/27/2017 02:52 PM, fdmanana@kernel.org wrote: >> >> From: Filipe Manana <fdmanana@suse.com> >> >> If the range being cleared was not marked for defrag and we are not >> about to clear the range from the defrag status, we don't need to >> lock and unlock the inode. >> >> Signed-off-by: Filipe Manana <fdmanana@suse.com> > > > Thanks Filipe, looks like it goes all the way back to: > > commit 47059d930f0e002ff851beea87d738146804726d > Author: Wang Shilong <wangsl.fnst@cn.fujitsu.com> > Date: Thu Jul 3 18:22:07 2014 +0800 > > Btrfs: make defragment work with nodatacow option > > I can't see how the inode lock is required here. This blames to me, thanks for fixing it. Reviewed-by: Wang Shilong <wangshilong1991@gmail.com> > > Reviewed-by: Chris Mason <clm@fb.com> > > -chris > >> --- >> fs/btrfs/inode.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >> index eb495e956d53..51c45c0a8553 100644 >> --- a/fs/btrfs/inode.c >> +++ b/fs/btrfs/inode.c >> @@ -1797,10 +1797,11 @@ static void btrfs_clear_bit_hook(void >> *private_data, >> u64 len = state->end + 1 - state->start; >> u32 num_extents = count_max_extents(len); >> - spin_lock(&inode->lock); >> - if ((state->state & EXTENT_DEFRAG) && (*bits & EXTENT_DEFRAG)) >> + if ((state->state & EXTENT_DEFRAG) && (*bits & EXTENT_DEFRAG)) { >> + spin_lock(&inode->lock); >> inode->defrag_bytes -= len; >> - spin_unlock(&inode->lock); >> + spin_unlock(&inode->lock); >> + } >> /* >> * set_bit and clear bit hooks normally require _irqsave/restore >> > -- > 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 -- 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
On 08/03/2017 11:25 AM, Wang Shilong wrote: > On Thu, Aug 3, 2017 at 11:00 PM, Chris Mason <clm@fb.com> wrote: >> >> >> On 07/27/2017 02:52 PM, fdmanana@kernel.org wrote: >>> >>> From: Filipe Manana <fdmanana@suse.com> >>> >>> If the range being cleared was not marked for defrag and we are not >>> about to clear the range from the defrag status, we don't need to >>> lock and unlock the inode. >>> >>> Signed-off-by: Filipe Manana <fdmanana@suse.com> >> >> >> Thanks Filipe, looks like it goes all the way back to: >> >> commit 47059d930f0e002ff851beea87d738146804726d >> Author: Wang Shilong <wangsl.fnst@cn.fujitsu.com> >> Date: Thu Jul 3 18:22:07 2014 +0800 >> >> Btrfs: make defragment work with nodatacow option >> >> I can't see how the inode lock is required here. > > This blames to me, thanks for fixing it. No blame ;) I'll take code that works any day. -chris -- 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
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index eb495e956d53..51c45c0a8553 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -1797,10 +1797,11 @@ static void btrfs_clear_bit_hook(void *private_data, u64 len = state->end + 1 - state->start; u32 num_extents = count_max_extents(len); - spin_lock(&inode->lock); - if ((state->state & EXTENT_DEFRAG) && (*bits & EXTENT_DEFRAG)) + if ((state->state & EXTENT_DEFRAG) && (*bits & EXTENT_DEFRAG)) { + spin_lock(&inode->lock); inode->defrag_bytes -= len; - spin_unlock(&inode->lock); + spin_unlock(&inode->lock); + } /* * set_bit and clear bit hooks normally require _irqsave/restore