linux-kernel - Re: [PATCH v2 19/28] iomap: Convert __iomap_zero

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: <YbN+KqqCG0032NMG@casper.infradead.org>
Date:   Fri, 10 Dec 2021 16:19:54 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     "Darrick J . Wong " <djwong@...nel.org>
Cc:     linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-block@...r.kernel.org,
        Jens Axboe <axboe@...nel.dk>,
        Christoph Hellwig <hch@...radead.org>
Subject: Re: [PATCH v2 19/28] iomap: Convert __iomap_zero_iter to use a folio

On Thu, Dec 09, 2021 at 09:38:03PM +0000, Matthew Wilcox wrote:
> @@ -891,16 +893,19 @@ static s64 __iomap_zero_iter(struct iomap_iter *iter, loff
> _t pos, u64 length)
>         struct page *page;
>         int status;
>         unsigned offset = offset_in_page(pos);
> -       unsigned bytes = min_t(u64, PAGE_SIZE - offset, length);
> 
> -       status = iomap_write_begin(iter, pos, bytes, &page);
> +       if (length > UINT_MAX)
> +               length = UINT_MAX;
> +       status = iomap_write_begin(iter, pos, length, &page);
>         if (status)
>                 return status;
> +       if (length > PAGE_SIZE - offset)
> +               length = PAGE_SIZE - offset;
> 
> -       zero_user(page, offset, bytes);
> +       zero_user(page, offset, length);
>         mark_page_accessed(page);
> 
> -       return iomap_write_end(iter, pos, bytes, bytes, page);
> +       return iomap_write_end(iter, pos, length, length, page);
>  }

After attempting the merge with Christoph's ill-timed refactoring,
I decided that eliding the use of 'bytes' here was the wrong approach,
because it very much needs to be put back in for the merge.

Here's the merge as I have it:

diff --cc fs/iomap/buffered-io.c
index f3176cf90351,d1aa0f0e7fd5..40356db3e856
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@@ -888,19 -926,12 +904,23 @@@ static loff_t iomap_zero_iter(struct io
                return length;

        do {
-               unsigned offset = offset_in_page(pos);
-               size_t bytes = min_t(u64, PAGE_SIZE - offset, length);
-               struct page *page;
 -              s64 bytes;
++              struct folio *folio;
 +              int status;
++              size_t offset;
++              size_t bytes = min_t(u64, SIZE_MAX, length);
 +
-               status = iomap_write_begin(iter, pos, bytes, &page);
++              status = iomap_write_begin(iter, pos, bytes, &folio);
 +              if (status)
 +                      return status;
 +
-               zero_user(page, offset, bytes);
-               mark_page_accessed(page);
++              offset = offset_in_folio(folio, pos);
++              if (bytes > folio_size(folio) - offset)
++                      bytes = folio_size(folio) - offset;
++
++              folio_zero_range(folio, offset, bytes);
++              folio_mark_accessed(folio);

-               bytes = iomap_write_end(iter, pos, bytes, bytes, page);
 -              if (IS_DAX(iter->inode))
 -                      bytes = dax_iomap_zero(pos, length, iomap);
 -              else
 -                      bytes = __iomap_zero_iter(iter, pos, length);
++              bytes = iomap_write_end(iter, pos, bytes, bytes, folio);
                if (bytes < 0)
                        return bytes;

I've pushed out a new tag:

https://git.infradead.org/users/willy/linux.git/shortlog/refs/tags/iomap-folio-5.17d