Skip to content

Commit

Permalink
Add a new counter: z_sync_writes_cnt to znode to keep track of active…
Browse files Browse the repository at this point in the history
… sync writes

Do a commit when the current writeback is not intended to be synced and there are active sync writebacks
Do a commit when there is an active page writeback to potentially speed it up if it's a non-sync writeback

Signed-off-by: Shaan Nobee <sniper111@gmail.com>
  • Loading branch information
shaan1337 committed Dec 1, 2021
1 parent 3b39eba commit 8970d11
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 10 deletions.
2 changes: 1 addition & 1 deletion include/os/linux/zfs/sys/zfs_vnops_os.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ extern int zfs_space(znode_t *zp, int cmd, flock64_t *bfp, int flag,
extern int zfs_fid(struct inode *ip, fid_t *fidp);
extern int zfs_getpage(struct inode *ip, struct page *pl[], int nr_pages);
extern int zfs_putpage(struct inode *ip, struct page *pp,
struct writeback_control *wbc);
struct writeback_control *wbc, boolean_t for_sync);
extern int zfs_dirty_inode(struct inode *ip, int flags);
extern int zfs_map(struct inode *ip, offset_t off, caddr_t *addrp,
size_t len, unsigned long vm_flags);
Expand Down
1 change: 1 addition & 0 deletions include/sys/zfs_znode.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ typedef struct znode {
uint64_t z_size; /* file size (cached) */
uint64_t z_pflags; /* pflags (cached) */
uint32_t z_sync_cnt; /* synchronous open count */
uint32_t z_sync_writes_cnt; /* synchronous write count */
mode_t z_mode; /* mode (cached) */
kmutex_t z_acl_lock; /* acl data lock */
zfs_acl_t *z_acl_cached; /* cached acl */
Expand Down
1 change: 1 addition & 0 deletions module/os/freebsd/zfs/zfs_znode.c
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ zfs_znode_alloc(zfsvfs_t *zfsvfs, dmu_buf_t *db, int blksz,
zp->z_blksz = blksz;
zp->z_seq = 0x7A4653;
zp->z_sync_cnt = 0;
zp->z_sync_writes_cnt = 0;
#if __FreeBSD_version >= 1300139
atomic_store_ptr(&zp->z_cached_symlink, NULL);
#endif
Expand Down
1 change: 1 addition & 0 deletions module/os/linux/zfs/zfs_ctldir.c
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ zfsctl_inode_alloc(zfsvfs_t *zfsvfs, uint64_t id,
zp->z_pflags = 0;
zp->z_mode = 0;
zp->z_sync_cnt = 0;
zp->z_sync_writes_cnt = 0;
ip->i_generation = 0;
ip->i_ino = id;
ip->i_mode = (S_IFDIR | S_IRWXUGO);
Expand Down
22 changes: 18 additions & 4 deletions module/os/linux/zfs/zfs_vnops_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -3420,9 +3420,11 @@ zfs_putpage_commit_cb(void *arg)
* Push a page out to disk, once the page is on stable storage the
* registered commit callback will be run as notification of completion.
*
* IN: ip - page mapped for inode.
* pp - page to push (page is locked)
* wbc - writeback control data
* IN: ip - page mapped for inode.
* pp - page to push (page is locked)
* wbc - writeback control data
* for_sync - does the caller intend to wait synchronously for the page
* writeback to complete in this or a future call?
*
* RETURN: 0 if success
* error code if failure
Expand All @@ -3432,7 +3434,7 @@ zfs_putpage_commit_cb(void *arg)
*/
/* ARGSUSED */
int
zfs_putpage(struct inode *ip, struct page *pp, struct writeback_control *wbc)
zfs_putpage(struct inode *ip, struct page *pp, struct writeback_control *wbc, boolean_t for_sync)
{
znode_t *zp = ITOZ(ip);
zfsvfs_t *zfsvfs = ITOZSB(ip);
Expand Down Expand Up @@ -3530,6 +3532,9 @@ zfs_putpage(struct inode *ip, struct page *pp, struct writeback_control *wbc)
zfs_rangelock_exit(lr);

if (wbc->sync_mode != WB_SYNC_NONE) {
/* speed up the page writeback if it's in non-SYNC mode */
zil_commit(zfsvfs->z_log, zp->z_id);

if (PageWriteback(pp))
wait_on_page_bit(pp, PG_writeback);
}
Expand Down Expand Up @@ -3604,6 +3609,15 @@ zfs_putpage(struct inode *ip, struct page *pp, struct writeback_control *wbc)
* performance reasons.
*/
zil_commit(zfsvfs->z_log, zp->z_id);
} else if (!for_sync && zp->z_sync_writes_cnt > 0) {
/*
* If the caller does not intend to wait synchronously
* for this page writeback to complete and there are active
* synchronous calls on this file, do a commit so that
* the latter don't accidentally end up waiting for
* our writeback to complete.
*/
zil_commit(zfsvfs->z_log, zp->z_id);
}

ZFS_EXIT(zfsvfs);
Expand Down
1 change: 1 addition & 0 deletions module/os/linux/zfs/zfs_znode.c
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ zfs_znode_alloc(zfsvfs_t *zfsvfs, dmu_buf_t *db, int blksz,
zp->z_blksz = blksz;
zp->z_seq = 0x7A4653;
zp->z_sync_cnt = 0;
zp->z_sync_writes_cnt = 0;

zfs_znode_sa_init(zfsvfs, zp, db, obj_type, hdl);

Expand Down
47 changes: 42 additions & 5 deletions module/os/linux/zfs/zpl_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
*/
unsigned int zfs_fallocate_reserve_percent = 110;

typedef struct zpl_putpage_data {
struct address_space *mapping;
boolean_t for_sync;
} zpl_putpage_data_t;

static int
zpl_open(struct inode *ip, struct file *filp)
{
Expand Down Expand Up @@ -127,17 +132,29 @@ static int
zpl_fsync(struct file *filp, int datasync)
{
struct inode *inode = filp->f_mapping->host;
znode_t *zp = ITOZ(inode);
zfsvfs_t *zfsvfs = ITOZSB(inode);
cred_t *cr = CRED();
int error;
fstrans_cookie_t cookie;

ZPL_ENTER(zfsvfs);
ZPL_VERIFY_ZP(zp);
atomic_inc_32(&zp->z_sync_writes_cnt);
ZPL_EXIT(zfsvfs);

crhold(cr);
cookie = spl_fstrans_mark();
error = -zfs_fsync(ITOZ(inode), datasync, cr);
spl_fstrans_unmark(cookie);
crfree(cr);
ASSERT3S(error, <=, 0);

ZPL_ENTER(zfsvfs);
ZPL_VERIFY_ZP(zp);
atomic_dec_32(&zp->z_sync_writes_cnt);
ZPL_EXIT(zfsvfs);

return (error);
}

Expand All @@ -161,10 +178,17 @@ static int
zpl_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
{
struct inode *inode = filp->f_mapping->host;
znode_t *zp = ITOZ(inode);
zfsvfs_t *zfsvfs = ITOZSB(inode);
cred_t *cr = CRED();
int error;
fstrans_cookie_t cookie;

ZPL_ENTER(zfsvfs);
ZPL_VERIFY_ZP(zp);
atomic_inc_32(&zp->z_sync_writes_cnt);
ZPL_EXIT(zfsvfs);

error = filemap_write_and_wait_range(inode->i_mapping, start, end);
if (error)
return (error);
Expand All @@ -176,6 +200,11 @@ zpl_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
crfree(cr);
ASSERT3S(error, <=, 0);

ZPL_ENTER(zfsvfs);
ZPL_VERIFY_ZP(zp);
atomic_dec_32(&zp->z_sync_writes_cnt);
ZPL_EXIT(zfsvfs);

return (error);
}

Expand Down Expand Up @@ -651,14 +680,14 @@ zpl_readpages(struct file *filp, struct address_space *mapping,
static int
zpl_putpage(struct page *pp, struct writeback_control *wbc, void *data)
{
struct address_space *mapping = data;
zpl_putpage_data_t *dt = data;
fstrans_cookie_t cookie;

ASSERT(PageLocked(pp));
ASSERT(!PageWriteback(pp));

cookie = spl_fstrans_mark();
(void) zfs_putpage(mapping->host, pp, wbc);
(void) zfs_putpage(dt->mapping->host, pp, wbc, dt->for_sync);
spl_fstrans_unmark(cookie);

return (0);
Expand All @@ -685,8 +714,12 @@ zpl_writepages(struct address_space *mapping, struct writeback_control *wbc)
* we run it once in non-SYNC mode so that the ZIL gets all the data,
* and then we commit it all in one go.
*/
zpl_putpage_data_t data;
data.mapping = mapping;
data.for_sync = (sync_mode == WB_SYNC_ALL);

wbc->sync_mode = WB_SYNC_NONE;
result = write_cache_pages(mapping, wbc, zpl_putpage, mapping);
result = write_cache_pages(mapping, wbc, zpl_putpage, &data);
if (sync_mode != wbc->sync_mode) {
ZPL_ENTER(zfsvfs);
ZPL_VERIFY_ZP(zp);
Expand All @@ -702,7 +735,7 @@ zpl_writepages(struct address_space *mapping, struct writeback_control *wbc)
* details). That being said, this is a no-op in most cases.
*/
wbc->sync_mode = sync_mode;
result = write_cache_pages(mapping, wbc, zpl_putpage, mapping);
result = write_cache_pages(mapping, wbc, zpl_putpage, &data);
}
return (result);
}
Expand All @@ -719,7 +752,11 @@ zpl_writepage(struct page *pp, struct writeback_control *wbc)
if (ITOZSB(pp->mapping->host)->z_os->os_sync == ZFS_SYNC_ALWAYS)
wbc->sync_mode = WB_SYNC_ALL;

return (zpl_putpage(pp, wbc, pp->mapping));
zpl_putpage_data_t data;
data.mapping = pp->mapping;
data.for_sync = (wbc->sync_mode == WB_SYNC_ALL);

return (zpl_putpage(pp, wbc, &data));
}

/*
Expand Down

0 comments on commit 8970d11

Please sign in to comment.