On 6/15/16, 03:13, "Christoph Hellwig" <hch@xxxxxxxxxxxxx> wrote:
>> +void
>> +nfs_lock_bio(struct nfs_inode *nfsi)
>
>bio stands for buffered I/O? This could really use a more descriptive
>name and/or a comment..
>
>> +{
>> + /* Be an optimist! */
>> + down_read(&nfsi->io_lock);
>> + if (test_bit(NFS_INO_ODIRECT, &nfsi->flags) == 0)
>> + return;
>> + up_read(&nfsi->io_lock);
>
>So if no direct I/O is going on this locks shared?
>
>> + /* Slow path.... */
>> + down_write(&nfsi->io_lock);
>> + clear_bit(NFS_INO_ODIRECT, &nfsi->flags);
>> + downgrade_write(&nfsi->io_lock);
>
>The whole locking here seems rather confusing. Why not use the XFS
>locking model:
The locking is actually simpler than XFS. We have 2 I/O modes: buffered I/O and direct I/O. The write lock is there to ensure safe transitions between those 2 modes, but once the mode is set, we _only_ use shared locks in order to allow parallelism.
>
>buffered write: exclusive
>buffered read: shared
>direct write: shared (exclusive for pagecache invalidate)
>direct read: shared (exclusive for pagecache invalidate)
>
>The nice thing is than in 4.7-rc i_mutex has been replaced with a
>rw_mutex so you can just use that in shared mode for direct I/O
>as-is without needing any new lock.
We would end up serialising reads and writes, since the latter grab an exclusive lock in generic_file_write(). Why do that if we don’t have to?
Cheers
Trond
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
