udf : skip finding mirror meta in same location with metadata.
PATCH v2 1/1 udf : skip finding mirror meta in same location with metadata.
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.
--- fs/udf/super.c | 51 +++++++++++++++++++++++++++++++-------------------- 1 files changed, 31 insertions(+), 20 deletions(-)
fs/udf/super.c +30 -19
@@ -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; + } }/*
Signature · 1 line
1.7.4.4
Quoted 7 lines · Namjae Jeon
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 <[email protected]>Signed-off-by Namjae Jeon <[email protected]>
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
Quoted 66 lines
fs/udf/super.c +30
-19@@ -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; + } }/* Signature · 1 line
1.7.4.4
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
2011/10/17 Jan Kara <[email protected]>:
Quoted 15 lines · Namjae Jeon
Quoted 7 lines
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 <[email protected]>Signed-off-by Namjae Jeon <[email protected]>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.
Quoted 5 lines
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.
Quoted 72 lines
Honza
Quoted 64 lines
fs/udf/super.c +30
-20@@ -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; + } } /* -- Quoted 2 lines
1.7.4.4
--
Jan Kara <[email protected]>
SUSE Labs, CR