On Thu, 31 Jan 2013 11:40:41 -0500, Josef Bacik wrote:
> On Thu, Jan 31, 2013 at 02:23:19AM -0700, Miao Xie wrote:
>> Currently, we can do unlocked dio reads, but the following race
>> is possible:
>>
>> dio_read_task                        truncate_task
>>                              ->btrfs_setattr()
>> ->btrfs_direct_IO
>>     ->__blockdev_direct_IO
>>       ->btrfs_get_block
>>                                ->btrfs_truncate()
>>                               #alloc truncated blocks
>>                               #to other inode
>>       ->submit_io()
>>      #INFORMATION LEAK
>>
>> In order to avoid this problem, we must serialize unlocked dio reads with
>> truncate by inode_dio_wait().
>>
> 
> So I had thinking about this, are we sure we don't want to just lock the 
> extent
> range when we truncate?  I'm good with this, but it seems like we might as 
> well
> and be consistent and use the extent locks.  What do you think?  Thanks,

But comparing with the current approach, the extent lock has the following 
problem:
        Dio_Read_Task                   Truncate_task
                                        truncate file
                                          set isize to 4096
                                          drop pages
        lock extent[4096, 8191]
        read extent[4096, 8191]
        unlock extent[4096, 8191]
                                          lock extent[4096, -1ULL]
                                          truncate item
                                          unlock extent[4096, -1ULL]
        lock extent[8192, ...]
        read extent[8192, ...]
          no extent item
          zero the buffer
        unlock extent[8192, ...]

we get the data that is mixed with new data.(Punch hole also has this problem, 
we need
fix)

Thanks
Miao
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to