* [RFC PATCH] btrfs: remove struct scrub_stx for superblock scrubbing
@ 2023-03-21 5:23 Anand Jain
2023-03-21 7:26 ` Qu Wenruo
0 siblings, 1 reply; 5+ messages in thread
From: Anand Jain @ 2023-03-21 5:23 UTC (permalink / raw)
To: linux-btrfs; +Cc: Anand Jain
Following the patchset that implements reader-friendly scrub code
made the struct scrub_stx is no longer required for scrubbing superblocks.
btrfs: scrub: use a more reader friendly code to implement scrub_simple_mirror()
Therefore, scrub_ctx does not need to be passed as a parameter,
(unless there are other plans for it).
This patch cleans up the code and is built on top of the above patchset.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/scrub.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index beccf763ae64..bc87277559d3 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -4909,12 +4909,12 @@ int scrub_enumerate_chunks(struct scrub_ctx *sctx,
return ret;
}
-static int scrub_one_super(struct scrub_ctx *sctx, struct btrfs_device *dev,
- struct page *page, u64 physical, u64 generation)
+static int scrub_one_super(struct btrfs_device *dev, struct page *page,
+ u64 physical, u64 generation)
{
- struct btrfs_fs_info *fs_info = sctx->fs_info;
struct bio_vec bvec;
struct bio bio;
+ struct btrfs_fs_info *fs_info = dev->fs_info;
struct btrfs_super_block *sb = page_address(page);
int ret;
@@ -4945,15 +4945,14 @@ static int scrub_one_super(struct scrub_ctx *sctx, struct btrfs_device *dev,
return ret;
}
-static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
- struct btrfs_device *scrub_dev)
+static noinline_for_stack int scrub_supers(struct btrfs_device *scrub_dev)
{
int i;
u64 bytenr;
u64 gen;
int ret = 0;
struct page *page;
- struct btrfs_fs_info *fs_info = sctx->fs_info;
+ struct btrfs_fs_info *fs_info = scrub_dev->fs_info;
if (BTRFS_FS_ERROR(fs_info))
return -EROFS;
@@ -4976,7 +4975,7 @@ static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
if (!btrfs_check_super_location(scrub_dev, bytenr))
continue;
- ret = scrub_one_super(sctx, scrub_dev, page, bytenr, gen);
+ ret = scrub_one_super(scrub_dev, page, bytenr, gen);
if (ret)
break;
}
@@ -5172,7 +5171,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,
* kick off writing super in log tree sync.
*/
mutex_lock(&fs_info->fs_devices->device_list_mutex);
- ret = scrub_supers(sctx, dev);
+ ret = scrub_supers(dev);
mutex_unlock(&fs_info->fs_devices->device_list_mutex);
spin_lock(&sctx->stat_lock);
--
2.38.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] btrfs: remove struct scrub_stx for superblock scrubbing
2023-03-21 5:23 [RFC PATCH] btrfs: remove struct scrub_stx for superblock scrubbing Anand Jain
@ 2023-03-21 7:26 ` Qu Wenruo
2023-03-21 10:11 ` Anand Jain
0 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2023-03-21 7:26 UTC (permalink / raw)
To: Anand Jain, linux-btrfs
On 2023/3/21 13:23, Anand Jain wrote:
> Following the patchset that implements reader-friendly scrub code
> made the struct scrub_stx is no longer required for scrubbing superblocks.
>
> btrfs: scrub: use a more reader friendly code to implement scrub_simple_mirror()
>
> Therefore, scrub_ctx does not need to be passed as a parameter,
> (unless there are other plans for it).
>
> This patch cleans up the code and is built on top of the above patchset.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
Looks good, if you're fine I can fold this into the offending patch in
the next update.
Thanks,
Qu
> ---
> fs/btrfs/scrub.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index beccf763ae64..bc87277559d3 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -4909,12 +4909,12 @@ int scrub_enumerate_chunks(struct scrub_ctx *sctx,
> return ret;
> }
>
> -static int scrub_one_super(struct scrub_ctx *sctx, struct btrfs_device *dev,
> - struct page *page, u64 physical, u64 generation)
> +static int scrub_one_super(struct btrfs_device *dev, struct page *page,
> + u64 physical, u64 generation)
> {
> - struct btrfs_fs_info *fs_info = sctx->fs_info;
> struct bio_vec bvec;
> struct bio bio;
> + struct btrfs_fs_info *fs_info = dev->fs_info;
> struct btrfs_super_block *sb = page_address(page);
> int ret;
>
> @@ -4945,15 +4945,14 @@ static int scrub_one_super(struct scrub_ctx *sctx, struct btrfs_device *dev,
> return ret;
> }
>
> -static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
> - struct btrfs_device *scrub_dev)
> +static noinline_for_stack int scrub_supers(struct btrfs_device *scrub_dev)
> {
> int i;
> u64 bytenr;
> u64 gen;
> int ret = 0;
> struct page *page;
> - struct btrfs_fs_info *fs_info = sctx->fs_info;
> + struct btrfs_fs_info *fs_info = scrub_dev->fs_info;
>
> if (BTRFS_FS_ERROR(fs_info))
> return -EROFS;
> @@ -4976,7 +4975,7 @@ static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
> if (!btrfs_check_super_location(scrub_dev, bytenr))
> continue;
>
> - ret = scrub_one_super(sctx, scrub_dev, page, bytenr, gen);
> + ret = scrub_one_super(scrub_dev, page, bytenr, gen);
> if (ret)
> break;
> }
> @@ -5172,7 +5171,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,
> * kick off writing super in log tree sync.
> */
> mutex_lock(&fs_info->fs_devices->device_list_mutex);
> - ret = scrub_supers(sctx, dev);
> + ret = scrub_supers(dev);
> mutex_unlock(&fs_info->fs_devices->device_list_mutex);
>
> spin_lock(&sctx->stat_lock);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] btrfs: remove struct scrub_stx for superblock scrubbing
2023-03-21 7:26 ` Qu Wenruo
@ 2023-03-21 10:11 ` Anand Jain
2023-03-23 1:13 ` Qu Wenruo
0 siblings, 1 reply; 5+ messages in thread
From: Anand Jain @ 2023-03-21 10:11 UTC (permalink / raw)
To: Qu Wenruo, Anand Jain, linux-btrfs
On 3/21/23 15:26, Qu Wenruo wrote:
>
>
> On 2023/3/21 13:23, Anand Jain wrote:
>> Following the patchset that implements reader-friendly scrub code
>> made the struct scrub_stx is no longer required for scrubbing
>> superblocks.
>>
>> btrfs: scrub: use a more reader friendly code to implement
>> scrub_simple_mirror()
>>
>> Therefore, scrub_ctx does not need to be passed as a parameter,
>> (unless there are other plans for it).
>>
>> This patch cleans up the code and is built on top of the above patchset.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>
> Looks good, if you're fine I can fold this into the offending patch in
> the next update.
>
IMO, this patch is a cleanup rather than a bug fix, so there
isn't offending patch. If it is folded to the patch 1/12, it
may be too many objectives in one patch.
Nonetheless, I have no objections if you still decide to fold it.
Thanks, Anand
> Thanks,
> Qu
>
>> ---
>> fs/btrfs/scrub.c | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
>> index beccf763ae64..bc87277559d3 100644
>> --- a/fs/btrfs/scrub.c
>> +++ b/fs/btrfs/scrub.c
>> @@ -4909,12 +4909,12 @@ int scrub_enumerate_chunks(struct scrub_ctx
>> *sctx,
>> return ret;
>> }
>> -static int scrub_one_super(struct scrub_ctx *sctx, struct
>> btrfs_device *dev,
>> - struct page *page, u64 physical, u64 generation)
>> +static int scrub_one_super(struct btrfs_device *dev, struct page *page,
>> + u64 physical, u64 generation)
>> {
>> - struct btrfs_fs_info *fs_info = sctx->fs_info;
>> struct bio_vec bvec;
>> struct bio bio;
>> + struct btrfs_fs_info *fs_info = dev->fs_info;
>> struct btrfs_super_block *sb = page_address(page);
>> int ret;
>> @@ -4945,15 +4945,14 @@ static int scrub_one_super(struct scrub_ctx
>> *sctx, struct btrfs_device *dev,
>> return ret;
>> }
>> -static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
>> - struct btrfs_device *scrub_dev)
>> +static noinline_for_stack int scrub_supers(struct btrfs_device
>> *scrub_dev)
>> {
>> int i;
>> u64 bytenr;
>> u64 gen;
>> int ret = 0;
>> struct page *page;
>> - struct btrfs_fs_info *fs_info = sctx->fs_info;
>> + struct btrfs_fs_info *fs_info = scrub_dev->fs_info;
>> if (BTRFS_FS_ERROR(fs_info))
>> return -EROFS;
>> @@ -4976,7 +4975,7 @@ static noinline_for_stack int
>> scrub_supers(struct scrub_ctx *sctx,
>> if (!btrfs_check_super_location(scrub_dev, bytenr))
>> continue;
>> - ret = scrub_one_super(sctx, scrub_dev, page, bytenr, gen);
>> + ret = scrub_one_super(scrub_dev, page, bytenr, gen);
>> if (ret)
>> break;
>> }
>> @@ -5172,7 +5171,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info
>> *fs_info, u64 devid, u64 start,
>> * kick off writing super in log tree sync.
>> */
>> mutex_lock(&fs_info->fs_devices->device_list_mutex);
>> - ret = scrub_supers(sctx, dev);
>> + ret = scrub_supers(dev);
>> mutex_unlock(&fs_info->fs_devices->device_list_mutex);
>> spin_lock(&sctx->stat_lock);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] btrfs: remove struct scrub_stx for superblock scrubbing
2023-03-21 10:11 ` Anand Jain
@ 2023-03-23 1:13 ` Qu Wenruo
2023-03-23 2:56 ` Anand Jain
0 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2023-03-23 1:13 UTC (permalink / raw)
To: Anand Jain, linux-btrfs
On 2023/3/21 18:11, Anand Jain wrote:
> On 3/21/23 15:26, Qu Wenruo wrote:
>>
>>
>> On 2023/3/21 13:23, Anand Jain wrote:
>>> Following the patchset that implements reader-friendly scrub code
>>> made the struct scrub_stx is no longer required for scrubbing
>>> superblocks.
>>>
>>> btrfs: scrub: use a more reader friendly code to implement
>>> scrub_simple_mirror()
>>>
>>> Therefore, scrub_ctx does not need to be passed as a parameter,
>>> (unless there are other plans for it).
>>>
>>> This patch cleans up the code and is built on top of the above patchset.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>
>> Looks good, if you're fine I can fold this into the offending patch in
>> the next update.
>>
>
> IMO, this patch is a cleanup rather than a bug fix, so there
> isn't offending patch. If it is folded to the patch 1/12, it
> may be too many objectives in one patch.
The cleanup is only possible because of the patch "btrfs: scrub: use
dedicated super block verification function to scrub one super block".
As the old code relies on the scrub_sectors() function, thus needing the
@sctx parameter.
And it's indeed my fault not fully cleaning up the parameters.
Thus I believe it's better to fold it into the mentioned patch.
Thanks,
Qu
>
> Nonetheless, I have no objections if you still decide to fold it.
>
> Thanks, Anand
>
>
>> Thanks,
>> Qu
>>
>>> ---
>>> fs/btrfs/scrub.c | 15 +++++++--------
>>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
>>> index beccf763ae64..bc87277559d3 100644
>>> --- a/fs/btrfs/scrub.c
>>> +++ b/fs/btrfs/scrub.c
>>> @@ -4909,12 +4909,12 @@ int scrub_enumerate_chunks(struct scrub_ctx
>>> *sctx,
>>> return ret;
>>> }
>>> -static int scrub_one_super(struct scrub_ctx *sctx, struct
>>> btrfs_device *dev,
>>> - struct page *page, u64 physical, u64 generation)
>>> +static int scrub_one_super(struct btrfs_device *dev, struct page *page,
>>> + u64 physical, u64 generation)
>>> {
>>> - struct btrfs_fs_info *fs_info = sctx->fs_info;
>>> struct bio_vec bvec;
>>> struct bio bio;
>>> + struct btrfs_fs_info *fs_info = dev->fs_info;
>>> struct btrfs_super_block *sb = page_address(page);
>>> int ret;
>>> @@ -4945,15 +4945,14 @@ static int scrub_one_super(struct scrub_ctx
>>> *sctx, struct btrfs_device *dev,
>>> return ret;
>>> }
>>> -static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
>>> - struct btrfs_device *scrub_dev)
>>> +static noinline_for_stack int scrub_supers(struct btrfs_device
>>> *scrub_dev)
>>> {
>>> int i;
>>> u64 bytenr;
>>> u64 gen;
>>> int ret = 0;
>>> struct page *page;
>>> - struct btrfs_fs_info *fs_info = sctx->fs_info;
>>> + struct btrfs_fs_info *fs_info = scrub_dev->fs_info;
>>> if (BTRFS_FS_ERROR(fs_info))
>>> return -EROFS;
>>> @@ -4976,7 +4975,7 @@ static noinline_for_stack int
>>> scrub_supers(struct scrub_ctx *sctx,
>>> if (!btrfs_check_super_location(scrub_dev, bytenr))
>>> continue;
>>> - ret = scrub_one_super(sctx, scrub_dev, page, bytenr, gen);
>>> + ret = scrub_one_super(scrub_dev, page, bytenr, gen);
>>> if (ret)
>>> break;
>>> }
>>> @@ -5172,7 +5171,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info
>>> *fs_info, u64 devid, u64 start,
>>> * kick off writing super in log tree sync.
>>> */
>>> mutex_lock(&fs_info->fs_devices->device_list_mutex);
>>> - ret = scrub_supers(sctx, dev);
>>> + ret = scrub_supers(dev);
>>> mutex_unlock(&fs_info->fs_devices->device_list_mutex);
>>> spin_lock(&sctx->stat_lock);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] btrfs: remove struct scrub_stx for superblock scrubbing
2023-03-23 1:13 ` Qu Wenruo
@ 2023-03-23 2:56 ` Anand Jain
0 siblings, 0 replies; 5+ messages in thread
From: Anand Jain @ 2023-03-23 2:56 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 23/03/2023 09:13, Qu Wenruo wrote:
>
>
> On 2023/3/21 18:11, Anand Jain wrote:
>> On 3/21/23 15:26, Qu Wenruo wrote:
>>>
>>>
>>> On 2023/3/21 13:23, Anand Jain wrote:
>>>> Following the patchset that implements reader-friendly scrub code
>>>> made the struct scrub_stx is no longer required for scrubbing
>>>> superblocks.
>>>>
>>>> btrfs: scrub: use a more reader friendly code to implement
>>>> scrub_simple_mirror()
>>>>
>>>> Therefore, scrub_ctx does not need to be passed as a parameter,
>>>> (unless there are other plans for it).
>>>>
>>>> This patch cleans up the code and is built on top of the above
>>>> patchset.
>>>>
>>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>>
>>> Looks good, if you're fine I can fold this into the offending patch
>>> in the next update.
>>>
>>
>> IMO, this patch is a cleanup rather than a bug fix, so there
>> isn't offending patch. If it is folded to the patch 1/12, it
>> may be too many objectives in one patch.
>
> The cleanup is only possible because of the patch "btrfs: scrub: use
> dedicated super block verification function to scrub one super block".
>
> As the old code relies on the scrub_sectors() function, thus needing the
> @sctx parameter.
>
> And it's indeed my fault not fully cleaning up the parameters.
>
> Thus I believe it's better to fold it into the mentioned patch.
Yes. Please go ahead.
Thanks, Anand
> Thanks,
> Qu
>>
>> Nonetheless, I have no objections if you still decide to fold it.
>>
>> Thanks, Anand
>>
>>
>>> Thanks,
>>> Qu
>>>
>>>> ---
>>>> fs/btrfs/scrub.c | 15 +++++++--------
>>>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
>>>> index beccf763ae64..bc87277559d3 100644
>>>> --- a/fs/btrfs/scrub.c
>>>> +++ b/fs/btrfs/scrub.c
>>>> @@ -4909,12 +4909,12 @@ int scrub_enumerate_chunks(struct scrub_ctx
>>>> *sctx,
>>>> return ret;
>>>> }
>>>> -static int scrub_one_super(struct scrub_ctx *sctx, struct
>>>> btrfs_device *dev,
>>>> - struct page *page, u64 physical, u64 generation)
>>>> +static int scrub_one_super(struct btrfs_device *dev, struct page
>>>> *page,
>>>> + u64 physical, u64 generation)
>>>> {
>>>> - struct btrfs_fs_info *fs_info = sctx->fs_info;
>>>> struct bio_vec bvec;
>>>> struct bio bio;
>>>> + struct btrfs_fs_info *fs_info = dev->fs_info;
>>>> struct btrfs_super_block *sb = page_address(page);
>>>> int ret;
>>>> @@ -4945,15 +4945,14 @@ static int scrub_one_super(struct scrub_ctx
>>>> *sctx, struct btrfs_device *dev,
>>>> return ret;
>>>> }
>>>> -static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
>>>> - struct btrfs_device *scrub_dev)
>>>> +static noinline_for_stack int scrub_supers(struct btrfs_device
>>>> *scrub_dev)
>>>> {
>>>> int i;
>>>> u64 bytenr;
>>>> u64 gen;
>>>> int ret = 0;
>>>> struct page *page;
>>>> - struct btrfs_fs_info *fs_info = sctx->fs_info;
>>>> + struct btrfs_fs_info *fs_info = scrub_dev->fs_info;
>>>> if (BTRFS_FS_ERROR(fs_info))
>>>> return -EROFS;
>>>> @@ -4976,7 +4975,7 @@ static noinline_for_stack int
>>>> scrub_supers(struct scrub_ctx *sctx,
>>>> if (!btrfs_check_super_location(scrub_dev, bytenr))
>>>> continue;
>>>> - ret = scrub_one_super(sctx, scrub_dev, page, bytenr, gen);
>>>> + ret = scrub_one_super(scrub_dev, page, bytenr, gen);
>>>> if (ret)
>>>> break;
>>>> }
>>>> @@ -5172,7 +5171,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info
>>>> *fs_info, u64 devid, u64 start,
>>>> * kick off writing super in log tree sync.
>>>> */
>>>> mutex_lock(&fs_info->fs_devices->device_list_mutex);
>>>> - ret = scrub_supers(sctx, dev);
>>>> + ret = scrub_supers(dev);
>>>> mutex_unlock(&fs_info->fs_devices->device_list_mutex);
>>>> spin_lock(&sctx->stat_lock);
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-23 2:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-21 5:23 [RFC PATCH] btrfs: remove struct scrub_stx for superblock scrubbing Anand Jain
2023-03-21 7:26 ` Qu Wenruo
2023-03-21 10:11 ` Anand Jain
2023-03-23 1:13 ` Qu Wenruo
2023-03-23 2:56 ` Anand Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).