lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 1 Mar 2024 19:26:55 +0000
From: Matthew Wilcox <willy@...radead.org>
To: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>
Cc: linux-fsdevel@...r.kernel.org, linux-xfs@...r.kernel.org,
	djwong@...nel.org, mcgrof@...nel.org, linux-mm@...ck.org,
	hare@...e.de, david@...morbit.com, akpm@...ux-foundation.org,
	gost.dev@...sung.com, linux-kernel@...r.kernel.org,
	chandan.babu@...cle.com, Pankaj Raghav <p.raghav@...sung.com>
Subject: Re: [PATCH v2 03/13] filemap: align the index to mapping_min_order
 in the page cache

On Fri, Mar 01, 2024 at 05:44:34PM +0100, Pankaj Raghav (Samsung) wrote:
> +#define DEFINE_READAHEAD_ALIGNED(ractl, f, r, m, i)			\
> +	struct readahead_control ractl = {				\
> +		.file = f,						\
> +		.mapping = m,						\
> +		.ra = r,						\
> +		._index = mapping_align_start_index(m, i),		\
> +	}

My point was that you didn't need to do any of this.

Look, I've tried to give constructive review, but I feel like I'm going
to have to be blunt.  There is no evidence of design or understanding
in these patches or their commit messages.  You don't have a coherent
message about "These things have to be aligned; these things can be at
arbitrary alignment".  If you have thought about it, it doesn't show.

Maybe you just need to go back over the patches and read them as a series,
but it feels like "Oh, there's a hole here, patch it; another hole here,
patch it" without thinking about what's going on and why.

I want to help, but it feels like it'd be easier to do all the work myself
at this point, and that's not good for me, and it's not good for you.

So, let's start off: Is the index in ractl aligned or not, and why do
you believe that's the right approach?  And review each of the patches
in this series with the answer to that question in mind because you are
currently inconsistent.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ