public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: replace a wrong memset() with memzero_page()
Date: Tue, 5 Apr 2022 16:04:23 +0200	[thread overview]
Message-ID: <20220405140423.GX15609@twin.jikos.cz> (raw)
In-Reply-To: <aee87ae7-bcf0-726e-b943-3499d4b142e8@gmx.com>

On Tue, Apr 05, 2022 at 01:08:41PM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/4/5 12:58, Christoph Hellwig wrote:
> > On Fri, Mar 25, 2022 at 09:38:16AM -0700, Christoph Hellwig wrote:
> >>> -	memset(kaddr + pgoff, 1, len);
> >>> +	memzero_page(page, pgoff, len);
> >>>   	flush_dcache_page(page);
> >>
> >> memzero_page already takes care of the flush_dcache_page.
> >
> > David, you've picked this up with the stay flush_dcache_page left in
> > place.  Plase try to fix it up instead of spreading the weird cargo cult
> > flush_dcache_page calls.
> 
> Please drop this patch, as discussed with Filipe, the memset() value
> 0x01 could be a poison value to distinguish from plain 0x00.

> Furthermore, we are not sure if we even want to zeroing out/poison the
> content.

I've read the discussion and have been thinking what should we do here,
I'd be for doing memset to 0 because 0x1 is quite arbitrary and does not
follow any established pattern for poison values, if there's anything
like that when returning data from kernel to userspace. I find zeros
OK here. Arguably "It's been 0x1 forever so we should not change it" for
backward compatibility reasons could apply, but this is not an interface
that applications use, the error code is the interface.

  reply	other threads:[~2022-04-05 21:15 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-25  9:37 [PATCH] btrfs: replace a wrong memset() with memzero_page() Qu Wenruo
2022-03-25 10:10 ` Johannes Thumshirn
2022-03-25 10:49   ` Qu Wenruo
2022-03-25 10:58     ` Johannes Thumshirn
2022-03-25 16:38 ` Christoph Hellwig
2022-03-26  1:15   ` Qu Wenruo
2022-04-05  4:58   ` Christoph Hellwig
2022-04-05  5:08     ` Qu Wenruo
2022-04-05 14:04       ` David Sterba [this message]
2022-04-05 13:59     ` David Sterba
2022-03-28 18:51 ` David Sterba
2022-03-28 18:58   ` David Sterba
2022-03-29  9:57   ` Filipe Manana
2022-03-29 10:49     ` Qu Wenruo
2022-03-29 11:39       ` Filipe Manana
2022-03-29 23:52         ` Qu Wenruo
2022-03-30  9:27           ` Filipe Manana
2022-03-30 10:34             ` Filipe Manana
2022-03-30 10:42               ` Qu Wenruo
2022-03-30 11:02                 ` Filipe Manana
2022-03-30 11:03                 ` Graham Cobb
2022-03-30 21:34                   ` Filipe Manana
2022-03-30 22:29                     ` Graham Cobb

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=20220405140423.GX15609@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=dsterba@suse.com \
    --cc=hch@infradead.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=quwenruo.btrfs@gmx.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox