* [PATCH] pagemap: update documentation
From: Konstantin Khlebnikov @ 2015-07-16 18:47 UTC (permalink / raw)
To: linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Naoya Horiguchi
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150714152516.29844.69929.stgit@buzz>
Notes about recent changes.
Signed-off-by: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
---
Documentation/vm/pagemap.txt | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index 3cfbbb333ea1..aab39aa7dd8f 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -16,12 +16,17 @@ There are three components to pagemap:
* Bits 0-4 swap type if swapped
* Bits 5-54 swap offset if swapped
* Bit 55 pte is soft-dirty (see Documentation/vm/soft-dirty.txt)
- * Bit 56 page exlusively mapped
+ * Bit 56 page exclusively mapped (since 4.2)
* Bits 57-60 zero
- * Bit 61 page is file-page or shared-anon
+ * Bit 61 page is file-page or shared-anon (since 3.5)
* Bit 62 page swapped
* Bit 63 page present
+ Since Linux 4.0 only users with the CAP_SYS_ADMIN capability can get PFNs:
+ for unprivileged users from 4.0 till 4.2 open fails with -EPERM, starting
+ from from 4.2 PFN field is zeroed if user has no CAP_SYS_ADMIN capability.
+ Reason: information about PFNs helps in exploiting Rowhammer vulnerability.
+
If the page is not present but in swap, then the PFN contains an
encoding of the swap file number and the page's offset into the
swap. Unmapped pages return a null PFN. This allows determining
@@ -160,3 +165,8 @@ Other notes:
Reading from any of the files will return -EINVAL if you are not starting
the read on an 8-byte boundary (e.g., if you sought an odd number of bytes
into the file), or if the size of the read is not a multiple of 8 bytes.
+
+Before Linux 3.11 pagemap bits 55-60 were used for "page-shift" (which is
+always 12 at most architectures). Since Linux 3.11 their meaning changes
+after first clear of soft-dirty bits. Since Linux 4.2 they are used for
+flags unconditionally.
^ permalink raw reply related
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Mathieu Desnoyers @ 2015-07-16 18:08 UTC (permalink / raw)
To: Ben Maurer
Cc: Paul Turner, Andrew Hunter, Peter Zijlstra, Ingo Molnar, rostedt,
Paul E. McKenney, Josh Triplett, Lai Jiangshan, Linus Torvalds,
Andrew Morton, linux-api, libc-alpha
In-Reply-To: <5CDDBDF2D36D9F43B9F5E99003F6A0D48D5F5DA0-f8hGUhss0nh9TZdEUguypQ2O0Ztt9esIQQ4Iyu8u01E@public.gmane.org>
----- On Jul 14, 2015, at 5:34 AM, Ben Maurer bmaurer-b10kYP2dOMg@public.gmane.org wrote:
> Mathieu Desnoyers wrote:
>> If we invoke this per-thread registration directly in the glibc NPTL
>> implementation,
>> in start_thread, do you think it would fit your requirements ?
>
> I guess this would basically be transparent to the user -- we'd just need to
> make sure that the registration happens very early, before any chance of
> calling malloc.
Yes, this is my thinking too.
>
> That said, having the ability for the kernel to understand that TLS
> implementation are laid out using the same offset on each thread seems like
> something that could be valuable long term. Doing so makes it possible to build
> other TLS-based features without forcing each thread to be registered.
AFAIU, using a fixed hardcoded ABI between kernel and user-space might make
transition from the pre-existing ABI (where this memory area is not
reserved) a bit tricky without registering the area, or getting a "feature"
flag, through a system call.
The related question then becomes: should we issue this system call once
per process, or once per thread at thread creation ? Issuing it once per
thread is marginally more costly for thread creation, but seems to be
easier to deal with internally within the kernel.
We could however ensure that only a single system call is needed per new-coming
thread, rather than one system call per feature. One way to do this would be
to register an area that may contain more than just the CPU id. It could
consist of an expandable structure with fixed offsets. When registered, we
could pass the size of that structure as an argument to the system call, so
the kernel knows which features are expected by user-space.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: nf_ct_sctp: minimal multihoming support
From: Michal Kubecek @ 2015-07-16 16:13 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
coreteam-Cap9r6Oaw4JrovVCs/uTlw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pablo Neira Ayuso,
Patrick McHardy, Jozsef Kadlecsik, David S. Miller
In-Reply-To: <20150716135059.GB14704-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On Thu, Jul 16, 2015 at 10:50:59AM -0300, Marcelo Ricardo Leitner wrote:
> On Tue, Jul 14, 2015 at 02:23:11PM +0200, Michal Kubecek wrote:
> > @@ -278,6 +292,14 @@ static int sctp_new_state(enum ip_conntrack_dir dir,
> > pr_debug("SCTP_CID_SHUTDOWN_COMPLETE\n");
> > i = 8;
> > break;
> > + case SCTP_CID_HEARTBEAT:
> > + pr_debug("SCTP_CID_HEARTBEAT");
> > + i = 9;
> > + break;
> > + case SCTP_CID_HEARTBEAT_ACK:
> > + pr_debug("SCTP_CID_HEARTBEAT_ACK");
> > + i = 10;
> > + break;
> > default:
> > /* Other chunks like DATA, SACK, HEARTBEAT and
> > its ACK do not cause a change in state */
>
> Would you update this comment on default case please? As with this
> patch, HB and its ACK may cause a change in state.
Thank you for catching this. I'll update the comment in v2 I'm going to
send tomorrow after some testing.
Michal Kubecek
^ permalink raw reply
* [PATCH v7 5/6] block: loop: introduce ioctl command of LOOP_SET_DIRECT_IO
From: Ming Lei @ 2015-07-16 15:37 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: Justin M. Forbes, Jeff Moyer, Tejun Heo, Christoph Hellwig,
Ming Lei, linux-api
In-Reply-To: <1437061068-26118-1-git-send-email-ming.lei@canonical.com>
If loop block is mounted via 'mount -o loop', it isn't easy
to pass file descriptor opened as O_DIRECT, so this patch
introduces a new command to support direct IO for this case.
Cc: linux-api@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/block/loop.c | 19 +++++++++++++++++++
include/uapi/linux/loop.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 35aa3dd..3f26a0a 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1214,6 +1214,20 @@ static int loop_set_capacity(struct loop_device *lo, struct block_device *bdev)
return figure_loop_size(lo, lo->lo_offset, lo->lo_sizelimit);
}
+static int loop_set_dio(struct loop_device *lo, unsigned long arg)
+{
+ int error = -ENXIO;
+ if (lo->lo_state != Lo_bound)
+ goto out;
+
+ __loop_update_dio(lo, !!arg);
+ if (lo->use_dio == !!arg)
+ return 0;
+ error = -EINVAL;
+ out:
+ return error;
+}
+
static int lo_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
{
@@ -1257,6 +1271,11 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
err = loop_set_capacity(lo, bdev);
break;
+ case LOOP_SET_DIRECT_IO:
+ err = -EPERM;
+ if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
+ err = loop_set_dio(lo, arg);
+ break;
default:
err = lo->ioctl ? lo->ioctl(lo, cmd, arg) : -EINVAL;
}
diff --git a/include/uapi/linux/loop.h b/include/uapi/linux/loop.h
index 949851c..c8125ec 100644
--- a/include/uapi/linux/loop.h
+++ b/include/uapi/linux/loop.h
@@ -87,6 +87,7 @@ struct loop_info64 {
#define LOOP_GET_STATUS64 0x4C05
#define LOOP_CHANGE_FD 0x4C06
#define LOOP_SET_CAPACITY 0x4C07
+#define LOOP_SET_DIRECT_IO 0x4C08
/* /dev/loop-control interface */
#define LOOP_CTL_ADD 0x4C80
--
1.9.1
^ permalink raw reply related
* [PATCH v7 4/6] block: loop: prepare for supporing direct IO
From: Ming Lei @ 2015-07-16 15:37 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: Justin M. Forbes, Jeff Moyer, Tejun Heo, Christoph Hellwig,
Ming Lei, linux-api
In-Reply-To: <1437061068-26118-1-git-send-email-ming.lei@canonical.com>
This patches provides one interface for enabling direct IO
from user space:
- userspace(such as losetup) can pass 'file' which is
opened/fcntl as O_DIRECT
Also __loop_update_dio() is introduced to check if direct I/O
can be used on current loop setting.
The last big change is to introduce LO_FLAGS_DIRECT_IO flag
for userspace to know if direct IO is used to access backing
file.
Cc: linux-api@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/block/loop.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-
drivers/block/loop.h | 1 +
include/uapi/linux/loop.h | 1 +
3 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index ce94b92..35aa3dd 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -164,6 +164,49 @@ static loff_t get_loop_size(struct loop_device *lo, struct file *file)
return get_size(lo->lo_offset, lo->lo_sizelimit, file);
}
+static void __loop_update_dio(struct loop_device *lo, bool dio)
+{
+ struct file *file = lo->lo_backing_file;
+ struct inode *inode = file->f_mapping->host;
+ bool use_dio;
+
+ /*
+ * loop block's logical block size is 512, now
+ * we support direct I/O only if the backing
+ * block devices' minimize I/O size is 512 and
+ * the offset is aligned with 512.
+ */
+ if (dio) {
+ if (inode->i_sb->s_bdev &&
+ bdev_io_min(inode->i_sb->s_bdev) == 512 &&
+ !(lo->lo_offset & 511))
+ use_dio = true;
+ else
+ use_dio = false;
+ } else {
+ use_dio = false;
+ }
+
+ if (lo->use_dio == use_dio)
+ return;
+
+ /* flush dirty pages before changing direct IO */
+ vfs_fsync(file, 0);
+
+ /*
+ * The flag of LO_FLAGS_DIRECT_IO is handled similarly with
+ * LO_FLAGS_READ_ONLY, both are set from kernel, and losetup
+ * will get updated by ioctl(LOOP_GET_STATUS)
+ */
+ blk_mq_freeze_queue(lo->lo_queue);
+ lo->use_dio = use_dio;
+ if (use_dio)
+ lo->lo_flags |= LO_FLAGS_DIRECT_IO;
+ else
+ lo->lo_flags &= ~LO_FLAGS_DIRECT_IO;
+ blk_mq_unfreeze_queue(lo->lo_queue);
+}
+
static int
figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
{
@@ -173,8 +216,12 @@ figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
if (unlikely((loff_t)x != size))
return -EFBIG;
- if (lo->lo_offset != offset)
+ if (lo->lo_offset != offset) {
lo->lo_offset = offset;
+
+ /* update dio if lo_offset is changed*/
+ __loop_update_dio(lo, lo->use_dio);
+ }
if (lo->lo_sizelimit != sizelimit)
lo->lo_sizelimit = sizelimit;
set_capacity(lo->lo_disk, x);
@@ -421,6 +468,11 @@ struct switch_request {
struct completion wait;
};
+static inline void loop_update_dio(struct loop_device *lo)
+{
+ __loop_update_dio(lo, io_is_direct(lo->lo_backing_file));
+}
+
/*
* Do the actual switch; called from the BIO completion routine
*/
@@ -441,6 +493,7 @@ static void do_loop_switch(struct loop_device *lo, struct switch_request *p)
mapping->host->i_bdev->bd_block_size : PAGE_SIZE;
lo->old_gfp_mask = mapping_gfp_mask(mapping);
mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
+ loop_update_dio(lo);
}
/*
@@ -627,11 +680,19 @@ static ssize_t loop_attr_partscan_show(struct loop_device *lo, char *buf)
return sprintf(buf, "%s\n", partscan ? "1" : "0");
}
+static ssize_t loop_attr_dio_show(struct loop_device *lo, char *buf)
+{
+ int dio = (lo->lo_flags & LO_FLAGS_DIRECT_IO);
+
+ return sprintf(buf, "%s\n", dio ? "1" : "0");
+}
+
LOOP_ATTR_RO(backing_file);
LOOP_ATTR_RO(offset);
LOOP_ATTR_RO(sizelimit);
LOOP_ATTR_RO(autoclear);
LOOP_ATTR_RO(partscan);
+LOOP_ATTR_RO(dio);
static struct attribute *loop_attrs[] = {
&loop_attr_backing_file.attr,
@@ -639,6 +700,7 @@ static struct attribute *loop_attrs[] = {
&loop_attr_sizelimit.attr,
&loop_attr_autoclear.attr,
&loop_attr_partscan.attr,
+ &loop_attr_dio.attr,
NULL,
};
@@ -783,6 +845,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
blk_queue_flush(lo->lo_queue, REQ_FLUSH);
+ loop_update_dio(lo);
set_capacity(lo->lo_disk, size);
bd_set_size(bdev, size << 9);
loop_sysfs_init(lo);
diff --git a/drivers/block/loop.h b/drivers/block/loop.h
index b6c7d21..d1de221 100644
--- a/drivers/block/loop.h
+++ b/drivers/block/loop.h
@@ -58,6 +58,7 @@ struct loop_device {
struct mutex lo_ctl_mutex;
struct kthread_worker worker;
struct task_struct *worker_task;
+ bool use_dio;
struct request_queue *lo_queue;
struct blk_mq_tag_set tag_set;
diff --git a/include/uapi/linux/loop.h b/include/uapi/linux/loop.h
index e0cecd2..949851c 100644
--- a/include/uapi/linux/loop.h
+++ b/include/uapi/linux/loop.h
@@ -21,6 +21,7 @@ enum {
LO_FLAGS_READ_ONLY = 1,
LO_FLAGS_AUTOCLEAR = 4,
LO_FLAGS_PARTSCAN = 8,
+ LO_FLAGS_DIRECT_IO = 16,
};
#include <asm/posix_types.h> /* for __kernel_old_dev_t */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH nf-next] netfilter: nf_ct_sctp: minimal multihoming support
From: Marcelo Ricardo Leitner @ 2015-07-16 13:50 UTC (permalink / raw)
To: Michal Kubecek
Cc: netfilter-devel, coreteam, linux-api, netdev, linux-kernel,
Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller
In-Reply-To: <20150714122311.8DA8EA0C9A@unicorn.suse.cz>
On Tue, Jul 14, 2015 at 02:23:11PM +0200, Michal Kubecek wrote:
> Currently nf_conntrack_proto_sctp module handles only packets between
> primary addresses used to establish the connection. Any packets between
> secondary addresses are classified as invalid so that usual firewall
> configurations drop them. Allowing HEARTBEAT and HEARTBEAT-ACK chunks to
> establish a new conntrack would allow traffic between secondary
> addresses to pass through. A more sophisticated solution based on the
> addresses advertised in the initial handshake (and possibly also later
> dynamic address addition and removal) would be much harder to implement.
> Moreover, in general we cannot assume to always see the initial
> handshake as it can be routed through a different path.
>
> The patch adds two new conntrack states:
>
> SCTP_CONNTRACK_HB_SENT - a HEARTBEAT chunk seen but not acked
> SCTP_CONNTRACK_HB_ACKED - a HEARTBEAT acked by HEARTBEAT-ACK
>
> State transition rules:
>
> - HB_SENT responds to usual chunks the same way as NONE (so that the
> behaviour changes as little as possible)
> - HB_ACKED responds to usual chunks the same way as ESTABLISHED does,
> except the resulting state is HB_ACKED rather than ESTABLISHED
> - previously existing states except NONE are preserved when HEARTBEAT or
> HEARTBEAT-ACK is seen
> - NONE (in the initial direction) changes to HB_SENT on HEARTBEAT
> and to CLOSED on HEARTBEAT-ACK
> - HB_SENT changes to HB_ACKED on HEARTBEAT-ACK in the reply direction
> - HB_SENT and HB_ACKED are preserved on HEARTBEAT/HEARTBEAT-ACK
> otherwise
>
> Default timeout values for new states are
>
> HB_SENT: 30 seconds (default hb_interval)
> HB_ACKED: 210 seconds (hb_interval * path_max_retry + max_rto)
>
> (We cannot expect to see the shutdown sequence so that the HB_ACKED
> timeout shouldn't be too long.)
>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> ---
> include/uapi/linux/netfilter/nf_conntrack_sctp.h | 2 +
> net/netfilter/nf_conntrack_proto_sctp.c | 110 ++++++++++++++++++-----
> 2 files changed, 90 insertions(+), 22 deletions(-)
>
> diff --git a/include/uapi/linux/netfilter/nf_conntrack_sctp.h b/include/uapi/linux/netfilter/nf_conntrack_sctp.h
> index ceeefe6681b5..3ec7a6082457 100644
> --- a/include/uapi/linux/netfilter/nf_conntrack_sctp.h
> +++ b/include/uapi/linux/netfilter/nf_conntrack_sctp.h
> @@ -13,6 +13,8 @@ enum sctp_conntrack {
> SCTP_CONNTRACK_SHUTDOWN_SENT,
> SCTP_CONNTRACK_SHUTDOWN_RECD,
> SCTP_CONNTRACK_SHUTDOWN_ACK_SENT,
> + SCTP_CONNTRACK_HB_SENT,
> + SCTP_CONNTRACK_HB_ACKED,
> SCTP_CONNTRACK_MAX
> };
>
> diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
> index b45da90fad32..efb6d5b16393 100644
> --- a/net/netfilter/nf_conntrack_proto_sctp.c
> +++ b/net/netfilter/nf_conntrack_proto_sctp.c
> @@ -42,6 +42,8 @@ static const char *const sctp_conntrack_names[] = {
> "SHUTDOWN_SENT",
> "SHUTDOWN_RECD",
> "SHUTDOWN_ACK_SENT",
> + "HEARTBEAT_SENT",
> + "HEARTBEAT_ACKED",
> };
>
> #define SECS * HZ
> @@ -57,6 +59,8 @@ static unsigned int sctp_timeouts[SCTP_CONNTRACK_MAX] __read_mostly = {
> [SCTP_CONNTRACK_SHUTDOWN_SENT] = 300 SECS / 1000,
> [SCTP_CONNTRACK_SHUTDOWN_RECD] = 300 SECS / 1000,
> [SCTP_CONNTRACK_SHUTDOWN_ACK_SENT] = 3 SECS,
> + [SCTP_CONNTRACK_HB_SENT] = 30 SECS,
> + [SCTP_CONNTRACK_HB_ACKED] = 210 SECS,
> };
>
> #define sNO SCTP_CONNTRACK_NONE
> @@ -67,6 +71,8 @@ static unsigned int sctp_timeouts[SCTP_CONNTRACK_MAX] __read_mostly = {
> #define sSS SCTP_CONNTRACK_SHUTDOWN_SENT
> #define sSR SCTP_CONNTRACK_SHUTDOWN_RECD
> #define sSA SCTP_CONNTRACK_SHUTDOWN_ACK_SENT
> +#define sHS SCTP_CONNTRACK_HB_SENT
> +#define sHA SCTP_CONNTRACK_HB_ACKED
> #define sIV SCTP_CONNTRACK_MAX
>
> /*
> @@ -88,6 +94,10 @@ SHUTDOWN_ACK_SENT - We have seen a SHUTDOWN_ACK chunk in the direction opposite
> to that of the SHUTDOWN chunk.
> CLOSED - We have seen a SHUTDOWN_COMPLETE chunk in the direction of
> the SHUTDOWN chunk. Connection is closed.
> +HEARTBEAT_SENT - We have seen a HEARTBEAT in a new flow.
> +HEARTBEAT_ACKED - We have seen a HEARTBEAT-ACK in the direction opposite to
> + that of the HEARTBEAT chunk. Secondary connection is
> + established.
> */
>
> /* TODO
> @@ -97,36 +107,40 @@ CLOSED - We have seen a SHUTDOWN_COMPLETE chunk in the direction of
> - Check the error type in the reply dir before transitioning from
> cookie echoed to closed.
> - Sec 5.2.4 of RFC 2960
> - - Multi Homing support.
> + - Full Multi Homing support.
> */
>
> /* SCTP conntrack state transitions */
> -static const u8 sctp_conntracks[2][9][SCTP_CONNTRACK_MAX] = {
> +static const u8 sctp_conntracks[2][11][SCTP_CONNTRACK_MAX] = {
> {
> /* ORIGINAL */
> -/* sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA */
> -/* init */ {sCW, sCW, sCW, sCE, sES, sSS, sSR, sSA},
> -/* init_ack */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA},
> -/* abort */ {sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL},
> -/* shutdown */ {sCL, sCL, sCW, sCE, sSS, sSS, sSR, sSA},
> -/* shutdown_ack */ {sSA, sCL, sCW, sCE, sES, sSA, sSA, sSA},
> -/* error */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA},/* Can't have Stale cookie*/
> -/* cookie_echo */ {sCL, sCL, sCE, sCE, sES, sSS, sSR, sSA},/* 5.2.4 - Big TODO */
> -/* cookie_ack */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA},/* Can't come in orig dir */
> -/* shutdown_comp*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sCL}
> +/* sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA */
> +/* init */ {sCW, sCW, sCW, sCE, sES, sSS, sSR, sSA, sCW, sHA},
> +/* init_ack */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},
> +/* abort */ {sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL},
> +/* shutdown */ {sCL, sCL, sCW, sCE, sSS, sSS, sSR, sSA, sCL, sSS},
> +/* shutdown_ack */ {sSA, sCL, sCW, sCE, sES, sSA, sSA, sSA, sSA, sHA},
> +/* error */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* Can't have Stale cookie*/
> +/* cookie_echo */ {sCL, sCL, sCE, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* 5.2.4 - Big TODO */
> +/* cookie_ack */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* Can't come in orig dir */
> +/* shutdown_comp*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sCL, sCL, sHA},
> +/* heartbeat */ {sHS, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA},
> +/* heartbeat_ack*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA}
> },
> {
> /* REPLY */
> -/* sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA */
> -/* init */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA},/* INIT in sCL Big TODO */
> -/* init_ack */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA},
> -/* abort */ {sIV, sCL, sCL, sCL, sCL, sCL, sCL, sCL},
> -/* shutdown */ {sIV, sCL, sCW, sCE, sSR, sSS, sSR, sSA},
> -/* shutdown_ack */ {sIV, sCL, sCW, sCE, sES, sSA, sSA, sSA},
> -/* error */ {sIV, sCL, sCW, sCL, sES, sSS, sSR, sSA},
> -/* cookie_echo */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA},/* Can't come in reply dir */
> -/* cookie_ack */ {sIV, sCL, sCW, sES, sES, sSS, sSR, sSA},
> -/* shutdown_comp*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sCL}
> +/* sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA */
> +/* init */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},/* INIT in sCL Big TODO */
> +/* init_ack */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},
> +/* abort */ {sIV, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sIV, sCL},
> +/* shutdown */ {sIV, sCL, sCW, sCE, sSR, sSS, sSR, sSA, sIV, sSR},
> +/* shutdown_ack */ {sIV, sCL, sCW, sCE, sES, sSA, sSA, sSA, sIV, sHA},
> +/* error */ {sIV, sCL, sCW, sCL, sES, sSS, sSR, sSA, sIV, sHA},
> +/* cookie_echo */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},/* Can't come in reply dir */
> +/* cookie_ack */ {sIV, sCL, sCW, sES, sES, sSS, sSR, sSA, sIV, sHA},
> +/* shutdown_comp*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sCL, sIV, sHA},
> +/* heartbeat */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA},
> +/* heartbeat_ack*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHA, sHA}
> }
> };
>
> @@ -278,6 +292,14 @@ static int sctp_new_state(enum ip_conntrack_dir dir,
> pr_debug("SCTP_CID_SHUTDOWN_COMPLETE\n");
> i = 8;
> break;
> + case SCTP_CID_HEARTBEAT:
> + pr_debug("SCTP_CID_HEARTBEAT");
> + i = 9;
> + break;
> + case SCTP_CID_HEARTBEAT_ACK:
> + pr_debug("SCTP_CID_HEARTBEAT_ACK");
> + i = 10;
> + break;
> default:
> /* Other chunks like DATA, SACK, HEARTBEAT and
> its ACK do not cause a change in state */
Would you update this comment on default case please? As with this
patch, HB and its ACK may cause a change in state.
Other than this, sctp-wise, patch looks good to me. Thanks Michal.
Marcelo
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: nf_ct_sctp: minimal multihoming support
From: Marcelo Ricardo Leitner @ 2015-07-16 13:18 UTC (permalink / raw)
To: Michal Kubecek
Cc: Florian Westphal, netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
coreteam-Cap9r6Oaw4JrovVCs/uTlw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pablo Neira Ayuso,
Patrick McHardy, Jozsef Kadlecsik, David S. Miller
In-Reply-To: <20150716120512.GA7200-OEaqT8BN2ewCVLCxKZUutA@public.gmane.org>
On Thu, Jul 16, 2015 at 02:05:12PM +0200, Michal Kubecek wrote:
> On Wed, Jul 15, 2015 at 05:35:08PM -0300, Marcelo Ricardo Leitner wrote:
> > Hi,
> >
> > On Tue, Jul 14, 2015 at 06:42:25PM +0200, Michal Kubecek wrote:
> > > On Tue, Jul 14, 2015 at 03:42:03PM +0200, Florian Westphal wrote:
> > > > Michal Kubecek <mkubecek-AlSwsSmVLrQ@public.gmane.org> wrote:
> > > > > + case SCTP_CID_HEARTBEAT:
> > > > > + pr_debug("SCTP_CID_HEARTBEAT");
> > > > > + i = 9;
> > > > > + break;
> > > > > + case SCTP_CID_HEARTBEAT_ACK:
> > > > > + pr_debug("SCTP_CID_HEARTBEAT_ACK");
> > > > > + i = 10;
> > > > > + break;
> > > > > default:
> > > > > /* Other chunks like DATA, SACK, HEARTBEAT and
> > > > > its ACK do not cause a change in state */
> > > > > @@ -329,6 +351,8 @@ static int sctp_packet(struct nf_conn *ct,
> > > > > !test_bit(SCTP_CID_COOKIE_ECHO, map) &&
> > > > > !test_bit(SCTP_CID_ABORT, map) &&
> > > > > !test_bit(SCTP_CID_SHUTDOWN_ACK, map) &&
> > > > > + !test_bit(SCTP_CID_HEARTBEAT, map) &&
> > > > > + !test_bit(SCTP_CID_HEARTBEAT_ACK, map) &&
> > > > > sh->vtag != ct->proto.sctp.vtag[dir]) {
> > > > > pr_debug("Verification tag check failed\n");
> > > > > goto out;
> > > > > @@ -357,6 +381,16 @@ static int sctp_packet(struct nf_conn *ct,
> > > > > /* Sec 8.5.1 (D) */
> > > > > if (sh->vtag != ct->proto.sctp.vtag[dir])
> > > > > goto out_unlock;
> > > > > + } else if (sch->type == SCTP_CID_HEARTBEAT ||
> > > > > + sch->type == SCTP_CID_HEARTBEAT_ACK) {
> > > > > + if (ct->proto.sctp.vtag[dir] == 0) {
> > > > > + pr_debug("Setting vtag %x for dir %d\n",
> > > > > + sh->vtag, dir);
> > > > > + ct->proto.sctp.vtag[dir] = sh->vtag;
> > > >
> > > > Could you please elaborate on the [dir] == 0 test?
> > > >
> > > > I see this might happen for SCTP_CID_HEARTBEAT_ACK, but why is this
> > > > needed for SCTP_CID_HEARTBEAT ?
> > > >
> > > > We found a conntrack entry so shouldn't the vtag[dir] already be > 0?
> > >
> > > Yes, you are right. This was originally intended to handle the case when
> > > a HEARTBEAT in the reply direction is seen before the HEARTBEAT-ACK but
> > > such HEARTBEAT would be dropped anyway in current version.
> >
> > And we have to keep the first vtag attempted because otherwise an
> > attacker could just probe for the right one until she gets a reply.
> >
> > IOW, if a different vtag is attempted, we should drop it as the packet
> > doesn't belong to that association/conntrack entry.
> >
> > As vtags are always != 0 in such case, that's a way to know if we
> > already have that information or not.
> >
> > > On the other hand, an alternative would be
> > >
> > > } else if (sch->type == SCTP_CID_HEARTBEAT_ACK &&
> > > ct->proto.sctp.vtag[dir] == 0) {
> > > pr_debug("Setting vtag %x for dir %d\n",
> > > sh->vtag, dir);
> > > ct->proto.sctp.vtag[dir] = sh->vtag;
> > > } else if ((sch->type == SCTP_CID_HEARTBEAT ||
> > > sch->type == SCTP_CID_HEARTBEAT_ACK) &&
> > > sh->vtag != ct->proto.sctp.vtag[dir]) {
> > > pr_debug("Verification tag check failed\n");
> > > goto out_unlock;
> > > }
> > >
> > > I'm not sure it looks better.
> >
> > Now it seems swapped, we should save the tag on HB and check on
> > HB_ACK only and would have to check against !dir entry. Like:
>
> I forgot to include the explanation of vtag setting/checking logic into
> the commit message. It is supposed to work like this:
>
> Normally, vtag is set from the INIT chunk for the reply direction and
> from the INIT-ACK chunk for the originating direction (i.e. each of
> these defines vtag value for the opposite direction). For secondary
Erf, indeed. I totally confused it and thought they would be equal on
both directions.
> conntracks, we can't rely on seeing INIT/INIT-ACK and even if we have
> seen them, we would need to connect two different conntracks. Therefore
> simplified logic is applied: vtag of first packet in each direction
> (HEARTBEAT in the originating and HEARTBEAT-ACK in reply direction) is
> saved and all following packets in that direction are compared with this
> saved value. While INIT and INIT-ACK define vtag for the opposite
> direction (that's where "!dir" comes from), vtags extracted from
> HEARTBEAT and HEARTBEAT-ACK are always for their direction. And we have
> to check vtags on packets with HEARTBEAT chunks as well because their
> vtags should match vtag of the first (set in sctp_new()).
Yes, that's pretty much it. Original code reads better here then.
Thanks,
Marcelo
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: nf_ct_sctp: minimal multihoming support
From: Michal Kubecek @ 2015-07-16 12:05 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Florian Westphal, netfilter-devel, coreteam, linux-api, netdev,
linux-kernel, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller
In-Reply-To: <20150715203508.GA14704@localhost.localdomain>
On Wed, Jul 15, 2015 at 05:35:08PM -0300, Marcelo Ricardo Leitner wrote:
> Hi,
>
> On Tue, Jul 14, 2015 at 06:42:25PM +0200, Michal Kubecek wrote:
> > On Tue, Jul 14, 2015 at 03:42:03PM +0200, Florian Westphal wrote:
> > > Michal Kubecek <mkubecek@suse.cz> wrote:
> > > > + case SCTP_CID_HEARTBEAT:
> > > > + pr_debug("SCTP_CID_HEARTBEAT");
> > > > + i = 9;
> > > > + break;
> > > > + case SCTP_CID_HEARTBEAT_ACK:
> > > > + pr_debug("SCTP_CID_HEARTBEAT_ACK");
> > > > + i = 10;
> > > > + break;
> > > > default:
> > > > /* Other chunks like DATA, SACK, HEARTBEAT and
> > > > its ACK do not cause a change in state */
> > > > @@ -329,6 +351,8 @@ static int sctp_packet(struct nf_conn *ct,
> > > > !test_bit(SCTP_CID_COOKIE_ECHO, map) &&
> > > > !test_bit(SCTP_CID_ABORT, map) &&
> > > > !test_bit(SCTP_CID_SHUTDOWN_ACK, map) &&
> > > > + !test_bit(SCTP_CID_HEARTBEAT, map) &&
> > > > + !test_bit(SCTP_CID_HEARTBEAT_ACK, map) &&
> > > > sh->vtag != ct->proto.sctp.vtag[dir]) {
> > > > pr_debug("Verification tag check failed\n");
> > > > goto out;
> > > > @@ -357,6 +381,16 @@ static int sctp_packet(struct nf_conn *ct,
> > > > /* Sec 8.5.1 (D) */
> > > > if (sh->vtag != ct->proto.sctp.vtag[dir])
> > > > goto out_unlock;
> > > > + } else if (sch->type == SCTP_CID_HEARTBEAT ||
> > > > + sch->type == SCTP_CID_HEARTBEAT_ACK) {
> > > > + if (ct->proto.sctp.vtag[dir] == 0) {
> > > > + pr_debug("Setting vtag %x for dir %d\n",
> > > > + sh->vtag, dir);
> > > > + ct->proto.sctp.vtag[dir] = sh->vtag;
> > >
> > > Could you please elaborate on the [dir] == 0 test?
> > >
> > > I see this might happen for SCTP_CID_HEARTBEAT_ACK, but why is this
> > > needed for SCTP_CID_HEARTBEAT ?
> > >
> > > We found a conntrack entry so shouldn't the vtag[dir] already be > 0?
> >
> > Yes, you are right. This was originally intended to handle the case when
> > a HEARTBEAT in the reply direction is seen before the HEARTBEAT-ACK but
> > such HEARTBEAT would be dropped anyway in current version.
>
> And we have to keep the first vtag attempted because otherwise an
> attacker could just probe for the right one until she gets a reply.
>
> IOW, if a different vtag is attempted, we should drop it as the packet
> doesn't belong to that association/conntrack entry.
>
> As vtags are always != 0 in such case, that's a way to know if we
> already have that information or not.
>
> > On the other hand, an alternative would be
> >
> > } else if (sch->type == SCTP_CID_HEARTBEAT_ACK &&
> > ct->proto.sctp.vtag[dir] == 0) {
> > pr_debug("Setting vtag %x for dir %d\n",
> > sh->vtag, dir);
> > ct->proto.sctp.vtag[dir] = sh->vtag;
> > } else if ((sch->type == SCTP_CID_HEARTBEAT ||
> > sch->type == SCTP_CID_HEARTBEAT_ACK) &&
> > sh->vtag != ct->proto.sctp.vtag[dir]) {
> > pr_debug("Verification tag check failed\n");
> > goto out_unlock;
> > }
> >
> > I'm not sure it looks better.
>
> Now it seems swapped, we should save the tag on HB and check on
> HB_ACK only and would have to check against !dir entry. Like:
I forgot to include the explanation of vtag setting/checking logic into
the commit message. It is supposed to work like this:
Normally, vtag is set from the INIT chunk for the reply direction and
from the INIT-ACK chunk for the originating direction (i.e. each of
these defines vtag value for the opposite direction). For secondary
conntracks, we can't rely on seeing INIT/INIT-ACK and even if we have
seen them, we would need to connect two different conntracks. Therefore
simplified logic is applied: vtag of first packet in each direction
(HEARTBEAT in the originating and HEARTBEAT-ACK in reply direction) is
saved and all following packets in that direction are compared with this
saved value. While INIT and INIT-ACK define vtag for the opposite
direction (that's where "!dir" comes from), vtags extracted from
HEARTBEAT and HEARTBEAT-ACK are always for their direction. And we have
to check vtags on packets with HEARTBEAT chunks as well because their
vtags should match vtag of the first (set in sctp_new()).
Michal Kubecek
^ permalink raw reply
* Re: [PATCH -mm v8 5/7] mmu-notifier: add clear_young callback
From: Paolo Bonzini @ 2015-07-16 11:35 UTC (permalink / raw)
To: Andres Lagar-Cavilla, Vladimir Davydov, kvm, Eric Northup
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <CAJu=L58yzBr8+XaV90x+S60YnJzd7Yr2fDEgaQ0bcCKpwzSAhw@mail.gmail.com>
On 15/07/2015 21:16, Andres Lagar-Cavilla wrote:
>> > +static int kvm_mmu_notifier_clear_young(struct mmu_notifier *mn,
>> > + struct mm_struct *mm,
>> > + unsigned long start,
>> > + unsigned long end)
>> > +{
>> > + struct kvm *kvm = mmu_notifier_to_kvm(mn);
>> > + int young, idx;
> For reclaim, the clear_flush_young notifier may blow up the secondary
> pte to estimate the access pattern, depending on hardware support (EPT
> access bits available in Haswell onwards, not sure about AMD, PPC,
> etc).
It seems like this problem is limited to pre-Haswell EPT.
I'm okay with the patch. If we find problems later we can always add a
parameter to kvm_age_hva so that it effectively doesn't do anything on
clear_young.
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH -mm v8 0/7] idle memory tracking
From: Vladimir Davydov @ 2015-07-16 10:02 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAJu=L59MjY4F4G2bZh+hrt7aqw3R9mWPeYK65smQWUvMhz85aA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Jul 15, 2015 at 01:47:15PM -0700, Andres Lagar-Cavilla wrote:
> I think the remaining question here is performance.
>
> Have you conducted any studies where
> - there is a workload
> - a daemon is poking kpageidle every N seconds/minutes
> - what is the daemon cpu consumption?
> - what is the workload degradation if any?
>
> N candidates include 30 seconds, 1 minute, 2 minutes, 5 minutes....
>
> Workload candidates include TPC, spec int memory intensive things like
> 429.mcf, stream (http://www.cs.virginia.edu/stream/ "sustainable
> memory bandwidth" vs floating point performance)
>
> I'm not asking for a research paper, but if, say, a 2 minute-period
> daemon introduces no degradation and adds up to a minute of cpu per
> hour, then we're golden.
Fair enough. Will do that soon and report back.
Thanks a lot for the review, it was really helpful!
Vladimir
^ permalink raw reply
* Re: [PATCH -mm v8 6/7] proc: add kpageidle file
From: Vladimir Davydov @ 2015-07-16 9:53 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <CAJu=L59He_qOEM3fEADLaKcV0YGY+QKQ_kPN=rSF8=U_UzAt2w@mail.gmail.com>
On Wed, Jul 15, 2015 at 12:42:28PM -0700, Andres Lagar-Cavilla wrote:
> On Wed, Jul 15, 2015 at 6:54 AM, Vladimir Davydov
> <vdavydov@parallels.com> wrote:
[...]
> > +static void kpageidle_clear_pte_refs(struct page *page)
> > +{
> > + struct rmap_walk_control rwc = {
> > + .rmap_one = kpageidle_clear_pte_refs_one,
> > + .anon_lock = page_lock_anon_vma_read,
> > + };
> > + bool need_lock;
> > +
> > + if (!page_mapped(page) ||
>
> Question: what about mlocked pages? Is there any point in calculating
> their idleness?
Those can be filtered out with the aid of /proc/kpageflags (this is what
the script attached to patch #0 of the series actually does). We have to
read the latter anyway in order to get information about THP. That said,
I prefer not to introduce any artificial checks for locked memory. Who
knows, may be one day somebody will use this API to track access pattern
to an mlocked area.
>
> > + !page_rmapping(page))
>
> Not sure, does this skip SwapCache pages? Is there any point in
> calculating their idleness?
A SwapCache page may be mapped, and if it is we should not skip it. If
it is unmapped, we have nothing to do.
Regarding idleness of SwapCache pages, I think we shouldn't
differentiate them from other user pages here, because a shmem/anon page
can migrate to-and-fro the swap cache occasionally during a
memory-active workload, and we don't want to lose its idle status then.
>
> > + return;
> > +
> > + need_lock = !PageAnon(page) || PageKsm(page);
> > + if (need_lock && !trylock_page(page))
> > + return;
> > +
> > + rmap_walk(page, &rwc);
> > +
> > + if (need_lock)
> > + unlock_page(page);
> > +}
[...]
> > @@ -1754,6 +1754,11 @@ static void __split_huge_page_refcount(struct page *page,
> > /* clear PageTail before overwriting first_page */
> > smp_wmb();
> >
> > + if (page_is_young(page))
> > + set_page_young(page_tail);
> > + if (page_is_idle(page))
> > + set_page_idle(page_tail);
> > +
>
> Why not in the block above?
>
> page_tail->flags |= (page->flags &
> ...
> #ifdef CONFIG_WHATEVER_IT_WAS
> 1 << PG_idle
> 1 << PG_young
> #endif
Too many ifdef's :-/ Note, the flags can be in page_ext, which mean we
would have to add something like this
#if defined(CONFIG_WHATEVER_IT_WAS) && defined(CONFIG_64BIT)
1 << PG_idle
1 << PG_young
#endif
<...>
#ifndef CONFIG_64BIT
if (page_is_young(page))
set_page_young(page_tail);
if (page_is_idle(page))
set_page_idle(page_tail);
#endif
which IMO looks less readable than what we have now.
Thanks,
Vladimir
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH -mm v8 4/7] proc: add kpagecgroup file
From: Vladimir Davydov @ 2015-07-16 9:28 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAJu=L58kZW2WRpx8wLx=FXdS29BJ+euLRdDcTXJKwf-VLT6SCA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Jul 15, 2015 at 12:03:18PM -0700, Andres Lagar-Cavilla wrote:
> For both /proc/kpage* interfaces you add (and more critically for the
> rmap-causing one, kpageidle):
>
> It's a good idea to do cond_sched(). Whether after each pfn, each Nth
> pfn, each put_user, I leave to you, but a reasonable cadence is
> needed, because user-space can call this on the entire physical
> address space, and that's a lot of work to do without re-scheduling.
I really don't think it's necessary. These files can only be
read/written by the root, who has plenty ways to kill the system anyway.
The program that is allowed to read/write these files must be conscious
and do it in batches of reasonable size. AFAICS the same reasoning
already lays behind /proc/kpagecount and /proc/kpageflag, which also do
not thrust the "right" batch size on their readers.
Thanks,
Vladimir
^ permalink raw reply
* Re: [PATCH -mm v8 0/7] idle memory tracking
From: Andres Lagar-Cavilla @ 2015-07-15 20:47 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <cover.1436967694.git.vdavydov@parallels.com>
On Wed, Jul 15, 2015 at 6:54 AM, Vladimir Davydov
<vdavydov@parallels.com> wrote:
> Hi,
>
> This patch set introduces a new user API for tracking user memory pages
> that have not been used for a given period of time. The purpose of this
> is to provide the userspace with the means of tracking a workload's
> working set, i.e. the set of pages that are actively used by the
> workload. Knowing the working set size can be useful for partitioning
> the system more efficiently, e.g. by tuning memory cgroup limits
> appropriately, or for job placement within a compute cluster.
>
> It is based on top of v4.2-rc1-mmotm-2015-07-06-16-25
>
> ---- USE CASES ----
>
> The unified cgroup hierarchy has memory.low and memory.high knobs, which
> are defined as the low and high boundaries for the workload working set
> size. However, the working set size of a workload may be unknown or
> change in time. With this patch set, one can periodically estimate the
> amount of memory unused by each cgroup and tune their memory.low and
> memory.high parameters accordingly, therefore optimizing the overall
> memory utilization.
>
> Another use case is balancing workloads within a compute cluster.
> Knowing how much memory is not really used by a workload unit may help
> take a more optimal decision when considering migrating the unit to
> another node within the cluster.
>
> Also, as noted by Minchan, this would be useful for per-process reclaim
> (https://lwn.net/Articles/545668/). With idle tracking, we could reclaim idle
> pages only by smart user memory manager.
>
> ---- USER API ----
>
> The user API consists of two new proc files:
>
> * /proc/kpageidle. This file implements a bitmap where each bit corresponds
> to a page, indexed by PFN. When the bit is set, the corresponding page is
> idle. A page is considered idle if it has not been accessed since it was
> marked idle. To mark a page idle one should set the bit corresponding to the
> page by writing to the file. A value written to the file is OR-ed with the
> current bitmap value. Only user memory pages can be marked idle, for other
> page types input is silently ignored. Writing to this file beyond max PFN
> results in the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is
> set.
>
> This file can be used to estimate the amount of pages that are not
> used by a particular workload as follows:
>
> 1. mark all pages of interest idle by setting corresponding bits in the
> /proc/kpageidle bitmap
> 2. wait until the workload accesses its working set
> 3. read /proc/kpageidle and count the number of bits set
>
> * /proc/kpagecgroup. This file contains a 64-bit inode number of the
> memory cgroup each page is charged to, indexed by PFN. Only available when
> CONFIG_MEMCG is set.
>
> This file can be used to find all pages (including unmapped file
> pages) accounted to a particular cgroup. Using /proc/kpageidle, one
> can then estimate the cgroup working set size.
>
> For an example of using these files for estimating the amount of unused
> memory pages per each memory cgroup, please see the script attached
> below.
>
> ---- REASONING ----
>
> The reason to introduce the new user API instead of using
> /proc/PID/{clear_refs,smaps} is that the latter has two serious
> drawbacks:
>
> - it does not count unmapped file pages
> - it affects the reclaimer logic
>
> The new API attempts to overcome them both. For more details on how it
> is achieved, please see the comment to patch 5.
>
> ---- CHANGE LOG ----
>
> Changes in v8:
>
> - clear referenced/accessed bit in secondary ptes while accessing
> /proc/kpageidle; this is required to estimate wss of KVM VMs (Andres)
> - check the young flag when collapsing a huge page
> - copy idle/young flags on page migration
Both good catches, thanks!
I think the remaining question here is performance.
Have you conducted any studies where
- there is a workload
- a daemon is poking kpageidle every N seconds/minutes
- what is the daemon cpu consumption?
- what is the workload degradation if any?
N candidates include 30 seconds, 1 minute, 2 minutes, 5 minutes....
Workload candidates include TPC, spec int memory intensive things like
429.mcf, stream (http://www.cs.virginia.edu/stream/ "sustainable
memory bandwidth" vs floating point performance)
I'm not asking for a research paper, but if, say, a 2 minute-period
daemon introduces no degradation and adds up to a minute of cpu per
hour, then we're golden.
Andres
>
> Changes in v7:
>
> This iteration addresses Andres's comments to v6:
>
> - do not reuse page_referenced for clearing idle flag, introduce a
> separate function instead; this way we won't issue expensive tlb
> flushes on /proc/kpageidle read/write
> - propagate young/idle flags from head to tail pages on thp split
> - skip compound tail pages while reading/writing /proc/kpageidle
> - cleanup page_referenced_one
>
> Changes in v6:
>
> - Split the patch introducing page_cgroup_ino helper to ease review.
> - Rebase on top of v4.1-rc7-mmotm-2015-06-09-16-55
>
> Changes in v5:
>
> - Fix possible race between kpageidle_clear_pte_refs() and
> __page_set_anon_rmap() by checking that a page is on an LRU list
> under zone->lru_lock (Minchan).
> - Export idle flag via /proc/kpageflags (Minchan).
> - Rebase on top of 4.1-rc3.
>
> Changes in v4:
>
> This iteration primarily addresses Minchan's comments to v3:
>
> - Implement /proc/kpageidle as a bitmap instead of using u64 per each page,
> because there does not seem to be any future uses for the other 63 bits.
> - Do not double-increase pra->referenced in page_referenced_one() if the page
> was young and referenced recently.
> - Remove the pointless (page_count == 0) check from kpageidle_get_page().
> - Rename kpageidle_clear_refs() to kpageidle_clear_pte_refs().
> - Improve comments to kpageidle-related functions.
> - Rebase on top of 4.1-rc2.
>
> Note it does not address Minchan's concern of possible __page_set_anon_rmap vs
> page_referenced race (see https://lkml.org/lkml/2015/5/3/220) since it is still
> unclear if this race can really happen (see https://lkml.org/lkml/2015/5/4/160)
>
> Changes in v3:
>
> - Enable CONFIG_IDLE_PAGE_TRACKING for 32 bit. Since this feature
> requires two extra page flags and there is no space for them on 32
> bit, page ext is used (thanks to Minchan Kim).
> - Minor code cleanups and comments improved.
> - Rebase on top of 4.1-rc1.
>
> Changes in v2:
>
> - The main difference from v1 is the API change. In v1 the user can
> only set the idle flag for all pages at once, and for clearing the
> Idle flag on pages accessed via page tables /proc/PID/clear_refs
> should be used.
> The main drawback of the v1 approach, as noted by Minchan, is that on
> big machines setting the idle flag for each pages can result in CPU
> bursts, which would be especially frustrating if the user only wanted
> to estimate the amount of idle pages for a particular process or VMA.
> With the new API a more fine-grained approach is possible: one can
> read a process's /proc/PID/pagemap and set/check the Idle flag only
> for those pages of the process's address space he or she is
> interested in.
> Another good point about the v2 API is that it is possible to limit
> /proc/kpage* scanning rate when the user wants to estimate the total
> number of idle pages, which is unachievable with the v1 approach.
> - Make /proc/kpagecgroup return the ino of the closest online ancestor
> in case the cgroup a page is charged to is offline.
> - Fix /proc/PID/clear_refs not clearing Young page flag.
> - Rebase on top of v4.0-rc6-mmotm-2015-04-01-14-54
>
> v7: https://lkml.org/lkml/2015/7/11/119
> v6: https://lkml.org/lkml/2015/6/12/301
> v5: https://lkml.org/lkml/2015/5/12/449
> v4: https://lkml.org/lkml/2015/5/7/580
> v3: https://lkml.org/lkml/2015/4/28/224
> v2: https://lkml.org/lkml/2015/4/7/260
> v1: https://lkml.org/lkml/2015/3/18/794
>
> ---- PATCH SET STRUCTURE ----
>
> The patch set is organized as follows:
>
> - patch 1 adds page_cgroup_ino() helper for the sake of
> /proc/kpagecgroup and patches 2-3 do related cleanup
> - patch 4 adds /proc/kpagecgroup, which reports cgroup ino each page is
> charged to
> - patch 5 introduces a new mmu notifier callback, clear_young, which is
> a lightweight version of clear_flush_young; it is used in patch 6
> - patch 6 implements the idle page tracking feature, including the
> userspace API, /proc/kpageidle
> - patch 7 exports idle flag via /proc/kpageflags
>
> ---- SIMILAR WORKS ----
>
> Originally, the patch for tracking idle memory was proposed back in 2011
> by Michel Lespinasse (see http://lwn.net/Articles/459269/). The main
> difference between Michel's patch and this one is that Michel
> implemented a kernel space daemon for estimating idle memory size per
> cgroup while this patch only provides the userspace with the minimal API
> for doing the job, leaving the rest up to the userspace. However, they
> both share the same idea of Idle/Young page flags to avoid affecting the
> reclaimer logic.
>
> ---- SCRIPT FOR COUNTING IDLE PAGES PER CGROUP ----
> #! /usr/bin/python
> #
>
> import os
> import stat
> import errno
> import struct
>
> CGROUP_MOUNT = "/sys/fs/cgroup/memory"
> BUFSIZE = 8 * 1024 # must be multiple of 8
>
>
> def get_hugepage_size():
> with open("/proc/meminfo", "r") as f:
> for s in f:
> k, v = s.split(":")
> if k == "Hugepagesize":
> return int(v.split()[0]) * 1024
>
> PAGE_SIZE = os.sysconf("SC_PAGE_SIZE")
> HUGEPAGE_SIZE = get_hugepage_size()
>
>
> def set_idle():
> f = open("/proc/kpageidle", "wb", BUFSIZE)
> while True:
> try:
> f.write(struct.pack("Q", pow(2, 64) - 1))
> except IOError as err:
> if err.errno == errno.ENXIO:
> break
> raise
> f.close()
>
>
> def count_idle():
> f_flags = open("/proc/kpageflags", "rb", BUFSIZE)
> f_cgroup = open("/proc/kpagecgroup", "rb", BUFSIZE)
>
> with open("/proc/kpageidle", "rb", BUFSIZE) as f:
> while f.read(BUFSIZE): pass # update idle flag
>
> idlememsz = {}
> while True:
> s1, s2 = f_flags.read(8), f_cgroup.read(8)
> if not s1 or not s2:
> break
>
> flags, = struct.unpack('Q', s1)
> cgino, = struct.unpack('Q', s2)
>
> unevictable = (flags >> 18) & 1
> huge = (flags >> 22) & 1
> idle = (flags >> 25) & 1
>
> if idle and not unevictable:
> idlememsz[cgino] = idlememsz.get(cgino, 0) + \
> (HUGEPAGE_SIZE if huge else PAGE_SIZE)
>
> f_flags.close()
> f_cgroup.close()
> return idlememsz
>
>
> if __name__ == "__main__":
> print "Setting the idle flag for each page..."
> set_idle()
>
> raw_input("Wait until the workload accesses its working set, "
> "then press Enter")
>
> print "Counting idle pages..."
> idlememsz = count_idle()
>
> for dir, subdirs, files in os.walk(CGROUP_MOUNT):
> ino = os.stat(dir)[stat.ST_INO]
> print dir + ": " + str(idlememsz.get(ino, 0) / 1024) + " kB"
> ---- END SCRIPT ----
>
> Comments are more than welcome.
>
> Thanks,
>
> Vladimir Davydov (7):
> memcg: add page_cgroup_ino helper
> hwpoison: use page_cgroup_ino for filtering by memcg
> memcg: zap try_get_mem_cgroup_from_page
> proc: add kpagecgroup file
> mmu-notifier: add clear_young callback
> proc: add kpageidle file
> proc: export idle flag via kpageflags
>
> Documentation/vm/pagemap.txt | 22 ++-
> fs/proc/page.c | 274 +++++++++++++++++++++++++++++++++
> fs/proc/task_mmu.c | 4 +-
> include/linux/memcontrol.h | 7 +-
> include/linux/mm.h | 98 ++++++++++++
> include/linux/mmu_notifier.h | 44 ++++++
> include/linux/page-flags.h | 11 ++
> include/linux/page_ext.h | 4 +
> include/uapi/linux/kernel-page-flags.h | 1 +
> mm/Kconfig | 12 ++
> mm/debug.c | 4 +
> mm/huge_memory.c | 11 +-
> mm/hwpoison-inject.c | 5 +-
> mm/memcontrol.c | 71 +++++----
> mm/memory-failure.c | 16 +-
> mm/migrate.c | 5 +
> mm/mmu_notifier.c | 17 ++
> mm/page_ext.c | 3 +
> mm/rmap.c | 5 +
> mm/swap.c | 2 +
> virt/kvm/kvm_main.c | 18 +++
> 21 files changed, 570 insertions(+), 64 deletions(-)
>
> --
> 2.1.4
>
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc@google.com
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: nf_ct_sctp: minimal multihoming support
From: Marcelo Ricardo Leitner @ 2015-07-15 20:35 UTC (permalink / raw)
To: Michal Kubecek
Cc: Florian Westphal, netfilter-devel, coreteam, linux-api, netdev,
linux-kernel, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller
In-Reply-To: <20150714164225.GB8478@unicorn.suse.cz>
Hi,
On Tue, Jul 14, 2015 at 06:42:25PM +0200, Michal Kubecek wrote:
> On Tue, Jul 14, 2015 at 03:42:03PM +0200, Florian Westphal wrote:
> > Michal Kubecek <mkubecek@suse.cz> wrote:
> > > + case SCTP_CID_HEARTBEAT:
> > > + pr_debug("SCTP_CID_HEARTBEAT");
> > > + i = 9;
> > > + break;
> > > + case SCTP_CID_HEARTBEAT_ACK:
> > > + pr_debug("SCTP_CID_HEARTBEAT_ACK");
> > > + i = 10;
> > > + break;
> > > default:
> > > /* Other chunks like DATA, SACK, HEARTBEAT and
> > > its ACK do not cause a change in state */
> > > @@ -329,6 +351,8 @@ static int sctp_packet(struct nf_conn *ct,
> > > !test_bit(SCTP_CID_COOKIE_ECHO, map) &&
> > > !test_bit(SCTP_CID_ABORT, map) &&
> > > !test_bit(SCTP_CID_SHUTDOWN_ACK, map) &&
> > > + !test_bit(SCTP_CID_HEARTBEAT, map) &&
> > > + !test_bit(SCTP_CID_HEARTBEAT_ACK, map) &&
> > > sh->vtag != ct->proto.sctp.vtag[dir]) {
> > > pr_debug("Verification tag check failed\n");
> > > goto out;
> > > @@ -357,6 +381,16 @@ static int sctp_packet(struct nf_conn *ct,
> > > /* Sec 8.5.1 (D) */
> > > if (sh->vtag != ct->proto.sctp.vtag[dir])
> > > goto out_unlock;
> > > + } else if (sch->type == SCTP_CID_HEARTBEAT ||
> > > + sch->type == SCTP_CID_HEARTBEAT_ACK) {
> > > + if (ct->proto.sctp.vtag[dir] == 0) {
> > > + pr_debug("Setting vtag %x for dir %d\n",
> > > + sh->vtag, dir);
> > > + ct->proto.sctp.vtag[dir] = sh->vtag;
> >
> > Could you please elaborate on the [dir] == 0 test?
> >
> > I see this might happen for SCTP_CID_HEARTBEAT_ACK, but why is this
> > needed for SCTP_CID_HEARTBEAT ?
> >
> > We found a conntrack entry so shouldn't the vtag[dir] already be > 0?
>
> Yes, you are right. This was originally intended to handle the case when
> a HEARTBEAT in the reply direction is seen before the HEARTBEAT-ACK but
> such HEARTBEAT would be dropped anyway in current version.
And we have to keep the first vtag attempted because otherwise an
attacker could just probe for the right one until she gets a reply.
IOW, if a different vtag is attempted, we should drop it as the packet
doesn't belong to that association/conntrack entry.
As vtags are always != 0 in such case, that's a way to know if we
already have that information or not.
> On the other hand, an alternative would be
>
> } else if (sch->type == SCTP_CID_HEARTBEAT_ACK &&
> ct->proto.sctp.vtag[dir] == 0) {
> pr_debug("Setting vtag %x for dir %d\n",
> sh->vtag, dir);
> ct->proto.sctp.vtag[dir] = sh->vtag;
> } else if ((sch->type == SCTP_CID_HEARTBEAT ||
> sch->type == SCTP_CID_HEARTBEAT_ACK) &&
> sh->vtag != ct->proto.sctp.vtag[dir]) {
> pr_debug("Verification tag check failed\n");
> goto out_unlock;
> }
>
> I'm not sure it looks better.
Now it seems swapped, we should save the tag on HB and check on
HB_ACK only and would have to check against !dir entry. Like:
comments with // here are just informative on this email and may not
necessary in the code
} else if (sch->type == SCTP_CID_HEARTBEAT)
if (!sh->vtag || sh->vtag != ct->proto.sctp.vtag[!dir]) {
/* Invalid */
goto out_unlock;
}
else if (!ct->proto.sctp.vtag[!dir]) {
pr_debug("Setting vtag %x for dir %d\n",
sh->vtag, dir);
// by saving it on !dir direction, we
// don't have to enable HB_ACK on that
// if() above of exceptions, just HB
// itself
ct->proto.sctp.vtag[!dir] = sh->vtag;
}
// fallthrough: no action because it's a vtag we
// have already seen
} else if (sch->type == SCTP_CID_HEARTBEAT_ACK) {
// it's guaranteed tht sh->vtag == ct->proto.sctp.vtag[dir]
// by the if() of the exceptions above, so we
// just have to check for vtag == 0
if (!sh->vtag) {
pr_debug("Verification tag check failed\n");
goto out_unlock;
}
// ok, it's hand-shaked, now save it on the
// other direction too so that further packets
// can pass that check
// if there was a vtag crossed some when, we
// don't care as this one is now established.
ct->proto.sctp.vtag[dir] = sh->vtag;
}
I was also working on such implementation. Please give me some time to
map one thing to another.
Marcelo
^ permalink raw reply
* Re: [PATCH -mm v8 6/7] proc: add kpageidle file
From: Andres Lagar-Cavilla @ 2015-07-15 19:42 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <a414d0458156434ca428c0c810db2e86878e1897.1436967694.git.vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Wed, Jul 15, 2015 at 6:54 AM, Vladimir Davydov
<vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> Knowing the portion of memory that is not used by a certain application
> or memory cgroup (idle memory) can be useful for partitioning the system
> efficiently, e.g. by setting memory cgroup limits appropriately.
> Currently, the only means to estimate the amount of idle memory provided
> by the kernel is /proc/PID/{clear_refs,smaps}: the user can clear the
> access bit for all pages mapped to a particular process by writing 1 to
> clear_refs, wait for some time, and then count smaps:Referenced.
> However, this method has two serious shortcomings:
>
> - it does not count unmapped file pages
> - it affects the reclaimer logic
>
> To overcome these drawbacks, this patch introduces two new page flags,
> Idle and Young, and a new proc file, /proc/kpageidle. A page's Idle flag
> can only be set from userspace by setting bit in /proc/kpageidle at the
> offset corresponding to the page, and it is cleared whenever the page is
> accessed either through page tables (it is cleared in page_referenced()
> in this case) or using the read(2) system call (mark_page_accessed()).
> Thus by setting the Idle flag for pages of a particular workload, which
> can be found e.g. by reading /proc/PID/pagemap, waiting for some time to
> let the workload access its working set, and then reading the kpageidle
> file, one can estimate the amount of pages that are not used by the
> workload.
>
> The Young page flag is used to avoid interference with the memory
> reclaimer. A page's Young flag is set whenever the Access bit of a page
> table entry pointing to the page is cleared by writing to kpageidle. If
> page_referenced() is called on a Young page, it will add 1 to its return
> value, therefore concealing the fact that the Access bit was cleared.
>
> Note, since there is no room for extra page flags on 32 bit, this
> feature uses extended page flags when compiled on 32 bit.
>
> Signed-off-by: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> ---
> Documentation/vm/pagemap.txt | 12 ++-
> fs/proc/page.c | 218 +++++++++++++++++++++++++++++++++++++++++++
> fs/proc/task_mmu.c | 4 +-
> include/linux/mm.h | 98 +++++++++++++++++++
> include/linux/page-flags.h | 11 +++
> include/linux/page_ext.h | 4 +
> mm/Kconfig | 12 +++
> mm/debug.c | 4 +
> mm/huge_memory.c | 11 ++-
> mm/migrate.c | 5 +
> mm/page_ext.c | 3 +
> mm/rmap.c | 5 +
> mm/swap.c | 2 +
> 13 files changed, 385 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
> index a9b7afc8fbc6..c9266340852c 100644
> --- a/Documentation/vm/pagemap.txt
> +++ b/Documentation/vm/pagemap.txt
> @@ -5,7 +5,7 @@ pagemap is a new (as of 2.6.25) set of interfaces in the kernel that allow
> userspace programs to examine the page tables and related information by
> reading files in /proc.
>
> -There are four components to pagemap:
> +There are five components to pagemap:
>
> * /proc/pid/pagemap. This file lets a userspace process find out which
> physical frame each virtual page is mapped to. It contains one 64-bit
> @@ -69,6 +69,16 @@ There are four components to pagemap:
> memory cgroup each page is charged to, indexed by PFN. Only available when
> CONFIG_MEMCG is set.
>
> + * /proc/kpageidle. This file implements a bitmap where each bit corresponds
> + to a page, indexed by PFN. When the bit is set, the corresponding page is
> + idle. A page is considered idle if it has not been accessed since it was
> + marked idle. To mark a page idle one should set the bit corresponding to the
> + page by writing to the file. A value written to the file is OR-ed with the
> + current bitmap value. Only user memory pages can be marked idle, for other
> + page types input is silently ignored. Writing to this file beyond max PFN
> + results in the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is
> + set.
> +
> Short descriptions to the page flags:
>
> 0. LOCKED
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 70d23245dd43..273537885ab4 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -5,6 +5,8 @@
> #include <linux/ksm.h>
> #include <linux/mm.h>
> #include <linux/mmzone.h>
> +#include <linux/rmap.h>
> +#include <linux/mmu_notifier.h>
> #include <linux/huge_mm.h>
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> @@ -16,6 +18,7 @@
>
> #define KPMSIZE sizeof(u64)
> #define KPMMASK (KPMSIZE - 1)
> +#define KPMBITS (KPMSIZE * BITS_PER_BYTE)
>
> /* /proc/kpagecount - an array exposing page counts
> *
> @@ -275,6 +278,217 @@ static const struct file_operations proc_kpagecgroup_operations = {
> };
> #endif /* CONFIG_MEMCG */
>
> +#ifdef CONFIG_IDLE_PAGE_TRACKING
> +/*
> + * Idle page tracking only considers user memory pages, for other types of
> + * pages the idle flag is always unset and an attempt to set it is silently
> + * ignored.
> + *
> + * We treat a page as a user memory page if it is on an LRU list, because it is
> + * always safe to pass such a page to rmap_walk(), which is essential for idle
> + * page tracking. With such an indicator of user pages we can skip isolated
> + * pages, but since there are not usually many of them, it will hardly affect
> + * the overall result.
> + *
> + * This function tries to get a user memory page by pfn as described above.
> + */
> +static struct page *kpageidle_get_page(unsigned long pfn)
> +{
> + struct page *page;
> + struct zone *zone;
> +
> + if (!pfn_valid(pfn))
> + return NULL;
> +
> + page = pfn_to_page(pfn);
> + if (!page || !PageLRU(page) ||
> + !get_page_unless_zero(page))
> + return NULL;
> +
> + zone = page_zone(page);
> + spin_lock_irq(&zone->lru_lock);
> + if (unlikely(!PageLRU(page))) {
> + put_page(page);
> + page = NULL;
> + }
> + spin_unlock_irq(&zone->lru_lock);
> + return page;
> +}
> +
> +static int kpageidle_clear_pte_refs_one(struct page *page,
> + struct vm_area_struct *vma,
> + unsigned long addr, void *arg)
> +{
> + struct mm_struct *mm = vma->vm_mm;
> + spinlock_t *ptl;
> + pmd_t *pmd;
> + pte_t *pte;
> + bool referenced = false;
> +
> + if (unlikely(PageTransHuge(page))) {
> + pmd = page_check_address_pmd(page, mm, addr,
> + PAGE_CHECK_ADDRESS_PMD_FLAG, &ptl);
> + if (pmd) {
> + referenced = pmdp_clear_young_notify(vma, addr, pmd);
> + spin_unlock(ptl);
> + }
> + } else {
> + pte = page_check_address(page, mm, addr, &ptl, 0);
> + if (pte) {
> + referenced = ptep_clear_young_notify(vma, addr, pte);
> + pte_unmap_unlock(pte, ptl);
> + }
> + }
> + if (referenced) {
> + clear_page_idle(page);
> + /*
> + * We cleared the referenced bit in a mapping to this page. To
> + * avoid interference with page reclaim, mark it young so that
> + * page_referenced() will return > 0.
> + */
> + set_page_young(page);
> + }
> + return SWAP_AGAIN;
> +}
> +
> +static void kpageidle_clear_pte_refs(struct page *page)
> +{
> + struct rmap_walk_control rwc = {
> + .rmap_one = kpageidle_clear_pte_refs_one,
> + .anon_lock = page_lock_anon_vma_read,
> + };
> + bool need_lock;
> +
> + if (!page_mapped(page) ||
Question: what about mlocked pages? Is there any point in calculating
their idleness?
> + !page_rmapping(page))
Not sure, does this skip SwapCache pages? Is there any point in
calculating their idleness?
> + return;
> +
> + need_lock = !PageAnon(page) || PageKsm(page);
> + if (need_lock && !trylock_page(page))
> + return;
> +
> + rmap_walk(page, &rwc);
> +
> + if (need_lock)
> + unlock_page(page);
> +}
> +
> +static ssize_t kpageidle_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + u64 __user *out = (u64 __user *)buf;
> + struct page *page;
> + unsigned long pfn, end_pfn;
> + ssize_t ret = 0;
> + u64 idle_bitmap = 0;
> + int bit;
> +
> + if (*ppos & KPMMASK || count & KPMMASK)
> + return -EINVAL;
> +
> + pfn = *ppos * BITS_PER_BYTE;
> + if (pfn >= max_pfn)
> + return 0;
> +
> + end_pfn = pfn + count * BITS_PER_BYTE;
> + if (end_pfn > max_pfn)
> + end_pfn = ALIGN(max_pfn, KPMBITS);
> +
> + for (; pfn < end_pfn; pfn++) {
> + bit = pfn % KPMBITS;
> + page = kpageidle_get_page(pfn);
> + if (page) {
> + if (page_is_idle(page)) {
> + /*
> + * The page might have been referenced via a
> + * pte, in which case it is not idle. Clear
> + * refs and recheck.
> + */
> + kpageidle_clear_pte_refs(page);
> + if (page_is_idle(page))
> + idle_bitmap |= 1ULL << bit;
> + }
> + put_page(page);
> + }
> + if (bit == KPMBITS - 1) {
Reminder to add cond_sched() or similar at some regular cadence.
> + if (put_user(idle_bitmap, out)) {
> + ret = -EFAULT;
> + break;
> + }
> + idle_bitmap = 0;
> + out++;
> + }
> + }
> +
> + *ppos += (char __user *)out - buf;
> + if (!ret)
> + ret = (char __user *)out - buf;
> + return ret;
> +}
> +
> +static ssize_t kpageidle_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + const u64 __user *in = (const u64 __user *)buf;
> + struct page *page;
> + unsigned long pfn, end_pfn;
> + ssize_t ret = 0;
> + u64 idle_bitmap = 0;
> + int bit;
> +
> + if (*ppos & KPMMASK || count & KPMMASK)
> + return -EINVAL;
> +
> + pfn = *ppos * BITS_PER_BYTE;
> + if (pfn >= max_pfn)
> + return -ENXIO;
> +
> + end_pfn = pfn + count * BITS_PER_BYTE;
> + if (end_pfn > max_pfn)
> + end_pfn = ALIGN(max_pfn, KPMBITS);
> +
> + for (; pfn < end_pfn; pfn++) {
> + bit = pfn % KPMBITS;
> + if (bit == 0) {
> + if (get_user(idle_bitmap, in)) {
Same...
> + ret = -EFAULT;
> + break;
> + }
> + in++;
> + }
> + if (idle_bitmap >> bit & 1) {
> + page = kpageidle_get_page(pfn);
> + if (page) {
> + kpageidle_clear_pte_refs(page);
> + set_page_idle(page);
> + put_page(page);
> + }
> + }
> + }
> +
> + *ppos += (const char __user *)in - buf;
> + if (!ret)
> + ret = (const char __user *)in - buf;
> + return ret;
> +}
> +
> +static const struct file_operations proc_kpageidle_operations = {
> + .llseek = mem_lseek,
> + .read = kpageidle_read,
> + .write = kpageidle_write,
> +};
> +
> +#ifndef CONFIG_64BIT
> +static bool need_page_idle(void)
> +{
> + return true;
> +}
> +struct page_ext_operations page_idle_ops = {
> + .need = need_page_idle,
> +};
> +#endif
> +#endif /* CONFIG_IDLE_PAGE_TRACKING */
> +
> static int __init proc_page_init(void)
> {
> proc_create("kpagecount", S_IRUSR, NULL, &proc_kpagecount_operations);
> @@ -282,6 +496,10 @@ static int __init proc_page_init(void)
> #ifdef CONFIG_MEMCG
> proc_create("kpagecgroup", S_IRUSR, NULL, &proc_kpagecgroup_operations);
> #endif
> +#ifdef CONFIG_IDLE_PAGE_TRACKING
> + proc_create("kpageidle", S_IRUSR | S_IWUSR, NULL,
> + &proc_kpageidle_operations);
> +#endif
> return 0;
> }
> fs_initcall(proc_page_init);
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 3b4d8255e806..3efd7f641f92 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -458,7 +458,7 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page,
>
> mss->resident += size;
> /* Accumulate the size in pages that have been accessed. */
> - if (young || PageReferenced(page))
> + if (young || page_is_young(page) || PageReferenced(page))
> mss->referenced += size;
> mapcount = page_mapcount(page);
> if (mapcount >= 2) {
> @@ -810,6 +810,7 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned long addr,
>
> /* Clear accessed and referenced bits. */
> pmdp_test_and_clear_young(vma, addr, pmd);
> + test_and_clear_page_young(page);
> ClearPageReferenced(page);
> out:
> spin_unlock(ptl);
> @@ -837,6 +838,7 @@ out:
>
> /* Clear accessed and referenced bits. */
> ptep_test_and_clear_young(vma, addr, pte);
> + test_and_clear_page_young(page);
> ClearPageReferenced(page);
> }
> pte_unmap_unlock(pte - 1, ptl);
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7f471789781a..de450c1191b9 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2205,5 +2205,103 @@ void __init setup_nr_node_ids(void);
> static inline void setup_nr_node_ids(void) {}
> #endif
>
> +#ifdef CONFIG_IDLE_PAGE_TRACKING
> +#ifdef CONFIG_64BIT
> +static inline bool page_is_young(struct page *page)
> +{
> + return PageYoung(page);
> +}
> +
> +static inline void set_page_young(struct page *page)
> +{
> + SetPageYoung(page);
> +}
> +
> +static inline bool test_and_clear_page_young(struct page *page)
> +{
> + return TestClearPageYoung(page);
> +}
> +
> +static inline bool page_is_idle(struct page *page)
> +{
> + return PageIdle(page);
> +}
> +
> +static inline void set_page_idle(struct page *page)
> +{
> + SetPageIdle(page);
> +}
> +
> +static inline void clear_page_idle(struct page *page)
> +{
> + ClearPageIdle(page);
> +}
> +#else /* !CONFIG_64BIT */
> +/*
> + * If there is not enough space to store Idle and Young bits in page flags, use
> + * page ext flags instead.
> + */
> +extern struct page_ext_operations page_idle_ops;
> +
> +static inline bool page_is_young(struct page *page)
> +{
> + return test_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
> +}
> +
> +static inline void set_page_young(struct page *page)
> +{
> + set_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
> +}
> +
> +static inline bool test_and_clear_page_young(struct page *page)
> +{
> + return test_and_clear_bit(PAGE_EXT_YOUNG,
> + &lookup_page_ext(page)->flags);
> +}
> +
> +static inline bool page_is_idle(struct page *page)
> +{
> + return test_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
> +}
> +
> +static inline void set_page_idle(struct page *page)
> +{
> + set_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
> +}
> +
> +static inline void clear_page_idle(struct page *page)
> +{
> + clear_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
> +}
> +#endif /* CONFIG_64BIT */
> +#else /* !CONFIG_IDLE_PAGE_TRACKING */
> +static inline bool page_is_young(struct page *page)
> +{
> + return false;
> +}
> +
> +static inline void set_page_young(struct page *page)
> +{
> +}
> +
> +static inline bool test_and_clear_page_young(struct page *page)
> +{
> + return false;
> +}
> +
> +static inline bool page_is_idle(struct page *page)
> +{
> + return false;
> +}
> +
> +static inline void set_page_idle(struct page *page)
> +{
> +}
> +
> +static inline void clear_page_idle(struct page *page)
> +{
> +}
> +#endif /* CONFIG_IDLE_PAGE_TRACKING */
> +
> #endif /* __KERNEL__ */
> #endif /* _LINUX_MM_H */
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 91b7f9b2b774..478f2241f284 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -109,6 +109,10 @@ enum pageflags {
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> PG_compound_lock,
> #endif
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
> + PG_young,
> + PG_idle,
> +#endif
> __NR_PAGEFLAGS,
>
> /* Filesystems */
> @@ -363,6 +367,13 @@ PAGEFLAG_FALSE(HWPoison)
> #define __PG_HWPOISON 0
> #endif
>
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
> +TESTPAGEFLAG(Young, young, PF_ANY)
> +SETPAGEFLAG(Young, young, PF_ANY)
> +TESTCLEARFLAG(Young, young, PF_ANY)
> +PAGEFLAG(Idle, idle, PF_ANY)
> +#endif
> +
> /*
> * On an anonymous page mapped into a user virtual memory area,
> * page->mapping points to its anon_vma, not to a struct address_space;
> diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> index c42981cd99aa..17f118a82854 100644
> --- a/include/linux/page_ext.h
> +++ b/include/linux/page_ext.h
> @@ -26,6 +26,10 @@ enum page_ext_flags {
> PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
> PAGE_EXT_DEBUG_GUARD,
> PAGE_EXT_OWNER,
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
> + PAGE_EXT_YOUNG,
> + PAGE_EXT_IDLE,
> +#endif
> };
>
> /*
> diff --git a/mm/Kconfig b/mm/Kconfig
> index e79de2bd12cd..db817e2c2ec8 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -654,3 +654,15 @@ config DEFERRED_STRUCT_PAGE_INIT
> when kswapd starts. This has a potential performance impact on
> processes running early in the lifetime of the systemm until kswapd
> finishes the initialisation.
> +
> +config IDLE_PAGE_TRACKING
> + bool "Enable idle page tracking"
> + select PROC_PAGE_MONITOR
> + select PAGE_EXTENSION if !64BIT
> + help
> + This feature allows to estimate the amount of user pages that have
> + not been touched during a given period of time. This information can
> + be useful to tune memory cgroup limits and/or for job placement
> + within a compute cluster.
> +
> + See Documentation/vm/pagemap.txt for more details.
> diff --git a/mm/debug.c b/mm/debug.c
> index 76089ddf99ea..6c1b3ea61bfd 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -48,6 +48,10 @@ static const struct trace_print_flags pageflag_names[] = {
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> {1UL << PG_compound_lock, "compound_lock" },
> #endif
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
> + {1UL << PG_young, "young" },
> + {1UL << PG_idle, "idle" },
> +#endif
> };
>
> static void dump_flags(unsigned long flags,
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 9671f51e954d..bb6d2ec1f268 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1754,6 +1754,11 @@ static void __split_huge_page_refcount(struct page *page,
> /* clear PageTail before overwriting first_page */
> smp_wmb();
>
> + if (page_is_young(page))
> + set_page_young(page_tail);
> + if (page_is_idle(page))
> + set_page_idle(page_tail);
> +
Why not in the block above?
page_tail->flags |= (page->flags &
...
#ifdef CONFIG_WHATEVER_IT_WAS
1 << PG_idle
1 << PG_young
#endif
> /*
> * __split_huge_page_splitting() already set the
> * splitting bit in all pmd that could map this
> @@ -2259,7 +2264,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> VM_BUG_ON_PAGE(PageLRU(page), page);
>
> /* If there is no mapped pte young don't collapse the page */
> - if (pte_young(pteval) || PageReferenced(page) ||
> + if (pte_young(pteval) ||
> + page_is_young(page) || PageReferenced(page) ||
> mmu_notifier_test_young(vma->vm_mm, address))
> referenced = true;
> }
> @@ -2686,7 +2692,8 @@ static int khugepaged_scan_pmd(struct mm_struct *mm,
> */
> if (page_count(page) != 1 + !!PageSwapCache(page))
> goto out_unmap;
> - if (pte_young(pteval) || PageReferenced(page) ||
> + if (pte_young(pteval) ||
> + page_is_young(page) || PageReferenced(page) ||
> mmu_notifier_test_young(vma->vm_mm, address))
> referenced = true;
> }
Cool finds, thanks for the thoroughness
Andres
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 236ee25e79d9..3e7bb4f2b51c 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -524,6 +524,11 @@ void migrate_page_copy(struct page *newpage, struct page *page)
> __set_page_dirty_nobuffers(newpage);
> }
>
> + if (page_is_young(page))
> + set_page_young(newpage);
> + if (page_is_idle(page))
> + set_page_idle(newpage);
> +
> /*
> * Copy NUMA information to the new page, to prevent over-eager
> * future migrations of this same page.
> diff --git a/mm/page_ext.c b/mm/page_ext.c
> index d86fd2f5353f..e4b3af054bf2 100644
> --- a/mm/page_ext.c
> +++ b/mm/page_ext.c
> @@ -59,6 +59,9 @@ static struct page_ext_operations *page_ext_ops[] = {
> #ifdef CONFIG_PAGE_OWNER
> &page_owner_ops,
> #endif
> +#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
> + &page_idle_ops,
> +#endif
> };
>
> static unsigned long total_usage;
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 49b244b1f18c..c96677ade3d1 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -798,6 +798,11 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> pte_unmap_unlock(pte, ptl);
> }
>
> + if (referenced)
> + clear_page_idle(page);
> + if (test_and_clear_page_young(page))
> + referenced++;
> +
> if (referenced) {
> pra->referenced++;
> pra->vm_flags |= vma->vm_flags;
> diff --git a/mm/swap.c b/mm/swap.c
> index ab7c338eda87..db43c9b4891d 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -623,6 +623,8 @@ void mark_page_accessed(struct page *page)
> } else if (!PageReferenced(page)) {
> SetPageReferenced(page);
> }
> + if (page_is_idle(page))
> + clear_page_idle(page);
> }
> EXPORT_SYMBOL(mark_page_accessed);
>
> --
> 2.1.4
>
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org
^ permalink raw reply
* Re: [PATCH -mm v8 7/7] proc: export idle flag via kpageflags
From: Andres Lagar-Cavilla @ 2015-07-15 19:17 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <024b60a19e5ef246c9af3c5ff7652e71576e0bcc.1436967694.git.vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Wed, Jul 15, 2015 at 6:54 AM, Vladimir Davydov
<vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> As noted by Minchan, a benefit of reading idle flag from
> /proc/kpageflags is that one can easily filter dirty and/or unevictable
> pages while estimating the size of unused memory.
>
> Note that idle flag read from /proc/kpageflags may be stale in case the
> page was accessed via a PTE, because it would be too costly to iterate
> over all page mappings on each /proc/kpageflags read to provide an
> up-to-date value. To make sure the flag is up-to-date one has to read
> /proc/kpageidle first.
>
> Signed-off-by: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Reviewed-by: Andres Lagar-Cavilla <andreslc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> Documentation/vm/pagemap.txt | 6 ++++++
> fs/proc/page.c | 3 +++
> include/uapi/linux/kernel-page-flags.h | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
> index c9266340852c..5896b7d7fd74 100644
> --- a/Documentation/vm/pagemap.txt
> +++ b/Documentation/vm/pagemap.txt
> @@ -64,6 +64,7 @@ There are five components to pagemap:
> 22. THP
> 23. BALLOON
> 24. ZERO_PAGE
> + 25. IDLE
>
> * /proc/kpagecgroup. This file contains a 64-bit inode number of the
> memory cgroup each page is charged to, indexed by PFN. Only available when
> @@ -124,6 +125,11 @@ Short descriptions to the page flags:
> 24. ZERO_PAGE
> zero page for pfn_zero or huge_zero page
>
> +25. IDLE
> + page has not been accessed since it was marked idle (see /proc/kpageidle)
> + Note that this flag may be stale in case the page was accessed via a PTE.
> + To make sure the flag is up-to-date one has to read /proc/kpageidle first.
> +
> [IO related page flags]
> 1. ERROR IO error occurred
> 3. UPTODATE page has up-to-date data
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 273537885ab4..13dcb823fe4e 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -150,6 +150,9 @@ u64 stable_page_flags(struct page *page)
> if (PageBalloon(page))
> u |= 1 << KPF_BALLOON;
>
> + if (page_is_idle(page))
> + u |= 1 << KPF_IDLE;
> +
> u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked);
>
> u |= kpf_copy_bit(k, KPF_SLAB, PG_slab);
> diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
> index a6c4962e5d46..5da5f8751ce7 100644
> --- a/include/uapi/linux/kernel-page-flags.h
> +++ b/include/uapi/linux/kernel-page-flags.h
> @@ -33,6 +33,7 @@
> #define KPF_THP 22
> #define KPF_BALLOON 23
> #define KPF_ZERO_PAGE 24
> +#define KPF_IDLE 25
>
>
> #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
> --
> 2.1.4
>
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org
^ permalink raw reply
* Re: [PATCH -mm v8 5/7] mmu-notifier: add clear_young callback
From: Andres Lagar-Cavilla @ 2015-07-15 19:16 UTC (permalink / raw)
To: Vladimir Davydov, Paolo Bonzini, kvm, Eric Northup
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <82693bd5b5dbf4e65657fa22288942650aa04a0a.1436967694.git.vdavydov@parallels.com>
On Wed, Jul 15, 2015 at 6:54 AM, Vladimir Davydov
<vdavydov@parallels.com> wrote:
> In the scope of the idle memory tracking feature, which is introduced by
> the following patch, we need to clear the referenced/accessed bit not
> only in primary, but also in secondary ptes. The latter is required in
> order to estimate wss of KVM VMs. At the same time we want to avoid
> flushing tlb, because it is quite expensive and it won't really affect
> the final result.
>
> Currently, there is no function for clearing pte young bit that would
> meet our requirements, so this patch introduces one. To achieve that we
> have to add a new mmu-notifier callback, clear_young, since there is no
> method for testing-and-clearing a secondary pte w/o flushing tlb. The
> new method is not mandatory and currently only implemented by KVM.
>
> Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
Reviewed-by: Andres Lagar-Cavilla <andreslc@google.com>
Added Paolo Bonzini, kvm list, Eric Northup.
> ---
> include/linux/mmu_notifier.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
> mm/mmu_notifier.c | 17 +++++++++++++++++
> virt/kvm/kvm_main.c | 18 ++++++++++++++++++
> 3 files changed, 79 insertions(+)
>
> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> index 61cd67f4d788..a5b17137c683 100644
> --- a/include/linux/mmu_notifier.h
> +++ b/include/linux/mmu_notifier.h
> @@ -66,6 +66,16 @@ struct mmu_notifier_ops {
> unsigned long end);
>
> /*
> + * clear_young is a lightweight version of clear_flush_young. Like the
> + * latter, it is supposed to test-and-clear the young/accessed bitflag
> + * in the secondary pte, but it may omit flushing the secondary tlb.
> + */
> + int (*clear_young)(struct mmu_notifier *mn,
> + struct mm_struct *mm,
> + unsigned long start,
> + unsigned long end);
> +
> + /*
> * test_young is called to check the young/accessed bitflag in
> * the secondary pte. This is used to know if the page is
> * frequently used without actually clearing the flag or tearing
> @@ -203,6 +213,9 @@ extern void __mmu_notifier_release(struct mm_struct *mm);
> extern int __mmu_notifier_clear_flush_young(struct mm_struct *mm,
> unsigned long start,
> unsigned long end);
> +extern int __mmu_notifier_clear_young(struct mm_struct *mm,
> + unsigned long start,
> + unsigned long end);
> extern int __mmu_notifier_test_young(struct mm_struct *mm,
> unsigned long address);
> extern void __mmu_notifier_change_pte(struct mm_struct *mm,
> @@ -231,6 +244,15 @@ static inline int mmu_notifier_clear_flush_young(struct mm_struct *mm,
> return 0;
> }
>
> +static inline int mmu_notifier_clear_young(struct mm_struct *mm,
> + unsigned long start,
> + unsigned long end)
> +{
> + if (mm_has_notifiers(mm))
> + return __mmu_notifier_clear_young(mm, start, end);
> + return 0;
> +}
> +
> static inline int mmu_notifier_test_young(struct mm_struct *mm,
> unsigned long address)
> {
> @@ -311,6 +333,28 @@ static inline void mmu_notifier_mm_destroy(struct mm_struct *mm)
> __young; \
> })
>
> +#define ptep_clear_young_notify(__vma, __address, __ptep) \
> +({ \
> + int __young; \
> + struct vm_area_struct *___vma = __vma; \
> + unsigned long ___address = __address; \
> + __young = ptep_test_and_clear_young(___vma, ___address, __ptep);\
> + __young |= mmu_notifier_clear_young(___vma->vm_mm, ___address, \
> + ___address + PAGE_SIZE); \
> + __young; \
> +})
> +
> +#define pmdp_clear_young_notify(__vma, __address, __pmdp) \
> +({ \
> + int __young; \
> + struct vm_area_struct *___vma = __vma; \
> + unsigned long ___address = __address; \
> + __young = pmdp_test_and_clear_young(___vma, ___address, __pmdp);\
> + __young |= mmu_notifier_clear_young(___vma->vm_mm, ___address, \
> + ___address + PMD_SIZE); \
> + __young; \
> +})
> +
> #define ptep_clear_flush_notify(__vma, __address, __ptep) \
> ({ \
> unsigned long ___addr = __address & PAGE_MASK; \
> diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
> index 3b9b3d0741b2..5fbdd367bbed 100644
> --- a/mm/mmu_notifier.c
> +++ b/mm/mmu_notifier.c
> @@ -123,6 +123,23 @@ int __mmu_notifier_clear_flush_young(struct mm_struct *mm,
> return young;
> }
>
> +int __mmu_notifier_clear_young(struct mm_struct *mm,
> + unsigned long start,
> + unsigned long end)
> +{
> + struct mmu_notifier *mn;
> + int young = 0, id;
> +
> + id = srcu_read_lock(&srcu);
> + hlist_for_each_entry_rcu(mn, &mm->mmu_notifier_mm->list, hlist) {
> + if (mn->ops->clear_young)
> + young |= mn->ops->clear_young(mn, mm, start, end);
> + }
> + srcu_read_unlock(&srcu, id);
> +
> + return young;
> +}
> +
> int __mmu_notifier_test_young(struct mm_struct *mm,
> unsigned long address)
> {
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 05148a43ef9c..61500cb028a3 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -388,6 +388,23 @@ static int kvm_mmu_notifier_clear_flush_young(struct mmu_notifier *mn,
> return young;
> }
>
> +static int kvm_mmu_notifier_clear_young(struct mmu_notifier *mn,
> + struct mm_struct *mm,
> + unsigned long start,
> + unsigned long end)
> +{
> + struct kvm *kvm = mmu_notifier_to_kvm(mn);
> + int young, idx;
For reclaim, the clear_flush_young notifier may blow up the secondary
pte to estimate the access pattern, depending on hardware support (EPT
access bits available in Haswell onwards, not sure about AMD, PPC,
etc).
This is ok, because it's reclaim, we need to know the access pattern,
chances are the page is a goner anyway.
However, not so sure about that cost in this context. Depending on
user-space, this will periodically tear down all EPT tables in the
system. That's tricky.
So please add a note to that effect, so in the fullness of time kvm
may be able to refuse enacting this notifier based on performance/VM
priority/foo concerns.
> +
> + idx = srcu_read_lock(&kvm->srcu);
> + spin_lock(&kvm->mmu_lock);
> + young = kvm_age_hva(kvm, start, end);
Also please add a comment along the lines of no one really knowing
when and if to flush the secondary tlb.
We might come up with a heuristic later, or leave up to the regular
system cadence. We just don't know at the moment.
Andres
> + spin_unlock(&kvm->mmu_lock);
> + srcu_read_unlock(&kvm->srcu, idx);
> +
> + return young;
> +}
> +
> static int kvm_mmu_notifier_test_young(struct mmu_notifier *mn,
> struct mm_struct *mm,
> unsigned long address)
> @@ -420,6 +437,7 @@ static const struct mmu_notifier_ops kvm_mmu_notifier_ops = {
> .invalidate_range_start = kvm_mmu_notifier_invalidate_range_start,
> .invalidate_range_end = kvm_mmu_notifier_invalidate_range_end,
> .clear_flush_young = kvm_mmu_notifier_clear_flush_young,
> + .clear_young = kvm_mmu_notifier_clear_young,
> .test_young = kvm_mmu_notifier_test_young,
> .change_pte = kvm_mmu_notifier_change_pte,
> .release = kvm_mmu_notifier_release,
> --
> 2.1.4
>
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc@google.com
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH -mm v8 4/7] proc: add kpagecgroup file
From: Andres Lagar-Cavilla @ 2015-07-15 19:03 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <c6cbd44b9d5127cdaaa6f7d330e9bf715ec55534.1436967694.git.vdavydov@parallels.com>
On Wed, Jul 15, 2015 at 6:54 AM, Vladimir Davydov
<vdavydov@parallels.com> wrote:
> /proc/kpagecgroup contains a 64-bit inode number of the memory cgroup
> each page is charged to, indexed by PFN. Having this information is
> useful for estimating a cgroup working set size.
>
> The file is present if CONFIG_PROC_PAGE_MONITOR && CONFIG_MEMCG.
>
> Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
> ---
> Documentation/vm/pagemap.txt | 6 ++++-
> fs/proc/page.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
> index 6bfbc172cdb9..a9b7afc8fbc6 100644
> --- a/Documentation/vm/pagemap.txt
> +++ b/Documentation/vm/pagemap.txt
> @@ -5,7 +5,7 @@ pagemap is a new (as of 2.6.25) set of interfaces in the kernel that allow
> userspace programs to examine the page tables and related information by
> reading files in /proc.
>
> -There are three components to pagemap:
> +There are four components to pagemap:
>
> * /proc/pid/pagemap. This file lets a userspace process find out which
> physical frame each virtual page is mapped to. It contains one 64-bit
> @@ -65,6 +65,10 @@ There are three components to pagemap:
> 23. BALLOON
> 24. ZERO_PAGE
>
> + * /proc/kpagecgroup. This file contains a 64-bit inode number of the
> + memory cgroup each page is charged to, indexed by PFN. Only available when
> + CONFIG_MEMCG is set.
> +
> Short descriptions to the page flags:
>
> 0. LOCKED
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 7eee2d8b97d9..70d23245dd43 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -9,6 +9,7 @@
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> #include <linux/hugetlb.h>
> +#include <linux/memcontrol.h>
> #include <linux/kernel-page-flags.h>
> #include <asm/uaccess.h>
> #include "internal.h"
> @@ -225,10 +226,62 @@ static const struct file_operations proc_kpageflags_operations = {
> .read = kpageflags_read,
> };
>
> +#ifdef CONFIG_MEMCG
> +static ssize_t kpagecgroup_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + u64 __user *out = (u64 __user *)buf;
> + struct page *ppage;
> + unsigned long src = *ppos;
> + unsigned long pfn;
> + ssize_t ret = 0;
> + u64 ino;
> +
> + pfn = src / KPMSIZE;
> + count = min_t(unsigned long, count, (max_pfn * KPMSIZE) - src);
> + if (src & KPMMASK || count & KPMMASK)
> + return -EINVAL;
> +
> + while (count > 0) {
> + if (pfn_valid(pfn))
> + ppage = pfn_to_page(pfn);
> + else
> + ppage = NULL;
> +
> + if (ppage)
> + ino = page_cgroup_ino(ppage);
> + else
> + ino = 0;
> +
For both /proc/kpage* interfaces you add (and more critically for the
rmap-causing one, kpageidle):
It's a good idea to do cond_sched(). Whether after each pfn, each Nth
pfn, each put_user, I leave to you, but a reasonable cadence is
needed, because user-space can call this on the entire physical
address space, and that's a lot of work to do without re-scheduling.
Andres
> + if (put_user(ino, out)) {
> + ret = -EFAULT;
> + break;
> + }
> +
> + pfn++;
> + out++;
> + count -= KPMSIZE;
> + }
> +
> + *ppos += (char __user *)out - buf;
> + if (!ret)
> + ret = (char __user *)out - buf;
> + return ret;
> +}
> +
> +static const struct file_operations proc_kpagecgroup_operations = {
> + .llseek = mem_lseek,
> + .read = kpagecgroup_read,
> +};
> +#endif /* CONFIG_MEMCG */
> +
> static int __init proc_page_init(void)
> {
> proc_create("kpagecount", S_IRUSR, NULL, &proc_kpagecount_operations);
> proc_create("kpageflags", S_IRUSR, NULL, &proc_kpageflags_operations);
> +#ifdef CONFIG_MEMCG
> + proc_create("kpagecgroup", S_IRUSR, NULL, &proc_kpagecgroup_operations);
> +#endif
> return 0;
> }
> fs_initcall(proc_page_init);
> --
> 2.1.4
>
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc@google.com
^ permalink raw reply
* Re: [PATCH -mm v8 2/7] hwpoison: use page_cgroup_ino for filtering by memcg
From: Andres Lagar-Cavilla @ 2015-07-15 19:00 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5c4d3594a5b0a79248fffae67ea677d54d06aacf.1436967694.git.vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Wed, Jul 15, 2015 at 6:54 AM, Vladimir Davydov
<vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> Hwpoison allows to filter pages by memory cgroup ino. Currently, it
> calls try_get_mem_cgroup_from_page to obtain the cgroup from a page and
> then its ino using cgroup_ino, but now we have an apter method for that,
> page_cgroup_ino, so use it instead.
>
> Signed-off-by: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Reviewed-by: Andres Lagar-Cavilla <andreslc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> mm/hwpoison-inject.c | 5 +----
> mm/memory-failure.c | 16 ++--------------
> 2 files changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/mm/hwpoison-inject.c b/mm/hwpoison-inject.c
> index bf73ac17dad4..5015679014c1 100644
> --- a/mm/hwpoison-inject.c
> +++ b/mm/hwpoison-inject.c
> @@ -45,12 +45,9 @@ static int hwpoison_inject(void *data, u64 val)
> /*
> * do a racy check with elevated page count, to make sure PG_hwpoison
> * will only be set for the targeted owner (or on a free page).
> - * We temporarily take page lock for try_get_mem_cgroup_from_page().
> * memory_failure() will redo the check reliably inside page lock.
> */
> - lock_page(hpage);
> err = hwpoison_filter(hpage);
> - unlock_page(hpage);
> if (err)
> goto put_out;
>
> @@ -126,7 +123,7 @@ static int pfn_inject_init(void)
> if (!dentry)
> goto fail;
>
> -#ifdef CONFIG_MEMCG_SWAP
> +#ifdef CONFIG_MEMCG
> dentry = debugfs_create_u64("corrupt-filter-memcg", 0600,
> hwpoison_dir, &hwpoison_filter_memcg);
> if (!dentry)
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 1cf7f2988422..97005396a507 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -130,27 +130,15 @@ static int hwpoison_filter_flags(struct page *p)
> * can only guarantee that the page either belongs to the memcg tasks, or is
> * a freed page.
> */
> -#ifdef CONFIG_MEMCG_SWAP
> +#ifdef CONFIG_MEMCG
> u64 hwpoison_filter_memcg;
> EXPORT_SYMBOL_GPL(hwpoison_filter_memcg);
> static int hwpoison_filter_task(struct page *p)
> {
> - struct mem_cgroup *mem;
> - struct cgroup_subsys_state *css;
> - unsigned long ino;
> -
> if (!hwpoison_filter_memcg)
> return 0;
>
> - mem = try_get_mem_cgroup_from_page(p);
> - if (!mem)
> - return -EINVAL;
> -
> - css = mem_cgroup_css(mem);
> - ino = cgroup_ino(css->cgroup);
> - css_put(css);
> -
> - if (ino != hwpoison_filter_memcg)
> + if (page_cgroup_ino(p) != hwpoison_filter_memcg)
> return -EINVAL;
>
> return 0;
> --
> 2.1.4
>
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org
^ permalink raw reply
* Re: [PATCH -mm v8 1/7] memcg: add page_cgroup_ino helper
From: Andres Lagar-Cavilla @ 2015-07-15 18:59 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <40a2af5afc7b70a133737226cd9e975df42936e7.1436967694.git.vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Wed, Jul 15, 2015 at 6:54 AM, Vladimir Davydov
<vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> This function returns the inode number of the closest online ancestor of
> the memory cgroup a page is charged to. It is required for exporting
> information about which page is charged to which cgroup to userspace,
> which will be introduced by a following patch.
>
> Signed-off-by: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Reviewed-by: Andres Lagar-Cavilla <andreslc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> include/linux/memcontrol.h | 1 +
> mm/memcontrol.c | 23 +++++++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 73b02b0a8f60..50069abebc3c 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -116,6 +116,7 @@ static inline bool mm_match_cgroup(struct mm_struct *mm,
>
> extern struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup *memcg);
> extern struct cgroup_subsys_state *mem_cgroup_css_from_page(struct page *page);
> +extern unsigned long page_cgroup_ino(struct page *page);
>
> struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *,
> struct mem_cgroup *,
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index acb93c554f6e..894dc2169979 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -631,6 +631,29 @@ struct cgroup_subsys_state *mem_cgroup_css_from_page(struct page *page)
> return &memcg->css;
> }
>
> +/**
> + * page_cgroup_ino - return inode number of the memcg a page is charged to
> + * @page: the page
> + *
> + * Look up the closest online ancestor of the memory cgroup @page is charged to
> + * and return its inode number or 0 if @page is not charged to any cgroup. It
> + * is safe to call this function without holding a reference to @page.
> + */
> +unsigned long page_cgroup_ino(struct page *page)
> +{
> + struct mem_cgroup *memcg;
> + unsigned long ino = 0;
> +
> + rcu_read_lock();
> + memcg = READ_ONCE(page->mem_cgroup);
> + while (memcg && !(memcg->css.flags & CSS_ONLINE))
> + memcg = parent_mem_cgroup(memcg);
> + if (memcg)
> + ino = cgroup_ino(memcg->css.cgroup);
> + rcu_read_unlock();
> + return ino;
> +}
> +
> static struct mem_cgroup_per_zone *
> mem_cgroup_page_zoneinfo(struct mem_cgroup *memcg, struct page *page)
> {
> --
> 2.1.4
>
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org
^ permalink raw reply
* Re: KDBUS breaks 'make installmandocs'
From: Jim Davis @ 2015-07-15 18:04 UTC (permalink / raw)
To: David Herrmann
Cc: Ulf Magnusson, Greg KH, Linux API, linux-kernel, Daniel Mack,
David Herrmann, Djalal Harouni
In-Reply-To: <CANq1E4QgzTFijvkfqa-sEYfVktJLEA3+oFh6Sm8NekLie6B9Ww-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Jul 15, 2015 at 7:19 AM, David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> diff --git a/Documentation/kdbus/Makefile b/Documentation/kdbus/Makefile
> index af87641..8caffe5 100644
> --- a/Documentation/kdbus/Makefile
> +++ b/Documentation/kdbus/Makefile
> @@ -38,3 +38,7 @@ mandocs: $(MANFILES)
> htmldocs: $(HTMLFILES)
>
> clean-files := $(MANFILES) $(HTMLFILES)
> +
> +# we don't support other %docs targets right now
> +%docs:
> + @true
Looks good, no errors with next-20150715. Thanks!
Jim
^ permalink raw reply
* Re: [PATCH v4 1/2] capabilities: Ambient capabilities
From: Andy Lutomirski @ 2015-07-15 17:09 UTC (permalink / raw)
To: James Morris
Cc: Andy Lutomirski, Andrew Morton, Serge E. Hallyn, Serge Hallyn,
James Morris, Jarkko Sakkinen, Ted Ts'o, Andrew G. Morgan,
Linux API, Mimi Zohar, Michael Kerrisk, Austin S Hemmelgarn,
linux-security-module, Aaron Jones, Serge Hallyn, LKML,
Markku Savela, Kees Cook, Jonathan Corbet, Christoph Lameter
In-Reply-To: <alpine.LRH.2.11.1507152033420.8351-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
On Wed, Jul 15, 2015 at 4:13 AM, James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org> wrote:
> On Tue, 14 Jul 2015, Andy Lutomirski wrote:
>
>> ===== The proposed change =====
>>
>> This patch adds a fifth capability mask called the ambient mask
>> (pA). pA does what most people expect pI to do.
>>
>
> This looks good, and I think it will lead to better overall security
> because people will find capabilities easier to use for real-world
> scenarios.
>
> Has it had enough security review? This is a significant new behavior
> being added to a widely enabled security module.
>
I've reviewed it, as have Kees and Serge. Of course, I wrote it, so
my review should be taken with many grains of salt.
I'll review it again between now and 4.3. All of my security thought
cycles are currently taken up by a really nasty bug that dwarfs and
problems this code could possibly introduce :)
--Andy
>
>
> - James
> --
> James Morris
> <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: nf_ct_sctp: minimal multihoming support
From: Pablo Neira Ayuso @ 2015-07-15 16:45 UTC (permalink / raw)
To: Michal Kubecek
Cc: netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
coreteam-Cap9r6Oaw4JrovVCs/uTlw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller
In-Reply-To: <20150714162850.GA8478-OEaqT8BN2ewCVLCxKZUutA@public.gmane.org>
On Tue, Jul 14, 2015 at 06:28:50PM +0200, Michal Kubecek wrote:
> On Tue, Jul 14, 2015 at 05:38:47PM +0200, Pablo Neira Ayuso wrote:
> > On Tue, Jul 14, 2015 at 02:23:11PM +0200, Michal Kubecek wrote:
> > > @@ -658,6 +696,18 @@ static struct ctl_table sctp_sysctl_table[] = {
> > > .mode = 0644,
> > > .proc_handler = proc_dointvec_jiffies,
> > > },
> > > + {
> > > + .procname = "nf_conntrack_sctp_timeout_heartbeat_sent",
> > > + .maxlen = sizeof(unsigned int),
> > > + .mode = 0644,
> > > + .proc_handler = proc_dointvec_jiffies,
> > > + },
> > > + {
> > > + .procname = "nf_conntrack_sctp_timeout_heartbeat_acked",
> > > + .maxlen = sizeof(unsigned int),
> > > + .mode = 0644,
> > > + .proc_handler = proc_dointvec_jiffies,
> > > + },
> > > { }
> > > };
> > >
> > > @@ -705,6 +755,18 @@ static struct ctl_table sctp_compat_sysctl_table[] = {
> > > .mode = 0644,
> > > .proc_handler = proc_dointvec_jiffies,
> > > },
> > > + {
> > > + .procname = "ip_conntrack_sctp_timeout_heartbeat_sent",
> > > + .maxlen = sizeof(unsigned int),
> > > + .mode = 0644,
> > > + .proc_handler = proc_dointvec_jiffies,
> > > + },
> > > + {
> > > + .procname = "ip_conntrack_sctp_timeout_heartbeat_acked",
> > > + .maxlen = sizeof(unsigned int),
> > > + .mode = 0644,
> > > + .proc_handler = proc_dointvec_jiffies,
> > > + },
> > > { }
> >
> > I don't see the nla_policy updates for the netlink cttimeout
> > interface.
>
> I didn't realize those were needed. It means adding two entries to
> sctp_timeout_nla_policy and two values to enum ctattr_timeout_sctp?
Exactly.
^ permalink raw reply
* [RFC PATCH 2/2] gpu: ipu-v3: use clock and de polarity from videomode
From: Manfred Schlaegl @ 2015-07-15 15:51 UTC (permalink / raw)
To: David Airlie, Philipp Zabel, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
Cc: dri-devel, linux-kernel, linux-api, Manfred Schlaegl,
Steve Longerbeam, Deepak Das, Jiada Wang, linux-fbdev
In-Reply-To: <55A67FDB.8010602@gmx.at>
This patch depends on "drm: add support for for clk and de polarity".
Since "drm: add support for for clk and de polarity", clock and data
polarity set in devicetree are passed correctly through drm_display_mode
to videomode flags used by ipuv3.
Removes custom configuration flags for clock and de polarities and use
polarity flags from given videomode.
Keeps compatibility to current settings and device tree descriptions,
by setting data enable polarity to high, if not explicitly set to low by
flags.
display-timings resulting polarity of pixel clock
past now
pixelclk-active unset 0 0
pixelclk-active = 0 0 0
pixelclk-active = 1 0 1 <--- corrected
display-timings resulting polarity of data enable
past now
de-active unset 1 1
de-active = 0 1 0 <--- corrected
de-active = 1 1 1
Tested on Freescale i.MX53(parallel) and i.MX6(LVDS).
Signed-off-by: Manfred Schlaegl <manfred.schlaegl@gmx.at>
---
drivers/gpu/drm/imx/ipuv3-crtc.c | 2 --
drivers/gpu/ipu-v3/ipu-di.c | 4 ++--
include/video/imx-ipu-v3.h | 2 --
3 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 7bc8301..6813271 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -172,8 +172,6 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
else
sig_cfg.clkflags = 0;
- sig_cfg.enable_pol = 1;
- sig_cfg.clk_pol = 0;
sig_cfg.bus_format = ipu_crtc->bus_format;
sig_cfg.v_to_h_sync = 0;
sig_cfg.hsync_pin = ipu_crtc->di_hsync_pin;
diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index 2970c6b..0b1a7d2 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -621,7 +621,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
}
}
- if (sig->clk_pol)
+ if (sig->mode.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
di_gen |= DI_GEN_POLARITY_DISP_CLK;
ipu_di_write(di, di_gen, DI_GENERAL);
@@ -632,7 +632,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
reg = ipu_di_read(di, DI_POL);
reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
- if (sig->enable_pol)
+ if (!(sig->mode.flags & DISPLAY_FLAGS_DE_LOW))
reg |= DI_POL_DRDY_POLARITY_15;
if (sig->data_pol)
reg |= DI_POL_DRDY_DATA_POLARITY;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index 85dedca..4178fa2 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -34,8 +34,6 @@ enum ipuv3_type {
*/
struct ipu_di_signal_cfg {
unsigned data_pol:1; /* true = inverted */
- unsigned clk_pol:1; /* true = rising edge */
- unsigned enable_pol:1;
struct videomode mode;
--
2.1.4
^ permalink raw reply related
* [RFC PATCH 1/2] drm: add support for for clk and de polarity
From: Manfred Schlaegl @ 2015-07-15 15:50 UTC (permalink / raw)
To: David Airlie, Philipp Zabel, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Manfred Schlaegl,
Steve Longerbeam, Deepak Das, Jiada Wang,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <55A67FDB.8010602-RbZlAiThDcE@public.gmane.org>
To get full support for parallel and LVDS displays with drm:
Add representation for clock and data enable polarity in drm_display_mode
flags (similar to HSYNC/VSYNC polarity) and update conversion functions
from/to videomode accordingly.
This is especially important for embedded devices where parallel(RGB) and
LVDS displays are still widely used and drm already plays an important
role.
Tested on Freescale i.MX53(parallel) and i.MX6(LVDS).
Background:
There was the ability to set polarity of clock and data enable signals
in devicetree(display-timing), struct display_timing and struct videomode,
but there was no representation for this in struct drm_display_mode.
Example on Freescale i.MX53/i.MX6 SoC's:
* A parallel display using different clock polarity is set up using
display-timing in devicetree
* ipuv3 parallel outputs clock with wrong polarity
Signed-off-by: Manfred Schlaegl <manfred.schlaegl-RbZlAiThDcE@public.gmane.org>
---
drivers/gpu/drm/drm_modes.c | 16 ++++++++++++++++
include/uapi/drm/drm_mode.h | 5 +++++
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index cd74a09..dbb28b7 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -605,6 +605,14 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
dmode->flags |= DRM_MODE_FLAG_PVSYNC;
else if (vm->flags & DISPLAY_FLAGS_VSYNC_LOW)
dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+ if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+ dmode->flags |= DRM_MODE_FLAG_PDE;
+ else if (vm->flags & DISPLAY_FLAGS_DE_LOW)
+ dmode->flags |= DRM_MODE_FLAG_NDE;
+ if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+ dmode->flags |= DRM_MODE_FLAG_PPIXDATA;
+ else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+ dmode->flags |= DRM_MODE_FLAG_NPIXDATA;
if (vm->flags & DISPLAY_FLAGS_INTERLACED)
dmode->flags |= DRM_MODE_FLAG_INTERLACE;
if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
@@ -646,6 +654,14 @@ void drm_display_mode_to_videomode(const struct drm_display_mode *dmode,
vm->flags |= DISPLAY_FLAGS_VSYNC_HIGH;
else if (dmode->flags & DRM_MODE_FLAG_NVSYNC)
vm->flags |= DISPLAY_FLAGS_VSYNC_LOW;
+ if (dmode->flags & DRM_MODE_FLAG_PDE)
+ vm->flags |= DISPLAY_FLAGS_DE_HIGH;
+ else if (dmode->flags & DRM_MODE_FLAG_NDE)
+ vm->flags |= DISPLAY_FLAGS_DE_LOW;
+ if (dmode->flags & DRM_MODE_FLAG_PPIXDATA)
+ vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
+ else if (dmode->flags & DRM_MODE_FLAG_NPIXDATA)
+ vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
if (dmode->flags & DRM_MODE_FLAG_INTERLACE)
vm->flags |= DISPLAY_FLAGS_INTERLACED;
if (dmode->flags & DRM_MODE_FLAG_DBLSCAN)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 359107a..cb4912b 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -72,6 +72,11 @@
#define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH (6<<14)
#define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14)
#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14)
+/* flags for display data enable and clock polarity */
+#define DRM_MODE_FLAG_PDE (1<<19)
+#define DRM_MODE_FLAG_NDE (1<<20)
+#define DRM_MODE_FLAG_PPIXDATA (1<<21)
+#define DRM_MODE_FLAG_NPIXDATA (1<<22)
/* DPMS flags */
--
2.1.4
^ permalink raw reply related
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