Re: [PATCH 2/5] btrfs: use a separate variable to store the num_bytes of the qgroup_reserve
David Sterba 于2018年8月8日周三 下午9:57写道：
>
> On Wed, Aug 08, 2018 at 11:04:37AM +0800, Lu Fengqi wrote:
> > On Tue, Aug 07, 2018 at 06:19:12PM +0200, David Sterba wrote:
> > >On Sat, Aug 04, 2018 at 09:10:54PM +0800, Lu Fengqi wrote:
> > >> After btrfs_qgroup_reserve_meta_prealloc(), num_bytes will be assigned
> > >> again by btrfs_calc_trans_metadata_size(). Therefore, once block_rsv
> > >> fails, we cannot properly free the num_bytes of the previous
> > >> qgroup_reserve.
> > >
> > >This does not look like a trivial cleanup at all. There was an unused
> > >parameter, removed in c4c129db5da8f070147f175 ("btrfs: drop unused
> > >parameter qgroup_reserved"), that introduced the bug. This was in this
> > >rc1 so it's a regression and I'll consider pushing it to the 4.18 final.
> >
> > I apologize for the inconvenience. I should add the Fixes tag, and really
> > shouldn't mix it into the trivial cleanup patch set.
>
> As the bug does not qualify as urgent regression, I'm not going to
> forward it to 4.18. Please update the subject and changelog so it
> reflects that's an actual fix. I'll add it to the 4.19 queue then.
> Thanks.
No problem. I will send it tomorrow.
-
Thanks,
Lu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] btrfs: use a separate variable to store the num_bytes of the qgroup_reserve
On Wed, Aug 08, 2018 at 11:04:37AM +0800, Lu Fengqi wrote:
> On Tue, Aug 07, 2018 at 06:19:12PM +0200, David Sterba wrote:
> >On Sat, Aug 04, 2018 at 09:10:54PM +0800, Lu Fengqi wrote:
> >> After btrfs_qgroup_reserve_meta_prealloc(), num_bytes will be assigned
> >> again by btrfs_calc_trans_metadata_size(). Therefore, once block_rsv
> >> fails, we cannot properly free the num_bytes of the previous
> >> qgroup_reserve.
> >
> >This does not look like a trivial cleanup at all. There was an unused
> >parameter, removed in c4c129db5da8f070147f175 ("btrfs: drop unused
> >parameter qgroup_reserved"), that introduced the bug. This was in this
> >rc1 so it's a regression and I'll consider pushing it to the 4.18 final.
>
> I apologize for the inconvenience. I should add the Fixes tag, and really
> shouldn't mix it into the trivial cleanup patch set.
As the bug does not qualify as urgent regression, I'm not going to
forward it to 4.18. Please update the subject and changelog so it
reflects that's an actual fix. I'll add it to the 4.19 queue then.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/5] btrfs: use a separate variable to store the num_bytes of the qgroup_reserve
> -Original Message-
> From: David Sterba [mailto:dste...@suse.cz]
> Sent: Wednesday, August 08, 2018 12:19 AM
> To: Lu, Fengqi/陆 丰麒
> Cc: linux-btrfs@vger.kernel.org; Gu, Jinxiang/顾 金香
> Subject: Re: [PATCH 2/5] btrfs: use a separate variable to store the
> num_bytes of the qgroup_reserve
>
> On Sat, Aug 04, 2018 at 09:10:54PM +0800, Lu Fengqi wrote:
> > After btrfs_qgroup_reserve_meta_prealloc(), num_bytes will be assigned
> > again by btrfs_calc_trans_metadata_size(). Therefore, once block_rsv
> > fails, we cannot properly free the num_bytes of the previous
> > qgroup_reserve.
>
> This does not look like a trivial cleanup at all. There was an unused
> parameter, removed in c4c129db5da8f070147f175 ("btrfs: drop unused
> parameter qgroup_reserved"), that introduced the bug.
Yes. It was introduced by fix above.
I missed the second assignment of num_bytes.
And it should use two variable to record reserve size of qgroup and blockgroup.
>This was in this
> rc1 so it's a regression and I'll consider pushing it to the 4.18 final.
>
N�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{�n谶�)��骅w* jg�报 �茛j/�赇z罐���2���ㄨ��&�)摺�a囤�� �G���h��j:+v���w��佶
Re: [PATCH 2/5] btrfs: use a separate variable to store the num_bytes of the qgroup_reserve
On Tue, Aug 07, 2018 at 06:19:12PM +0200, David Sterba wrote:
>On Sat, Aug 04, 2018 at 09:10:54PM +0800, Lu Fengqi wrote:
>> After btrfs_qgroup_reserve_meta_prealloc(), num_bytes will be assigned
>> again by btrfs_calc_trans_metadata_size(). Therefore, once block_rsv
>> fails, we cannot properly free the num_bytes of the previous
>> qgroup_reserve.
>
>This does not look like a trivial cleanup at all. There was an unused
>parameter, removed in c4c129db5da8f070147f175 ("btrfs: drop unused
>parameter qgroup_reserved"), that introduced the bug. This was in this
>rc1 so it's a regression and I'll consider pushing it to the 4.18 final.
>
>
I apologize for the inconvenience. I should add the Fixes tag, and really
shouldn't mix it into the trivial cleanup patch set.
--
Thanks,
Lu
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] btrfs: use a separate variable to store the num_bytes of the qgroup_reserve
On Sat, Aug 04, 2018 at 09:10:54PM +0800, Lu Fengqi wrote:
> After btrfs_qgroup_reserve_meta_prealloc(), num_bytes will be assigned
> again by btrfs_calc_trans_metadata_size(). Therefore, once block_rsv
> fails, we cannot properly free the num_bytes of the previous
> qgroup_reserve.
This does not look like a trivial cleanup at all. There was an unused
parameter, removed in c4c129db5da8f070147f175 ("btrfs: drop unused
parameter qgroup_reserved"), that introduced the bug. This was in this
rc1 so it's a regression and I'll consider pushing it to the 4.18 final.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
