Re: ll_rw_block audit

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

 



On Wed 23-06-10 12:43:25, Christoph Hellwig wrote:
> I've just sent a patch to fsdevel to remove the SWRITE* flags and add
> a new helper to write buffers using proper locking, which tricked me
> into an audit of all ll_rw_block users.  There's not a lot left now,
> so it might be time to kill them.  For writes we only have 6 users left:
> 
> fs/buffer.c:		ll_rw_block(WRITE, 1, &bh);
> fs/hfs/hfs_fs.h:	ll_rw_block(WRITE, 1, &bh);
> fs/reiserfs/journal.c:	ll_rw_block(WRITE, 1, &bh);
> fs/reiserfs/journal.c:	ll_rw_block(WRITE, 1, &tbh);
> 
> plus two odd commented out ones in jbd/jbd2:
> 
> fs/jbd/recovery.c:			/* ll_rw_block(WRITE, 1, &nbh); */
> fs/jbd2/recovery.c:			/* ll_rw_block(WRITE, 1, &nbh); */
  Just nuke these. We mark the buffer dirty shortly before this and
there's no reason to send replayed buffer to disk just now...

> The one in hfs is obviously a broken attempt to implement
> sync_dirty_buffer, I'll send a patch to fix it.
> 
> That leaves:
> 
>   - write_boundary_block.
> 
> 	This one is only used by __mpage_writepage and is a data
> 	integrity operation if WB_SYNC_ALL is set.  It should probably
> 	use write_dirty_buffer at least for that case.  Is it worth
> 	keeping the trylock for WB_SYNC_NONE writes?
  I don't think it must be a data integrity write in WB_SYNC_ALL mode.
All the "buffer_boundary" logic is just an optimization used by some
filesystems (ext2, ext3, gfs2) to get nicely linear IO when data blocks
are intermixed with indirect blocks. In fact, noone even guarantees that
the block written/read by buffer_boundary logic belongs to the file...
Data integrity guarantees are achieved by different means
(either by sync_mapping_buffer() in case of ext2, or by journal commits
in case of ext3, and probably gfs2).

> 	Btw, what's the reason this function is in buffer.c instead
> 	of mpage.c?
  Dunno...
  
>  - write_ordered_buffers and flush_commit_list in reiserfs.  Both of
>    these look like data integrity operations to me, so using
>    write_dirty_buffer seems in order.
  Yes, they are data integrity writes. The first one is writing
data=ordered mode buffer, the second one is writing metadata block to a
journal.

> For the read side we have 38 users. Most of them wait on the buffer
> in some more or less broken form.  These should be replaced with the
> bh_uptodate_or_lock/bh_submit_read combination, or possibly a helper
> combining the two.  There's also a lot of places that look like
> broken reimplementations of __bread_slow.

								Honza
-- 
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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