* Re: [PATCH v5 1/5] block: add ioprio_check_cap function
From: Jeff Moyer @ 2018-05-21 20:58 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api, hch
In-Reply-To: <20180521160147.2372-2-adam.manzanares@wdc.com>
adam.manzanares@wdc.com writes:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> Aio per command iopriority support introduces a second interface between
> userland and the kernel capable of passing iopriority. The aio interface also
> needs the ability to verify that the submitting context has sufficient
> priviledges to submit IOPRIO_RT commands. This patch creates the
> ioprio_check_cap function to be used by the ioprio_set system call and also by
> the aio interface.
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
> ---
> block/ioprio.c | 22 ++++++++++++++++------
> include/linux/ioprio.h | 2 ++
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/block/ioprio.c b/block/ioprio.c
> index 6f5d0b6625e3..f9821080c92c 100644
> --- a/block/ioprio.c
> +++ b/block/ioprio.c
> @@ -61,15 +61,10 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
> }
> EXPORT_SYMBOL_GPL(set_task_ioprio);
>
> -SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
> +int ioprio_check_cap(int ioprio)
> {
> int class = IOPRIO_PRIO_CLASS(ioprio);
> int data = IOPRIO_PRIO_DATA(ioprio);
> - struct task_struct *p, *g;
> - struct user_struct *user;
> - struct pid *pgrp;
> - kuid_t uid;
> - int ret;
>
> switch (class) {
> case IOPRIO_CLASS_RT:
> @@ -92,6 +87,21 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
> return -EINVAL;
> }
>
> + return 0;
> +}
> +
> +SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
> +{
> + struct task_struct *p, *g;
> + struct user_struct *user;
> + struct pid *pgrp;
> + kuid_t uid;
> + int ret;
> +
> + ret = ioprio_check_cap(ioprio);
> + if (ret)
> + return ret;
> +
> ret = -ESRCH;
> rcu_read_lock();
> switch (which) {
> diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h
> index 627efac73e6d..4a28cec49ec3 100644
> --- a/include/linux/ioprio.h
> +++ b/include/linux/ioprio.h
> @@ -77,4 +77,6 @@ extern int ioprio_best(unsigned short aprio, unsigned short bprio);
>
> extern int set_task_ioprio(struct task_struct *task, int ioprio);
>
> +extern int ioprio_check_cap(int ioprio);
> +
> #endif
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v5 0/5] AIO add per-command iopriority
From: Jeff Moyer @ 2018-05-21 20:57 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api, hch
In-Reply-To: <20180521160147.2372-1-adam.manzanares@wdc.com>
Hi, Adam,
adam.manzanares@wdc.com writes:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> This is the per-I/O equivalent of the ioprio_set system call.
> See the following link for performance implications on a SATA HDD:
> https://lkml.org/lkml/2016/12/6/495
>
> First patch factors ioprio_check_cap function out of ioprio_set system call to
> also be used by the aio ioprio interface.
>
> Second patch converts kiocb ki_hint field to a u16 to avoid kiocb bloat.
>
> Third patch passes ioprio hint from aio iocb to kiocb and inititalizes kiocb
> ioprio value appropriately when it is not explicitly set.
>
> Fourth patch enables the feature for blkdev.
>
> Fifth patch enables the feature for iomap direct IO
>
> Note: this work is based on top of linux-vfs/for-next
I'll cook up a libaio test case. Can you put together a man-pages
update for this?
Thanks!
Jeff
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 13/13] debug audit: read container ID of a process
From: Paul Moore @ 2018-05-21 20:06 UTC (permalink / raw)
To: Eric W. Biederman, Steve Grubb, Richard Guy Briggs
Cc: simo, jlayton, linux-api, containers, LKML, Eric Paris, dhowells,
carlos, linux-audit, viro, luto, netdev, linux-fsdevel, cgroups,
serge
In-Reply-To: <87muwshl4z.fsf@xmission.com>
On Mon, May 21, 2018 at 3:19 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Steve Grubb <sgrubb@redhat.com> writes:
>
>> On Friday, March 16, 2018 5:00:40 AM EDT Richard Guy Briggs wrote:
>>> Add support for reading the container ID from the proc filesystem.
>>
>> I think this could be useful in general. Please consider this to be part of
>> the full patch set and not something merely used to debug the patches.
>
> Only with an audit specific name.
>
> As it is:
>
> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> The truth is the containerid name really stinks and is quite confusing
> and does not imply that the label applies only to audit. And little
> things like this make me extremely uncofortable with it.
It also makes the audit container ID (notice how I *always* call it
the *audit* container ID? that is not an accident) available for
userspace applications to abuse. Perhaps in the future we can look at
ways to make this more available to applications, but this patch is
not the answer.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 13/13] debug audit: read container ID of a process
From: Eric W. Biederman @ 2018-05-21 19:19 UTC (permalink / raw)
To: Steve Grubb
Cc: simo-H+wXaHxf7aLQT0dZR+AlfA, jlayton-H+wXaHxf7aLQT0dZR+AlfA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
eparis-FjpueFixGhCM4zKIHC2jIg, dhowells-H+wXaHxf7aLQT0dZR+AlfA,
carlos-H+wXaHxf7aLQT0dZR+AlfA, linux-audit-H+wXaHxf7aLQT0dZR+AlfA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
luto-DgEjT+Ai2ygdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3001737.MkQ41rgtZF@x2>
Steve Grubb <sgrubb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
> On Friday, March 16, 2018 5:00:40 AM EDT Richard Guy Briggs wrote:
>> Add support for reading the container ID from the proc filesystem.
>
> I think this could be useful in general. Please consider this to be part of
> the full patch set and not something merely used to debug the patches.
Only with an audit specific name.
As it is:
Nacked-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
The truth is the containerid name really stinks and is quite confusing
and does not imply that the label applies only to audit. And little
things like this make me extremely uncofortable with it.
Eric
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 03/13] audit: log container info of syscalls
From: Steve Grubb @ 2018-05-21 19:19 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, eparis, serge
In-Reply-To: <20180517214102.qhg4gofwrbsn2eru@madcap2.tricolour.ca>
On Thursday, May 17, 2018 5:41:02 PM EDT Richard Guy Briggs wrote:
> On 2018-05-17 17:09, Steve Grubb wrote:
> > On Fri, 16 Mar 2018 05:00:30 -0400
> >
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> > > Create a new audit record AUDIT_CONTAINER_INFO to document the
> > > container ID of a process if it is present.
> >
> > As mentioned in a previous email, I think AUDIT_CONTAINER is more
> > suitable for the container record. One more comment below...
> >
> > > Called from audit_log_exit(), syscalls are covered.
> > >
> > > A sample raw event:
> > > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257
> > > success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2
> > > ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
> > > sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash"
> > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> > > key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257):
> > > cwd="/root" type=PATH msg=audit(1519924845.499:257): item=0
> > > name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0
> > > rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT
> > > cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > > type=PATH msg=audit(1519924845.499:257): item=1
> > > name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0
> > > ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0
> > > nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000
> > > cap_fe=0 cap_fver=0 type=PROCTITLE msg=audit(1519924845.499:257):
> > > proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F7
> > > 46D702F746D70636F6E7461696E65726964 type=CONTAINER_INFO
> > > msg=audit(1519924845.499:257): op=task
> > > contid=123458
> > >
> > > See: https://github.com/linux-audit/audit-kernel/issues/32
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > >
> > > include/linux/audit.h | 5 +++++
> > > include/uapi/linux/audit.h | 1 +
> > > kernel/audit.c | 20 ++++++++++++++++++++
> > > kernel/auditsc.c | 2 ++
> > > 4 files changed, 28 insertions(+)
> > >
> > > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > > index fe4ba3f..3acbe9d 100644
> > > --- a/include/linux/audit.h
> > > +++ b/include/linux/audit.h
> > > @@ -154,6 +154,8 @@ extern void
> > > audit_log_link_denied(const char *operation, extern int
> > > audit_log_task_context(struct audit_buffer *ab); extern void
> > > audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk);
> > > +extern int audit_log_container_info(struct task_struct *tsk,
> > > + struct audit_context *context);
> > >
> > > extern int audit_update_lsm_rules(void);
> > >
> > > @@ -205,6 +207,9 @@ static inline int audit_log_task_context(struct
> > > audit_buffer *ab) static inline void audit_log_task_info(struct
> > > audit_buffer *ab, struct task_struct *tsk)
> > >
> > > { }
> > >
> > > +static inline int audit_log_container_info(struct task_struct *tsk,
> > > + struct audit_context
> > > *context); +{ }
> > >
> > > #define audit_enabled 0
> > > #endif /* CONFIG_AUDIT */
> > >
> > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > index 921a71f..e83ccbd 100644
> > > --- a/include/uapi/linux/audit.h
> > > +++ b/include/uapi/linux/audit.h
> > > @@ -115,6 +115,7 @@
> > >
> > > #define AUDIT_REPLACE 1329 /* Replace auditd
> > >
> > > if this packet unanswerd */ #define AUDIT_KERN_MODULE
> > > 1330 /* Kernel Module events */ #define
> > > AUDIT_FANOTIFY 1331 /* Fanotify access decision
> > > */ +#define AUDIT_CONTAINER_INFO 1332 /* Container ID
> > > information */ #define AUDIT_AVC 1400 /* SE
> > > Linux avc denial or grant */ #define AUDIT_SELINUX_ERR
> > > 1401 /* Internal SE Linux Errors */ diff --git
> > > a/kernel/audit.c b/kernel/audit.c index 3f2f143..a12f21f 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -2049,6 +2049,26 @@ void audit_log_session_info(struct
> > > audit_buffer *ab) audit_log_format(ab, " auid=%u ses=%u", auid,
> > > sessionid); }
> > >
> > > +/*
> > > + * audit_log_container_info - report container info
> > > + * @tsk: task to be recorded
> > > + * @context: task or local context for record
> > > + */
> > > +int audit_log_container_info(struct task_struct *tsk, struct
> > > audit_context *context) +{
> > > + struct audit_buffer *ab;
> > > +
> > > + if (!audit_containerid_set(tsk))
> > > + return 0;
> > > + /* Generate AUDIT_CONTAINER_INFO with container ID */
> > > + ab = audit_log_start(context, GFP_KERNEL,
> > > AUDIT_CONTAINER_INFO);
> > > + if (!ab)
> > > + return -ENOMEM;
> > > + audit_log_format(ab, "contid=%llu",
> > > audit_get_containerid(tsk));
> > > + audit_log_end(ab);
> > > + return 0;
> > > +}
> > > +
> > >
> > > void audit_log_key(struct audit_buffer *ab, char *key)
> > > {
> > >
> > > audit_log_format(ab, " key=");
> > >
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index a6b0a52..65be110 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -1453,6 +1453,8 @@ static void audit_log_exit(struct audit_context
> > > *context, struct task_struct *ts
> > >
> > > audit_log_proctitle(tsk, context);
> > >
> > > + audit_log_container_info(tsk, context);
> >
> > Would there be any problem moving audit_log_container_info before
> > audit_log_proctitle? There are some assumptions that proctitle is the
> > last record in some situations.
>
> I see no problem doing that.
Actually...just leave it as is. I have to fix things for simple events and
they do not have a proctitle record. So, leave it as you intended and I'll
work around this on my end.
Thanks,
-Steve
> > > /* Send end of event record to help user space know we are
> > >
> > > finished */ ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
> > >
> > > if (ab)
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 13/13] debug audit: read container ID of a process
From: Steve Grubb @ 2018-05-21 19:16 UTC (permalink / raw)
To: linux-audit
Cc: Richard Guy Briggs, cgroups, containers, linux-api, linux-fsdevel,
LKML, netdev, ebiederm, luto, jlayton, carlos, dhowells, viro,
simo, eparis, serge
In-Reply-To: <1081821010c124fe4e35984ec3dac1654453bb7c.1521179281.git.rgb@redhat.com>
On Friday, March 16, 2018 5:00:40 AM EDT Richard Guy Briggs wrote:
> Add support for reading the container ID from the proc filesystem.
I think this could be useful in general. Please consider this to be part of
the full patch set and not something merely used to debug the patches.
-Steve
> This is a read from the proc entry of the form /proc/PID/containerid
> where PID is the process ID of the task whose container ID is sought.
>
> The read expects up to a u64 value (unset: 18446744073709551615).
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> fs/proc/base.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 6ce4fbe..f66d1e2 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1300,6 +1300,21 @@ static ssize_t proc_sessionid_read(struct file *
> file, char __user * buf, .llseek = generic_file_llseek,
> };
>
> +static ssize_t proc_containerid_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct inode *inode = file_inode(file);
> + struct task_struct *task = get_proc_task(inode);
> + ssize_t length;
> + char tmpbuf[TMPBUFLEN*2];
> +
> + if (!task)
> + return -ESRCH;
> + length = scnprintf(tmpbuf, TMPBUFLEN*2, "%llu",
> audit_get_containerid(task)); + put_task_struct(task);
> + return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
> +}
> +
> static ssize_t proc_containerid_write(struct file *file, const char __user
> *buf, size_t count, loff_t *ppos)
> {
> @@ -1330,6 +1345,7 @@ static ssize_t proc_containerid_write(struct file
> *file, const char __user *buf, }
>
> static const struct file_operations proc_containerid_operations = {
> + .read = proc_containerid_read,
> .write = proc_containerid_write,
> .llseek = generic_file_llseek,
> };
> @@ -2996,7 +3012,7 @@ static int proc_pid_patch_state(struct seq_file *m,
> struct pid_namespace *ns, #ifdef CONFIG_AUDITSYSCALL
> REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations),
> REG("sessionid", S_IRUGO, proc_sessionid_operations),
> - REG("containerid", S_IWUSR, proc_containerid_operations),
> + REG("containerid", S_IWUSR|S_IRUSR, proc_containerid_operations),
> #endif
> #ifdef CONFIG_FAULT_INJECTION
> REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
> @@ -3391,7 +3407,7 @@ static int proc_tid_comm_permission(struct inode
> *inode, int mask) #ifdef CONFIG_AUDITSYSCALL
> REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations),
> REG("sessionid", S_IRUGO, proc_sessionid_operations),
> - REG("containerid", S_IWUSR, proc_containerid_operations),
> + REG("containerid", S_IWUSR|S_IRUSR, proc_containerid_operations),
> #endif
> #ifdef CONFIG_FAULT_INJECTION
> REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
^ permalink raw reply
* Re: [PATCH v5 04/28] fpga: mgr: add compat_id support
From: Alan Tull @ 2018-05-21 17:33 UTC (permalink / raw)
To: Wu Hao
Cc: Moritz Fischer, linux-fpga, linux-kernel, linux-api, Kang, Luwei,
Zhang, Yi Z, Liu, Song
In-Reply-To: <20180521030307.GA26500@hao-dev>
On Sun, May 20, 2018 at 10:03 PM, Wu Hao <hao.wu@intel.com> wrote:
> On Mon, May 07, 2018 at 04:09:06PM -0500, Alan Tull wrote:
>> On Tue, May 1, 2018 at 9:50 PM, Wu Hao <hao.wu@intel.com> wrote:
>>
>> Hi Hao,
>>
>> Looks good!
>>
>> > This patch introduces compat_id support to fpga manager, it adds
>> > a fpga_compat_id pointer to fpga manager data structure to allow
>> > fpga manager drivers to save the compatibility id. This compat_id
>> > could be used for compatibility checking before doing partial
>> > reconfiguration to associated fpga regions.
>> >
>> > Signed-off-by: Wu Hao <hao.wu@intel.com>
>> Acked-by: Alan Tull <atull@kernel.org>
>
> Hi Alan
>
> Thanks a lot for the acked-by.
>
> Did you get a chance to look into other patches?
What I'm looking for mostly is: is it clear that this code was written
to be reused. What do you think? Was it? Is there a way that intent
could be made clear in the code? This patchset has a history of being
a one-off solution for a single platform, doing things to work around
the FPGA framework instead of doing what the framework was intended to
do. The FPGA framework was written so that any FPGA could be used
with interface. Currently in the upstream that means any of the
supported FPGAs can be programmed with the same of-fpga-region code.
It didn't have to get rewritten for each fpga.
This patchset adds 5000 lines and I understand that another 4000 is
coming to add to this. Has that been written so that the upper layer
can be reused? Or will the 'reusable' version be another huge
patchset? Do you see my point? Up to this point I've been trying to
help figure out what changes could make this reusable. If you could
get with someone to take responsibility for architecting this patchset
to be clearly reusable, that could speed things up.
If there are improvements to the current FPGA framework that can help
this work, I'm interested and open to suggestions/code in that
direction..
Alan
>
> Thanks
> Hao
^ permalink raw reply
* [PATCH v5 5/5] fs: iomap dio set bio prio from kiocb prio
From: adam.manzanares @ 2018-05-21 16:01 UTC (permalink / raw)
To: viro, linux-fsdevel, axboe, bcrl
Cc: tglx, mingo, pombredanne, kstewart, gregkh, bigeasy, jack,
darrick.wong, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api, hch, Adam Manzanares
In-Reply-To: <20180521160147.2372-1-adam.manzanares@wdc.com>
From: Adam Manzanares <adam.manzanares@wdc.com>
Now that kiocb has an ioprio field copy this over to the bio when it is
created from the kiocb during direct IO.
Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
fs/iomap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/iomap.c b/fs/iomap.c
index afd163586aa0..65aae194aeca 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -919,6 +919,7 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
bio->bi_iter.bi_sector =
(iomap->addr + pos - iomap->offset) >> 9;
bio->bi_write_hint = dio->iocb->ki_hint;
+ bio->bi_ioprio = dio->iocb->ki_ioprio;
bio->bi_private = dio;
bio->bi_end_io = iomap_dio_bio_end_io;
--
2.15.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH v5 4/5] fs: blkdev set bio prio from kiocb prio
From: adam.manzanares @ 2018-05-21 16:01 UTC (permalink / raw)
To: viro, linux-fsdevel, axboe, bcrl
Cc: tglx, mingo, pombredanne, kstewart, gregkh, bigeasy, jack,
darrick.wong, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api, hch, Adam Manzanares
In-Reply-To: <20180521160147.2372-1-adam.manzanares@wdc.com>
From: Adam Manzanares <adam.manzanares@wdc.com>
Now that kiocb has an ioprio field copy this over to the bio when it is
created from the kiocb.
Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
fs/block_dev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7ec920e27065..da1e94d2bb75 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -355,6 +355,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
bio->bi_write_hint = iocb->ki_hint;
bio->bi_private = dio;
bio->bi_end_io = blkdev_bio_end_io;
+ bio->bi_ioprio = iocb->ki_ioprio;
ret = bio_iov_iter_get_pages(bio, iter);
if (unlikely(ret)) {
--
2.15.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH v5 3/5] fs: Add aio iopriority support
From: adam.manzanares @ 2018-05-21 16:01 UTC (permalink / raw)
To: viro, linux-fsdevel, axboe, bcrl
Cc: tglx, mingo, pombredanne, kstewart, gregkh, bigeasy, jack,
darrick.wong, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api, hch, Adam Manzanares
In-Reply-To: <20180521160147.2372-1-adam.manzanares@wdc.com>
From: Adam Manzanares <adam.manzanares@wdc.com>
This is the per-I/O equivalent of the ioprio_set system call.
When IOCB_FLAG_IOPRIO is set on the iocb aio_flags field, then we set the
newly added kiocb ki_ioprio field to the value in the iocb aio_reqprio field.
We set the blkdev bio iopriority unconditionally, so we need to guarantee the
kiocb is initialized properly. Added changes to the loopback driver and
init_sync_kiocb to achieve this.
This patch depends on block: add ioprio_check_cap function.
Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
drivers/block/loop.c | 3 +++
fs/aio.c | 16 ++++++++++++++++
include/linux/fs.h | 3 +++
include/uapi/linux/aio_abi.h | 1 +
4 files changed, 23 insertions(+)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 5d4e31655d96..dd98dfd97f5e 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -76,6 +76,8 @@
#include <linux/miscdevice.h>
#include <linux/falloc.h>
#include <linux/uio.h>
+#include <linux/ioprio.h>
+
#include "loop.h"
#include <linux/uaccess.h>
@@ -559,6 +561,7 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
cmd->iocb.ki_filp = file;
cmd->iocb.ki_complete = lo_rw_aio_complete;
cmd->iocb.ki_flags = IOCB_DIRECT;
+ cmd->iocb.ki_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
if (cmd->css)
kthread_associate_blkcg(cmd->css);
diff --git a/fs/aio.c b/fs/aio.c
index f3eae5d5771b..44b4572be524 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1451,6 +1451,22 @@ static int aio_prep_rw(struct kiocb *req, struct iocb *iocb)
if (iocb->aio_flags & IOCB_FLAG_RESFD)
req->ki_flags |= IOCB_EVENTFD;
req->ki_hint = file_write_hint(req->ki_filp);
+ if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
+ /*
+ * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
+ * aio_reqprio is interpreted as an I/O scheduling
+ * class and priority.
+ */
+ ret = ioprio_check_cap(iocb->aio_reqprio);
+ if (ret) {
+ pr_debug("aio ioprio check cap error\n");
+ return -EINVAL;
+ }
+
+ req->ki_ioprio = iocb->aio_reqprio;
+ } else
+ req->ki_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
+
ret = kiocb_set_rw_flags(req, iocb->aio_rw_flags);
if (unlikely(ret))
fput(req->ki_filp);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 50de40dbbb85..73b749ed3ea1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -36,6 +36,7 @@
#include <linux/delayed_call.h>
#include <linux/uuid.h>
#include <linux/errseq.h>
+#include <linux/ioprio.h>
#include <asm/byteorder.h>
#include <uapi/linux/fs.h>
@@ -302,6 +303,7 @@ struct kiocb {
void *private;
int ki_flags;
u16 ki_hint;
+ u16 ki_ioprio; /* See linux/ioprio.h */
} __randomize_layout;
static inline bool is_sync_kiocb(struct kiocb *kiocb)
@@ -1942,6 +1944,7 @@ static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
.ki_filp = filp,
.ki_flags = iocb_flags(filp),
.ki_hint = ki_hint_validate(file_write_hint(filp)),
+ .ki_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0),
};
}
diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
index 2c0a3415beee..d4e768d55d14 100644
--- a/include/uapi/linux/aio_abi.h
+++ b/include/uapi/linux/aio_abi.h
@@ -55,6 +55,7 @@ enum {
* is valid.
*/
#define IOCB_FLAG_RESFD (1 << 0)
+#define IOCB_FLAG_IOPRIO (1 << 1)
/* read() from /dev/aio returns these structures. */
struct io_event {
--
2.15.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH v5 2/5] fs: Convert kiocb rw_hint from enum to u16
From: adam.manzanares @ 2018-05-21 16:01 UTC (permalink / raw)
To: viro, linux-fsdevel, axboe, bcrl
Cc: tglx, mingo, pombredanne, kstewart, gregkh, bigeasy, jack,
darrick.wong, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api, hch, Adam Manzanares
In-Reply-To: <20180521160147.2372-1-adam.manzanares@wdc.com>
From: Adam Manzanares <adam.manzanares@wdc.com>
In order to avoid kiocb bloat for per command iopriority support, rw_hint
is converted from enum to a u16. Added a guard around ki_hint assigment.
Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
include/linux/fs.h | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7f07977bdfd7..50de40dbbb85 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -284,6 +284,8 @@ enum rw_hint {
WRITE_LIFE_EXTREME = RWH_WRITE_LIFE_EXTREME,
};
+#define MAX_KI_HINT ((1 << 16) - 1) /* ki_hint type is u16 */
+
#define IOCB_EVENTFD (1 << 0)
#define IOCB_APPEND (1 << 1)
#define IOCB_DIRECT (1 << 2)
@@ -299,7 +301,7 @@ struct kiocb {
void (*ki_complete)(struct kiocb *iocb, long ret, long ret2);
void *private;
int ki_flags;
- enum rw_hint ki_hint;
+ u16 ki_hint;
} __randomize_layout;
static inline bool is_sync_kiocb(struct kiocb *kiocb)
@@ -1927,12 +1929,19 @@ static inline enum rw_hint file_write_hint(struct file *file)
static inline int iocb_flags(struct file *file);
+static inline u16 ki_hint_validate(enum rw_hint hint)
+{
+ if (hint > MAX_KI_HINT)
+ return 0;
+ return hint;
+}
+
static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
{
*kiocb = (struct kiocb) {
.ki_filp = filp,
.ki_flags = iocb_flags(filp),
- .ki_hint = file_write_hint(filp),
+ .ki_hint = ki_hint_validate(file_write_hint(filp)),
};
}
--
2.15.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH v5 1/5] block: add ioprio_check_cap function
From: adam.manzanares @ 2018-05-21 16:01 UTC (permalink / raw)
To: viro, linux-fsdevel, axboe, bcrl
Cc: tglx, mingo, pombredanne, kstewart, gregkh, bigeasy, jack,
darrick.wong, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api, hch, Adam Manzanares
In-Reply-To: <20180521160147.2372-1-adam.manzanares@wdc.com>
From: Adam Manzanares <adam.manzanares@wdc.com>
Aio per command iopriority support introduces a second interface between
userland and the kernel capable of passing iopriority. The aio interface also
needs the ability to verify that the submitting context has sufficient
priviledges to submit IOPRIO_RT commands. This patch creates the
ioprio_check_cap function to be used by the ioprio_set system call and also by
the aio interface.
Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
block/ioprio.c | 22 ++++++++++++++++------
include/linux/ioprio.h | 2 ++
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/block/ioprio.c b/block/ioprio.c
index 6f5d0b6625e3..f9821080c92c 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -61,15 +61,10 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
}
EXPORT_SYMBOL_GPL(set_task_ioprio);
-SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
+int ioprio_check_cap(int ioprio)
{
int class = IOPRIO_PRIO_CLASS(ioprio);
int data = IOPRIO_PRIO_DATA(ioprio);
- struct task_struct *p, *g;
- struct user_struct *user;
- struct pid *pgrp;
- kuid_t uid;
- int ret;
switch (class) {
case IOPRIO_CLASS_RT:
@@ -92,6 +87,21 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
return -EINVAL;
}
+ return 0;
+}
+
+SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
+{
+ struct task_struct *p, *g;
+ struct user_struct *user;
+ struct pid *pgrp;
+ kuid_t uid;
+ int ret;
+
+ ret = ioprio_check_cap(ioprio);
+ if (ret)
+ return ret;
+
ret = -ESRCH;
rcu_read_lock();
switch (which) {
diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h
index 627efac73e6d..4a28cec49ec3 100644
--- a/include/linux/ioprio.h
+++ b/include/linux/ioprio.h
@@ -77,4 +77,6 @@ extern int ioprio_best(unsigned short aprio, unsigned short bprio);
extern int set_task_ioprio(struct task_struct *task, int ioprio);
+extern int ioprio_check_cap(int ioprio);
+
#endif
--
2.15.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH v5 0/5] AIO add per-command iopriority
From: adam.manzanares @ 2018-05-21 16:01 UTC (permalink / raw)
To: viro, linux-fsdevel, axboe, bcrl
Cc: tglx, mingo, pombredanne, kstewart, gregkh, bigeasy, jack,
darrick.wong, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api, hch, Adam Manzanares
From: Adam Manzanares <adam.manzanares@wdc.com>
This is the per-I/O equivalent of the ioprio_set system call.
See the following link for performance implications on a SATA HDD:
https://lkml.org/lkml/2016/12/6/495
First patch factors ioprio_check_cap function out of ioprio_set system call to
also be used by the aio ioprio interface.
Second patch converts kiocb ki_hint field to a u16 to avoid kiocb bloat.
Third patch passes ioprio hint from aio iocb to kiocb and inititalizes kiocb
ioprio value appropriately when it is not explicitly set.
Fourth patch enables the feature for blkdev.
Fifth patch enables the feature for iomap direct IO
Note: this work is based on top of linux-vfs/for-next
v2: merge patches
use IOCB_FLAG_IOPRIO
validate intended use with IOCB_IOPRIO
add linux-api and linux-block to cc
v3: add ioprio_check_cap function
convert kiocb ki_hint to u16
use ioprio_check_cap when adding ioprio to kiocb in aio.c
v4: handle IOCB_IOPRIO in aio_prep_rw
note patch 3 depends on patch 1 in commit msg
v5: rename ki_hint_valid -> ki_hint_validate
remove ki_hint_validate comment and whitespace
remove IOCB_IOPRIO flag
initialize kiocb to have no priority
Adam Manzanares (5):
block: add ioprio_check_cap function
fs: Convert kiocb rw_hint from enum to u16
fs: Add aio iopriority support
fs: blkdev set bio prio from kiocb prio
fs: iomap dio set bio prio from kiocb prio
block/ioprio.c | 22 ++++++++++++++++------
drivers/block/loop.c | 3 +++
fs/aio.c | 16 ++++++++++++++++
fs/block_dev.c | 1 +
fs/iomap.c | 1 +
include/linux/fs.h | 16 ++++++++++++++--
include/linux/ioprio.h | 2 ++
include/uapi/linux/aio_abi.h | 1 +
8 files changed, 54 insertions(+), 8 deletions(-)
--
2.15.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v2 0/4] Interface for higher order contiguous allocations
From: Vlastimil Babka @ 2018-05-21 12:00 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton
In-Reply-To: <20180503232935.22539-1-mike.kravetz@oracle.com>
On 05/04/2018 01:29 AM, Mike Kravetz wrote:
> Vlastimil and Michal brought up the issue of allocation alignment. The
> routine will currently align to 'nr_pages' (which is the requested size
> argument). It does this by examining and trying to allocate the first
> nr_pages aligned/nr_pages sized range. If this fails, it moves on to the
> next nr_pages aligned/nr_pages sized range until success or all potential
> ranges are exhausted.
As I've noted in my patch 3/4 review, in fact nr_pages is first rounded
up to an order, which makes this simpler, but suboptimal. I think we
could perhaps assume that nr_pages that's a power of two should be
aligned as such, and other values of nr_pages need no alignment? This
should fit existing users, and can be extended to explicit alignment
when such user appears?
> If we allow an alignment to be specified, we will
> need to potentially check all alignment aligned/nr_pages sized ranges.
> In the worst case where alignment = PAGE_SIZE, this could result in huge
> increase in the number of ranges to check.
> To help cut down on the number of ranges to check, we could identify the
> first page that causes a range allocation failure and start the next
> range at the next aligned boundary. I tried this, and we still end up
> with a huge number of ranges and wasted CPU cycles.
I think the wasted cycle issues is due to the current code structure,
which is based on the CMA use-case, which assumes that the allocations
will succeed, because the areas are reserved and may contain only
movable allocations
find_alloc_contig_pages()
__alloc_contig_pages_nodemask()
contig_pfn_range_valid()
- performs only very basic pfn validity and belongs-to-zone checks
alloc_contig_range()
start_isolate_page_range()
for (pfn per pageblock) - the main cycle
set_migratetype_isolate()
has_unmovable_pages() - cancel if yes
move_freepages_block() - expensive!
__alloc_contig_migrate_range()
etc (not important)
So I think the problem is that in the main cycle we might do a number of
expensive move_freepages_block() operations, then hit a block where
has_unmovable_pages() is true, cancel and do more expensive
undo_isolate_page_range() operations.
If we instead first scanned the range with has_unmovable_pages() and
only start doing the expensive work when we find a large enough (aligned
or not depending on caller) range, it should be much faster and there
should be no algorithmic difference between aligned and non-aligned case.
Thanks,
Vlastimil
^ permalink raw reply
* Re: [PATCH v2 3/4] mm: add find_alloc_contig_pages() interface
From: Vlastimil Babka @ 2018-05-21 8:54 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton
In-Reply-To: <20180503232935.22539-4-mike.kravetz@oracle.com>
On 05/04/2018 01:29 AM, Mike Kravetz wrote:
> find_alloc_contig_pages() is a new interface that attempts to locate
> and allocate a contiguous range of pages. It is provided as a more
How about dropping the 'find_' from the name, so it's more like other
allocator functions? All of them have to 'find' the free pages in some
sense.
> convenient interface than alloc_contig_range() which is currently
> used by CMA and gigantic huge pages.
>
> When attempting to allocate a range of pages, migration is employed
> if possible. There is no guarantee that the routine will succeed.
> So, the user must be prepared for failure and have a fall back plan.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> include/linux/gfp.h | 12 +++++
> mm/page_alloc.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 86a0d06463ab..b0d11777d487 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -573,6 +573,18 @@ static inline bool pm_suspended_storage(void)
> extern int alloc_contig_range(unsigned long start, unsigned long end,
> unsigned migratetype, gfp_t gfp_mask);
> extern void free_contig_range(unsigned long pfn, unsigned long nr_pages);
> +extern struct page *find_alloc_contig_pages(unsigned long nr_pages, gfp_t gfp,
> + int nid, nodemask_t *nodemask);
> +extern void free_contig_pages(struct page *page, unsigned long nr_pages);
> +#else
> +static inline struct page *find_alloc_contig_pages(unsigned long nr_pages,
> + gfp_t gfp, int nid, nodemask_t *nodemask)
> +{
> + return NULL;
> +}
> +static inline void free_contig_pages(struct page *page, unsigned long nr_pages)
> +{
> +}
> #endif
>
> #ifdef CONFIG_CMA
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index cb1a5e0be6ee..d0a2d0da9eae 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -67,6 +67,7 @@
> #include <linux/ftrace.h>
> #include <linux/lockdep.h>
> #include <linux/nmi.h>
> +#include <linux/mmzone.h>
>
> #include <asm/sections.h>
> #include <asm/tlbflush.h>
> @@ -7913,8 +7914,12 @@ int alloc_contig_range(unsigned long start, unsigned long end,
>
> /* Make sure the range is really isolated. */
> if (test_pages_isolated(outer_start, end, false)) {
> - pr_info_ratelimited("%s: [%lx, %lx) PFNs busy\n",
> - __func__, outer_start, end);
> +#ifdef MIGRATE_CMA
> + /* Only print messages for CMA allocations */
> + if (migratetype == MIGRATE_CMA)
I think is_migrate_cma() can be used to avoid the #ifdef.
> + pr_info_ratelimited("%s: [%lx, %lx) PFNs busy\n",
> + __func__, outer_start, end);
> +#endif
> ret = -EBUSY;
> goto done;
> }
> @@ -7950,6 +7955,133 @@ void free_contig_range(unsigned long pfn, unsigned long nr_pages)
> }
> WARN(count != 0, "%ld pages are still in use!\n", count);
> }
> +
> +/*
> + * Only check for obvious pfn/pages which can not be used/migrated. The
> + * migration code will do the final check. Under stress, this minimal set
> + * has been observed to provide the best results. The checks can be expanded
> + * if needed.
Hm I kind of doubt this is optimal, it doesn't test almost anything
besides basic validity, so it won't exclude ranges where the allocation
will fail. I will write more in a reply to the header where complexity
is discussed.
> + */
> +static bool contig_pfn_range_valid(struct zone *z, unsigned long start_pfn,
> + unsigned long nr_pages)
> +{
> + unsigned long i, end_pfn = start_pfn + nr_pages;
> + struct page *page;
> +
> + for (i = start_pfn; i < end_pfn; i++) {
> + if (!pfn_valid(i))
> + return false;
> +
> + page = pfn_to_online_page(i);
> +
> + if (page_zone(page) != z)
> + return false;
> +
> + }
> +
> + return true;
> +}
> +
> +/*
> + * Search for and attempt to allocate contiguous allocations greater than
> + * MAX_ORDER.
> + */
> +static struct page *__alloc_contig_pages_nodemask(gfp_t gfp,
> + unsigned long order,
> + int nid, nodemask_t *nodemask)
> +{
> + unsigned long nr_pages, pfn, flags;
> + struct page *ret_page = NULL;
> + struct zonelist *zonelist;
> + struct zoneref *z;
> + struct zone *zone;
> + int rc;
> +
> + nr_pages = 1 << order;
> + zonelist = node_zonelist(nid, gfp);
> + for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp),
> + nodemask) {
> + pgdat_resize_lock(zone->zone_pgdat, &flags);
> + pfn = ALIGN(zone->zone_start_pfn, nr_pages);
> + while (zone_spans_pfn(zone, pfn + nr_pages - 1)) {
> + if (contig_pfn_range_valid(zone, pfn, nr_pages)) {
> + struct page *page = pfn_to_online_page(pfn);
> + unsigned int migratetype;
> +
> + /*
> + * All pageblocks in range must be of same
> + * migrate type.
> + */
> + migratetype = get_pageblock_migratetype(page);
> + pgdat_resize_unlock(zone->zone_pgdat, &flags);
> +
> + rc = alloc_contig_range(pfn, pfn + nr_pages,
> + migratetype, gfp);
> + if (!rc) {
> + ret_page = pfn_to_page(pfn);
> + return ret_page;
> + }
> + pgdat_resize_lock(zone->zone_pgdat, &flags);
> + }
> + pfn += nr_pages;
> + }
> + pgdat_resize_unlock(zone->zone_pgdat, &flags);
> + }
> +
> + return ret_page;
> +}
> +
> +/**
> + * find_alloc_contig_pages() -- attempt to find and allocate a contiguous
> + * range of pages
> + * @nr_pages: number of pages to find/allocate
> + * @gfp: gfp mask used to limit search as well as during compaction
> + * @nid: target node
> + * @nodemask: mask of other possible nodes
> + *
> + * Pages can be freed with a call to free_contig_pages(), or by manually
> + * calling __free_page() for each page allocated.
> + *
> + * Return: pointer to 'order' pages on success, or NULL if not successful.
> + */
> +struct page *find_alloc_contig_pages(unsigned long nr_pages, gfp_t gfp,
> + int nid, nodemask_t *nodemask)
> +{
> + unsigned long i, alloc_order, order_pages;
> + struct page *pages;
> +
> + /*
> + * Underlying allocators perform page order sized allocations.
> + */
> + alloc_order = get_count_order(nr_pages);
So if takes arbitrary nr_pages but convert it to order anyway? I think
that's rather suboptimal and wasteful... e.g. a range could be skipped
because some of the pages added by rounding cannot be migrated away.
Vlastimil
> + if (alloc_order < MAX_ORDER) {
> + pages = __alloc_pages_nodemask(gfp, (unsigned int)alloc_order,
> + nid, nodemask);
> + split_page(pages, alloc_order);
> + } else {
> + pages = __alloc_contig_pages_nodemask(gfp, alloc_order, nid,
> + nodemask);
> + }
> +
> + if (pages) {
> + /*
> + * More pages than desired could have been allocated due to
> + * rounding up to next page order. Free any excess pages.
> + */
> + order_pages = 1UL << alloc_order;
> + for (i = nr_pages; i < order_pages; i++)
> + __free_page(pages + i);
> + }
> +
> + return pages;
> +}
> +EXPORT_SYMBOL_GPL(find_alloc_contig_pages);
> +
> +void free_contig_pages(struct page *page, unsigned long nr_pages)
> +{
> + free_contig_range(page_to_pfn(page), nr_pages);
> +}
> +EXPORT_SYMBOL_GPL(free_contig_pages);
> #endif
>
> #if defined CONFIG_MEMORY_HOTPLUG || defined CONFIG_CMA
>
^ permalink raw reply
* Re: [PATCH v5 04/28] fpga: mgr: add compat_id support
From: Wu Hao @ 2018-05-21 3:03 UTC (permalink / raw)
To: Alan Tull
Cc: Moritz Fischer, linux-fpga, linux-kernel, linux-api, Kang, Luwei,
Zhang, Yi Z, song.liu
In-Reply-To: <CANk1AXRsWsd++wjWH44CCWN+cmscy_WjLJhaGGgxmmNe-u1OLw@mail.gmail.com>
On Mon, May 07, 2018 at 04:09:06PM -0500, Alan Tull wrote:
> On Tue, May 1, 2018 at 9:50 PM, Wu Hao <hao.wu@intel.com> wrote:
>
> Hi Hao,
>
> Looks good!
>
> > This patch introduces compat_id support to fpga manager, it adds
> > a fpga_compat_id pointer to fpga manager data structure to allow
> > fpga manager drivers to save the compatibility id. This compat_id
> > could be used for compatibility checking before doing partial
> > reconfiguration to associated fpga regions.
> >
> > Signed-off-by: Wu Hao <hao.wu@intel.com>
> Acked-by: Alan Tull <atull@kernel.org>
Hi Alan
Thanks a lot for the acked-by.
Did you get a chance to look into other patches?
Thanks
Hao
^ permalink raw reply
* Re: [PATCH 10/32] aio: implement IOCB_CMD_POLL
From: Christoph Hellwig @ 2018-05-20 17:32 UTC (permalink / raw)
To: Al Viro
Cc: Christoph Hellwig, Avi Kivity, linux-aio, linux-fsdevel, netdev,
linux-api, linux-kernel
In-Reply-To: <20180520073332.GA30522@ZenIV.linux.org.uk>
On Sun, May 20, 2018 at 08:33:39AM +0100, Al Viro wrote:
> > ... get buggered on attempt to dereference a pointer fetched from freed and
> > reused object.
>
> FWIW, how painful would it be to pull the following trick:
> * insert into wait queue under ->ctx_lock
> * have wakeup do schedule_work() with aio_complete() done from that
> * have ->ki_cancel() grab queue lock, remove from queue and use
> the same schedule_work()
>
> That way you'd get ->ki_cancel() with the same semantics as originally for
> everything - "ask politely to finish ASAP", and called in the same locking
> environment for everyone - under ->ctx_lock, that is. queue lock nests
> inside ->ctx_lock; no magical flags, etc.
>
> The cost is schedule_work() for each async poll-related completion as you
> have for fsync. I don't know whether that's too costly or not; it certainly
> simplifies the things, but whether it's OK performance-wise...
I think it is doable:
http://git.infradead.org/users/hch/vfs.git/commitdiff/c441130e405465268ea10c9ddd5639c155f779e8
downside is that sizeof(struct aio_kiocb) grows a bit.
For the completion performance we can use a spin_trylock to still avoid
the context switch for the common case:
http://git.infradead.org/users/hch/vfs.git/commitdiff/6cc1827afbea87c52fe425cf533bfcf5f3308163
^ permalink raw reply
* Re: [PATCH 07/14] powerpc: Add support for restartable sequences
From: Boqun Feng @ 2018-05-20 14:08 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Will Deacon, Peter Zijlstra, Paul E. McKenney, Andy Lutomirski,
Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas <cata>
In-Reply-To: <418003803.516.1526667437396.JavaMail.zimbra@efficios.com>
On Fri, May 18, 2018 at 02:17:17PM -0400, Mathieu Desnoyers wrote:
> ----- On May 17, 2018, at 7:50 PM, Boqun Feng boqun.feng@gmail.com wrote:
> [...]
> >> > I think you're right. So we have to introduce callsite to rseq_syscall()
> >> > in syscall path, something like:
> >> >
> >> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> >> > index 51695608c68b..a25734a96640 100644
> >> > --- a/arch/powerpc/kernel/entry_64.S
> >> > +++ b/arch/powerpc/kernel/entry_64.S
> >> > @@ -222,6 +222,9 @@ system_call_exit:
> >> > mtmsrd r11,1
> >> > #endif /* CONFIG_PPC_BOOK3E */
> >> >
> >> > + addi r3,r1,STACK_FRAME_OVERHEAD
> >> > + bl rseq_syscall
> >> > +
> >> > ld r9,TI_FLAGS(r12)
> >> > li r11,-MAX_ERRNO
> >> > andi.
> >> > r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
> >> >
>
> By the way, I think this is not the right spot to call rseq_syscall, because
> interrupts are disabled. I think we should move this hunk right after system_call_exit.
>
Good point.
> Would you like to implement and test an updated patch adding those calls for ppc 32 and 64 ?
>
I'd like to help, but I don't have a handy ppc environment for test...
So I made the below patch which has only been build-tested, hope it
could be somewhat helpful.
Regards,
Boqun
--------------------------------->8
Subject: [PATCH] powerpc: Add syscall detection for restartable sequences
Syscalls are not allowed inside restartable sequences, so add a call to
rseq_syscall() at the very beginning of system call exiting path for
CONFIG_DEBUG_RSEQ=y kernel. This could help us to detect whether there
is a syscall issued inside restartable sequences.
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
arch/powerpc/kernel/entry_32.S | 5 +++++
arch/powerpc/kernel/entry_64.S | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index eb8d01bae8c6..2f134eebe7ed 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -365,6 +365,11 @@ syscall_dotrace_cont:
blrl /* Call handler */
.globl ret_from_syscall
ret_from_syscall:
+#ifdef CONFIG_DEBUG_RSEQ
+ /* Check whether the syscall is issued inside a restartable sequence */
+ addi r3,r1,STACK_FRAME_OVERHEAD
+ bl rseq_syscall
+#endif
mr r6,r3
CURRENT_THREAD_INFO(r12, r1)
/* disable interrupts so current_thread_info()->flags can't change */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 2cb5109a7ea3..2e2d59bb45d0 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -204,6 +204,11 @@ system_call: /* label this so stack traces look sane */
* This is blacklisted from kprobes further below with _ASM_NOKPROBE_SYMBOL().
*/
system_call_exit:
+#ifdef CONFIG_DEBUG_RSEQ
+ /* Check whether the syscall is issued inside a restartable sequence */
+ addi r3,r1,STACK_FRAME_OVERHEAD
+ bl rseq_syscall
+#endif
/*
* Disable interrupts so current_thread_info()->flags can't change,
* and so that we don't get interrupted after loading SRR0/1.
--
2.16.2
^ permalink raw reply related
* Re: [PATCH v5 3/7] proc: add helpers to set and get proc hidepid and gid mount options
From: kbuild test robot @ 2018-05-20 7:53 UTC (permalink / raw)
To: Alexey Gladkov
Cc: kbuild-all, Kees Cook, Andy Lutomirski, Andrew Morton,
linux-fsdevel, linux-kernel, kernel-hardening,
linux-security-module, linux-api, Greg Kroah-Hartman,
Alexander Viro, Akinobu Mita, Oleg Nesterov, Jeff Layton,
Ingo Molnar, Alexey Dobriyan, Eric W. Biederman, Linus Torvalds,
aniel Micay, Jonathan Corbet, bfields, Steph
In-Reply-To: <20180511093528.GA1250@comp-core-i7-2640m-0182e6>
[-- Attachment #1: Type: text/plain, Size: 5866 bytes --]
Hi Djalal,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc5]
[cannot apply to next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Alexey-Gladkov/proc-add-proc_fs_info-struct-to-store-proc-information/20180512-002936
config: xtensa-allnoconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
init/do_mounts.o: In function `proc_fs_pid_gid':
>> do_mounts.c:(.text+0x3c): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
arch/xtensa/kernel/setup.o: In function `proc_fs_pid_gid':
setup.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/fork.o: In function `proc_fs_pid_gid':
fork.c:(.text+0x2a8): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/exec_domain.o: In function `proc_fs_pid_gid':
exec_domain.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/cpu.o: In function `proc_fs_pid_gid':
cpu.c:(.text+0xf0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/exit.o: In function `proc_fs_pid_gid':
exit.c:(.text+0x188): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/resource.o: In function `proc_fs_pid_gid':
resource.c:(.text+0x344): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sysctl.o: In function `proc_fs_pid_gid':
sysctl.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/pid.o: In function `proc_fs_pid_gid':
pid.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/core.o: In function `proc_fs_pid_gid':
core.c:(.text+0x6c8): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/loadavg.o: In function `proc_fs_pid_gid':
loadavg.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/clock.o: In function `proc_fs_pid_gid':
clock.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/cputime.o: In function `proc_fs_pid_gid':
cputime.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/idle.o: In function `proc_fs_pid_gid':
idle.c:(.text+0x98): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/fair.o: In function `proc_fs_pid_gid':
fair.c:(.text+0xac0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/rt.o: In function `proc_fs_pid_gid':
rt.c:(.text+0x734): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/deadline.o: In function `proc_fs_pid_gid':
deadline.c:(.text+0x10b0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/wait.o: In function `proc_fs_pid_gid':
wait.c:(.text+0x120): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/wait_bit.o: In function `proc_fs_pid_gid':
wait_bit.c:(.text+0x78): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/swait.o: In function `proc_fs_pid_gid':
swait.c:(.text+0x24): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/sched/completion.o: In function `proc_fs_pid_gid':
completion.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
kernel/time/timer_list.o: In function `proc_fs_pid_gid':
timer_list.c:(.text+0x1c): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
mm/vmstat.o: In function `proc_fs_pid_gid':
vmstat.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
mm/slab_common.o: In function `proc_fs_pid_gid':
slab_common.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
mm/vmalloc.o: In function `proc_fs_pid_gid':
vmalloc.c:(.text+0x514): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
mm/slub.o: In function `proc_fs_pid_gid':
slub.c:(.text+0x8a4): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
fs/filesystems.o: In function `proc_fs_pid_gid':
filesystems.c:(.text+0x44): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
drivers/char/misc.o: In function `proc_fs_pid_gid':
misc.c:(.text+0xe0): multiple definition of `proc_fs_pid_gid'
init/main.o:main.c:(.text+0x30): first defined here
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 4180 bytes --]
^ permalink raw reply
* Re: [PATCH 10/32] aio: implement IOCB_CMD_POLL
From: Al Viro @ 2018-05-20 7:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180520053219.GZ30522@ZenIV.linux.org.uk>
On Sun, May 20, 2018 at 06:32:25AM +0100, Al Viro wrote:
> > + spin_lock_irqsave(&ctx->ctx_lock, flags);
> > + list_add_tail(&aiocb->ki_list, &ctx->delayed_cancel_reqs);
> > + spin_unlock(&ctx->ctx_lock);
>
> ... and io_cancel(2) comes, finds it and inhume^Wcompletes it, leaving us to...
>
> > + spin_lock(&req->head->lock);
>
> ... get buggered on attempt to dereference a pointer fetched from freed and
> reused object.
FWIW, how painful would it be to pull the following trick:
* insert into wait queue under ->ctx_lock
* have wakeup do schedule_work() with aio_complete() done from that
* have ->ki_cancel() grab queue lock, remove from queue and use
the same schedule_work()
That way you'd get ->ki_cancel() with the same semantics as originally for
everything - "ask politely to finish ASAP", and called in the same locking
environment for everyone - under ->ctx_lock, that is. queue lock nests
inside ->ctx_lock; no magical flags, etc.
The cost is schedule_work() for each async poll-related completion as you
have for fsync. I don't know whether that's too costly or not; it certainly
simplifies the things, but whether it's OK performance-wise...
Comments?
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 10/32] aio: implement IOCB_CMD_POLL
From: Al Viro @ 2018-05-20 5:32 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180515194833.6906-11-hch@lst.de>
On Tue, May 15, 2018 at 09:48:11PM +0200, Christoph Hellwig wrote:
> +static ssize_t aio_poll(struct aio_kiocb *aiocb, struct iocb *iocb)
> +{
> + struct kioctx *ctx = aiocb->ki_ctx;
> + struct poll_iocb *req = &aiocb->poll;
> + unsigned long flags;
> + __poll_t mask;
> +
> + /* reject any unknown events outside the normal event mask. */
> + if ((u16)iocb->aio_buf != iocb->aio_buf)
> + return -EINVAL;
> + /* reject fields that are not defined for poll */
> + if (iocb->aio_offset || iocb->aio_nbytes || iocb->aio_rw_flags)
> + return -EINVAL;
> +
> + req->events = demangle_poll(iocb->aio_buf) | POLLERR | POLLHUP;
> + req->file = fget(iocb->aio_fildes);
> + if (unlikely(!req->file))
> + return -EBADF;
> + if (!file_has_poll_mask(req->file))
> + goto out_fail;
> +
> + req->head = req->file->f_op->get_poll_head(req->file, req->events);
> + if (!req->head)
> + goto out_fail;
> + if (IS_ERR(req->head)) {
> + mask = EPOLLERR;
> + goto done;
> + }
> +
> + init_waitqueue_func_entry(&req->wait, aio_poll_wake);
> + aiocb->ki_cancel = aio_poll_cancel;
> +
> + spin_lock_irqsave(&ctx->ctx_lock, flags);
> + list_add_tail(&aiocb->ki_list, &ctx->delayed_cancel_reqs);
> + spin_unlock(&ctx->ctx_lock);
... and io_cancel(2) comes, finds it and inhume^Wcompletes it, leaving us to...
> + spin_lock(&req->head->lock);
... get buggered on attempt to dereference a pointer fetched from freed and
reused object.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation
From: Al Viro @ 2018-05-20 5:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180515194833.6906-9-hch@lst.de>
On Tue, May 15, 2018 at 09:48:09PM +0200, Christoph Hellwig wrote:
> case -EIOCBQUEUED:
> + if (req->ki_filp->f_op->cancel_kiocb) {
> + struct aio_kiocb *iocb =
> + container_of(req, struct aio_kiocb, rw);
> + struct kioctx *ctx = iocb->ki_ctx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ctx->ctx_lock, flags);
> + list_add_tail(&iocb->ki_list, &ctx->active_reqs);
Use after free - that list insertion used to be done by drivers and doing
so before any ->ki_complete() calls might've happened used to be their
responsibility. Now you've taken that to the point after ->read_iter()
(or ->write_iter()) return, so there's no way in hell to guarantee it's
not been completed (and freed) by that point.
Incidentally, none of the callers gives a damn about the difference between
0 and -EIOCBQUEUED now, so aio_rw_ret() might as well had been made void...
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 3/6] workqueue: Make worker_attach/detach_pool() update worker->pool
From: Lai Jiangshan @ 2018-05-19 6:44 UTC (permalink / raw)
To: Tejun Heo
Cc: Linus Torvalds, Andrew Morton, LKML, Linux API, kernel-team,
Craig Small
In-Reply-To: <20180517043448.3152269-4-tj@kernel.org>
On Thu, May 17, 2018 at 12:34 PM, Tejun Heo <tj@kernel.org> wrote:
> For historical reasons, the worker attach/detach functions don't
> currently manage worker->pool and the callers are manually and
> inconsistently updating it.
>
> This patch moves worker->pool updates into the worker attach/detach
> functions. This makes worker->pool consistent and clearly defines how
> worker->pool updates are synchronized.
>
> This will help later workqueue visibility improvements by allowing
> safe access to workqueue information from worker->task.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
> kernel/workqueue.c | 16 ++++++++--------
> kernel/workqueue_internal.h | 2 +-
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 91fe0a6..2fde50f 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1741,6 +1741,7 @@ static void worker_attach_to_pool(struct worker *worker,
> worker->flags |= WORKER_UNBOUND;
>
> list_add_tail(&worker->node, &pool->workers);
> + worker->pool = pool;
>
> mutex_unlock(&wq_pool_attach_mutex);
> }
> @@ -1748,19 +1749,21 @@ static void worker_attach_to_pool(struct worker *worker,
> /**
> * worker_detach_from_pool() - detach a worker from its pool
> * @worker: worker which is attached to its pool
> - * @pool: the pool @worker is attached to
> *
> * Undo the attaching which had been done in worker_attach_to_pool(). The
> * caller worker shouldn't access to the pool after detached except it has
> * other reference to the pool.
> */
> -static void worker_detach_from_pool(struct worker *worker,
> - struct worker_pool *pool)
> +static void worker_detach_from_pool(struct worker *worker)
> {
> + struct worker_pool *pool = worker->pool;
> struct completion *detach_completion = NULL;
>
> mutex_lock(&wq_pool_attach_mutex);
> +
> list_del(&worker->node);
> + worker->pool = NULL;
> +
> if (list_empty(&pool->workers))
> detach_completion = pool->detach_completion;
> mutex_unlock(&wq_pool_attach_mutex);
> @@ -1799,7 +1802,6 @@ static struct worker *create_worker(struct worker_pool *pool)
> if (!worker)
> goto fail;
>
> - worker->pool = pool;
> worker->id = id;
>
> if (pool->cpu >= 0)
> @@ -2236,7 +2238,7 @@ static int worker_thread(void *__worker)
>
> set_task_comm(worker->task, "kworker/dying");
> ida_simple_remove(&pool->worker_ida, worker->id);
> - worker_detach_from_pool(worker, pool);
> + worker_detach_from_pool(worker);
> kfree(worker);
> return 0;
> }
> @@ -2367,7 +2369,6 @@ static int rescuer_thread(void *__rescuer)
> worker_attach_to_pool(rescuer, pool);
>
> spin_lock_irq(&pool->lock);
> - rescuer->pool = pool;
>
> /*
> * Slurp in all works issued via this workqueue and
> @@ -2417,10 +2418,9 @@ static int rescuer_thread(void *__rescuer)
> if (need_more_worker(pool))
> wake_up_worker(pool);
>
> - rescuer->pool = NULL;
> spin_unlock_irq(&pool->lock);
>
> - worker_detach_from_pool(rescuer, pool);
> + worker_detach_from_pool(rescuer);
>
> spin_lock_irq(&wq_mayday_lock);
> }
> diff --git a/kernel/workqueue_internal.h b/kernel/workqueue_internal.h
> index d390d1b..4a182e0 100644
> --- a/kernel/workqueue_internal.h
> +++ b/kernel/workqueue_internal.h
> @@ -37,7 +37,7 @@ struct worker {
> /* 64 bytes boundary on 64bit, 32 on 32bit */
>
> struct task_struct *task; /* I: worker task */
> - struct worker_pool *pool; /* I: the associated pool */
> + struct worker_pool *pool; /* A: the associated pool */
> /* L: for rescuers */
I guess ` /* L: for rescuers */ ` needed to be removed.
And there are many usages of worker->pool without attached_mutex
heled. The only locking annotation "A:" is not enough. Additional
comment is needed too.
1) called from the worker(including rescuer) task: it is safe because only
the worker task itself can call the attach()/detach().
2) called from the destroy_worker() (only normal worker): it is safe
because it is immutable before the worker detaches itself.
> struct list_head node; /* A: anchored at pool->workers */
> /* A: runs through worker->node */
> --
> 2.9.5
>
^ permalink raw reply
* Re: [PATCH v2 1/4] mm: change type of free_contig_range(nr_pages) to unsigned long
From: Mike Kravetz @ 2018-05-18 22:01 UTC (permalink / raw)
To: Vlastimil Babka, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton, Joonsoo Kim,
Marek Szyprowski
In-Reply-To: <87d74ce0-b135-064d-a589-64235e44c388@suse.cz>
On 05/18/2018 02:12 AM, Vlastimil Babka wrote:
> On 05/04/2018 01:29 AM, Mike Kravetz wrote:
>> free_contig_range() is currently defined as:
>> void free_contig_range(unsigned long pfn, unsigned nr_pages);
>> change to,
>> void free_contig_range(unsigned long pfn, unsigned long nr_pages);
>>
>> Some callers are passing a truncated unsigned long today. It is
>> highly unlikely that these values will overflow an unsigned int.
>> However, this should be changed to an unsigned long to be consistent
>> with other page counts.
>>
>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>> ---
>> include/linux/gfp.h | 2 +-
>> mm/cma.c | 2 +-
>> mm/hugetlb.c | 2 +-
>> mm/page_alloc.c | 6 +++---
>> 4 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>> index 1a4582b44d32..86a0d06463ab 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -572,7 +572,7 @@ static inline bool pm_suspended_storage(void)
>> /* The below functions must be run on a range from a single zone. */
>> extern int alloc_contig_range(unsigned long start, unsigned long end,
>> unsigned migratetype, gfp_t gfp_mask);
>> -extern void free_contig_range(unsigned long pfn, unsigned nr_pages);
>> +extern void free_contig_range(unsigned long pfn, unsigned long nr_pages);
>> #endif
>>
>> #ifdef CONFIG_CMA
>> diff --git a/mm/cma.c b/mm/cma.c
>> index aa40e6c7b042..f473fc2b7cbd 100644
>> --- a/mm/cma.c
>> +++ b/mm/cma.c
>> @@ -563,7 +563,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned int count)
>>
>> VM_BUG_ON(pfn + count > cma->base_pfn + cma->count);
>>
>> - free_contig_range(pfn, count);
>> + free_contig_range(pfn, (unsigned long)count);
>
> I guess this cast from uint to ulong doesn't need to be explicit? But
> instead, cma_release() signature could be also changed to ulong, because
> some of its callers do pass those?
Correct, that cast is not needed.
I like the idea of changing cma_release() to take ulong. Until you mentioned
this, I did not realize that some callers were passing in a truncated ulong.
As noted in my commit message, this truncation is unlikely to be an issue.
But, I think we should 'fix' them if we can.
I'll spin another version with this change.
>
>> cma_clear_bitmap(cma, pfn, count);
>> trace_cma_release(pfn, pages, count);
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 218679138255..c81072ce7510 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1055,7 +1055,7 @@ static void destroy_compound_gigantic_page(struct page *page,
>>
>> static void free_gigantic_page(struct page *page, unsigned int order)
>> {
>> - free_contig_range(page_to_pfn(page), 1 << order);
>> + free_contig_range(page_to_pfn(page), 1UL << order);
>> }
>>
>> static int __alloc_gigantic_page(unsigned long start_pfn,
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 905db9d7962f..0fd5e8e2456e 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -7937,9 +7937,9 @@ int alloc_contig_range(unsigned long start, unsigned long end,
>> return ret;
>> }
>>
>> -void free_contig_range(unsigned long pfn, unsigned nr_pages)
>> +void free_contig_range(unsigned long pfn, unsigned long nr_pages)
>> {
>> - unsigned int count = 0;
>> + unsigned long count = 0;
>>
>> for (; nr_pages--; pfn++) {
>> struct page *page = pfn_to_page(pfn);
>> @@ -7947,7 +7947,7 @@ void free_contig_range(unsigned long pfn, unsigned nr_pages)
>> count += page_count(page) != 1;
>> __free_page(page);
>> }
>> - WARN(count != 0, "%d pages are still in use!\n", count);
>> + WARN(count != 0, "%ld pages are still in use!\n", count);
>
> Maybe change to %lu while at it?
Yes
> BTW, this warning can theoretically produce false positives, because
> page users have to deal with page_count() being incremented by e.g.
> parallel pfn scanners using get_page_unless_zero(). We also don't detect
> refcount leaks in general. Should we remove it or change it to VM_WARN
> if it's still useful for debugging?
Added Marek on Cc: as he is the one who originally added this message (although
it has been a long time). I do not know what specific issue he was concerned
with. A search found a few bug reports related to this warning. In these
cases, it clearly was not a false positive but some other issue. It 'appears'
the message helped in those cases.
However, I would hate for a support organization to spend a bunch of time
doing investigation for a false positive. At the very least, I think we
should add a message/comment about the possibility of false positives in the
code. I would be inclined to change it to VM_WARN, but it would be good
to get input from others who might find it useful as it.
--
Mike Kravetz
>
>> }
>> #endif
>>
>>
>
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: Add aio iopriority support for block_dev
From: kbuild test robot @ 2018-05-18 20:08 UTC (permalink / raw)
Cc: kbuild-all, viro, linux-fsdevel, axboe, bcrl, mingo, peterz,
pombredanne, gregkh, bigeasy, rgoldwyn, linux-block, linux-kernel,
linux-aio, linux-api, Adam Manzanares
In-Reply-To: <20180517203803.2664-4-adam.manzanares@wdc.com>
[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]
Hi Adam,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20180516]
[cannot apply to linus/master block/for-next v4.17-rc5 v4.17-rc4 v4.17-rc3 v4.17-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/adam-manzanares-wdc-com/AIO-add-per-command-iopriority/20180519-031848
config: x86_64-randconfig-x013-201819 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
fs/aio.c: In function 'aio_prep_rw':
>> fs/aio.c:1460:9: error: implicit declaration of function 'ioprio_check_cap'; did you mean 'param_check_charp'? [-Werror=implicit-function-declaration]
ret = ioprio_check_cap(iocb->aio_reqprio);
^~~~~~~~~~~~~~~~
param_check_charp
cc1: some warnings being treated as errors
vim +1460 fs/aio.c
1440
1441 static int aio_prep_rw(struct kiocb *req, struct iocb *iocb)
1442 {
1443 int ret;
1444
1445 req->ki_filp = fget(iocb->aio_fildes);
1446 if (unlikely(!req->ki_filp))
1447 return -EBADF;
1448 req->ki_complete = aio_complete_rw;
1449 req->ki_pos = iocb->aio_offset;
1450 req->ki_flags = iocb_flags(req->ki_filp);
1451 if (iocb->aio_flags & IOCB_FLAG_RESFD)
1452 req->ki_flags |= IOCB_EVENTFD;
1453 req->ki_hint = file_write_hint(req->ki_filp);
1454 if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
1455 /*
1456 * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
1457 * aio_reqprio is interpreted as an I/O scheduling
1458 * class and priority.
1459 */
> 1460 ret = ioprio_check_cap(iocb->aio_reqprio);
1461 if (ret) {
1462 pr_debug("aio ioprio check cap error\n");
1463 return -EINVAL;
1464 }
1465
1466 req->ki_ioprio = iocb->aio_reqprio;
1467 req->ki_flags |= IOCB_IOPRIO;
1468 }
1469
1470 ret = kiocb_set_rw_flags(req, iocb->aio_rw_flags);
1471 if (unlikely(ret))
1472 fput(req->ki_filp);
1473 return ret;
1474 }
1475
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24001 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox