* [PATCH] udf : skip mirror metadata FE since metadata FE is ok.
@ 2011-10-19 15:03 Namjae Jeon
2011-10-20 23:47 ` Jan Kara
0 siblings, 1 reply; 4+ messages in thread
From: Namjae Jeon @ 2011-10-19 15:03 UTC (permalink / raw)
To: jack; +Cc: linux-kernel, Namjae Jeon, Ashish Sangwan
By Jan Kara's suggestion, This patch skip mirror metadata FE since metadata FE is ok. And try to read it only the first time udf_get_pblock_meta25() fails to map the block from metadata FE.
Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
Signed-off-by: Ashish Sangwan <ashishsangwan2@gmail.com>
---
fs/udf/partition.c | 20 ++++++++++++++++++++
fs/udf/super.c | 33 ++++++++++++++++-----------------
2 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/fs/udf/partition.c b/fs/udf/partition.c
index f3e472c..8bfc25b 100644
--- a/fs/udf/partition.c
+++ b/fs/udf/partition.c
@@ -311,6 +311,7 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
struct udf_meta_data *mdata;
uint32_t retblk;
struct inode *inode;
+ struct kernel_lb_addr addr;
udf_debug("READING from METADATA\n");
@@ -323,6 +324,25 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
retblk = udf_try_read_meta(inode, block, partition, offset);
if (retblk == 0xFFFFFFFF) {
udf_warn(sb, "error reading from METADATA, trying to read from MIRROR\n");
+ if (mdata->s_mirror_fe == NULL) {
+ /* mirror file entry */
+ addr.logicalBlockNum = mdata->s_mirror_file_loc;
+ addr.partitionReferenceNum = map->s_partition_num;
+ udf_debug("Mirror metadata file location: block = %d part = %d\n",
+ addr.logicalBlockNum,
+ addr.partitionReferenceNum);
+ mdata->s_mirror_fe = udf_iget(sb, &addr);
+
+ if (mdata->s_mirror_fe == NULL)
+ udf_err(sb, "mirror inode efe not found");
+ 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;
+ }
+ }
+
inode = mdata->s_mirror_fe;
if (!inode)
return 0xFFFFFFFF;
diff --git a/fs/udf/super.c b/fs/udf/super.c
index e58123a..659d088 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -857,29 +857,28 @@ 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 (fe_error) {
+ /* mirror file entry */
+ addr.logicalBlockNum = mdata->s_mirror_file_loc;
+ addr.partitionReferenceNum = map->s_partition_num;
- udf_debug("Mirror metadata file location: block = %d part = %d\n",
- addr.logicalBlockNum, addr.partitionReferenceNum);
+ udf_debug("Mirror metadata file location: block = %d part = %d\n",
+ addr.logicalBlockNum, addr.partitionReferenceNum);
- mdata->s_mirror_fe = udf_iget(sb, &addr);
+ mdata->s_mirror_fe = udf_iget(sb, &addr);
- if (mdata->s_mirror_fe == NULL) {
- if (fe_error) {
+ if (mdata->s_mirror_fe == NULL) {
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)
+ } 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;
goto error_exit;
- }
+ }
+ } else
+ mdata->s_mirror_fe = NULL;
/*
* bitmap file entry
--
1.7.4.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] udf : skip mirror metadata FE since metadata FE is ok.
2011-10-19 15:03 [PATCH] udf : skip mirror metadata FE since metadata FE is ok Namjae Jeon
@ 2011-10-20 23:47 ` Jan Kara
2011-10-23 1:24 ` NamJae Jeon
2011-10-23 1:24 ` NamJae Jeon
0 siblings, 2 replies; 4+ messages in thread
From: Jan Kara @ 2011-10-20 23:47 UTC (permalink / raw)
To: Namjae Jeon; +Cc: jack, linux-kernel, Ashish Sangwan
On Thu 20-10-11 00:03:18, Namjae Jeon wrote:
> By Jan Kara's suggestion, This patch skip mirror metadata FE since
> metadata FE is ok. And try to read it only the first time
> udf_get_pblock_meta25() fails to map the block from metadata FE.
>
> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
> Signed-off-by: Ashish Sangwan <ashishsangwan2@gmail.com>
> ---
> fs/udf/partition.c | 20 ++++++++++++++++++++
> fs/udf/super.c | 33 ++++++++++++++++-----------------
> 2 files changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/fs/udf/partition.c b/fs/udf/partition.c
> index f3e472c..8bfc25b 100644
> --- a/fs/udf/partition.c
> +++ b/fs/udf/partition.c
> @@ -311,6 +311,7 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
> struct udf_meta_data *mdata;
> uint32_t retblk;
> struct inode *inode;
> + struct kernel_lb_addr addr;
>
> udf_debug("READING from METADATA\n");
>
> @@ -323,6 +324,25 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
> retblk = udf_try_read_meta(inode, block, partition, offset);
> if (retblk == 0xFFFFFFFF) {
> udf_warn(sb, "error reading from METADATA, trying to read from MIRROR\n");
> + if (mdata->s_mirror_fe == NULL) {
This won't be good because if s_mirror_fe cannot be loaded as well, you'd
just try loading it on every block mapping which failed from the primary
metadata map. So you should have a flag somewhere whether in mdata, whether
you tried loading mirror_fe or not and use that.
> + /* mirror file entry */
> + addr.logicalBlockNum = mdata->s_mirror_file_loc;
> + addr.partitionReferenceNum = map->s_partition_num;
> + udf_debug("Mirror metadata file location: block = %d part = %d\n",
> + addr.logicalBlockNum,
> + addr.partitionReferenceNum);
> + mdata->s_mirror_fe = udf_iget(sb, &addr);
> +
> + if (mdata->s_mirror_fe == NULL)
> + udf_err(sb, "mirror inode efe not found");
> + 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;
> + }
Also please create a helper function for loading metadata/mirror metadat
FE so that we don't have loading duplicated in three places... Thanks.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] udf : skip mirror metadata FE since metadata FE is ok.
2011-10-20 23:47 ` Jan Kara
@ 2011-10-23 1:24 ` NamJae Jeon
2011-10-23 1:24 ` NamJae Jeon
1 sibling, 0 replies; 4+ messages in thread
From: NamJae Jeon @ 2011-10-23 1:24 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-kernel, Ashish Sangwan
2011/10/21 Jan Kara <jack@suse.cz>:
> On Thu 20-10-11 00:03:18, Namjae Jeon wrote:
>> By Jan Kara's suggestion, This patch skip mirror metadata FE since
>> metadata FE is ok. And try to read it only the first time
>> udf_get_pblock_meta25() fails to map the block from metadata FE.
>>
>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
>> Signed-off-by: Ashish Sangwan <ashishsangwan2@gmail.com>
>> ---
>> fs/udf/partition.c | 20 ++++++++++++++++++++
>> fs/udf/super.c | 33 ++++++++++++++++-----------------
>> 2 files changed, 36 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/udf/partition.c b/fs/udf/partition.c
>> index f3e472c..8bfc25b 100644
>> --- a/fs/udf/partition.c
>> +++ b/fs/udf/partition.c
>> @@ -311,6 +311,7 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
>> struct udf_meta_data *mdata;
>> uint32_t retblk;
>> struct inode *inode;
>> + struct kernel_lb_addr addr;
>>
>> udf_debug("READING from METADATA\n");
>>
>> @@ -323,6 +324,25 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
>> retblk = udf_try_read_meta(inode, block, partition, offset);
>> if (retblk == 0xFFFFFFFF) {
>> udf_warn(sb, "error reading from METADATA, trying to read from MIRROR\n");
>> + if (mdata->s_mirror_fe == NULL) {
> This won't be good because if s_mirror_fe cannot be loaded as well, you'd
> just try loading it on every block mapping which failed from the primary
> metadata map. So you should have a flag somewhere whether in mdata, whether
> you tried loading mirror_fe or not and use that.
>
>> + /* mirror file entry */
>> + addr.logicalBlockNum = mdata->s_mirror_file_loc;
>> + addr.partitionReferenceNum = map->s_partition_num;
>> + udf_debug("Mirror metadata file location: block = %d part = %d\n",
>> + addr.logicalBlockNum,
>> + addr.partitionReferenceNum);
>> + mdata->s_mirror_fe = udf_iget(sb, &addr);
>> +
>> + if (mdata->s_mirror_fe == NULL)
>> + udf_err(sb, "mirror inode efe not found");
>> + 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;
>> + }
> Also please create a helper function for loading metadata/mirror metadat
> FE so that we don't have loading duplicated in three places... Thanks.
Hi Jan.
Thanks for your review.
I will do it.
>
> Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] udf : skip mirror metadata FE since metadata FE is ok.
2011-10-20 23:47 ` Jan Kara
2011-10-23 1:24 ` NamJae Jeon
@ 2011-10-23 1:24 ` NamJae Jeon
1 sibling, 0 replies; 4+ messages in thread
From: NamJae Jeon @ 2011-10-23 1:24 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-kernel, Ashish Sangwan
2011/10/21 Jan Kara <jack@suse.cz>:
> On Thu 20-10-11 00:03:18, Namjae Jeon wrote:
>> By Jan Kara's suggestion, This patch skip mirror metadata FE since
>> metadata FE is ok. And try to read it only the first time
>> udf_get_pblock_meta25() fails to map the block from metadata FE.
>>
>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
>> Signed-off-by: Ashish Sangwan <ashishsangwan2@gmail.com>
>> ---
>> fs/udf/partition.c | 20 ++++++++++++++++++++
>> fs/udf/super.c | 33 ++++++++++++++++-----------------
>> 2 files changed, 36 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/udf/partition.c b/fs/udf/partition.c
>> index f3e472c..8bfc25b 100644
>> --- a/fs/udf/partition.c
>> +++ b/fs/udf/partition.c
>> @@ -311,6 +311,7 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
>> struct udf_meta_data *mdata;
>> uint32_t retblk;
>> struct inode *inode;
>> + struct kernel_lb_addr addr;
>>
>> udf_debug("READING from METADATA\n");
>>
>> @@ -323,6 +324,25 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
>> retblk = udf_try_read_meta(inode, block, partition, offset);
>> if (retblk == 0xFFFFFFFF) {
>> udf_warn(sb, "error reading from METADATA, trying to read from MIRROR\n");
>> + if (mdata->s_mirror_fe == NULL) {
> This won't be good because if s_mirror_fe cannot be loaded as well, you'd
> just try loading it on every block mapping which failed from the primary
> metadata map. So you should have a flag somewhere whether in mdata, whether
> you tried loading mirror_fe or not and use that.
>
>> + /* mirror file entry */
>> + addr.logicalBlockNum = mdata->s_mirror_file_loc;
>> + addr.partitionReferenceNum = map->s_partition_num;
>> + udf_debug("Mirror metadata file location: block = %d part = %d\n",
>> + addr.logicalBlockNum,
>> + addr.partitionReferenceNum);
>> + mdata->s_mirror_fe = udf_iget(sb, &addr);
>> +
>> + if (mdata->s_mirror_fe == NULL)
>> + udf_err(sb, "mirror inode efe not found");
>> + 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;
>> + }
> Also please create a helper function for loading metadata/mirror metadat
> FE so that we don't have loading duplicated in three places... Thanks.
Hi Jan.
Thanks for your review.
I will do it.
>
> Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-10-23 1:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-19 15:03 [PATCH] udf : skip mirror metadata FE since metadata FE is ok Namjae Jeon
2011-10-20 23:47 ` Jan Kara
2011-10-23 1:24 ` NamJae Jeon
2011-10-23 1:24 ` NamJae Jeon
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).