Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated version of SPI driver that now runs with real time priority and ... #147

Closed
wants to merge 1 commit into from

Conversation

msperl
Copy link
Contributor

@msperl msperl commented Oct 28, 2012

...makes use of the onemessage interface that is available with 3.6.y

See issue 141.

@nneves
Copy link

nneves commented Nov 13, 2012

Just for reference, this pull request is based on this topic discussion, right?

http://raspberrypi.org/phpBB3/viewtopic.php?f=44&t=19489

Really looking forward to see this integrated!

@msperl
Copy link
Contributor Author

msperl commented Nov 13, 2012

yes - correctly.

BTW: Do you have an idea how we could export THE VideoCore Bus addresses in a sane manner to drivers like this?
Because then we could make the Driver work via DMA directly without explicit requirement for allocating Pages explicitly in the DMA range. This might be of advantage the spidev device which then could allow for direct DMA to userspace without additional copying...
Obviously this could be blocking integration with the upstream kernel...

@nneves
Copy link

nneves commented Nov 13, 2012

Unfortunately I can not help here, I've been tracking the initial discussion and found this to be pretty interesting to a project that I want to test in near future, for a microcontroller fast data acquisition module that would also take advantage from the direct DMA access to userspace.

I just wanted to make a reference to the original discussion so that others could track all of your previous work and see how important this may be!

BTW, thank you for all your hard work!

@msperl
Copy link
Contributor Author

msperl commented Nov 13, 2012

No Problem - I was actually asking the Foundation if they could not also get the documentation for the Camera interface open as well - it is essentially a differential SPI interface with 2 Data lanes, so we could actually use the interface as well for SPI stuff - and actually read 2 datastreams (for ADCs) in parallel. This would be nice for my long-term-sonar imaging project where i would need to read out multiple ADCs syncronously at high frequencies...

@msperl
Copy link
Contributor Author

msperl commented Dec 9, 2012

What about pulling that patch in?

BTW: as to DMA (from userspace), it seems as if the dmaer (DMAHelper) could get used to make direct DMA to/from userspace a possibility. Again this is a possibility for some sampling needs. With the 512MB RPIs this might be more of interrest, as more pages can get locked in memory...

@anroOfCode
Copy link

This looks like great work! I'll be pulling this into my kernel right now and it should help performance for some work I'm doing on getting a radio kernel module up and running. I was worried about the SPI latencies you demonstrated on the RPi forums.

I'll report back when I've had some time to test everything. Thanks for the good work!

@msperl
Copy link
Contributor Author

msperl commented Oct 18, 2013

I am closing this for now - a newer DMA version based on the upstream spi-BCM2835.c will appear at some point...

@msperl msperl closed this Oct 18, 2013
popcornmix pushed a commit that referenced this pull request Oct 22, 2016
commit 8571410 upstream.

When dma_common_free_remap, the input parameter 'size' may not
be page aligned. And, met kernel warning when doing iommu dma
for usb on i.MX8 platform:
"
WARNING: CPU: 0 PID: 869 at mm/vmalloc.c:70 vunmap_page_range+0x1cc/0x1d0()
Modules linked in:
CPU: 0 PID: 869 Comm: kworker/u8:2 Not tainted 4.1.12-00444-gc5f9d1d-dirty #147
Hardware name: Freescale i.MX8DV Sabreauto (DT)
Workqueue: ci_otg ci_otg_work
Call trace:
[<ffffffc000089920>] dump_backtrace+0x0/0x124
[<ffffffc000089a54>] show_stack+0x10/0x1c
[<ffffffc0006d1e6c>] dump_stack+0x84/0xc8
[<ffffffc0000b4568>] warn_slowpath_common+0x98/0xd0
[<ffffffc0000b4664>] warn_slowpath_null+0x14/0x20
[<ffffffc000170348>] vunmap_page_range+0x1c8/0x1d0
[<ffffffc000170388>] unmap_kernel_range+0x20/0x88
[<ffffffc000460ad0>] dma_common_free_remap+0x74/0x84
[<ffffffc0000940d8>] __iommu_free_attrs+0x9c/0x178
[<ffffffc0005032bc>] ehci_mem_cleanup+0x140/0x194
[<ffffffc000503548>] ehci_stop+0x8c/0xdc
[<ffffffc0004e8258>] usb_remove_hcd+0xf0/0x1cc
[<ffffffc000516bc0>] host_stop+0x1c/0x58
[<ffffffc000514240>] ci_otg_work+0xdc/0x120
[<ffffffc0000c9c34>] process_one_work+0x134/0x33c
[<ffffffc0000c9f78>] worker_thread+0x13c/0x47c
[<ffffffc0000cf43c>] kthread+0xd8/0xf0
"

For dma_common_pages_remap:
dma_common_pages_remap
   |->get_vm_area_caller
        |->__get_vm_area_node
            |->size = PAGE_ALIGN(size);   Round up to page aligned

So, in dma_common_free_remap, we also need a page aligned size,
pass 'PAGE_ALIGN(size)' to unmap_kernel_range.

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
popcornmix pushed a commit that referenced this pull request Oct 22, 2016
commit 8571410 upstream.

When dma_common_free_remap, the input parameter 'size' may not
be page aligned. And, met kernel warning when doing iommu dma
for usb on i.MX8 platform:
"
WARNING: CPU: 0 PID: 869 at mm/vmalloc.c:70 vunmap_page_range+0x1cc/0x1d0()
Modules linked in:
CPU: 0 PID: 869 Comm: kworker/u8:2 Not tainted 4.1.12-00444-gc5f9d1d-dirty #147
Hardware name: Freescale i.MX8DV Sabreauto (DT)
Workqueue: ci_otg ci_otg_work
Call trace:
[<ffffffc000089920>] dump_backtrace+0x0/0x124
[<ffffffc000089a54>] show_stack+0x10/0x1c
[<ffffffc0006d1e6c>] dump_stack+0x84/0xc8
[<ffffffc0000b4568>] warn_slowpath_common+0x98/0xd0
[<ffffffc0000b4664>] warn_slowpath_null+0x14/0x20
[<ffffffc000170348>] vunmap_page_range+0x1c8/0x1d0
[<ffffffc000170388>] unmap_kernel_range+0x20/0x88
[<ffffffc000460ad0>] dma_common_free_remap+0x74/0x84
[<ffffffc0000940d8>] __iommu_free_attrs+0x9c/0x178
[<ffffffc0005032bc>] ehci_mem_cleanup+0x140/0x194
[<ffffffc000503548>] ehci_stop+0x8c/0xdc
[<ffffffc0004e8258>] usb_remove_hcd+0xf0/0x1cc
[<ffffffc000516bc0>] host_stop+0x1c/0x58
[<ffffffc000514240>] ci_otg_work+0xdc/0x120
[<ffffffc0000c9c34>] process_one_work+0x134/0x33c
[<ffffffc0000c9f78>] worker_thread+0x13c/0x47c
[<ffffffc0000cf43c>] kthread+0xd8/0xf0
"

For dma_common_pages_remap:
dma_common_pages_remap
   |->get_vm_area_caller
        |->__get_vm_area_node
            |->size = PAGE_ALIGN(size);   Round up to page aligned

So, in dma_common_free_remap, we also need a page aligned size,
pass 'PAGE_ALIGN(size)' to unmap_kernel_range.

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
popcornmix pushed a commit that referenced this pull request Oct 22, 2016
commit 8571410 upstream.

When dma_common_free_remap, the input parameter 'size' may not
be page aligned. And, met kernel warning when doing iommu dma
for usb on i.MX8 platform:
"
WARNING: CPU: 0 PID: 869 at mm/vmalloc.c:70 vunmap_page_range+0x1cc/0x1d0()
Modules linked in:
CPU: 0 PID: 869 Comm: kworker/u8:2 Not tainted 4.1.12-00444-gc5f9d1d-dirty #147
Hardware name: Freescale i.MX8DV Sabreauto (DT)
Workqueue: ci_otg ci_otg_work
Call trace:
[<ffffffc000089920>] dump_backtrace+0x0/0x124
[<ffffffc000089a54>] show_stack+0x10/0x1c
[<ffffffc0006d1e6c>] dump_stack+0x84/0xc8
[<ffffffc0000b4568>] warn_slowpath_common+0x98/0xd0
[<ffffffc0000b4664>] warn_slowpath_null+0x14/0x20
[<ffffffc000170348>] vunmap_page_range+0x1c8/0x1d0
[<ffffffc000170388>] unmap_kernel_range+0x20/0x88
[<ffffffc000460ad0>] dma_common_free_remap+0x74/0x84
[<ffffffc0000940d8>] __iommu_free_attrs+0x9c/0x178
[<ffffffc0005032bc>] ehci_mem_cleanup+0x140/0x194
[<ffffffc000503548>] ehci_stop+0x8c/0xdc
[<ffffffc0004e8258>] usb_remove_hcd+0xf0/0x1cc
[<ffffffc000516bc0>] host_stop+0x1c/0x58
[<ffffffc000514240>] ci_otg_work+0xdc/0x120
[<ffffffc0000c9c34>] process_one_work+0x134/0x33c
[<ffffffc0000c9f78>] worker_thread+0x13c/0x47c
[<ffffffc0000cf43c>] kthread+0xd8/0xf0
"

For dma_common_pages_remap:
dma_common_pages_remap
   |->get_vm_area_caller
        |->__get_vm_area_node
            |->size = PAGE_ALIGN(size);   Round up to page aligned

So, in dma_common_free_remap, we also need a page aligned size,
pass 'PAGE_ALIGN(size)' to unmap_kernel_range.

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
0lxb pushed a commit to 0lxb/rpi_linux that referenced this pull request Jan 30, 2024
…find_and_get_node_by_id()

The BPF helper bpf_cgroup_from_id() calls kernfs_find_and_get_node_by_id()
which acquires kernfs_idr_lock, which is an non-raw non-IRQ-safe lock. This
can lead to deadlocks as bpf_cgroup_from_id() can be called from any BPF
programs including e.g. the ones that attach to functions which are holding
the scheduler rq lock.

Consider the following BPF program:

  SEC("fentry/__set_cpus_allowed_ptr_locked")
  int BPF_PROG(__set_cpus_allowed_ptr_locked, struct task_struct *p,
	       struct affinity_context *affn_ctx, struct rq *rq, struct rq_flags *rf)
  {
	  struct cgroup *cgrp = bpf_cgroup_from_id(p->cgroups->dfl_cgrp->kn->id);

	  if (cgrp) {
		  bpf_printk("%d[%s] in %s", p->pid, p->comm, cgrp->kn->name);
		  bpf_cgroup_release(cgrp);
	  }
	  return 0;
  }

__set_cpus_allowed_ptr_locked() is called with rq lock held and the above
BPF program calls bpf_cgroup_from_id() within leading to the following
lockdep warning:

  =====================================================
  WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
  6.7.0-rc3-work-00053-g07124366a1d7-dirty raspberrypi#147 Not tainted
  -----------------------------------------------------
  repro/1620 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
  ffffffff833b3688 (kernfs_idr_lock){+.+.}-{2:2}, at: kernfs_find_and_get_node_by_id+0x1e/0x70

		and this task is already holding:
  ffff888237ced698 (&rq->__lock){-.-.}-{2:2}, at: task_rq_lock+0x4e/0xf0
  which would create a new lock dependency:
   (&rq->__lock){-.-.}-{2:2} -> (kernfs_idr_lock){+.+.}-{2:2}
  ...
   Possible interrupt unsafe locking scenario:

	 CPU0                    CPU1
	 ----                    ----
    lock(kernfs_idr_lock);
				 local_irq_disable();
				 lock(&rq->__lock);
				 lock(kernfs_idr_lock);
    <Interrupt>
      lock(&rq->__lock);

		 *** DEADLOCK ***
  ...
  Call Trace:
   dump_stack_lvl+0x55/0x70
   dump_stack+0x10/0x20
   __lock_acquire+0x781/0x2a40
   lock_acquire+0xbf/0x1f0
   _raw_spin_lock+0x2f/0x40
   kernfs_find_and_get_node_by_id+0x1e/0x70
   cgroup_get_from_id+0x21/0x240
   bpf_cgroup_from_id+0xe/0x20
   bpf_prog_98652316e9337a5a___set_cpus_allowed_ptr_locked+0x96/0x11a
   bpf_trampoline_6442545632+0x4f/0x1000
   __set_cpus_allowed_ptr_locked+0x5/0x5a0
   sched_setaffinity+0x1b3/0x290
   __x64_sys_sched_setaffinity+0x4f/0x60
   do_syscall_64+0x40/0xe0
   entry_SYSCALL_64_after_hwframe+0x46/0x4e

Let's fix it by protecting kernfs_node and kernfs_root with RCU and making
kernfs_find_and_get_node_by_id() acquire rcu_read_lock() instead of
kernfs_idr_lock.

This adds an rcu_head to kernfs_node making it larger by 16 bytes on 64bit.
Combined with the preceding rearrange patch, the net increase is 8 bytes.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andrea Righi <andrea.righi@canonical.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
popcornmix pushed a commit that referenced this pull request Apr 16, 2024
…find_and_get_node_by_id()

[ Upstream commit 4207b55 ]

The BPF helper bpf_cgroup_from_id() calls kernfs_find_and_get_node_by_id()
which acquires kernfs_idr_lock, which is an non-raw non-IRQ-safe lock. This
can lead to deadlocks as bpf_cgroup_from_id() can be called from any BPF
programs including e.g. the ones that attach to functions which are holding
the scheduler rq lock.

Consider the following BPF program:

  SEC("fentry/__set_cpus_allowed_ptr_locked")
  int BPF_PROG(__set_cpus_allowed_ptr_locked, struct task_struct *p,
	       struct affinity_context *affn_ctx, struct rq *rq, struct rq_flags *rf)
  {
	  struct cgroup *cgrp = bpf_cgroup_from_id(p->cgroups->dfl_cgrp->kn->id);

	  if (cgrp) {
		  bpf_printk("%d[%s] in %s", p->pid, p->comm, cgrp->kn->name);
		  bpf_cgroup_release(cgrp);
	  }
	  return 0;
  }

__set_cpus_allowed_ptr_locked() is called with rq lock held and the above
BPF program calls bpf_cgroup_from_id() within leading to the following
lockdep warning:

  =====================================================
  WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
  6.7.0-rc3-work-00053-g07124366a1d7-dirty #147 Not tainted
  -----------------------------------------------------
  repro/1620 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
  ffffffff833b3688 (kernfs_idr_lock){+.+.}-{2:2}, at: kernfs_find_and_get_node_by_id+0x1e/0x70

		and this task is already holding:
  ffff888237ced698 (&rq->__lock){-.-.}-{2:2}, at: task_rq_lock+0x4e/0xf0
  which would create a new lock dependency:
   (&rq->__lock){-.-.}-{2:2} -> (kernfs_idr_lock){+.+.}-{2:2}
  ...
   Possible interrupt unsafe locking scenario:

	 CPU0                    CPU1
	 ----                    ----
    lock(kernfs_idr_lock);
				 local_irq_disable();
				 lock(&rq->__lock);
				 lock(kernfs_idr_lock);
    <Interrupt>
      lock(&rq->__lock);

		 *** DEADLOCK ***
  ...
  Call Trace:
   dump_stack_lvl+0x55/0x70
   dump_stack+0x10/0x20
   __lock_acquire+0x781/0x2a40
   lock_acquire+0xbf/0x1f0
   _raw_spin_lock+0x2f/0x40
   kernfs_find_and_get_node_by_id+0x1e/0x70
   cgroup_get_from_id+0x21/0x240
   bpf_cgroup_from_id+0xe/0x20
   bpf_prog_98652316e9337a5a___set_cpus_allowed_ptr_locked+0x96/0x11a
   bpf_trampoline_6442545632+0x4f/0x1000
   __set_cpus_allowed_ptr_locked+0x5/0x5a0
   sched_setaffinity+0x1b3/0x290
   __x64_sys_sched_setaffinity+0x4f/0x60
   do_syscall_64+0x40/0xe0
   entry_SYSCALL_64_after_hwframe+0x46/0x4e

Let's fix it by protecting kernfs_node and kernfs_root with RCU and making
kernfs_find_and_get_node_by_id() acquire rcu_read_lock() instead of
kernfs_idr_lock.

This adds an rcu_head to kernfs_node making it larger by 16 bytes on 64bit.
Combined with the preceding rearrange patch, the net increase is 8 bytes.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andrea Righi <andrea.righi@canonical.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Link: https://lore.kernel.org/r/20240109214828.252092-4-tj@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
popcornmix pushed a commit that referenced this pull request Apr 18, 2024
…find_and_get_node_by_id()

[ Upstream commit 4207b55 ]

The BPF helper bpf_cgroup_from_id() calls kernfs_find_and_get_node_by_id()
which acquires kernfs_idr_lock, which is an non-raw non-IRQ-safe lock. This
can lead to deadlocks as bpf_cgroup_from_id() can be called from any BPF
programs including e.g. the ones that attach to functions which are holding
the scheduler rq lock.

Consider the following BPF program:

  SEC("fentry/__set_cpus_allowed_ptr_locked")
  int BPF_PROG(__set_cpus_allowed_ptr_locked, struct task_struct *p,
	       struct affinity_context *affn_ctx, struct rq *rq, struct rq_flags *rf)
  {
	  struct cgroup *cgrp = bpf_cgroup_from_id(p->cgroups->dfl_cgrp->kn->id);

	  if (cgrp) {
		  bpf_printk("%d[%s] in %s", p->pid, p->comm, cgrp->kn->name);
		  bpf_cgroup_release(cgrp);
	  }
	  return 0;
  }

__set_cpus_allowed_ptr_locked() is called with rq lock held and the above
BPF program calls bpf_cgroup_from_id() within leading to the following
lockdep warning:

  =====================================================
  WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
  6.7.0-rc3-work-00053-g07124366a1d7-dirty #147 Not tainted
  -----------------------------------------------------
  repro/1620 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
  ffffffff833b3688 (kernfs_idr_lock){+.+.}-{2:2}, at: kernfs_find_and_get_node_by_id+0x1e/0x70

		and this task is already holding:
  ffff888237ced698 (&rq->__lock){-.-.}-{2:2}, at: task_rq_lock+0x4e/0xf0
  which would create a new lock dependency:
   (&rq->__lock){-.-.}-{2:2} -> (kernfs_idr_lock){+.+.}-{2:2}
  ...
   Possible interrupt unsafe locking scenario:

	 CPU0                    CPU1
	 ----                    ----
    lock(kernfs_idr_lock);
				 local_irq_disable();
				 lock(&rq->__lock);
				 lock(kernfs_idr_lock);
    <Interrupt>
      lock(&rq->__lock);

		 *** DEADLOCK ***
  ...
  Call Trace:
   dump_stack_lvl+0x55/0x70
   dump_stack+0x10/0x20
   __lock_acquire+0x781/0x2a40
   lock_acquire+0xbf/0x1f0
   _raw_spin_lock+0x2f/0x40
   kernfs_find_and_get_node_by_id+0x1e/0x70
   cgroup_get_from_id+0x21/0x240
   bpf_cgroup_from_id+0xe/0x20
   bpf_prog_98652316e9337a5a___set_cpus_allowed_ptr_locked+0x96/0x11a
   bpf_trampoline_6442545632+0x4f/0x1000
   __set_cpus_allowed_ptr_locked+0x5/0x5a0
   sched_setaffinity+0x1b3/0x290
   __x64_sys_sched_setaffinity+0x4f/0x60
   do_syscall_64+0x40/0xe0
   entry_SYSCALL_64_after_hwframe+0x46/0x4e

Let's fix it by protecting kernfs_node and kernfs_root with RCU and making
kernfs_find_and_get_node_by_id() acquire rcu_read_lock() instead of
kernfs_idr_lock.

This adds an rcu_head to kernfs_node making it larger by 16 bytes on 64bit.
Combined with the preceding rearrange patch, the net increase is 8 bytes.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andrea Righi <andrea.righi@canonical.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Link: https://lore.kernel.org/r/20240109214828.252092-4-tj@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants