On Wed, Sep 03, 2014 at 06:38:24PM +0400, Pavel Emelyanov wrote: > On 09/02/2014 11:53 PM, Jeff Layton wrote: > > On Tue, 2 Sep 2014 15:43:00 -0400 > > "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote: > > > >> On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote: > >>> On 09/02/2014 10:44 PM, J. Bruce Fields wrote: > >>>> On Tue, Sep 02, 2014 at 09:17:34PM +0400, Pavel Emelyanov wrote: > >>>>> Hi, > >>>>> > >>>>> There's a problem with getting information about who has a flock on > >>>>> a specific file. The thing is that the "owner" field, that is shown in > >>>>> /proc/locks is the pid of the task who created the flock, not the one > >>>>> who _may_ hold it. > >>>>> > >>>>> If the flock creator shared the file with some other task (by forking > >>>>> or via scm_rights) and then died or closed the file, the information > >>>>> shown in proc no longer corresponds to the reality. > >>>>> > >>>>> This is critical for CRIU project, that tries to dump (and restore) > >>>>> the state of running tasks. For example, let's take two tasks A and B > >>>>> both opened a file "/foo", one of tasks places a LOCK_SH lock on the > >>>>> file and then "obfuscated" the owner field in /proc/locks. After this > >>>>> we have no ways to find out who is the lock holder. > >>>>> > >>>>> I'd like to note, that for LOCK_EX this problem is not critical -- we > >>>>> may go to both tasks and "ask" them to LOCK_EX the file again (we can > >>>>> do it in CRIU, I can tell more if required). The one who succeeds is > >>>>> the lock holder. > >>>> > >>>> It could be both, actually, right? > >>> > >>> Two succeeding with LOCK_EX? AFAIU no. Am I missing something? > >> > >> After a fork, two processes "own" the lock, right?: > >> > >> int main(int argc, char *argv[]) > >> { > >> int fd, ret; > >> > >> fd = open(argv[1], O_RDWR); > >> ret = flock(fd, LOCK_EX); > >> if (ret) > >> err(1, "flock"); > >> ret = fork(); > >> if (ret == -1) > >> err(1, "flock"); > >> ret = flock(fd, LOCK_EX); > >> if (ret) > >> err(1, "flock"); > >> printf("%d got exclusive lock\n", getpid()); > >> sleep(1000); > >> } > >> > >> $ touch TMP > >> $ ./test TMP > >> 15882 got exclusive lock > >> 15883 got exclusive lock > >> ^C > >> > >> I may misunderstand what you're doing. > >> > > > > Yeah, I don't understand either. > > > > Flock locks are owned by the file description. The task that set > > them is really irrelevant once they are set. > > > > In the second flock() call there, you're just "modifying" an existing > > lock (which turns out to be a noop here). > > > > So, I don't quite understand the problem this solves. I get that you're > > trying to reestablish the flock "state" after a checkpoint/restore > > event, but why does it matter what task actually sets the locks as long > > as they're set on the correct set of fds? > > Sorry for confusion. Let me try to explain it more clearly. > > First, what I meant talking about two LOCK_EX locks. Let's consider > this scenario: > > pid = fork() > fd = open("/foo"); /* both parent and child has _different_ files */ > if (pid == 0) > /* child only */ > flock(fd, LOCK_EX); > > at this point we have two different files pointing to "/foo" and > only one of them has LOCK_EX on it. So if try to LOCK_EX it again, > only at child's file this would succeed. So we can distinguish which > file is locked using this method. > > > > Now, what problem this patch is trying to solve. It's quite tricky, > but still. Let's imagine this scenario: > > pid = fork(); > fd = open("/foo"); /* yet again -- two different files */ > if (pid == 0) { > flock(fd, LOCK_SH); > pid2 = fork(); > if (pid2 != 0) > exit(0); > } > > at this point we have: > > task A -- the original task with file "/foo" opened > task B -- the first child, that exited at the end > task C -- the 2nd child, that "inherited" a file with the lock from B > > Note, that file at A and file at C are two different files (struct > file-s). And it's only the C's one that is locked. > > The problem is that the /proc/locks shows the pid of B in this lock's > owner field. And we have no glue to find out who the real lock owner > is using the /proc/locks. > > If we try to do the trickery like the one we did with LOCK_EX above, > this is what we would get. > > If putting the 2nd LOCK_SH from A and from C, both attempts would succeed, > so this is not the solution. > > If we try to LOCK_EX from A and C, only C would succeed, so this seem > to be the solution, but it's actually not. If there's another pair of > A' and C' tasks holding the same "/foo" and having the LOCK_SH on C', > this trick would stop working as none of the tasks would be able to > put such lock on this file. > > > Thus, we need some way to find out whether a task X has a lock on file F. > This patch is one of the ways of doing this. > > Hope this explanation is more clear. Thanks, I think I understand. Remind me how you figure out which file descriptors point to the same file description (struct file)? --b. -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html