Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1598521

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

From Al Viro <viro@ZenIV.linux.org.uk>
Newsgroups linux.kernel
Subject Re: [PATCH v2] statx: optimize copy of struct statx to userspace
Date 2017-03-12 02:30 +0100
Message-ID <tk0uB-2J5-3@gated-at.bofh.it> (permalink)
References <tjX3H-fX-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sat, Mar 11, 2017 at 01:45:55PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> I found that statx() was significantly slower than stat().  As a
> microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
> file to the same with statx() passed a NULL path:

Umm...

> +	struct statx tmp;
> +
> +	tmp.stx_mask = stat->result_mask;
> +	tmp.stx_blksize = stat->blksize;
> +	tmp.stx_attributes = stat->attributes;
> +	tmp.stx_nlink = stat->nlink;
> +	tmp.stx_uid = from_kuid_munged(current_user_ns(), stat->uid);
> +	tmp.stx_gid = from_kgid_munged(current_user_ns(), stat->gid);
> +	tmp.stx_mode = stat->mode;
> +	memset(tmp.__spare0, 0, sizeof(tmp.__spare0));
> +	tmp.stx_ino = stat->ino;
> +	tmp.stx_size = stat->size;
> +	tmp.stx_blocks = stat->blocks;
> +	memset(tmp.__spare1, 0, sizeof(tmp.__spare1));
> +	init_statx_timestamp(&tmp.stx_atime, &stat->atime);
> +	init_statx_timestamp(&tmp.stx_btime, &stat->btime);
> +	init_statx_timestamp(&tmp.stx_ctime, &stat->ctime);
> +	init_statx_timestamp(&tmp.stx_mtime, &stat->mtime);
> +	tmp.stx_rdev_major = MAJOR(stat->rdev);
> +	tmp.stx_rdev_minor = MINOR(stat->rdev);
> +	tmp.stx_dev_major = MAJOR(stat->dev);
> +	tmp.stx_dev_minor = MINOR(stat->dev);
> +	memset(tmp.__spare2, 0, sizeof(tmp.__spare2));
> +
> +	return copy_to_user(buffer, &tmp, sizeof(tmp)) ? -EFAULT : 0;

That relies upon there being no padding in the damn structure.
It's true and probably will be true on any target, but
	a) it's bloody well worth stating explicitly
and
	b) struct statx tmp = {.stx_mask = stat->result_mask};
will get rid of those memset() you've got there by implicit
zeroing of fields missing in partial structure initializer.
Padding is *not* included into that, but you are relying upon
having no padding anyway...

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2] statx: optimize copy of struct statx to userspace Eric Biggers <ebiggers3@gmail.com> - 2017-03-11 22:50 +0100
  Re: [PATCH v2] statx: optimize copy of struct statx to userspace Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-12 02:30 +0100
    Re: [PATCH v2] statx: optimize copy of struct statx to userspace Eric Biggers <ebiggers3@gmail.com> - 2017-03-12 03:20 +0100
      Re: [PATCH v2] statx: optimize copy of struct statx to userspace Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-12 03:40 +0100
        Re: [PATCH v2] statx: optimize copy of struct statx to userspace Eric Biggers <ebiggers3@gmail.com> - 2017-03-12 05:10 +0100
          Re: [PATCH v2] statx: optimize copy of struct statx to userspace Eric Biggers <ebiggers3@gmail.com> - 2017-03-12 07:10 +0100
            Re: [PATCH v2] statx: optimize copy of struct statx to userspace Andreas Dilger <adilger@dilger.ca> - 2017-03-13 05:40 +0100
              Re: [PATCH v2] statx: optimize copy of struct statx to userspace Florian Weimer <fweimer@redhat.com> - 2017-03-13 11:30 +0100
                Re: [PATCH v2] statx: optimize copy of struct statx to userspace Eric Biggers <ebiggers3@gmail.com> - 2017-03-13 19:20 +0100

csiph-web