2011/11/11 Sachin Prabhu <sprabhu@xxxxxxxxxx>:
> On Tue, 2011-11-08 at 23:08 +0400, Pavel Shilovsky wrote:
>
>> Let's predict the following situation:
>>
>> 1) f1 = open(filename)
>> 2) f2 = open(filename) # the same process
>> 3) f1.lock(0, 1) # lock file from 0 to 1
>> 4) f2.lock(1, 2) # lock file from 1 to 2
>> 5) f1.unlock(0, 2)
>>
>> it unlocks both locks (3) and (4) from the VFS cache but unlock only
>> (3) lock from CIFS-lock cache and server side (see cifs_unlock_range -
>> it unlocks only locks held by the same fd, not the same process) - the
>> lock (4) is still there.
>>
>> 6) f2.unlock(1, 2)
>>
>> it appears into if (unlock) {}, fails on FL_EXISTS with -ENOENT and
>> goto out. The result we can't unlock the existing lock - the problem!
>>
>
> Hello Pavel,
>
> Thanks for your reply.
>
> In my opinion this behaviour is wrong. The application running on the
> Linux box will expect the locking behaviour to follow Posix locking
> semantics. The current behaviour will break these applications which
> expect the system to work in this manner.
>
> After step 5, the client should unlock the whole range and send 2
> separate unlock requests to the file server using 2 different netfids if
> required.
>
>
I agree with you - that's wrong according to POSIX. I only wanted to
point that your patch can't live with the existing code behavior. May
be it is the time to change this long life bug.
I think flock patch should not bring any new behavior (even if it
fixes something) and should be split into several patches:
1) add flock support only (no changes into the existing code behavior);
2), 3), ...) - any other changes (bugfixes) you want to make one-by-one.
It lets us bisect possible problems if they appear to be.
--
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
