Re: [syzbot] [dri?] [virtualization?] upstream boot error: INFO: task hung in virtio_gpu_queue_fenced_ctrl_buffer

2024-01-24 Thread Hillf Danton
On Wed, 24 Jan 2024 02:15:21 -0800
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:615d30064886 Merge tag 'trace-v6.8-rc1' of git://git.kerne..
> git tree:   upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=167456f7e8
> kernel config:  https://syzkaller.appspot.com/x/.config?x=e6c3b3d5f71246cb
> dashboard link: https://syzkaller.appspot.com/bug?extid=22e2c28c99235275f109
> compiler:   gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for 
> Debian) 2.40
> 
> Downloadable assets:
> disk image (non-bootable): 
> https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-615d3006.raw.xz
> vmlinux: 
> https://storage.googleapis.com/syzbot-assets/4bf0b27acaa4/vmlinux-615d3006.xz
> kernel image: 
> https://storage.googleapis.com/syzbot-assets/3133809ff35d/bzImage-615d3006.xz
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+22e2c28c99235275f...@syzkaller.appspotmail.com
> 
> INFO: task swapper/0:1 blocked for more than 143 seconds.
>   Not tainted 6.8.0-rc1-syzkaller-00029-g615d30064886 #0
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:swapper/0   state:D stack:22288 pid:1 tgid:1 ppid:0  
> flags:0x4000
> Call Trace:
>  
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0xf12/0x5c00 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0xe9/0x270 kernel/sched/core.c:6817
>  virtio_gpu_queue_ctrl_sgs drivers/gpu/drm/virtio/virtgpu_vq.c:341 [inline]
>  virtio_gpu_queue_fenced_ctrl_buffer+0x497/0xff0 
> drivers/gpu/drm/virtio/virtgpu_vq.c:415
>  virtio_gpu_resource_flush drivers/gpu/drm/virtio/virtgpu_plane.c:162 [inline]
>  virtio_gpu_primary_plane_update+0x1059/0x1590 
> drivers/gpu/drm/virtio/virtgpu_plane.c:237
>  drm_atomic_helper_commit_planes+0x92f/0xfe0 
> drivers/gpu/drm/drm_atomic_helper.c:2800
>  drm_atomic_helper_commit_tail+0x69/0xf0 
> drivers/gpu/drm/drm_atomic_helper.c:1749
>  commit_tail+0x353/0x410 drivers/gpu/drm/drm_atomic_helper.c:1834
>  drm_atomic_helper_commit+0x2f9/0x380 drivers/gpu/drm/drm_atomic_helper.c:2072
>  drm_atomic_commit+0x20b/0x2d0 drivers/gpu/drm/drm_atomic.c:1514
>  drm_client_modeset_commit_atomic+0x6c2/0x810 
> drivers/gpu/drm/drm_client_modeset.c:1051
>  drm_client_modeset_commit_locked+0x14d/0x580 
> drivers/gpu/drm/drm_client_modeset.c:1154
>  pan_display_atomic drivers/gpu/drm/drm_fb_helper.c:1370 [inline]
>  drm_fb_helper_pan_display+0x2a5/0x990 drivers/gpu/drm/drm_fb_helper.c:1430
>  fb_pan_display+0x477/0x7c0 drivers/video/fbdev/core/fbmem.c:191
>  bit_update_start+0x49/0x1f0 drivers/video/fbdev/core/bitblit.c:390
>  fbcon_switch+0xbb3/0x12e0 drivers/video/fbdev/core/fbcon.c:2170
>  redraw_screen+0x2bd/0x750 drivers/tty/vt/vt.c:969
>  fbcon_prepare_logo+0x9f8/0xc80 drivers/video/fbdev/core/fbcon.c:616
>  con2fb_init_display drivers/video/fbdev/core/fbcon.c:803 [inline]
>  set_con2fb_map+0xcea/0x1050 drivers/video/fbdev/core/fbcon.c:867
>  do_fb_registered drivers/video/fbdev/core/fbcon.c:3007 [inline]
>  fbcon_fb_registered+0x21d/0x660 drivers/video/fbdev/core/fbcon.c:3023
>  do_register_framebuffer drivers/video/fbdev/core/fbmem.c:449 [inline]
>  register_framebuffer+0x4b2/0x860 drivers/video/fbdev/core/fbmem.c:515
>  __drm_fb_helper_initial_config_and_unlock+0xd7c/0x1650 
> drivers/gpu/drm/drm_fb_helper.c:1871
>  drm_fb_helper_initial_config drivers/gpu/drm/drm_fb_helper.c:1936 [inline]
>  drm_fb_helper_initial_config+0x44/0x60 drivers/gpu/drm/drm_fb_helper.c:1928
>  drm_fbdev_generic_client_hotplug+0x19e/0x270 
> drivers/gpu/drm/drm_fbdev_generic.c:279
>  drm_client_register+0x195/0x280 drivers/gpu/drm/drm_client.c:141
>  drm_fbdev_generic_setup+0x184/0x340 drivers/gpu/drm/drm_fbdev_generic.c:341
>  virtio_gpu_probe+0x1be/0x3c0 drivers/gpu/drm/virtio/virtgpu_drv.c:105
>  virtio_dev_probe+0x5e4/0x980 drivers/virtio/virtio.c:311
>  call_driver_probe drivers/base/dd.c:579 [inline]
>  really_probe+0x234/0xc90 drivers/base/dd.c:658
>  __driver_probe_device+0x1de/0x4b0 drivers/base/dd.c:800
>  driver_probe_device+0x4c/0x1a0 drivers/base/dd.c:830
>  __driver_attach+0x274/0x570 drivers/base/dd.c:1216
>  bus_for_each_dev+0x13c/0x1d0 drivers/base/bus.c:368
>  bus_add_driver+0x2e9/0x630 drivers/base/bus.c:673
>  driver_register+0x15c/0x4a0 drivers/base/driver.c:246
>  do_one_initcall+0x11c/0x650 init/main.c:1236
>  do_initcall_level init/main.c:1298 [inline]
>  do_initcalls init/main.c:1314 [inline]
>  do_basic_setup init/main.c:1333 [inline]
>  kernel_init_freeable+0x687/0xc10 init/main.c:1551
>  kernel_init+0x1c/0x2a0 init/main.c:1441
>  ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:242
>  
> INFO: task kworker/0:0:8 blocked for more than 143 seconds.
>   Not tainted 6.8.0-rc1-syzkaller-00029-g615d30064886 #0
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" 

Re: [PATCH v1 4/6] drm/lima: handle spurious timeouts due to high irq latency

2024-01-21 Thread Hillf Danton
On Wed, 17 Jan 2024 04:12:10 +0100 Erico Nunes 
>  
> @@ -401,9 +399,33 @@ static enum drm_gpu_sched_stat 
> lima_sched_timedout_job(struct drm_sched_job *job
>   struct lima_sched_pipe *pipe = to_lima_pipe(job->sched);
>   struct lima_sched_task *task = to_lima_task(job);
>   struct lima_device *ldev = pipe->ldev;
> + struct lima_ip *ip = pipe->processor[0];
> +
> + /*
> +  * If the GPU managed to complete this jobs fence, the timeout is
> +  * spurious. Bail out.
> +  */
> + if (dma_fence_is_signaled(task->done_fence)) {
> + DRM_WARN("%s spurious timeout\n", lima_ip_name(ip));
> + return DRM_GPU_SCHED_STAT_NOMINAL;
> + }

Given 500ms in lima_sched_pipe_init(), no timeout is spurious by define,
and stop selling bandaid like this because you have options like locating
the reasons behind timeout.
> +
> + /*
> +  * Lima IRQ handler may take a long time to process an interrupt
> +  * if there is another IRQ handler hogging the processing.
> +  * In order to catch such cases and not report spurious Lima job
> +  * timeouts, synchronize the IRQ handler and re-check the fence
> +  * status.
> +  */
> + synchronize_irq(ip->irq);
> +
> + if (dma_fence_is_signaled(task->done_fence)) {
> + DRM_WARN("%s unexpectedly high interrupt latency\n", 
> lima_ip_name(ip));
> + return DRM_GPU_SCHED_STAT_NOMINAL;
> + }
>  
>   if (!pipe->error)
> - DRM_ERROR("lima job timeout\n");
> + DRM_ERROR("%s lima job timeout\n", lima_ip_name(ip));
>  
>   drm_sched_stop(>base, >base);
>  


Re: INFO: rcu_sched detected expedited stalls

2023-04-12 Thread Hillf Danton
On Wed, 12 Apr 2023 10:15:29 +0100 Rui Salvaterra 
> Hi, everyone,
> 
> I apologise in advance if I've sent this to {too many, the wrong}
> people. For some time now, I've been getting sporadic (in about one
> out of five boots) DRM-related RCU complaints on an Ivy Bridge-based
> (Core i7-3720QM) Mac Mini. Call trace (on Linux 6.3-rc6) follows:

Feel free to post again after taking a look at the popular format of
syzbot report[1].

[1] https://lore.kernel.org/lkml/f1a9d205f909f...@google.com/

> [5.794026] rcu: INFO: rcu_sched detected expedited stalls on
> CPUs/tasks: { 0- } 3 jiffies s: 309 root: 0x1/.
> [5.794044] rcu: blocking rcu_node structures (internal RCU debug):
> [5.794045] Sending NMI from CPU 1 to CPUs 0:
> [5.794049] NMI backtrace for cpu 0
> [5.794051] CPU: 0 PID: 537 Comm: Xorg Not tainted 6.3.0-rc6-debug+ #57
> [5.794053] Hardware name: Apple Inc. Macmini6,2/Mac-F65AE981FFA204ED, 
> BIOS 429.0.0.0.0 03/18/2022


Re: [PATCH v3 01/11] dmaengine: Add API function dmaengine_prep_slave_dma_array()

2023-04-03 Thread Hillf Danton
On 3 Apr 2023 17:47:50 +0200 Paul Cercueil 
> This function can be used to initiate a scatter-gather DMA transfer
> where the DMA addresses and lengths are located inside arrays.
> 
> The major difference with dmaengine_prep_slave_sg() is that it supports
> specifying the lengths of each DMA transfer; as trying to override the
> length of the transfer with dmaengine_prep_slave_sg() is a very tedious
> process. The introduction of a new API function is also justified by the
> fact that scatterlists are on their way out.

Given sg's wayout and conceptually iovec and kvec (in include/linux/uio.h),
what you add should have been dma_vec to ease people making use of it.

struct dma_vec {
dma_addr_t  addr;
size_t  len;
};
> 
> Signed-off-by: Paul Cercueil 
> 
> ---
> v3: New patch
> ---
>  include/linux/dmaengine.h | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index c3656e590213..62efa28c009a 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -912,6 +912,11 @@ struct dma_device {
>   struct dma_async_tx_descriptor *(*device_prep_dma_interrupt)(
>   struct dma_chan *chan, unsigned long flags);
>  
> + struct dma_async_tx_descriptor *(*device_prep_slave_dma_array)(
> + struct dma_chan *chan, dma_addr_t *addrs,
> + size_t *lengths, size_t nb,
> + enum dma_transfer_direction direction,
> + unsigned long flags);

Then the callback looks like

struct dma_async_tx_descriptor *(*device_prep_slave_vec)(
struct dma_chan *chan,
struct dma_vec *vec,
int nvec,
enum dma_transfer_direction direction,
unsigned long flags);


Re: [PATCH 3/4] binder: Add flags to relinquish ownership of fds

2023-01-09 Thread Hillf Danton
On 9 Jan 2023 21:38:06 + T.J. Mercier 
>  
> @@ -2275,6 +2276,26 @@ static int binder_translate_fd(u32 fd, binder_size_t 
> fd_offset,
>   goto err_security;
>   }
>  
> + if (IS_ENABLED(CONFIG_MEMCG) && (flags & BINDER_FD_FLAG_XFER_CHARGE)) {
> + struct dma_buf *dmabuf;
> +
> + if (unlikely(!is_dma_buf_file(file))) {
> + binder_user_error(
> + "%d:%d got transaction with XFER_CHARGE for 
> non-dmabuf fd, %d\n",
> + proc->pid, thread->pid, fd);
> + ret = -EINVAL;
> + goto err_dmabuf;
> + }

It barely makes sense to expose is_dma_buf_file() only for this.
> +
> + dmabuf = file->private_data;
> + ret = dma_buf_transfer_charge(dmabuf, target_proc->tsk);
> + if (ret) {
> + pr_warn("%d:%d Unable to transfer DMA-BUF fd charge to 
> %d\n",
> + proc->pid, thread->pid, target_proc->pid);
> + goto err_xfer;
> + }
> + }
> +

This whole hunk should go to dma-buf instead by adding change to
dma_buf_transfer_charge() for instance.


Re: [PATCH v2 2/2] drm/msm: Hangcheck progress detection

2022-11-01 Thread Hillf Danton
On 1 Nov 2022 15:33:10 -0700 Rob Clark 
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -500,6 +500,21 @@ static void hangcheck_timer_reset(struct msm_gpu *gpu)
>   round_jiffies_up(jiffies + 
> msecs_to_jiffies(priv->hangcheck_period)));
>  }
>  
> +static bool made_progress(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
> +{
> + if (ring->hangcheck_progress_retries >= 
> DRM_MSM_HANGCHECK_PROGRESS_RETRIES)
> + return false;
> +
> + if (!gpu->funcs->progress)
> + return false;

Retry can not make difference without the progress callback provided.

> +
> + if (!gpu->funcs->progress(gpu, ring))
> + return false;
> +
> + ring->hangcheck_progress_retries++;
> + return true;
> +}
> +
>  static void hangcheck_handler(struct timer_list *t)
>  {
>   struct msm_gpu *gpu = from_timer(gpu, t, hangcheck_timer);
> @@ -511,9 +526,12 @@ static void hangcheck_handler(struct timer_list *t)
>   if (fence != ring->hangcheck_fence) {
>   /* some progress has been made.. ya! */
>   ring->hangcheck_fence = fence;
> - } else if (fence_before(fence, ring->fctx->last_fence)) {
> + ring->hangcheck_progress_retries = 0;
> + } else if (fence_before(fence, ring->fctx->last_fence) &&
> + !made_progress(gpu, ring)) {
>   /* no progress and not done.. hung! */
>   ring->hangcheck_fence = fence;
> + ring->hangcheck_progress_retries = 0;
>   DRM_DEV_ERROR(dev->dev, "%s: hangcheck detected gpu lockup rb 
> %d!\n",
>   gpu->name, ring->id);
>   DRM_DEV_ERROR(dev->dev, "%s: completed fence: %u\n",

Cutting DRM_MSM_HANGCHECK_DEFAULT_PERIOD down to 250ms leads to report of
false hang detected in case of no ->progress implemented.

> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 585fd9c8d45a..d8f355e9f0b2 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -78,6 +78,8 @@ struct msm_gpu_funcs {
>   struct msm_gem_address_space *(*create_private_address_space)
>   (struct msm_gpu *gpu);
>   uint32_t (*get_rptr)(struct msm_gpu *gpu, struct msm_ringbuffer *ring);
> +
> + bool (*progress)(struct msm_gpu *gpu, struct msm_ringbuffer *ring);
>  };
>  
>  /* Additional state for iommu faults: */
> @@ -236,7 +238,8 @@ struct msm_gpu {
>*/
>  #define DRM_MSM_INACTIVE_PERIOD   66 /* in ms (roughly four frames) */
>  
> -#define DRM_MSM_HANGCHECK_DEFAULT_PERIOD 500 /* in ms */
> +#define DRM_MSM_HANGCHECK_DEFAULT_PERIOD 250 /* in ms */
> +#define DRM_MSM_HANGCHECK_PROGRESS_RETRIES 3


Re: [Freedreno] [PATCH 3/3] drm/msm/gpu: Remove mutex from wait_event condition

2022-03-17 Thread Hillf Danton
On Thu, 17 Mar 2022 14:07:45 -0700 Rob Clark wrote:
> On Thu, Mar 17, 2022 at 1:45 PM Akhil P Oommen  
> wrote:
> >
> > On 3/11/2022 5:16 AM, Rob Clark wrote:
> > > From: Rob Clark 
> > >
> > > The mutex wasn't really protecting anything before.  Before the previous
> > > patch we could still be racing with the scheduler's kthread, as that is
> > > not necessarily frozen yet.  Now that we've parked the sched threads,
> > > the only race is with jobs retiring, and that is harmless, ie.
> > >
> > > Signed-off-by: Rob Clark 
> > > ---
> > >   drivers/gpu/drm/msm/adreno/adreno_device.c | 11 +--
> > >   1 file changed, 1 insertion(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
> > > b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > index 0440a98988fc..661dfa7681fb 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > @@ -607,15 +607,6 @@ static int adreno_runtime_resume(struct device *dev)
> > >   return gpu->funcs->pm_resume(gpu);
> > >   }
> > >
> > > -static int active_submits(struct msm_gpu *gpu)
> > > -{
> > > - int active_submits;
> > > - mutex_lock(>active_lock);
> > > - active_submits = gpu->active_submits;
> > > - mutex_unlock(>active_lock);
> > I assumed that this lock here was to ensure proper barriers while
> > reading active_submits. Is that not required?
> 
> There is a spinlock in prepare_to_wait_event() ahead of checking the
> condition, which AFAIU is a sufficient barrier

set_current_state() is instead - feel free to grep it in 

Hillf
> 
> BR,
> -R
> 
> >
> > -Akhil.
> > > - return active_submits;
> > > -}
> > > -
> > >   static int adreno_runtime_suspend(struct device *dev)
> > >   {
> > >   struct msm_gpu *gpu = dev_to_gpu(dev);
> > > @@ -669,7 +660,7 @@ static int adreno_system_suspend(struct device *dev)
> > >   suspend_scheduler(gpu);
> > >
> > >   remaining = wait_event_timeout(gpu->retire_event,
> > > -active_submits(gpu) == 0,
> > > +gpu->active_submits == 0,
> > >  msecs_to_jiffies(1000));
> > >   if (remaining == 0) {
> > >   dev_err(dev, "Timeout waiting for GPU to suspend\n");
> >


Re: [RFC PATCH 2/4] DRM: Add support of AI Processor Unit (APU)

2021-09-18 Thread Hillf Danton
On Fri, 17 Sep 2021 14:59:43 +0200 Alexandre Bailon wrote:
> +static DEFINE_IDA(req_ida);
> +static LIST_HEAD(complete_node);

I see accesses to complete_node in apu_drm_callback(), apu_job_timedout()
and ioctl_gem_dequeue() without working out the serialization to avoid
list corruption. Can you add a comment to specify it?

> +
> +int apu_drm_callback(struct apu_core *apu_core, void *data, int len)
> +{
> + struct apu_request *apu_req, *tmp;
> + struct apu_dev_request *hdr = data;
> + unsigned long flags;
> +
> + spin_lock_irqsave(_core->ctx_lock, flags);
> + list_for_each_entry_safe(apu_req, tmp, _core->requests, node) {
> + struct apu_job *job = apu_req->job;
> +
> + if (job && hdr->id == job->id) {
> + kref_get(>refcount);
> + job->result = hdr->result;
> + if (job->size_out)
> + memcpy(job->data_out, hdr->data + job->size_in,
> +min(job->size_out, hdr->size_out));
> + job->size_out = hdr->size_out;
> + list_add(>node, _node);
> + list_del(_req->node);
> + ida_simple_remove(_ida, hdr->id);
> + kfree(apu_req);
> + drm_send_event(job->apu_drm->drm,
> +>event->pending_event);
> + dma_fence_signal_locked(job->done_fence);
> + }
> + }
> + spin_unlock_irqrestore(_core->ctx_lock, flags);
> +
> + return 0;
> +}


Re: [PATCH v2 5/5] drm/ingenic: Add option to alloc cached GEM buffers

2021-03-10 Thread Hillf Danton
On Wed, 10 Mar 2021 19:01:01 + Paul Cercueil wrote:
>Le lun. 8 mars 2021 � 11:47, Hillf Danton  a �crit :
>> On Sun,  7 Mar 2021 20:28:35 +  Paul Cercueil wrote:
>>>  With the module parameter ingenic-drm.cached_gem_buffers, it is 
>>> possible
>>>  to specify that we want GEM buffers backed by non-coherent memory.
>>> 
>>>  This dramatically speeds up software rendering on Ingenic SoCs, 
>>> even for
>>>  tasks where write-combine memory should in theory be faster (e.g. 
>>> simple
>>>  blits).
>> 
>> Wondering if it is due to the tricks at [1].
>> 
>> If so, is dma_alloc_noncoherent() necessary in this patchset?
>
>You confuse non-contiguous with non-coherent, which are two different 
>things.

You misunderstood me. From [1] we know coherent caching is arch thing,
so your proposal is not mandatory on ARM IMHO - what baffles me is
noncoherent back memory can speed up device, coherent ot not, regardless
of arch. Can you point me to the reasons behind your speedup?

>
>Cheers,
>-Paul
>
>> Christoph can you give us a concise lesson on noncoherency covering 
>> at least
>> noncoherent device, noncoherent memory(used in this work), no coherent
>> caching(in [1]), their links to speedup, and the thumb rule to handle
>> noncoherency in workdays. It feels toe curling every time I see 
>> noncoherence
>> going downtown with speedup hand in hand.
>> 
>> [1] Subject: [PATCH 6/6] media: uvcvideo: Use dma_alloc_noncontiguos 
>> API
>> https://lore.kernel.org/lkml/20210301085236.947011-7-...@lst.de/#t
>> 
>>> 
>>>  Leave it disabled by default, since it is specific to one use-case
>>>  (software rendering).
>>> 
>>>  v2: Rework code to work with new DRM APIs regarding plane states
>>> 
>>>  Signed-off-by: Paul Cercueil 
>>>  ---
>>>   drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 49 
>>> ++-
>>>   drivers/gpu/drm/ingenic/ingenic-drm.h |  4 ++
>>>   drivers/gpu/drm/ingenic/ingenic-ipu.c | 14 ++-
>>>   3 files changed, 63 insertions(+), 4 deletions(-)
>>> 
>>>  diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
>>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>  index d60e1eefc9d1..ba1ac0fcda74 100644
>>>  --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>  +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>  @@ -9,6 +9,7 @@
>>>   #include 
>>>   #include 
>>>   #include 
>>>  +#include 
>>>   #include 
>>>   #include 
>>>   #include 
>>>  @@ -23,6 +24,7 @@
>>>   #include 
>>>   #include 
>>>   #include 
>>>  +#include 
>>>   #include 
>>>   #include 
>>>   #include 
>>>  @@ -99,6 +101,11 @@ struct ingenic_drm {
>>> struct notifier_block clock_nb;
>>>   };
>>> 
>>>  +static bool ingenic_drm_cached_gem_buf;
>>>  +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, 
>>> bool, 0400);
>>>  +MODULE_PARM_DESC(cached_gem_buffers,
>>>  +   "Enable fully cached GEM buffers [default=false]");
>>>  +
>>>   static bool ingenic_drm_writeable_reg(struct device *dev, unsigned 
>>> int reg)
>>>   {
>>> switch (reg) {
>>>  @@ -410,6 +417,8 @@ static int 
>>> ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>>>  old_plane_state->fb->format->format != 
>>> new_plane_state->fb->format->format))
>>> crtc_state->mode_changed = true;
>>> 
>>>  +  drm_atomic_helper_check_plane_damage(state, new_plane_state);
>>>  +
>>> return 0;
>>>   }
>>> 
>>>  @@ -541,10 +550,20 @@ static void ingenic_drm_update_palette(struct 
>>> ingenic_drm *priv,
>>> }
>>>   }
>>> 
>>>  +void ingenic_drm_sync_data(struct device *dev,
>>>  + struct drm_plane_state *old_state,
>>>  + struct drm_plane_state *state)
>>>  +{
>>>  +  if (ingenic_drm_cached_gem_buf)
>>>  +  drm_gem_cma_sync_data(dev, old_state, state);
>>>  +}
>>>  +
>>>   static void ingenic_drm_plane_atomic_update(struct drm_plane 
>>> *plane,
>>> struct drm_atomic_state *state)
>>>   {
>>> struct ingenic_drm *priv = drm_device_get_priv(plane->dev);
>>>  +  struct drm_p

Re: [PATCH v2 5/5] drm/ingenic: Add option to alloc cached GEM buffers

2021-03-08 Thread Hillf Danton
On Sun,  7 Mar 2021 20:28:35 +  Paul Cercueil wrote:
> With the module parameter ingenic-drm.cached_gem_buffers, it is possible
> to specify that we want GEM buffers backed by non-coherent memory.
> 
> This dramatically speeds up software rendering on Ingenic SoCs, even for
> tasks where write-combine memory should in theory be faster (e.g. simple
> blits).

Wondering if it is due to the tricks at [1].

If so, is dma_alloc_noncoherent() necessary in this patchset?

Christoph can you give us a concise lesson on noncoherency covering at least
noncoherent device, noncoherent memory(used in this work), no coherent
caching(in [1]), their links to speedup, and the thumb rule to handle
noncoherency in workdays. It feels toe curling every time I see noncoherence
going downtown with speedup hand in hand.

[1] Subject: [PATCH 6/6] media: uvcvideo: Use dma_alloc_noncontiguos API
https://lore.kernel.org/lkml/20210301085236.947011-7-...@lst.de/#t

> 
> Leave it disabled by default, since it is specific to one use-case
> (software rendering).
> 
> v2: Rework code to work with new DRM APIs regarding plane states
> 
> Signed-off-by: Paul Cercueil 
> ---
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 49 ++-
>  drivers/gpu/drm/ingenic/ingenic-drm.h |  4 ++
>  drivers/gpu/drm/ingenic/ingenic-ipu.c | 14 ++-
>  3 files changed, 63 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index d60e1eefc9d1..ba1ac0fcda74 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -23,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -99,6 +101,11 @@ struct ingenic_drm {
>   struct notifier_block clock_nb;
>  };
>  
> +static bool ingenic_drm_cached_gem_buf;
> +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool, 
> 0400);
> +MODULE_PARM_DESC(cached_gem_buffers,
> +  "Enable fully cached GEM buffers [default=false]");
> +
>  static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg)
>  {
>   switch (reg) {
> @@ -410,6 +417,8 @@ static int ingenic_drm_plane_atomic_check(struct 
> drm_plane *plane,
>old_plane_state->fb->format->format != 
> new_plane_state->fb->format->format))
>   crtc_state->mode_changed = true;
>  
> + drm_atomic_helper_check_plane_damage(state, new_plane_state);
> +
>   return 0;
>  }
>  
> @@ -541,10 +550,20 @@ static void ingenic_drm_update_palette(struct 
> ingenic_drm *priv,
>   }
>  }
>  
> +void ingenic_drm_sync_data(struct device *dev,
> +struct drm_plane_state *old_state,
> +struct drm_plane_state *state)
> +{
> + if (ingenic_drm_cached_gem_buf)
> + drm_gem_cma_sync_data(dev, old_state, state);
> +}
> +
>  static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>   struct drm_atomic_state *state)
>  {
>   struct ingenic_drm *priv = drm_device_get_priv(plane->dev);
> + struct drm_plane_state *oldstate = drm_atomic_get_old_plane_state(state,
> +   
> plane);
>   struct drm_plane_state *newstate = drm_atomic_get_new_plane_state(state,
> 
> plane);
>   struct drm_crtc_state *crtc_state;
> @@ -554,6 +573,8 @@ static void ingenic_drm_plane_atomic_update(struct 
> drm_plane *plane,
>   u32 fourcc;
>  
>   if (newstate && newstate->fb) {
> + ingenic_drm_sync_data(priv->dev, oldstate, newstate);
> +
>   crtc_state = newstate->crtc->state;
>  
>   addr = drm_fb_cma_get_gem_addr(newstate->fb, newstate, 0);
> @@ -743,6 +764,26 @@ static void ingenic_drm_disable_vblank(struct drm_crtc 
> *crtc)
>   regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_EOF_IRQ, 0);
>  }
>  
> +static struct drm_framebuffer *
> +ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
> +   const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + if (ingenic_drm_cached_gem_buf)
> + return drm_gem_fb_create_with_dirty(dev, file, mode_cmd);
> +
> + return drm_gem_fb_create(dev, file, mode_cmd);
> +}
> +
> +static int ingenic_drm_gem_cma_dumb_create(struct drm_file *file_priv,
> +struct drm_device *drm,
> +struct drm_mode_create_dumb *args)
> +{
> + if (ingenic_drm_cached_gem_buf)
> + return drm_gem_cma_dumb_create_noncoherent(file_priv, drm, 
> args);
> +
> + return drm_gem_cma_dumb_create(file_priv, drm, args);
> +}
> +
>  

Re: 5.11-rc1 TTM list corruption

2021-01-01 Thread Hillf Danton
On Thu, 31 Dec 2020 11:40:20 +0100 Borislav Petkov wrote:
> Hi folks,
> 
> got this when trying to suspend my workstation to disk, it was still
> responsive so I could catch the splat:
> 
> [22020.334381] [ cut here ]
> [22020.339057] list_del corruption. next->prev should be 8b7a9a40, 
> but was 8881020bced0
> [22020.347764] WARNING: CPU: 12 PID: 13134 at lib/list_debug.c:54 
> __list_del_entry_valid+0x8a/0x90
> [22020.356397] Modules linked in: fuse essiv authenc nft_counter nf_tables 
> libcrc32c nfnetlink loop dm_crypt dm_mod amd64_edac edac_mce_amd kvm_amd 
> snd_hda_codec_realtek snd_hda_codec_generic led_class kvm ledtrig_audio 
> snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core 
> snd_pcm snd_timer irqbypass crct10dif_pclmul snd crc32_pclmul crc32c_intel 
> ghash_clmulni_intel pcspkr k10temp soundcore gpio_amdpt gpio_generic 
> acpi_cpufreq radeon aesni_intel glue_helper crypto_simd cryptd pinctrl_amd
> [22020.400855] CPU: 12 PID: 13134 Comm: hib.sh Not tainted 5.11.0-rc1+ #2
> [22020.400857] Hardware name: Micro-Star International Co., Ltd. MS-7B79/X470 
> GAMING PRO (MS-7B79), BIOS 1.70 01/23/2019
> [22020.400858] RIP: 0010:__list_del_entry_valid+0x8a/0x90
> [22020.400861] Code: 46 00 0f 0b 31 c0 c3 48 89 f2 48 89 fe 48 c7 c7 78 30 0f 
> 82 e8 24 6c 46 00 0f 0b 31 c0 c3 48 c7 c7 b8 30 0f 82 e8 13 6c 46 00 <0f> 0b 
> 31 c0 c3 cc 48 85 d2 89 f8 74 20 48 8d 0c 16 0f b6 16 48 ff
> [22020.400863] RSP: 0018:c90001fbbcf8 EFLAGS: 00010292
> [22020.441503] RAX: 0054 RBX: 8b7a9a40 RCX: 
> 
> [22020.441505] RDX: 8887fef26600 RSI: 8887fef17450 RDI: 
> 8887fef17450
> [22020.441505] RBP: 3f82 R08: 8887fef17450 R09: 
> c90001fbbb38
> [22020.441506] R10: 0001 R11: 0001 R12: 
> 
> [22020.441507] R13: 0080 R14: 0480 R15: 
> 019b
> [22020.441508] FS:  7f51c72f9740() GS:8887fef0() 
> knlGS:
> [22020.490045] CS:  0010 DS:  ES:  CR0: 80050033
> [22020.490046] CR2: 5557afb81018 CR3: 00012099e000 CR4: 
> 003506e0
> [22020.490047] Call Trace:
> [22020.490048]  ttm_pool_shrink+0x61/0xd0
> [22020.508965]  ttm_pool_shrinker_scan+0xa/0x20
> [22020.508966]  shrink_slab.part.0.constprop.0+0x1a1/0x330
> [22020.508970]  drop_slab_node+0x37/0x50
> [22020.522011]  drop_slab+0x33/0x60
> [22020.522012]  drop_caches_sysctl_handler+0x70/0x80
> [22020.522015]  proc_sys_call_handler+0x140/0x220
> [22020.534286]  new_sync_write+0x10b/0x190
> [22020.534289]  vfs_write+0x1b7/0x290
> [22020.534291]  ksys_write+0x60/0xe0
> [22020.544762]  do_syscall_64+0x33/0x40
> [22020.544765]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [22020.553320] RIP: 0033:0x7f51c73eaff3
> [22020.553322] Code: 8b 15 a1 ee 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb 
> b7 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 01 00 00 00 0f 05 <48> 3d 
> 00 f0 ff ff 77 55 c3 0f 1f 40 00 48 83 ec 28 48 89 54 24 18
> [22020.553324] RSP: 002b:7ffd0a748ef8 EFLAGS: 0246 ORIG_RAX: 
> 0001
> [22020.553325] RAX: ffda RBX: 0002 RCX: 
> 7f51c73eaff3
> [22020.553326] RDX: 0002 RSI: 56039fd0ee70 RDI: 
> 0001
> [22020.553327] RBP: 56039fd0ee70 R08: 000a R09: 
> 0001
> [22020.553327] R10: 56039fd0e770 R11: 0246 R12: 
> 0002
> [22020.611218] R13: 7f51c74bb6a0 R14: 0002 R15: 
> 7f51c74bb8a0
> [22020.611220] ---[ end trace f7ea94a6ddb98f71 ]---
> 
> -- 
> Regards/Gruss,
> Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette
> 

Here is a typo fix. Wish it helps spot the reason behind the slpat.

--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -188,10 +188,12 @@ static int ttm_pool_map(struct ttm_pool
addr = dma->addr;
} else {
size_t size = (1ULL << order) * PAGE_SIZE;
+   int err;
 
addr = dma_map_page(pool->dev, p, 0, size, DMA_BIDIRECTIONAL);
-   if (dma_mapping_error(pool->dev, **dma_addr))
-   return -EFAULT;
+   err = dma_mapping_error(pool->dev, addr);
+   if (err)
+   return err;
}
 
for (i = 1 << order; i ; --i) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 5/7] dma-buf: system_heap: Allocate higher order pages if available

2020-10-30 Thread Hillf Danton
On Thu, 29 Oct 2020 12:34:51 -0700 John Stultz wrote:
> 
> As for your comment on HPAGE_PMD_ORDER (9 on arm64/arm) and
> PAGE_ALLOC_COSTLY_ORDER(3), I'm not totally sure I understand your
> question? Are you suggesting those values would be more natural orders
> to choose from?

The numbers, 9 and 3, are not magic themselves but under the mm diretory
they draw more attentions than others do. Sometimes it would take two
minutes for me to work out that HPAGE_PMD_ORDER does not mean 1MiB, on
platforms like arm64 or not.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 7/7] dma-buf: system_heap: Add a system-uncached heap re-using the system heap

2020-10-30 Thread Hillf Danton
On Thu, 29 Oct 2020 15:28:34 -0700 John Stultz wrote:
> On Thu, Oct 29, 2020 at 12:10 AM Hillf Danton  wrote:
> > On Thu, 29 Oct 2020 00:16:24 + John Stultz wrote:
> > > @@ -194,6 +210,9 @@ static int system_heap_mmap(struct dma_buf *dmabuf, 
> > > struct vm_area_struct *vma)
> > >   struct sg_page_iter piter;
> > >   int ret;
> > >
> > > + if (buffer->uncached)
> > > + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> > > +
> >
> > Wonder why you turn back to dma_mmap_wc() and friends?
> 
> Sorry, can you expand on what you are proposing here instead?  I'm not
> sure I see how dma_alloc/mmap/*_wc() quite fits here.

I just wondered if *_wc() could save you two minutes or three. Can you
shed some light on your concerns about their unfitness?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 7/7] dma-buf: system_heap: Add a system-uncached heap re-using the system heap

2020-10-30 Thread Hillf Danton
On Thu, 29 Oct 2020 21:04:30 -0700 John Stultz wrote:
> 
> But I'll try to share my thoughts:
> 
> So the system heap allows for allocation of non-contiguous buffers
> (currently allocated from page_alloc), which we keep track using
> sglists.
> Since the resulting dmabufs are shared between multiple devices, we
> want to provide a *specific type of memory* (in this case
> non-contiguous system memory), rather than what the underlying
> dma_alloc_attr() allocates for a specific device.

If the memory slice(just a page for simple case) from
dma_alloc_attr(for device-A) does not work for device-B, nor can
page_alloc do the job imho.
> 
> My sense is dma_mmap_wc() likely ought to be paired with switching to
> using dma_alloc_wc() as well, which calls down to dma_alloc_attr().
> Maybe one could use dma_alloc_attr against the heap device to allocate
> chunks that we track in the sglist. But I'm not sure how that saves us
> much other than possibly swapping dma_mmap_wc() for remap_pfn_range()?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 5/7] dma-buf: system_heap: Allocate higher order pages if available

2020-10-29 Thread Hillf Danton
On Thu, 29 Oct 2020 00:16:22 + John Stultz wrote:
> 
> +#define HIGH_ORDER_GFP  (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \
> + | __GFP_NORETRY) & ~__GFP_RECLAIM) \
> + | __GFP_COMP)
> +#define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO | __GFP_COMP)
> +static gfp_t order_flags[] = {HIGH_ORDER_GFP, LOW_ORDER_GFP, LOW_ORDER_GFP};
> +static const unsigned int orders[] = {8, 4, 0};
> +#define NUM_ORDERS ARRAY_SIZE(orders)

A two-line comment helps much understand the ORDERs above if it specifies the
reasons behind the detour to HPAGE_PMD_ORDER and PAGE_ALLOC_COSTLY_ORDER.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 7/7] dma-buf: system_heap: Add a system-uncached heap re-using the system heap

2020-10-29 Thread Hillf Danton
On Thu, 29 Oct 2020 00:16:24 + John Stultz wrote:
> 
> This adds a heap that allocates non-contiguous buffers that are
> marked as writecombined, so they are not cached by the CPU.
> 
> This is useful, as most graphics buffers are usually not touched
> by the CPU or only written into once by the CPU. So when mapping
> the buffer over and over between devices, we can skip the CPU
> syncing, which saves a lot of cache management overhead, greatly
> improving performance.
> 
> For folk using ION, there was a ION_FLAG_CACHED flag, which
> signaled if the returned buffer should be CPU cacheable or not.
> With DMA-BUF heaps, we do not yet have such a flag, and by default
> the current heaps (system and cma) produce CPU cachable buffers.
> So for folks transitioning from ION to DMA-BUF Heaps, this fills
> in some of that missing functionality.
> 
> There has been a suggestion to make this functionality a flag
> (DMAHEAP_FLAG_UNCACHED?) on the system heap, similar to how
> ION used the ION_FLAG_CACHED. But I want to make sure an
> _UNCACHED flag would truely be a generic attribute across all
> heaps. So far that has been unclear, so having it as a separate
> heap seemes better for now. (But I'm open to discussion on this
> point!)
> 
> This is a rework of earlier efforts to add a uncached system heap,
> done utilizing the exisitng system heap, adding just a bit of
> logic to handle the uncached case.
> 
> Feedback would be very welcome!
> 
> Many thanks to Liam Mark for his help to get this working.
> 
> Pending opensource users of this code include:
> * AOSP HiKey960 gralloc:
>   - https://android-review.googlesource.com/c/device/linaro/hikey/+/1399519
>   - Visibly improves performance over the system heap
> * AOSP Codec2 (possibly, needs more review):
>   - 
> https://android-review.googlesource.com/c/platform/frameworks/av/+/1360640/17/media/codec2/vndk/C2DmaBufAllocator.cpp#325
> 
> Cc: Sumit Semwal 
> Cc: Liam Mark 
> Cc: Laura Abbott 
> Cc: Brian Starkey 
> Cc: Hridya Valsaraju 
> Cc: Suren Baghdasaryan 
> Cc: Sandeep Patil 
> Cc: Daniel Mentz 
> Cc: Chris Goldsworthy 
> Cc: Ørjan Eide 
> Cc: Robin Murphy 
> Cc: Ezequiel Garcia 
> Cc: Simon Ser 
> Cc: James Jones 
> Cc: linux-me...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz 
> ---
> v4:
> * Make sys_uncached_heap static, as
> Reported-by: kernel test robot 
> * Fix wrong return value, caught by smatch
> Reported-by: kernel test robot 
> Reported-by: Dan Carpenter 
> * Ensure we call flush/invalidate_kernel_vmap_range() in the
>   uncached cases to try to address feedback about VIVT caches
>   from Christoph
> * Reorder a few lines as suggested by BrianS
> * Avoid holding the initial mapping for the lifetime of the buffer
>   as suggested by BrianS
> * Fix a unlikely race between allocate and updating the dma_mask
>   that BrianS noticed.
> ---
>  drivers/dma-buf/heaps/system_heap.c | 111 
>  1 file changed, 95 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma-buf/heaps/system_heap.c 
> b/drivers/dma-buf/heaps/system_heap.c
> index ef4b2c1032df..a328c76249d2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -22,6 +22,7 @@
>  #include 
>  
>  static struct dma_heap *sys_heap;
> +static struct dma_heap *sys_uncached_heap;
>  
>  struct system_heap_buffer {
>   struct dma_heap *heap;
> @@ -31,6 +32,8 @@ struct system_heap_buffer {
>   struct sg_table sg_table;
>   int vmap_cnt;
>   void *vaddr;
> +
> + bool uncached;
>  };
>  
>  struct dma_heap_attachment {
> @@ -38,6 +41,8 @@ struct dma_heap_attachment {
>   struct sg_table *table;
>   struct list_head list;
>   bool mapped;
> +
> + bool uncached;
>  };
>  
>  #define HIGH_ORDER_GFP  (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \
> @@ -94,7 +99,7 @@ static int system_heap_attach(struct dma_buf *dmabuf,
>   a->dev = attachment->dev;
>   INIT_LIST_HEAD(>list);
>   a->mapped = false;
> -
> + a->uncached = buffer->uncached;
>   attachment->priv = a;
>  
>   mutex_lock(>lock);
> @@ -124,9 +129,13 @@ static struct sg_table *system_heap_map_dma_buf(struct 
> dma_buf_attachment *attac
>  {
>   struct dma_heap_attachment *a = attachment->priv;
>   struct sg_table *table = a->table;
> + int attr = 0;
>   int ret;
>  
> - ret = dma_map_sgtable(attachment->dev, table, direction, 0);
> + if (a->uncached)
> + attr = DMA_ATTR_SKIP_CPU_SYNC;
> +
> + ret = dma_map_sgtable(attachment->dev, table, direction, attr);
>   if (ret)
>   return ERR_PTR(ret);
>  
> @@ -139,9 +148,12 @@ static void system_heap_unmap_dma_buf(struct 
> dma_buf_attachment *attachment,
> enum dma_data_direction direction)
>  {
>   struct dma_heap_attachment *a = attachment->priv;
> + int attr = 0;
>  
> + if (a->uncached)
> + attr = 

Re: [PATCH 13/14] drm/msm: Drop struct_mutex in shrinker path

2020-10-07 Thread Hillf Danton


On Mon, 5 Oct 2020 20:40:12 Rob Clark  wrote:
> On Mon, Oct 5, 2020 at 5:44 PM Hillf Danton  wrote:
> > On Mon, 5 Oct 2020 18:17:01 Kristian H. Kristensen wrote:
> > > On Mon, Oct 5, 2020 at 4:02 PM Daniel Vetter  wrote:
> > > >
> > > > On Mon, Oct 05, 2020 at 05:24:19PM +0800, Hillf Danton wrote:
> > > > >
> > > > > On Sun,  4 Oct 2020 12:21:45
> > > > > > From: Rob Clark 
> > > > > >
> > > > > > Now that the inactive_list is protected by mm_lock, and everything
> > > > > > else on per-obj basis is protected by obj->lock, we no longer depend
> > > > > > on struct_mutex.
> > > > > >
> > > > > > Signed-off-by: Rob Clark 
> > > > > > ---
> > > > > >  drivers/gpu/drm/msm/msm_gem.c  |  1 -
> > > > > >  drivers/gpu/drm/msm/msm_gem_shrinker.c | 54 
> > > > > > --
> > > > > >  2 files changed, 55 deletions(-)
> > > > > >
> > > > > [...]
> > > > >
> > > > > > @@ -71,13 +33,8 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, 
> > > > > > struct shrink_control *sc)
> > > > > >  {
> > > > > > struct msm_drm_private *priv =
> > > > > > container_of(shrinker, struct msm_drm_private, 
> > > > > > shrinker);
> > > > > > -   struct drm_device *dev = priv->dev;
> > > > > > struct msm_gem_object *msm_obj;
> > > > > > unsigned long freed = 0;
> > > > > > -   bool unlock;
> > > > > > -
> > > > > > -   if (!msm_gem_shrinker_lock(dev, ))
> > > > > > -   return SHRINK_STOP;
> > > > > >
> > > > > > mutex_lock(>mm_lock);
> > > > >
> > > > > Better if the change in behavior is documented that SHRINK_STOP will
> > > > > no longer be needed.
> > > >
> > > > btw I read through this and noticed you have your own obj lock, plus
> > > > mutex_lock_nested. I strongly recommend to just cut over to 
> > > > dma_resv_lock
> > > > for all object lock needs (soc drivers have been terrible with this
> > > > unfortuntaly), and in the shrinker just use dma_resv_trylock instead of
> > > > trying to play clever games outsmarting lockdep.
> >
> > The trylock makes page reclaimers turn to their next target e.g. inode
> > cache instead of waiting for the mutex to be released. It makes sense
> > for instance in scenarios of mild memory pressure.
> 
> is there some behind-the-scenes signalling for this, or is this just
> down to what the shrinker callbacks return?

Lets see what Dave may have in his mind about your questions.

> Generally when we get
> into shrinking, there are a big set of purgable bo's to consider, so
> the shrinker callback return wouldn't be considering just one
> potentially lock contended bo (buffer object).  Ie failing one
> trylock, we just move on to the next.
> 
> fwiw, what I've seen on the userspace bo cache vs shrinker (anything
> that is shrinker potential is in userspace bo cache and
> MADV(WONTNEED)) is that in steady state I see a very strong recycling
> of bo's (which avoids allocating and mmap'ing or mapping to gpu a new
> buffer object), so it is definitely a win in mmap/realloc bandwidth..
> in steady state there is a lot of free and realloc of same-sized
> buffers from frame to frame.
> 
> But in transient situations like moving to new game level when there
> is a heavy memory pressure and lots of freeing old
> buffers/textures/etc and then allocating new ones, I see shrinker
> kicking in hard (in android situations, not so much so with
> traditional linux userspace)
> 
> BR,
> -R
> 
> >
> > > >
> > > > I recently wrote an entire blog length rant on why I think
> > > > mutex_lock_nested is too dangerous to be useful:
> > > >
> > > > https://blog.ffwll.ch/2020/08/lockdep-false-positives.html
> > > >
> > > > Not anything about this here, just general comment. The problem extends 
> > > > to
> > > > shmem helpers and all that also having their own locks for everything.
> > >
> > > This is definitely a tangible improvement though - very happy to see
> > > msm_gem_shrinker_lock() go.
> > >
> > > Reviewed-by: Kristian H. Kristensen 
> > >
> > > > -Daniel
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > http://blog.ffwll.ch
> > > > ___
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 13/14] drm/msm: Drop struct_mutex in shrinker path

2020-10-06 Thread Hillf Danton


On Mon, 5 Oct 2020 18:17:01 Kristian H. Kristensen wrote:
> On Mon, Oct 5, 2020 at 4:02 PM Daniel Vetter  wrote:
> >
> > On Mon, Oct 05, 2020 at 05:24:19PM +0800, Hillf Danton wrote:
> > >
> > > On Sun,  4 Oct 2020 12:21:45
> > > > From: Rob Clark 
> > > >
> > > > Now that the inactive_list is protected by mm_lock, and everything
> > > > else on per-obj basis is protected by obj->lock, we no longer depend
> > > > on struct_mutex.
> > > >
> > > > Signed-off-by: Rob Clark 
> > > > ---
> > > >  drivers/gpu/drm/msm/msm_gem.c  |  1 -
> > > >  drivers/gpu/drm/msm/msm_gem_shrinker.c | 54 --
> > > >  2 files changed, 55 deletions(-)
> > > >
> > > [...]
> > >
> > > > @@ -71,13 +33,8 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, 
> > > > struct shrink_control *sc)
> > > >  {
> > > > struct msm_drm_private *priv =
> > > > container_of(shrinker, struct msm_drm_private, shrinker);
> > > > -   struct drm_device *dev = priv->dev;
> > > > struct msm_gem_object *msm_obj;
> > > > unsigned long freed = 0;
> > > > -   bool unlock;
> > > > -
> > > > -   if (!msm_gem_shrinker_lock(dev, ))
> > > > -   return SHRINK_STOP;
> > > >
> > > > mutex_lock(>mm_lock);
> > >
> > > Better if the change in behavior is documented that SHRINK_STOP will
> > > no longer be needed.
> >
> > btw I read through this and noticed you have your own obj lock, plus
> > mutex_lock_nested. I strongly recommend to just cut over to dma_resv_lock
> > for all object lock needs (soc drivers have been terrible with this
> > unfortuntaly), and in the shrinker just use dma_resv_trylock instead of
> > trying to play clever games outsmarting lockdep.

The trylock makes page reclaimers turn to their next target e.g. inode
cache instead of waiting for the mutex to be released. It makes sense
for instance in scenarios of mild memory pressure.

> >
> > I recently wrote an entire blog length rant on why I think
> > mutex_lock_nested is too dangerous to be useful:
> >
> > https://blog.ffwll.ch/2020/08/lockdep-false-positives.html
> >
> > Not anything about this here, just general comment. The problem extends to
> > shmem helpers and all that also having their own locks for everything.
> 
> This is definitely a tangible improvement though - very happy to see
> msm_gem_shrinker_lock() go.
> 
> Reviewed-by: Kristian H. Kristensen 
> 
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 13/14] drm/msm: Drop struct_mutex in shrinker path

2020-10-06 Thread Hillf Danton


On Sun,  4 Oct 2020 12:21:45
> From: Rob Clark 
> 
> Now that the inactive_list is protected by mm_lock, and everything
> else on per-obj basis is protected by obj->lock, we no longer depend
> on struct_mutex.
> 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/msm/msm_gem.c  |  1 -
>  drivers/gpu/drm/msm/msm_gem_shrinker.c | 54 --
>  2 files changed, 55 deletions(-)
> 
[...]

> @@ -71,13 +33,8 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct 
> shrink_control *sc)
>  {
>   struct msm_drm_private *priv =
>   container_of(shrinker, struct msm_drm_private, shrinker);
> - struct drm_device *dev = priv->dev;
>   struct msm_gem_object *msm_obj;
>   unsigned long freed = 0;
> - bool unlock;
> -
> - if (!msm_gem_shrinker_lock(dev, ))
> - return SHRINK_STOP;
>  
>   mutex_lock(>mm_lock);

Better if the change in behavior is documented that SHRINK_STOP will
no longer be needed.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/3] drm/atomic: Use kthread worker for nonblocking commits

2020-09-21 Thread Hillf Danton


Sat, 19 Sep 2020 12:37:25 -0700 Rob Clark wrote:
>  
> @@ -1797,6 +1797,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>struct drm_atomic_state *state,
>bool nonblock)
>  {
> + struct kthread_worker *worker = NULL;
>   int ret;
>  
>   if (state->async_update) {
> @@ -1814,7 +1815,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>   if (ret)
>   return ret;
>  
> - INIT_WORK(>commit_work, commit_work);
> + kthread_init_work(>commit_kwork, commit_work);
>  
>   ret = drm_atomic_helper_prepare_planes(dev, state);
>   if (ret)
> @@ -1857,8 +1858,12 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>*/
>  
>   drm_atomic_state_get(state);
> +
>   if (nonblock)
> - queue_work(system_unbound_wq, >commit_work);
> + worker = drm_atomic_pick_worker(state);
> +
> + if (worker)
> + kthread_queue_work(worker, >commit_kwork);
>   else
>   commit_tail(state);

A minor change in behavior: commit_tail() would have nothing to do with
either worker or work, rather than the fallback in case no worker is
available.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 3/3] drm: Add a client-cap to set scheduling mode

2020-09-21 Thread Hillf Danton


On Sat, 19 Sep 2020 12:37:26 -0700 Rob Clark wrote:
> +/**
> + * drm_crtc_set_sched_mode:
> + * @dev: DRM device
> + * @mode: one of DRM_CLIENT_CAP_SCHED_x
> + *
> + * Set the scheduling mode for per-CRTC kthread workers.  This controls
> + * whether nonblocking atomic commits will run with SCHED_NORMAL or
> + * SCHED_FIFO (rt) priority.
> + */
> +void drm_crtc_set_sched_mode(struct drm_device *dev, int mode)
> +{
> + struct drm_crtc *crtc;
> +
> + drm_for_each_crtc(crtc, dev) {
> + switch (mode) {
> + case DRM_CLIENT_CAP_SCHED_NORMAL:
> + /* zero is default nice value for kthreads: */
> + sched_set_normal(crtc->worker->task, 0);
> + break;
> + case DRM_CLIENT_CAP_SCHED_FIFO:
> + sched_set_fifo(crtc->worker->task);
> + break;

Better if they are done in kernel/kthread.c wrt FIFO worker.

Off-topic: if that's fine, what's preventing you from doing the same
in kernel/workqueue.c for the workqueus you create manually?

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: KASAN: use-after-free Read in vkms_dumb_create

2020-04-28 Thread Hillf Danton


Sun, 26 Apr 2020 20:48:12 -0700
> syzbot found the following crash on:
> 
> HEAD commit:c578ddb3 Merge tag 'linux-kselftest-5.7-rc3' of git://git...
> git tree:   upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=10fbf0d810
> kernel config:  https://syzkaller.appspot.com/x/.config?x=b7a70e992f2f9b68
> dashboard link: https://syzkaller.appspot.com/bug?extid=e3372a2afe1e7ef04bc7
> compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=1543833010
> 
> Bisection is inconclusive: the first bad commit could be any of:
> 
> 85b5bafb drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()
> dff1c703 drm/tinydrm: Use drm_fbdev_generic_setup()
> 23167fa9 drm/panel: simple: Add support for Rocktech RK070ER9427 LCD panel
> 9060d7f4 drm/fb-helper: Finish the generic fbdev emulation
> 2230ca12 dt-bindings: display: Document the EDT et* displays in one file.
> e896c132 drm/debugfs: Add internal client debugfs file
> 894a677f drm/cma-helper: Use the generic fbdev emulation
> aa7e6455 drm/panel: Add support for the EDT ETM0700G0BDH6
> 244007ec drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap
> aad34de2 drm/panel: Add support for the EDT ETM0700G0EDH6
> 7a6aca49 dt-bindings: Add vendor prefix for DLC Display Co., Ltd.
> d536540f drm/fb-helper: Add generic fbdev emulation .fb_probe function
> 0ca0c827 drm/panel: simple: Add DLC DLC0700YZG-1 panel
> c76f0f7c drm: Begin an API for in-kernel clients
> 5ba57bab drm: vkms: select DRM_KMS_HELPER
> 5fa8e4a2 drm/panel: Make of_drm_find_panel() return an ERR_PTR() instead of 
> NULL
> 008095e0 drm/vc4: Add support for the transposer block
> c59eb3cf drm/panel: Let of_drm_find_panel() return -ENODEV when the panel is 
> disabled
> 1ebe99a7 drm/vc4: Call drm_atomic_helper_fake_vblank() in the commit path
> 2e64a174 drm/of: Make drm_of_find_panel_or_bridge() fail when the device is 
> disabled
> 1b9883ea drm/vc4: Support the case where the DSI device is disabled
> 6fb42b66 drm/atomic: Call fake_vblank() from the generic commit_tail() helpers
> b0b7aa40 dt-bindings: display: Add DT bindings for BOE HV070WSA-100 panel
> b25c60af drm/crtc: Add a generic infrastructure to fake VBLANK events
> 184d3cf4 drm/vc4: Use wait_for_flip_done() instead of wait_for_vblanks()
> ae8cf41b drm/panel: simple: Add support for BOE HV070WSA-100 panel to 
> simple-panel
> 814bde99 drm/connector: Make ->atomic_commit() optional
> 955f60db drm: Add support for extracting sync signal drive edge from videomode
> 3b39ad7a drm/panel: simple: Add newhaven, nhd-4.3-480272ef-atxl LCD
> 425132fd drm/connector: Pass a drm_connector_state to ->atomic_commit()
> a5d2ade6 drm/panel: simple: Add support for Innolux G070Y2-L01
> b82c1f8f drm/atomic: Avoid connector to writeback_connector casts
> 03fa9aa3 dt-bindings: Add DataImage, Inc. vendor prefix
> 73915b2b drm/writeback: Fix the "overview" section of the doc
> 97ceb1fb drm/panel: simple: Add support for DataImage SCF0700C48GGU18
> e22e9531 Merge drm-upstream/drm-next into drm-misc-next
> 3d5664f9 drm/panel: ili9881c: Fix missing assignment to error return ret
> a0120245 drm/crc: Only report a single overflow when a CRC fd is opened
> 7ad4e463 drm/panel: p079zca: Refactor panel driver to support multiple panels
> 8adbbb2e drm/stm: ltdc: rework reset sequence
> 48bd379a drm/panel: p079zca: Add variable unprepare_delay properties
> 7868e507 drm/stm: ltdc: filter mode pixel clock vs pad constraint
> 731edd4c dt-bindings: Add Innolux P097PFG panel bindings
> f8878bb2 drm: print plane state normalized zpos value
> ca52bea9 drm/atomic-helper: Use bitwise or for filling a bitmask
> de04a462 drm/panel: p079zca: Support Innolux P097PFG panel
> 2bb7a39c dt-bindings: Add vendor prefix for kingdisplay
> a65020d0 drm/v3d: Fix a grammar nit in the scheduler docs.
> 2dd4f211 drm/v3d: Add missing v3d documentation structure.
> ebc950fd dt-bindings: Add KINGDISPLAY KD097D04 panel bindings
> cd0e0ca6 drm/panel: type promotion bug in s6e8aa0_read_mtp_id()
> e0d01811 drm/v3d: Remove unnecessary dma_fence_ops.
> 624bb0c0 drm/v3d: Delay the scheduler timeout if we're still making progress.
> b6d83fcc drm/panel: p079zca: Use of_device_get_match_data()
> 408633d2 drm/v3d: use new return type vm_fault_t in v3d_gem_fault
> decac6b0 dt-bindings: display: sun4i-drm: Add R40 display engine compatible
> 0b7510d1 drm/tilcdc: Use drm_connector_has_possible_encoder()
> d978a94b drm/sun4i: Add R40 display engine compatible
> af11942e drm/sun4i: tcon-top: Cleanup clock handling
> f8222409 drm/msm: Use drm_connector_has_possible_encoder()
> 38cb8d96 drm: Add drm_connector_has_possible_encoder()
> da82107e drm/sun4i: tcon: Release node when traversing of graph
> 7a667775 dt-bindings: display: sun4i-drm: Add R40 TV TCON description
> 7b71ca24 drm/radeon: Use drm_connector_for_each_possible_encoder()
> 4a068c5c drm/sun4i: DW HDMI: Release nodes if error happens during CRTC search
> ddba766d drm/nouveau: Use 

Re: BUG: kernel NULL pointer dereference, address: 0000000000000026 after switching to 5.7 kernel

2020-04-11 Thread Hillf Danton


On Sat, 11 Apr 2020 00:51:48 +0500 Mikhail Gavrilov wrote:
> Hi folks.
> After upgrade kernel to 5.7 I see every boot in kernel log following
> error messages:
> 
> [2.569513] [drm] Found UVD firmware ENC: 1.2 DEC: .43 Family ID: 19
> [2.569538] [drm] PSP loading UVD firmware
> [2.570038] BUG: kernel NULL pointer dereference, address: 0026
> [2.570045] #PF: supervisor read access in kernel mode
> [2.570050] #PF: error_code(0x) - not-present page
> [2.570055] PGD 0 P4D 0
> [2.570060] Oops:  [#1] SMP NOPTI
> [2.570065] CPU: 5 PID: 667 Comm: uvd_enc_1.1 Not tainted
> 5.7.0-0.rc0.git6.1.2.fc33.x86_64 #1
> [2.570072] Hardware name: System manufacturer System Product
> Name/ROG STRIX X570-I GAMING, BIOS 1405 11/19/2019
> [2.570085] RIP: 0010:__kthread_should_park+0x5/0x30
> [2.570090] Code: 00 e9 fe fe ff ff e8 ca 3a 08 00 e9 49 fe ff ff
> 48 89 df e8 dd 38 08 00 84 c0 0f 84 6a ff ff ff e9 a6 fe ff ff 0f 1f
> 44 00 00  47 26 20 74 12 48 8b 87 88 09 00 00 48 8b 00 48 c1 e8 02
> 83 e0
> [2.570103] RSP: 0018:ad8141723e50 EFLAGS: 00010246
> [2.570107] RAX: 7fff RBX: 8a8d1d116ed8 RCX: 
> 
> [2.570112] RDX:  RSI:  RDI: 
> 
> [2.570116] RBP: 8a8d28c11300 R08:  R09: 
> 
> [2.570120] R10:  R11:  R12: 
> 8a8d1d152e40
> [2.570125] R13: 8a8d1d117280 R14: 8a8d1d116ed8 R15: 
> 8a8d1ca68000
> [2.570131] FS:  () GS:8a8d3aa0()
> knlGS:
> [2.570137] CS:  0010 DS:  ES:  CR0: 80050033
> [2.570142] CR2: 0026 CR3: 0007e3dc6000 CR4: 
> 003406e0
> [2.570147] Call Trace:
> [2.570157]  drm_sched_get_cleanup_job+0x42/0x130 [gpu_sched]
> [2.570166]  drm_sched_main+0x6f/0x530 [gpu_sched]
> [2.570173]  ? lockdep_hardirqs_on+0x11e/0x1b0
> [2.570179]  ? drm_sched_get_cleanup_job+0x130/0x130 [gpu_sched]
> [2.570185]  kthread+0x131/0x150
> [2.570189]  ? __kthread_bind_mask+0x60/0x60
> [2.570196]  ret_from_fork+0x27/0x50
> [2.570203] Modules linked in: fjes(-) amdgpu(+) amd_iommu_v2
> gpu_sched ttm drm_kms_helper drm crc32c_intel igb nvme nvme_core dca
> i2c_algo_bit wmi pinctrl_amd br_netfilter bridge stp llc fuse
> [2.570223] CR2: 0026
> [2.570228] ---[ end trace 80c25d326e1e0d7c ]---
> [2.570233] RIP: 0010:__kthread_should_park+0x5/0x30
> [2.570238] Code: 00 e9 fe fe ff ff e8 ca 3a 08 00 e9 49 fe ff ff
> 48 89 df e8 dd 38 08 00 84 c0 0f 84 6a ff ff ff e9 a6 fe ff ff 0f 1f
> 44 00 00  47 26 20 74 12 48 8b 87 88 09 00 00 48 8b 00 48 c1 e8 02
> 83 e0
> [2.570250] RSP: 0018:ad8141723e50 EFLAGS: 00010246
> [2.570255] RAX: 7fff RBX: 8a8d1d116ed8 RCX: 
> 
> [2.570260] RDX:  RSI:  RDI: 
> 
> [2.570265] RBP: 8a8d28c11300 R08:  R09: 
> 
> [2.570271] R10:  R11:  R12: 
> 8a8d1d152e40
> [2.570276] R13: 8a8d1d117280 R14: 8a8d1d116ed8 R15: 
> 8a8d1ca68000
> [2.570281] FS:  () GS:8a8d3aa0()
> knlGS:
> [2.570287] CS:  0010 DS:  ES:  CR0: 80050033
> [2.570292] CR2: 0026 CR3: 0007e3dc6000 CR4: 
> 003406e0
> [2.570299] BUG: sleeping function called from invalid context at
> include/linux/percpu-rwsem.h:49
> [2.570306] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid:
> 667, name: uvd_enc_1.1
> [2.570311] INFO: lockdep is turned off.
> [2.570315] irq event stamp: 14
> [2.570319] hardirqs last  enabled at (13): []
> _raw_spin_unlock_irqrestore+0x46/0x60
> [2.570330] hardirqs last disabled at (14): []
> trace_hardirqs_off_thunk+0x1a/0x1c
> [2.570338] softirqs last  enabled at (0): []
> copy_process+0x706/0x1bc0
> [2.570345] softirqs last disabled at (0): [<>] 0x0
> [2.570351] CPU: 5 PID: 667 Comm: uvd_enc_1.1 Tainted: G  D
>   5.7.0-0.rc0.git6.1.2.fc33.x86_64 #1
> [2.570358] Hardware name: System manufacturer System Product
> Name/ROG STRIX X570-I GAMING, BIOS 1405 11/19/2019
> [2.570365] Call Trace:
> [2.570373]  dump_stack+0x8b/0xc8
> [2.570380]  ___might_sleep.cold+0xb6/0xc6
> [2.570385]  exit_signals+0x1c/0x2d0
> [2.570390]  do_exit+0xb1/0xc30
> [2.570395]  ? kthread+0x131/0x150
> [2.570400]  rewind_stack_do_exit+0x17/0x20
> [2.570559] [drm] Found VCE firmware Version: 57.6 Binary ID: 4
> [2.570572] [drm] PSP loading VCE firmware
> [3.146462] [drm] reserve 0x40 from 0x83fe80 for PSP TMR
> 
> $ /usr/src/kernels/`uname -r`/scripts/faddr2line
> /lib/debug/lib/modules/`uname -r`/vmlinux __kthread_should_park+0x5
> __kthread_should_park+0x5/0x30:
> 

Re: [PATCH 02/28] staging: android: ion: use vmap instead of vm_map_ram

2020-04-09 Thread Hillf Danton


On Wed,  8 Apr 2020 13:59:00 +0200
> 
> vm_map_ram can keep mappings around after the vm_unmap_ram.  Using that
> with non-PAGE_KERNEL mappings can lead to all kinds of aliasing issues.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/staging/android/ion/ion_heap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion_heap.c 
> b/drivers/staging/android/ion/ion_heap.c
> index 473b465724f1..a2d5c6df4b96 100644
> --- a/drivers/staging/android/ion/ion_heap.c
> +++ b/drivers/staging/android/ion/ion_heap.c
> @@ -99,12 +99,12 @@ int ion_heap_map_user(struct ion_heap *heap, struct 
> ion_buffer *buffer,
>  
>  static int ion_heap_clear_pages(struct page **pages, int num, pgprot_t 
> pgprot)
>  {
> - void *addr = vm_map_ram(pages, num, -1, pgprot);
> + void *addr = vmap(pages, num, VM_MAP);

A merge glitch?

void *vmap(struct page **pages, unsigned int count,
   unsigned long flags, pgprot_t prot)
>  
>   if (!addr)
>   return -ENOMEM;
>   memset(addr, 0, PAGE_SIZE * num);
> - vm_unmap_ram(addr, num);
> + vunmap(addr);
>  
>   return 0;
>  }
> -- 
> 2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: KASAN: use-after-free Read in dmabuffs_dname

2020-03-07 Thread Hillf Danton


On Fri, 06 Mar 2020 21:41:13 -0800
> syzbot found the following crash on:
> 
> HEAD commit:63623fd4 Merge tag 'for-linus' of git://git.kernel.org/pub..
> git tree:   upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=11653ac3e0
> kernel config:  https://syzkaller.appspot.com/x/.config?x=9833e26bab355358
> dashboard link: https://syzkaller.appspot.com/bug?extid=3643a18836bce555bff6
> compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> userspace arch: i386
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+3643a18836bce555b...@syzkaller.appspotmail.com
> 
> ==
> BUG: KASAN: use-after-free in dmabuffs_dname+0x4f4/0x560 
> drivers/dma-buf/dma-buf.c:48
> Read of size 8 at addr 8880a6b390e8 by task syz-executor.1/2394
> 
> CPU: 1 PID: 2394 Comm: syz-executor.1 Not tainted 5.6.0-rc3-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS 
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x197/0x210 lib/dump_stack.c:118
>  print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374
>  __kasan_report.cold+0x1b/0x32 mm/kasan/report.c:506
>  kasan_report+0x12/0x20 mm/kasan/common.c:641
>  __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135
>  dmabuffs_dname+0x4f4/0x560 drivers/dma-buf/dma-buf.c:48
>  tomoyo_realpath_from_path+0x165/0x660 security/tomoyo/realpath.c:259
>  tomoyo_get_realpath security/tomoyo/file.c:151 [inline]
>  tomoyo_check_open_permission+0x2a3/0x3e0 security/tomoyo/file.c:771
>  tomoyo_file_open security/tomoyo/tomoyo.c:319 [inline]
>  tomoyo_file_open+0xa9/0xd0 security/tomoyo/tomoyo.c:314
>  security_file_open+0x71/0x300 security/security.c:1529
>  do_dentry_open+0x37a/0x1380 fs/open.c:784
>  vfs_open+0xa0/0xd0 fs/open.c:914
>  do_last fs/namei.c:3490 [inline]
>  path_openat+0x12ee/0x3490 fs/namei.c:3607
>  do_filp_open+0x192/0x260 fs/namei.c:3637
>  do_sys_openat2+0x5eb/0x7e0 fs/open.c:1149
>  do_sys_open+0xf2/0x180 fs/open.c:1165
>  __do_compat_sys_open fs/open.c:1212 [inline]
>  __se_compat_sys_open fs/open.c:1210 [inline]
>  __ia32_compat_sys_open+0x79/0xb0 fs/open.c:1210
>  do_syscall_32_irqs_on arch/x86/entry/common.c:337 [inline]
>  do_fast_syscall_32+0x27b/0xe16 arch/x86/entry/common.c:408
>  entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
> RIP: 0023:0xf7fd8e39
> Code: 1d 00 00 00 89 d3 5b 5e 5d c3 8b 04 24 c3 8b 1c 24 c3 8b 3c 24 c3 90 90 
> 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 
> 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
> RSP: 002b:f5db2014 EFLAGS: 0296 ORIG_RAX: 0005
> RAX: ffda RBX: f5db204c RCX: 
> RDX:  RSI: 0c09 RDI: f5db204c
> RBP: f5db2168 R08:  R09: 
> R10:  R11:  R12: 
> R13:  R14:  R15: 
> 
> Allocated by task 2388:
>  save_stack+0x23/0x90 mm/kasan/common.c:72
>  set_track mm/kasan/common.c:80 [inline]
>  __kasan_kmalloc mm/kasan/common.c:515 [inline]
>  __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:488
>  kasan_kmalloc+0x9/0x10 mm/kasan/common.c:529
>  __do_kmalloc mm/slab.c:3656 [inline]
>  __kmalloc+0x163/0x770 mm/slab.c:3665
>  kmalloc include/linux/slab.h:560 [inline]
>  kzalloc include/linux/slab.h:669 [inline]
>  dma_buf_export+0x24d/0xa80 drivers/dma-buf/dma-buf.c:533
>  ion_alloc drivers/staging/android/ion/ion.c:386 [inline]
>  ion_ioctl+0x5a9/0xd20 drivers/staging/android/ion/ion.c:495
>  compat_ptr_ioctl+0x6e/0xa0 fs/ioctl.c:804
>  __do_compat_sys_ioctl fs/ioctl.c:857 [inline]
>  __se_compat_sys_ioctl fs/ioctl.c:808 [inline]
>  __ia32_compat_sys_ioctl+0x245/0x2c0 fs/ioctl.c:808
>  do_syscall_32_irqs_on arch/x86/entry/common.c:337 [inline]
>  do_fast_syscall_32+0x27b/0xe16 arch/x86/entry/common.c:408
>  entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
> 
> Freed by task 2380:
>  save_stack+0x23/0x90 mm/kasan/common.c:72
>  set_track mm/kasan/common.c:80 [inline]
>  kasan_set_free_info mm/kasan/common.c:337 [inline]
>  __kasan_slab_free+0x102/0x150 mm/kasan/common.c:476
>  kasan_slab_free+0xe/0x10 mm/kasan/common.c:485
>  __cache_free mm/slab.c:3426 [inline]
>  kfree+0x10a/0x2c0 mm/slab.c:3757
>  dma_buf_release+0x343/0x420 drivers/dma-buf/dma-buf.c:111
>  __fput+0x2ff/0x890 fs/file_table.c:280
>  fput+0x16/0x20 fs/file_table.c:313
>  task_work_run+0x145/0x1c0 kernel/task_work.c:113
>  tracehook_notify_resume include/linux/tracehook.h:188 [inline]
>  exit_to_usermode_loop+0x316/0x380 arch/x86/entry/common.c:164
>  prepare_exit_to_usermode arch/x86/entry/common.c:195 [inline]
>  syscall_return_slowpath 

Re: [PATCH v4 5/9] drm/ttm, drm/vmwgfx: Support huge TTM pagefaults

2020-03-02 Thread Hillf Danton


On Thu, 20 Feb 2020 13:27:15 +0100 Thomas Hellstrom wrote:
> +
> +static vm_fault_t ttm_bo_vm_huge_fault(struct vm_fault *vmf,
> +enum page_entry_size pe_size)
> +{
> + struct vm_area_struct *vma = vmf->vma;
> + pgprot_t prot;
> + struct ttm_buffer_object *bo = vma->vm_private_data;
> + vm_fault_t ret;
> + pgoff_t fault_page_size = 0;
> + bool write = vmf->flags & FAULT_FLAG_WRITE;
> +
> + switch (pe_size) {
> + case PE_SIZE_PMD:
> + fault_page_size = HPAGE_PMD_SIZE >> PAGE_SHIFT;
> + break;
> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> + case PE_SIZE_PUD:
> + fault_page_size = HPAGE_PUD_SIZE >> PAGE_SHIFT;
> + break;
> +#endif
> + default:
> + WARN_ON_ONCE(1);
> + return VM_FAULT_FALLBACK;
> + }
> +
> + /* Fallback on write dirty-tracking or COW */
> + if (write && ttm_pgprot_is_wrprotecting(vma->vm_page_prot))
> + return VM_FAULT_FALLBACK;
> +
> + ret = ttm_bo_vm_reserve(bo, vmf);
> + if (ret)
> + return ret;
> +
> + prot = vm_get_page_prot(vma->vm_flags);
> + ret = ttm_bo_vm_fault_reserved(vmf, prot, 1, fault_page_size);
> + if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
> + return ret;

Seems it does not make much sense to check VM_FAULT_RETRY and return as
at least resv lock is left behind without care.

> +
> + dma_resv_unlock(bo->base.resv);
> +
> + return ret;
> +}

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: KASAN: use-after-free Read in vgem_gem_dumb_create

2020-02-03 Thread Hillf Danton


On Sat, 1 Feb 2020 09:17:57 +0300 Dan Carpenter wrote:
> On Sat, Feb 01, 2020 at 12:32:09PM +0800, Hillf Danton wrote:
> >
> > Release obj in error path.
> > 
> > --- a/drivers/gpu/drm/vgem/vgem_drv.c
> > +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> > @@ -196,10 +196,10 @@ static struct drm_gem_object *vgem_gem_c
> > return ERR_CAST(obj);
> >  
> > ret = drm_gem_handle_create(file, >base, handle);
> > -   drm_gem_object_put_unlocked(>base);
> > -   if (ret)
> > +   if (ret) {
> > +   drm_gem_object_put_unlocked(>base);
> > return ERR_PTR(ret);
> > -
> > +   }
> > return >base;
> 
> Oh yeah.  It's weird that we never noticed the success path was broken.
> It's been that way for three years and no one noticed at all.  Very
> strange.
> 
> Anyway, it already gets freed on error in drm_gem_handle_create() so
> we should just delete the drm_gem_object_put_unlocked() here it looks
> like.

Good catch, Dan :P
Would you please post a patch sometime convenient next week?

Thanks
Hillf

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: KASAN: use-after-free Read in vgem_gem_dumb_create

2020-02-03 Thread Hillf Danton


Fri, 31 Jan 2020 14:28:10 -0800 (PST)
> syzbot found the following crash on:
> 
> HEAD commit:39bed42d Merge tag 'for-linus-hmm' of git://git.kernel.org..
> git tree:   upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=179465bee0
> kernel config:  https://syzkaller.appspot.com/x/.config?x=2646535f8818ae25
> dashboard link: https://syzkaller.appspot.com/bug?extid=0dc774d419e916c8
> compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=16251279e0
> 
> The bug was bisected to:
> 
> commit 7611750784664db46d0db95631e322aeb263dde7
> Author: Alex Deucher 
> Date:   Wed Jun 21 16:31:41 2017 +
> 
> drm/amdgpu: use kernel is_power_of_2 rather than local version
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=11628df1e0
> final crash:https://syzkaller.appspot.com/x/report.txt?x=13628df1e0
> console output: https://syzkaller.appspot.com/x/log.txt?x=15628df1e0
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+0dc774d419e91...@syzkaller.appspotmail.com
> Fixes: 761175078466 ("drm/amdgpu: use kernel is_power_of_2 rather than local 
> version")
> 
> ==
> BUG: KASAN: use-after-free in vgem_gem_dumb_create+0x238/0x250 
> drivers/gpu/drm/vgem/vgem_drv.c:221
> Read of size 8 at addr 88809fa67908 by task syz-executor.0/14871
> 
> CPU: 0 PID: 14871 Comm: syz-executor.0 Not tainted 5.5.0-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS 
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x197/0x210 lib/dump_stack.c:118
>  print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374
>  __kasan_report.cold+0x1b/0x32 mm/kasan/report.c:506
>  kasan_report+0x12/0x20 mm/kasan/common.c:639
>  __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135
>  vgem_gem_dumb_create+0x238/0x250 drivers/gpu/drm/vgem/vgem_drv.c:221
>  drm_mode_create_dumb+0x282/0x310 drivers/gpu/drm/drm_dumb_buffers.c:94
>  drm_mode_create_dumb_ioctl+0x26/0x30 drivers/gpu/drm/drm_dumb_buffers.c:100
>  drm_ioctl_kernel+0x244/0x300 drivers/gpu/drm/drm_ioctl.c:786
>  drm_ioctl+0x54e/0xa60 drivers/gpu/drm/drm_ioctl.c:886
>  vfs_ioctl fs/ioctl.c:47 [inline]
>  ksys_ioctl+0x123/0x180 fs/ioctl.c:747
>  __do_sys_ioctl fs/ioctl.c:756 [inline]
>  __se_sys_ioctl fs/ioctl.c:754 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x45b349
> Code: ad b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 
> 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 
> 83 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:7f871af46c78 EFLAGS: 0246 ORIG_RAX: 0010
> RAX: ffda RBX: 7f871af476d4 RCX: 0045b349
> RDX: 2180 RSI: c02064b2 RDI: 0003
> RBP: 0075bf20 R08:  R09: 
> R10:  R11: 0246 R12: 
> R13: 0285 R14: 004d14d0 R15: 0075bf2c
> 
> Allocated by task 14871:
>  save_stack+0x23/0x90 mm/kasan/common.c:72
>  set_track mm/kasan/common.c:80 [inline]
>  __kasan_kmalloc mm/kasan/common.c:513 [inline]
>  __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:486
>  kasan_kmalloc+0x9/0x10 mm/kasan/common.c:527
>  kmem_cache_alloc_trace+0x158/0x790 mm/slab.c:3551
>  kmalloc include/linux/slab.h:556 [inline]
>  kzalloc include/linux/slab.h:670 [inline]
>  __vgem_gem_create+0x49/0x100 drivers/gpu/drm/vgem/vgem_drv.c:165
>  vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:194 [inline]
>  vgem_gem_dumb_create+0xd7/0x250 drivers/gpu/drm/vgem/vgem_drv.c:217
>  drm_mode_create_dumb+0x282/0x310 drivers/gpu/drm/drm_dumb_buffers.c:94
>  drm_mode_create_dumb_ioctl+0x26/0x30 drivers/gpu/drm/drm_dumb_buffers.c:100
>  drm_ioctl_kernel+0x244/0x300 drivers/gpu/drm/drm_ioctl.c:786
>  drm_ioctl+0x54e/0xa60 drivers/gpu/drm/drm_ioctl.c:886
>  vfs_ioctl fs/ioctl.c:47 [inline]
>  ksys_ioctl+0x123/0x180 fs/ioctl.c:747
>  __do_sys_ioctl fs/ioctl.c:756 [inline]
>  __se_sys_ioctl fs/ioctl.c:754 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Freed by task 14871:
>  save_stack+0x23/0x90 mm/kasan/common.c:72
>  set_track mm/kasan/common.c:80 [inline]
>  kasan_set_free_info mm/kasan/common.c:335 [inline]
>  __kasan_slab_free+0x102/0x150 mm/kasan/common.c:474
>  kasan_slab_free+0xe/0x10 mm/kasan/common.c:483
>  __cache_free mm/slab.c:3426 [inline]
>  kfree+0x10a/0x2c0 mm/slab.c:3757
>  vgem_gem_free_object+0xbe/0xe0 drivers/gpu/drm/vgem/vgem_drv.c:68
>  drm_gem_object_free+0x100/0x220 

Re: KASAN: use-after-free Read in fbcon_cursor

2019-12-17 Thread Hillf Danton


On Sun, 15 Dec 2019 12:35:09 -0800
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:07c4b9e9 Merge tag 'scsi-fixes' of git://git.kernel.org/pu..
> git tree:   upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=14b61f41e0
> kernel config:  https://syzkaller.appspot.com/x/.config?x=79f79de2a27d3e3d
> dashboard link: https://syzkaller.appspot.com/bug?extid=9116ecc1978ca3a12f43
> compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=119fa6b6e0
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+9116ecc1978ca3a12...@syzkaller.appspotmail.com
> 
> ==
> BUG: KASAN: use-after-free in fbcon_cursor+0x4ef/0x660  
> drivers/video/fbdev/core/fbcon.c:1380
> Read of size 2 at addr 8880959ff0cc by task syz-executor.0/10203
> 
> CPU: 1 PID: 10203 Comm: syz-executor.0 Not tainted 5.5.0-rc1-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
> Google 01/01/2011
> Call Trace:
>   __dump_stack lib/dump_stack.c:77 [inline]
>   dump_stack+0x197/0x210 lib/dump_stack.c:118
>   print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374
>   __kasan_report.cold+0x1b/0x41 mm/kasan/report.c:506
>   kasan_report+0x12/0x20 mm/kasan/common.c:639
>   __asan_report_load2_noabort+0x14/0x20 mm/kasan/generic_report.c:133
>   fbcon_cursor+0x4ef/0x660 drivers/video/fbdev/core/fbcon.c:1380
>   fbcon_scrolldelta+0x679/0x1220 drivers/video/fbdev/core/fbcon.c:2877
>   fbcon_set_origin+0x43/0x50 drivers/video/fbdev/core/fbcon.c:2928
>   set_origin+0xf3/0x400 drivers/tty/vt/vt.c:919
>   vc_do_resize+0xacc/0x1460 drivers/tty/vt/vt.c:1264
>   vc_resize+0x4d/0x60 drivers/tty/vt/vt.c:1304
>   vt_ioctl+0x14bb/0x26d0 drivers/tty/vt/vt_ioctl.c:840
>   tty_ioctl+0xa37/0x14f0 drivers/tty/tty_io.c:2660
>   vfs_ioctl fs/ioctl.c:47 [inline]
>   file_ioctl fs/ioctl.c:545 [inline]
>   do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
>   ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
>   __do_sys_ioctl fs/ioctl.c:756 [inline]
>   __se_sys_ioctl fs/ioctl.c:754 [inline]
>   __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>   do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>   entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x45a909
> Code: ad b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
> ff 0f 83 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:7f1a84ca0c78 EFLAGS: 0246 ORIG_RAX: 0010
> RAX: ffda RBX: 0003 RCX: 0045a909
> RDX: 2000 RSI: 5609 RDI: 0003
> RBP: 0075bf20 R08:  R09: 
> R10:  R11: 0246 R12: 7f1a84ca16d4
> R13: 004c7009 R14: 004dd670 R15: 
> 
> Allocated by task 9734:
>   save_stack+0x23/0x90 mm/kasan/common.c:72
>   set_track mm/kasan/common.c:80 [inline]
>   __kasan_kmalloc mm/kasan/common.c:513 [inline]
>   __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:486
>   kasan_kmalloc+0x9/0x10 mm/kasan/common.c:527
>   __do_kmalloc mm/slab.c:3656 [inline]
>   __kmalloc+0x163/0x770 mm/slab.c:3665
>   kmalloc include/linux/slab.h:561 [inline]
>   kzalloc include/linux/slab.h:670 [inline]
>   vc_do_resize+0x262/0x1460 drivers/tty/vt/vt.c:1187
>   vc_resize+0x4d/0x60 drivers/tty/vt/vt.c:1304
>   vt_ioctl+0x14bb/0x26d0 drivers/tty/vt/vt_ioctl.c:840
>   tty_ioctl+0xa37/0x14f0 drivers/tty/tty_io.c:2660
>   vfs_ioctl fs/ioctl.c:47 [inline]
>   file_ioctl fs/ioctl.c:545 [inline]
>   do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
>   ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
>   __do_sys_ioctl fs/ioctl.c:756 [inline]
>   __se_sys_ioctl fs/ioctl.c:754 [inline]
>   __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>   do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>   entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Freed by task 10203:
>   save_stack+0x23/0x90 mm/kasan/common.c:72
>   set_track mm/kasan/common.c:80 [inline]
>   kasan_set_free_info mm/kasan/common.c:335 [inline]
>   __kasan_slab_free+0x102/0x150 mm/kasan/common.c:474
>   kasan_slab_free+0xe/0x10 mm/kasan/common.c:483
>   __cache_free mm/slab.c:3426 [inline]
>   kfree+0x10a/0x2c0 mm/slab.c:3757
>   vc_do_resize+0xa69/0x1460 drivers/tty/vt/vt.c:1261
>   vc_resize+0x4d/0x60 drivers/tty/vt/vt.c:1304
>   vt_ioctl+0x14bb/0x26d0 drivers/tty/vt/vt_ioctl.c:840
>   tty_ioctl+0xa37/0x14f0 drivers/tty/tty_io.c:2660
>   vfs_ioctl fs/ioctl.c:47 [inline]
>   file_ioctl fs/ioctl.c:545 [inline]
>   do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
>   ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
>   __do_sys_ioctl fs/ioctl.c:756 [inline]
>   __se_sys_ioctl fs/ioctl.c:754 [inline]
>   __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>   do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>   

Re: KASAN: use-after-free Read in fb_mode_is_equal

2019-12-09 Thread Hillf Danton

On Sat, 07 Dec 2019 02:05:08 -0800
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
> git tree:   upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=16997c82e0
> kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
> dashboard link: https://syzkaller.appspot.com/bug?extid=f11cda116c57db68c227
> compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+f11cda116c57db68c...@syzkaller.appspotmail.com
> 
> ==
> BUG: KASAN: use-after-free in fb_mode_is_equal+0x297/0x300  
> drivers/video/fbdev/core/modedb.c:924
> Read of size 4 at addr 8880992d5d9c by task syz-executor.0/32283
> 
> CPU: 0 PID: 32283 Comm: syz-executor.0 Not tainted 5.4.0-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
> Google 01/01/2011
> Call Trace:
>   __dump_stack lib/dump_stack.c:77 [inline]
>   dump_stack+0x197/0x210 lib/dump_stack.c:118
>   print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374
>   __kasan_report.cold+0x1b/0x41 mm/kasan/report.c:506
>   kasan_report+0x12/0x20 mm/kasan/common.c:639
>   __asan_report_load4_noabort+0x14/0x20 mm/kasan/generic_report.c:134
>   fb_mode_is_equal+0x297/0x300 drivers/video/fbdev/core/modedb.c:924
>   fbcon_mode_deleted+0x12c/0x190 drivers/video/fbdev/core/fbcon.c:3060
>   fb_set_var+0xab9/0xdd0 drivers/video/fbdev/core/fbmem.c:971
>   do_fb_ioctl+0x390/0x7d0 drivers/video/fbdev/core/fbmem.c:1104
>   fb_ioctl+0xe6/0x130 drivers/video/fbdev/core/fbmem.c:1180
>   vfs_ioctl fs/ioctl.c:47 [inline]
>   file_ioctl fs/ioctl.c:545 [inline]
>   do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
>   ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
>   __do_sys_ioctl fs/ioctl.c:756 [inline]
>   __se_sys_ioctl fs/ioctl.c:754 [inline]
>   __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>   do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>   entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x45a6f9
> Code: ad b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
> ff 0f 83 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:7f7aefd54c78 EFLAGS: 0246 ORIG_RAX: 0010
> RAX: ffda RBX: 0003 RCX: 0045a6f9
> RDX: 2000 RSI: 4601 RDI: 0003
> RBP: 0075bf20 R08:  R09: 
> R10:  R11: 0246 R12: 7f7aefd556d4
> R13: 004c2ef7 R14: 004d8138 R15: 
> 
> Allocated by task 9205:
>   save_stack+0x23/0x90 mm/kasan/common.c:72
>   set_track mm/kasan/common.c:80 [inline]
>   __kasan_kmalloc mm/kasan/common.c:513 [inline]
>   __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:486
>   kasan_kmalloc+0x9/0x10 mm/kasan/common.c:527
>   kmem_cache_alloc_trace+0x158/0x790 mm/slab.c:3551
>   kmalloc include/linux/slab.h:556 [inline]
>   fb_add_videomode drivers/video/fbdev/core/modedb.c:1073 [inline]
>   fb_add_videomode+0x2fb/0x610 drivers/video/fbdev/core/modedb.c:1057
>   fb_set_var+0x5ef/0xdd0 drivers/video/fbdev/core/fbmem.c:1041
>   do_fb_ioctl+0x390/0x7d0 drivers/video/fbdev/core/fbmem.c:1104
>   fb_ioctl+0xe6/0x130 drivers/video/fbdev/core/fbmem.c:1180
>   vfs_ioctl fs/ioctl.c:47 [inline]
>   file_ioctl fs/ioctl.c:545 [inline]
>   do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
>   ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
>   __do_sys_ioctl fs/ioctl.c:756 [inline]
>   __se_sys_ioctl fs/ioctl.c:754 [inline]
>   __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>   do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>   entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Freed by task 32276:
>   save_stack+0x23/0x90 mm/kasan/common.c:72
>   set_track mm/kasan/common.c:80 [inline]
>   kasan_set_free_info mm/kasan/common.c:335 [inline]
>   __kasan_slab_free+0x102/0x150 mm/kasan/common.c:474
>   kasan_slab_free+0xe/0x10 mm/kasan/common.c:483
>   __cache_free mm/slab.c:3426 [inline]
>   kfree+0x10a/0x2c0 mm/slab.c:3757
>   fb_delete_videomode+0x3fa/0x540 drivers/video/fbdev/core/modedb.c:1104
>   fb_set_var+0xac8/0xdd0 drivers/video/fbdev/core/fbmem.c:974
>   do_fb_ioctl+0x390/0x7d0 drivers/video/fbdev/core/fbmem.c:1104
>   fb_ioctl+0xe6/0x130 drivers/video/fbdev/core/fbmem.c:1180
>   vfs_ioctl fs/ioctl.c:47 [inline]
>   file_ioctl fs/ioctl.c:545 [inline]
>   do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
>   ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
>   __do_sys_ioctl fs/ioctl.c:756 [inline]
>   __se_sys_ioctl fs/ioctl.c:754 [inline]
>   __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>   do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>   entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> The 

Re: [Spice-devel] Xorg indefinitely hangs in kernelspace

2019-10-04 Thread Hillf Danton

On Thu, 3 Oct 2019 09:45:55 +0300 Jaak Ristioja wrote:
> On 30.09.19 16:29, Frediano Ziglio wrote:
> >   Why didn't you update bug at 
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1813620?
> > I know it can seem tedious but would help tracking it.
> 
> I suppose the lack on centralized tracking and handling of Linux kernel
> bugs is a delicate topic, so I don't want to rant much more on that.
> Updating that bug would tedious and time-consuming indeed, which is why
> I haven't done that. To be honest, I don't have enough time and motivation.

Give the diff below a go only when it is convenient and only if it makes
a bit of sense to you.

--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -110,6 +110,7 @@ int ttm_eu_reserve_buffers(struct ww_acq
ww_acquire_init(ticket, _ww_class);
 
list_for_each_entry(entry, list, head) {
+   bool lockon = false;
struct ttm_buffer_object *bo = entry->bo;
 
ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
@@ -150,6 +151,7 @@ int ttm_eu_reserve_buffers(struct ww_acq
dma_resv_lock_slow(bo->base.resv, ticket);
ret = 0;
}
+   lockon = !ret;
}
 
if (!ret && entry->num_shared)
@@ -157,6 +159,8 @@ int ttm_eu_reserve_buffers(struct ww_acq

entry->num_shared);
 
if (unlikely(ret != 0)) {
+   if (lockon)
+   dma_resv_unlock(bo->base.resv);
if (ret == -EINTR)
ret = -ERESTARTSYS;
if (ticket) {

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm_sched with panfrost crash on T820

2019-10-01 Thread Hillf Danton

On Mon, 30 Sep 2019 11:17:45 +0200 Neil Armstrong wrote:
> 
> Did a new run from 5.3:
> 
> [   35.971972] Call trace:
> [   35.974391]  drm_sched_increase_karma+0x5c/0xf0
>   10667f3810667F94
>   drivers/gpu/drm/scheduler/sched_main.c:335
> 
> The crashing line is :
> if (bad->s_fence->scheduled.context ==
> entity->fence_context) {
> 
> Doesn't seem related to guilty job.

Bail out if s_fence is no longer fresh.

--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -333,6 +333,10 @@ void drm_sched_increase_karma(struct drm
 
spin_lock(>lock);
list_for_each_entry_safe(entity, tmp, >entities, 
list) {
+   if (!smp_load_acquire(>s_fence)) {
+   spin_unlock(>lock);
+   return;
+   }
if (bad->s_fence->scheduled.context ==
entity->fence_context) {
if (atomic_read(>karma) >
@@ -543,7 +547,7 @@ EXPORT_SYMBOL(drm_sched_job_init);
 void drm_sched_job_cleanup(struct drm_sched_job *job)
 {
dma_fence_put(>s_fence->finished);
-   job->s_fence = NULL;
+   smp_store_release(>s_fence, 0);
 }
 EXPORT_SYMBOL(drm_sched_job_cleanup);
 
--

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RESEND][PATCH v8 3/5] dma-buf: heaps: Add system heap to dmabuf heaps

2019-09-30 Thread Hillf Danton

On Fri,  6 Sep 2019 18:47:09 + John Stultz wrote:
> 
> +static int system_heap_allocate(struct dma_heap *heap,
> + unsigned long len,
> + unsigned long fd_flags,
> + unsigned long heap_flags)
> +{
> + struct heap_helper_buffer *helper_buffer;
> + struct dma_buf *dmabuf;
> + int ret = -ENOMEM;
> + pgoff_t pg;
> +
> + helper_buffer = kzalloc(sizeof(*helper_buffer), GFP_KERNEL);
> + if (!helper_buffer)
> + return -ENOMEM;
> +
> + init_heap_helper_buffer(helper_buffer, system_heap_free);
> + helper_buffer->flags = heap_flags;
> + helper_buffer->heap = heap;
> + helper_buffer->size = len;
> +
A couple of lines looks needed to handle len if it is not
PAGE_SIZE aligned.

> + helper_buffer->pagecount = len / PAGE_SIZE;
> + helper_buffer->pages = kmalloc_array(helper_buffer->pagecount,
> +  sizeof(*helper_buffer->pages),
> +  GFP_KERNEL);

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RESEND][PATCH v8 4/5] dma-buf: heaps: Add CMA heap to dmabuf heaps

2019-09-30 Thread Hillf Danton

On Fri,  6 Sep 2019 18:47:09 + John Stultz wrote:
> 
> + cma_pages = cma_alloc(cma_heap->cma, nr_pages, align, false);
> + if (!cma_pages)
> + goto free_buf;
> +
> + if (PageHighMem(cma_pages)) {
> + unsigned long nr_clear_pages = nr_pages;
> + struct page *page = cma_pages;
> +
> + while (nr_clear_pages > 0) {
> + void *vaddr = kmap_atomic(page);
> +
> + memset(vaddr, 0, PAGE_SIZE);
> + kunmap_atomic(vaddr);
> + page++;
> + nr_clear_pages--;
> + }
> + } else {
> + memset(page_address(cma_pages), 0, size);
> + }

Take a breath after zeroing a page, and a peep at pending signal.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RESEND][PATCH v8 1/5] dma-buf: Add dma-buf heaps framework

2019-09-30 Thread Hillf Danton

On Fri,  6 Sep 2019 18:47:08 + John Stultz wrote:
> +/**
> + * dma_heap_get_data() - get per-heap driver data
> + * @heap: DMA-Heap to retrieve private data for
> + *
> + * Returns:
> + * The per-heap data for the heap.
> + */
> +void *dma_heap_get_data(struct dma_heap *heap);
> +

It will help readers more than thought understand this framework
if s/get_data/get_drvdata/

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 0/7] Emulated coherent graphics memory take 2

2019-09-16 Thread Hillf Danton

On Fri, 13 Sep 2019 11:32:06 +0200
> 
> The mm patch page walk interface has been reworked to be similar to the
> reworked page-walk code (mm/pagewalk.c). There have been two other solutions
> to consider:
> 1) Using the page-walk code. That is currently not possible since it requires
> the mmap-sem to be held for the struct vm_area_struct vm_flags and for huge
> page splitting. The pagewalk code in this patchset can't hold the mmap sems
> since it will lead to locking inversion.

Specify the locking scenario, if non-rfc is planned, to help understand
the new wheel this patchset looks to create, as two days of finding it in
the works after ba4e7d973dd0 failed.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 3/7] drm/ttm: TTM fault handler helpers

2019-09-16 Thread Hillf Danton

On Fri, 13 Sep 2019 11:32:09 +0200
> 
>   err = ttm_mem_io_lock(man, true);
> - if (unlikely(err != 0)) {
> - ret = VM_FAULT_NOPAGE;
> - goto out_unlock;
> - }
> + if (unlikely(err != 0))
> + return VM_FAULT_NOPAGE;
>   err = ttm_mem_io_reserve_vm(bo);
> - if (unlikely(err != 0)) {
> - ret = VM_FAULT_SIGBUS;
> - goto out_io_unlock;
> - }
> + if (unlikely(err != 0))
> + return VM_FAULT_SIGBUS;
> 
Hehe, no hurry.

> @@ -295,8 +307,28 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>   ret = VM_FAULT_NOPAGE;
>  out_io_unlock:
>   ttm_mem_io_unlock(man);
> -out_unlock:
> + return ret;
> +}
> +EXPORT_SYMBOL(ttm_bo_vm_fault_reserved);

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Xorg indefinitely hangs in kernelspace

2019-09-10 Thread Hillf Danton

On Mon, 9 Sep 2019 from Gerd Hoffmann 
>
> Hmm, I think the patch is wrong.

Hmm...it should have added change only in the error path, leaving locks
for drivers to release if job is done with no error returned.

> As far I know it is the qxl drivers's
> job to call ttm_eu_backoff_reservation().

Like other drivers, qxl is currently doing the right.

> Doing that automatically in
> ttm will most likely break other ttm users.
>
You are right. They are responsible for doing backoff if error happens
while validating buffers afterwards.


--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -111,8 +111,10 @@ int ttm_eu_reserve_buffers(struct ww_acq
 
list_for_each_entry(entry, list, head) {
struct ttm_buffer_object *bo = entry->bo;
+   bool lockon;
 
ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
+   lockon = !ret;
if (!ret && unlikely(atomic_read(>cpu_writers) > 0)) {
reservation_object_unlock(bo->resv);
 
@@ -151,6 +153,7 @@ int ttm_eu_reserve_buffers(struct ww_acq
ret = 0;
}
}
+   lockon = !ret;
 
if (!ret && entry->num_shared)
ret = reservation_object_reserve_shared(bo->resv,
@@ -163,6 +166,8 @@ int ttm_eu_reserve_buffers(struct ww_acq
ww_acquire_done(ticket);
ww_acquire_fini(ticket);
}
+   if (lockon)
+   ttm_eu_backoff_reservation_reverse(list, entry);
return ret;
}
 
--

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Xorg indefinitely hangs in kernelspace

2019-09-10 Thread Hillf Danton
Hi,

On Mon, 9 Sep 2019 from Gerd Hoffmann 
>
> Hmm, I think the patch is wrong.  As far I know it is the qxl drivers's
> job to call ttm_eu_backoff_reservation().  Doing that automatically in
> ttm will most likely break other ttm users.
>
Perhaps.

>So I guess the call is missing in the qxl driver somewhere, most likely
>in some error handling code path given that this bug is a relatively
>rare event.
>
>There is only a single ttm_eu_reserve_buffers() call in qxl.
>So how about this?
>
No preference in either way if it is a right cure.

BTW a quick peep at the mainline tree shows not every
ttm_eu_reserve_buffers() pairs with ttm_eu_backoff_reservation()
without qxl being taken in account.

Hillf
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Spice-devel] Xorg indefinitely hangs in kernelspace

2019-09-09 Thread Hillf Danton
>From Frediano Ziglio 
>
> Where does it came this patch?

My fingers tapping the keyboard.

> Is it already somewhere?

No idea yet.

> Is it supposed to fix this issue?

It may do nothing else as far as I can tell.

> Does it affect some other card beside QXL?

Perhaps.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Xorg indefinitely hangs in kernelspace

2019-09-06 Thread Hillf Danton

On Tue, 6 Aug 2019 21:00:10 +0300 From:   Jaak Ristioja 
> Hello!
> 
> I'm writing to report a crash in the QXL / DRM code in the Linux kernel.
> I originally filed the issue on LaunchPad and more details can be found
> there, although I doubt whether these details are useful.
> 
>   https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1813620
> 
> I first experienced these issues with:
> 
> * Ubuntu 18.04 (probably kernel 4.15.something)
> * Ubuntu 18.10 (kernel 4.18.0-13)
> * Ubuntu 19.04 (kernel 5.0.0-13-generic)
> * Ubuntu 19.04 (mainline kernel 5.1-rc7)
> * Ubuntu 19.04 (mainline kernel 5.2.0-050200rc1-generic)
> 
> Here is the crash output from dmesg:
> 
> [354073.713350] INFO: task Xorg:920 blocked for more than 120 seconds.
> [354073.717755]   Not tainted 5.2.0-050200rc1-generic #201905191930
> [354073.722277] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> disables this message.
> [354073.738332] XorgD0   920854 0x00404004
> [354073.738334] Call Trace:
> [354073.738340]  __schedule+0x2ba/0x650
> [354073.738342]  schedule+0x2d/0x90
> [354073.738343]  schedule_preempt_disabled+0xe/0x10
> [354073.738345]  __ww_mutex_lock.isra.11+0x3e0/0x750
> [354073.738346]  __ww_mutex_lock_slowpath+0x16/0x20
> [354073.738347]  ww_mutex_lock+0x34/0x50
> [354073.738352]  ttm_eu_reserve_buffers+0x1f9/0x2e0 [ttm]
> [354073.738356]  qxl_release_reserve_list+0x67/0x150 [qxl]
> [354073.738358]  ? qxl_bo_pin+0xaa/0x190 [qxl]
> [354073.738359]  qxl_cursor_atomic_update+0x1b0/0x2e0 [qxl]
> [354073.738367]  drm_atomic_helper_commit_planes+0xb9/0x220 [drm_kms_helper]
> [354073.738371]  drm_atomic_helper_commit_tail+0x2b/0x70 [drm_kms_helper]
> [354073.738374]  commit_tail+0x67/0x70 [drm_kms_helper]
> [354073.738378]  drm_atomic_helper_commit+0x113/0x120 [drm_kms_helper]
> [354073.738390]  drm_atomic_commit+0x4a/0x50 [drm]
> [354073.738394]  drm_atomic_helper_update_plane+0xe9/0x100 [drm_kms_helper]
> [354073.738402]  __setplane_atomic+0xd3/0x120 [drm]
> [354073.738410]  drm_mode_cursor_universal+0x142/0x270 [drm]
> [354073.738418]  drm_mode_cursor_common+0xcb/0x220 [drm]
> [354073.738425]  ? drm_mode_cursor_ioctl+0x60/0x60 [drm]
> [354073.738432]  drm_mode_cursor2_ioctl+0xe/0x10 [drm]
> [354073.738438]  drm_ioctl_kernel+0xb0/0x100 [drm]
> [354073.738440]  ? ___sys_recvmsg+0x16c/0x200
> [354073.738445]  drm_ioctl+0x233/0x410 [drm]
> [354073.738452]  ? drm_mode_cursor_ioctl+0x60/0x60 [drm]
> [354073.738454]  ? timerqueue_add+0x57/0x90
> [354073.738456]  ? enqueue_hrtimer+0x3c/0x90
> [354073.738458]  do_vfs_ioctl+0xa9/0x640
> [354073.738459]  ? fput+0x13/0x20
> [354073.738461]  ? __sys_recvmsg+0x88/0xa0
> [354073.738462]  ksys_ioctl+0x67/0x90
> [354073.738463]  __x64_sys_ioctl+0x1a/0x20
> [354073.738465]  do_syscall_64+0x5a/0x140
> [354073.738467]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [354073.738468] RIP: 0033:0x7ffad14d3417
> [354073.738472] Code: Bad RIP value.
> [354073.738472] RSP: 002b:7ffdd5679978 EFLAGS: 3246 ORIG_RAX:
> 0010
> [354073.738473] RAX: ffda RBX: 56428a474610 RCX:
> 7ffad14d3417
> [354073.738474] RDX: 7ffdd56799b0 RSI: c02464bb RDI:
> 000e
> [354073.738474] RBP: 7ffdd56799b0 R08: 0040 R09:
> 0010
> [354073.738475] R10: 003f R11: 3246 R12:
> c02464bb
> [354073.738475] R13: 000e R14:  R15:
> 56428a4721d0
> [354073.738511] INFO: task kworker/1:0:27625 blocked for more than 120 
> seconds.
> [354073.745154]   Not tainted 5.2.0-050200rc1-generic #201905191930
> [354073.751900] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> disables this message.
> [354073.762197] kworker/1:0 D0 27625  2 0x80004000
> [354073.762205] Workqueue: events qxl_client_monitors_config_work_func [qxl]
> [354073.762206] Call Trace:
> [354073.762211]  __schedule+0x2ba/0x650
> [354073.762214]  schedule+0x2d/0x90
> [354073.762215]  schedule_preempt_disabled+0xe/0x10
> [354073.762216]  __ww_mutex_lock.isra.11+0x3e0/0x750
> [354073.762217]  ? __switch_to_asm+0x34/0x70
> [354073.762218]  ? __switch_to_asm+0x40/0x70
> [354073.762219]  ? __switch_to_asm+0x40/0x70
> [354073.762220]  __ww_mutex_lock_slowpath+0x16/0x20
> [354073.762221]  ww_mutex_lock+0x34/0x50
> [354073.762235]  drm_modeset_lock+0x35/0xb0 [drm]
> [354073.762243]  drm_modeset_lock_all_ctx+0x5d/0xe0 [drm]
> [354073.762251]  drm_modeset_lock_all+0x5e/0xb0 [drm]
> [354073.762252]  qxl_display_read_client_monitors_config+0x1e1/0x370 [qxl]
> [354073.762254]  qxl_client_monitors_config_work_func+0x15/0x20 [qxl]
> [354073.762256]  process_one_work+0x20f/0x410
> [354073.762257]  worker_thread+0x34/0x400
> [354073.762259]  kthread+0x120/0x140
> [354073.762260]  ? process_one_work+0x410/0x410
> [354073.762261]  ? __kthread_parkme+0x70/0x70
> [354073.762262]  ret_from_fork+0x35/0x40
> 

--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -97,8 

Re: gnome-shell stuck because of amdgpu driver [5.3 RC5]

2019-09-04 Thread Hillf Danton
On Tue, 3 Sep 2019 11:48:12 +0500 From:   Mikhail Gavrilov 

> On Fri, 30 Aug 2019 at 08:30, Hillf Danton  wrote:
> >
> > Add a warning to show if it makes sense in field: neither regression nor
> > problem will have been observed with the warning printed.
>
> I caught the problem.
> 
>
> [21793.094289] [ cut here ]
> [21793.094296] gnome shell stuck warning
> [21793.094391] WARNING: CPU: 14 PID: 1768 at
> drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:332
>
Thanks Mike.

Describe the problems you are experiencing please.
Say is the screen locked up? Machine lockedup? 
Anything unnormal after you see the warning?

Hillf
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: gnome-shell stuck because of amdgpu driver [5.3 RC5]

2019-09-04 Thread Hillf Danton
Daniel Vetter 
>>
>> Now 11:01pm and "gnome shell stuck warning" not appear since 19:17. So
>> looks like issue happens only when computer blocked and monitor in
>> power save mode.
>
> I'd bet on runtime pm or some other power saving feature in amdgpu
> shutting the interrupt handling down before we've handled all the
> interrupts. That would then result in a stuck fence.
>
> Do we already know which fence is stuck?

It is welcomed to shed a thread of light on how to collect/print that info.
Say line:xxx-yyy in path/to/amdgpu/zzz.c

Thanks
Hillf

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: gnome-shell stuck because of amdgpu driver [5.3 RC5]

2019-08-30 Thread Hillf Danton

On Fri, 30 Aug 2019 06:04:06 +0800 Mikhail Gavrilov wrote:
> On Sun, Aug 25, 2019 at 10:13:05PM +0800, Hillf Danton wrote:
> > Can we try to add the fallback timer manually?
> >
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > @@ -322,6 +322,10 @@ int amdgpu_fence_wait_empty(struct amdgp
> > }
> > rcu_read_unlock();
> > 
> > +   if (!timer_pending(>fence_drv.fallback_timer))
> > +   mod_timer(>fence_drv.fallback_timer,
> > +   jiffies + (AMDGPU_FENCE_JIFFIES_TIMEOUT << 1));
> > +
> > r = dma_fence_wait(fence, false);
> > dma_fence_put(fence);
> > return r;
> > --
> >
> > Or simply wait with an ear on signal and timeout if adding timer
> > seems to go a bit too far?
> >
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > @@ -322,7 +322,12 @@ int amdgpu_fence_wait_empty(struct amdgp
> > }
> > rcu_read_unlock();
> > 
> > -   r = dma_fence_wait(fence, false);
> > +   if (0 < dma_fence_wait_timeout(fence, true,
> > +   AMDGPU_FENCE_JIFFIES_TIMEOUT +
> > +   (AMDGPU_FENCE_JIFFIES_TIMEOUT >> 3)))
> > +   r = 0;
> > +   else
> > +   r = -EINVAL;
> > dma_fence_put(fence);

WARN(r, "gnome shell stuck warning\n");

> > return r;
> >  }
> 
> I tested both patches on top of 5.3 RC6. Each patch I was tested more
> than 24 hours and I don't seen any regressions or problems with them.
> 
Add a warning to show if it makes sense in field: neither regression nor
problem will have been observed with the warning printed.

Thanks
Hillf

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: gnome-shell stuck because of amdgpu driver [5.3 RC5]

2019-08-25 Thread Hillf Danton

On Sun, 25 Aug 2019 04:28:01 -0700 Mikhail Gavrilov wrote:
> Hi folks,
> I left unblocked gnome-shell at noon, and when I returned at the
> evening I discovered than monitor not sleeping and show open gnome
> activity. At first, I thought that some application did not let fall
> asleep the system. But when I try to move the mouse, I realized that
> the system hanged. So I connect via ssh and tried to investigate the
> problem. I did not see anything strange in kernel logs. And my last
> idea before trying to kill the gnome-shell process was dumps tasks
> that are in uninterruptable (blocked) state.
> 
> After [Alt + PrnScr + W] I saw this:
> 
> [32840.701909] sysrq: Show Blocked State
> [32840.701976]   taskPC stack   pid father
> [32840.702407] gnome-shell D11240  1900   1830 0x
> [32840.702438] Call Trace:
> [32840.702446]  ? __schedule+0x352/0x900
> [32840.702453]  schedule+0x3a/0xb0
> [32840.702457]  schedule_timeout+0x289/0x3c0
> [32840.702461]  ? find_held_lock+0x32/0x90
> [32840.702464]  ? find_held_lock+0x32/0x90
> [32840.702469]  ? mark_held_locks+0x50/0x80
> [32840.702473]  ? _raw_spin_unlock_irqrestore+0x4b/0x60
> [32840.702478]  dma_fence_default_wait+0x1f5/0x340
> [32840.702482]  ? dma_fence_free+0x20/0x20
> [32840.702487]  dma_fence_wait_timeout+0x182/0x1e0
> [32840.702533]  amdgpu_fence_wait_empty+0xe7/0x210 [amdgpu]
> [32840.702577]  amdgpu_pm_compute_clocks+0x70/0x5f0 [amdgpu]
> [32840.702641]  dm_pp_apply_display_requirements+0x19e/0x1c0 [amdgpu]
> [32840.702705]  dce12_update_clocks+0xd8/0x110 [amdgpu]
> [32840.702766]  dc_commit_state+0x414/0x590 [amdgpu]
> [32840.702834]  amdgpu_dm_atomic_commit_tail+0xd1e/0x1cf0 [amdgpu]
> [32840.702840]  ? reacquire_held_locks+0xed/0x210
> [32840.702848]  ? ttm_eu_backoff_reservation+0xa5/0x160 [ttm]
> [32840.702853]  ? find_held_lock+0x32/0x90
> [32840.702855]  ? find_held_lock+0x32/0x90
> [32840.702860]  ? __lock_acquire+0x247/0x1910
> [32840.702867]  ? find_held_lock+0x32/0x90
> [32840.702871]  ? mark_held_locks+0x50/0x80
> [32840.702874]  ? _raw_spin_unlock_irq+0x29/0x40
> [32840.702877]  ? lockdep_hardirqs_on+0xf0/0x180
> [32840.702881]  ? _raw_spin_unlock_irq+0x29/0x40
> [32840.702884]  ? wait_for_completion_timeout+0x75/0x190
> [32840.702895]  ? commit_tail+0x3c/0x70 [drm_kms_helper]
> [32840.702902]  commit_tail+0x3c/0x70 [drm_kms_helper]
> [32840.702909]  drm_atomic_helper_commit+0xe3/0x150 [drm_kms_helper]
> [32840.702922]  drm_atomic_connector_commit_dpms+0xd7/0x100 [drm]
> [32840.702936]  set_property_atomic+0xcc/0x140 [drm]
> [32840.702955]  drm_mode_obj_set_property_ioctl+0xcb/0x1c0 [drm]
> [32840.702968]  ? drm_mode_obj_find_prop_id+0x40/0x40 [drm]
> [32840.702978]  drm_ioctl_kernel+0xaa/0xf0 [drm]
> [32840.702990]  drm_ioctl+0x208/0x390 [drm]
> [32840.703003]  ? drm_mode_obj_find_prop_id+0x40/0x40 [drm]
> [32840.703007]  ? sched_clock_cpu+0xc/0xc0
> [32840.703012]  ? lockdep_hardirqs_on+0xf0/0x180
> [32840.703053]  amdgpu_drm_ioctl+0x49/0x80 [amdgpu]
> [32840.703058]  do_vfs_ioctl+0x411/0x750
> [32840.703065]  ksys_ioctl+0x5e/0x90
> [32840.703069]  __x64_sys_ioctl+0x16/0x20
> [32840.703072]  do_syscall_64+0x5c/0xb0
> [32840.703076]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [32840.703079] RIP: 0033:0x7f8bcab0f00b
> [32840.703084] Code: Bad RIP value.
> [32840.703086] RSP: 002b:7ffe76c62338 EFLAGS: 0246 ORIG_RAX: 
> 0010
> [32840.703089] RAX: ffda RBX: 7ffe76c62370 RCX: 
> 7f8bcab0f00b
> [32840.703092] RDX: 7ffe76c62370 RSI: c01864ba RDI: 
> 0009
> [32840.703094] RBP: c01864ba R08: 0003 R09: 
> c0c0c0c0
> [32840.703096] R10: 56476c86a018 R11: 0246 R12: 
> 56476c8ad940
> [32840.703098] R13: 0009 R14: 0002 R15: 
> 0003
> [root@localhost ~]#
> [root@localhost ~]# ps aux | grep gnome-shell
> mikhail 1900  0.3  1.1 6447496 378696 tty2   Dl+  Aug24   2:10 > 
> /usr/bin/gnome-shell
> mikhail 2099  0.0  0.0 519984 23392 ?Ssl  Aug24   0:00 > 
> /usr/libexec/gnome-shell-calendar-server
> mikhail12214  0.0  0.0 399484 29660 pts/2Sl+  Aug24   0:00 > 
> /usr/bin/python3 /usr/bin/chrome-gnome-shell
> chrome-extension://gphhapmejobijbbhgpjhcjognlahblep/
> root   22957  0.0  0.0 216120  2456 pts/10   S+   03:59   0:00 > grep 
> --color=auto gnome-shell
> 
> After it, I tried to kill gnome-shell process with signal 9, but the
> process won't terminate after several unsuccessful attempts.
> 
> Only [Alt + PrnScr + B] helped reboot the hanging system.
> I am writing here because I hope some ampgpu hackers cal look in the
> trace and understand that is happening.
> 
> Sorry, I dont know how to reproduce this bug. But the problem itself
> is very annoying.
> 
> Thanks.
> 
> GPU: AMD Radeon VII
> Kernel: 5.3 RC5
> 
Can we try to add the fallback timer manually?

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ 

Re: The issue with page allocation 5.3 rc1-rc2 (seems drm culprit here)

2019-08-09 Thread Hillf Danton

On Thu, 8 Aug 2019 13:32:06 +0800 Alex Deucher wrote:
> 
> On Wed, Aug 7, 2019 at 11:49 PM Mikhail Gavrilov wrote:
> >
> > Unfortunately error "gnome-shell: page allocation failure: order:4,
> > mode:0x40cc0(GFP_KERNEL|__GFP_COMP),
> > nodemask=(null),cpuset=/,mems_allowed=0" still happens even with
> > applying this patch.

Thanks Mikhail.

No surpring to see the warning because of kvmalloc on top of the current
kmalloc. Any other difference observed?

> I think we can just drop the kmalloc altogether.

Dropping kmalloc altogether OTOH makes the reason for the vmalloc
fallback IMO, Sir?

> How about this patch?
> 
> From: Alex Deucher 
> Date: Thu, 8 Aug 2019 00:29:23 -0500
> Subject: [PATCH] drm/amd/display: use kvmalloc for dc_state
> 
> It's large and doesn't need contiguous memory.
> 
> Signed-off-by: Alex Deucher 
> ---

Looks good to me if with a kvfree added.

>  drivers/gpu/drm/amd/display/dc/core/dc.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 252b621d93a9..ef780a4e484a 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include 
> +#include 
>  
>  #include "dm_services.h"
>  
> @@ -1183,8 +1184,8 @@ bool dc_post_update_surfaces_to_stream(struct dc *dc)
>  
>  struct dc_state *dc_create_state(struct dc *dc)
>  {
> - struct dc_state *context = kzalloc(sizeof(struct dc_state),
> -GFP_KERNEL);
> + struct dc_state *context = kvzalloc(sizeof(struct dc_state),
> + GFP_KERNEL);
>  
>   if (!context)
>   return NULL;
> @@ -1204,11 +1205,11 @@ struct dc_state *dc_create_state(struct dc *dc)
>  struct dc_state *dc_copy_state(struct dc_state *src_ctx)
>  {
>   int i, j;
> - struct dc_state *new_ctx = kmemdup(src_ctx,
> - sizeof(struct dc_state), GFP_KERNEL);
> + struct dc_state *new_ctx = kvmalloc(sizeof(struct dc_state), 
> GFP_KERNEL);
>  
>   if (!new_ctx)
>   return NULL;
> + memcpy(new_ctx, src_ctx, sizeof(struct dc_state));
>  
>   for (i = 0; i < MAX_PIPES; i++) {
>   struct pipe_ctx *cur_pipe = 
> _ctx->res_ctx.pipe_ctx[i];
> -- 
> 2.20.1
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: The issue with page allocation 5.3 rc1-rc2 (seems drm culprit here)

2019-08-06 Thread Hillf Danton

On Tue, 6 Aug 2019 01:15:01 +0800 Mikhail Gavrilov wrote:
>
> Unfortunately couldn't check this patch because, with the patch, the
> kernel did not compile.
> Here is compile error messages:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c: In function
> 'dc_create_state':
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1178:13: error:
> implicit declaration of function 'kvzalloc'; did you mean 'kzalloc'?
> [-Werror=implicit-function-declaration]
>  1178 |   context = kvzalloc(sizeof(struct dc_state),
>   | ^~~~
>   | kzalloc
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1178:11: warning:
> assignment to 'struct dc_state *' from 'int' makes pointer from
> integer without a cast [-Wint-conversion]
>  1178 |   context = kvzalloc(sizeof(struct dc_state),
>   |   ^
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c: In function 
> 'dc_copy_state':
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1203:13: error:
> implicit declaration of function 'kvmalloc'; did you mean 'kmalloc'?
> [-Werror=implicit-function-declaration]
>  1203 |   new_ctx = kvmalloc(sizeof(*new_ctx), GFP_KERNEL);
>   | ^~~~
>   | kmalloc
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1203:11: warning:
> assignment to 'struct dc_state *' from 'int' makes pointer from
> integer without a cast [-Wint-conversion]
>  1203 |   new_ctx = kvmalloc(sizeof(*new_ctx), GFP_KERNEL);
>   |   ^
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c: In function 
> 'dc_state_free':
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1242:2: error:
> implicit declaration of function 'kvfree'; did you mean 'kzfree'?
> [-Werror=implicit-function-declaration]
>  1242 |  kvfree(context);
>   |  ^~
>   |  kzfree
> cc1: some warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:274:
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.o] Error 1
> make[4]: *** Waiting for unfinished jobs
> make[3]: *** [scripts/Makefile.build:490: drivers/gpu/drm/amd/amdgpu] Error 2
> make[3]: *** Waiting for unfinished jobs
> make: *** [Makefile:1084: drivers] Error 2

My bad, respin with one header file added.

Hillf
-8<---

--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -23,6 +23,7 @@
  */

 #include 
+#include 

 #include "dm_services.h"

@@ -1174,8 +1175,12 @@ struct dc_state *dc_create_state(struct
struct dc_state *context = kzalloc(sizeof(struct dc_state),
   GFP_KERNEL);

-   if (!context)
-   return NULL;
+   if (!context) {
+   context = kvzalloc(sizeof(struct dc_state),
+  GFP_KERNEL);
+   if (!context)
+   return NULL;
+   }
/* Each context must have their own instance of VBA and in order to
 * initialize and obtain IP and SOC the base DML instance from DC is
 * initially copied into every context
@@ -1195,8 +1200,13 @@ struct dc_state *dc_copy_state(struct dc
struct dc_state *new_ctx = kmemdup(src_ctx,
sizeof(struct dc_state), GFP_KERNEL);

-   if (!new_ctx)
-   return NULL;
+   if (!new_ctx) {
+   new_ctx = kvmalloc(sizeof(*new_ctx), GFP_KERNEL);
+   if (new_ctx)
+   *new_ctx = *src_ctx;
+   else
+   return NULL;
+   }

for (i = 0; i < MAX_PIPES; i++) {
struct pipe_ctx *cur_pipe = 
_ctx->res_ctx.pipe_ctx[i];
@@ -1230,7 +1240,7 @@ static void dc_state_free(struct kref *k
 {
struct dc_state *context = container_of(kref, struct dc_state, 
refcount);
dc_resource_state_destruct(context);
-   kfree(context);
+   kvfree(context);
 }

 void dc_release_state(struct dc_state *context)
--

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 5/9] drm/ttm: TTM fault handler helpers

2019-06-13 Thread Hillf Danton

Hello Thomas

On Wed, 12 Jun 2019 08:42:39 +0200 Thomas Hellstrom wrote:
> From: Thomas Hellstrom 
> 
> With the vmwgfx dirty tracking, the default TTM fault handler is not
> completely sufficient (vmwgfx need to modify the vma->vm_flags member,
> and also needs to restrict the number of prefaults).
> 
> We also want to replicate the new ttm_bo_vm_reserve() functionality
> 
> So start turning the TTM vm code into helpers: ttm_bo_vm_fault_reserved()
> and ttm_bo_vm_reserve(), and provide a default TTM fault handler for other
> drivers to use.
> 
> Cc: "Christian König" 
> 
> Signed-off-by: Thomas Hellstrom 
> Reviewed-by: "Christian König"  #v1
> ---
>  drivers/gpu/drm/ttm/ttm_bo_vm.c | 175 +++-
>  include/drm/ttm/ttm_bo_api.h|  10 ++
>  2 files changed, 113 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 196e13a0adad..2d9862fcf6fd 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -42,8 +42,6 @@
>  #include 
>  #include 
>  
> -#define TTM_BO_VM_NUM_PREFAULT 16
> -
>  static vm_fault_t ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
>   struct vm_fault *vmf)
>  {
> @@ -106,31 +104,30 @@ static unsigned long ttm_bo_io_mem_pfn(struct 
> ttm_buffer_object *bo,
>   + page_offset;
>  }
>  
> -static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
> +/**
> + * ttm_bo_vm_reserve - Reserve a buffer object in a retryable vm callback
> + * @bo: The buffer object
> + * @vmf: The fault structure handed to the callback
> + *
> + * vm callbacks like fault() and *_mkwrite() allow for the mm_sem to be 
> dropped
> + * during long waits, and after the wait the callback will be restarted. This
> + * is to allow other threads using the same virtual memory space concurrent
> + * access to map(), unmap() completely unrelated buffer objects. TTM buffer
> + * object reservations sometimes wait for GPU and should therefore be
> + * considered long waits. This function reserves the buffer object 
> interruptibly
> + * taking this into account. Starvation is avoided by the vm system not
> + * allowing too many repeated restarts.
> + * This function is intended to be used in customized fault() and _mkwrite()
> + * handlers.
> + *
> + * Return:
> + *0 on success and the bo was reserved.
> + *VM_FAULT_RETRY if blocking wait.
> + *VM_FAULT_NOPAGE if blocking wait and retrying was not allowed.
> + */
> +vm_fault_t ttm_bo_vm_reserve(struct ttm_buffer_object *bo,
> +  struct vm_fault *vmf)
>  {
> - struct vm_area_struct *vma = vmf->vma;
> - struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
> - vma->vm_private_data;
> - struct ttm_bo_device *bdev = bo->bdev;
> - unsigned long page_offset;
> - unsigned long page_last;
> - unsigned long pfn;
> - struct ttm_tt *ttm = NULL;
> - struct page *page;
> - int err;
> - int i;
> - vm_fault_t ret = VM_FAULT_NOPAGE;
> - unsigned long address = vmf->address;
> - struct ttm_mem_type_manager *man =
> - >man[bo->mem.mem_type];
> - struct vm_area_struct cvma;
> -
> - /*
> -  * Work around locking order reversal in fault / nopfn
> -  * between mmap_sem and bo_reserve: Perform a trylock operation
> -  * for reserve, and if it fails, retry the fault after waiting
> -  * for the buffer to become unreserved.
> -  */
Is it likely to not cut the comment as the trylock is still there?

>   if (unlikely(!reservation_object_trylock(bo->resv))) {
>   if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
>   if (!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
> @@ -151,14 +148,55 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>   return VM_FAULT_NOPAGE;
>   }
>  
> + return 0;
> +}
> +EXPORT_SYMBOL(ttm_bo_vm_reserve);
> +
> +/**
> + * ttm_bo_vm_fault_reserved - TTM fault helper
> + * @vmf: The struct vm_fault given as argument to the fault callback
> + * @prot: The page protection to be used for this memory area.
> + * @num_prefault: Maximum number of prefault pages. The caller may want to
> + * specify this based on madvice settings and the size of the GPU object
> + * backed by the memory.
> + *
> + * This function inserts one or more page table entries pointing to the
> + * memory backing the buffer object, and then returns a return code
> + * instructing the caller to retry the page access.
> + *
> + * Return:
> + *   VM_FAULT_NOPAGE on success or pending signal
> + *   VM_FAULT_SIGBUS on unspecified error
> + *   VM_FAULT_OOM on out-of-memory
> + *   VM_FAULT_RETRY if retryable wait
> + */
> +vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
> + pgprot_t prot,
> + pgoff_t num_prefault)
> +{
> + struct vm_area_struct *vma = vmf->vma;
> + struct 

Re: [PATCH 02/12] dma-buf: add dma_buf_(un)map_attachment_locked variants v3

2019-05-28 Thread Hillf Danton

On Mon, 27 May 2019 18:56:20 +0800 Christian Koenig wrote:
> Thanks for the comments, but you are looking at a completely outdated 
> patchset.
> 
> If you are interested in the newest one please ping me and I'm going to CC you
> when I send out the next version.
> 
Ping...

Thanks
Hillf

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/12] drm: remove prime sg_table caching

2019-05-27 Thread Hillf Danton

On Tue, 16 Apr 2019 20:38:35 +0200 Christian König wrote:
> @ -331,14 +282,19 @@ EXPORT_SYMBOL(drm_gem_map_dma_buf);
>   * @sgt: scatterlist info of the buffer to unmap
>   * @dir: direction of DMA transfer
>   *
> - * Not implemented. The unmap is done at drm_gem_map_detach().  This can be
> - * used as the _buf_ops.unmap_dma_buf callback.
> + * This can be used as the _buf_ops.unmap_dma_buf callback.
>   */
>  void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
>  struct sg_table *sgt,
>  enum dma_data_direction dir)
>  {
> - /* nothing to be done here */
> + if (!sgt)

if (WARN_ON(!sgt))   ?
> + return;
> +
> + dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
> +DMA_ATTR_SKIP_CPU_SYNC);
> + sg_free_table(sgt);
> + kfree(sgt);
>  }
>  EXPORT_SYMBOL(drm_gem_unmap_dma_buf);
>  
> -- 
> 2.17.1
> 
BR
Hillf

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/12] dma-buf: lock the reservation object during (un)map_dma_buf v3

2019-05-27 Thread Hillf Danton

On Tue, 16 Apr 2019 20:38:32 +0200 Christian König wrote:
> @@ -688,9 +689,9 @@ struct sg_table *dma_buf_map_attachment(struct 
> dma_buf_attachment *attach,
>   if (attach->sgt)
>   return attach->sgt;
>  
> - sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
> - if (!sg_table)
> - sg_table = ERR_PTR(-ENOMEM);
> + reservation_object_lock(attach->dmabuf->resv, NULL);
> + sg_table = dma_buf_map_attachment_locked(attach, direction);
> + reservation_object_unlock(attach->dmabuf->resv);
>  
Looks better if sg_table is checked after mapping, and feed error info
back in case there is anything unusual.

>   return sg_table;
>  }

Best Regards
Hillf

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/12] dma-buf: add explicit buffer pinning

2019-05-27 Thread Hillf Danton

On Tue, 16 Apr 2019 20:38:34 +0200 Christian König wrote:
> + /**
> +  * @unpin_dma_buf:
> +  *
> +  * This is called by dma_buf_unpin and lets the exporter know that an
> +  * importer doesn't need to the DMA-buf to stay were it is any more.
> +  *
s/need to/need/  s/were/where/

> +  * This is called with the dmabuf->resv object locked.
> +  *
> +  * This callback is optional.
> +  */
> + void (*unpin)(struct dma_buf *);
> +
BR
Hillf

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[patch] drm/ttm: capture overflow of mapping boundary in ttm_bo_kmap()

2019-04-02 Thread Hillf Danton
Simply complain if the combined result of start_page and num_pages goes over
the pages bo has. Let me know if the added warning is too heavy a hammer.

Cc: Christian Koenig 
Cc: Huang Rui 
Cc: Junwei Zhang 
Signed-off-by: Hillf Danton 
---

--- a/drivers/gpu/drm/ttm/ttm_bo_util.c 2019-04-02 09:27:24.521761100 +0800
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c 2019-04-02 09:37:52.108898500 +0800
@@ -631,6 +631,8 @@ int ttm_bo_kmap(struct ttm_buffer_object
return -EINVAL;
if (start_page > bo->num_pages)
return -EINVAL;
+   if (WARN_ON(start_page > bo->num_pages - num_pages))
+   return -EINVAL;
 
(void) ttm_mem_io_lock(man, false);
ret = ttm_mem_io_reserve(bo->bdev, >mem);
--

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 03/12] staging: android: ion: Duplicate sg_table

2017-03-05 Thread Hillf Danton

On March 03, 2017 5:45 AM Laura Abbott wrote: 
> 
> +static struct sg_table *dup_sg_table(struct sg_table *table)
> +{
> + struct sg_table *new_table;
> + int ret, i;
> + struct scatterlist *sg, *new_sg;
> +
> + new_table = kzalloc(sizeof(*new_table), GFP_KERNEL);
> + if (!new_table)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
> + if (ret) {
> + kfree(table);

Free new table?

> + return ERR_PTR(-ENOMEM);
> + }
> +
> + new_sg = new_table->sgl;
> + for_each_sg(table->sgl, sg, table->nents, i) {
> + memcpy(new_sg, sg, sizeof(*sg));
> + sg->dma_address = 0;
> + new_sg = sg_next(new_sg);
> + }
> +

Do we need a helper, sg_copy_table(dst_table, src_table)?

> + return new_table;
> +}
> +

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC 6/6] drivers: staging: ion: add ION_IOC_TAG ioctl

2016-10-12 Thread Hillf Danton
On Wednesday, October 12, 2016 7:50 AM Ruchi Kandoi wrote:
> +/**
> + * struct ion_fd_data - metadata passed from userspace for a handle

s/fd/tag/ ?

> + * @handle:  a handle
> + * @tag: a string describing the buffer
> + *
> + * For ION_IOC_TAG userspace populates the handle field with
> + * the handle returned from ion alloc and type contains the memtrack_type 
> which
> + * accurately describes the usage for the memory.
> + */
> +struct ion_tag_data {
> + ion_user_handle_t handle;
> + char tag[ION_MAX_TAG_LEN];
> +};
> +



OOPS with 3.9.0rc2+

2013-03-14 Thread Hillf Danton
On Wed, Mar 13, 2013 at 11:44 PM, Borislav Petkov  wrote:
> + dri-devel.
>
> On Wed, Mar 13, 2013 at 02:34:31PM +0100, Rolf Offermanns wrote:
>> Hi,
>>
>> I get a kernel oops / panic with a 3.9.0rc2+ kernel (git from 2h ago) on my 
>> Sony
>> Vaio laptop. It happened with rc1, too.
>>
>> Unfortunately I only have a screen photo as nothing gets written to the 
>> system
>> log.
>>
>> Here is a transcript:
>> -[cut here]
>> WARNING: at linux-2.6/drivers/gpu/drm/drm.crtc.c:84 
>> drm_warn_on_modeset_not_all_locked+0x80/0x90 [drm]()
>> Hardware name: VPCF23A9E
>> Modules linked in: [...]
>> Pid: 1894, comm: plasma-desktop Tainted: G   D W  3.9.0-rc2+ #1
>> Call Trace:
>> warn_slowpath_common+0x7f/0xc0
>>  warn_slowpath_null+0x1a/020
>>  drm_warn_on_modeset_not_all_locked+0x80/0x90 [drm]
>>  drm_fb_helper_restore_fbdev_mode+0x1c/0x80 [drm_kms_helper]
>>  drm_fb_helper_force_kernel_mode+0x4d/0xa0 [drm_kms_helper]
>>  drm_fb_helper_panic+0x2b/0x30 [drm_kms_helper]
>>  notifier_call_chain+0x4d/0x70
>>  __atomic_notifier_call_chain+0x12/0x20
>>  atomic_notifier_call_chain+0x16/0x20
>>  panic+0xef/0x1d0
>>  oops_end+0xe2/0xf0
>>  die+0x58/0x90
>>  do_trap+0x6b/0170
>>  ? __atomic_notifier_call_chain+0x12/0x20
>>  do_divide_error+0x96/0xa0
>>  ? intel_pstate_timer_func+0x15d/0x380
>>  ? check_preempt_curr+0x75/0xa0
>>  ? ttwu_do_wakeup+0x2c/0xf0
>>  divide_error+0x1e/0x30

Hmm, registered timer results in divide err,
what is it?

>>  ? intel_pstate_timer_func+0x15d/0x380
>>  ? intel_pstate_timer_func+0xbc/0x380
>>  ? pid_param_set+0x120/0x120
>>  call_timer_fn+0x3a/0x120
>>  ? pid_param_set+0x120/0x120
>>  run_timer_softirq+0x1fe/0x2b0
>>  __do_softirq+0xe0/0x230
>>  irq_exit+0xa5/0xb0
>>  smp_apic_timer_interrupt+0x6e/0x99
>>  apic_timer_interrupt+0x6d/0x80
>>   ? sysret_audit+0x17/0x21
>>  -[ end trace  ]--
>>  NOHZ: local_softirq_pending 282
>>
>>
>>  Is this of any use at all? How can I help track this down? Shall I attach a
>>  serial console (USB only)?
>>
>> lspci:
>>
>> 00:00.0 Host bridge: Intel Corporation 2nd Generation Core Processor Family 
>> DRAM Controller (rev 09)
>> 00:01.0 PCI bridge: Intel Corporation Xeon E3-1200/2nd Generation Core 
>> Processor Family PCI Express Root Port (rev 09)
>> 00:16.0 Communication controller: Intel Corporation 6 Series/C200 Series 
>> Chipset Family MEI Controller #1 (rev 04)
>> 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset 
>> Family USB Enhanced Host Controller #2 (rev 04)
>> 00:1b.0 Audio device: Intel Corporation 6 Series/C200 Series Chipset Family 
>> High Definition Audio Controller (rev 04)
>> 00:1c.0 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family 
>> PCI Express Root Port 1 (rev b4)
>> 00:1c.1 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family 
>> PCI Express Root Port 2 (rev b4)
>> 00:1c.2 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family 
>> PCI Express Root Port 3 (rev b4)
>> 00:1c.3 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family 
>> PCI Express Root Port 4 (rev b4)
>> 00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset 
>> Family USB Enhanced Host Controller #1 (rev 04)
>> 00:1f.0 ISA bridge: Intel Corporation HM65 Express Chipset Family LPC 
>> Controller (rev 04)
>> 00:1f.2 SATA controller: Intel Corporation 6 Series/C200 Series Chipset 
>> Family 6 port SATA AHCI Controller (rev 04)
>> 00:1f.3 SMBus: Intel Corporation 6 Series/C200 Series Chipset Family SMBus 
>> Controller (rev 04)
>> 01:00.0 VGA compatible controller: NVIDIA Corporation GF108 [GeForce GT 
>> 540M] (rev a1)
>> 01:00.1 Audio device: NVIDIA Corporation GF108 High Definition Audio 
>> Controller (rev a1)
>> 02:00.0 Network controller: Atheros Communications Inc. AR9485 Wireless 
>> Network Adapter (rev 01)
>> 03:00.0 SD Host controller: Ricoh Co Ltd PCIe SDXC/MMC Host Controller (rev 
>> 09) 03:00.1 System peripheral: Ricoh Co Ltd Device e232 (rev 06)
>> 03:00.3 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 PCIe IEEE 1394 Controller 
>> (rev 05)
>> 04:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller 
>> (rev 04)
>> 05:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168 
>> PCI Express Gigabit Ethernet controller (rev 06)
>>
>>
>> cpuinfo:
>> processor   : 0-7
>> vendor_id   : GenuineIntel
>> cpu family  : 6
>> model   : 42
>> model name  : Intel(R) Core(TM) i7-2670QM CPU @ 2.20GHz
>> stepping: 7
>> microcode   : 0x28
>> cpu MHz : 800.000
>> cache size  : 6144 KB
>> physical id : 0
>> siblings: 8
>> core id : 0
>> cpu cores   : 4
>> apicid  : 0
>> initial apicid  : 0
>> fpu : yes
>> fpu_exception   : yes
>> cpuid level : 13
>> wp  : yes
>> flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
>> cmov
>> pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx 

Re: OOPS with 3.9.0rc2+

2013-03-14 Thread Hillf Danton
On Wed, Mar 13, 2013 at 11:44 PM, Borislav Petkov b...@alien8.de wrote:
 + dri-devel.

 On Wed, Mar 13, 2013 at 02:34:31PM +0100, Rolf Offermanns wrote:
 Hi,

 I get a kernel oops / panic with a 3.9.0rc2+ kernel (git from 2h ago) on my 
 Sony
 Vaio laptop. It happened with rc1, too.

 Unfortunately I only have a screen photo as nothing gets written to the 
 system
 log.

 Here is a transcript:
 -[cut here]
 WARNING: at linux-2.6/drivers/gpu/drm/drm.crtc.c:84 
 drm_warn_on_modeset_not_all_locked+0x80/0x90 [drm]()
 Hardware name: VPCF23A9E
 Modules linked in: [...]
 Pid: 1894, comm: plasma-desktop Tainted: G   D W  3.9.0-rc2+ #1
 Call Trace:
  IRQ   warn_slowpath_common+0x7f/0xc0
  warn_slowpath_null+0x1a/020
  drm_warn_on_modeset_not_all_locked+0x80/0x90 [drm]
  drm_fb_helper_restore_fbdev_mode+0x1c/0x80 [drm_kms_helper]
  drm_fb_helper_force_kernel_mode+0x4d/0xa0 [drm_kms_helper]
  drm_fb_helper_panic+0x2b/0x30 [drm_kms_helper]
  notifier_call_chain+0x4d/0x70
  __atomic_notifier_call_chain+0x12/0x20
  atomic_notifier_call_chain+0x16/0x20
  panic+0xef/0x1d0
  oops_end+0xe2/0xf0
  die+0x58/0x90
  do_trap+0x6b/0170
  ? __atomic_notifier_call_chain+0x12/0x20
  do_divide_error+0x96/0xa0
  ? intel_pstate_timer_func+0x15d/0x380
  ? check_preempt_curr+0x75/0xa0
  ? ttwu_do_wakeup+0x2c/0xf0
  divide_error+0x1e/0x30

Hmm, registered timer results in divide err,
what is it?

  ? intel_pstate_timer_func+0x15d/0x380
  ? intel_pstate_timer_func+0xbc/0x380
  ? pid_param_set+0x120/0x120
  call_timer_fn+0x3a/0x120
  ? pid_param_set+0x120/0x120
  run_timer_softirq+0x1fe/0x2b0
  __do_softirq+0xe0/0x230
  irq_exit+0xa5/0xb0
  smp_apic_timer_interrupt+0x6e/0x99
  apic_timer_interrupt+0x6d/0x80
  EOI ? sysret_audit+0x17/0x21
  -[ end trace  ]--
  NOHZ: local_softirq_pending 282


  Is this of any use at all? How can I help track this down? Shall I attach a
  serial console (USB only)?

 lspci:

 00:00.0 Host bridge: Intel Corporation 2nd Generation Core Processor Family 
 DRAM Controller (rev 09)
 00:01.0 PCI bridge: Intel Corporation Xeon E3-1200/2nd Generation Core 
 Processor Family PCI Express Root Port (rev 09)
 00:16.0 Communication controller: Intel Corporation 6 Series/C200 Series 
 Chipset Family MEI Controller #1 (rev 04)
 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset 
 Family USB Enhanced Host Controller #2 (rev 04)
 00:1b.0 Audio device: Intel Corporation 6 Series/C200 Series Chipset Family 
 High Definition Audio Controller (rev 04)
 00:1c.0 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family 
 PCI Express Root Port 1 (rev b4)
 00:1c.1 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family 
 PCI Express Root Port 2 (rev b4)
 00:1c.2 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family 
 PCI Express Root Port 3 (rev b4)
 00:1c.3 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family 
 PCI Express Root Port 4 (rev b4)
 00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset 
 Family USB Enhanced Host Controller #1 (rev 04)
 00:1f.0 ISA bridge: Intel Corporation HM65 Express Chipset Family LPC 
 Controller (rev 04)
 00:1f.2 SATA controller: Intel Corporation 6 Series/C200 Series Chipset 
 Family 6 port SATA AHCI Controller (rev 04)
 00:1f.3 SMBus: Intel Corporation 6 Series/C200 Series Chipset Family SMBus 
 Controller (rev 04)
 01:00.0 VGA compatible controller: NVIDIA Corporation GF108 [GeForce GT 
 540M] (rev a1)
 01:00.1 Audio device: NVIDIA Corporation GF108 High Definition Audio 
 Controller (rev a1)
 02:00.0 Network controller: Atheros Communications Inc. AR9485 Wireless 
 Network Adapter (rev 01)
 03:00.0 SD Host controller: Ricoh Co Ltd PCIe SDXC/MMC Host Controller (rev 
 09) 03:00.1 System peripheral: Ricoh Co Ltd Device e232 (rev 06)
 03:00.3 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 PCIe IEEE 1394 Controller 
 (rev 05)
 04:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller 
 (rev 04)
 05:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168 
 PCI Express Gigabit Ethernet controller (rev 06)


 cpuinfo:
 processor   : 0-7
 vendor_id   : GenuineIntel
 cpu family  : 6
 model   : 42
 model name  : Intel(R) Core(TM) i7-2670QM CPU @ 2.20GHz
 stepping: 7
 microcode   : 0x28
 cpu MHz : 800.000
 cache size  : 6144 KB
 physical id : 0
 siblings: 8
 core id : 0
 cpu cores   : 4
 apicid  : 0
 initial apicid  : 0
 fpu : yes
 fpu_exception   : yes
 cpuid level : 13
 wp  : yes
 flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
 cmov
 pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp 
 lm
 constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc
 aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 
 cx16
 xtpr pdcm pcid sse4_1 sse4_2 x2apic popcnt 

[PATCH] mm: extend prefault helpers to fault in more than PAGE_SIZE

2012-02-16 Thread Hillf Danton
On Thu, Feb 16, 2012 at 8:01 PM, Daniel Vetter  
wrote:
> @@ -416,17 +417,20 @@ static inline int fault_in_pages_writeable(char __user 
> *uaddr, int size)
> ? ? ? ? * Writing zeroes into userspace here is OK, because we know that if
> ? ? ? ? * the zero gets there, we'll be overwriting it.
> ? ? ? ? */
> - ? ? ? ret = __put_user(0, uaddr);
> + ? ? ? while (uaddr <= end) {
> + ? ? ? ? ? ? ? ret = __put_user(0, uaddr);
> + ? ? ? ? ? ? ? if (ret != 0)
> + ? ? ? ? ? ? ? ? ? ? ? return ret;
> + ? ? ? ? ? ? ? uaddr += PAGE_SIZE;
> + ? ? ? }

What if
 uaddr & ~PAGE_MASK == PAGE_SIZE -3 &&
end & ~PAGE_MASK == 2


Re: [PATCH] mm: extend prefault helpers to fault in more than PAGE_SIZE

2012-02-16 Thread Hillf Danton
On Thu, Feb 16, 2012 at 8:01 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 @@ -416,17 +417,20 @@ static inline int fault_in_pages_writeable(char __user 
 *uaddr, int size)
         * Writing zeroes into userspace here is OK, because we know that if
         * the zero gets there, we'll be overwriting it.
         */
 -       ret = __put_user(0, uaddr);
 +       while (uaddr = end) {
 +               ret = __put_user(0, uaddr);
 +               if (ret != 0)
 +                       return ret;
 +               uaddr += PAGE_SIZE;
 +       }

What if
 uaddr  ~PAGE_MASK == PAGE_SIZE -3 
end  ~PAGE_MASK == 2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] hugetlb: release pages in the error path of hugetlb_cow()

2011-11-15 Thread Hillf Danton
commit ea4039a34c4c206d015d34a49d0b00868e37db1d upstream.

If we fail to prepare an anon_vma, the {new, old}_page should be released,
or they will leak.

Signed-off-by: Hillf Danton 
Reviewed-by: Andrea Arcangeli 
Cc: Hugh Dickins 
Cc: Johannes Weiner 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
---
 mm/hugetlb.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index bfcf153..2b57cd9 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2415,6 +2415,8 @@ retry_avoidcopy:
 * anon_vma prepared.
 */
if (unlikely(anon_vma_prepare(vma))) {
+   page_cache_release(new_page);
+   page_cache_release(old_page);
/* Caller expects lock to be held */
spin_lock(>page_table_lock);
return VM_FAULT_OOM;
-- 
1.7.7.3


-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic