linux-kernel - Re: [PATCH v4 20/25] mm/filemap: Convert wait_on_page_bit to wait_on_folio

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]

Message-ID: <YFI+xNeN+NrgszI7@infradead.org>
Date:   Wed, 17 Mar 2021 18:39:16 +0100
From:   Christoph Hellwig <hch@...radead.org>
To:     "Matthew Wilcox (Oracle)" <willy@...radead.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v4 20/25] mm/filemap: Convert wait_on_page_bit to
 wait_on_folio_bit

> +	if (FolioWriteback(folio) &&
> +	    wait_on_folio_bit_killable(folio, PG_writeback) < 0)
>  		return VM_FAULT_RETRY;

This really screams for a proper wait_on_page_writeback_killable helper
rather than hardcoding the PG_* bit in a random file system.  It also
seems to have missed the conversion to a while loop in
wait_on_page_writeback.

Also this patch seems to be different in style to other by not for now
always using page wrappers in the file systems.  Not that I really care
either way, but it seems inconsistent with the rest.

>  /*
> - * This is exported only for wait_on_page_locked/wait_on_page_writeback, etc.,
> + * This is exported only for wait_on_folio_locked/wait_on_folio_writeback, etc.,
>   * and should not be used directly.
>   */
> -extern void wait_on_page_bit(struct page *page, int bit_nr);
> -extern int wait_on_page_bit_killable(struct page *page, int bit_nr);
> +extern void wait_on_folio_bit(struct folio *folio, int bit_nr);
> +extern int wait_on_folio_bit_killable(struct folio *folio, int bit_nr);

Well, the above code obviously ignored this comment :(  Maybe an
__ prefix is a bit more of hint?