| Message ID | 20190606014544.8339-8-ira.weiny@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | RDMA/FS DAX truncate proposal | expand |
On Wed 05-06-19 18:45:40, ira.weiny@intel.com wrote: > From: Ira Weiny <ira.weiny@intel.com> > > If pages are actively gup pinned fail the truncate operation. > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > fs/ext4/inode.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 75f543f384e4..1ded83ec08c0 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -4250,6 +4250,9 @@ int ext4_break_layouts(struct inode *inode, loff_t offset, loff_t len) > if (!page) > return 0; > > + if (page_gup_pinned(page)) > + return -ETXTBSY; > + > error = ___wait_var_event(&page->_refcount, > atomic_read(&page->_refcount) == 1, > TASK_INTERRUPTIBLE, 0, 0, This caught my eye. Does this mean that now truncate for a file which has temporary gup users (such buffers for DIO) can fail with ETXTBUSY? That doesn't look desirable. If we would mandate layout lease while pages are pinned as I suggested, this could be dealt with by checking for leases with pins (breaking such lease would return error and not break it) and if breaking leases succeeds (i.e., there are no long-term pinned pages), we'd just wait for the remaining references as we do now. Honza
On Thu, Jun 06, 2019 at 12:58:55PM +0200, Jan Kara wrote: > On Wed 05-06-19 18:45:40, ira.weiny@intel.com wrote: > > From: Ira Weiny <ira.weiny@intel.com> > > > > If pages are actively gup pinned fail the truncate operation. > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > --- > > fs/ext4/inode.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > > index 75f543f384e4..1ded83ec08c0 100644 > > --- a/fs/ext4/inode.c > > +++ b/fs/ext4/inode.c > > @@ -4250,6 +4250,9 @@ int ext4_break_layouts(struct inode *inode, loff_t offset, loff_t len) > > if (!page) > > return 0; > > > > + if (page_gup_pinned(page)) > > + return -ETXTBSY; > > + > > error = ___wait_var_event(&page->_refcount, > > atomic_read(&page->_refcount) == 1, > > TASK_INTERRUPTIBLE, 0, 0, > > This caught my eye. Does this mean that now truncate for a file which has > temporary gup users (such buffers for DIO) can fail with ETXTBUSY? I thought about that before and I _thought_ I had accounted for it. But I think you are right... > > That > doesn't look desirable. No not desirable at all... Ah it just dawned on my why I thought it was ok... I was wrong. :-/ > If we would mandate layout lease while pages are > pinned as I suggested, this could be dealt with by checking for leases with > pins (breaking such lease would return error and not break it) and if > breaking leases succeeds (i.e., there are no long-term pinned pages), we'd > just wait for the remaining references as we do now. Agreed. But I'm going to respond with some of the challenges of this (and ideas I had) when replying to your other email. Ira > > Honza > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 75f543f384e4..1ded83ec08c0 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -4250,6 +4250,9 @@ int ext4_break_layouts(struct inode *inode, loff_t offset, loff_t len) if (!page) return 0; + if (page_gup_pinned(page)) + return -ETXTBSY; + error = ___wait_var_event(&page->_refcount, atomic_read(&page->_refcount) == 1, TASK_INTERRUPTIBLE, 0, 0,