Re: [PATCH v4 0/3] mm, treewide: Rename kzfree() to kfree_sensitive()

[David Sterba]

On Tue, Jun 16, 2020 at 11:53:50AM -0700, Joe Perches wrote:
> On Mon, 2020-06-15 at 21:57 -0400, Waiman Long wrote:
> >  v4:
> >   - Break out the memzero_explicit() change as suggested by Dan Carpenter
> > so that it can be backported to stable.
> >   - Drop the "crypto: Remove unnecessary memzero_explicit()" patch for
> > now as there can be a bit more discussion on what is best. It will be
> > introduced as a separate patch later on after this one is merged.
> 
> To this larger audience and last week without reply:
> https://lore.kernel.org/lkml/573b3fbd5927c643920e1364230c296b23e7584d.ca...@perches.com/
> 
> Are there _any_ fastpath uses of kfree or vfree?

I'd consider kfree performance critical for cases where it is called
under locks. If possible the kfree is moved outside of the critical
section, but we have rbtrees or lists that get deleted under locks and
restructuring the code to do eg. splice and free it outside of the lock
is not always possible.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 3/3] btrfs: Use kfree() in btrfs_ioctl_get_subvol_info()

[David Sterba]

On Mon, Jun 15, 2020 at 09:57:18PM -0400, Waiman Long wrote:
> In btrfs_ioctl_get_subvol_info(), there is a classic case where kzalloc()
> was incorrectly paired with kzfree(). According to David Sterba, there
> isn't any sensitive information in the subvol_info that needs to be
> cleared before freeing. So kfree_sensitive() isn't really needed,
> use kfree() instead.
> 
> Reported-by: David Sterba 
> Signed-off-by: Waiman Long 
> ---
>  fs/btrfs/ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index f1dd9e4271e9..e8f7c5f00894 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2692,7 +2692,7 @@ static int btrfs_ioctl_get_subvol_info(struct file 
> *file, void __user *argp)
>   btrfs_put_root(root);
>  out_free:
>   btrfs_free_path(path);
> - kfree_sensitive(subvol_info);
> + kfree(subvol_info);

I would rather merge a patch doing to kzfree -> kfree instead of doing
the middle step to switch it to kfree_sensitive. If it would help
integration of your patchset I can push it to the next rc so there are
no kzfree left in the btrfs code. Treewide change like that can take
time so it would be one less problem to care about for you.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] mm, treewide: Rename kzfree() to kfree_sensitive()

[David Sterba]

On Mon, Apr 13, 2020 at 05:15:49PM -0400, Waiman Long wrote:
>  fs/btrfs/ioctl.c  |  2 +-


> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 40b729dce91c..eab3f8510426 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2691,7 +2691,7 @@ static int btrfs_ioctl_get_subvol_info(struct file 
> *file, void __user *argp)
>   btrfs_put_root(root);
>  out_free:
>   btrfs_free_path(path);
> - kzfree(subvol_info);
> + kfree_sensitive(subvol_info);

This is not in a sensitive context so please switch it to plain kfree.
With that you have my acked-by. Thanks.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC] errno.h: Provide EFSCORRUPTED for everybody

[David Sterba]

On Sat, Nov 02, 2019 at 08:38:23AM +1100, Dave Chinner wrote:
> On Fri, Nov 01, 2019 at 09:57:31PM +0100, Geert Uytterhoeven wrote:
> > Hi Valdis,
> > 
> > On Thu, Oct 31, 2019 at 2:11 AM Valdis Kletnieks
> >  wrote:
> > > Three questions: (a) ACK/NAK on this patch, (b) should it be all in one
> > > patch, or one to add to errno.h and 6 patches for 6 filesystems?), and
> > > (c) if one patch, who gets to shepherd it through?
> > >
> > > There's currently 6 filesystems that have the same #define. Move it
> > > into errno.h so it's defined in just one place.
> > >
> > > Signed-off-by: Valdis Kletnieks 
> > 
> > Thanks for your patch!
> > 
> > > --- a/include/uapi/asm-generic/errno.h
> > > +++ b/include/uapi/asm-generic/errno.h
> > > @@ -98,6 +98,7 @@
> > >  #defineEINPROGRESS 115 /* Operation now in progress */
> > >  #defineESTALE  116 /* Stale file handle */
> > >  #defineEUCLEAN 117 /* Structure needs cleaning */
> > > +#defineEFSCORRUPTEDEUCLEAN
> > 
> > I have two questions:
> > a) Why not use EUCLEAN everywhere instead?
> > Having two different names for the same errno complicates grepping.
> 
> Because:
>   a) EUCLEAN is horrible for code documentation. EFSCORRUPTED
>   describes exactly the error being returned and/or checked for.
> 
>   b) we've used EFSCORRUPTED in XFS since 1993. i.e. it was an
>   official, published error value on Irix, and we've kept it
>   in the linux code for the past ~20 years because of a)
> 
>   c) Userspace programs that include filesystem specific
>   headers have already been exposed to and use EFSCORRUPTED,
>   so we can't remove/change it without breaking userspace.
> 
>   d) EUCLEAN has a convenient userspace string description
>   that is appropriate for filesystem corruption: "Structure
>   needs cleaning" is precisely what needs to happen. Repair of
>   the filesystem (i.e. recovery to a clean state) is what is
>   required to fix the error

The description is very confusing to users that are also not filesystem
developers. "Structure needs cleaning" says what needs to be done but
not what happened. Unlike other error codes like "not enough memory",
"IO error" etc. We don't have EBUYMEM / "Buy more memory" instead of
ENOMEM.

Fuzzing tests and crafted images produce most of the EUCLEAN errors and
in this context "structure needs cleaning" makes even less sense.

> > b) Perhaps both errors should use different values?
> 
> That horse bolted to userspace years ago - this is just formalising
> the practice that has spread across multiple linux filesystems from
> XFS over the past ~10 years..

EFSCORRUPTED is a appropriate name but to share the number with EUCLEAN
was a terrible decision and now every filesystem has to suffer and
explain to users what the code really means and point to the the sad
story when asked "So why don't you have a separate code?".

I wonder what userspace package really depends on the value, that would
eg. change code flow. Uncommon error values usually lead to a message
and exit.

Debian code search shows only jython, e2fsprogs, xfsprogs, python2.7,
pypy, linux, partclone using EFSCORRUPTED. So 2 of them are filesystem
packages that can handle that, python/jython only defines the value for
IRIX platform. The rest is linux kernel, not relevant.

So please give me one example where userspace will break.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v8 11/24] erofs: introduce xattr & posixacl support

[David Sterba]

On Mon, Sep 02, 2019 at 09:51:59PM +0800, Chao Yu wrote:
> On 2019-9-2 21:06, David Sterba wrote:
> > On Mon, Sep 02, 2019 at 05:57:11AM -0700, Christoph Hellwig wrote:
> >>> +config EROFS_FS_XATTR
> >>> + bool "EROFS extended attributes"
> >>> + depends on EROFS_FS
> >>> + default y
> >>> + help
> >>> +   Extended attributes are name:value pairs associated with inodes by
> >>> +   the kernel or by users (see the attr(5) manual page, or visit
> >>> +   <http://acl.bestbits.at/> for details).
> >>> +
> >>> +   If unsure, say N.
> >>> +
> >>> +config EROFS_FS_POSIX_ACL
> >>> + bool "EROFS Access Control Lists"
> >>> + depends on EROFS_FS_XATTR
> >>> + select FS_POSIX_ACL
> >>> + default y
> >>
> >> Is there any good reason to make these optional these days?
> > 
> > I objected against adding so many config options, not to say for the
> > standard features. The various cache strategies or other implementation
> > details have been removed but I agree that making xattr/acl configurable
> > is not necessary as well.
> 
> I can see similar *_ACL option in btrfs/ext4/xfs, should we remove them as 
> well
> due to the same reason?

Oh right, I think the reasons are historical and that we can remove the
options nowadays. From the compatibility POV this should be safe, with
ACLs compiled out, no tool would use them, and no harm done when the
code is present but not used.

There were some efforts by embedded guys to make parts of kernel more
configurable to allow removing subsystems to reduce the final image
size. In this case I don't think it would make any noticeable
difference, eg. the size of fs/btrfs/acl.o on release config is 1.6KiB,
while the whole module is over 1.3MiB.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 01/24] erofs: add on-disk layout

[David Sterba]

On Mon, Sep 02, 2019 at 10:43:03AM +0200, Pavel Machek wrote:
> > > > Rather than they didn't run "gdb" or "pahole" and change it by mistake.
> > > 
> > > I think Christoph is not right here.
> > > 
> > > Using external tools for validation is extra work
> > > when necessary for understanding the code.
> > 
> > The advantage of using the external tools that the information about
> > offsets is provably correct ...
> 
> No. gdb tells you what the actual offsets _are_.

Ok, reading your reply twice, I think we have different perspectives. I
don't trust the comments.

The tool I had in mind is pahole that parses dwarf information about the
structures, the same as gdb does. The actual value of the struct members
is the thing that needs to be investigated in memory dumps or disk image
dumps.

> > > The expected offset is somewhat valuable, but
> > > perhaps the form is a bit off given the visual
> > > run-in to the field types.
> > > 
> > > The extra work with this form is manipulating all
> > > the offsets whenever a structure change occurs.
> > 
> > ... while this is error prone.
> 
> While the comment tells you what they _should be_.

That's exactly the source of confusion and bugs. For me an acceptable
way of asserting that a value has certain offset is a build check, eg.
like

BUILD_BUG_ON(strct my_superblock, magic, 16);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 05/24] erofs: add inode operations

[David Sterba]

On Sun, Sep 01, 2019 at 05:34:00PM +0800, Gao Xiang wrote:
> > > +static int read_inode(struct inode *inode, void *data)
> > > +{
> > > + struct erofs_vnode *vi = EROFS_V(inode);
> > > + struct erofs_inode_v1 *v1 = data;
> > > + const unsigned int advise = le16_to_cpu(v1->i_advise);
> > > + erofs_blk_t nblks = 0;
> > > +
> > > + vi->datamode = __inode_data_mapping(advise);
> > 
> > What is the deal with these magic underscores here and various
> > other similar helpers?
> 
> Fixed in
> https://lore.kernel.org/linux-fsdevel/20190901055130.30572-17-hsiang...@aol.com/
> 
> underscores means 'internal' in my thought, it seems somewhat
> some common practice of Linux kernel, or some recent discussions
> about it?... I didn't notice these discussions...

I know about a few valid uses of the underscores:

* pattern where the __underscored version does not do locking, while the other
  does
* similarly for atomic and non-atomic version
* macro that needs to manipulate the argument name (like glue some
  prefix, so the macro does not have underscores and is supposed to be
  used instead of the function with underscores that needs the full name
  of a variable/constant/..
* underscore function takes a few more parameters to further tune the
  behaviour, but most users are fine with the defaults and that is
  provided as a function without underscores
* in case you have just one function of the kind, don't use the underscores

I can lookup examples if you're interested or if the brief description
is not sufficient. The list covers what I've seen and used, but the list
may be incomplete.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v8 11/24] erofs: introduce xattr & posixacl support

[David Sterba]

On Mon, Sep 02, 2019 at 05:57:11AM -0700, Christoph Hellwig wrote:
> > +config EROFS_FS_XATTR
> > +   bool "EROFS extended attributes"
> > +   depends on EROFS_FS
> > +   default y
> > +   help
> > + Extended attributes are name:value pairs associated with inodes by
> > + the kernel or by users (see the attr(5) manual page, or visit
> > +  for details).
> > +
> > + If unsure, say N.
> > +
> > +config EROFS_FS_POSIX_ACL
> > +   bool "EROFS Access Control Lists"
> > +   depends on EROFS_FS_XATTR
> > +   select FS_POSIX_ACL
> > +   default y
> 
> Is there any good reason to make these optional these days?

I objected against adding so many config options, not to say for the
standard features. The various cache strategies or other implementation
details have been removed but I agree that making xattr/acl configurable
is not necessary as well.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 01/24] erofs: add on-disk layout

[David Sterba]

On Thu, Aug 29, 2019 at 08:58:17AM -0700, Joe Perches wrote:
> On Thu, 2019-08-29 at 18:32 +0800, Gao Xiang wrote:
> > Hi Christoph,
> > 
> > On Thu, Aug 29, 2019 at 02:59:54AM -0700, Christoph Hellwig wrote:
> > > > --- /dev/null
> > > > +++ b/fs/erofs/erofs_fs.h
> > > > @@ -0,0 +1,316 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-only OR Apache-2.0 */
> > > > +/*
> > > > + * linux/fs/erofs/erofs_fs.h
> > > 
> > > Please remove the pointless file names in the comment headers.
> > 
> > Already removed in the latest version.
> > 
> > > > +struct erofs_super_block {
> > > > +/*  0 */__le32 magic;   /* in the little endian */
> > > > +/*  4 */__le32 checksum;/* crc32c(super_block) */
> > > > +/*  8 */__le32 features;/* (aka. feature_compat) */
> > > > +/* 12 */__u8 blkszbits; /* support block_size == PAGE_SIZE 
> > > > only */
> > > 
> > > Please remove all the byte offset comments.  That is something that can
> > > easily be checked with gdb or pahole.
> > 
> > I have no idea the actual issue here.
> > It will help all developpers better add fields or calculate
> > these offsets in their mind, and with care.
> > 
> > Rather than they didn't run "gdb" or "pahole" and change it by mistake.
> 
> I think Christoph is not right here.
> 
> Using external tools for validation is extra work
> when necessary for understanding the code.

The advantage of using the external tools that the information about
offsets is provably correct ...

> The expected offset is somewhat valuable, but
> perhaps the form is a bit off given the visual
> run-in to the field types.
> 
> The extra work with this form is manipulating all
> the offsets whenever a structure change occurs.

... while this is error prone.

> The comments might be better with a form more like:
> 
> struct erofs_super_block {/* offset description */
>   __le32 magic;   /*   0  */
>   __le32 checksum;/*   4  crc32c(super_block) */
>   __le32 features;/*   8  (aka. feature_compat) */
>   __u8 blkszbits; /*  12  support block_size == PAGE_SIZE only */
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: exfat: add exfat filesystem code to staging

[David Sterba]

On Fri, Aug 30, 2019 at 10:06:25AM +0800, Chao Yu wrote:
> On 2019/8/29 23:43, Dan Carpenter wrote:
> >> p.s. There are 2947 (un)likely places in fs/ directory.
> > 
> > I was complaining about you adding new pointless ones, not existing
> > ones.  The likely/unlikely annotations are supposed to be functional and
> > not decorative.  I explained this very clearly.
> > 
> > Probably most of the annotations in fs/ are wrong but they are also
> > harmless except for the slight messiness.  However there are definitely
> > some which are important so removing them all isn't a good idea.
> 
> Hi Dan,
> 
> Could you please pick up one positive example using likely and unlikely
> correctly? so we can follow the example, rather than removing them all 
> blindly.

Remove all of them and re-add with explanation if and how each is going
to make things better. If you can't reason about, prove by benchmarks or
point to inefficient asm code generated, then don't add them again.

The feedback I got from CPU and compiler people over the years is not to
bother using the annotations. CPUs are doing dynamic branch prediction
and compilers are applying tons of optimizations.

GCC docs state about the builtin: "In general, you should prefer to use
actual profile feedback for this (-fprofile-arcs), as programmers are
notoriously bad at predicting how their programs actually perform."
(https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 23/24] erofs: introduce cached decompression

[David Sterba]

On Mon, Jul 22, 2019 at 06:58:59PM +0800, Gao Xiang wrote:
> On 2019/7/22 6:18, David Sterba wrote:
> > On Mon, Jul 22, 2019 at 10:50:42AM +0800, Gao Xiang wrote:
> >> +choice
> >> +  prompt "EROFS Data Decompression mode"
> >> +  depends on EROFS_FS_ZIP
> >> +  default EROFS_FS_ZIP_CACHE_READAROUND
> >> +  help
> >> +EROFS supports three options for decompression.
> >> +"In-place I/O Only" consumes the minimum memory
> >> +with lowest random read.
> >> +
> >> +"Cached Decompression for readaround" consumes
> >> +the maximum memory with highest random read.
> >> +
> >> +If unsure, select "Cached Decompression for readaround"
> >> +
> >> +config EROFS_FS_ZIP_CACHE_DISABLED
> >> +  bool "In-place I/O Only"
> >> +  help
> >> +Read compressed data into page cache and do in-place
> >> +I/O decompression directly.
> >> +
> >> +config EROFS_FS_ZIP_CACHE_READAHEAD
> >> +  bool "Cached Decompression for readahead"
> >> +  help
> >> +For each request, it caches the last compressed page
> >> +for further reading.
> >> +It still does in-place I/O for the rest compressed pages.
> >> +
> >> +config EROFS_FS_ZIP_CACHE_READAROUND
> >> +  bool "Cached Decompression for readaround"
> >> +  help
> >> +For each request, it caches the both end compressed pages
> >> +for further reading.
> >> +It still does in-place I/O for the rest compressed pages.
> >> +
> >> +Recommended for performance priority.
> > 
> > The number of individual Kconfig options is quite high, are you sure you
> > need them to be split like that?
> 
> You mean the above? these are 3 cache strategies, which impact the
> runtime memory consumption and performance. I tend to leave the above
> as it-is...

No, I mean all Kconfig options, they're scattered over several patches,
best seen in the checked out branch. The cache strategies are actually
just one config option (choice).

> I'm not sure vm_map_ram() is always better than vmap() for all
> platforms (it has noticeable performance impact). However that
> seems true for my test machines (x86-64, arm64).
> 
> If vm_map_ram() is always the optimal choice compared with vmap(),
> I will remove vmap() entirely, that is OK. But I am not sure for
> every platforms though.

You can select the implementation by platform, I don't know what are the
criteria like cpu type etc, but I expect it's something that can be
determined at module load time. Eventually a module parameter can be the
the way to set it.

> > And so on. I'd suggest to go through all the options and reconsider them
> > to be built-in, or runtime settings. Debugging features like the fault
> > injections could be useful on non-debugging builds too, so a separate
> > option is fine, otherwise grouping other debugging options under the
> > main EROFS_FS_DEBUG would look more logical.
> 
> The remaining one is EROFS_FS_CLUSTER_PAGE_LIMIT. It impacts the total
> size of z_erofs_pcluster structure. It's a hard limit, and should be
> configured as small as possible. I can remove it right now since multi-block
> compression is not available now. However, it will be added again after
> multi-block compression is supported.
> 
> So, How about leave it right now and use the default value?

>From the Kconfig and build-time settings perspective I think it's
misplaced. This affects testing, you'd have to rebuild and reinstall the
module to test any change, while it's "just" a number that can be either
module parameter, sysfs knob, mount option or special ioctl.

But I may be wrong, EROFS is a special purpose filesystem, so the
fine-grained build options might make sense (eg. due to smaller code).
The question should be how does each option affect typical production
build targets. Fewer is IMHO better.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 23/24] erofs: introduce cached decompression

[David Sterba]

On Mon, Jul 22, 2019 at 10:50:42AM +0800, Gao Xiang wrote:
> +choice
> + prompt "EROFS Data Decompression mode"
> + depends on EROFS_FS_ZIP
> + default EROFS_FS_ZIP_CACHE_READAROUND
> + help
> +   EROFS supports three options for decompression.
> +   "In-place I/O Only" consumes the minimum memory
> +   with lowest random read.
> +
> +   "Cached Decompression for readaround" consumes
> +   the maximum memory with highest random read.
> +
> +   If unsure, select "Cached Decompression for readaround"
> +
> +config EROFS_FS_ZIP_CACHE_DISABLED
> + bool "In-place I/O Only"
> + help
> +   Read compressed data into page cache and do in-place
> +   I/O decompression directly.
> +
> +config EROFS_FS_ZIP_CACHE_READAHEAD
> + bool "Cached Decompression for readahead"
> + help
> +   For each request, it caches the last compressed page
> +   for further reading.
> +   It still does in-place I/O for the rest compressed pages.
> +
> +config EROFS_FS_ZIP_CACHE_READAROUND
> + bool "Cached Decompression for readaround"
> + help
> +   For each request, it caches the both end compressed pages
> +   for further reading.
> +   It still does in-place I/O for the rest compressed pages.
> +
> +   Recommended for performance priority.

The number of individual Kconfig options is quite high, are you sure you
need them to be split like that?

Eg. the xattrs, acls and security labels seem to be part of the basic
set of features so I wonder who does not want to enable them by default.
I think you copied ext4 as a skeleton for the options, but for a new
filesystem it's not necessary copy the history where I think features
were added over time.

Then eg. the option EROFS_FS_IO_MAX_RETRIES looks like a runtime
setting, the config help text does not explain anything about the change
in behaviour leaving the user with 'if not sure take the defaut'.

EROFS_FS_USE_VM_MAP_RAM is IMO a very low implementation detail, why
does it need to be config option at all?

And so on. I'd suggest to go through all the options and reconsider them
to be built-in, or runtime settings. Debugging features like the fault
injections could be useful on non-debugging builds too, so a separate
option is fine, otherwise grouping other debugging options under the
main EROFS_FS_DEBUG would look more logical.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] mm: remove cleancache.c

[David Sterba]

On Mon, May 27, 2019 at 12:32:06PM +0200, Juergen Gross wrote:
> With the removal of tmem and xen-selfballoon the only user of
> cleancache is gone. Remove it, too.
> 
> Signed-off-by: Juergen Gross 
> ---
>  Documentation/vm/cleancache.rst  | 296 
>  Documentation/vm/frontswap.rst   |  10 +-
>  Documentation/vm/index.rst   |   1 -
>  MAINTAINERS  |   7 -
>  drivers/staging/erofs/data.c |   6 -
>  drivers/staging/erofs/internal.h |   1 -
>  fs/block_dev.c   |   5 -

For the btrfs part:

>  fs/btrfs/extent_io.c |   9 --
>  fs/btrfs/super.c     |   2 -

Acked-by: David Sterba 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

[David Sterba]

On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
> 
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
> 
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
> 
> Signed-off-by: Arnd Bergmann 
> ---

>  fs/btrfs/super.c    | 2 +-

Acked-by: David Sterba 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 07/12] fs: btrfs: Use ktime_get_real_ts for root ctime

[David Sterba]

On Fri, Apr 07, 2017 at 05:57:05PM -0700, Deepa Dinamani wrote:
> btrfs_root_item maintains the ctime for root updates.
> This is not part of vfs_inode.
> 
> Since current_time() uses struct inode* as an argument
> as Linus suggested, this cannot be used to update root
> times unless, we modify the signature to use inode.
> 
> Since btrfs uses nanosecond time granularity, it can also
> use ktime_get_real_ts directly to obtain timestamp for
> the root. It is necessary to use the timespec time api
> here because the same btrfs_set_stack_timespec_*() apis
> are used for vfs inode times as well. These can be
> transitioned to using timespec64 when btrfs internally
> changes to use timespec64 as well.
> 
> Signed-off-by: Deepa Dinamani 
> Acked-by: David Sterba 
> Reviewed-by: Arnd Bergmann 

I'm going to add the patch to my 4.12 queue and will let Andrew know.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel