UDF: alternate anchor block detection

UDF: alternate anchor block detection

[Amit Sahrawat]

While mounting UDF media, when the primary AVDP is not found at block
256, UDF code tries to read-in the alternate AVDP.
In the function udf_find_anchor, udf_scan_anchors is called 3 times,
where each call to udf_scan_anchors read 12 blocks.
In case there is no alternate AVDP stored, a total of 36 blocks are
read before mount fails - causing time delay for Mount Failure.

1) After first call to udf_scan_anchors and before the second call
there is varconv conversion, for the older drivers, which skips 7
blocks after every 32 blocks. What are these older drivers? Do we
still require this code?

2) After varconv conversion, why is there a third call to
udf_scan_anchors? In the 1st call and 3rd call to udf_scan_anchors,
exactly same blocks are read, so this 3rd call seems to be redundant.

I suggest this modified code for udf_find_anchor :-
lastblock = udf_scan_anchors(sb,udf_variable_to_fixed(sbi->s_last_block),fileset);
        if (lastblock)
                goto out;
         else {
                /* VARCONV didn't help. Clear it. */
                UDF_CLEAR_FLAG(sb, UDF_FLAG_VARCONV);
                return 0;
        }

Please share your opinion to the above '2' points.


Thanks & Regards,
Amit Sahrawat

Re: UDF: alternate anchor block detection

[Marcin Slusarz]

On Mon, Oct 03, 2011 at 05:16:15PM +0530, Amit Sahrawat wrote:
> While mounting UDF media, when the primary AVDP is not found at block
> 256, UDF code tries to read-in the alternate AVDP.
> In the function udf_find_anchor, udf_scan_anchors is called 3 times,
> where each call to udf_scan_anchors read 12 blocks.
> In case there is no alternate AVDP stored, a total of 36 blocks are
> read before mount fails - causing time delay for Mount Failure.
> 
> 1) After first call to udf_scan_anchors and before the second call
> there is varconv conversion, for the older drivers, which skips 7
> blocks after every 32 blocks. What are these older drivers? Do we
> still require this code?

I'm not sure what "drivers" are you talking about, but "UDF_SET_FLAG(sb,
UDF_FLAG_VARCONV)" does not convert anything - it just sets superblock
flag.

> 2) After varconv conversion, why is there a third call to
> udf_scan_anchors? In the 1st call and 3rd call to udf_scan_anchors,
> exactly same blocks are read, so this 3rd call seems to be redundant.

Because UDF_FLAG_VARCONV changes behavior of udf_scan_anchors.

Marcin

Re: UDF alternate anchor block detection

[Jan Kara]

  Hello,

On Fri 30-09-11 11:02:34, Ashish Sangwan wrote:
> While mounting UDF media, when the primary AVDP is not found at block 256,
> UDF code tries to read-in the alternate AVDP.
> In the function udf_find_anchor, udf_scan_anchors is called 3 times, where
> each call to udf_scan_anchors read 12 blocks.
> In case there is no alternate AVDP stored, a total of 36 blocks are read
> before mount fails - causing time delay for Mount Failure.
> 
> After first call to udf_scan_anchors and before the second call there is
> varconv conversion, for the older drivers, which skips 7 blocks after every
> 32 blocks. What are these older drivers? Do we still require this code?
  Frankly, I don't know what are these older drives. The code has been like
this when I started maintaining it and I didn't want to introduce
regressions so I kept the varconv code in.

> After varconv conversion, why is there a third call to udf_scan_anchors? In
> the 1st call and 3rd call to udf_scan_anchors, exactly same blocks are read,
> so this 3rd call seems to be redundant.
  This is not true - the second call uses
udf_variable_to_fixed(sbi->s_last_block) as the last block on the media
while the third call uses sbi->s_last_block as the last block on the media
(both these scans are performed with varconv enabled). Again I have not
seen a drive / medium where this would be needed but I believe original
author had some reason to put the code in.

I agree that the anchor searching code is a pain but that's how the
standard is defined...

> I suggest this modified code for udf_find_anchor :-
> lastblock =
> udf_scan_anchors(sb,udf_variable_to_fixed(sbi->s_last_block),fileset);
>         if (lastblock)
>                 goto out;
>          else {
>                 /* VARCONV didn't help. Clear it. */
>                 UDF_CLEAR_FLAG(sb, UDF_FLAG_VARCONV);
>                 return 0;
>         }
> 
> Please give your opininon to these 2 above points. If the code change seems
> relevant than I can make patch for it.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: UDF alternate anchor block detection

[NamJae Jeon]

2011/10/7 Jan Kara <jack@suse.cz>:
>  Hello,
>
> On Fri 30-09-11 11:02:34, Ashish Sangwan wrote:
>> While mounting UDF media, when the primary AVDP is not found at block 256,
>> UDF code tries to read-in the alternate AVDP.
>> In the function udf_find_anchor, udf_scan_anchors is called 3 times, where
>> each call to udf_scan_anchors read 12 blocks.
>> In case there is no alternate AVDP stored, a total of 36 blocks are read
>> before mount fails - causing time delay for Mount Failure.
>>
>> After first call to udf_scan_anchors and before the second call there is
>> varconv conversion, for the older drivers, which skips 7 blocks after every
>> 32 blocks. What are these older drivers? Do we still require this code?
>  Frankly, I don't know what are these older drives. The code has been like
> this when I started maintaining it and I didn't want to introduce
> regressions so I kept the varconv code in.
>
>> After varconv conversion, why is there a third call to udf_scan_anchors? In
>> the 1st call and 3rd call to udf_scan_anchors, exactly same blocks are read,
>> so this 3rd call seems to be redundant.
>  This is not true - the second call uses
> udf_variable_to_fixed(sbi->s_last_block) as the last block on the media
> while the third call uses sbi->s_last_block as the last block on the media
> (both these scans are performed with varconv enabled). Again I have not
> seen a drive / medium where this would be needed but I believe original
> author had some reason to put the code in.
I agree. It would cause side-effect if old code is skipped without
knowing author's intent.
So It is correct to be kept this like now.
>
> I agree that the anchor searching code is a pain but that's how the
> standard is defined...
Actually, proper exception will be needed for metadata very slow loading disc.
I will share with you if there is good solution after considering.
Thanks.
>
>> I suggest this modified code for udf_find_anchor :-
>> lastblock =
>> udf_scan_anchors(sb,udf_variable_to_fixed(sbi->s_last_block),fileset);
>>         if (lastblock)
>>                 goto out;
>>          else {
>>                 /* VARCONV didn't help. Clear it. */
>>                 UDF_CLEAR_FLAG(sb, UDF_FLAG_VARCONV);
>>                 return 0;
>>         }
>>
>> Please give your opininon to these 2 above points. If the code change seems
>> relevant than I can make patch for it.
>
>                                                                Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>