* Re: [PATCHSET v15] io_uring IO interface
From: Jens Axboe @ 2019-02-21 17:48 UTC (permalink / raw)
To: Marek Majkowski
Cc: avi, hch, jannh, jmoyer, linux-aio, linux-api, linux-block, viro
In-Reply-To: <20190221121022.7867-1-marek@cloudflare.com>
On 2/21/19 5:10 AM, Marek Majkowski wrote:
>> From: Jens Axboe <axboe@kernel.dk>
>> Subject: [PATCHSET v15] io_uring IO interface
>> Message-ID: <20190211190049.7888-1-axboe@kernel.dk> (raw)
>>
>> Some final tweaks, mostly cosmetic, but also two important fixes:
>>
>> 1) Ensure that we account the skb appropriately against the socket.
>> Some network config options apparently return is an skb with
>> ->truesize != 0 when allocated with a size of 0, ensure we add
>> those as references against sock->sk_wmem_alloc. Reported by
>> Matt Mullins.
>
> Jens,
>
> I tried using io_uring with network sockets. It seem to be doing the
> right thing. One bit is missing though: "flags" as in recv(2).
>
> In perfect world I would like to specify at least:
> - MSG_DONTWAIT
> - MSG_WAITALL
> - MSG_NOSIGNAL
>
> Right now, unless I'm missing something, io_uring_sqe doesn't have a
> place where we could store these. "flags" is needed for any
> non-trivial network I/O.
We have flags for sqes, depending on the type. You can add to the
union that already holds rw_flags/fsync_flags/poll_events? There's
also a (smaller) flags field that applies for all types, which
currently only holds the fixed file flag.
If you're talking on a per-syscall type of flag, io_uring_enter(2)
does take a flags member.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 11/19] block: implement bio helper to add iter bvec pages to bio
From: Jens Axboe @ 2019-02-21 17:45 UTC (permalink / raw)
To: Ming Lei; +Cc: linux-aio, linux-block, linux-api, hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190220225856.GB28313@ming.t460p>
On 2/20/19 3:58 PM, Ming Lei wrote:
> On Mon, Feb 11, 2019 at 12:00:41PM -0700, Jens Axboe wrote:
>> For an ITER_BVEC, we can just iterate the iov and add the pages
>> to the bio directly. This requires that the caller doesn't releases
>> the pages on IO completion, we add a BIO_NO_PAGE_REF flag for that.
>>
>> The current two callers of bio_iov_iter_get_pages() are updated to
>> check if they need to release pages on completion. This makes them
>> work with bvecs that contain kernel mapped pages already.
>>
>> Reviewed-by: Hannes Reinecke <hare@suse.com>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>> block/bio.c | 59 ++++++++++++++++++++++++++++++++-------
>> fs/block_dev.c | 5 ++--
>> fs/iomap.c | 5 ++--
>> include/linux/blk_types.h | 1 +
>> 4 files changed, 56 insertions(+), 14 deletions(-)
>>
>> diff --git a/block/bio.c b/block/bio.c
>> index 4db1008309ed..330df572cfb8 100644
>> --- a/block/bio.c
>> +++ b/block/bio.c
>> @@ -828,6 +828,23 @@ int bio_add_page(struct bio *bio, struct page *page,
>> }
>> EXPORT_SYMBOL(bio_add_page);
>>
>> +static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
>> +{
>> + const struct bio_vec *bv = iter->bvec;
>> + unsigned int len;
>> + size_t size;
>> +
>> + len = min_t(size_t, bv->bv_len, iter->count);
>> + size = bio_add_page(bio, bv->bv_page, len,
>> + bv->bv_offset + iter->iov_offset);
>
> iter->iov_offset needs to be subtracted from 'len', looks
> the following delta change[1] is required, otherwise memory corruption
> can be observed when running xfstests over loop/dio.
Thanks, I folded this in.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCHSET v15] io_uring IO interface
From: Marek Majkowski @ 2019-02-21 12:10 UTC (permalink / raw)
To: axboe; +Cc: avi, hch, jannh, jmoyer, linux-aio, linux-api, linux-block, viro
In-Reply-To: <20190211190049.7888-1-axboe@kernel.dk>
> From: Jens Axboe <axboe@kernel.dk>
> Subject: [PATCHSET v15] io_uring IO interface
> Message-ID: <20190211190049.7888-1-axboe@kernel.dk> (raw)
>
> Some final tweaks, mostly cosmetic, but also two important fixes:
>
> 1) Ensure that we account the skb appropriately against the socket.
> Some network config options apparently return is an skb with
> ->truesize != 0 when allocated with a size of 0, ensure we add
> those as references against sock->sk_wmem_alloc. Reported by
> Matt Mullins.
Jens,
I tried using io_uring with network sockets. It seem to be doing the
right thing. One bit is missing though: "flags" as in recv(2).
In perfect world I would like to specify at least:
- MSG_DONTWAIT
- MSG_WAITALL
- MSG_NOSIGNAL
Right now, unless I'm missing something, io_uring_sqe doesn't have a
place where we could store these. "flags" is needed for any
non-trivial network I/O.
A separate discussion is about io_uring supporting more complex
network stuff in future like 'struct msghdr', MSG_ERRQUEUE or CMSG.
Cheers,
Marek
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [RFC PATCH v3 07/18] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
From: Richard Weinberger @ 2019-02-21 9:33 UTC (permalink / raw)
To: Eric Biggers, linux-fscrypt
Cc: linux-ext4, open list:ABI/API, linux-f2fs-devel, keyrings,
linux-mtd, linux-crypto, linux-fsdevel, Satya Tangirala,
Paul Crowley
In-Reply-To: <20190221054938.GA12467@sol.localdomain>
Eric,
Am Donnerstag, 21. Februar 2019, 06:49:39 CET schrieb Eric Biggers:
> Hi Richard,
>
> On Thu, Feb 21, 2019 at 12:52:38AM +0100, Richard Weinberger wrote:
> > On Wed, Feb 20, 2019 at 7:55 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > +#define FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE \
> > > + (CONST_STRLEN("fscrypt-") + FIELD_SIZEOF(struct super_block, s_id))
> > > +
> > > +#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_DESCRIPTOR_SIZE + 1)
> > > +
> > > +static void format_fs_keyring_description(
> > > + char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE],
> > > + const struct super_block *sb)
> > > +{
> > > + sprintf(description, "fscrypt-%s", sb->s_id);
> > > +}
> >
> > I fear ->s_id is not the right thing.
> > For filesystems such as ext4 ->s_id is the name of the backing block device,
> > so it is per filesysem instance unique.
> > But this is not guaranteed. For UBIFS ->s_id is just "ubifs", always.
> > So the names will clash.
> >
>
> What name do you suggest using for UBIFS filesystems? The keyring name could be
> set by the filesystem via a fscrypt_operations callback if needed.
IMHO the BDI name should be used.
> Note that the keyring name isn't particularly important, since the ioctls will
> work regardless. But we might as well choose something logical, since the
> keyring name will still show up in /proc/keys.
I'm not done with reviewing your patches, but will it be possible to use keyctl?
For the a unique name is helpful. :)
Thanks,
//richard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* [RFC PATCH] mm,mremap: Bail out earlier in mremap_to under map pressure
From: Oscar Salvador @ 2019-02-21 8:54 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-api, hughd, kirill, vbabka, joel, jglisse,
yang.shi, mgorman, Oscar Salvador
When using mremap() syscall in addition to MREMAP_FIXED flag,
mremap() calls mremap_to() which does the following:
1) unmaps the destination region where we are going to move the map
2) If the new region is going to be smaller, we unmap the last part
of the old region
Then, we will eventually call move_vma() to do the actual move.
move_vma() checks whether we are at least 4 maps below max_map_count
before going further, otherwise it bails out with -ENOMEM.
The problem is that we might have already unmapped the vma's in steps
1) and 2), so it is not possible for userspace to figure out the state
of the vma's after it gets -ENOMEM, and it gets tricky for userspace
to clean up properly on error path.
While it is true that we can return -ENOMEM for more reasons
(e.g: see may_expand_vm() or move_page_tables()), I think that we can
avoid this scenario in concret if we check early in mremap_to() if the
operation has high chances to succeed map-wise.
Should not be that the case, we can bail out before we even try to unmap
anything, so we make sure the vma's are left untouched in case we are likely
to be short of maps.
The thumb-rule now is to rely on the worst-scenario case we can have.
That is when both vma's (old region and new region) are going to be split
in 3, so we get two more maps to the ones we already hold (one per each).
If current map count + 2 maps still leads us to 4 maps below the threshold,
we are going to pass the check in move_vma().
Of course, this is not free, as it might generate false positives when it is
true that we are tight map-wise, but the unmap operation can release several
vma's leading us to a good state.
Because of that I am sending this as a RFC.
Another approach was also investigated [1], but it may be too much hassle
for what it brings.
[1] https://lore.kernel.org/lkml/20190219155320.tkfkwvqk53tfdojt@d104.suse.de/
Signed-off-by: Oscar Salvador <osalvador@suse.de>
---
mm/mremap.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/mm/mremap.c b/mm/mremap.c
index 3320616ed93f..e3edef6b7a12 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -516,6 +516,23 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
if (addr + old_len > new_addr && new_addr + new_len > addr)
goto out;
+ /*
+ * move_vma() need us to stay 4 maps below the threshold, otherwise
+ * it will bail out at the very beginning.
+ * That is a problem if we have already unmaped the regions here
+ * (new_addr, and old_addr), because userspace will not know the
+ * state of the vma's after it gets -ENOMEM.
+ * So, to avoid such scenario we can pre-compute if the whole
+ * operation has high chances to success map-wise.
+ * Worst-scenario case is when both vma's (new_addr and old_addr) get
+ * split in 3 before unmaping it.
+ * That means 2 more maps (1 for each) to the ones we already hold.
+ * Check whether current map count plus 2 still leads us to 4 maps below
+ * the threshold, otherwise return -ENOMEM here to be more safe.
+ */
+ if ((mm->map_count + 2) >= sysctl_max_map_count - 3)
+ return -ENOMEM;
+
ret = do_munmap(mm, new_addr, new_len, uf_unmap_early);
if (ret)
goto out;
--
2.13.7
^ permalink raw reply related
* Re: [RFC PATCH v3 07/18] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
From: Eric Biggers @ 2019-02-21 5:49 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-ext4, open list:ABI/API, linux-f2fs-devel, linux-fscrypt,
keyrings, linux-mtd, linux-crypto, linux-fsdevel, Satya Tangirala,
Paul Crowley
In-Reply-To: <CAFLxGvy7WsV7Qo42yTbsZf9YnW563a0k6Yx2-NmASNtoektu_Q@mail.gmail.com>
Hi Richard,
On Thu, Feb 21, 2019 at 12:52:38AM +0100, Richard Weinberger wrote:
> On Wed, Feb 20, 2019 at 7:55 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > +#define FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE \
> > + (CONST_STRLEN("fscrypt-") + FIELD_SIZEOF(struct super_block, s_id))
> > +
> > +#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_DESCRIPTOR_SIZE + 1)
> > +
> > +static void format_fs_keyring_description(
> > + char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE],
> > + const struct super_block *sb)
> > +{
> > + sprintf(description, "fscrypt-%s", sb->s_id);
> > +}
>
> I fear ->s_id is not the right thing.
> For filesystems such as ext4 ->s_id is the name of the backing block device,
> so it is per filesysem instance unique.
> But this is not guaranteed. For UBIFS ->s_id is just "ubifs", always.
> So the names will clash.
>
> --
> Thanks,
> //richard
What name do you suggest using for UBIFS filesystems? The keyring name could be
set by the filesystem via a fscrypt_operations callback if needed.
Note that the keyring name isn't particularly important, since the ioctls will
work regardless. But we might as well choose something logical, since the
keyring name will still show up in /proc/keys.
- Eric
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [RFC PATCH v3 07/18] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
From: Richard Weinberger @ 2019-02-20 23:52 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-ext4, open list:ABI/API, linux-f2fs-devel, linux-fscrypt,
keyrings, linux-mtd, linux-crypto, linux-fsdevel, Satya Tangirala,
Paul Crowley
In-Reply-To: <20190220065249.32099-8-ebiggers@kernel.org>
On Wed, Feb 20, 2019 at 7:55 AM Eric Biggers <ebiggers@kernel.org> wrote:
> +#define FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE \
> + (CONST_STRLEN("fscrypt-") + FIELD_SIZEOF(struct super_block, s_id))
> +
> +#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_DESCRIPTOR_SIZE + 1)
> +
> +static void format_fs_keyring_description(
> + char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE],
> + const struct super_block *sb)
> +{
> + sprintf(description, "fscrypt-%s", sb->s_id);
> +}
I fear ->s_id is not the right thing.
For filesystems such as ext4 ->s_id is the name of the backing block device,
so it is per filesysem instance unique.
But this is not guaranteed. For UBIFS ->s_id is just "ubifs", always.
So the names will clash.
--
Thanks,
//richard
^ permalink raw reply
* Re: [RFC PATCH v3 04/18] fs: add ->s_master_keys to struct super_block
From: Richard Weinberger @ 2019-02-20 23:19 UTC (permalink / raw)
To: Eric Biggers
Cc: Satya Tangirala, open list:ABI/API, linux-f2fs-devel,
linux-fscrypt, keyrings, linux-mtd, linux-crypto, linux-fsdevel,
linux-ext4, Paul Crowley
In-Reply-To: <20190220065249.32099-5-ebiggers@kernel.org>
On Wed, Feb 20, 2019 at 7:55 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Add an ->s_master_keys keyring to 'struct super_block'. New fscrypt
> ioctls will allow adding and removing encryption keys from this keyring.
> This will enable solving multiple interrelated problems with how fscrypt
> keys are provided and managed currently, including:
>
> - Making the key status (which is currently per-process) match the
> filesystem-level status of which encrypted files are "unlocked".
>
> - Supporting a proper API to remove encryption keys, "locking" the
> corresponding encrypted files.
>
> - Caching an HMAC transform for each master key, allowing the use of
> HKDF while still retaining good performance.
>
> - Preventing denial of service via keyctl_invalidate().
>
> Similar to the existing ->s_cop, the keyring is added to the VFS-level
> superblock struct rather than separately to the ext4, f2fs, and ubifs
> superblock structs so that it can be used by the shared code in
> fs/crypto/. To minimize overhead, the keyring will only be allocated if
> userspace actually adds a key; otherwise will stay NULL.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> fs/super.c | 3 +++
> include/linux/fs.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/fs/super.c b/fs/super.c
> index 48e25eba8465..7ca05dda905c 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -291,6 +291,9 @@ static void __put_super(struct super_block *s)
> security_sb_free(s);
> put_user_ns(s->s_user_ns);
> kfree(s->s_subtype);
> +#ifdef CONFIG_FS_ENCRYPTION
> + key_put(s->s_master_keys);
> +#endif
Please wrap this in a static inline function such that you can get rid
of the ifdef here.
Just like put_user_ns() does.
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH 11/19] block: implement bio helper to add iter bvec pages to bio
From: Ming Lei @ 2019-02-20 22:58 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-aio, linux-block, linux-api, hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190211190049.7888-13-axboe@kernel.dk>
On Mon, Feb 11, 2019 at 12:00:41PM -0700, Jens Axboe wrote:
> For an ITER_BVEC, we can just iterate the iov and add the pages
> to the bio directly. This requires that the caller doesn't releases
> the pages on IO completion, we add a BIO_NO_PAGE_REF flag for that.
>
> The current two callers of bio_iov_iter_get_pages() are updated to
> check if they need to release pages on completion. This makes them
> work with bvecs that contain kernel mapped pages already.
>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> block/bio.c | 59 ++++++++++++++++++++++++++++++++-------
> fs/block_dev.c | 5 ++--
> fs/iomap.c | 5 ++--
> include/linux/blk_types.h | 1 +
> 4 files changed, 56 insertions(+), 14 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 4db1008309ed..330df572cfb8 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -828,6 +828,23 @@ int bio_add_page(struct bio *bio, struct page *page,
> }
> EXPORT_SYMBOL(bio_add_page);
>
> +static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
> +{
> + const struct bio_vec *bv = iter->bvec;
> + unsigned int len;
> + size_t size;
> +
> + len = min_t(size_t, bv->bv_len, iter->count);
> + size = bio_add_page(bio, bv->bv_page, len,
> + bv->bv_offset + iter->iov_offset);
iter->iov_offset needs to be subtracted from 'len', looks
the following delta change[1] is required, otherwise memory corruption
can be observed when running xfstests over loop/dio.
Another interesting thing is that bio_add_page() is capable of
adding multi contiguous pages actually, especially loop uses
ITER_BVEC to pass multi-page bvecs. Even though pages in loop's
ITER_BVEC may belong to user-space, looks it is still safe to not
grab the page ref given it has been done by fs.
[1]
diff --git a/block/bio.c b/block/bio.c
index 3b49963676fc..df99bb3816a1 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -842,7 +842,10 @@ static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
unsigned int len;
size_t size;
- len = min_t(size_t, bv->bv_len, iter->count);
+ if (WARN_ON_ONCE(iter->iov_offset > bv->bv_len))
+ return -EINVAL;
+
+ len = min_t(size_t, bv->bv_len - iter->iov_offset, iter->count);
size = bio_add_page(bio, bv->bv_page, len,
bv->bv_offset + iter->iov_offset);
if (size == len) {
Thanks,
Ming
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* Re: [PATCHv6 07/10] acpi/hmat: Register processor domain to its memory
From: Rafael J. Wysocki @ 2019-02-20 22:50 UTC (permalink / raw)
To: Keith Busch
Cc: Rafael J. Wysocki, Dave Hansen, Linux Kernel Mailing List,
ACPI Devel Maling List, Linux Memory Management List, Linux API,
Greg Kroah-Hartman, Dan Williams
In-Reply-To: <20190220224419.GC5478@localhost.localdomain>
On Wed, Feb 20, 2019 at 11:44 PM Keith Busch <keith.busch@intel.com> wrote:
>
> On Wed, Feb 20, 2019 at 11:21:45PM +0100, Rafael J. Wysocki wrote:
> > On Wed, Feb 20, 2019 at 11:11 PM Dave Hansen <dave.hansen@intel.com> wrote:
> > > On 2/20/19 2:02 PM, Rafael J. Wysocki wrote:
> > > >> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> > > >> index c9637e2e7514..08e972ead159 100644
> > > >> --- a/drivers/acpi/hmat/Kconfig
> > > >> +++ b/drivers/acpi/hmat/Kconfig
> > > >> @@ -2,6 +2,7 @@
> > > >> config ACPI_HMAT
> > > >> bool "ACPI Heterogeneous Memory Attribute Table Support"
> > > >> depends on ACPI_NUMA
> > > >> + select HMEM_REPORTING
> > > > If you want to do this here, I'm not sure that defining HMEM_REPORTING
> > > > as a user-selectable option is a good idea. In particular, I don't
> > > > really think that setting ACPI_HMAT without it makes a lot of sense.
> > > > Apart from this, the patch looks reasonable to me.
> > >
> > > I guess the question is whether we would want to allow folks to consume
> > > the HMAT inside the kernel while not reporting it out via
> > > HMEM_REPORTING. We have some in-kernel users of the HMAT lined up like
> > > mitigations for memory-side caches.
> > >
> > > It's certainly possible that folks would want to consume those
> > > mitigations without anything in sysfs. They might not even want or need
> > > NUMA support itself, for instance.
> > >
> > > So, what should we do?
> > >
> > > config HMEM_REPORTING
> > > bool # no user-visible prompt
> > > default y if ACPI_HMAT
> > >
> > > So folks can override in their .config, but they don't see a prompt?
> >
> > Maybe it would be better to make HMEM_REPORTING do "select ACPI_HMAT if ACPI".
> >
> > The mitigations could then do that too if they depend on HMAT and
> > ACPI_HMAT need not be user-visible at all.
>
> That sounds okay, though it would create unreachable code if !ACPI since
> that's the only user for the new reporting interfaces.
Until there are other users of it, you can make HMEM_REPORTING depend
on ACPI_NUMA and select ACPI_HMAT.
^ permalink raw reply
* Re: [PATCHv6 07/10] acpi/hmat: Register processor domain to its memory
From: Keith Busch @ 2019-02-20 22:44 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Dave Hansen, Linux Kernel Mailing List, ACPI Devel Maling List,
Linux Memory Management List, Linux API, Greg Kroah-Hartman,
Dan Williams
In-Reply-To: <CAJZ5v0izS-MBcC3ZsRKK59zWcJOMQ672sRuv_GCVrsYR36Wa8w@mail.gmail.com>
On Wed, Feb 20, 2019 at 11:21:45PM +0100, Rafael J. Wysocki wrote:
> On Wed, Feb 20, 2019 at 11:11 PM Dave Hansen <dave.hansen@intel.com> wrote:
> > On 2/20/19 2:02 PM, Rafael J. Wysocki wrote:
> > >> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> > >> index c9637e2e7514..08e972ead159 100644
> > >> --- a/drivers/acpi/hmat/Kconfig
> > >> +++ b/drivers/acpi/hmat/Kconfig
> > >> @@ -2,6 +2,7 @@
> > >> config ACPI_HMAT
> > >> bool "ACPI Heterogeneous Memory Attribute Table Support"
> > >> depends on ACPI_NUMA
> > >> + select HMEM_REPORTING
> > > If you want to do this here, I'm not sure that defining HMEM_REPORTING
> > > as a user-selectable option is a good idea. In particular, I don't
> > > really think that setting ACPI_HMAT without it makes a lot of sense.
> > > Apart from this, the patch looks reasonable to me.
> >
> > I guess the question is whether we would want to allow folks to consume
> > the HMAT inside the kernel while not reporting it out via
> > HMEM_REPORTING. We have some in-kernel users of the HMAT lined up like
> > mitigations for memory-side caches.
> >
> > It's certainly possible that folks would want to consume those
> > mitigations without anything in sysfs. They might not even want or need
> > NUMA support itself, for instance.
> >
> > So, what should we do?
> >
> > config HMEM_REPORTING
> > bool # no user-visible prompt
> > default y if ACPI_HMAT
> >
> > So folks can override in their .config, but they don't see a prompt?
>
> Maybe it would be better to make HMEM_REPORTING do "select ACPI_HMAT if ACPI".
>
> The mitigations could then do that too if they depend on HMAT and
> ACPI_HMAT need not be user-visible at all.
That sounds okay, though it would create unreachable code if !ACPI since
that's the only user for the new reporting interfaces.
^ permalink raw reply
* Re: [PATCHv6 07/10] acpi/hmat: Register processor domain to its memory
From: Rafael J. Wysocki @ 2019-02-20 22:21 UTC (permalink / raw)
To: Dave Hansen
Cc: Rafael J. Wysocki, Keith Busch, Linux Kernel Mailing List,
ACPI Devel Maling List, Linux Memory Management List, Linux API,
Greg Kroah-Hartman, Dan Williams
In-Reply-To: <9ab5d6ba-4cb6-a6f1-894d-d79b77c8bc21@intel.com>
On Wed, Feb 20, 2019 at 11:11 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 2/20/19 2:02 PM, Rafael J. Wysocki wrote:
> >> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> >> index c9637e2e7514..08e972ead159 100644
> >> --- a/drivers/acpi/hmat/Kconfig
> >> +++ b/drivers/acpi/hmat/Kconfig
> >> @@ -2,6 +2,7 @@
> >> config ACPI_HMAT
> >> bool "ACPI Heterogeneous Memory Attribute Table Support"
> >> depends on ACPI_NUMA
> >> + select HMEM_REPORTING
> > If you want to do this here, I'm not sure that defining HMEM_REPORTING
> > as a user-selectable option is a good idea. In particular, I don't
> > really think that setting ACPI_HMAT without it makes a lot of sense.
> > Apart from this, the patch looks reasonable to me.
>
> I guess the question is whether we would want to allow folks to consume
> the HMAT inside the kernel while not reporting it out via
> HMEM_REPORTING. We have some in-kernel users of the HMAT lined up like
> mitigations for memory-side caches.
>
> It's certainly possible that folks would want to consume those
> mitigations without anything in sysfs. They might not even want or need
> NUMA support itself, for instance.
>
> So, what should we do?
>
> config HMEM_REPORTING
> bool # no user-visible prompt
> default y if ACPI_HMAT
>
> So folks can override in their .config, but they don't see a prompt?
Maybe it would be better to make HMEM_REPORTING do "select ACPI_HMAT if ACPI".
The mitigations could then do that too if they depend on HMAT and
ACPI_HMAT need not be user-visible at all.
^ permalink raw reply
* Re: [PATCHv6 07/10] acpi/hmat: Register processor domain to its memory
From: Dan Williams @ 2019-02-20 22:20 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Dave Hansen, Keith Busch, Linux Kernel Mailing List,
ACPI Devel Maling List, Linux Memory Management List, Linux API,
Greg Kroah-Hartman
In-Reply-To: <CAJZ5v0hwyXnsLmCKNJvzOvcG-HD1UmuWNGFvoB02=2Ks1hE0bA@mail.gmail.com>
On Wed, Feb 20, 2019 at 2:17 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Feb 20, 2019 at 11:14 PM Dan Williams <dan.j.williams@intel.com> wrote:
> >
> > On Wed, Feb 20, 2019 at 2:11 PM Dave Hansen <dave.hansen@intel.com> wrote:
> > >
> > > On 2/20/19 2:02 PM, Rafael J. Wysocki wrote:
> > > >> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> > > >> index c9637e2e7514..08e972ead159 100644
> > > >> --- a/drivers/acpi/hmat/Kconfig
> > > >> +++ b/drivers/acpi/hmat/Kconfig
> > > >> @@ -2,6 +2,7 @@
> > > >> config ACPI_HMAT
> > > >> bool "ACPI Heterogeneous Memory Attribute Table Support"
> > > >> depends on ACPI_NUMA
> > > >> + select HMEM_REPORTING
> > > > If you want to do this here, I'm not sure that defining HMEM_REPORTING
> > > > as a user-selectable option is a good idea. In particular, I don't
> > > > really think that setting ACPI_HMAT without it makes a lot of sense.
> > > > Apart from this, the patch looks reasonable to me.
> > >
> > > I guess the question is whether we would want to allow folks to consume
> > > the HMAT inside the kernel while not reporting it out via
> > > HMEM_REPORTING. We have some in-kernel users of the HMAT lined up like
> > > mitigations for memory-side caches.
> > >
> > > It's certainly possible that folks would want to consume those
> > > mitigations without anything in sysfs. They might not even want or need
> > > NUMA support itself, for instance.
> > >
> > > So, what should we do?
> > >
> > > config HMEM_REPORTING
> > > bool # no user-visible prompt
> > > default y if ACPI_HMAT
> > >
> > > So folks can override in their .config, but they don't see a prompt?
> >
> > I would add an "&& ACPI_NUMA" to that default as well.
>
> But ACPI_HMAT depends on ACPI_NUMA already, or am I missing anything?
Oh, my mistake, sorry.
^ permalink raw reply
* Re: [PATCHv6 07/10] acpi/hmat: Register processor domain to its memory
From: Rafael J. Wysocki @ 2019-02-20 22:16 UTC (permalink / raw)
To: Dan Williams
Cc: Dave Hansen, Rafael J. Wysocki, Keith Busch,
Linux Kernel Mailing List, ACPI Devel Maling List,
Linux Memory Management List, Linux API, Greg Kroah-Hartman
In-Reply-To: <CAPcyv4iP032bqAgCZ8czRXkJ_gXz0H1EVC+ypf6NhKQ65aKczg@mail.gmail.com>
On Wed, Feb 20, 2019 at 11:14 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Wed, Feb 20, 2019 at 2:11 PM Dave Hansen <dave.hansen@intel.com> wrote:
> >
> > On 2/20/19 2:02 PM, Rafael J. Wysocki wrote:
> > >> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> > >> index c9637e2e7514..08e972ead159 100644
> > >> --- a/drivers/acpi/hmat/Kconfig
> > >> +++ b/drivers/acpi/hmat/Kconfig
> > >> @@ -2,6 +2,7 @@
> > >> config ACPI_HMAT
> > >> bool "ACPI Heterogeneous Memory Attribute Table Support"
> > >> depends on ACPI_NUMA
> > >> + select HMEM_REPORTING
> > > If you want to do this here, I'm not sure that defining HMEM_REPORTING
> > > as a user-selectable option is a good idea. In particular, I don't
> > > really think that setting ACPI_HMAT without it makes a lot of sense.
> > > Apart from this, the patch looks reasonable to me.
> >
> > I guess the question is whether we would want to allow folks to consume
> > the HMAT inside the kernel while not reporting it out via
> > HMEM_REPORTING. We have some in-kernel users of the HMAT lined up like
> > mitigations for memory-side caches.
> >
> > It's certainly possible that folks would want to consume those
> > mitigations without anything in sysfs. They might not even want or need
> > NUMA support itself, for instance.
> >
> > So, what should we do?
> >
> > config HMEM_REPORTING
> > bool # no user-visible prompt
> > default y if ACPI_HMAT
> >
> > So folks can override in their .config, but they don't see a prompt?
>
> I would add an "&& ACPI_NUMA" to that default as well.
But ACPI_HMAT depends on ACPI_NUMA already, or am I missing anything?
^ permalink raw reply
* Re: [PATCHv6 07/10] acpi/hmat: Register processor domain to its memory
From: Dan Williams @ 2019-02-20 22:13 UTC (permalink / raw)
To: Dave Hansen
Cc: Rafael J. Wysocki, Keith Busch, Linux Kernel Mailing List,
ACPI Devel Maling List, Linux Memory Management List, Linux API,
Greg Kroah-Hartman
In-Reply-To: <9ab5d6ba-4cb6-a6f1-894d-d79b77c8bc21@intel.com>
On Wed, Feb 20, 2019 at 2:11 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 2/20/19 2:02 PM, Rafael J. Wysocki wrote:
> >> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> >> index c9637e2e7514..08e972ead159 100644
> >> --- a/drivers/acpi/hmat/Kconfig
> >> +++ b/drivers/acpi/hmat/Kconfig
> >> @@ -2,6 +2,7 @@
> >> config ACPI_HMAT
> >> bool "ACPI Heterogeneous Memory Attribute Table Support"
> >> depends on ACPI_NUMA
> >> + select HMEM_REPORTING
> > If you want to do this here, I'm not sure that defining HMEM_REPORTING
> > as a user-selectable option is a good idea. In particular, I don't
> > really think that setting ACPI_HMAT without it makes a lot of sense.
> > Apart from this, the patch looks reasonable to me.
>
> I guess the question is whether we would want to allow folks to consume
> the HMAT inside the kernel while not reporting it out via
> HMEM_REPORTING. We have some in-kernel users of the HMAT lined up like
> mitigations for memory-side caches.
>
> It's certainly possible that folks would want to consume those
> mitigations without anything in sysfs. They might not even want or need
> NUMA support itself, for instance.
>
> So, what should we do?
>
> config HMEM_REPORTING
> bool # no user-visible prompt
> default y if ACPI_HMAT
>
> So folks can override in their .config, but they don't see a prompt?
I would add an "&& ACPI_NUMA" to that default as well.
^ permalink raw reply
* Re: [PATCHv6 07/10] acpi/hmat: Register processor domain to its memory
From: Dave Hansen @ 2019-02-20 22:11 UTC (permalink / raw)
To: Rafael J. Wysocki, Keith Busch
Cc: Linux Kernel Mailing List, ACPI Devel Maling List,
Linux Memory Management List, Linux API, Greg Kroah-Hartman,
Dan Williams
In-Reply-To: <CAJZ5v0gjv0DZvYMTPBLnUmMtu8=g0zFd4x-cpP11Kzv+6XCwUw@mail.gmail.com>
On 2/20/19 2:02 PM, Rafael J. Wysocki wrote:
>> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
>> index c9637e2e7514..08e972ead159 100644
>> --- a/drivers/acpi/hmat/Kconfig
>> +++ b/drivers/acpi/hmat/Kconfig
>> @@ -2,6 +2,7 @@
>> config ACPI_HMAT
>> bool "ACPI Heterogeneous Memory Attribute Table Support"
>> depends on ACPI_NUMA
>> + select HMEM_REPORTING
> If you want to do this here, I'm not sure that defining HMEM_REPORTING
> as a user-selectable option is a good idea. In particular, I don't
> really think that setting ACPI_HMAT without it makes a lot of sense.
> Apart from this, the patch looks reasonable to me.
I guess the question is whether we would want to allow folks to consume
the HMAT inside the kernel while not reporting it out via
HMEM_REPORTING. We have some in-kernel users of the HMAT lined up like
mitigations for memory-side caches.
It's certainly possible that folks would want to consume those
mitigations without anything in sysfs. They might not even want or need
NUMA support itself, for instance.
So, what should we do?
config HMEM_REPORTING
bool # no user-visible prompt
default y if ACPI_HMAT
So folks can override in their .config, but they don't see a prompt?
^ permalink raw reply
* Re: [PATCHv6 09/10] acpi/hmat: Register memory side cache attributes
From: Rafael J. Wysocki @ 2019-02-20 22:05 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Kernel Mailing List, ACPI Devel Maling List,
Linux Memory Management List, Linux API, Greg Kroah-Hartman,
Rafael Wysocki, Dave Hansen, Dan Williams
In-Reply-To: <20190214171017.9362-10-keith.busch@intel.com>
On Thu, Feb 14, 2019 at 6:10 PM Keith Busch <keith.busch@intel.com> wrote:
>
> Register memory side cache attributes with the memory's node if HMAT
> provides the side cache iniformation table.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/acpi/hmat/hmat.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/acpi/hmat/hmat.c b/drivers/acpi/hmat/hmat.c
> index 6833c4897ff4..e2a15f53fe45 100644
> --- a/drivers/acpi/hmat/hmat.c
> +++ b/drivers/acpi/hmat/hmat.c
> @@ -314,6 +314,7 @@ static __init int hmat_parse_cache(union acpi_subtable_headers *header,
> const unsigned long end)
> {
> struct acpi_hmat_cache *cache = (void *)header;
> + struct node_cache_attrs cache_attrs;
> u32 attrs;
>
> if (cache->header.length < sizeof(*cache)) {
> @@ -327,6 +328,37 @@ static __init int hmat_parse_cache(union acpi_subtable_headers *header,
> cache->memory_PD, cache->cache_size, attrs,
> cache->number_of_SMBIOShandles);
>
> + cache_attrs.size = cache->cache_size;
> + cache_attrs.level = (attrs & ACPI_HMAT_CACHE_LEVEL) >> 4;
> + cache_attrs.line_size = (attrs & ACPI_HMAT_CACHE_LINE_SIZE) >> 16;
> +
> + switch ((attrs & ACPI_HMAT_CACHE_ASSOCIATIVITY) >> 8) {
> + case ACPI_HMAT_CA_DIRECT_MAPPED:
> + cache_attrs.associativity = NODE_CACHE_DIRECT_MAP;
> + break;
> + case ACPI_HMAT_CA_COMPLEX_CACHE_INDEXING:
> + cache_attrs.associativity = NODE_CACHE_INDEXED;
> + break;
> + case ACPI_HMAT_CA_NONE:
> + default:
> + cache_attrs.associativity = NODE_CACHE_OTHER;
> + break;
> + }
> +
> + switch ((attrs & ACPI_HMAT_WRITE_POLICY) >> 12) {
> + case ACPI_HMAT_CP_WB:
> + cache_attrs.write_policy = NODE_CACHE_WRITE_BACK;
> + break;
> + case ACPI_HMAT_CP_WT:
> + cache_attrs.write_policy = NODE_CACHE_WRITE_THROUGH;
> + break;
> + case ACPI_HMAT_CP_NONE:
> + default:
> + cache_attrs.write_policy = NODE_CACHE_WRITE_OTHER;
> + break;
> + }
> +
> + node_add_cache(pxm_to_node(cache->memory_PD), &cache_attrs);
> return 0;
> }
>
> --
> 2.14.4
>
^ permalink raw reply
* Re: [PATCHv6 08/10] acpi/hmat: Register performance attributes
From: Rafael J. Wysocki @ 2019-02-20 22:04 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Kernel Mailing List, ACPI Devel Maling List,
Linux Memory Management List, Linux API, Greg Kroah-Hartman,
Rafael Wysocki, Dave Hansen, Dan Williams
In-Reply-To: <20190214171017.9362-9-keith.busch@intel.com>
On Thu, Feb 14, 2019 at 6:10 PM Keith Busch <keith.busch@intel.com> wrote:
>
> Save the best performance access attributes and register these with the
> memory's node if HMAT provides the locality table. While HMAT does make
> it possible to know performance for all possible initiator-target
> pairings, we export only the local pairings at this time.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/acpi/hmat/hmat.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/hmat/hmat.c b/drivers/acpi/hmat/hmat.c
> index b29f7160c7bb..6833c4897ff4 100644
> --- a/drivers/acpi/hmat/hmat.c
> +++ b/drivers/acpi/hmat/hmat.c
> @@ -549,12 +549,27 @@ static __init void hmat_register_target_initiators(struct memory_target *target)
> }
> }
>
> +static __init void hmat_register_target_perf(struct memory_target *target)
> +{
> + unsigned mem_nid = pxm_to_node(target->memory_pxm);
> +
> + if (!target->hmem_attrs.read_bandwidth &&
> + !target->hmem_attrs.read_latency &&
> + !target->hmem_attrs.write_bandwidth &&
> + !target->hmem_attrs.write_latency)
> + return;
> +
> + node_set_perf_attrs(mem_nid, &target->hmem_attrs, 0);
> +}
> +
> static __init void hmat_register_targets(void)
> {
> struct memory_target *target;
>
> - list_for_each_entry(target, &targets, node)
> + list_for_each_entry(target, &targets, node) {
> hmat_register_target_initiators(target);
> + hmat_register_target_perf(target);
> + }
> }
>
> static __init void hmat_free_structures(void)
> --
> 2.14.4
>
^ permalink raw reply
* Re: [PATCHv6 07/10] acpi/hmat: Register processor domain to its memory
From: Rafael J. Wysocki @ 2019-02-20 22:02 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Kernel Mailing List, ACPI Devel Maling List,
Linux Memory Management List, Linux API, Greg Kroah-Hartman,
Rafael Wysocki, Dave Hansen, Dan Williams
In-Reply-To: <20190214171017.9362-8-keith.busch@intel.com>
On Thu, Feb 14, 2019 at 6:10 PM Keith Busch <keith.busch@intel.com> wrote:
>
> If the HMAT Subsystem Address Range provides a valid processor proximity
> domain for a memory domain, or a processor domain matches the performance
> access of the valid processor proximity domain, register the memory
> target with that initiator so this relationship will be visible under
> the node's sysfs directory.
>
> By registering only the best performing relationships, this provides the
> most useful information applications may want to know when considering
> which CPU they should run on for a given memory node, or which memory
> node they should allocate memory from for a given CPU.
>
> Since HMAT requires valid address ranges have an equivalent SRAT entry,
> verify each memory target satisfies this requirement.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
> drivers/acpi/hmat/Kconfig | 1 +
> drivers/acpi/hmat/hmat.c | 396 +++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 396 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> index c9637e2e7514..08e972ead159 100644
> --- a/drivers/acpi/hmat/Kconfig
> +++ b/drivers/acpi/hmat/Kconfig
> @@ -2,6 +2,7 @@
> config ACPI_HMAT
> bool "ACPI Heterogeneous Memory Attribute Table Support"
> depends on ACPI_NUMA
> + select HMEM_REPORTING
If you want to do this here, I'm not sure that defining HMEM_REPORTING
as a user-selectable option is a good idea. In particular, I don't
really think that setting ACPI_HMAT without it makes a lot of sense.
Apart from this, the patch looks reasonable to me.
^ permalink raw reply
* Re: [RFC PATCH 00/27] Containers and using authenticated filesystems
From: Steve French @ 2019-02-20 18:54 UTC (permalink / raw)
To: David Howells
Cc: Eric W. Biederman, keyrings, trond.myklebust, Steve French,
linux-security-module, linux-nfs, CIFS, linux-fsdevel, rgb, LKML,
Linux Containers, Linux API, samba-technical
In-Reply-To: <22055.1550619729@warthog.procyon.org.uk>
On Tue, Feb 19, 2019 at 5:42 PM David Howells <dhowells@redhat.com> wrote:
>
> Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> > So you missed the main mailing lists for discussion of this kind of
> > thing
>
> Yeah, sorry about that. I was primarily aiming it at Trond and Steve as I'd
> like to consider how to go about interpolating request_key() into NFS and CIFS
> so that they can make use of the key-related facilities that this makes
> available with AFS.
I am interested in this discussion because I have gotten various questions
about using Containers better on SMB3 mounts, and the question about
doing request_key better comes up **a lot** on SMB3 mounts (not just
for kerberos, Active Directory), and usability could be improved of some
of the cifs-utils that cifs.ko depends on.
Note that various virtualization/container identify features were added to the
protocol a few years ago (which we don't yet implement in Linux) but which
probably be **very** useful to followup on how these could be exposed
to help containers on network mounts in Linux. See in particular this
new protocol feature (implemented by various servers including Windows
but not by Linux client yet) described in the protocol spec (MS-SMB2 section
2.2.9.2.1) - the "SMB2_REMOTED_IDENTITY_TREE_CONNECT context"
which can be sent at mount time:
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/ee7ff411-93e0-484f-9f73-31916fee4cb8
This may be of interest to Samba server developers as well
> > and the maintainer.
>
> That would be me. I maintain keyrings.
--
Thanks,
Steve
^ permalink raw reply
* Re: [RFC PATCH v3 00/18] fscrypt: key management improvements
From: Eric Biggers @ 2019-02-20 18:36 UTC (permalink / raw)
To: David Howells
Cc: linux-ext4, linux-api, linux-f2fs-devel, linux-fscrypt, keyrings,
linux-mtd, linux-crypto, linux-fsdevel, Satya Tangirala,
Paul Crowley
In-Reply-To: <14023.1550686042@warthog.procyon.org.uk>
Hi David,
On Wed, Feb 20, 2019 at 06:07:22PM +0000, David Howells wrote:
> I have a couple of patches that add ACLs to keyrings, that you can find at the
> top of the branch here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-acl
>
> I have other patches that allow tags to be used as subjects in the ACL, with a
> container supplying a tag, e.g.:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=container&id=e0fdc5613a32b9475b025f58e7a2267329b3f3a4
>
> Might something similar be of use to you here, perhaps specifying a tag
> referring to the blockdev of interest rather than the container?
>
> David
I don't think so. The main point of adding keys directly to the filesystem is
that it's generally inappropriate to apply OS-level access control or
process-based visibility restrictions to fscrypt keys given that:
- The unlocked/locked status of files is already filesystem-level.
- The purpose of encryption is orthogonal to OS-level access control.
The ioctl based interface also makes it *much* easier to implement all the
semantics needed for removing fscrypt keys.
I don't see how key ACLs would be appropriate here, except that key ACLs would
allow userspace to opt-in to fixing the denial of service vulnerability where
keyctl_invalidate() only requires Search permission. But for fscrypt that's
addressed by my proposal too, and is just one of many problems it addresses.
- Eric
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCHv6 00/10] Heterogenous memory node attributes
From: Keith Busch @ 2019-02-20 18:25 UTC (permalink / raw)
To: linux-kernel, linux-acpi, linux-mm, linux-api
Cc: Greg Kroah-Hartman, Rafael Wysocki, Dave Hansen, Dan Williams
In-Reply-To: <20190214171017.9362-1-keith.busch@intel.com>
On Thu, Feb 14, 2019 at 10:10:07AM -0700, Keith Busch wrote:
> Platforms may provide multiple types of cpu attached system memory. The
> memory ranges for each type may have different characteristics that
> applications may wish to know about when considering what node they want
> their memory allocated from.
>
> It had previously been difficult to describe these setups as memory
> rangers were generally lumped into the NUMA node of the CPUs. New
> platform attributes have been created and in use today that describe
> the more complex memory hierarchies that can be created.
>
> This series' objective is to provide the attributes from such systems
> that are useful for applications to know about, and readily usable with
> existing tools and libraries. Those applications may query performance
> attributes relative to a particular CPU they're running on in order to
> make more informed choices for where they want to allocate hot and cold
> data. This works with mbind() or the numactl library.
Hi all,
So this seems very calm at this point. Unless there are any late concerns
or suggestions, could we open consideration for queueing in a staging
tree for a future merge window?
Thanks,
Keith
> Keith Busch (10):
> acpi: Create subtable parsing infrastructure
> acpi: Add HMAT to generic parsing tables
> acpi/hmat: Parse and report heterogeneous memory
> node: Link memory nodes to their compute nodes
> node: Add heterogenous memory access attributes
> node: Add memory-side caching attributes
> acpi/hmat: Register processor domain to its memory
> acpi/hmat: Register performance attributes
> acpi/hmat: Register memory side cache attributes
> doc/mm: New documentation for memory performance
>
> Documentation/ABI/stable/sysfs-devices-node | 89 +++-
> Documentation/admin-guide/mm/numaperf.rst | 164 +++++++
> arch/arm64/kernel/acpi_numa.c | 2 +-
> arch/arm64/kernel/smp.c | 4 +-
> arch/ia64/kernel/acpi.c | 12 +-
> arch/x86/kernel/acpi/boot.c | 36 +-
> drivers/acpi/Kconfig | 1 +
> drivers/acpi/Makefile | 1 +
> drivers/acpi/hmat/Kconfig | 9 +
> drivers/acpi/hmat/Makefile | 1 +
> drivers/acpi/hmat/hmat.c | 677 ++++++++++++++++++++++++++
> drivers/acpi/numa.c | 16 +-
> drivers/acpi/scan.c | 4 +-
> drivers/acpi/tables.c | 76 ++-
> drivers/base/Kconfig | 8 +
> drivers/base/node.c | 351 ++++++++++++-
> drivers/irqchip/irq-gic-v2m.c | 2 +-
> drivers/irqchip/irq-gic-v3-its-pci-msi.c | 2 +-
> drivers/irqchip/irq-gic-v3-its-platform-msi.c | 2 +-
> drivers/irqchip/irq-gic-v3-its.c | 6 +-
> drivers/irqchip/irq-gic-v3.c | 10 +-
> drivers/irqchip/irq-gic.c | 4 +-
> drivers/mailbox/pcc.c | 2 +-
> include/linux/acpi.h | 6 +-
> include/linux/node.h | 60 ++-
> 25 files changed, 1480 insertions(+), 65 deletions(-)
> create mode 100644 Documentation/admin-guide/mm/numaperf.rst
> create mode 100644 drivers/acpi/hmat/Kconfig
> create mode 100644 drivers/acpi/hmat/Makefile
> create mode 100644 drivers/acpi/hmat/hmat.c
^ permalink raw reply
* Re: [RFC PATCH v3 00/18] fscrypt: key management improvements
From: David Howells @ 2019-02-20 18:07 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-ext4, linux-api, linux-f2fs-devel, dhowells, linux-fscrypt,
keyrings, linux-mtd, linux-crypto, linux-fsdevel, Satya Tangirala,
Paul Crowley
In-Reply-To: <20190220065249.32099-1-ebiggers@kernel.org>
I have a couple of patches that add ACLs to keyrings, that you can find at the
top of the branch here:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-acl
I have other patches that allow tags to be used as subjects in the ACL, with a
container supplying a tag, e.g.:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=container&id=e0fdc5613a32b9475b025f58e7a2267329b3f3a4
Might something similar be of use to you here, perhaps specifying a tag
referring to the blockdev of interest rather than the container?
David
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Nicolai Stange @ 2019-02-20 15:49 UTC (permalink / raw)
To: Nicolai Stange
Cc: Vlastimil Babka, Kirill A. Shutemov, Tejun Heo, Kevin Easton,
Cyril Hrubis, Daniel Gruss, Andy Lutomirski, Linus Torvalds,
Dave Chinner, Dominique Martinet, Jiri Kosina, Matthew Wilcox,
Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko,
Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wj+xyz_GKjgKpU6SF3qeqouGmRoR8uFxzg_c1VpeGEJMw@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
<snip>
> So in order to use it as a signal, first you have to first scrub the
> cache (because if the page was already there, there's no signal at
> all), and then for the signal to be as useful as possible, you're also
> going to want to try to get out more than one bit of information: you
> are going to try to see the patterns and the timings of how it gets
> filled.
>
> And that's actually quite painful. You don't know the initial cache
> state, and you're not (in general) controlling the machine entirely,
> because there's also that actual other entity that you're trying to
> attack and see what it does.
>
> So what you want to do is basically to first make sure the cache is
> scrubbed (only for the pages you're interested in!), then trigger
> whatever behavior you are looking for, and then look how that affected
> the cache.
>
> In other words, you want *multiple* residency status check - first to
> see what the cache state is (because you're going to want that for
> scrubbing), then to see that "yes, it's gone" when doing the
> scrubbing, and then to see the *pattern* and timings of how things are
> brought in.
<snap>
In an attempt to gain a better understanding of the guided eviction part
resp. the relevance of mincore() & friends to that, I worked on
reproducing the results from [1], section 6.1 ("Efficient Page Cache
Eviction on Linux").
In case anybody wants to run their own experiments: the sources can
be found at [2].
Disclaimer: I don't have access to the sources used by the [1]-paper's
authors nor do I know anything about their experimental setup. So it
might very well be the case, that my implementation is completely
different and/or inefficient.
Anyways, quoting from [1], section 6.1:
"Eviction Set 1. These are pages already in the page cache,
used by other processes. To keep them in the page cache,
a thread continuously accesses these pages while also keep-
ing the system load low by using sched yield and sleep.
Consequently, they are among the most recently accessed
pages of the system and eviction of these pages becomes
highly unlikely."
I had two questions:
1.) Do the actual contents of "Eviction set 1" matter for the guided
eviction's performance or can they as well be arbitrary but fixed?
Because if the set's actual contents weren't of any importance,
then mincore() would not be needed to initialize it with "pages
already in the page cache".
2.) How does keeping some fixed set resident + doing some IO compare to
simply streaming a huge random file through the page cache?
(To make it explicit: I didn't look into the probe part of the attack
or the checking of the victim page's residency status as a
termination condition for the eviction run.)
Obviously, there are two primary factors affecting the victim page
eviction performance: the file page cache size and disk read
bandwidth.
Naively speaking, I would suppose that keeping a certain set resident is
a cheap and stable way to constrain IO to the remaining portion of the
page cache and thus, reduce the amount of data required to be read from
disk until the victim page gets evicted.
Results summary (details can be found at the end of this mail):
- The baseline benchmark of simply streaming random data
through the page cache behaves as expected:
avg of "Inactive(file)" / avg of "victim page eviction time"
yields ~480MB/s, which approx. matches my disk's read bandwidth
(note: the victim page was mapped as !PROT_EXEC).
- I didn't do any sophisticated fine-tuning wrt. to parameters, but
for the configuration yielding the best result, the average victim
page eviction time was 147ms (stddev(*): 69ms, stderr: 1ms) with the
"random but fixed resident set method". That's an improvement by a
factor of 2.6 over the baseline "streaming random data method" (with
the same amount of anonymous memory, i.e. "Eviction set 3",
allocated: 7GB out of a total of 8GB).
- In principle, question 1.) can't be answered by experiment without
controlling the initial, legitimate system workload. I did some lax
tests on my desktop running firefox, libreoffice etc. though and of
course, overall responsiveness got a lot better if the "Eviction set
1" had been populated with pages already resident at the time the
"attack" started. But the victim page eviction times didn't seem to
improve -- at least not by factors such that my biased mind would
have been able to recognize any change.
In conclusion, keeping an arbitrary, but fixed "Eviction set 1" resident
improved the victim page eviction performance by some factor over the
"streaming" baseline, where "Eviction set 1" was populated from a
single, attacker-controlled file and mincore() was not needed for
determining its initial contents.
To my surprise though, I needed to rely on mincore() at some other
place, namely *my* implementation of keeping the resident set
resident. My first naive approach was to have a single thread repeatedly
iterating over the pages and reading the first few bytes from each
through a mmapped area. That did not work out, because, even for smaller
resident sets of 1GB and if run with realtime priority, the accessing
thread would at some point in time encounter a reclaimed page and have
to wait for the page fault to get served. While waiting for that, even
more of the resident pages are likely to get reclaimed, causing
additional delays later on. Eventually, the resident page accessor
looses the game and will encounter page faults for almost the whole
resident set (which isn't resident anymore). I worked around this by
making the accessor thread check page residency via mincore(), touch
only the resident ones and queue the others to some refault ("rewarm")
thread. From briefly looking at iostat, this rewarmer thread actually
seemed to saturate the disk and thus, there was no need for additional
IO to put pressure on the page cache. For completeness, the amount of
pages from the resident set actually found resident made up for ~97% of
all file page cache pages (Inactive+Active(file)).
Note that the way mincore() is used here is different than for the
probe part of the attack: for probing, we'd like to know when a victim
page has been faulted in again, while the residency keeper needs to
check that some page has not been reclaimed before accessing
it. Furthermore, mincore() is run on pages from an attacker-controlled
and -owned file here. AFAICS, the patches currently under review
(c.f. [3]) don't mitigate against this latter abuse of mincore() & Co.
I personally doubt that doing something about it would be worth it
though: first of all, until proven otherwise, I'm assuming that the
improvement of the "resident set" based eviction method over the
"streaming" baseline is not by orders of magnitude and that the victim
page eviction times interesting to attackers (let's say better than
500ms) are achievable only under certain conditions: no swap and the
ability to block a large fraction of total memory with anonymous
mappings.
What's more, I can imagine that there are other ways to keep the
resident set resident without relying on mincore() at all: I haven't
tried it, but simply spawning a larger number of accessor threads, each
at a different position within "Eviction set 1", and hoping that most of
the time at least one of them finds a resident page to touch, might
work, too.
Thanks,
Nicolai
Experiment setup + numbers
==========================
My laptop has 8GB of RAM, a SSD and runs OpenSUSE 42.3 kernel package
version 4.4.165-81. I stopped almost all services, but didn't setup any
CPU isolation.
I ran 5 x 2 experiments where I allocated (and filled, of course)
3,4,5,6,7GB of anonymous memory each and compared the baseline
"read-in-a-huge-file" (mapped PROT_EXEC) results with the resident set
based eviction for each of these. While doing so, I continuously
measured eviction times of a !PROT_EXEC victim page and reported the
'Active(file)' + 'Inactive(file)' statistics from /proc/meminfo.
Baseline "streaming" benchmark results:
anon mem (GB): 7 6 5 4 3
Inactive(file) (MB): 189.3389 709.7063 1221.0910 1735.7510 2247.3340
eviction time (ms): 386.7712 1453.3975 2533.6084 3622.4995 4694.4668
quotient (MB/s): 489.5372 488.3085 481.9573 479.1584 478.7198
and, for comparison with the results from the resident set based eviction
below:
Inactive+Active(file) (MB): 358 1390 2415 3445 4469
Resident set based eviction:
For the results below, I chose to draw the resident set from a single
file filled with random data and made it total mem - allocated anon
mem in size. The disk bandwidth was saturated all the time.
anon mem (GB): 7 6 5 4 3
eviction time (ms): 146.6296 428.9594 620.2084 863.1423 977.8963
improvement over baseline: 2.6 3.4 4.1 4.2 4.8
Inactive+Active(file) (MB): 429 1449 2471 3494 4515
resident from res. set (MB): 417 1428 2426 3424 4429
fraction: 97.3% 98.6% 98.2% 98.0% 98.1%
[1] https://arxiv.org/abs/1901.01161 ("Page Cache Attacks")
[2] https://github.com/nicstange/pgc
[3] https://lkml.kernel.org/r/20190130124420.1834-1-vbabka@suse.cz
(*) The distribution of eviction times is not Gaussian.
--
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [RFC PATCH 00/27] Containers and using authenticated filesystems
From: Christian Brauner @ 2019-02-20 14:18 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David Howells, linux-cifs, linux-nfs, linux-api, Linux Containers,
linux-kernel, sfrench, linux-security-module, keyrings,
linux-fsdevel, trond.myklebust
In-Reply-To: <8736ojybw7.fsf@xmission.com>
On Tue, Feb 19, 2019 at 10:35:20AM -0600, Eric W. Biederman wrote:
>
> So you missed the main mailing lists for discussion of this kind of
> thing, and the maintainer. So I have reservations about the quality of
> your due diligence already.
>
> Looking at your description you are introducing a container id.
> You don't descibe which namespace your contianer id lives in.
> Without the container id living in a container this breaks
> nested containers and process migration aka CRIU.
>
> So based on the your description.
>
> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
>
>
> David Howells <dhowells@redhat.com> writes:
>
> > Here's a collection of patches that containerises the kernel keys and makes
> > it possible to separate keys by namespace. This can be extended to any
> > filesystem that uses request_key() to obtain the pertinent authentication
> > token on entry to VFS or socket methods.
/me puts on kernel hat:
I'm not neccessarily opposed to making containers kernel objects even
though I have been for quite a while (for brevity I'll use "kcontainers"
for this). But I think the approach taken here is a little misguided.
This patchsets pushes the argument that kcontainers are needed because
of keyrings and authenticated filesystems and is designed around this
use-case. Imho, that is bound to fall short of requirements and
use-cases that have been piling up over the years.
If we want to make kcontainers a thing we need to have a separate
discussion and a separate patchset that is *solely* concerned with
creating a kcontainer api. And frankly, that is likely going to take a
long time.
At this point containers have become a real "thing" on Linux - like it
or not. So justifying it to making them in-kernel citizens doesn't need
the detour over keyrings or something else. We should just discuss
whether we think that the benefits of kcontainers (e.g. security)
outweight the costs (e.g. maintenance).
/me puts on runtime maintainer hat:
One thing that is true is that userspace containers (let's call them
"ucontainers") as implemented by runtimes today will not go away. We
have been living with this ad-hoc concept and it's various
implementations on upstream Linux at least since 2008. And kernels
without kcontainers will be with us until the end of (Linux)time
probably. So anyone who thinks that kcontainers will replace ucontainers
and that'll be it will be thoroughly disappointed in the end.
It is also very likely that not all use-cases we can currently cover
with ucontainers can be covered by kcontainers. Now that might be ok but
if we ever introduce kcontainers through a proper kernel api we will end
up maintaining ucontainers and kcontainers simultaneously. That's a
burden we shouldn't underestimate.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox