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
next prev parent 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.