Re: [f2fs-dev] [PATCH RFC PKS/PMEM 33/58] fs/cramfs: Utilize new kmap_thread()

[Dan Williams]

On Tue, Oct 13, 2020 at 12:37 PM Matthew Wilcox  wrote:
>
> On Tue, Oct 13, 2020 at 11:44:29AM -0700, Dan Williams wrote:
> > On Fri, Oct 9, 2020 at 12:52 PM  wrote:
> > >
> > > From: Ira Weiny 
> > >
> > > The kmap() calls in this FS are localized to a single thread.  To avoid
> > > the over head of global PKRS updates use the new kmap_thread() call.
> > >
> > > Cc: Nicolas Pitre 
> > > Signed-off-by: Ira Weiny 
> > > ---
> > >  fs/cramfs/inode.c | 10 +-
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> > > index 912308600d39..003c014a42ed 100644
> > > --- a/fs/cramfs/inode.c
> > > +++ b/fs/cramfs/inode.c
> > > @@ -247,8 +247,8 @@ static void *cramfs_blkdev_read(struct super_block 
> > > *sb, unsigned int offset,
> > > struct page *page = pages[i];
> > >
> > > if (page) {
> > > -   memcpy(data, kmap(page), PAGE_SIZE);
> > > -   kunmap(page);
> > > +   memcpy(data, kmap_thread(page), PAGE_SIZE);
> > > +   kunmap_thread(page);
> >
> > Why does this need a sleepable kmap? This looks like a textbook
> > kmap_atomic() use case.
>
> There's a lot of code of this form.  Could we perhaps have:
>
> static inline void copy_to_highpage(struct page *to, void *vfrom, unsigned 
> int size)
> {
> char *vto = kmap_atomic(to);
>
> memcpy(vto, vfrom, size);
> kunmap_atomic(vto);
> }
>
> in linux/highmem.h ?

Nice, yes, that could also replace the local ones in lib/iov_iter.c
(memcpy_{to,from}_page())


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH RFC PKS/PMEM 33/58] fs/cramfs: Utilize new kmap_thread()

[Dan Williams]

On Fri, Oct 9, 2020 at 12:52 PM  wrote:
>
> From: Ira Weiny 
>
> The kmap() calls in this FS are localized to a single thread.  To avoid
> the over head of global PKRS updates use the new kmap_thread() call.
>
> Cc: Nicolas Pitre 
> Signed-off-by: Ira Weiny 
> ---
>  fs/cramfs/inode.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> index 912308600d39..003c014a42ed 100644
> --- a/fs/cramfs/inode.c
> +++ b/fs/cramfs/inode.c
> @@ -247,8 +247,8 @@ static void *cramfs_blkdev_read(struct super_block *sb, 
> unsigned int offset,
> struct page *page = pages[i];
>
> if (page) {
> -   memcpy(data, kmap(page), PAGE_SIZE);
> -   kunmap(page);
> +   memcpy(data, kmap_thread(page), PAGE_SIZE);
> +   kunmap_thread(page);

Why does this need a sleepable kmap? This looks like a textbook
kmap_atomic() use case.


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v8 1/1] f2fs: dax: implement direct access

[Dan Williams]

On Wed, Jul 26, 2017 at 1:11 PM, Christoph Hellwig  wrote:
> On Wed, Jul 26, 2017 at 12:16:11PM -0700, Dan Williams wrote:
>> Silently turn on DAX if HMAT says its ok?
>
> Yes, absolutely.  I want my system to do the right thing by default,
> and if HMAT says bypassing the page cache is a clear advatange it
> should be the default.
>
>> I think we would instead
>> want a "-o autodax" for that case and then "-o dax" and "-o nodax" for
>> the force cases.
>
> Why?
>

I'm worried about the case where HMAT says pmem >= dram performance,
but dax semantics like disabling delayed allocation and
dirty-cacheline tracking end up hurting performance, but I guess we
can handle that on a case by case basis with targeted kernel
optimizations.

>> I think it's easier to administer than the dax mount option. If
>> someone wants dax on only in a sub-tree they can set the flag on that
>> parent directory and have a policy in dax filesystems that children
>> inherit the dax policy from the parent. That seems a better
>> administrative model than trying to get it all right globally at mount
>> time.
>
> And why exactly? If DAX is faster for file a in directory X it will
> be just as fast for a file b in directory Y.

So I want the inode setting for the pmem < dram performance case where
I know that access patterns of the application using file b in
directory Y can still yield better performance without page cache. For
example, the working set is larger than dram capacity.

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v8 1/1] f2fs: dax: implement direct access

[Dan Williams]

On Wed, Jul 26, 2017 at 10:20 AM, Christoph Hellwig  wrote:
> On Wed, Jul 26, 2017 at 10:11:08AM -0700, Dan Williams wrote:
>> Until HMAT came along we had no data in the kernel how to pick a sane
>> default, but we could now very easily make a "if pmem performance <
>> dram, disable dax by default" policy in the kernel.
>
> I'd rather do it the other way around - if HMAT is present and
> pmem performance >= dram use dax.  Else require the explicit -o dax
> for now to enable it.  If an explicit -o nodax is specified disable
> DAX even if HMAT says it is faster.

Silently turn on DAX if HMAT says its ok? I think we would instead
want a "-o autodax" for that case and then "-o dax" and "-o nodax" for
the force cases.

>> The question for this patch is do we want to add yet another
>> filesystem that adds "-o dax" or require use of per-inode flags to
>> enable dax.
>
> Please stick to the mount option.  After spending a lot of time with
> DAX and various memory techologies I'm pretty confident that the inode
> flag is the wrong thing to do.

I think it's easier to administer than the dax mount option. If
someone wants dax on only in a sub-tree they can set the flag on that
parent directory and have a policy in dax filesystems that children
inherit the dax policy from the parent. That seems a better
administrative model than trying to get it all right globally at mount
time.

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v8 1/1] f2fs: dax: implement direct access

[Dan Williams]

On Wed, Jul 26, 2017 at 12:26 AM, Christoph Hellwig  wrote:
> On Tue, Jul 25, 2017 at 05:15:10PM -0700, Dan Williams wrote:
>> We're in the process of walking back and potentially deprecating the
>> use of the dax mount option for xfs and ext4 since dax can have
>> negative performance implications if page cache memory happens to be
>> faster than pmem. It should be limited to applications that
>> specifically want the semantic, not globally enabled for the entire
>> mount. xfs has went ahead and added the XFS_DIFLAG2_DAX indoe flag for
>> per-inode enabling of dax.
>>
>> I'm wondering if any new filesystem that adds dax support at this
>> point should do so with inode flags and not a mount option?
>
> That tradeoff is not one that the application should make, but one that
> should depend on the storage medium. To make things worse it might also
> depend on the type of access. E.g. with certain media it makes a lot of
> sense to cache writes in the page cache, but generally not reads.
> I've been spending some time to analyze how that could be done, but
> I've not made real progress on it.
>
> XFS_DIFLAG2_DAX is unfortunately totally unhelpful with that.

It allows for opt-in for applications, or administrators of those
applications, that know the type of access. There's also the new HMAT
(heterogeneous memory attributes table) in ACPI that can indicate the
relative performance of pmem to system-ram if userspace needs data to
make a decision. It would be interesting to have an automatic policy
in the kernel, but we also need a mechanism for explicit
configurations.

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v8 1/1] f2fs: dax: implement direct access

[Dan Williams]

On Wed, Jul 26, 2017 at 10:01 AM, Christoph Hellwig  wrote:
> On Wed, Jul 26, 2017 at 09:53:07AM -0700, Dan Williams wrote:
>> It allows for opt-in for applications, or administrators of those
>> applications, that know the type of access.
>
> That's BS.  We need to provide the best possible way to access the
> media to an application.  And whether that's DAX or the page cache
> is an implementation detail that should not matter to the application.
>
> Which doesn't mean there shouldn't be ways to override the default
> that the kernel chose based on hardware details, but it's certainly
> not something for the application to hardcode, but something for
> the adminstrator to decide.

Until HMAT came along we had no data in the kernel how to pick a sane
default, but we could now very easily make a "if pmem performance <
dram, disable dax by default" policy in the kernel.

The question for this patch is do we want to add yet another
filesystem that adds "-o dax" or require use of per-inode flags to
enable dax.

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v8 1/1] f2fs: dax: implement direct access

[Dan Williams]

[ adding linux-nvdimm ]

On Thu, Jul 20, 2017 at 5:10 AM, sunqiuyang  wrote:
> From: Qiuyang Sun 
>
> This patch implements Direct Access (DAX) in F2FS, including:
>  - a mount option to choose whether to enable DAX or not

We're in the process of walking back and potentially deprecating the
use of the dax mount option for xfs and ext4 since dax can have
negative performance implications if page cache memory happens to be
faster than pmem. It should be limited to applications that
specifically want the semantic, not globally enabled for the entire
mount. xfs has went ahead and added the XFS_DIFLAG2_DAX indoe flag for
per-inode enabling of dax.

I'm wondering if any new filesystem that adds dax support at this
point should do so with inode flags and not a mount option?

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 01/10] fs crypto: add basic definitions for per-file encryption

[Dan Williams]

On Mon, Feb 29, 2016 at 5:35 PM, Jaegeuk Kim  wrote:
> On Sun, Feb 28, 2016 at 09:41:22PM -0800, Randy Dunlap wrote:
>> On 02/25/16 11:25, Jaegeuk Kim wrote:
>> > This patch adds definitions for per-file encryption used by ext4 and f2fs.
>> >
>> > Signed-off-by: Jaegeuk Kim 
>> > ---
>> >  include/linux/fs.h   |   8 ++
>> >  include/linux/fscrypto.h | 239 
>> > +++
>> >  include/uapi/linux/fs.h  |  18 
>> >  3 files changed, 265 insertions(+)
>> >  create mode 100644 include/linux/fscrypto.h
>> >
>> > diff --git a/include/linux/fs.h b/include/linux/fs.h
>> > index ae68100..d8f57cf 100644
>> > --- a/include/linux/fs.h
>> > +++ b/include/linux/fs.h
>> > @@ -53,6 +53,8 @@ struct swap_info_struct;
>> >  struct seq_file;
>> >  struct workqueue_struct;
>> >  struct iov_iter;
>> > +struct fscrypt_info;
>> > +struct fscrypt_operations;
>> >
>> >  extern void __init inode_init(void);
>> >  extern void __init inode_init_early(void);
>> > @@ -678,6 +680,10 @@ struct inode {
>> > struct hlist_head   i_fsnotify_marks;
>> >  #endif
>> >
>> > +#ifdef CONFIG_FS_ENCRYPTION
>> > +   struct fscrypt_info *i_crypt_info;
>> > +#endif
>> > +
>> > void*i_private; /* fs or device private pointer */
>> >  };
>> >
>> > @@ -1323,6 +1329,8 @@ struct super_block {
>> >  #endif
>> > const struct xattr_handler **s_xattr;
>> >
>> > +   const struct fscrypt_operations *s_cop;
>> > +
>> > struct hlist_bl_heads_anon; /* anonymous dentries for 
>> > (nfs) exporting */
>> > struct list_heads_mounts;   /* list of mounts; _not_ for 
>> > fs use */
>> > struct block_device *s_bdev;
>> > diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
>> > new file mode 100644
>> > index 000..b0aed92
>> > --- /dev/null
>> > +++ b/include/linux/fscrypto.h
>> > @@ -0,0 +1,239 @@
>> > +/*
>> > + * General per-file encryption definition
>> > + *
>> > + * Copyright (C) 2015, Google, Inc.
>> > + *
>> > + * Written by Michael Halcrow, 2015.
>> > + * Modified by Jaegeuk Kim, 2015.
>> > + */
>> > +
>> > +#ifndef _LINUX_FSCRYPTO_H
>> > +#define _LINUX_FSCRYPTO_H
>> > +
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +
>> > +#define FS_KEY_DERIVATION_NONCE_SIZE   16
>> > +#define FS_ENCRYPTION_CONTEXT_FORMAT_V11
>> > +
>> > +#define FS_POLICY_FLAGS_PAD_4  0x00
>> > +#define FS_POLICY_FLAGS_PAD_8  0x01
>> > +#define FS_POLICY_FLAGS_PAD_16 0x02
>> > +#define FS_POLICY_FLAGS_PAD_32 0x03
>> > +#define FS_POLICY_FLAGS_PAD_MASK   0x03
>> > +#define FS_POLICY_FLAGS_VALID  0x03
>> > +
>> > +/* Encryption algorithms */
>> > +#define FS_ENCRYPTION_MODE_INVALID 0
>> > +#define FS_ENCRYPTION_MODE_AES_256_XTS 1
>> > +#define FS_ENCRYPTION_MODE_AES_256_GCM 2
>> > +#define FS_ENCRYPTION_MODE_AES_256_CBC 3
>> > +#define FS_ENCRYPTION_MODE_AES_256_CTS 4
>> > +
>> > +/**
>> > + * Encryption context for inode
>> > + *
>> > + * Protector format:
>> > + *  1 byte: Protector format (1 = this version)
>> > + *  1 byte: File contents encryption mode
>> > + *  1 byte: File names encryption mode
>> > + *  1 byte: Flags
>> > + *  8 bytes: Master Key descriptor
>> > + *  16 bytes: Encryption Key derivation nonce
>> > + */
>> > +struct fscrypt_context {
>> > +   char format;
>> > +   char contents_encryption_mode;
>> > +   char filenames_encryption_mode;
>> > +   char flags;
>> > +   char master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
>> > +   char nonce[FS_KEY_DERIVATION_NONCE_SIZE];
>>
>> how about u8 instead of char?
>
> It seems that it needs to user u8 instead of char for other variables as well.
> I'll take a look at all the usages.

I think it needs to be __u8 otherwise I get this in a userspace program:

In file included from test/blk_namespaces.c:17:0:
/usr/include/linux/fs.h:256:2: error: unknown type name ‘u8’
  u8 version;
  ^
/usr/include/linux/fs.h:257:2: error: unknown type name ‘u8’
  u8 contents_encryption_mode;
  ^
/usr/include/linux/fs.h:258:2: error: unknown type name ‘u8’
  u8 filenames_encryption_mode;
  ^
/usr/include/linux/fs.h:259:2: error: unknown type name ‘u8’
  u8 flags;
  ^
/usr/include/linux/fs.h:260:2: error: unknown type name ‘u8’
  u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
  ^

--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel