[RFC v2] Decode UFFDIO_* ioctls

[Dr. David Alan Gilbert]

* Dmitry V. Levin (ldv at altlinux.org) wrote:
> On Tue, May 03, 2016 at 08:23:43PM +0100, Dr. David Alan Gilbert wrote:
> [...]
> > > > --- /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?
> 
> We have just a few users of this feature, and these users seem to be OK
> with the current behaviour, which means we probably need another syntax
> for this case.

Hmm this is confusing; I see icmp flags are one of the options;
so strace ping:

setsockopt(3, SOL_RAW, ICMP_FILTER, ~(ICMP_ECHOREPLY|ICMP_DEST_UNREACH|ICMP_SOURCE_QUENCH|ICMP_REDIRECT|ICMP_TIME_EXCEEDED|ICMP_PARAMETERPROB), 4) = 0

if we look at the source for ping:
        struct icmp_filter filt;
        filt.data = ~((1<<ICMP_SOURCE_QUENCH)|
                      (1<<ICMP_DEST_UNREACH)|
                      (1<<ICMP_TIME_EXCEEDED)|
                      (1<<ICMP_PARAMETERPROB)|
                      (1<<ICMP_REDIRECT)|
                      (1<<ICMP_ECHOREPLY));
        if (setsockopt(icmp_sock, SOL_RAW, ICMP_FILTER, (char*)&filt, sizeof(filt)) == -1)

so I reckon the current decode is wrong; however, it is more readable
and if we prefer readable I'm happy to stick with it.

> I'd also consider another representation of "ioctls" field, e.g.
> ioctls=1<<(_UFFDIO_WAKE|_UFFDIO_COPY|_UFFDIO_ZEROPAGE)

Hmm, that would be quite a different result.

> > 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?
> 
> One way is to decode flags in the test, I've recently added printflags
> and printxval to libtests exactly for this purpose.

Ah, excellent.

> Another way is to output a regexp for egrep match instead of expected
> output for exact diff match.

Yep, I'll have a look at using either/or to write that test.

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