From: "Benjamin Coddington" <bcodding@redhat.com>
To: "Konstantin Khorenko" <khorenko@virtuozzo.com>,
"Jeff Layton" <jlayton@kernel.org>
Cc: "Kirill Gorkunov" <kgorkunov@virtuozzo.com>,
"Andrey Vagin" <avagin@virtuozzo.com>,
"J. Bruce Fields" <bfields@fieldses.org>,
"Alexander Viro" <viro@zeniv.linux.org.uk>,
"Vasily Averin" <vvs@virtuozzo.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes
Date: Thu, 14 Jun 2018 07:13:22 -0400 [thread overview]
Message-ID: <98D5E5F1-65D6-44C1-9E7C-C5A9043BC29F@redhat.com> (raw)
In-Reply-To: <20180608142712.32460-1-khorenko@virtuozzo.com>
On 8 Jun 2018, at 10:27, Konstantin Khorenko wrote:
> The behavior has been changed after 9d5b86ac13c5 ("fs/locks: Remove
> fl_nspid
> and use fs-specific l_pid for remote locks")
> and now /proc/$PID/fdinfo/$FD does not show the info about the lock
> * if the flock owner process is dead and its pid has been already
> freed
> or
> * if the lock owner is not visible in current pidns.
>
> CRIU uses this interface to store locks info during dump and thus can
> break
> on v4.13 and newer.
>
> So let's show info about locks anyway in described cases (like it was
> before
> 9d5b86ac13c5), but show pid number saved in file_lock struct if we are
> in
> init_pid_ns (patch 1) or just zero otherwise (patch 2) like we do with
> SID.
>
> Reproducer:
> process A process A1 process A2
> fork()--------->
> exit() open()
> flock()
> fork()--------->
> exit() sleep()
>
> Before the patch:
> ================
> (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
> pos: 4
> flags: 02100002
> mnt_id: 257
> lock: (root@vz7)/:
>
> After the patch:
> ===============
> (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
> pos: 4
> flags: 02100002
> mnt_id: 295
> lock: 1: FLOCK ADVISORY WRITE ${PID_A1} b6:f8a61:529946 0 EOF
>
> ===============
> # cat flock1.c
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/file.h>
> #include <fcntl.h>
>
> int main(void)
> {
> int fd;
> int err;
> pid_t child_pid;
>
> child_pid = fork();
> if (child_pid == -1)
> perror("fork failed");
> if (child_pid) {
> exit(0);
> }
>
> fd = open("/tmp/a", O_CREAT | O_RDWR);
> if (fd == -1)
> perror("Failed to open the file");
>
> err = flock(fd, LOCK_EX);
> if (err == -1)
> perror("flock failed");
>
> child_pid = fork();
> if (child_pid == -1)
> perror("fork failed");
> if (child_pid)
> exit(0);
>
> sleep(10000);
>
> return 0;
> }
>
> Konstantin Khorenko (2):
> fs/lock: skip lock owner pid translation in case we are in
> init_pid_ns
> fs/lock: show locks taken by processes from another pidns
These look good to me too. It makes sense that we treat pid numbers out
of
our namespace in the same way we'd treat remote pids, by returning 0
instead
of skipping their display altogether. You can add my
Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
to these two.
Ben
next prev parent reply other threads:[~2018-06-14 11:13 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-08 14:27 [PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes Konstantin Khorenko
2018-06-08 14:27 ` [PATCH 1/2] fs/lock: skip lock owner pid translation in case we are in init_pid_ns Konstantin Khorenko
2018-06-12 4:35 ` Andrey Vagin
2018-06-08 14:27 ` [PATCH 2/2] fs/lock: show locks taken by processes from another pidns Konstantin Khorenko
2018-06-12 4:53 ` Andrey Vagin
2018-06-14 11:00 ` Jeff Layton
2018-06-19 20:25 ` Andrey Vagin
2018-06-08 16:02 ` [PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes Cyrill Gorcunov
2018-06-14 10:41 ` Jeff Layton
2018-06-14 11:13 ` Benjamin Coddington [this message]
2018-08-09 7:16 ` Murphy Zhou
2018-08-09 8:20 ` Konstantin Khorenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=98D5E5F1-65D6-44C1-9E7C-C5A9043BC29F@redhat.com \
--to=bcodding@redhat.com \
--cc=avagin@virtuozzo.com \
--cc=bfields@fieldses.org \
--cc=jlayton@kernel.org \
--cc=kgorkunov@virtuozzo.com \
--cc=khorenko@virtuozzo.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
--cc=vvs@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).