lkml.org 
[lkml]   [2020]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] exfat: retain 'VolumeFlags' properly
From
Date

>>>> Also, rename ERR_MEDIUM to MED_FAILURE.
>>> I think that MEDIA_FAILURE looks better.
>>
>> I think so too.
>> If so, should I change VOL_DIRTY to VOLUME_DIRTY?
> Yes, maybe.

OK.
I'll rename both in v2.


>>>> - p_boot->vol_flags = cpu_to_le16(new_flag);
>>>> + p_boot->vol_flags = cpu_to_le16(new_flags);
>>> How about set or clear only dirty bit to on-disk volume flags instead
>>> of using
>>> sbi->vol_flags_noclear ?
>>> if (set)
>>> p_boot->vol_flags |= cpu_to_le16(VOL_DIRTY);
>>> else
>>> p_boot->vol_flags &= cpu_to_le16(~VOL_DIRTY);

Please let me know about this code.
Does this code assume that 'sbi->vol_flags'(last vol_flag value) is not used?

If you use sbi->vol_flags, I think the original idea is fine.

sbi-> vol_flags = new_flag;
p_boot->vol_flags = cpu_to_le16(new_flag);


>> In this way, the initial VOL_DIRTY cannot be retained.
>> The purpose of this patch is to avoid losing the initial VOL_DIRTY and MED_FAILURE.
>> Furthermore, I'm also thinking of setting MED_FAILURE.
> I know what you do. I mean this function does not need to be called if volume dirty
> Is already set on on-disk volume flags as I said earlier.

Hmm?
Does it mean the caller check that volume was dirty at mount, and caller determine
whether to call exfat_set_vol_flags() or not?
If so, MEDIA_FAILUR needs to be set independently of the volume-dirty state.


>> However, the formula for 'new_flags' may be a bit complicated.
>> Is it better to change to the following?
>>
>> if (new_flags == VOL_CLEAN)
>> new_flags = sbi->vol_flags & ~VOL_DIRTY
>> else
>> new_flags |= sbi->vol_flags;
>>
>> new_flags |= sbi->vol_flags_noclear;
>>
>>
>> one more point,
>> Is there a better name than 'vol_flags_noclear'?
>> (I can't come up with a good name anymore)
> It looks complicated. It would be better to simply set/clear VOLUME DIRTY bit.

I think exfat_set_vol_flags() gets a little complicated,
because it needs the followings (with bit operation)
a) Set/Clear VOLUME_DIRTY.
b) Set MEDIA_FAILUR.
c) Do not change other flags.
d) Retain VOLUME_DIRTY/MEDIA_FAILUR as it is when mounted.

'vol_flags_noclear' is used for d).

Bit-by-bit operation makes the code redundant.
I think it's not a bad way to handle multiple bits.

What do you think?


BR
---
Tetsuhiro Kohada <kohada.t2@gmail.com>

\
 
 \ /
  Last update: 2020-07-15 12:06    [W:0.081 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site