On 2014-09-01 20:43, Andreas Rohner wrote: > Hi Ryusuke, > On 2014-09-01 19:59, Ryusuke Konishi wrote: >> On Sun, 31 Aug 2014 17:47:13 +0200, Andreas Rohner wrote: >>> Under normal circumstances nilfs_sync_fs() writes out the super block, >>> which causes a flush of the underlying block device. But this depends on >>> the THE_NILFS_SB_DIRTY flag, which is only set if the pointer to the >>> last segment crosses a segment boundary. So if only a small amount of >>> data is written before the call to nilfs_sync_fs(), no flush of the >>> block device occurs. >>> >>> In the above case an additional call to blkdev_issue_flush() is needed. >>> To prevent unnecessary overhead, the new flag THE_NILFS_FLUSHED is >>> introduced, which is cleared whenever new logs are written and set >>> whenever the block device is flushed. >>> >>> Signed-off-by: Andreas Rohner <[email protected]> >> >> The patch looks good to me except that I feel the use of atomic >> test-and-set bitwise operations something unfavorable (though it's >> logically correct). I will try to send this to upstream as is unless >> a comment comes to mind. > > I originally thought, that it is necessary to do it atomically to avoid > a race condition, but I am not so sure about that any more. I think the > only case we have to avoid is, to call set_nilfs_flushed() after > blkdev_issue_flush(), because this could race with the > clear_nilfs_flushed() from the segment construction. So this should also > work: > > + if (wait && !err && nilfs_test_opt(nilfs, BARRIER) && > + !nilfs_flushed(nilfs)) { > + set_nilfs_flushed(nilfs); > + err = blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL); > + if (err != -EIO) > + err = 0; > + } > +
On the other hand, it says in the comments to set_bit(), that it can be reordered on architectures other than x86. test_and_set_bit() implies a memory barrier on all architectures. But I don't think the processor would reorder set_nilfs_flushed() after the external function call to blkdev_issue_flush(), would it? /** * set_bit - Atomically set a bit in memory * @nr: the bit to set * @addr: the address to start counting from * * This function is atomic and may not be reordered. See __set_bit() * if you do not require the atomic guarantees. * * Note: there are no guarantees that this function will not be reordered * on non x86 architectures, so if you are writing portable code, * make sure not to rely on its reordering guarantees. */ br, Andreas Rohner -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
