Messages in this thread | | | Date | Sun, 7 Nov 2010 15:14:03 +0100 | From | Jan Kara <> | Subject | Re: [PATCH 1/4] udf: Add missed protection for s_lvid_dirty |
| |
On Sat 06-11-10 14:05:30, Christoph Hellwig wrote: > On Sat, Nov 06, 2010 at 06:47:08PM +0100, Alessio Igor Bogani wrote: > > As reported in udf_sb.h the udf_sb_infoi's structure member s_lvid_dirty should > > be protected by s_alloc_mutex. Added that mutex on a couple of places where it > > miss. > > The whole s_lvid_dirty flag doesn't make any sense to me. As a start it > simply duplicates s_dirty in the VFS superblock, but even more it just > controls the dirty state of s_lvid_bh. I think you could simply kill > s_lvid_dirty, plus s_dirty inside udf and replace all calls to > udf_updated_lvid with a simple mark_buffer_dirty(sbi->s_lvid_bh) and > also get rid of all the locking around it. Well, I should have probably commented more on it :). The problem is that people sometimes tend to mount CD-RW with UDF filesystem directly under Linux. Now LVID is updated on each allocation so if you write to the media, the block carrying LVID is written rather often (like once per 5 seconds or so) which quickly deteriorates that block on the media. So we basically want to hide the dirty bit on that buffer from VFS so that it does not write the block so often. Looking at it now, cleaner fix might be to move the counters to sbi and write them to the LVID only during umount. So that might be a nice cleanup as well.
> While looking at this I also noticed that large parts of udf_open_lvid > and udf_close_lvid are basically duplicate. The only difference seems > to be setting an integrityType of LVID_INTEGRITY_TYPE_OPEN vs > LVID_INTEGRITY_TYPE_CLOSE and updating a few revision counters on close. > If you're interested in working on udf that seems like a nice little > cleanup. Yes, that could be cleaned up.
Honza -- Jan Kara <jack@suse.cz> SUSE Labs, CR
| |