[PATCH for 4.11] userfaultfd: report actual registered features in fdinfo

[PATCH for 4.11] userfaultfd: report actual registered features in fdinfo

[Mike Rapoport]

fdinfo for userfault file descriptor reports UFFD_API_FEATURES. Up until
recently, the UFFD_API_FEATURES was defined as 0, therefore corresponding
field in fdinfo always contained zero. Now, with introduction of several
additional features, UFFD_API_FEATURES is not longer 0 and it seems better
to report actual features requested for the userfaultfd object described by
the fdinfo. First, the applications that were using userfault will still
see zero at the features field in fdinfo. Next, reporting actual features
rather than available features, gives clear indication of what userfault
features are used by an application.

Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
---
 fs/userfaultfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 1d227b0..f7555fc 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1756,7 +1756,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
 	 *	protocols: aa:... bb:...
 	 */
 	seq_printf(m, "pending:\t%lu\ntotal:\t%lu\nAPI:\t%Lx:%x:%Lx\n",
-		   pending, total, UFFD_API, UFFD_API_FEATURES,
+		   pending, total, UFFD_API, ctx->features,
 		   UFFD_API_IOCTLS|UFFD_API_RANGE_IOCTLS);
 }
 #endif
-- 
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH for 4.11] userfaultfd: report actual registered features in fdinfo

[Andrea Arcangeli]

On Sun, Apr 02, 2017 at 04:36:21PM +0300, Mike Rapoport wrote:
> fdinfo for userfault file descriptor reports UFFD_API_FEATURES. Up until
> recently, the UFFD_API_FEATURES was defined as 0, therefore corresponding
> field in fdinfo always contained zero. Now, with introduction of several
> additional features, UFFD_API_FEATURES is not longer 0 and it seems better
> to report actual features requested for the userfaultfd object described by
> the fdinfo. First, the applications that were using userfault will still
> see zero at the features field in fdinfo. Next, reporting actual features
> rather than available features, gives clear indication of what userfault
> features are used by an application.
> 
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
>  fs/userfaultfd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 1d227b0..f7555fc 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1756,7 +1756,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
>  	 *	protocols: aa:... bb:...
>  	 */
>  	seq_printf(m, "pending:\t%lu\ntotal:\t%lu\nAPI:\t%Lx:%x:%Lx\n",
> -		   pending, total, UFFD_API, UFFD_API_FEATURES,
> +		   pending, total, UFFD_API, ctx->features,
>  		   UFFD_API_IOCTLS|UFFD_API_RANGE_IOCTLS);
>  }
>  #endif

Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>

I wonder if we've been a bit overkill in showing these details in
/proc as this innocent change is technically an ABI visible change
now. It's intended only for informational/debug purposes, no software
should attempt to decode it, so it'd be better in debugfs, but the
per-thread fds aren't anywhere in debugfs so it's shown there where
it's all already in place to provide it with a few liner function.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH for 4.11] userfaultfd: report actual registered features in fdinfo

[Mike Rapoport]

On Mon, Apr 03, 2017 at 04:35:23PM +0200, Andrea Arcangeli wrote:
> On Sun, Apr 02, 2017 at 04:36:21PM +0300, Mike Rapoport wrote:
> > fdinfo for userfault file descriptor reports UFFD_API_FEATURES. Up until
> > recently, the UFFD_API_FEATURES was defined as 0, therefore corresponding
> > field in fdinfo always contained zero. Now, with introduction of several
> > additional features, UFFD_API_FEATURES is not longer 0 and it seems better
> > to report actual features requested for the userfaultfd object described by
> > the fdinfo. First, the applications that were using userfault will still
> > see zero at the features field in fdinfo. Next, reporting actual features
> > rather than available features, gives clear indication of what userfault
> > features are used by an application.
> > 
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > ---
> >  fs/userfaultfd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> > index 1d227b0..f7555fc 100644
> > --- a/fs/userfaultfd.c
> > +++ b/fs/userfaultfd.c
> > @@ -1756,7 +1756,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
> >  	 *	protocols: aa:... bb:...
> >  	 */
> >  	seq_printf(m, "pending:\t%lu\ntotal:\t%lu\nAPI:\t%Lx:%x:%Lx\n",
> > -		   pending, total, UFFD_API, UFFD_API_FEATURES,
> > +		   pending, total, UFFD_API, ctx->features,
> >  		   UFFD_API_IOCTLS|UFFD_API_RANGE_IOCTLS);
> >  }
> >  #endif
> 
> Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
> 
> I wonder if we've been a bit overkill in showing these details in
> /proc as this innocent change is technically an ABI visible change
> now. It's intended only for informational/debug purposes, no software
> should attempt to decode it, so it'd be better in debugfs, but the
> per-thread fds aren't anywhere in debugfs so it's shown there where
> it's all already in place to provide it with a few liner function.
> 

Actually, I've found these details in /proc useful when I was experimenting
with checkpoint-restore of an application that uses userfaultfd. With
interface in /proc/<pid>/ we know exactly which process use userfaultfd and
can act appropriately.

--
Sincerely yours,
Mike.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH for 4.11] userfaultfd: report actual registered features in fdinfo

[Andrea Arcangeli]

Hello Mike,

On Mon, Apr 03, 2017 at 06:10:24PM +0300, Mike Rapoport wrote:
> Actually, I've found these details in /proc useful when I was experimenting
> with checkpoint-restore of an application that uses userfaultfd. With
> interface in /proc/<pid>/ we know exactly which process use userfaultfd and
> can act appropriately.

You've to be somewhat serialized by other means though, because
"exactly" has a limit with fdinfo. For example by the time read()
returns, the uffd may have been closed already by the app (just the
uffd isn't ->release()d yet as the last fput has yet to run, the
fdinfo runs the last fput in such case). As long as you can cope with
this and you've a stable fdinfo it's ok.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH for 4.11] userfaultfd: report actual registered features in fdinfo

[Mike Rapoport]

Hello Andrea,

On Mon, Apr 03, 2017 at 06:30:34PM +0200, Andrea Arcangeli wrote:
> Hello Mike,
> 
> On Mon, Apr 03, 2017 at 06:10:24PM +0300, Mike Rapoport wrote:
> > Actually, I've found these details in /proc useful when I was experimenting
> > with checkpoint-restore of an application that uses userfaultfd. With
> > interface in /proc/<pid>/ we know exactly which process use userfaultfd and
> > can act appropriately.
> 
> You've to be somewhat serialized by other means though, because
> "exactly" has a limit with fdinfo. For example by the time read()
> returns, the uffd may have been closed already by the app (just the
> uffd isn't ->release()d yet as the last fput has yet to run, the
> fdinfo runs the last fput in such case). As long as you can cope with
> this and you've a stable fdinfo it's ok.
> 

Well, by the time CRIU checkpoints open file descriptors, the process is
already stopped, hence we are not racing with anything here.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>