Re: [PATCH] btrfs: replace a wrong memset() with memzero_page()

David Sterba
2022-04-05T21:15:07Z

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.