| 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
| ||
|
Message-ID: <a87e93cc-6faf-64fb-add8-39bfcd7febba@huawei.com>
Date: Fri, 11 Sep 2020 16:35:19 +0800
From: "yukuai (C)" <yukuai3@...wei.com>
To: Matthew Wilcox <willy@...radead.org>
CC: <hch@...radead.org>, <darrick.wong@...cle.com>,
<david@...morbit.com>, <linux-xfs@...r.kernel.org>,
<linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<yi.zhang@...wei.com>
Subject: Re: [RFC PATCH V4] iomap: add support to track dirty state of sub
pages
Hi,
Sorry that after copy and paste, the content of the patch somehow
changed and looks strange.
Best regards,
Yu Kuai
On 2020/09/11 16:27, yukuai (C) wrote:
> On 2020/08/21 20:44, Matthew Wilcox wrote:
>> On Fri, Aug 21, 2020 at 08:33:06PM +0800, Yu Kuai wrote:
>>> changes from v3: - add IOMAP_STATE_ARRAY_SIZE - replace set_bit /
>>> clear_bit with bitmap_set / bitmap_clear - move
>>> iomap_set_page_dirty() out of 'iop->state_lock' - merge
>>> iomap_set/clear_range_dirty() and iomap_iop_clear/clear_range_dirty()
>>
>> I'm still working on the iomap parts of the THP series (fixing up
>> invalidatepage right now), but here are some of the relevant bits
>> (patch series to follow)
>>
>
> Hi, Matthew
>
> Since your THP iomap patches were reviewed, I made some modifications
> based on these patches.
>
> Best regards,
> Yu Kuai
>
> ---
> fs/iomap/buffered-io.c | 92 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 74 insertions(+), 18 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index edf5eea56cf5..bc7f57748be8 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -23,13 +23,17 @@
>
> /*
> * Structure allocated for each page or THP when block size < page size
> - * to track sub-page uptodate status and I/O completions.
> + * to track sub-page status and I/O completions.
> */
> struct iomap_page {
> atomic_t read_bytes_pending;
> atomic_t write_bytes_pending;
> - spinlock_t uptodate_lock;
> - unsigned long uptodate[];
> + spinlock_t state_lock;
> + /*
> + * The first half bits are used to track sub-page uptodate status,
> + * the second half bits are for dirty status.
> + */
> + unsigned long state[];
> };
>
> static inline struct iomap_page *to_iomap_page(struct page *page)
> @@ -57,9 +61,9 @@ iomap_page_create(struct inode *inode, struct page *page)
> if (iop || nr_blocks <= 1)
> return iop;
>
> - iop = kzalloc(struct_size(iop, uptodate, BITS_TO_LONGS(nr_blocks)),
> + iop = kzalloc(struct_size(iop, state, BITS_TO_LONGS(2 * nr_blocks)),
> GFP_NOFS | __GFP_NOFAIL);
> - spin_lock_init(&iop->uptodate_lock);
> + spin_lock_init(&iop->state_lock);
> attach_page_private(page, iop);
> return iop;
> }
> @@ -74,7 +78,7 @@ iomap_page_release(struct page *page)
> return;
> WARN_ON_ONCE(atomic_read(&iop->read_bytes_pending));
> WARN_ON_ONCE(atomic_read(&iop->write_bytes_pending));
> - WARN_ON_ONCE(bitmap_full(iop->uptodate, nr_blocks) !=
> + WARN_ON_ONCE(bitmap_full(iop->state, nr_blocks) !=
> PageUptodate(page));
> kfree(iop);
> }
> @@ -105,7 +109,7 @@ iomap_adjust_read_range(struct inode *inode, struct
> iomap_page *iop,
>
> /* move forward for each leading block marked uptodate */
> for (i = first; i <= last; i++) {
> - if (!test_bit(i, iop->uptodate))
> + if (!test_bit(i, iop->state))
> break;
> *pos += block_size;
> poff += block_size;
> @@ -115,7 +119,7 @@ iomap_adjust_read_range(struct inode *inode, struct
> iomap_page *iop,
>
> /* truncate len if we find any trailing uptodate block(s) */
> for ( ; i <= last; i++) {
> - if (test_bit(i, iop->uptodate)) {
> + if (test_bit(i, iop->state)) {
> plen -= (last - i + 1) * block_size;
> last = i - 1;
> break;
> @@ -139,6 +143,55 @@ iomap_adjust_read_range(struct inode *inode, struct
> iomap_page *iop,
> *lenp = plen;
> }
>
> +static void
> +iomap_set_range_dirty(struct page *page, unsigned int off,
> + unsigned int len)
> +{
> + struct inode *inode = page->mapping->host;
> + unsigned int blocks_per_page = i_blocks_per_page(inode, page);
> + unsigned int first = (off >> inode->i_blkbits) + blocks_per_page;
> + unsigned int last = ((off + len - 1) >> inode->i_blkbits) +
> blocks_per_page;
> + unsigned long flags;
> + struct iomap_page *iop;
> +
> + if (PageError(page))
> + return;
> +
> + if (len)
> + iomap_set_page_dirty(page);
> +
> + if (!page_has_private(page))
> + return;
> +
> + iop = to_iomap_page(page);
> + spin_lock_irqsave(&iop->state_lock, flags);
> + bitmap_set(iop->state, first, last - first + 1);
> + spin_unlock_irqrestore(&iop->state_lock, flags);
> +}
> +
> +static void
> +iomap_clear_range_dirty(struct page *page, unsigned int off,
> + unsigned int len)
> +{
> + struct inode *inode = page->mapping->host;
> + unsigned int blocks_per_page = i_blocks_per_page(inode, page);
> + unsigned int first = (off >> inode->i_blkbits) + blocks_per_page;
> + unsigned int last = ((off + len - 1) >> inode->i_blkbits) +
> blocks_per_page;
> + unsigned long flags;
> + struct iomap_page *iop;
> +
> + if (PageError(page))
> + return;
> +
> + if (!page_has_private(page))
> + return;
> +
> + iop = to_iomap_page(page);
> + spin_lock_irqsave(&iop->state_lock, flags);
> + bitmap_clear(iop->state, first, last - first + 1);
> + spin_unlock_irqrestore(&iop->state_lock, flags);
> +}
> +
> static void
> iomap_iop_set_range_uptodate(struct page *page, unsigned off, unsigned
> len)
> {
> @@ -148,11 +201,11 @@ iomap_iop_set_range_uptodate(struct page *page,
> unsigned off, unsigned len)
> unsigned last = (off + len - 1) >> inode->i_blkbits;
> unsigned long flags;
>
> - spin_lock_irqsave(&iop->uptodate_lock, flags);
> - bitmap_set(iop->uptodate, first, last - first + 1);
> - if (bitmap_full(iop->uptodate, i_blocks_per_page(inode, page)))
> + spin_lock_irqsave(&iop->state_lock, flags);
> + bitmap_set(iop->state, first, last - first + 1);
> + if (bitmap_full(iop->state, i_blocks_per_page(inode, page)))
> SetPageUptodate(page);
> - spin_unlock_irqrestore(&iop->uptodate_lock, flags);
> + spin_unlock_irqrestore(&iop->state_lock, flags);
> }
>
> static void
> @@ -445,7 +498,7 @@ iomap_is_partially_uptodate(struct page *page,
> unsigned long from,
>
> if (iop) {
> for (i = first; i <= last; i++)
> - if (!test_bit(i, iop->uptodate))
> + if (!test_bit(i, iop->state))
> return 0;
> return 1;
> }
> @@ -683,7 +736,7 @@ static size_t __iomap_write_end(struct inode *inode,
> loff_t pos, size_t len,
> if (unlikely(copied < len && !PageUptodate(page)))
> return 0;
> iomap_set_range_uptodate(page, offset_in_page(pos), len);
> - iomap_set_page_dirty(page);
> + iomap_set_range_dirty(page, offset_in_page(pos), len);
> return copied;
> }
>
> @@ -997,7 +1050,6 @@ iomap_page_mkwrite_actor(struct inode *inode,
> loff_t pos, loff_t length,
> } else {
> WARN_ON_ONCE(!PageUptodate(page));
> iomap_page_create(inode, page);
> - set_page_dirty(page);
> }
>
> return length;
> @@ -1007,7 +1059,7 @@ vm_fault_t iomap_page_mkwrite(struct vm_fault
> *vmf, const struct iomap_ops *ops)
> {
> struct page *page = vmf->page;
> struct inode *inode = file_inode(vmf->vma->vm_file);
> - unsigned long length;
> + unsigned int length, dirty_bits;
> loff_t offset;
> ssize_t ret;
>
> @@ -1016,6 +1068,7 @@ vm_fault_t iomap_page_mkwrite(struct vm_fault
> *vmf, const struct iomap_ops *ops)
> if (ret < 0)
> goto out_unlock;
> length = ret;
> + dirty_bits = ret;
>
> offset = page_offset(page);
> while (length > 0) {
> @@ -1028,6 +1081,7 @@ vm_fault_t iomap_page_mkwrite(struct vm_fault
> *vmf, const struct iomap_ops *ops)
> length -= ret;
> }
>
> + iomap_set_range_dirty(page, 0, dirty_bits);
> wait_for_stable_page(page);
> return VM_FAULT_LOCKED;
> out_unlock:
> @@ -1340,11 +1394,12 @@ iomap_writepage_map(struct iomap_writepage_ctx
> *wpc,
> struct iomap_page *iop = to_iomap_page(page);
> struct iomap_ioend *ioend, *next;
> unsigned len = i_blocksize(inode);
> + unsigned int blocks_per_page = i_blocks_per_page(inode, page);
> u64 file_offset; /* file offset of page */
> int error = 0, count = 0, i;
> LIST_HEAD(submit_list);
>
> - WARN_ON_ONCE(i_blocks_per_page(inode, page) > 1 && !iop);
> + WARN_ON_ONCE(blocks_per_page > 1 && !iop);
> WARN_ON_ONCE(iop && atomic_read(&iop->write_bytes_pending) != 0);
>
> /*
> @@ -1355,7 +1410,7 @@ iomap_writepage_map(struct iomap_writepage_ctx *wpc,
> for (i = 0, file_offset = page_offset(page);
> i < (PAGE_SIZE >> inode->i_blkbits) && file_offset < end_offset;
> i++, file_offset += len) {
> - if (iop && !test_bit(i, iop->uptodate))
> + if (iop && !test_bit(i, iop->state + blocks_per_page))
> continue;
>
> error = wpc->ops->map_blocks(wpc, inode, file_offset);
> @@ -1404,6 +1459,7 @@ iomap_writepage_map(struct iomap_writepage_ctx *wpc,
> */
> set_page_writeback_keepwrite(page);
> } else {
> + iomap_clear_range_dirty(page, 0, PAGE_SIZE);
> clear_page_dirty_for_io(page);
> set_page_writeback(page);
> }
Powered by blists - more mailing lists