* [PATCH v2] udf : skip finding mirror meta in same location with metadata.
@ 2011-10-15 8:41 Namjae Jeon
2011-10-17 13:41 ` Jan Kara
0 siblings, 1 reply; 3+ messages in thread
From: Namjae Jeon @ 2011-10-15 8:41 UTC (permalink / raw)
To: jack; +Cc: linux-kernel, Namjae Jeon, Ashish Sangwan
If metadata duplicate flag is clear, it means there is mirror metadata file in same location with metadata file.
So this patch try to skip finding mirror partition on this case.
Mount time is improved and avoid unnecessarily loading mirror metadata.
Signed-off-by: Ashish Sangwan <ashishsangwan2@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
---
fs/udf/super.c | 51 +++++++++++++++++++++++++++++++--------------------
1 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/fs/udf/super.c b/fs/udf/super.c
index e58123a..f76f9e6 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -857,28 +857,39 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
mdata->s_metadata_fe = NULL;
}
- /* mirror file entry */
- addr.logicalBlockNum = mdata->s_mirror_file_loc;
- addr.partitionReferenceNum = map->s_partition_num;
+ if (!mdata->s_dup_md_flag) {
+ udf_info("There is mirror metadata file in same location with metadata file.");
+ if (fe_error) {
+ udf_err(sb, "So mirror inode efe can not found too");
+ goto error_exit;
+ } else {
+ mdata->s_mirror_fe = mdata->s_metadata_fe;
+ }
+ } else {
- udf_debug("Mirror metadata file location: block = %d part = %d\n",
- addr.logicalBlockNum, addr.partitionReferenceNum);
+ /* mirror file entry */
+ addr.logicalBlockNum = mdata->s_mirror_file_loc;
+ addr.partitionReferenceNum = map->s_partition_num;
- mdata->s_mirror_fe = udf_iget(sb, &addr);
+ udf_debug("Mirror metadata file location: block = %d part = %d\n",
+ addr.logicalBlockNum, addr.partitionReferenceNum);
- if (mdata->s_mirror_fe == NULL) {
- if (fe_error) {
- udf_err(sb, "mirror inode efe not found and metadata inode is missing too, exiting...\n");
- goto error_exit;
- } else
- udf_warn(sb, "mirror inode efe not found, but metadata inode is OK\n");
- } else if (UDF_I(mdata->s_mirror_fe)->i_alloc_type !=
- ICBTAG_FLAG_AD_SHORT) {
- udf_warn(sb, "mirror inode efe does not have short allocation descriptors!\n");
- iput(mdata->s_mirror_fe);
- mdata->s_mirror_fe = NULL;
- if (fe_error)
- goto error_exit;
+ mdata->s_mirror_fe = udf_iget(sb, &addr);
+
+ if (mdata->s_mirror_fe == NULL) {
+ if (fe_error) {
+ udf_err(sb, "mirror inode efe not found and metadata inode is missing too, exiting...\n");
+ goto error_exit;
+ } else
+ udf_warn(sb, "mirror inode efe not found, but metadata inode is OK\n");
+ } else if (UDF_I(mdata->s_mirror_fe)->i_alloc_type !=
+ ICBTAG_FLAG_AD_SHORT) {
+ udf_warn(sb, "mirror inode efe does not have short allocation descriptors!\n");
+ iput(mdata->s_mirror_fe);
+ mdata->s_mirror_fe = NULL;
+ if (fe_error)
+ goto error_exit;
+ }
}
/*
--
1.7.4.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] udf : skip finding mirror meta in same location with metadata.
2011-10-15 8:41 [PATCH v2] udf : skip finding mirror meta in same location with metadata Namjae Jeon
@ 2011-10-17 13:41 ` Jan Kara
2011-10-18 17:06 ` NamJae Jeon
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2011-10-17 13:41 UTC (permalink / raw)
To: Namjae Jeon; +Cc: jack, linux-kernel, Ashish Sangwan
On Sat 15-10-11 17:41:46, Namjae Jeon wrote:
> If metadata duplicate flag is clear, it means there is mirror metadata
> file in same location with metadata file. So this patch try to skip
> finding mirror partition on this case. Mount time is improved and avoid
> unnecessarily loading mirror metadata.
>
> Signed-off-by: Ashish Sangwan <ashishsangwan2@gmail.com>
> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
Thanks for the patch but I do not completely agree. The point is that if
there is error which prevents us to read metadata of "metadata FE", then we
can very well succeed with reading equivalent metadata from the "mirror
metadata FE" even when "duplicated" flag is set - metadata is never shared
according to the specification (OSTA UDF Spec, section 2.2.10, "Flags"
bullet). That is exactly what the current code tries to achieve.
Now I agree that in the majority of cases we don't need to read mirror
metadata FE since the original one is OK so there is some room for
improvement. What we could do is just read metadata FE and if it succeeds,
skip reading mirror metadata FE and try to read it only the first time
udf_get_pblock_meta25() fails to map the block from metadata FE.
Honza
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index e58123a..f76f9e6 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -857,28 +857,39 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
> mdata->s_metadata_fe = NULL;
> }
>
> - /* mirror file entry */
> - addr.logicalBlockNum = mdata->s_mirror_file_loc;
> - addr.partitionReferenceNum = map->s_partition_num;
> + if (!mdata->s_dup_md_flag) {
> + udf_info("There is mirror metadata file in same location with metadata file.");
> + if (fe_error) {
> + udf_err(sb, "So mirror inode efe can not found too");
> + goto error_exit;
> + } else {
> + mdata->s_mirror_fe = mdata->s_metadata_fe;
> + }
> + } else {
>
> - udf_debug("Mirror metadata file location: block = %d part = %d\n",
> - addr.logicalBlockNum, addr.partitionReferenceNum);
> + /* mirror file entry */
> + addr.logicalBlockNum = mdata->s_mirror_file_loc;
> + addr.partitionReferenceNum = map->s_partition_num;
>
> - mdata->s_mirror_fe = udf_iget(sb, &addr);
> + udf_debug("Mirror metadata file location: block = %d part = %d\n",
> + addr.logicalBlockNum, addr.partitionReferenceNum);
>
> - if (mdata->s_mirror_fe == NULL) {
> - if (fe_error) {
> - udf_err(sb, "mirror inode efe not found and metadata inode is missing too, exiting...\n");
> - goto error_exit;
> - } else
> - udf_warn(sb, "mirror inode efe not found, but metadata inode is OK\n");
> - } else if (UDF_I(mdata->s_mirror_fe)->i_alloc_type !=
> - ICBTAG_FLAG_AD_SHORT) {
> - udf_warn(sb, "mirror inode efe does not have short allocation descriptors!\n");
> - iput(mdata->s_mirror_fe);
> - mdata->s_mirror_fe = NULL;
> - if (fe_error)
> - goto error_exit;
> + mdata->s_mirror_fe = udf_iget(sb, &addr);
> +
> + if (mdata->s_mirror_fe == NULL) {
> + if (fe_error) {
> + udf_err(sb, "mirror inode efe not found and metadata inode is missing too, exiting...\n");
> + goto error_exit;
> + } else
> + udf_warn(sb, "mirror inode efe not found, but metadata inode is OK\n");
> + } else if (UDF_I(mdata->s_mirror_fe)->i_alloc_type !=
> + ICBTAG_FLAG_AD_SHORT) {
> + udf_warn(sb, "mirror inode efe does not have short allocation descriptors!\n");
> + iput(mdata->s_mirror_fe);
> + mdata->s_mirror_fe = NULL;
> + if (fe_error)
> + goto error_exit;
> + }
> }
>
> /*
> --
> 1.7.4.4
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] udf : skip finding mirror meta in same location with metadata.
2011-10-17 13:41 ` Jan Kara
@ 2011-10-18 17:06 ` NamJae Jeon
0 siblings, 0 replies; 3+ messages in thread
From: NamJae Jeon @ 2011-10-18 17:06 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-kernel, Ashish Sangwan
2011/10/17 Jan Kara <jack@suse.cz>:
> On Sat 15-10-11 17:41:46, Namjae Jeon wrote:
>> If metadata duplicate flag is clear, it means there is mirror metadata
>> file in same location with metadata file. So this patch try to skip
>> finding mirror partition on this case. Mount time is improved and avoid
>> unnecessarily loading mirror metadata.
>>
>> Signed-off-by: Ashish Sangwan <ashishsangwan2@gmail.com>
>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
> Thanks for the patch but I do not completely agree. The point is that if
> there is error which prevents us to read metadata of "metadata FE", then we
> can very well succeed with reading equivalent metadata from the "mirror
> metadata FE" even when "duplicated" flag is set - metadata is never shared
> according to the specification (OSTA UDF Spec, section 2.2.10, "Flags"
> bullet). That is exactly what the current code tries to achieve.
>
> Now I agree that in the majority of cases we don't need to read mirror
> metadata FE since the original one is OK so there is some room for
> improvement. What we could do is just read metadata FE and if it succeeds,
> skip reading mirror metadata FE and try to read it only the first time
> udf_get_pblock_meta25() fails to map the block from metadata FE.
Thanks for your idea. Actually I and ashish are also considering to
post same suggestion with your idea.
I will make the new patch included your suggestion today.
>
> Honza
>> diff --git a/fs/udf/super.c b/fs/udf/super.c
>> index e58123a..f76f9e6 100644
>> --- a/fs/udf/super.c
>> +++ b/fs/udf/super.c
>> @@ -857,28 +857,39 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
>> mdata->s_metadata_fe = NULL;
>> }
>>
>> - /* mirror file entry */
>> - addr.logicalBlockNum = mdata->s_mirror_file_loc;
>> - addr.partitionReferenceNum = map->s_partition_num;
>> + if (!mdata->s_dup_md_flag) {
>> + udf_info("There is mirror metadata file in same location with metadata file.");
>> + if (fe_error) {
>> + udf_err(sb, "So mirror inode efe can not found too");
>> + goto error_exit;
>> + } else {
>> + mdata->s_mirror_fe = mdata->s_metadata_fe;
>> + }
>> + } else {
>>
>> - udf_debug("Mirror metadata file location: block = %d part = %d\n",
>> - addr.logicalBlockNum, addr.partitionReferenceNum);
>> + /* mirror file entry */
>> + addr.logicalBlockNum = mdata->s_mirror_file_loc;
>> + addr.partitionReferenceNum = map->s_partition_num;
>>
>> - mdata->s_mirror_fe = udf_iget(sb, &addr);
>> + udf_debug("Mirror metadata file location: block = %d part = %d\n",
>> + addr.logicalBlockNum, addr.partitionReferenceNum);
>>
>> - if (mdata->s_mirror_fe == NULL) {
>> - if (fe_error) {
>> - udf_err(sb, "mirror inode efe not found and metadata inode is missing too, exiting...\n");
>> - goto error_exit;
>> - } else
>> - udf_warn(sb, "mirror inode efe not found, but metadata inode is OK\n");
>> - } else if (UDF_I(mdata->s_mirror_fe)->i_alloc_type !=
>> - ICBTAG_FLAG_AD_SHORT) {
>> - udf_warn(sb, "mirror inode efe does not have short allocation descriptors!\n");
>> - iput(mdata->s_mirror_fe);
>> - mdata->s_mirror_fe = NULL;
>> - if (fe_error)
>> - goto error_exit;
>> + mdata->s_mirror_fe = udf_iget(sb, &addr);
>> +
>> + if (mdata->s_mirror_fe == NULL) {
>> + if (fe_error) {
>> + udf_err(sb, "mirror inode efe not found and metadata inode is missing too, exiting...\n");
>> + goto error_exit;
>> + } else
>> + udf_warn(sb, "mirror inode efe not found, but metadata inode is OK\n");
>> + } else if (UDF_I(mdata->s_mirror_fe)->i_alloc_type !=
>> + ICBTAG_FLAG_AD_SHORT) {
>> + udf_warn(sb, "mirror inode efe does not have short allocation descriptors!\n");
>> + iput(mdata->s_mirror_fe);
>> + mdata->s_mirror_fe = NULL;
>> + if (fe_error)
>> + goto error_exit;
>> + }
>> }
>>
>> /*
>> --
>> 1.7.4.4
>>
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-10-18 17:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-15 8:41 [PATCH v2] udf : skip finding mirror meta in same location with metadata Namjae Jeon
2011-10-17 13:41 ` Jan Kara
2011-10-18 17:06 ` NamJae Jeon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox