Messages in this thread | | | Subject | Re: [RFC PATCH V3] iomap: add support to track dirty state of sub pages | From | "yukuai (C)" <> | Date | Thu, 20 Aug 2020 09:38:53 +0800 |
| |
On 2020/8/19 20:56, Gao Xiang wrote: > On Wed, Aug 19, 2020 at 08:05:42PM +0800, Yu Kuai wrote: > > ... > >> +static void >> +iomap_iop_set_range_dirty(struct page *page, unsigned int off, >> + unsigned int len) >> +{ >> + struct iomap_page *iop = to_iomap_page(page); >> + struct inode *inode = page->mapping->host; >> + unsigned int first = DIRTY_BITS(off >> inode->i_blkbits); >> + unsigned int last = DIRTY_BITS((off + len - 1) >> inode->i_blkbits); >> + unsigned long flags; >> + unsigned int i; >> + >> + spin_lock_irqsave(&iop->state_lock, flags); >> + for (i = first; i <= last; i++) >> + set_bit(i, iop->state); >> + >> + if (last >= first) >> + iomap_set_page_dirty(page); > > set_page_dirty() in the atomic context? >
Hi,
You'are right, this shouldn't be inside spin_lock.
>> + >> + spin_unlock_irqrestore(&iop->state_lock, flags); >> +} >> + >> +static void >> +iomap_set_range_dirty(struct page *page, unsigned int off, >> + unsigned int len) >> +{ >> + if (PageError(page)) >> + return; >> + >> + if (page_has_private(page)) >> + iomap_iop_set_range_dirty(page, off, len); > > > I vaguely remembered iomap doesn't always set up PagePrivate. >
If so, maybe I should move iomap_set_page_dirty() to ioamp_set_range_dirty().
Thanks, Yu Kuai
> > @@ -705,7 +770,7 @@ __iomap_write_end(struct inode *inode, loff_t pos, unsigned 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; > } > > so here could be suspectable, but I might be wrong here since > I just take a quick look. > > Thanks, > Gao Xiang > > > . >
| |