Hi, Axel, Looks mostly good to me, but a few nitpickings below. On Wed, Dec 02, 2020 at 01:15:42PM -0800, Axel Rasmussen wrote: [...] > +static void uffd_error(const char *message, __s64 code) > +{ > + fprintf(stderr, "%s: %" PRId64 "\n", message, (int64_t)code); > + exit(1); > +} IMHO a macro that can take arbitrary parameters would be nicer, but if it satisfy our need, definitely ok too. [...] > @@ -340,7 +348,8 @@ static void wp_range(int ufd, __u64 start, __u64 len, bool wp) > prms.mode = wp ? UFFDIO_WRITEPROTECT_MODE_WP : 0; > > if (ioctl(ufd, UFFDIO_WRITEPROTECT, &prms)) { > - fprintf(stderr, "clear WP failed for address 0x%Lx\n", start); > + fprintf(stderr, "clear WP failed for address 0x%" PRIx64 "\n", > + (uint64_t)start); > exit(1); Is it intended to not use uffd_error() here? > } > } [...] > @@ -979,26 +981,20 @@ static int __uffdio_zeropage(int ufd, unsigned long offset, bool retry) > if (ret) { > /* real retval in ufdio_zeropage.zeropage */ > if (has_zeropage) { > - if (uffdio_zeropage.zeropage == -EEXIST) { > - fprintf(stderr, "UFFDIO_ZEROPAGE -EEXIST\n"); > - exit(1); > - } else { > - fprintf(stderr, "UFFDIO_ZEROPAGE error %Ld\n", > - uffdio_zeropage.zeropage); > - exit(1); > - } > + uffd_error(uffdio_zeropage.zeropage == -EEXIST ? > + "UFFDIO_ZEROPAGE -EEXIST" : > + "UFFDIO_ZEROPAGE error", Nit: The indents here are a bit odd.. Thanks, -- Peter Xu