[Cluster-devel] [GFS2 PATCH] GFS2: Add to tail, not head, of transaction

2018-05-30 Thread Bob Peterson
Hi,

Before this patch, frunction gfs2_trans_add_meta called list_add
to add a buffer to a transaction, tr_buf. Later, in the before_commit
functions, it traversed the list in sequential order, which meant
that they were processed in a sub-optimal order. For example, blocks
could go out in 54321 order rather than 12345, causing media heads
to bounce unnecessarily.

This makes no difference for small IO operations, but large writes
benefit greatly when they add lots of indirect blocks to a
transaction, and those blocks are allocated in ascending order,
as the block allocator tries to do.

This patch changes it to list_add_tail so they are traversed (and
therefore written back) in the same order as they are added to
the transaction.

In one of the more extreme examples, I did a test where I had
10 simultaneous instances of iozone, each writing 25GB of data,
using one of our performance testing machines. The results are:

 Without the patch   With the patch
 -   --
Children see throughput  1395068.00 kB/sec   1527073.61 kB/sec
Parent sees throughput   1348544.00 kB/sec   1485594.66 kB/sec

These numbers are artificially inflated because I was also running
with Andreas Gruenbacher's iomap-write patch set plus my rgrp
sharing patch set in both these cases. Still, it shows a 9 percent
performance boost for both children and parent throughput.

Signed-off-by: Bob Peterson 
---
 fs/gfs2/trans.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index c75cacaa349b..c4c00b90935c 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -247,7 +247,7 @@ void gfs2_trans_add_meta(struct gfs2_glock *gl, struct 
buffer_head *bh)
gfs2_pin(sdp, bd->bd_bh);
mh->__pad0 = cpu_to_be64(0);
mh->mh_jid = cpu_to_be32(sdp->sd_jdesc->jd_jid);
-   list_add(>bd_list, >tr_buf);
+   list_add_tail(>bd_list, >tr_buf);
tr->tr_num_buf_new++;
 out_unlock:
gfs2_log_unlock(sdp);



[Cluster-devel] [GFS2 PATCH] GFS2: rgrp free blocks used incorrectly

2018-05-30 Thread Bob Peterson
Hi,

Before this patch, several functions in rgrp.c checked the value of
rgd->rd_free_clone. That does not take into account blocks that were
reserved by a multi-block reservation. This causes a problem when
space gets tight in the file system. For example, when function
gfs2_inplace_reserve checks to see if a rgrp has enough blocks to
satisfy the request, it can accept a rgrp that it should reject
because, although there are enough blocks to satisfy the request
_now_, those blocks may be reserved for another running process.

A second problem with this occurs when we've reserved the remaining
blocks in an rgrp: function rg_mblk_search() can reject an rgrp
improperly because it calculates:

   u32 free_blocks = rgd->rd_free_clone - rgd->rd_reserved;

But rd_reserved includes blocks that the current process just
reserved in its own call to inplace_reserve. For example, it can
reserve the last 128 blocks of an rgrp, then reject that same rgrp
because the above calculates out to free_blocks = 0;

Consequences include, but are not limited to, (1) leaving holes,
and thus increasing file system fragmentation, and (2) reporting
file system is full long before it actually is.

This patch introduces a new function, rgd_free, which returns the
number of clone-free blocks (blocks that are truly free as opposed
to blocks that are still being used because an unlinked file is
still open) minus the number of blocks reserved by processes, but
not counting the blocks we ourselves reserved (because obviously
we need to allocate them).

Signed-off-by: Bob Peterson 
---
 fs/gfs2/rgrp.c | 33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 8b683917a27e..8be47b81011a 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1487,6 +1487,31 @@ static void rs_insert(struct gfs2_inode *ip)
trace_gfs2_rs(rs, TRACE_RS_INSERT);
 }
 
+/**
+ * rgd_free - return the number of free blocks we can allocate.
+ * @rgd: the resource group
+ *
+ * This function returns the number of free blocks for an rgrp.
+ * That's the clone-free blocks (blocks that are free, not including those
+ * still being used for unlinked files that haven't been deleted.)
+ *
+ * It also subtracts any blocks reserved by someone else, but does not
+ * include free blocks that are still part of our current reservation,
+ * because obviously we can (and will) allocate them.
+ */
+static inline u32 rgd_free(struct gfs2_rgrpd *rgd, struct gfs2_blkreserv *rs)
+{
+   u32 tot_reserved, tot_free;
+
+   BUG_ON(rgd->rd_reserved < rs->rs_free);
+   tot_reserved = rgd->rd_reserved - rs->rs_free;
+
+   BUG_ON(rgd->rd_free_clone < tot_reserved);
+   tot_free = rgd->rd_free_clone - tot_reserved;
+
+   return tot_free;
+}
+
 /**
  * rg_mblk_search - find a group of multiple free blocks to form a reservation
  * @rgd: the resource group descriptor
@@ -1502,7 +1527,7 @@ static void rg_mblk_search(struct gfs2_rgrpd *rgd, struct 
gfs2_inode *ip,
u64 goal;
struct gfs2_blkreserv *rs = >i_res;
u32 extlen;
-   u32 free_blocks = rgd->rd_free_clone - rgd->rd_reserved;
+   u32 free_blocks = rgd_free(rgd, rs);
int ret;
struct inode *inode = >i_inode;
 
@@ -2053,11 +2078,11 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct 
gfs2_alloc_parms *ap)
goto check_rgrp;
 
/* If rgrp has enough free space, use it */
-   if (rs->rs_rbm.rgd->rd_free_clone >= ap->target ||
+   if (rgd_free(rs->rs_rbm.rgd, rs) >= ap->target ||
(loops == 2 && ap->min_target &&
-rs->rs_rbm.rgd->rd_free_clone >= ap->min_target)) {
+rgd_free(rs->rs_rbm.rgd, rs) >= ap->min_target)) {
ip->i_rgd = rs->rs_rbm.rgd;
-   ap->allowed = ip->i_rgd->rd_free_clone;
+   ap->allowed = rgd_free(rs->rs_rbm.rgd, rs);
return 0;
}
 check_rgrp:



[Cluster-devel] [GFS2 PATCH] GFS2: Fix allocation error bug with recursive rgrp glocking

2018-05-30 Thread Bob Peterson
Hi,

Before this patch function gfs2_write_begin, upon discovering an
error, called gfs2_trim_blocks while the rgrp glock was still held.
That's because gfs2_inplace_release is not called until later.
This patch reorganizes the logic a bit so gfs2_inplace_release
is called to release the lock prior to the call to gfs2_trim_blocks,
thus preventing the glock recursion.

Signed-off-by: Bob Peterson 
---
 fs/gfs2/aops.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index f58716567972..629e614b324c 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -746,16 +746,13 @@ static int gfs2_write_begin(struct file *file, struct 
address_space *mapping,
unlock_page(page);
put_page(page);
 
-   gfs2_trans_end(sdp);
-   if (pos + len > ip->i_inode.i_size)
-   gfs2_trim_blocks(>i_inode);
-   goto out_trans_fail;
-
 out_endtrans:
gfs2_trans_end(sdp);
 out_trans_fail:
if (alloc_required) {
gfs2_inplace_release(ip);
+   if (pos + len > ip->i_inode.i_size)
+   gfs2_trim_blocks(>i_inode);
 out_qunlock:
gfs2_quota_unlock(ip);
}



Re: [Cluster-devel] [PATCH v5 00/14] gfs2 iomap write support

2018-05-30 Thread Bob Peterson
Hi,

- Original Message -
> Here's an update of my gfs2 iomap write patch queue, with support for
> buffered writes as well as direct I/O reads and writes through iomap.
> This update fixes a few bugs in the gfs2 specific parts.  The patches
> are still meant for the upcoming merge window.
> 
> The first five patches are minor gfs2 specific cleanups and
> improvements.

If this is true, I propose that we treat the first 5 patches in the
series as separate individual patches and push them accordingly.
Then we can treat the subsequent 9 patches as the gfs2 iomap-write set.
Do you see any reason why we can't treat them as separate patches?

Regards,

Bob Peterson
Red Hat File Systems



Re: [Cluster-devel] [RHEL6.10 PATCH] GFS2: allow gfs2_write_calc_reserv to use minimum values for rindex

2018-05-30 Thread Bob Peterson
Hi,

Please ignore the last email. I sent it to the wrong list.
This is a rhel6 patch, not meant for the development kernel tree.

Bob Peterson

- Original Message -
> Hi,
> 
> QE found problems with the previous patch for bug #1384184.
> This replacement patch reverts the first patch and adds new code
> that passes the QE tests.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1384184
> Upstream Status: N/A (not needed upstream)
> Build Info:
> https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=16448851
> Tested: by Justin Payne on virt host-029
> 
> GFS2: allow gfs2_write_calc_reserv to use minimum values for rindex
> 
> Before this patch function gfs2_write_calc_reserv built in several
> "slop" blocks into its calculations for the number of blocks required
> to do a write. This was fine for the most part, but when the file
> system is down to its last blocks, and needs to be extended with
> gfs2_grow, the rindex file needs to use a minimum amount of blocks.
> This patch makes a special exception for the rindex file, which
> now calculates a bare minimum number of blocks we need to allocate.
> 
> Signed-off-by: Bob Peterson 



[Cluster-devel] [RHEL6.10 PATCH] GFS2: allow gfs2_write_calc_reserv to use minimum values for rindex

2018-05-30 Thread Bob Peterson
Hi,

QE found problems with the previous patch for bug #1384184.
This replacement patch reverts the first patch and adds new code
that passes the QE tests.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1384184
Upstream Status: N/A (not needed upstream)
Build Info: https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=16448851
Tested: by Justin Payne on virt host-029

GFS2: allow gfs2_write_calc_reserv to use minimum values for rindex

Before this patch function gfs2_write_calc_reserv built in several
"slop" blocks into its calculations for the number of blocks required
to do a write. This was fine for the most part, but when the file
system is down to its last blocks, and needs to be extended with
gfs2_grow, the rindex file needs to use a minimum amount of blocks.
This patch makes a special exception for the rindex file, which
now calculates a bare minimum number of blocks we need to allocate.

Signed-off-by: Bob Peterson 
---
 fs/gfs2/bmap.c |  2 +-
 fs/gfs2/bmap.h | 13 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 2d2b14371b72..9e6d6775fbf7 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -1375,7 +1375,7 @@ int gfs2_write_alloc_required(struct gfs2_inode *ip, u64 
offset,
 
*alloc_required = 0;
 
-   if (!len || >i_inode == sdp->sd_rindex)
+   if (!len)
return 0;
 
if (gfs2_is_stuffed(ip)) {
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index c68252a61a26..191f5df092f2 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -35,6 +35,19 @@ static inline void gfs2_write_calc_reserv(const struct 
gfs2_inode *ip,
unsigned int tmp;
 
BUG_ON(gfs2_is_dir(ip));
+   if (>i_inode == sdp->sd_rindex) {
+   const u64 *arr = sdp->sd_heightsize;
+   int h = ip->i_height;
+
+   *data_blocks = (len + sdp->sd_sb.sb_bsize -
+   1) >> sdp->sd_sb.sb_bsize_shift;
+   *ind_blocks = 0;
+   while (i_size_read(>i_inode) + len > arr[h]) {
+   h++;
+   (*ind_blocks)++;
+   }
+   return;
+   }
*data_blocks = (len >> sdp->sd_sb.sb_bsize_shift) + 3;
*ind_blocks = 3 * (sdp->sd_max_height - 1);
 



Re: [Cluster-devel] [PATCH 11/34] iomap: move IOMAP_F_BOUNDARY to gfs2

2018-05-30 Thread Andreas Gruenbacher
On 30 May 2018 at 12:12, Steven Whitehouse  wrote:
> Hi,
>
> On 30/05/18 11:10, Christoph Hellwig wrote:
>>
>> On Wed, May 30, 2018 at 11:02:08AM +0100, Steven Whitehouse wrote:
>>>
>>> In that case,  maybe it would be simpler to drop it for GFS2. Unless we
>>> are getting a lot of benefit from it, then we should probably just follow
>>> the generic pattern here. Eventually we'll move everything to iomap, so
>>> that the bh mapping interface will be gone. That implies that we might be
>>> able to drop it now, to avoid this complication during the conversion.
>>>
>>> Andreas, do you see any issues with that?

We're not handling reads through iomap yet, so I'd be happier with
keeping that flag in one form or the other until we get there. This
will go away eventually anyway.

>> I suspect it actually is doing the wrong thing today.  It certainly
>> does for SSDs, and it probably doesn't do a useful thing for modern
>> disks with intelligent caches either.
>
>
> Yes, agreed that it makes no sense for SSDs,

Thanks,
Andreas



Re: [Cluster-devel] [PATCH] gfs2-utils: Fix bashism in test script

2018-05-30 Thread Andrew Price

On 30/05/18 08:09, Valentin Vidic wrote:

Tests fail when /bin/sh is not /bin/bash due to missing let command:

  21: Values of rg_data0, rg_data, rg_bitbytesFAILED (mkfs.at:146)
  /<>/tests/rgrifieldscheck.sh: 19: let: not found


Applied - thanks!

Andy


---
  tests/rgrifieldscheck.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/rgrifieldscheck.sh b/tests/rgrifieldscheck.sh
index 85f6408e..1fbf6729 100755
--- a/tests/rgrifieldscheck.sh
+++ b/tests/rgrifieldscheck.sh
@@ -16,5 +16,5 @@ do
exit 1
fi
  
-	test $field = ri_bitbytes && let i++

+   test $field = ri_bitbytes && i=$((i+1))
  done





Re: [Cluster-devel] [PATCH 11/34] iomap: move IOMAP_F_BOUNDARY to gfs2

2018-05-30 Thread Steven Whitehouse

Hi,


On 30/05/18 11:10, Christoph Hellwig wrote:

On Wed, May 30, 2018 at 11:02:08AM +0100, Steven Whitehouse wrote:

In that case,  maybe it would be simpler to drop it for GFS2. Unless we
are getting a lot of benefit from it, then we should probably just follow
the generic pattern here. Eventually we'll move everything to iomap, so
that the bh mapping interface will be gone. That implies that we might be
able to drop it now, to avoid this complication during the conversion.

Andreas, do you see any issues with that?

I suspect it actually is doing the wrong thing today.  It certainly
does for SSDs, and it probably doesn't do a useful thing for modern
disks with intelligent caches either.


Yes, agreed that it makes no sense for SSDs,

Steve.





Re: [Cluster-devel] [PATCH 11/34] iomap: move IOMAP_F_BOUNDARY to gfs2

2018-05-30 Thread Christoph Hellwig
On Wed, May 30, 2018 at 11:02:08AM +0100, Steven Whitehouse wrote:
> In that case,  maybe it would be simpler to drop it for GFS2. Unless we 
> are getting a lot of benefit from it, then we should probably just follow 
> the generic pattern here. Eventually we'll move everything to iomap, so 
> that the bh mapping interface will be gone. That implies that we might be 
> able to drop it now, to avoid this complication during the conversion.
>
> Andreas, do you see any issues with that?

I suspect it actually is doing the wrong thing today.  It certainly
does for SSDs, and it probably doesn't do a useful thing for modern
disks with intelligent caches either.



Re: [Cluster-devel] [PATCH 11/34] iomap: move IOMAP_F_BOUNDARY to gfs2

2018-05-30 Thread Steven Whitehouse

Hi,


On 30/05/18 10:59, Christoph Hellwig wrote:

On Wed, May 30, 2018 at 10:30:32AM +0100, Steven Whitehouse wrote:

I may have missed the context here, but I thought that the boundary was a
generic thing meaning "there will have to be a metadata read before more
blocks can be mapped" so I'm not sure why that would now be GFS2 specific?

It was always a hack.  But with iomap it doesn't make any sensee to start
with, all metadata I/O happens in iomap_begin, so there is no point in
marking an iomap with flags like this for the actual iomap interface.


In that case,  maybe it would be simpler to drop it for GFS2. Unless we 
are getting a lot of benefit from it, then we should probably just 
follow the generic pattern here. Eventually we'll move everything to 
iomap, so that the bh mapping interface will be gone. That implies that 
we might be able to drop it now, to avoid this complication during the 
conversion.


Andreas, do you see any issues with that?

Steve.





Re: [Cluster-devel] [PATCH 11/34] iomap: move IOMAP_F_BOUNDARY to gfs2

2018-05-30 Thread Christoph Hellwig
On Wed, May 30, 2018 at 10:30:32AM +0100, Steven Whitehouse wrote:
> I may have missed the context here, but I thought that the boundary was a 
> generic thing meaning "there will have to be a metadata read before more 
> blocks can be mapped" so I'm not sure why that would now be GFS2 specific?

It was always a hack.  But with iomap it doesn't make any sensee to start
with, all metadata I/O happens in iomap_begin, so there is no point in
marking an iomap with flags like this for the actual iomap interface.



Re: [Cluster-devel] [PATCH v5 14/14] gfs2: Handle stuffed files in iomap_{begin, end}

2018-05-30 Thread Christoph Hellwig
> + if (gfs2_is_stuffed(ip)) {
> + unstuff = pos + length > gfs2_max_stuffed_size(ip);
> +
> + if (!unstuff) {
> + iomap->page = grab_cache_page_write_begin(
> + inode->i_mapping, 0, flags);
> + ret = -ENOMEM;
> + if (!iomap->page)
> + goto out;
> +
> + if (!PageUptodate(iomap->page)) {
> + ret = stuffed_readpage(ip, iomap->page);
> + if (ret)
> + goto out;
> + }
> + }
> + }

grab_cache_page_write_begin needs to remain in iomap.  Please just
set up a pointer for later copying here.

This will be important to support e.g. iomap_readpage(s) and to easily
support direct I/O from inline data (just copy to/from the stuffed
data pointer in iomap).



[Cluster-devel] [PATCH v5 10/14] iomap: Complete partial direct I/O writes synchronously

2018-05-30 Thread Andreas Gruenbacher
According to xfstest generic/240, applications seem to expect direct I/O
writes to either complete as a whole or to fail; short direct I/O writes
are apparently not appreciated.  This means that when only part of an
asynchronous direct I/O write succeeds, we can either fail the entire
write, or we can wait for the partial write to complete and retry the
remaining write using buffered I/O.  The old __blockdev_direct_IO helper
has code for waiting for partial writes to complete; the new
iomap_dio_rw iomap helper does not.

The above mentioned fallback mode is used by gfs2, which doesn't allow
block allocations under direct I/O to avoid taking cluster-wide
exclusive locks.  As a consequence, an asynchronous direct I/O write to
a file range that ends in a hole will result in a short write.  When
that happens, we want to complete the remaining write as buffered I/O.

To allow that, change iomap_dio_rw to wait for short direct I/O writes
like __blockdev_direct_IO does instead of returning -EIOCBQUEUED.

This makes xfstest generic/240 work on gfs2.

Signed-off-by: Andreas Gruenbacher 
Cc: Dave Chinner 
---
 fs/iomap.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/fs/iomap.c b/fs/iomap.c
index 27d97a290623..fcd3e26aadb7 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -720,6 +720,7 @@ struct iomap_dio {
atomic_tref;
unsignedflags;
int error;
+   boolwait_for_completion;
 
union {
/* used during submission and for synchronous completion: */
@@ -821,9 +822,8 @@ static void iomap_dio_bio_end_io(struct bio *bio)
iomap_dio_set_error(dio, blk_status_to_errno(bio->bi_status));
 
if (atomic_dec_and_test(>ref)) {
-   if (is_sync_kiocb(dio->iocb)) {
+   if (dio->wait_for_completion) {
struct task_struct *waiter = dio->submit.waiter;
-
WRITE_ONCE(dio->submit.waiter, NULL);
wake_up_process(waiter);
} else if (dio->flags & IOMAP_DIO_WRITE) {
@@ -1014,13 +1014,12 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
dio->end_io = end_io;
dio->error = 0;
dio->flags = 0;
+   dio->wait_for_completion = is_sync_kiocb(iocb);
 
dio->submit.iter = iter;
-   if (is_sync_kiocb(iocb)) {
-   dio->submit.waiter = current;
-   dio->submit.cookie = BLK_QC_T_NONE;
-   dio->submit.last_queue = NULL;
-   }
+   dio->submit.waiter = current;
+   dio->submit.cookie = BLK_QC_T_NONE;
+   dio->submit.last_queue = NULL;
 
if (iov_iter_rw(iter) == READ) {
if (pos >= dio->i_size)
@@ -1057,7 +1056,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
dio_warn_stale_pagecache(iocb->ki_filp);
ret = 0;
 
-   if (iov_iter_rw(iter) == WRITE && !is_sync_kiocb(iocb) &&
+   if (iov_iter_rw(iter) == WRITE && !dio->wait_for_completion &&
!inode->i_sb->s_dio_done_wq) {
ret = sb_init_dio_done_wq(inode->i_sb);
if (ret < 0)
@@ -1072,8 +1071,10 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
iomap_dio_actor);
if (ret <= 0) {
/* magic error code to fall back to buffered I/O */
-   if (ret == -ENOTBLK)
+   if (ret == -ENOTBLK) {
+   dio->wait_for_completion = true;
ret = 0;
+   }
break;
}
pos += ret;
@@ -1086,8 +1087,9 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
if (ret < 0)
iomap_dio_set_error(dio, ret);
 
+   smp_mb__before_atomic();
if (!atomic_dec_and_test(>ref)) {
-   if (!is_sync_kiocb(iocb))
+   if (!dio->wait_for_completion)
return -EIOCBQUEUED;
 
for (;;) {
-- 
2.17.0



[Cluster-devel] [PATCH v5 05/14] gfs2: Iomap cleanups and improvements

2018-05-30 Thread Andreas Gruenbacher
Clean up gfs2_iomap_alloc and gfs2_iomap_get.  Document how
gfs2_iomap_alloc works: it now needs to be called separately after
gfs2_iomap_get where necessary; this will be used later by iomap write.
Move gfs2_iomap_ops into bmap.c.

Introduce a new gfs2_iomap_get_alloc helper and use it in
fallocate_chunk: gfs2_iomap_begin will become unsuitable for fallocate
with proper iomap write support.

In fallocate_chunk, zero-initialize struct iomap.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/bmap.c  | 203 
 fs/gfs2/bmap.h  |   6 +-
 fs/gfs2/file.c  |   6 +-
 fs/gfs2/inode.c |   4 -
 4 files changed, 125 insertions(+), 94 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 5226c3bfbcf7..8ea65aea34da 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -572,22 +572,6 @@ static int gfs2_hole_size(struct inode *inode, sector_t 
lblock, u64 len,
return ret;
 }
 
-static inline void bmap_lock(struct gfs2_inode *ip, int create)
-{
-   if (create)
-   down_write(>i_rw_mutex);
-   else
-   down_read(>i_rw_mutex);
-}
-
-static inline void bmap_unlock(struct gfs2_inode *ip, int create)
-{
-   if (create)
-   up_write(>i_rw_mutex);
-   else
-   up_read(>i_rw_mutex);
-}
-
 static inline __be64 *gfs2_indirect_init(struct metapath *mp,
 struct gfs2_glock *gl, unsigned int i,
 unsigned offset, u64 bn)
@@ -614,15 +598,11 @@ enum alloc_state {
 };
 
 /**
- * gfs2_bmap_alloc - Build a metadata tree of the requested height
+ * gfs2_iomap_alloc - Build a metadata tree of the requested height
  * @inode: The GFS2 inode
- * @lblock: The logical starting block of the extent
- * @bh_map: This is used to return the mapping details
- * @zero_new: True if newly allocated blocks should be zeroed
+ * @iomap: The iomap structure
+ * @flags: iomap flags
  * @mp: The metapath, with proper height information calculated
- * @maxlen: The max number of data blocks to alloc
- * @dblock: Pointer to return the resulting new block
- * @dblks: Pointer to return the number of blocks allocated
  *
  * In this routine we may have to alloc:
  *   i) Indirect blocks to grow the metadata tree height
@@ -635,6 +615,13 @@ enum alloc_state {
  * blocks are available, there will only be one request per bmap call)
  * and uses the state machine to initialise the blocks in order.
  *
+ * Right now, this function will allocate at most one indirect block
+ * worth of data -- with a default block size of 4K, that's slightly
+ * less than 2M.  If this limitation is ever removed to allow huge
+ * allocations, we would probably still want to limit the iomap size we
+ * return to avoid stalling other tasks during huge writes; the next
+ * iomap iteration would then find the blocks already allocated.
+ *
  * Returns: errno on error
  */
 
@@ -649,6 +636,7 @@ static int gfs2_iomap_alloc(struct inode *inode, struct 
iomap *iomap,
unsigned dblks = 0;
unsigned ptrs_per_blk;
const unsigned end_of_metadata = mp->mp_fheight - 1;
+   int ret;
enum alloc_state state;
__be64 *ptr;
__be64 zero_bn = 0;
@@ -659,6 +647,8 @@ static int gfs2_iomap_alloc(struct inode *inode, struct 
iomap *iomap,
 
gfs2_trans_add_meta(ip->i_gl, dibh);
 
+   down_write(>i_rw_mutex);
+
if (mp->mp_fheight == mp->mp_aheight) {
struct buffer_head *bh;
int eob;
@@ -694,11 +684,10 @@ static int gfs2_iomap_alloc(struct inode *inode, struct 
iomap *iomap,
blks = dblks + iblks;
i = mp->mp_aheight;
do {
-   int error;
n = blks - alloced;
-   error = gfs2_alloc_blocks(ip, , , 0, NULL);
-   if (error)
-   return error;
+   ret = gfs2_alloc_blocks(ip, , , 0, NULL);
+   if (ret)
+   goto out;
alloced += n;
if (state != ALLOC_DATA || gfs2_is_jdata(ip))
gfs2_trans_add_unrevoke(sdp, bn, n);
@@ -754,7 +743,7 @@ static int gfs2_iomap_alloc(struct inode *inode, struct 
iomap *iomap,
dblks = n;
ptr = metapointer(end_of_metadata, mp);
iomap->addr = bn << inode->i_blkbits;
-   iomap->flags |= IOMAP_F_NEW;
+   iomap->flags |= IOMAP_F_MERGED | IOMAP_F_NEW;
while (n-- > 0)
*ptr++ = cpu_to_be64(bn++);
break;
@@ -764,8 +753,10 @@ static int gfs2_iomap_alloc(struct inode *inode, struct 
iomap *iomap,
iomap->length = (u64)dblks << inode->i_blkbits;
ip->i_height = mp->mp_fheight;
gfs2_add_inode_blocks(>i_inode, alloced);
-   gfs2_dinode_out(ip, mp->mp_bh[0]->b_data);
-   return 0;
+   gfs2_dinode_out(ip, 

[Cluster-devel] [PATCH v5 02/14] gfs2: hole_size improvement

2018-05-30 Thread Andreas Gruenbacher
Reimplement function hole_size based on a generic function for walking
the metadata tree and rename hole_size to gfs2_hole_size.  While
previously, multiple invocations of hole_size were sometimes needed to
walk across the entire hole, the new implementation always returns the
entire hole at once (provided that the caller is interested in the total
size).

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/bmap.c | 210 +++--
 1 file changed, 153 insertions(+), 57 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index fcf2f7d166de..69f846418ad5 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -278,6 +278,21 @@ static inline __be64 *metapointer(unsigned int height, 
const struct metapath *mp
return p + mp->mp_list[height];
 }
 
+static inline const __be64 *metaend(unsigned int height, const struct metapath 
*mp)
+{
+   const struct buffer_head *bh = mp->mp_bh[height];
+   return (const __be64 *)(bh->b_data + bh->b_size);
+}
+
+static void clone_metapath(struct metapath *clone, struct metapath *mp)
+{
+   unsigned int hgt;
+
+   *clone = *mp;
+   for (hgt = 0; hgt < mp->mp_aheight; hgt++)
+   get_bh(clone->mp_bh[hgt]);
+}
+
 static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start, __be64 *end)
 {
const __be64 *t;
@@ -419,6 +434,142 @@ static inline unsigned int gfs2_extent_length(void 
*start, unsigned int len, __b
return (ptr - first);
 }
 
+typedef const __be64 *(*gfs2_metadata_walker)(
+   struct metapath *mp,
+   const __be64 *start, const __be64 *end,
+   u64 factor, void *data);
+
+#define WALK_STOP ((__be64 *)0)
+#define WALK_NEXT ((__be64 *)1)
+
+static int gfs2_walk_metadata(struct inode *inode, sector_t lblock,
+   u64 len, struct metapath *mp, gfs2_metadata_walker walker,
+   void *data)
+{
+   struct metapath clone;
+   struct gfs2_inode *ip = GFS2_I(inode);
+   struct gfs2_sbd *sdp = GFS2_SB(inode);
+   const __be64 *start, *end, *ptr;
+   u64 factor = 1;
+   unsigned int hgt;
+   int ret = 0;
+
+   for (hgt = ip->i_height - 1; hgt >= mp->mp_aheight; hgt--)
+   factor *= sdp->sd_inptrs;
+
+   for (;;) {
+   u64 step;
+
+   /* Walk indirect block. */
+   start = metapointer(hgt, mp);
+   end = metaend(hgt, mp);
+
+   step = (end - start) * factor;
+   if (step > len)
+   end = start + DIV_ROUND_UP_ULL(len, factor);
+
+   ptr = walker(mp, start, end, factor, data);
+   if (ptr == WALK_STOP)
+   break;
+   if (step >= len)
+   break;
+   len -= step;
+   if (ptr != WALK_NEXT) {
+   BUG_ON(!*ptr);
+   mp->mp_list[hgt] += ptr - start;
+   goto fill_up_metapath;
+   }
+
+lower_metapath:
+   /* Decrease height of metapath. */
+   if (mp != ) {
+   clone_metapath(, mp);
+   mp = 
+   }
+   brelse(mp->mp_bh[hgt]);
+   mp->mp_bh[hgt] = NULL;
+   if (!hgt)
+   break;
+   hgt--;
+   factor *= sdp->sd_inptrs;
+
+   /* Advance in metadata tree. */
+   (mp->mp_list[hgt])++;
+   start = metapointer(hgt, mp);
+   end = metaend(hgt, mp);
+   if (start >= end) {
+   mp->mp_list[hgt] = 0;
+   if (!hgt)
+   break;
+   goto lower_metapath;
+   }
+
+fill_up_metapath:
+   /* Increase height of metapath. */
+   if (mp != ) {
+   clone_metapath(, mp);
+   mp = 
+   }
+   ret = fillup_metapath(ip, mp, ip->i_height - 1);
+   if (ret < 0)
+   break;
+   hgt += ret;
+   for (; ret; ret--)
+   do_div(factor, sdp->sd_inptrs);
+   mp->mp_aheight = hgt + 1;
+   }
+   if (mp == )
+   release_metapath(mp);
+   return ret;
+}
+
+struct gfs2_hole_walker_args {
+   u64 blocks;
+};
+
+static const __be64 *gfs2_hole_walker(struct metapath *mp,
+   const __be64 *start, const __be64 *end,
+   u64 factor, void *data)
+{
+   struct gfs2_hole_walker_args *args = data;
+   const __be64 *ptr;
+
+   for (ptr = start; ptr < end; ptr++) {
+   if (*ptr) {
+   args->blocks += (ptr - start) * factor;
+   if (mp->mp_aheight == mp->mp_fheight)
+   return WALK_STOP;
+   return ptr;  /* increase height */
+   }
+  

[Cluster-devel] [PATCH v5 12/14] gfs2: Remove gfs2_write_{begin, end}

2018-05-30 Thread Andreas Gruenbacher
Now that generic_file_write_iter is no longer used, there are no
remaining users of these address space operations.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/aops.c | 210 -
 1 file changed, 210 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index b0a219fae162..cc80fd71f3dd 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -639,132 +639,6 @@ static int gfs2_readpages(struct file *file, struct 
address_space *mapping,
return ret;
 }
 
-/**
- * gfs2_write_begin - Begin to write to a file
- * @file: The file to write to
- * @mapping: The mapping in which to write
- * @pos: The file offset at which to start writing
- * @len: Length of the write
- * @flags: Various flags
- * @pagep: Pointer to return the page
- * @fsdata: Pointer to return fs data (unused by GFS2)
- *
- * Returns: errno
- */
-
-static int gfs2_write_begin(struct file *file, struct address_space *mapping,
-   loff_t pos, unsigned len, unsigned flags,
-   struct page **pagep, void **fsdata)
-{
-   struct gfs2_inode *ip = GFS2_I(mapping->host);
-   struct gfs2_sbd *sdp = GFS2_SB(mapping->host);
-   struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
-   unsigned int data_blocks = 0, ind_blocks = 0, rblocks;
-   unsigned requested = 0;
-   int alloc_required;
-   int error = 0;
-   pgoff_t index = pos >> PAGE_SHIFT;
-   unsigned from = pos & (PAGE_SIZE - 1);
-   struct page *page;
-
-   gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, >i_gh);
-   error = gfs2_glock_nq(>i_gh);
-   if (unlikely(error))
-   goto out_uninit;
-   if (>i_inode == sdp->sd_rindex) {
-   error = gfs2_glock_nq_init(m_ip->i_gl, LM_ST_EXCLUSIVE,
-  GL_NOCACHE, _ip->i_gh);
-   if (unlikely(error)) {
-   gfs2_glock_dq(>i_gh);
-   goto out_uninit;
-   }
-   }
-
-   alloc_required = gfs2_write_alloc_required(ip, pos, len);
-
-   if (alloc_required || gfs2_is_jdata(ip))
-   gfs2_write_calc_reserv(ip, len, _blocks, _blocks);
-
-   if (alloc_required) {
-   struct gfs2_alloc_parms ap = { .aflags = 0, };
-   requested = data_blocks + ind_blocks;
-   ap.target = requested;
-   error = gfs2_quota_lock_check(ip, );
-   if (error)
-   goto out_unlock;
-
-   error = gfs2_inplace_reserve(ip, );
-   if (error)
-   goto out_qunlock;
-   }
-
-   rblocks = RES_DINODE + ind_blocks;
-   if (gfs2_is_jdata(ip))
-   rblocks += data_blocks ? data_blocks : 1;
-   if (ind_blocks || data_blocks)
-   rblocks += RES_STATFS + RES_QUOTA;
-   if (>i_inode == sdp->sd_rindex)
-   rblocks += 2 * RES_STATFS;
-   if (alloc_required)
-   rblocks += gfs2_rg_blocks(ip, requested);
-
-   error = gfs2_trans_begin(sdp, rblocks,
-PAGE_SIZE/sdp->sd_sb.sb_bsize);
-   if (error)
-   goto out_trans_fail;
-
-   error = -ENOMEM;
-   flags |= AOP_FLAG_NOFS;
-   page = grab_cache_page_write_begin(mapping, index, flags);
-   *pagep = page;
-   if (unlikely(!page))
-   goto out_endtrans;
-
-   if (gfs2_is_stuffed(ip)) {
-   error = 0;
-   if (pos + len > gfs2_max_stuffed_size(ip)) {
-   error = gfs2_unstuff_dinode(ip, page);
-   if (error == 0)
-   goto prepare_write;
-   } else if (!PageUptodate(page)) {
-   error = stuffed_readpage(ip, page);
-   }
-   goto out;
-   }
-
-prepare_write:
-   error = __block_write_begin(page, from, len, gfs2_block_map);
-out:
-   if (error == 0)
-   return 0;
-
-   unlock_page(page);
-   put_page(page);
-
-   gfs2_trans_end(sdp);
-   if (pos + len > ip->i_inode.i_size)
-   gfs2_trim_blocks(>i_inode);
-   goto out_trans_fail;
-
-out_endtrans:
-   gfs2_trans_end(sdp);
-out_trans_fail:
-   if (alloc_required) {
-   gfs2_inplace_release(ip);
-out_qunlock:
-   gfs2_quota_unlock(ip);
-   }
-out_unlock:
-   if (>i_inode == sdp->sd_rindex) {
-   gfs2_glock_dq(_ip->i_gh);
-   gfs2_holder_uninit(_ip->i_gh);
-   }
-   gfs2_glock_dq(>i_gh);
-out_uninit:
-   gfs2_holder_uninit(>i_gh);
-   return error;
-}
-
 /**
  * adjust_fs_space - Adjusts the free space available due to gfs2_grow
  * @inode: the rindex inode
@@ -845,84 +719,6 @@ int gfs2_stuffed_write_end(struct inode *inode, struct 
buffer_head *dibh,
return copied;
 }
 
-/**
- * gfs2_write_end
- * @file: The file to write to
- * @mapping: The address space 

[Cluster-devel] [PATCH v5 04/14] gfs2: Remove ordered write mode handling from gfs2_trans_add_data

2018-05-30 Thread Andreas Gruenbacher
In journaled data mode, we need to add each buffer head to the current
transaction.  In ordered write mode, we only need to add the inode to
the ordered inode list.  So far, both cases are handled in
gfs2_trans_add_data.  This makes the code look misleading and is
inefficient for small block sizes as well.  Handle both cases separately
instead.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/aops.c  |  7 ---
 fs/gfs2/bmap.c  | 12 
 fs/gfs2/log.h   |  7 ++-
 fs/gfs2/quota.c |  5 -
 fs/gfs2/trans.c | 27 ---
 5 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index e8c0387652ed..a1c6b5de9200 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -54,8 +54,7 @@ static void gfs2_page_add_databufs(struct gfs2_inode *ip, 
struct page *page,
continue;
if (start >= to)
break;
-   if (gfs2_is_jdata(ip))
-   set_buffer_uptodate(bh);
+   set_buffer_uptodate(bh);
gfs2_trans_add_data(ip->i_gl, bh);
}
 }
@@ -891,8 +890,10 @@ static int gfs2_write_end(struct file *file, struct 
address_space *mapping,
goto out2;
}
 
-   if (!gfs2_is_writeback(ip))
+   if (gfs2_is_jdata(ip))
gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
+   else
+   gfs2_ordered_add_inode(ip);
 
ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
page = NULL;
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 69f846418ad5..5226c3bfbcf7 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -89,10 +89,12 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip, 
struct buffer_head *dibh,
map_bh(bh, inode->i_sb, block);
 
set_buffer_uptodate(bh);
-   if (!gfs2_is_jdata(ip))
-   mark_buffer_dirty(bh);
-   if (!gfs2_is_writeback(ip))
+   if (gfs2_is_jdata(ip))
gfs2_trans_add_data(ip->i_gl, bh);
+   else {
+   mark_buffer_dirty(bh);
+   gfs2_ordered_add_inode(ip);
+   }
 
if (release) {
unlock_page(page);
@@ -1028,8 +1030,10 @@ static int gfs2_block_zero_range(struct inode *inode, 
loff_t from,
err = 0;
}
 
-   if (!gfs2_is_writeback(ip))
+   if (gfs2_is_jdata(ip))
gfs2_trans_add_data(ip->i_gl, bh);
+   else
+   gfs2_ordered_add_inode(ip);
 
zero_user(page, offset, length);
mark_buffer_dirty(bh);
diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
index 1862e310a067..20241436126d 100644
--- a/fs/gfs2/log.h
+++ b/fs/gfs2/log.h
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include "incore.h"
+#include "inode.h"
 
 /**
  * gfs2_log_lock - acquire the right to mess with the log manager
@@ -50,8 +51,12 @@ static inline void gfs2_log_pointers_init(struct gfs2_sbd 
*sdp,
 
 static inline void gfs2_ordered_add_inode(struct gfs2_inode *ip)
 {
-   struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
+   struct gfs2_sbd *sdp;
 
+   if (!gfs2_is_ordered(ip))
+   return;
+
+   sdp = GFS2_SB(>i_inode);
if (!test_bit(GIF_ORDERED, >i_flags)) {
spin_lock(>sd_ordered_lock);
if (!test_and_set_bit(GIF_ORDERED, >i_flags))
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 7a98abd340ee..e8585dfd209f 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -735,7 +735,10 @@ static int gfs2_write_buf_to_page(struct gfs2_inode *ip, 
unsigned long index,
if (!buffer_uptodate(bh))
goto unlock_out;
}
-   gfs2_trans_add_data(ip->i_gl, bh);
+   if (gfs2_is_jdata(ip))
+   gfs2_trans_add_data(ip->i_gl, bh);
+   else
+   gfs2_ordered_add_inode(ip);
 
/* If we need to write to the next block as well */
if (to_write > (bsize - boff)) {
diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index c75cacaa349b..064c9a0ef046 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -143,32 +143,21 @@ static struct gfs2_bufdata *gfs2_alloc_bufdata(struct 
gfs2_glock *gl,
  * @gl: The inode glock associated with the buffer
  * @bh: The buffer to add
  *
- * This is used in two distinct cases:
- * i) In ordered write mode
- *We put the data buffer on a list so that we can ensure that it's
- *synced to disk at the right time
- * ii) In journaled data mode
- *We need to journal the data block in the same way as metadata in
- *the functions above. The difference is that here we have a tag
- *which is two __be64's being the block number (as per meta data)
- *and a flag which says whether the data block needs escaping or
- *not. This means we need a new log entry for each 251 or so data
- *blocks, which isn't an enormous overhead but twice as 

[Cluster-devel] [PATCH v5 08/14] gfs2: iomap buffered write support

2018-05-30 Thread Andreas Gruenbacher
With the traditional page-based writes, blocks are allocated separately
for each page written to.  With iomap writes, we can allocate a lot more
blocks at once, with a fraction of the allocation overhead for each
page.

Split calculating the number of blocks that can be allocated at a given
position (gfs2_alloc_size) off from gfs2_iomap_alloc: that size
determines the number of blocks to allocate and reserve in the journal.

In gfs2_iomap_alloc, set the type of newly allocated extents to
IOMAP_MAPPED so that iomap_to_bh will set the bh states correctly:
otherwise, the bhs would not be marked as mapped, confusing
__mpage_writepage.  This means that we need to check for the IOMAP_F_NEW
flag in fallocate_chunk now.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/aops.c |  20 +--
 fs/gfs2/aops.h |  22 
 fs/gfs2/bmap.c | 346 -
 fs/gfs2/file.c |  46 ++-
 4 files changed, 388 insertions(+), 46 deletions(-)
 create mode 100644 fs/gfs2/aops.h

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index a1c6b5de9200..3d9633175aa8 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "gfs2.h"
 #include "incore.h"
@@ -36,10 +37,11 @@
 #include "super.h"
 #include "util.h"
 #include "glops.h"
+#include "aops.h"
 
 
-static void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
-  unsigned int from, unsigned int len)
+void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
+   unsigned int from, unsigned int len)
 {
struct buffer_head *head = page_buffers(page);
unsigned int bsize = head->b_size;
@@ -462,7 +464,7 @@ static int gfs2_jdata_writepages(struct address_space 
*mapping,
  * Returns: errno
  */
 
-static int stuffed_readpage(struct gfs2_inode *ip, struct page *page)
+int stuffed_readpage(struct gfs2_inode *ip, struct page *page)
 {
struct buffer_head *dibh;
u64 dsize = i_size_read(>i_inode);
@@ -773,7 +775,7 @@ static int gfs2_write_begin(struct file *file, struct 
address_space *mapping,
  * adjust_fs_space - Adjusts the free space available due to gfs2_grow
  * @inode: the rindex inode
  */
-static void adjust_fs_space(struct inode *inode)
+void adjust_fs_space(struct inode *inode)
 {
struct gfs2_sbd *sdp = inode->i_sb->s_fs_info;
struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
@@ -819,11 +821,11 @@ static void adjust_fs_space(struct inode *inode)
  * This copies the data from the page into the inode block after
  * the inode data structure itself.
  *
- * Returns: errno
+ * Returns: copied bytes or errno
  */
-static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head 
*dibh,
- loff_t pos, unsigned copied,
- struct page *page)
+int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
+  loff_t pos, unsigned copied,
+  struct page *page)
 {
struct gfs2_inode *ip = GFS2_I(inode);
u64 to = pos + copied;
@@ -862,7 +864,7 @@ static int gfs2_stuffed_write_end(struct inode *inode, 
struct buffer_head *dibh,
  * The main write_end function for GFS2. We just put our locking around the VFS
  * provided functions.
  *
- * Returns: errno
+ * Returns: copied bytes or errno
  */
 
 static int gfs2_write_end(struct file *file, struct address_space *mapping,
diff --git a/fs/gfs2/aops.h b/fs/gfs2/aops.h
new file mode 100644
index ..976bb32dd405
--- /dev/null
+++ b/fs/gfs2/aops.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc.  All rights reserved.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU General Public License version 2.
+ */
+
+#ifndef __AOPS_DOT_H__
+#define __AOPS_DOT_H__
+
+#include "incore.h"
+
+extern int stuffed_readpage(struct gfs2_inode *ip, struct page *page);
+extern int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head 
*dibh,
+ loff_t pos, unsigned copied,
+ struct page *page);
+extern void adjust_fs_space(struct inode *inode);
+extern void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
+  unsigned int from, unsigned int len);
+
+#endif /* __AOPS_DOT_H__ */
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 8ea65aea34da..d70672a1a29c 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -28,6 +28,7 @@
 #include "trans.h"
 #include "dir.h"
 #include "util.h"
+#include "aops.h"
 #include "trace_gfs2.h"
 
 /* This doesn't need to be that large as max 64 bit pointers in a 4k
@@ -41,6 +42,8 @@ struct metapath {
int mp_aheight; /* actual height (lookup height) */
 };
 
+static int punch_hole(struct gfs2_inode *ip, u64 offset, u64 

[Cluster-devel] [PATCH v5 01/14] gfs2: Update find_metapath comment

2018-05-30 Thread Andreas Gruenbacher
Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/bmap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 0590e93494f7..fcf2f7d166de 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -176,8 +176,8 @@ int gfs2_unstuff_dinode(struct gfs2_inode *ip, struct page 
*page)
 /**
  * find_metapath - Find path through the metadata tree
  * @sdp: The superblock
- * @mp: The metapath to return the result in
  * @block: The disk block to look up
+ * @mp: The metapath to return the result in
  * @height: The pre-calculated height of the metadata tree
  *
  *   This routine returns a struct metapath structure that defines a path
@@ -188,8 +188,7 @@ int gfs2_unstuff_dinode(struct gfs2_inode *ip, struct page 
*page)
  *   filesystem with a blocksize of 4096.
  *
  *   find_metapath() would return a struct metapath structure set to:
- *   mp_offset = 101342453, mp_height = 3, mp_list[0] = 0, mp_list[1] = 48,
- *   and mp_list[2] = 165.
+ *   mp_fheight = 3, mp_list[0] = 0, mp_list[1] = 48, and mp_list[2] = 165.
  *
  *   That means that in order to get to the block containing the byte at
  *   offset 101342453, we would load the indirect block pointed to by pointer
-- 
2.17.0



[Cluster-devel] [PATCH v5 09/14] gfs2: gfs2_extent_length cleanup

2018-05-30 Thread Andreas Gruenbacher
Now that gfs2_extent_length is no longer used for determining the size
of a hole and always with an upper size limit, the function can be
simplified.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/bmap.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index d70672a1a29c..05c5486debd6 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -406,22 +406,17 @@ static void release_metapath(struct metapath *mp)
 
 /**
  * gfs2_extent_length - Returns length of an extent of blocks
- * @start: Start of the buffer
- * @len: Length of the buffer in bytes
- * @ptr: Current position in the buffer
- * @limit: Max extent length to return (0 = unlimited)
+ * @bh: The metadata block
+ * @ptr: Current position in @bh
+ * @limit: Max extent length to return
  * @eob: Set to 1 if we hit "end of block"
  *
- * If the first block is zero (unallocated) it will return the number of
- * unallocated blocks in the extent, otherwise it will return the number
- * of contiguous blocks in the extent.
- *
  * Returns: The length of the extent (minimum of one block)
  */
 
-static inline unsigned int gfs2_extent_length(void *start, unsigned int len, 
__be64 *ptr, size_t limit, int *eob)
+static inline unsigned int gfs2_extent_length(struct buffer_head *bh, __be64 
*ptr, size_t limit, int *eob)
 {
-   const __be64 *end = (start + len);
+   const __be64 *end = (__be64 *)(bh->b_data + bh->b_size);
const __be64 *first = ptr;
u64 d = be64_to_cpu(*ptr);
 
@@ -430,14 +425,11 @@ static inline unsigned int gfs2_extent_length(void 
*start, unsigned int len, __b
ptr++;
if (ptr >= end)
break;
-   if (limit && --limit == 0)
-   break;
-   if (d)
-   d++;
+   d++;
} while(be64_to_cpu(*ptr) == d);
if (ptr >= end)
*eob = 1;
-   return (ptr - first);
+   return ptr - first;
 }
 
 typedef const __be64 *(*gfs2_metadata_walker)(
@@ -883,7 +875,7 @@ static int gfs2_iomap_get(struct inode *inode, loff_t pos, 
loff_t length,
goto do_alloc;
 
bh = mp->mp_bh[ip->i_height - 1];
-   len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, len, );
+   len = gfs2_extent_length(bh, ptr, len, );
 
iomap->addr = be64_to_cpu(*ptr) << inode->i_blkbits;
iomap->length = len << inode->i_blkbits;
-- 
2.17.0



[Cluster-devel] [PATCH v5 13/14] iomap: inline data should be an iomap type, not a flag

2018-05-30 Thread Andreas Gruenbacher
From: Christoph Hellwig 

Inline data is fundamentally different from our normal mapped case in that
it doesn't even have a block address.  So instead of having a flag for it
it should be an entirely separate iomap range type.

[Minimally adjusted to fit into the gfs2 iomap-write patch queue.]

Signed-off-by: Christoph Hellwig 
---
 fs/ext4/inline.c  | 4 ++--
 fs/gfs2/bmap.c| 8 
 fs/iomap.c| 8 
 include/linux/iomap.h | 2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 70cf4c7b268a..e1f00891ef95 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1835,8 +1835,8 @@ int ext4_inline_data_iomap(struct inode *inode, struct 
iomap *iomap)
iomap->offset = 0;
iomap->length = min_t(loff_t, ext4_get_inline_size(inode),
  i_size_read(inode));
-   iomap->type = 0;
-   iomap->flags = IOMAP_F_DATA_INLINE;
+   iomap->type = IOMAP_INLINE;
+   iomap->flags = 0;
 
 out:
up_read(_I(inode)->xattr_sem);
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 69d87a38a209..c157af31dc56 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -754,8 +754,8 @@ static void gfs2_stuffed_iomap(struct inode *inode, struct 
iomap *iomap,
  sizeof(struct gfs2_dinode);
iomap->offset = 0;
iomap->length = length;
-   iomap->type = IOMAP_MAPPED;
-   iomap->flags = IOMAP_F_DATA_INLINE;
+   iomap->type = IOMAP_INLINE;
+   iomap->flags = 0;
 }
 
 /**
@@ -980,7 +980,7 @@ static int gfs2_iomap_begin_write(struct inode *inode, 
loff_t pos, loff_t length
if (ret)
goto out_release;
 
-   alloc_required = unstuff || iomap->type != IOMAP_MAPPED;
+   alloc_required = unstuff || iomap->type == IOMAP_HOLE;
 
if (alloc_required || gfs2_is_jdata(ip))
gfs2_write_calc_reserv(ip, iomap->length, _blocks, 
_blocks);
@@ -1021,7 +1021,7 @@ static int gfs2_iomap_begin_write(struct inode *inode, 
loff_t pos, loff_t length
goto out_trans_end;
}
 
-   if (iomap->type != IOMAP_MAPPED) {
+   if (iomap->type == IOMAP_HOLE) {
ret = gfs2_iomap_alloc(inode, iomap, flags, );
if (ret) {
gfs2_trans_end(sdp);
diff --git a/fs/iomap.c b/fs/iomap.c
index fcd3e26aadb7..819b6c56ecfe 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -522,22 +522,22 @@ static int iomap_to_fiemap(struct fiemap_extent_info *fi,
case IOMAP_HOLE:
/* skip holes */
return 0;
+   case IOMAP_MAPPED:
+   break;
case IOMAP_DELALLOC:
flags |= FIEMAP_EXTENT_DELALLOC | FIEMAP_EXTENT_UNKNOWN;
break;
case IOMAP_UNWRITTEN:
flags |= FIEMAP_EXTENT_UNWRITTEN;
break;
-   case IOMAP_MAPPED:
-   break;
+   case IOMAP_INLINE:
+   flags |= FIEMAP_EXTENT_DATA_INLINE;
}
 
if (iomap->flags & IOMAP_F_MERGED)
flags |= FIEMAP_EXTENT_MERGED;
if (iomap->flags & IOMAP_F_SHARED)
flags |= FIEMAP_EXTENT_SHARED;
-   if (iomap->flags & IOMAP_F_DATA_INLINE)
-   flags |= FIEMAP_EXTENT_DATA_INLINE;
 
return fiemap_fill_next_extent(fi, iomap->offset,
iomap->addr != IOMAP_NULL_ADDR ? iomap->addr : 0,
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 423f7ecc1231..0a018a58d84e 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -18,6 +18,7 @@ struct vm_fault;
 #define IOMAP_DELALLOC 0x02/* delayed allocation blocks */
 #define IOMAP_MAPPED   0x03/* blocks allocated at @addr */
 #define IOMAP_UNWRITTEN0x04/* blocks allocated at @addr in 
unwritten state */
+#define IOMAP_INLINE   0x05
 
 /*
  * Flags for all iomap mappings:
@@ -34,7 +35,6 @@ struct vm_fault;
  */
 #define IOMAP_F_MERGED 0x10/* contains multiple blocks/extents */
 #define IOMAP_F_SHARED 0x20/* block shared with another file */
-#define IOMAP_F_DATA_INLINE0x40/* data inline in the inode */
 
 /*
  * Magic value for addr:
-- 
2.17.0



[Cluster-devel] [PATCH v5 07/14] iomap: Mark newly allocated buffer heads as new

2018-05-30 Thread Andreas Gruenbacher
In iomap_to_bh, not only mark buffer heads in IOMAP_UNWRITTEN maps as
new, but also buffer heads in IOMAP_MAPPED maps with the IOMAP_F_NEW
flag set.  This will be used by filesystems like gfs2, which allocate
blocks in iomap->begin.

Minor corrections to the comment for IOMAP_UNWRITTEN maps.

Signed-off-by: Andreas Gruenbacher 
---
 fs/buffer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 249b83fafe48..5220d9efcd18 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1900,15 +1900,15 @@ iomap_to_bh(struct inode *inode, sector_t block, struct 
buffer_head *bh,
break;
case IOMAP_UNWRITTEN:
/*
-* For unwritten regions, we always need to ensure that
-* sub-block writes cause the regions in the block we are not
-* writing to are zeroed. Set the buffer as new to ensure this.
+* For unwritten regions, we always need to ensure that regions
+* in the block we are not writing to are zeroed. Mark the
+* buffer as new to ensure this.
 */
set_buffer_new(bh);
set_buffer_unwritten(bh);
/* FALLTHRU */
case IOMAP_MAPPED:
-   if (offset >= i_size_read(inode))
+   if ((iomap->flags & IOMAP_F_NEW) || offset >= 
i_size_read(inode))
set_buffer_new(bh);
bh->b_blocknr = (iomap->addr + offset - iomap->offset) >>
inode->i_blkbits;
-- 
2.17.0



[Cluster-devel] [PATCH v5 00/14] gfs2 iomap write support

2018-05-30 Thread Andreas Gruenbacher
Here's an update of my gfs2 iomap write patch queue, with support for
buffered writes as well as direct I/O reads and writes through iomap.
This update fixes a few bugs in the gfs2 specific parts.  The patches
are still meant for the upcoming merge window.

The first five patches are minor gfs2 specific cleanups and
improvements.

The actual fun starts with patch "iomap: Add write_{begin,end} iomap
operations" which adds callbacks into the filesystem for any per-page
processing.  Christoph has suggested that at least the stuffed inode
(aka. inline data) handline should be moved to the iomap_{begin,end}
operations.  As the last patch tries to demonstrate, this is a bit
tricky, so even though Christoph's approach is slightly cleaner
conceptually, it doesn't look like an improvement overall.

Patches "iomap: Mark newly allocated buffer heads as new" and "iomap:
Complete partial direct I/O writes synchronously" have been added as
discussed.

A 4.17-rc7 based version of the patches can be found here:

  
https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/log/?h=iomap-write

Thanks,
Andreas

Andreas Gruenbacher (13):
  gfs2: Update find_metapath comment
  gfs2: hole_size improvement
  gfs2: gfs2_stuffed_write_end cleanup
  gfs2: Remove ordered write mode handling from gfs2_trans_add_data
  gfs2: Iomap cleanups and improvements
  iomap: Add write_{begin,end} iomap operations
  iomap: Mark newly allocated buffer heads as new
  gfs2: iomap buffered write support
  gfs2: gfs2_extent_length cleanup
  iomap: Complete partial direct I/O writes synchronously
  gfs2: iomap direct I/O support
  gfs2: Remove gfs2_write_{begin,end}
  gfs2: Handle stuffed files in iomap_{begin,end}

Christoph Hellwig (1):
  iomap: inline data should be an iomap type, not a flag

 fs/buffer.c   |   8 +-
 fs/ext2/inode.c   |   2 +
 fs/ext4/inline.c  |   4 +-
 fs/ext4/inode.c   |   2 +
 fs/gfs2/aops.c| 344 +--
 fs/gfs2/aops.h|  22 ++
 fs/gfs2/bmap.c| 772 --
 fs/gfs2/bmap.h|   6 +-
 fs/gfs2/file.c| 174 +-
 fs/gfs2/inode.c   |   4 -
 fs/gfs2/log.h |   7 +-
 fs/gfs2/quota.c   |   5 +-
 fs/gfs2/trans.c   |  27 +-
 fs/iomap.c| 109 +++---
 fs/xfs/xfs_iomap.c|   2 +
 include/linux/iomap.h |  25 +-
 16 files changed, 920 insertions(+), 593 deletions(-)
 create mode 100644 fs/gfs2/aops.h

-- 
2.17.0



[Cluster-devel] [PATCH v5 11/14] gfs2: iomap direct I/O support

2018-05-30 Thread Andreas Gruenbacher
The page unmapping previously done in gfs2_direct_IO is now done
generically in iomap_dio_rw.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/aops.c | 100 +--
 fs/gfs2/bmap.c |  15 +-
 fs/gfs2/file.c | 138 +
 3 files changed, 143 insertions(+), 110 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 3d9633175aa8..b0a219fae162 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -84,12 +84,6 @@ static int gfs2_get_block_noalloc(struct inode *inode, 
sector_t lblock,
return 0;
 }
 
-static int gfs2_get_block_direct(struct inode *inode, sector_t lblock,
-struct buffer_head *bh_result, int create)
-{
-   return gfs2_block_map(inode, lblock, bh_result, 0);
-}
-
 /**
  * gfs2_writepage_common - Common bits of writepage
  * @page: The page to be written
@@ -1021,96 +1015,6 @@ static void gfs2_invalidatepage(struct page *page, 
unsigned int offset,
try_to_release_page(page, 0);
 }
 
-/**
- * gfs2_ok_for_dio - check that dio is valid on this file
- * @ip: The inode
- * @offset: The offset at which we are reading or writing
- *
- * Returns: 0 (to ignore the i/o request and thus fall back to buffered i/o)
- *  1 (to accept the i/o request)
- */
-static int gfs2_ok_for_dio(struct gfs2_inode *ip, loff_t offset)
-{
-   /*
-* Should we return an error here? I can't see that O_DIRECT for
-* a stuffed file makes any sense. For now we'll silently fall
-* back to buffered I/O
-*/
-   if (gfs2_is_stuffed(ip))
-   return 0;
-
-   if (offset >= i_size_read(>i_inode))
-   return 0;
-   return 1;
-}
-
-
-
-static ssize_t gfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
-{
-   struct file *file = iocb->ki_filp;
-   struct inode *inode = file->f_mapping->host;
-   struct address_space *mapping = inode->i_mapping;
-   struct gfs2_inode *ip = GFS2_I(inode);
-   loff_t offset = iocb->ki_pos;
-   struct gfs2_holder gh;
-   int rv;
-
-   /*
-* Deferred lock, even if its a write, since we do no allocation
-* on this path. All we need change is atime, and this lock mode
-* ensures that other nodes have flushed their buffered read caches
-* (i.e. their page cache entries for this inode). We do not,
-* unfortunately have the option of only flushing a range like
-* the VFS does.
-*/
-   gfs2_holder_init(ip->i_gl, LM_ST_DEFERRED, 0, );
-   rv = gfs2_glock_nq();
-   if (rv)
-   goto out_uninit;
-   rv = gfs2_ok_for_dio(ip, offset);
-   if (rv != 1)
-   goto out; /* dio not valid, fall back to buffered i/o */
-
-   /*
-* Now since we are holding a deferred (CW) lock at this point, you
-* might be wondering why this is ever needed. There is a case however
-* where we've granted a deferred local lock against a cached exclusive
-* glock. That is ok provided all granted local locks are deferred, but
-* it also means that it is possible to encounter pages which are
-* cached and possibly also mapped. So here we check for that and sort
-* them out ahead of the dio. The glock state machine will take care of
-* everything else.
-*
-* If in fact the cached glock state (gl->gl_state) is deferred (CW) in
-* the first place, mapping->nr_pages will always be zero.
-*/
-   if (mapping->nrpages) {
-   loff_t lstart = offset & ~(PAGE_SIZE - 1);
-   loff_t len = iov_iter_count(iter);
-   loff_t end = PAGE_ALIGN(offset + len) - 1;
-
-   rv = 0;
-   if (len == 0)
-   goto out;
-   if (test_and_clear_bit(GIF_SW_PAGED, >i_flags))
-   unmap_shared_mapping_range(ip->i_inode.i_mapping, 
offset, len);
-   rv = filemap_write_and_wait_range(mapping, lstart, end);
-   if (rv)
-   goto out;
-   if (iov_iter_rw(iter) == WRITE)
-   truncate_inode_pages_range(mapping, lstart, end);
-   }
-
-   rv = __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev, iter,
- gfs2_get_block_direct, NULL, NULL, 0);
-out:
-   gfs2_glock_dq();
-out_uninit:
-   gfs2_holder_uninit();
-   return rv;
-}
-
 /**
  * gfs2_releasepage - free the metadata associated with a page
  * @page: the page that's being released
@@ -1191,7 +1095,7 @@ static const struct address_space_operations 
gfs2_writeback_aops = {
.bmap = gfs2_bmap,
.invalidatepage = gfs2_invalidatepage,
.releasepage = gfs2_releasepage,
-   .direct_IO = gfs2_direct_IO,
+   .direct_IO = noop_direct_IO,
.migratepage = buffer_migrate_page,
.is_partially_uptodate = 

[Cluster-devel] [PATCH v5 03/14] gfs2: gfs2_stuffed_write_end cleanup

2018-05-30 Thread Andreas Gruenbacher
First, change the sanity check in gfs2_stuffed_write_end to check for
the actual write size instead of the requested write size.

Second, use the existing teardown code in gfs2_write_end instead of
duplicating it in gfs2_stuffed_write_end.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/aops.c | 49 ++---
 1 file changed, 18 insertions(+), 31 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index f58716567972..e8c0387652ed 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -814,7 +814,6 @@ static void adjust_fs_space(struct inode *inode)
  * @inode: The inode
  * @dibh: The buffer_head containing the on-disk inode
  * @pos: The file position
- * @len: The length of the write
  * @copied: How much was actually copied by the VFS
  * @page: The page
  *
@@ -824,17 +823,15 @@ static void adjust_fs_space(struct inode *inode)
  * Returns: errno
  */
 static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head 
*dibh,
- loff_t pos, unsigned len, unsigned copied,
+ loff_t pos, unsigned copied,
  struct page *page)
 {
struct gfs2_inode *ip = GFS2_I(inode);
-   struct gfs2_sbd *sdp = GFS2_SB(inode);
-   struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
u64 to = pos + copied;
void *kaddr;
unsigned char *buf = dibh->b_data + sizeof(struct gfs2_dinode);
 
-   BUG_ON(pos + len > gfs2_max_stuffed_size(ip));
+   BUG_ON(pos + copied > gfs2_max_stuffed_size(ip));
 
kaddr = kmap_atomic(page);
memcpy(buf + pos, kaddr + pos, copied);
@@ -850,20 +847,6 @@ static int gfs2_stuffed_write_end(struct inode *inode, 
struct buffer_head *dibh,
i_size_write(inode, to);
mark_inode_dirty(inode);
}
-
-   if (inode == sdp->sd_rindex) {
-   adjust_fs_space(inode);
-   sdp->sd_rindex_uptodate = 0;
-   }
-
-   brelse(dibh);
-   gfs2_trans_end(sdp);
-   if (inode == sdp->sd_rindex) {
-   gfs2_glock_dq(_ip->i_gh);
-   gfs2_holder_uninit(_ip->i_gh);
-   }
-   gfs2_glock_dq(>i_gh);
-   gfs2_holder_uninit(>i_gh);
return copied;
 }
 
@@ -877,9 +860,8 @@ static int gfs2_stuffed_write_end(struct inode *inode, 
struct buffer_head *dibh,
  * @page: The page that has been written
  * @fsdata: The fsdata (unused in GFS2)
  *
- * The main write_end function for GFS2. We have a separate one for
- * stuffed files as they are slightly different, otherwise we just
- * put our locking around the VFS provided functions.
+ * The main write_end function for GFS2. We just put our locking around the VFS
+ * provided functions.
  *
  * Returns: errno
  */
@@ -900,32 +882,37 @@ static int gfs2_write_end(struct file *file, struct 
address_space *mapping,
BUG_ON(gfs2_glock_is_locked_by_me(ip->i_gl) == NULL);
 
ret = gfs2_meta_inode_buffer(ip, );
-   if (unlikely(ret)) {
-   unlock_page(page);
-   put_page(page);
-   goto failed;
-   }
+   if (unlikely(ret))
+   goto out;
 
-   if (gfs2_is_stuffed(ip))
-   return gfs2_stuffed_write_end(inode, dibh, pos, len, copied, 
page);
+   if (gfs2_is_stuffed(ip)) {
+   ret = gfs2_stuffed_write_end(inode, dibh, pos, copied, page);
+   page = NULL;
+   goto out2;
+   }
 
if (!gfs2_is_writeback(ip))
gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
 
ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
+   page = NULL;
if (tr->tr_num_buf_new)
__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
else
gfs2_trans_add_meta(ip->i_gl, dibh);
 
-
+out2:
if (inode == sdp->sd_rindex) {
adjust_fs_space(inode);
sdp->sd_rindex_uptodate = 0;
}
 
brelse(dibh);
-failed:
+out:
+   if (page) {
+   unlock_page(page);
+   put_page(page);
+   }
gfs2_trans_end(sdp);
gfs2_inplace_release(ip);
if (ip->i_qadata && ip->i_qadata->qa_qd_num)
-- 
2.17.0



[Cluster-devel] [PATCH v5 14/14] gfs2: Handle stuffed files in iomap_{begin, end}

2018-05-30 Thread Andreas Gruenbacher
Proof of concept: move the gfs2 stuffed file handling from the
iomap_write_{begin,end} handlers to the iomap_{begin,end} handlers.
With this, the page written to is locked before faulting in the page to
read from, so when both refer to the same page, we end up with a
deadlock as demonstrated by xfstest generic/248.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/bmap.c| 91 ---
 fs/iomap.c| 21 ++
 include/linux/iomap.h |  1 +
 3 files changed, 57 insertions(+), 56 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index c157af31dc56..05b599a70de6 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -966,15 +966,30 @@ static int gfs2_iomap_begin_write(struct inode *inode, 
loff_t pos, loff_t length
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
unsigned int data_blocks = 0, ind_blocks = 0, rblocks;
-   bool unstuff, alloc_required;
+   bool unstuff = false, alloc_required;
int ret;
 
ret = gfs2_write_lock(inode);
if (ret)
return ret;
 
-   unstuff = gfs2_is_stuffed(ip) &&
- pos + length > gfs2_max_stuffed_size(ip);
+   if (gfs2_is_stuffed(ip)) {
+   unstuff = pos + length > gfs2_max_stuffed_size(ip);
+
+   if (!unstuff) {
+   iomap->page = grab_cache_page_write_begin(
+   inode->i_mapping, 0, flags);
+   ret = -ENOMEM;
+   if (!iomap->page)
+   goto out;
+
+   if (!PageUptodate(iomap->page)) {
+   ret = stuffed_readpage(ip, iomap->page);
+   if (ret)
+   goto out;
+   }
+   }
+   }
 
ret = gfs2_iomap_get(inode, pos, length, flags, iomap, );
if (ret)
@@ -1044,6 +1059,12 @@ static int gfs2_iomap_begin_write(struct inode *inode, 
loff_t pos, loff_t length
gfs2_quota_unlock(ip);
 out_release:
release_metapath();
+out:
+   if (iomap->page) {
+   unlock_page(iomap->page);
+   put_page(iomap->page);
+   iomap->page = NULL;
+   }
gfs2_write_unlock(inode);
return ret;
 }
@@ -1073,54 +1094,10 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t 
pos, loff_t length,
return ret;
 }
 
-static int
-gfs2_iomap_write_begin(struct inode *inode, loff_t pos, unsigned len,
-  unsigned flags, struct page **pagep, struct iomap *iomap)
-{
-   struct gfs2_inode *ip = GFS2_I(inode);
-   struct page *page;
-   int ret;
-
-   if (gfs2_is_stuffed(ip)) {
-   BUG_ON(pos + len > gfs2_max_stuffed_size(ip));
-
-   page = grab_cache_page_write_begin(inode->i_mapping, 0, flags);
-   if (!page)
-   return -ENOMEM;
-
-   if (!PageUptodate(page)) {
-   ret = stuffed_readpage(ip, page);
-   if (ret) {
-   unlock_page(page);
-   put_page(page);
-   return ret;
-   }
-   }
-   *pagep = page;
-   return 0;
-   }
-
-   return iomap_write_begin(inode, pos, len, flags, pagep, iomap);
-}
-
 static int gfs2_iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
unsigned copied, struct page *page)
 {
struct gfs2_inode *ip = GFS2_I(inode);
-   struct buffer_head *dibh;
-   int ret;
-
-   if (gfs2_is_stuffed(ip)) {
-   ret = gfs2_meta_inode_buffer(ip, );
-   if (ret) {
-   unlock_page(page);
-   put_page(page);
-   return ret;
-   }
-   ret = gfs2_stuffed_write_end(inode, dibh, pos, copied, page);
-   brelse(dibh);
-   return ret;
-   }
 
if (gfs2_is_jdata(ip))
gfs2_page_add_databufs(ip, page, offset_in_page(pos), len);
@@ -1139,6 +1116,23 @@ static int gfs2_iomap_end(struct inode *inode, loff_t 
pos, loff_t length,
if ((flags & (IOMAP_WRITE | IOMAP_DIRECT)) != IOMAP_WRITE)
return 0;
 
+   if (iomap->page) {
+   struct buffer_head *dibh;
+
+   ret = gfs2_meta_inode_buffer(ip, );
+   if (ret) {
+   unlock_page(iomap->page);
+   put_page(iomap->page);
+   iomap->page = NULL;
+   goto out;
+   }
+   ret = gfs2_stuffed_write_end(inode, dibh, pos, written,
+iomap->page);
+   iomap->page = NULL;
+   brelse(dibh);
+ 

[Cluster-devel] [PATCH v5 06/14] iomap: Add write_{begin, end} iomap operations

2018-05-30 Thread Andreas Gruenbacher
Add write_begin and write_end operations to struct iomap_ops to provide
a way of overriding the default behavior of iomap_write_begin and
iomap_write_end.  This is needed for implementing data journaling: in
the data journaling case, pages are written into the journal before
being written back to their proper on-disk locations.

Signed-off-by: Andreas Gruenbacher 
Cc: Dave Chinner 
---
 fs/ext2/inode.c   |  2 ++
 fs/ext4/inode.c   |  2 ++
 fs/iomap.c| 62 ++-
 fs/xfs/xfs_iomap.c|  2 ++
 include/linux/iomap.h | 22 +++
 5 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 71635909df3b..3da481f5e36e 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -848,6 +848,8 @@ ext2_iomap_end(struct inode *inode, loff_t offset, loff_t 
length,
 
 const struct iomap_ops ext2_iomap_ops = {
.iomap_begin= ext2_iomap_begin,
+   .write_begin= iomap_write_begin,
+   .write_end  = iomap_write_end,
.iomap_end  = ext2_iomap_end,
 };
 #else
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1e50c5efae67..05c8e8015f13 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3604,6 +3604,8 @@ static int ext4_iomap_end(struct inode *inode, loff_t 
offset, loff_t length,
 
 const struct iomap_ops ext4_iomap_ops = {
.iomap_begin= ext4_iomap_begin,
+   .write_begin= iomap_write_begin,
+   .write_end  = iomap_write_end,
.iomap_end  = ext4_iomap_end,
 };
 
diff --git a/fs/iomap.c b/fs/iomap.c
index afd163586aa0..27d97a290623 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -108,7 +108,7 @@ iomap_write_failed(struct inode *inode, loff_t pos, 
unsigned len)
truncate_pagecache_range(inode, max(pos, i_size), pos + len);
 }
 
-static int
+int
 iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned 
flags,
struct page **pagep, struct iomap *iomap)
 {
@@ -137,8 +137,9 @@ iomap_write_begin(struct inode *inode, loff_t pos, unsigned 
len, unsigned flags,
*pagep = page;
return status;
 }
+EXPORT_SYMBOL_GPL(iomap_write_begin);
 
-static int
+int
 iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
unsigned copied, struct page *page)
 {
@@ -150,12 +151,19 @@ iomap_write_end(struct inode *inode, loff_t pos, unsigned 
len,
iomap_write_failed(inode, pos, len);
return ret;
 }
+EXPORT_SYMBOL_GPL(iomap_write_end);
+
+struct iomap_write_args {
+   struct iov_iter *iter;
+   const struct iomap_ops *ops;
+};
 
 static loff_t
 iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
struct iomap *iomap)
 {
-   struct iov_iter *i = data;
+   struct iomap_write_args *args = data;
+   struct iov_iter *i = args->iter;
long status = 0;
ssize_t written = 0;
unsigned int flags = AOP_FLAG_NOFS;
@@ -188,7 +196,7 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t 
length, void *data,
break;
}
 
-   status = iomap_write_begin(inode, pos, bytes, flags, ,
+   status = args->ops->write_begin(inode, pos, bytes, flags, ,
iomap);
if (unlikely(status))
break;
@@ -200,7 +208,7 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t 
length, void *data,
 
flush_dcache_page(page);
 
-   status = iomap_write_end(inode, pos, bytes, copied, page);
+   status = args->ops->write_end(inode, pos, bytes, copied, page);
if (unlikely(status < 0))
break;
copied = status;
@@ -237,10 +245,14 @@ iomap_file_buffered_write(struct kiocb *iocb, struct 
iov_iter *iter,
 {
struct inode *inode = iocb->ki_filp->f_mapping->host;
loff_t pos = iocb->ki_pos, ret = 0, written = 0;
+   struct iomap_write_args args = {
+   .iter = iter,
+   .ops = ops,
+   };
 
while (iov_iter_count(iter)) {
ret = iomap_apply(inode, pos, iov_iter_count(iter),
-   IOMAP_WRITE, ops, iter, iomap_write_actor);
+   IOMAP_WRITE, ops, , iomap_write_actor);
if (ret <= 0)
break;
pos += ret;
@@ -271,6 +283,7 @@ static loff_t
 iomap_dirty_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
struct iomap *iomap)
 {
+   const struct iomap_ops *ops = data;
long status = 0;
ssize_t written = 0;
 
@@ -286,15 +299,15 @@ iomap_dirty_actor(struct inode *inode, loff_t pos, loff_t 
length, void *data,
if (IS_ERR(rpage))
return PTR_ERR(rpage);
 
-   status = 

Re: [Cluster-devel] [PATCH 11/34] iomap: move IOMAP_F_BOUNDARY to gfs2

2018-05-30 Thread Steven Whitehouse

Hi,


On 30/05/18 06:50, Darrick J. Wong wrote:

On Wed, May 23, 2018 at 04:43:34PM +0200, Christoph Hellwig wrote:

Just define a range of fs specific flags and use that in gfs2 instead of
exposing this internal flag flobally.

Signed-off-by: Christoph Hellwig 

Looks ok to me, but better if the gfs2 folks [cc'd now] ack this...
Reviewed-by: Darrick J. Wong 

--D
I may have missed the context here, but I thought that the boundary was 
a generic thing meaning "there will have to be a metadata read before 
more blocks can be mapped" so I'm not sure why that would now be GFS2 
specific?


Steve.


---
  fs/gfs2/bmap.c| 8 +---
  include/linux/iomap.h | 9 +++--
  2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index cbeedd3cfb36..8efa6297e19c 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -683,6 +683,8 @@ static void gfs2_stuffed_iomap(struct inode *inode, struct 
iomap *iomap)
iomap->type = IOMAP_INLINE;
  }
  
+#define IOMAP_F_GFS2_BOUNDARY IOMAP_F_PRIVATE

+
  /**
   * gfs2_iomap_begin - Map blocks from an inode to disk blocks
   * @inode: The inode
@@ -774,7 +776,7 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, 
loff_t length,
bh = mp.mp_bh[ip->i_height - 1];
len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, lend - lblock, 
);
if (eob)
-   iomap->flags |= IOMAP_F_BOUNDARY;
+   iomap->flags |= IOMAP_F_GFS2_BOUNDARY;
iomap->length = (u64)len << inode->i_blkbits;
  
  out_release:

@@ -846,12 +848,12 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
  
  	if (iomap.length > bh_map->b_size) {

iomap.length = bh_map->b_size;
-   iomap.flags &= ~IOMAP_F_BOUNDARY;
+   iomap.flags &= ~IOMAP_F_GFS2_BOUNDARY;
}
if (iomap.addr != IOMAP_NULL_ADDR)
map_bh(bh_map, inode->i_sb, iomap.addr >> inode->i_blkbits);
bh_map->b_size = iomap.length;
-   if (iomap.flags & IOMAP_F_BOUNDARY)
+   if (iomap.flags & IOMAP_F_GFS2_BOUNDARY)
set_buffer_boundary(bh_map);
if (iomap.flags & IOMAP_F_NEW)
set_buffer_new(bh_map);
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 13d19b4c29a9..819e0cd2a950 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -27,8 +27,7 @@ struct vm_fault;
   * written data and requires fdatasync to commit them to persistent storage.
   */
  #define IOMAP_F_NEW   0x01/* blocks have been newly allocated */
-#define IOMAP_F_BOUNDARY   0x02/* mapping ends at metadata boundary */
-#define IOMAP_F_DIRTY  0x04/* uncommitted metadata */
+#define IOMAP_F_DIRTY  0x02/* uncommitted metadata */
  
  /*

   * Flags that only need to be reported for IOMAP_REPORT requests:
@@ -36,6 +35,12 @@ struct vm_fault;
  #define IOMAP_F_MERGED0x10/* contains multiple 
blocks/extents */
  #define IOMAP_F_SHARED0x20/* block shared with another 
file */
  
+/*

+ * Flags from 0x1000 up are for file system specific usage:
+ */
+#define IOMAP_F_PRIVATE0x1000
+
+
  /*
   * Magic value for addr:
   */
--
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Cluster-devel] [PATCH] gfs2-utils: Fix bashism in test script

2018-05-30 Thread Valentin Vidic
Tests fail when /bin/sh is not /bin/bash due to missing let command:

 21: Values of rg_data0, rg_data, rg_bitbytesFAILED (mkfs.at:146)
 /<>/tests/rgrifieldscheck.sh: 19: let: not found
---
 tests/rgrifieldscheck.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/rgrifieldscheck.sh b/tests/rgrifieldscheck.sh
index 85f6408e..1fbf6729 100755
--- a/tests/rgrifieldscheck.sh
+++ b/tests/rgrifieldscheck.sh
@@ -16,5 +16,5 @@ do
exit 1
fi
 
-   test $field = ri_bitbytes && let i++
+   test $field = ri_bitbytes && i=$((i+1))
 done
-- 
2.17.0