Re: [PATCH v2 1/8] kcsan: Add Kernel Concurrency Sanitizer infrastructure

2019-10-23 Thread Oleg Nesterov
On 10/22, Marco Elver wrote: > > On Tue, 22 Oct 2019 at 17:49, Oleg Nesterov wrote: > > > > Just for example. Suppose that task->state = TASK_UNINTERRUPTIBLE, this task > > does __set_current_state(TASK_RUNNING), another CPU does > > wake_up_process(task) > &

Re: [PATCH v2 1/8] kcsan: Add Kernel Concurrency Sanitizer infrastructure

2019-10-22 Thread Oleg Nesterov
On 10/17, Marco Elver wrote: > > + /* > + * Delay this thread, to increase probability of observing a racy > + * conflicting access. > + */ > + udelay(get_delay()); > + > + /* > + * Re-read value, and check if it is as expected; if not, we infer a > + * racy acc

Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section

2019-07-15 Thread Oleg Nesterov
d_held(), > + RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(), Yes, this is what I meant. Sorry for confusion, I should have mentioned that rcu_sync_is_idle() was recently updated when I suggested to use the new helper. Acked-by: Oleg Nesterov

Re: [PATCH v5 4/7] cgroup: cgroup v2 freezer

2018-12-20 Thread Oleg Nesterov
On 12/18, Roman Gushchin wrote: > > > > > > --- a/kernel/freezer.c > > > > > +++ b/kernel/freezer.c > > > > > @@ -134,7 +134,7 @@ bool freeze_task(struct task_struct *p) > > > > > return false; > > > > > > > > > > spin_lock_irqsave(&freezer_lock, flags); > > > > > - if

Re: [PATCH v5 4/7] cgroup: cgroup v2 freezer

2018-12-18 Thread Oleg Nesterov
On 12/18, Roman Gushchin wrote: > > On Wed, Dec 12, 2018 at 06:49:02PM +0100, Oleg Nesterov wrote: > > > > and btw what about suspend? try_to_freeze_tasks() will obviously > > > > fail > > > > if there is a ->frozen thread? > > > > >

Re: [PATCH v5 4/7] cgroup: cgroup v2 freezer

2018-12-12 Thread Oleg Nesterov
On 12/11, Roman Gushchin wrote: > > On Tue, Dec 11, 2018 at 05:26:32PM +0100, Oleg Nesterov wrote: > > On 12/07, Roman Gushchin wrote: > > > > > > Cgroup v2 freezer tries to put tasks into a state similar to jobctl > > > stop. This means that tasks can be ki

Re: [PATCH v5 4/7] cgroup: cgroup v2 freezer

2018-12-11 Thread Oleg Nesterov
On 12/07, Roman Gushchin wrote: > > Cgroup v2 freezer tries to put tasks into a state similar to jobctl > stop. This means that tasks can be killed, ptraced (using > PTRACE_SEIZE*), and interrupted. It is possible to attach to > a frozen task, get some information (e.g. read registers) and detach.

Re: [PATCH v4 4/7] cgroup: cgroup v2 freezer

2018-12-03 Thread Oleg Nesterov
On 11/30, Roman Gushchin wrote: > > void recalc_sigpending(void) > { > if (!recalc_sigpending_tsk(current) && !freezing(current) && > - !klp_patch_pending(current)) > + !klp_patch_pending(current) && !cgroup_task_frozen(current)) I must have missed something... but shouldn'

Re: [PATCH v4 4/7] cgroup: cgroup v2 freezer

2018-12-03 Thread Oleg Nesterov
On 11/30, Roman Gushchin wrote: > > +void cgroup_enter_frozen(void) > +{ > + if (!current->frozen) { > + struct cgroup *cgrp; > + > + spin_lock_irq(&css_set_lock); > + current->frozen = true; > + cgrp = task_dfl_cgroup(current); > + cg

Re: [PATCH v4 4/7] cgroup: cgroup v2 freezer

2018-12-03 Thread Oleg Nesterov
To be honest, I fail to understand this patch. At least after a quick glance, I will try to read it again tomorrow but so far I do not even understand the desired semantics wrt signals/ptrace. On 11/30, Roman Gushchin wrote: > > @@ -368,6 +369,8 @@ static inline int signal_pending_state(long state

Re: [PATCH v3 6/9] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-05-24 Thread Oleg Nesterov
Hi Ravi, sorry for delay! I am trying to recall what this code should do ;) At first glance, I do not see any serious problem in this version... except it doesn't apply to Linus's tree. just one question for now. On 04/17, Ravi Bangoria wrote: > > @@ -941,6 +1091,9 @@ typedef bool (*filter_func_

Re: [PATCH v3 0/9] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-05-04 Thread Oleg Nesterov
Sorry Ravi, I saved the new version for review and forgot about it... I'll try to do this on weekend. On 05/03, Ravi Bangoria wrote: > > On 04/17/2018 10:02 AM, Ravi Bangoria wrote: > > Userspace Statically Defined Tracepoints[1] are dtrace style markers > > inside userspace applications. Applica

Re: [PATCH v2 7/9] trace_uprobe/sdt: Fix multiple update of same reference counter

2018-04-10 Thread Oleg Nesterov
Hi Ravi, On 04/10, Ravi Bangoria wrote: > > > and what if __mmu_notifier_register() fails simply because signal_pending() > > == T? > > see mm_take_all_locks(). > > > > at first glance this all look suspicious and sub-optimal, > > Yes. I should have added checks for failure cases. > Will fix them

Re: [PATCH v2 7/9] trace_uprobe/sdt: Fix multiple update of same reference counter

2018-04-09 Thread Oleg Nesterov
On 04/04, Ravi Bangoria wrote: > > +static void sdt_add_mm_list(struct trace_uprobe *tu, struct mm_struct *mm) > +{ > + struct mmu_notifier *mn; > + struct sdt_mm_list *sml = kzalloc(sizeof(*sml), GFP_KERNEL); > + > + if (!sml) > + return; > + sml->mm = mm; > + list_

Re: [PATCH v2 7/9] trace_uprobe/sdt: Fix multiple update of same reference counter

2018-04-09 Thread Oleg Nesterov
On 04/04, Ravi Bangoria wrote: > > +static void sdt_add_mm_list(struct trace_uprobe *tu, struct mm_struct *mm) > +{ > + struct mmu_notifier *mn; > + struct sdt_mm_list *sml = kzalloc(sizeof(*sml), GFP_KERNEL); > + > + if (!sml) > + return; > + sml->mm = mm; > + list_

Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-03-19 Thread Oleg Nesterov
On 03/19, Ravi Bangoria wrote: > > Hi Oleg, > > On 03/14/2018 10:29 PM, Oleg Nesterov wrote: > > On 03/13, Ravi Bangoria wrote: > >> +static bool sdt_valid_vma(struct trace_uprobe *tu, struct vm_area_struct > >> *vma) > >> +{ > >>

Re: [PATCH 6/8] trace_uprobe/sdt: Fix multiple update of same reference counter

2018-03-19 Thread Oleg Nesterov
Hi Ravi, On 03/19, Ravi Bangoria wrote: > > On 03/16/2018 11:20 PM, Oleg Nesterov wrote: > > > > And it seems that you are trying to confuse yourself, not only me ;) Just > > suppose that an application does mmap+munmap in a loop and the mapped region > > conta

Re: [PATCH 6/8] trace_uprobe/sdt: Fix multiple update of same reference counter

2018-03-16 Thread Oleg Nesterov
On 03/16, Ravi Bangoria wrote: > > On 03/15/2018 08:19 PM, Oleg Nesterov wrote: > > On 03/13, Ravi Bangoria wrote: > >> For tiny binaries/libraries, different mmap regions points to the > >> same file portion. In such cases, we may increment reference counte

Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-03-16 Thread Oleg Nesterov
On 03/15, Steven Rostedt wrote: > > On Tue, 13 Mar 2018 18:26:00 +0530 > Ravi Bangoria wrote: > > > +static void sdt_increment_ref_ctr(struct trace_uprobe *tu) > > +{ > > + struct uprobe_map_info *info; > > + struct vm_area_struct *vma; > > + unsigned long vaddr; > > + > > + uprobe_start_d

Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-03-16 Thread Oleg Nesterov
On 03/16, Ravi Bangoria wrote: > > On 03/15/2018 08:00 PM, Oleg Nesterov wrote: > > Note to mention that sdt_find_vma() can return NULL but the callers do > > vma_offset_to_vaddr(vma) without any check. > > If the "mm" we are passing to sdt_find_vma() is r

Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-03-15 Thread Oleg Nesterov
On 03/13, Ravi Bangoria wrote: > > +sdt_update_ref_ctr(struct mm_struct *mm, unsigned long vaddr, short d) > +{ > + void *kaddr; > + struct page *page; > + struct vm_area_struct *vma; > + int ret = 0; > + unsigned short orig = 0; > + > + if (vaddr == 0) > + retur

Re: [PATCH 6/8] trace_uprobe/sdt: Fix multiple update of same reference counter

2018-03-15 Thread Oleg Nesterov
On 03/13, Ravi Bangoria wrote: > > For tiny binaries/libraries, different mmap regions points to the > same file portion. In such cases, we may increment reference counter > multiple times. Yes, > But while de-registration, reference counter will get > decremented only by once could you explain

Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-03-15 Thread Oleg Nesterov
On 03/15, Oleg Nesterov wrote: > > > +static struct vm_area_struct * > > +sdt_find_vma(struct mm_struct *mm, struct trace_uprobe *tu) > > +{ > > + struct vm_area_struct *tmp; > > + > > + for (tmp = mm->mmap; tmp != NULL; tmp = tmp->vm_ne

Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-03-15 Thread Oleg Nesterov
On 03/13, Ravi Bangoria wrote: > > @@ -1053,6 +1056,9 @@ int uprobe_mmap(struct vm_area_struct *vma) > struct uprobe *uprobe, *u; > struct inode *inode; > > + if (uprobe_mmap_callback) > + uprobe_mmap_callback(vma); > + > if (no_uprobe_events() || !valid_vma(vma, t

Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)

2018-03-14 Thread Oleg Nesterov
On 03/13, Ravi Bangoria wrote: > > +static bool sdt_valid_vma(struct trace_uprobe *tu, struct vm_area_struct > *vma) > +{ > + unsigned long vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset); > + > + return tu->ref_ctr_offset && > + vma->vm_file && > + file_inode(

Re: [PATCH v5 1/3] mm, proc: Implement /proc//totmaps

2016-09-07 Thread Oleg Nesterov
On 09/05, robert.f...@collabora.com wrote: > > @@ -2854,6 +2854,7 @@ static const struct pid_entry tgid_base_stuff[] = { > REG("clear_refs", S_IWUSR, proc_clear_refs_operations), > REG("smaps", S_IRUGO, proc_pid_smaps_operations), > REG("pagemap",S_IRUSR, proc_pagemap_ope