Re: [PATCH] exfat: retain 'VolumeFlags' properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxx>



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux