Re: [PATCH v9 11/15] filemap: Add hwpoison handling to filemap_read()

[Matthew Wilcox <willy@infradead.org>]

On Fri, Sep 18, 2026 at 03:47:18PM +0200, David Hildenbrand (Arm) wrote:
> On 8/13/26 18:33, Matthew Wilcox wrote:
> > On Thu, Aug 13, 2026 at 11:55:44AM +0100, Pedro Falcato wrote:
> >>> +static size_t adjust_range_hwpoison(const struct folio *folio, size_t offset,
> >>> +		size_t bytes)
> >>> +{
> >>> +	const struct page *page = folio_page(folio, offset / PAGE_SIZE);
> >>> +	size_t safe_bytes;
> >> 						    offset >> PAGE_SHIFT would probably be more idiomatic
> > 
> > $ git grep '/ PAGE_SIZE' mm fs |wc -l
> > 116
> > $ git grep '>> PAGE_SHIFT' mm fs |wc -l
> > 656
> > 
> > it seems relatively common.  Personally I find / easier to understand
> > because I sometimes get confused between left and right shifts.  I
> > don't think I've ever been confused between multiply and divide ;-)
> 
> Linus once explained why ">> PAGE_SHIFT" is preferable when the type of the
> function might be signed, as the shift generates better code.
> 
> I personally find "offset / PAGE_SIZE" more intuitive, but I also have sympathy
> for PHYS_PFN() to abstract the operation.

Oof.  I appreciate PHYS_PFN() is literally 'shift by PAGE_SHIFT' but
it implies that the parameter is a physical address, which this isn't
(it's a byte offset within a folio).

> (SIZE_TO_PAGES would be a bit mouthful so I wouldn't ever dare suggesting to
> introduce a new helper)

I wouldn't mind a new helper, but it'd have to be a pithy name, and
I don't have one in mind.