linux-kernel - Re: [PATCH v2] fs: clean up usage of noop_dirty

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] [day] [month] [year] [list]

Message-ID: <20230828144242.GZ3390869@ZenIV>
Date:   Mon, 28 Aug 2023 15:42:42 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Xueshi Hu <xueshi.hu@...rtx.com>
Cc:     hch@...radead.org, dan.j.williams@...el.com,
        vishal.l.verma@...el.com, dave.jiang@...el.com,
        jayalk@...works.biz, daniel@...ll.ch, deller@....de,
        bcrl@...ck.org, brauner@...nel.org, jack@...e.com, tytso@....edu,
        adilger.kernel@...ger.ca, miklos@...redi.hu,
        mike.kravetz@...cle.com, muchun.song@...ux.dev, djwong@...nel.org,
        willy@...radead.org, akpm@...ux-foundation.org, hughd@...gle.com,
        nvdimm@...ts.linux.dev, linux-cxl@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, linux-aio@...ck.org,
        linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-mm@...ck.org, linux-xfs@...r.kernel.org
Subject: Re: [PATCH v2] fs: clean up usage of noop_dirty_folio

On Mon, Aug 28, 2023 at 03:54:49PM +0800, Xueshi Hu wrote:
> In folio_mark_dirty(), it can automatically fallback to
> noop_dirty_folio() if a_ops->dirty_folio is not registered.
> 
> As anon_aops, dev_dax_aops and fb_deferred_io_aops becames empty, remove
> them too.

I'd put the last sentence as 'In dev_dax_aops and fb_deferred_io_aops replacing
.dirty_folio with NULL makes them identical to default (empty_aops) and since
we never compare ->a_ops pointer with either of those, we can remove them
completely'.

There could've been places like
#define is_fb_deferred(mapping) (mapping)->a_ops == fb_deferred_io_aops
and those would've been broken by that.  The fact that there's nothing
of that sort in the tree ought to be mentioned in commit message.

Note that we *do* have places where method table comparisons are used
in predicates like that, so it's not a pure theory; sure, missing that
would've probably ended up with broken build, but that can easily be
dependent upon the config (and that, alas, is also not a pure theory -
BTDT).  In this case the change is correct, fortunately...

Other than that part of commit message -

Acked-by: Al Viro <viro@...iv.linux.org.uk>