All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Bart Van Assche <Bart.VanAssche@wdc.com>
Cc: "hch@lst.de" <hch@lst.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"torvalds@linux-foundation.org" <torvalds@linux-foundation.org>
Subject: Re: [PATCH] linux/types.h: Restore the ability to disable sparse endianness checks
Date: Mon, 16 Oct 2017 16:34:48 +0300	[thread overview]
Message-ID: <20171016160316-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <1507653494.2815.27.camel@wdc.com>

On Tue, Oct 10, 2017 at 04:38:16PM +0000, Bart Van Assche wrote:
> On Mon, 2017-10-09 at 20:42 +0300, Michael S. Tsirkin wrote:
> > On Mon, Oct 09, 2017 at 03:07:42PM +0000, Bart Van Assche wrote:
> > > On Mon, 2017-10-09 at 16:22 +0300, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 06, 2017 at 10:23:53AM -0700, Bart Van Assche wrote:
> > > > > The purpose of patch "linux/types.h: enable endian checks for all
> > > > > sparse builds" was to encourage driver authors to annotate
> > > > > endianness correctly in their drivers. However, since that patch
> > > > > went upstream no endianness annotations in drivers have been fixed.
> > > > > I think that this shows that the followed approach does not work,
> > > > > probably because several driver authors do not use sparse. Restore
> > > > > the ability to disable sparse endianness checks such that it
> > > > > becomes again easy to review other sparse diagnostics for people
> > > > > who want to analyze drivers they are not the author of.
> > > > > 
> > > > > References: commit 05de97003c77 ("linux/types.h: enable endian checks for all sparse builds")
> > > > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> > > > > Cc: Christoph Hellwig <hch@lst.de>
> > > > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > > 
> > > > I still think the new default is good.  You probably want ability to
> > > > disable these checks selectively for the specific drivers though. Makes
> > > > it easier to spot what needs to be fixed.
> > > 
> > > I agree with what you wrote. And what you described is what my patch implements
> > > - keep endianness checking enabled by default and make it possible to disable it
> > > selectively. Does that mean that you agree with the patch I posted?
> > 
> > I just mean I'd expect a patchset setting the flag for the broken
> > drivers. Presumably this will help trigger some action.
> 
> Hello MST,
> 
> If I would add something like ccflags-y += -D__CHECK_ENDIAN__=0 to the
> Makefile of drivers that are not endianness clean then that would make it
> easier for driver authors to ignore endianness warnings reported by sparse.

They already seem to ignore it.

> I prefer that they have to add CF=-D__CHECK_ENDIAN__=0 to the command line
> when verifying a driver with sparse to get rid of the sparse endianness
> warnings. Do you agree with this?
> 
> Thanks,
> 
> Bart.

I don't see how it'll help make things better. OTOH if the specific
drivers are tagged in the makefile, they can be gradually moved out to
staging or something to help trigger action.

-- 
MST

  reply	other threads:[~2017-10-16 13:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-06 17:23 [PATCH] linux/types.h: Restore the ability to disable sparse endianness checks Bart Van Assche
2017-10-06 17:35 ` Christoph Hellwig
2017-10-06 17:43   ` Bart Van Assche
2017-10-16  9:33     ` Javier González
2017-10-09 13:22 ` Michael S. Tsirkin
2017-10-09 15:07   ` Bart Van Assche
2017-10-09 17:42     ` Michael S. Tsirkin
2017-10-10 16:38       ` Bart Van Assche
2017-10-16 13:34         ` Michael S. Tsirkin [this message]
2017-10-16 13:57           ` Bart Van Assche
2017-10-16 15:27             ` Michael S. Tsirkin
2017-10-16 15:36               ` Bart Van Assche
2017-10-16 16:50                 ` Michael S. Tsirkin
2017-10-16 17:36                   ` Bart Van Assche

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=20171016160316-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=Bart.VanAssche@wdc.com \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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.