On 9/18/23 15:41, Matthew Wilcox wrote:
On Mon, Sep 18, 2023 at 01:04:59PM +0200, Hannes Reinecke wrote:
+++ b/mm/filemap.c
@@ -507,9 +507,14 @@ static void __filemap_fdatawait_range(struct address_space *mapping,
pgoff_t end = end_byte >> PAGE_SHIFT;
struct folio_batch fbatch;
unsigned nr_folios;
+ unsigned int order = mapping_min_folio_order(mapping);
folio_batch_init(&fbatch);
+ if (order) {
+ index = ALIGN_DOWN(index, 1 << order);
+ end = ALIGN_DOWN(end, 1 << order);
+ }
while (index <= end) {
unsigned i;
I don't understand why this function needs to change at all.
filemap_get_folios_tag() should return any folios which overlap
(index, end). And aligning 'end' _down_ certainly sets off alarm bells
for me. We surely would need to align _up_. Except i don't think we
need to do anything to this function.
Because 'end' is the _last_ valid index, not the index at which the
iteration stops (cf 'index <= end') here. And as the index remains in 4k
units we need to align both, index and end, to the nearest folio.
@@ -2482,7 +2487,8 @@ static int filemap_create_folio(struct file *file,
struct folio *folio;
int error;
- folio = filemap_alloc_folio(mapping_gfp_mask(mapping), 0);
+ folio = filemap_alloc_folio(mapping_gfp_mask(mapping),
+ mapping_min_folio_order(mapping));
if (!folio)
return -ENOMEM;
Surely we need to align 'index' here?
Surely.
@@ -2542,9 +2548,16 @@ static int filemap_get_pages(struct kiocb *iocb, size_t count,
pgoff_t last_index;
struct folio *folio;
int err = 0;
+ unsigned int order = mapping_min_folio_order(mapping);
/* "last_index" is the index of the page beyond the end of the read */
last_index = DIV_ROUND_UP(iocb->ki_pos + count, PAGE_SIZE);
+ if (order) {
+ /* Align with folio order */
+ WARN_ON(index % 1 << order);
+ index = ALIGN_DOWN(index, 1 << order);
+ last_index = ALIGN(last_index, 1 << order);
+ }
Not sure I see the point of this. filemap_get_read_batch() returns any
folio which contains 'index'.
Does it? Cool. Then of course we don't need to align the index here.
retry:
if (fatal_signal_pending(current))
return -EINTR;
@@ -2561,7 +2574,7 @@ static int filemap_get_pages(struct kiocb *iocb, size_t count,
if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_WAITQ))
return -EAGAIN;
err = filemap_create_folio(filp, mapping,
- iocb->ki_pos >> PAGE_SHIFT, fbatch);
+ index, fbatch);
... ah, you align index here. I wonder if we wouldn't be better passing
iocb->ki_pos to filemap_create_folio() to emphasise that the caller
can't assume anything about the alignment/size of the folio.
I can check if that makes a difference.
@@ -3676,7 +3689,8 @@ static struct folio *do_read_cache_folio(struct address_space *mapping,
repeat:
folio = filemap_get_folio(mapping, index);
if (IS_ERR(folio)) {
- folio = filemap_alloc_folio(gfp, 0);
+ folio = filemap_alloc_folio(gfp,
+ mapping_min_folio_order(mapping));
if (!folio)
return ERR_PTR(-ENOMEM);
err = filemap_add_folio(mapping, folio, index, gfp);
This needs to align index.
Why, but of course. Will check.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
