[CRIU] [PATCH 3/6] locks: Don't dump locks in per-task manner (v2)

[Pavel Emelyanov]

On 08/28/2014 04:51 PM, Andrew Vagin wrote:
> On Tue, Aug 26, 2014 at 09:30:40PM +0400, Pavel Emelyanov wrote:
>> We have a problem with file locks (bug #2512) -- the /proc/locks
>> file shows the ID of lock creator, not the owner. Thus, if the
>> creator died, but holder is still alive, criu fails to dump the
>> lock held by latter task.
>>
>> The proposal is to find who _might_ hold the lock by checking
>> for dev:inode pairs on lock vs file descriptors being dumped.
>> If the creator of the lock is still alive, then he will take
>> the priority.
>>
>> One thing to note about flocks -- these belong to file entries,
>> not to tasks. Thus, when we meet one, we should check whether
>> the flock is really held by task's FD by trying to set yet
>> another one. In case of success -- lock really belongs to fd
>> we dump, in case it doesn't trylock should fail.
>>
>> At the very end -- walk the list of locks and dump them all at
>> once, which is possible by merge of per-task file-locks images
>> into one global one.
>>
>> Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
>> ---
> 
> ...
> 
>> +int note_file_lock(struct pid *pid, int fd, int lfd, struct fd_parms *p)
>> +{
>> +	struct file_lock *fl;
>> +
>> +	list_for_each_entry(fl, &file_lock_list, list) {
>> +		if (!lock_file_match(fl, p))
>> +			continue;
>> +
>> +		if (fl->fl_kind == FL_POSIX) {
>> +			/*
>> +			 * POSIX locks cannot belong to anyone
>> +			 * but creator.
>> +			 */
>> +			if (fl->fl_owner != pid->real)
>> +				continue;
>> +		} else /* fl->fl_kind == FL_FLOCK */ {
>> +			int ret;
>> +
>> +			/*
>> +			 * FLOCKs can be inherited across fork,
>> +			 * thus we can have any task as lock
>> +			 * owner. But the creator is preferred
>> +			 * anyway.
>> +			 */
>> +
>> +			if (fl->fl_owner != pid->real &&
>> +					fl->real_owner != -1)
>> +				continue;
>> +
>> +			pr_debug("Checking lock holder %d:%d\n", pid->real, fd);
>> +			ret = flock(lfd, LOCK_EX | LOCK_NB);
> 
> I don't understand how do you find descriptors, which hold shared locks.

If EX lock is approved, this denotes that the current FD holds one,
be it SH or EX doesn't matter.

> """
> LOCK_SH  Place a shared lock.  More than one process may hold a shared
> lock for a given file at a given time.
> 
> LOCK_EX  Place an exclusive lock.  Only one process may hold an
> exclusive lock for a given file at a given time.
> """
> 
>> +			pr_debug("   `- %d/%d\n", ret, errno);
>> +			if (ret != 0) {
>> +				if (errno != EAGAIN) {
>> +					pr_err("Bogus lock test result %d\n", ret);
>> +					return -1;
>> +				}
>> +
>> +				continue;
>> +			}
>> +
>> +			/*
>> +			 * The ret == 0 means, that new lock doesn't conflict
>> +			 * with any others on the file. But since we do know, 
>> +			 * that there should be some other one (file is found
>> +			 * in /proc/locks), it means that the lock is already
>> +			 * on file pointed by fd.
>> +			 */
>> +		}
>> +
>> +		fl->real_owner = pid->virt;
>> +		fl->owners_fd = fd;
>> +
>> +		pr_info("Found lock entry %d.%d %d vs %d\n",
>> +				pid->real, pid->virt, fd,
>> +				fl->fl_owner);
>> +	}
> .
>