All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: "Liu, Chuansheng" <chuansheng.liu@intel.com>
Cc: Peter Suti <peter.suti@streamunlimited.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Javier Martinez Canillas <javierm@redhat.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
	"linux-staging@lists.linux.dev" <linux-staging@lists.linux.dev>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3] staging: fbtft: core: set smem_len before fb_deferred_io_init call
Date: Thu, 28 Jul 2022 16:39:03 +0300	[thread overview]
Message-ID: <20220728133903.GU2316@kadam> (raw)
In-Reply-To: <BL1PR11MB544569ACF9F4EE383F07959597969@BL1PR11MB5445.namprd11.prod.outlook.com>

On Thu, Jul 28, 2022 at 12:49:15PM +0000, Liu, Chuansheng wrote:
> Hi Peter,
> 
> > 
> > The fbtft_framebuffer_alloc() calls fb_deferred_io_init() before
> > initializing info->fix.smem_len.  It is set to zero by the
> > framebuffer_alloc() function.  It will trigger a WARN_ON() at the
> > start of fb_deferred_io_init() and the function will not do anything.
> 
> Please show us the WARN_ON() call stack, then everything is clear.
> It is the driver itself issue to be fixed. I saw Thomas made such
> WARN_ON().

I don't understand the confusion here.  The code is very simple and the
description seems very clear.  No need to redo the patch.  I think
Peter tested it before the WARN_ON() was added so he probably didn't
see the WARN_ON().  I told him to add that because it's pretty obvious
what will happen just from reading the code.

> 
> > 
> > Fixes: 856082f021a2 ("fbdev: defio: fix the pagelist corruption")
> 
> Don't agree with such description.

I don't see how you can disagree?  Before your patch the
fb_deferred_io_init() did not use info->fix.smem_len and now it does.

regards,
dan carpenter

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: "Liu, Chuansheng" <chuansheng.liu@intel.com>
Cc: "linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-staging@lists.linux.dev" <linux-staging@lists.linux.dev>,
	Javier Martinez Canillas <javierm@redhat.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Peter Suti <peter.suti@streamunlimited.com>
Subject: Re: [PATCH v3] staging: fbtft: core: set smem_len before fb_deferred_io_init call
Date: Thu, 28 Jul 2022 16:39:03 +0300	[thread overview]
Message-ID: <20220728133903.GU2316@kadam> (raw)
In-Reply-To: <BL1PR11MB544569ACF9F4EE383F07959597969@BL1PR11MB5445.namprd11.prod.outlook.com>

On Thu, Jul 28, 2022 at 12:49:15PM +0000, Liu, Chuansheng wrote:
> Hi Peter,
> 
> > 
> > The fbtft_framebuffer_alloc() calls fb_deferred_io_init() before
> > initializing info->fix.smem_len.  It is set to zero by the
> > framebuffer_alloc() function.  It will trigger a WARN_ON() at the
> > start of fb_deferred_io_init() and the function will not do anything.
> 
> Please show us the WARN_ON() call stack, then everything is clear.
> It is the driver itself issue to be fixed. I saw Thomas made such
> WARN_ON().

I don't understand the confusion here.  The code is very simple and the
description seems very clear.  No need to redo the patch.  I think
Peter tested it before the WARN_ON() was added so he probably didn't
see the WARN_ON().  I told him to add that because it's pretty obvious
what will happen just from reading the code.

> 
> > 
> > Fixes: 856082f021a2 ("fbdev: defio: fix the pagelist corruption")
> 
> Don't agree with such description.

I don't see how you can disagree?  Before your patch the
fb_deferred_io_init() did not use info->fix.smem_len and now it does.

regards,
dan carpenter

  reply	other threads:[~2022-07-28 13:39 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-26  8:21 [PATCH] staging: fbtft: core: set smem_len before fb_deferred_io_init call Peter Suti
2022-07-26  8:21 ` Peter Suti
2022-07-26 16:13 ` Dan Carpenter
2022-07-26 16:13   ` Dan Carpenter
2022-07-27  7:07   ` [PATCH v2] " Peter Suti
2022-07-27  7:07     ` Peter Suti
2022-07-27  7:13     ` Greg Kroah-Hartman
2022-07-27  7:13       ` Greg Kroah-Hartman
2022-07-27  7:35       ` [PATCH v3] " Peter Suti
2022-07-27  7:35         ` Peter Suti
2022-07-28 12:49         ` Liu, Chuansheng
2022-07-28 12:49           ` Liu, Chuansheng
2022-07-28 13:39           ` Dan Carpenter [this message]
2022-07-28 13:39             ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220728133903.GU2316@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=chuansheng.liu@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=javierm@redhat.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=peter.suti@streamunlimited.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.