[RFC v2] Decode UFFDIO_* ioctls

[Dr. David Alan Gilbert]

* Dmitry V. Levin (ldv at altlinux.org) wrote:
> On Fri, Apr 29, 2016 at 08:32:47PM +0100, Dr. David Alan Gilbert wrote:
> > * Dmitry V. Levin (ldv at altlinux.org) wrote:
> > > On Sat, Apr 23, 2016 at 05:39:42AM +0300, Dmitry V. Levin wrote:
> > > > On Fri, Apr 22, 2016 at 07:02:08PM +0100, Dr. David Alan Gilbert (git) wrote:
> > > [...]
> > > > >    don't use my decode_flags64 - I guess I should be using printflags
> > > > >        but printflags uses struct xlat and they're both unsigned int
> > > > >        where I'm dealing in uint64_t's; perhaps if I just rework
> > > > >        printflags/xlat it'll all work ???
> > > > 
> > > > With regards to printing 64-bit flags, see the thread starting at
> > > > https://sourceforge.net/p/strace/mailman/message/34979546/
> > > 
> > > The change of struct xlat is in, you can now generate xlat files filled
> > > with 64-bit values by using "#val_type uint64_t" directive, and print
> > > 64-bit values using e.g. printxval64() and printflags64().
> > 
> > Thanks;  it's not 100% happy yet; I was trying to use the 1<< notation
> > and ended up with this minor fix:
> > 
> > diff --git a/xlat.h b/xlat.h
> > index 64141b3..2f7643c 100644
> > --- a/xlat.h
> > +++ b/xlat.h
> > @@ -10,7 +10,7 @@ struct xlat {
> >  # define XLAT(val)                     { (unsigned)(val), #val }
> >  # define XLAT_PAIR(val, str)           { (unsigned)(val), str  }
> >  # define XLAT_TYPE(type, val)          {     (type)(val), #val }
> > -# define XLAT_TYPE_PAIR(val, str)      {     (type)(val), str  }
> > +# define XLAT_TYPE_PAIR(type, val, str)        {     (type)(val), str  }
> >  # define XLAT_END                      {               0, 0    }
> 
> Oops.  Fixed, thanks.

Thanks.

> > however, that's not enough, because the '1' is the wrong type,
> > we really need ((type)1) << thing
> > and I'm not too sure what the best way of fixing that in gen.sh is;
> > you can see it getting that wrong in UFFDIO_API:
> > 
> > ioctl(4, UFFDIO_API, {api=0xaa, features.in=0, features.out=0, ioctls=_UFFDIO_REGISTER|_UFFDIO_UNREGISTER|0x8000000000000000}) = 0
> 
> I've changed gen.sh to generate 1ULL<<val,
> it will hopefully work in all cases.

Yep, that's better;

ioctl(4, UFFDIO_API, {api=0xaa, features.in=0, features.out=0, ioctls=_UFFDIO_REGISTER|_UFFDIO_UNREGISTER|_UFFDIO_API}) = 0

> 
> > +			tprintf(", {dst=%#" PRI__x64 ", src=%#" PRI__x64
> > +				", len=%" PRI__u64, ", mode=",
> > +				uc.dst, uc.src, uc.len);
> 
> The comma after PRI__u64 makes gcc -Werror very unhappy,
> and rightly so.  Try to configure build with --enable-gcc-Werror.

Yeh, it's got a good point - fixed, and enabled.

> 
> > --- /dev/null
> > +++ b/xlat/uffd_register_ioctl_flags.in
> > @@ -0,0 +1,5 @@
> > +#val_type uint64_t
> > +1<< _UFFDIO_WAKE
> > +1<< _UFFDIO_COPY
> > +1<< _UFFDIO_ZEROPAGE
> > +
> 
> gen.sh doesn't handle space characters after 1<< properly (yet).
> git am complains about empty line at EOF.

Fixed. Hmm, I've removed the spaces, but the output I'm getting is:
[pid  9644] ioctl(20, UFFDIO_REGISTER{range={start=0x7ff674200000, len=0x40000}, mode=UFFDIO_REGISTER_MODE_MISSING, ioctls=_UFFDIO_WAKE|_UFFDIO_COPY|_UFFDIO_ZEROPAGE}) = 0

shouldn't I be seeing 1ull<<_UFFDIO_WAKE | 1ull << _UFFDIO_COPY etc?
The .h gen.sh has created has:
#if defined(_UFFDIO_WAKE) || (defined(HAVE_DECL__UFFDIO_WAKE) && HAVE_DECL__UFFDIO_WAKE)
  XLAT_TYPE_PAIR(uint64_t, 1ULL<<_UFFDIO_WAKE, "_UFFDIO_WAKE"),
#endif
#if defined(_UFFDIO_COPY) || (defined(HAVE_DECL__UFFDIO_COPY) && HAVE_DECL__UFFDIO_COPY)
  XLAT_TYPE_PAIR(uint64_t, 1ULL<<_UFFDIO_COPY, "_UFFDIO_COPY"),
#endif
#if defined(_UFFDIO_ZEROPAGE) || (defined(HAVE_DECL__UFFDIO_ZEROPAGE) && HAVE_DECL__UFFDIO_ZEROPAGE)
  XLAT_TYPE_PAIR(uint64_t, 1ULL<<_UFFDIO_ZEROPAGE, "_UFFDIO_ZEROPAGE"),
#endif


> > --- /dev/null
> > +++ b/xlat/uffd_zeropage_flags.in
> > @@ -0,0 +1,3 @@
> > +#val_type uint64_t
> > +UFFDIO_ZEROPAGE_MODE_DONTWAKE
> > +
> 
> git am complains about this as well.

Fixed.

Thanks.

So the thing that I'm left with is the tests;  the cases I'm most worried about
are how to handle the return data from the kernel in the case where the flags
the kernel returns varies on kernel versions.
Is the right thing for the test to do the decode manually and then
print what it expects strace to print?

Dave

> 
> 
> -- 
> ldv



> ------------------------------------------------------------------------------
> Find and fix application performance issues faster with Applications Manager
> Applications Manager provides deep performance insights into multiple tiers of
> your business applications. It resolves application problems quickly and
> reduces your MTTR. Get your free trial!
> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z

> _______________________________________________
> Strace-devel mailing list
> Strace-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel

--
Dr. David Alan Gilbert / dgilbert at redhat.com / Manchester, UK