2011/11/8 Sachin Prabhu <sprabhu@xxxxxxxxxx>:
> Pavel,
>
> The code behaves in a similar manner to the previous versions of the
> code in case of non posix locks. This simply adds support for FL_FLOCK
> in addition to Posix lock.
>
> Let me explain what has to happen for the goto out to be called in the 2
> if conditions which seem to be the problem.
>
> 1)
> if (unlock) {
> /*Check for existance of lock*/
> flock->fl_flags |= FL_EXISTS;
> if (do_vfs_lock(flock->fl_file, flock) == -ENOENT)
> goto out;
>
> The comment associated with FL_EXISTS in include/linux/fs.h is
> #define FL_EXISTS 16 /* when unlocking, test for existence */
>
> If this was an unlock request and the call to unlock the file reports
> that the lock doesn't exists then goto out and exit with -ENOENT. This
> is correct since there is no lock on this file to unlock.
>
> If the lock did exist, then it unlocks the lock, proceeds normally and
> calls cifs_unlock_range(cfile, flock, xid) further below which unlocks
> the lock held in the local brlocks cache.
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!
>
> 2)
> } else {
> /*Check if we can obtain lock*/
> flock->fl_flags |= FL_ACCESS;
> rc = do_vfs_lock(flock->fl_file, flock);
> if (rc < 0)
> goto out;
>
>
> The comment associated with FL_ACCESS in include/linux/fs.h is
> #define FL_ACCESS 8 /* not trying to lock, just looking */
>
> It attempts to lock the file. If this attempt fails, it refuses to do
> the remote lock call since we will not be able to get a local lock in
> this case. In this case, exit with the error message returned by
> do_vfs_lock().
>
> If it face no problems obtaining a lock locally then proceed to perform
> normal locking operations as before and finally obtain a lock on the
> local system with
>
> out_lock:
> if (rc == 0 && lock) {
> /*We have to sleep here*/
> flock->fl_flags |= FL_SLEEP;
> do_vfs_lock(file, flock);
> }
>
> We have to use do_vfs_lock() in this case since we are now dealing with
> both flocks as well as posix locks. The same use case you explained
> earlier holds where any existing flocks and posix locks on the file have
> to be cleared when the file is closed. The old call to
> posix_lock_file_wait will only end up adding a posix lock to the file
> when we instead need a flock.
>
>
> Sachin Prabhu
>
>
--
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
