* new helpers to clean up extent tree lookups V2
@ 2016-11-21 16:38 Christoph Hellwig
2016-11-21 16:38 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
` (14 more replies)
0 siblings, 15 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Now that Eric send out xfs_iext_count before I could get to it I need
to get my other iext helpers out ASAP before running into another
rebase :)
This series adds two new helpers that make iterating the extent list
a lot cleaner. They are the first step towards better abstracting out
access to the extent list and thus allowing us to experiment with
better data structures for it. The next step is the manipulations
in xfs_bmap_{add,del}_extent*, which will be more work than these
helpers, but I have some of that in progress.
Changes since V1:
- incorporate review feedback from Brian (various minor items)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 01/14] xfs: new inode extent list lookup helpers
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 02/14] xfs: cleanup xfs_bmap_last_before Christoph Hellwig
` (13 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
xfs_iext_lookup_extent looks up a single extent at the passed in offset,
and returns the extent covering the area, or the one behind it in case
of a hole, as well as the index of the returned extent in arguments,
as well as a simple bool as return value that is set to false if no
extent could be found because the offset is behind EOF. It is a simpler
replacement for xfs_bmap_search_extent that leaves looking up the rarely
needed previous extent to the caller and has a nicer calling convention.
xfs_iext_get_extent is a helper for iterating over the extent list,
it takes an extent index as input, and returns the extent at that index
in it's expanded form in an argument if it exists. The actual return
value is a bool whether the index is valid or not.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_inode_fork.c | 46 ++++++++++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_inode_fork.h | 6 ++++++
2 files changed, 52 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 5fbe24c..222e103 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -2003,3 +2003,49 @@ xfs_ifork_init_cow(
ip->i_cformat = XFS_DINODE_FMT_EXTENTS;
ip->i_cnextents = 0;
}
+
+/*
+ * Lookup the extent covering bno.
+ *
+ * If there is an extent covering bno return the extent index, and store the
+ * expanded extent structure in *gotp, and the extent index in *idx.
+ * If there is no extent covering bno, but there is an extent after it (e.g.
+ * it lies in a hole) return that extent in *gotp and its index in *idx
+ * instead.
+ * If bno is beyond the last extent return false, and return the index after
+ * the last valid index in *idxp.
+ */
+bool
+xfs_iext_lookup_extent(
+ struct xfs_inode *ip,
+ struct xfs_ifork *ifp,
+ xfs_fileoff_t bno,
+ xfs_extnum_t *idxp,
+ struct xfs_bmbt_irec *gotp)
+{
+ struct xfs_bmbt_rec_host *ep;
+
+ XFS_STATS_INC(ip->i_mount, xs_look_exlist);
+
+ ep = xfs_iext_bno_to_ext(ifp, bno, idxp);
+ if (!ep)
+ return false;
+ xfs_bmbt_get_all(ep, gotp);
+ return true;
+}
+
+/*
+ * Return true if there is an extent at index idx, and return the expanded
+ * extent structure at idx in that case. Else return false.
+ */
+bool
+xfs_iext_get_extent(
+ struct xfs_ifork *ifp,
+ xfs_extnum_t idx,
+ struct xfs_bmbt_irec *gotp)
+{
+ if (idx < 0 || idx >= xfs_iext_count(ifp))
+ return false;
+ xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), gotp);
+ return true;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 8bf112e..7fb8365 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -182,6 +182,12 @@ void xfs_iext_irec_compact_pages(struct xfs_ifork *);
void xfs_iext_irec_compact_full(struct xfs_ifork *);
void xfs_iext_irec_update_extoffs(struct xfs_ifork *, int, int);
+bool xfs_iext_lookup_extent(struct xfs_inode *ip,
+ struct xfs_ifork *ifp, xfs_fileoff_t bno,
+ xfs_extnum_t *idxp, struct xfs_bmbt_irec *gotp);
+bool xfs_iext_get_extent(struct xfs_ifork *ifp, xfs_extnum_t idx,
+ struct xfs_bmbt_irec *gotp);
+
extern struct kmem_zone *xfs_ifork_zone;
extern void xfs_ifork_init_cow(struct xfs_inode *ip);
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 02/14] xfs: cleanup xfs_bmap_last_before
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
2016-11-21 16:38 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 17:19 ` Brian Foster
2016-11-21 16:38 ` [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read Christoph Hellwig
` (12 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Rewrite the function using xfs_iext_lookup_extent and xfs_iext_get_extent,
and massage the flow into something easily understandable.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 64 ++++++++++++++++++++++++------------------------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 5c3c4dd..98f490b 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1523,44 +1523,44 @@ xfs_bmap_first_unused(
*/
int /* error */
xfs_bmap_last_before(
- xfs_trans_t *tp, /* transaction pointer */
- xfs_inode_t *ip, /* incore inode */
- xfs_fileoff_t *last_block, /* last block */
- int whichfork) /* data or attr fork */
+ struct xfs_trans *tp, /* transaction pointer */
+ struct xfs_inode *ip, /* incore inode */
+ xfs_fileoff_t *last_block, /* last block */
+ int whichfork) /* data or attr fork */
{
- xfs_fileoff_t bno; /* input file offset */
- int eof; /* hit end of file */
- xfs_bmbt_rec_host_t *ep; /* pointer to last extent */
- int error; /* error return value */
- xfs_bmbt_irec_t got; /* current extent value */
- xfs_ifork_t *ifp; /* inode fork pointer */
- xfs_extnum_t lastx; /* last extent used */
- xfs_bmbt_irec_t prev; /* previous extent value */
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
+ struct xfs_bmbt_irec got;
+ xfs_extnum_t idx;
+ int error;
- if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
- return -EIO;
- if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
+ switch (XFS_IFORK_FORMAT(ip, whichfork)) {
+ case XFS_DINODE_FMT_LOCAL:
*last_block = 0;
return 0;
+ case XFS_DINODE_FMT_BTREE:
+ case XFS_DINODE_FMT_EXTENTS:
+ break;
+ default:
+ return -EIO;
}
- ifp = XFS_IFORK_PTR(ip, whichfork);
- if (!(ifp->if_flags & XFS_IFEXTENTS) &&
- (error = xfs_iread_extents(tp, ip, whichfork)))
- return error;
- bno = *last_block - 1;
- ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
- &prev);
- if (eof || xfs_bmbt_get_startoff(ep) > bno) {
- if (prev.br_startoff == NULLFILEOFF)
- *last_block = 0;
- else
- *last_block = prev.br_startoff + prev.br_blockcount;
+
+ if (!(ifp->if_flags & XFS_IFEXTENTS)) {
+ error = xfs_iread_extents(tp, ip, whichfork);
+ if (error)
+ return error;
}
- /*
- * Otherwise *last_block is already the right answer.
- */
+
+ if (xfs_iext_lookup_extent(ip, ifp, *last_block - 1, &idx, &got)) {
+ if (got.br_startoff <= *last_block - 1)
+ return 0;
+ }
+
+ if (xfs_iext_get_extent(ifp, idx - 1, &got)) {
+ *last_block = got.br_startoff + got.br_blockcount;
+ return 0;
+ }
+
+ *last_block = 0;
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
2016-11-21 16:38 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
2016-11-21 16:38 ` [PATCH 02/14] xfs: cleanup xfs_bmap_last_before Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write Christoph Hellwig
` (11 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 98f490b..1a0fee4 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4145,12 +4145,11 @@ xfs_bmapi_read(
struct xfs_mount *mp = ip->i_mount;
struct xfs_ifork *ifp;
struct xfs_bmbt_irec got;
- struct xfs_bmbt_irec prev;
xfs_fileoff_t obno;
xfs_fileoff_t end;
- xfs_extnum_t lastx;
+ xfs_extnum_t idx;
int error;
- int eof;
+ bool eof = false;
int n = 0;
int whichfork = xfs_bmapi_whichfork(flags);
@@ -4190,7 +4189,8 @@ xfs_bmapi_read(
return error;
}
- xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got, &prev);
+ if (!xfs_iext_lookup_extent(ip, ifp, bno, &idx, &got))
+ eof = true;
end = bno + len;
obno = bno;
@@ -4221,10 +4221,8 @@ xfs_bmapi_read(
break;
/* Else go on to the next record. */
- if (++lastx < xfs_iext_count(ifp))
- xfs_bmbt_get_all(xfs_iext_get_ext(ifp, lastx), &got);
- else
- eof = 1;
+ if (!xfs_iext_get_extent(ifp, ++idx, &got))
+ eof = true;
}
*nmap = n;
return 0;
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (2 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 17:19 ` Brian Foster
2016-11-21 16:38 ` [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi Christoph Hellwig
` (10 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 1a0fee4..9a8621d 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4561,7 +4561,7 @@ xfs_bmapi_write(
struct xfs_ifork *ifp;
struct xfs_bmalloca bma = { NULL }; /* args for xfs_bmap_alloc */
xfs_fileoff_t end; /* end of mapped file region */
- int eof; /* after the end of extents */
+ bool eof = false; /* after the end of extents */
int error; /* error return */
int n; /* current extent index */
xfs_fileoff_t obno; /* old block number (offset) */
@@ -4639,12 +4639,14 @@ xfs_bmapi_write(
goto error0;
}
- xfs_bmap_search_extents(ip, bno, whichfork, &eof, &bma.idx, &bma.got,
- &bma.prev);
n = 0;
end = bno + len;
obno = bno;
+ if (!xfs_iext_lookup_extent(ip, ifp, bno, &bma.idx, &bma.got))
+ eof = true;
+ if (!xfs_iext_get_extent(ifp, bma.idx - 1, &bma.prev))
+ bma.prev.br_startoff = NULLFILEOFF;
bma.tp = tp;
bma.ip = ip;
bma.total = total;
@@ -4731,11 +4733,8 @@ xfs_bmapi_write(
/* Else go on to the next record. */
bma.prev = bma.got;
- if (++bma.idx < xfs_iext_count(ifp)) {
- xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma.idx),
- &bma.got);
- } else
- eof = 1;
+ if (!xfs_iext_get_extent(ifp, ++bma.idx, &bma.got))
+ eof = true;
}
*nmap = n;
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (3 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc Christoph Hellwig
` (9 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 40 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9a8621d..3ed2157 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5433,8 +5433,6 @@ __xfs_bunmapi(
{
xfs_btree_cur_t *cur; /* bmap btree cursor */
xfs_bmbt_irec_t del; /* extent being deleted */
- int eof; /* is deleting at eof */
- xfs_bmbt_rec_host_t *ep; /* extent record pointer */
int error; /* error return value */
xfs_extnum_t extno; /* extent number in list */
xfs_bmbt_irec_t got; /* current extent record */
@@ -5444,7 +5442,6 @@ __xfs_bunmapi(
int logflags; /* transaction logging flags */
xfs_extlen_t mod; /* rt extent offset */
xfs_mount_t *mp; /* mount structure */
- xfs_bmbt_irec_t prev; /* previous extent record */
xfs_fileoff_t start; /* first file offset deleted */
int tmp_logflags; /* partial logging flags */
int wasdel; /* was a delayed alloc extent */
@@ -5483,18 +5480,17 @@ __xfs_bunmapi(
isrt = (whichfork == XFS_DATA_FORK) && XFS_IS_REALTIME_INODE(ip);
start = bno;
bno = start + len - 1;
- ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
- &prev);
/*
* Check to see if the given block number is past the end of the
* file, back up to the last block if so...
*/
- if (eof) {
- ep = xfs_iext_get_ext(ifp, --lastx);
- xfs_bmbt_get_all(ep, &got);
+ if (!xfs_iext_lookup_extent(ip, ifp, bno, &lastx, &got)) {
+ ASSERT(lastx > 0);
+ xfs_iext_get_extent(ifp, --lastx, &got);
bno = got.br_startoff + got.br_blockcount - 1;
}
+
logflags = 0;
if (ifp->if_flags & XFS_IFBROOT) {
ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE);
@@ -5525,8 +5521,7 @@ __xfs_bunmapi(
if (got.br_startoff > bno) {
if (--lastx < 0)
break;
- ep = xfs_iext_get_ext(ifp, lastx);
- xfs_bmbt_get_all(ep, &got);
+ xfs_iext_get_extent(ifp, lastx, &got);
}
/*
* Is the last block of this extent before the range
@@ -5540,7 +5535,6 @@ __xfs_bunmapi(
* Then deal with the (possibly delayed) allocated space
* we found.
*/
- ASSERT(ep != NULL);
del = got;
wasdel = isnullstartblock(del.br_startblock);
if (got.br_startoff < start) {
@@ -5621,15 +5615,12 @@ __xfs_bunmapi(
*/
ASSERT(bno >= del.br_blockcount);
bno -= del.br_blockcount;
- if (got.br_startoff > bno) {
- if (--lastx >= 0) {
- ep = xfs_iext_get_ext(ifp,
- lastx);
- xfs_bmbt_get_all(ep, &got);
- }
- }
+ if (got.br_startoff > bno && --lastx >= 0)
+ xfs_iext_get_extent(ifp, lastx, &got);
continue;
} else if (del.br_state == XFS_EXT_UNWRITTEN) {
+ struct xfs_bmbt_irec prev;
+
/*
* This one is already unwritten.
* It must have a written left neighbor.
@@ -5637,8 +5628,7 @@ __xfs_bunmapi(
* try again.
*/
ASSERT(lastx > 0);
- xfs_bmbt_get_all(xfs_iext_get_ext(ifp,
- lastx - 1), &prev);
+ xfs_iext_get_extent(ifp, lastx - 1, &prev);
ASSERT(prev.br_state == XFS_EXT_NORM);
ASSERT(!isnullstartblock(prev.br_startblock));
ASSERT(del.br_startblock ==
@@ -5736,13 +5726,9 @@ __xfs_bunmapi(
*/
if (bno != (xfs_fileoff_t)-1 && bno >= start) {
if (lastx >= 0) {
- ep = xfs_iext_get_ext(ifp, lastx);
- if (xfs_bmbt_get_startoff(ep) > bno) {
- if (--lastx >= 0)
- ep = xfs_iext_get_ext(ifp,
- lastx);
- }
- xfs_bmbt_get_all(ep, &got);
+ xfs_iext_get_extent(ifp, lastx, &got);
+ if (got.br_startoff > bno && --lastx >= 0)
+ xfs_iext_get_extent(ifp, lastx, &got);
}
extno++;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (4 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 17:19 ` Brian Foster
2016-11-21 16:38 ` [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay Christoph Hellwig
` (8 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
We can easily lookup the previous extent for the cases where we need it,
which saves the callers from looking it up for us later in the series.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 8 ++++++--
fs/xfs/libxfs/xfs_bmap.h | 3 +--
fs/xfs/xfs_iomap.c | 3 +--
fs/xfs/xfs_reflink.c | 2 +-
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 3ed2157..f5de137 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4235,7 +4235,6 @@ xfs_bmapi_reserve_delalloc(
xfs_fileoff_t aoff,
xfs_filblks_t len,
struct xfs_bmbt_irec *got,
- struct xfs_bmbt_irec *prev,
xfs_extnum_t *lastx,
int eof)
{
@@ -4257,7 +4256,12 @@ xfs_bmapi_reserve_delalloc(
else
extsz = xfs_get_extsz_hint(ip);
if (extsz) {
- error = xfs_bmap_extsize_align(mp, got, prev, extsz, rt, eof,
+ struct xfs_bmbt_irec prev;
+
+ if (!xfs_iext_get_extent(ifp, *lastx - 1, &prev))
+ prev.br_startoff = NULLFILEOFF;
+
+ error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
1, 0, &aoff, &alen);
ASSERT(!error);
}
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 7cae6ec..e3c2b5a 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -243,8 +243,7 @@ struct xfs_bmbt_rec_host *
struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
xfs_fileoff_t aoff, xfs_filblks_t len,
- struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *prev,
- xfs_extnum_t *lastx, int eof);
+ struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
enum xfs_bmap_intent_type {
XFS_BMAP_MAP = 1,
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 436e109..59ffcac 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -622,8 +622,7 @@ xfs_file_iomap_begin_delay(
retry:
error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
- end_fsb - offset_fsb, &got,
- &prev, &idx, eof);
+ end_fsb - offset_fsb, &got, &idx, eof);
switch (error) {
case 0:
break;
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 0edf835..52cdfba 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -293,7 +293,7 @@ xfs_reflink_reserve_cow(
retry:
error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
- end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
+ end_fsb - imap->br_startoff, &got, &idx, eof);
switch (error) {
case 0:
break;
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (5 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow Christoph Hellwig
` (7 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
And only lookup the previous extent inside xfs_iomap_prealloc_size
if we actually need it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_iomap.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 59ffcac..2272190 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -395,11 +395,12 @@ xfs_iomap_prealloc_size(
struct xfs_inode *ip,
loff_t offset,
loff_t count,
- xfs_extnum_t idx,
- struct xfs_bmbt_irec *prev)
+ xfs_extnum_t idx)
{
struct xfs_mount *mp = ip->i_mount;
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
xfs_fileoff_t offset_fsb = XFS_B_TO_FSBT(mp, offset);
+ struct xfs_bmbt_irec prev;
int shift = 0;
int64_t freesp;
xfs_fsblock_t qblocks;
@@ -419,8 +420,8 @@ xfs_iomap_prealloc_size(
*/
if ((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) ||
XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_dalign) ||
- idx == 0 ||
- prev->br_startoff + prev->br_blockcount < offset_fsb)
+ !xfs_iext_get_extent(ifp, idx - 1, &prev) ||
+ prev.br_startoff + prev.br_blockcount < offset_fsb)
return mp->m_writeio_blocks;
/*
@@ -439,8 +440,8 @@ xfs_iomap_prealloc_size(
* always extends to MAXEXTLEN rather than falling short due to things
* like stripe unit/width alignment of real extents.
*/
- if (prev->br_blockcount <= (MAXEXTLEN >> 1))
- alloc_blocks = prev->br_blockcount << 1;
+ if (prev.br_blockcount <= (MAXEXTLEN >> 1))
+ alloc_blocks = prev.br_blockcount << 1;
else
alloc_blocks = XFS_B_TO_FSB(mp, offset);
if (!alloc_blocks)
@@ -538,7 +539,6 @@ xfs_file_iomap_begin_delay(
xfs_fileoff_t end_fsb, orig_end_fsb;
int error = 0, eof = 0;
struct xfs_bmbt_irec got;
- struct xfs_bmbt_irec prev;
xfs_extnum_t idx;
ASSERT(!XFS_IS_REALTIME_INODE(ip));
@@ -563,8 +563,7 @@ xfs_file_iomap_begin_delay(
goto out_unlock;
}
- xfs_bmap_search_extents(ip, offset_fsb, XFS_DATA_FORK, &eof, &idx,
- &got, &prev);
+ eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got);
if (!eof && got.br_startoff <= offset_fsb) {
if (xfs_is_reflink_inode(ip)) {
bool shared;
@@ -601,8 +600,7 @@ xfs_file_iomap_begin_delay(
if (eof) {
xfs_fsblock_t prealloc_blocks;
- prealloc_blocks =
- xfs_iomap_prealloc_size(ip, offset, count, idx, &prev);
+ prealloc_blocks = xfs_iomap_prealloc_size(ip, offset, count, idx);
if (prealloc_blocks) {
xfs_extlen_t align;
xfs_off_t end_offset;
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (6 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping Christoph Hellwig
` (6 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_reflink.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 52cdfba..35e02ce 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -243,10 +243,11 @@ xfs_reflink_reserve_cow(
struct xfs_bmbt_irec *imap,
bool *shared)
{
- struct xfs_bmbt_irec got, prev;
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+ struct xfs_bmbt_irec got;
xfs_fileoff_t end_fsb, orig_end_fsb;
- int eof = 0, error = 0;
- bool trimmed;
+ int error = 0;
+ bool eof = false, trimmed;
xfs_extnum_t idx;
xfs_extlen_t align;
@@ -258,8 +259,9 @@ xfs_reflink_reserve_cow(
* extent list is generally faster than going out to the shared extent
* tree.
*/
- xfs_bmap_search_extents(ip, imap->br_startoff, XFS_COW_FORK, &eof, &idx,
- &got, &prev);
+
+ if (!xfs_iext_lookup_extent(ip, ifp, imap->br_startoff, &idx, &got))
+ eof = true;
if (!eof && got.br_startoff <= imap->br_startoff) {
trace_xfs_reflink_cow_found(ip, imap);
xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (7 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow Christoph Hellwig
` (5 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Use xfs_iext_lookup_extent to look up the extent, drop a useless check,
drop a unneeded return value and clean up the general style a little bit.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_aops.c | 16 ++++++++--------
fs/xfs/xfs_reflink.c | 33 ++++++++++-----------------------
fs/xfs/xfs_reflink.h | 2 +-
3 files changed, 19 insertions(+), 32 deletions(-)
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index dd6cacf..ab266d6 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -777,7 +777,7 @@ xfs_map_cow(
{
struct xfs_inode *ip = XFS_I(inode);
struct xfs_bmbt_irec imap;
- bool is_cow = false, need_alloc = false;
+ bool is_cow = false;
int error;
/*
@@ -795,7 +795,7 @@ xfs_map_cow(
* Else we need to check if there is a COW mapping at this offset.
*/
xfs_ilock(ip, XFS_ILOCK_SHARED);
- is_cow = xfs_reflink_find_cow_mapping(ip, offset, &imap, &need_alloc);
+ is_cow = xfs_reflink_find_cow_mapping(ip, offset, &imap);
xfs_iunlock(ip, XFS_ILOCK_SHARED);
if (!is_cow)
@@ -805,7 +805,7 @@ xfs_map_cow(
* And if the COW mapping has a delayed extent here we need to
* allocate real space for it now.
*/
- if (need_alloc) {
+ if (isnullstartblock(imap.br_startblock)) {
error = xfs_iomap_write_allocate(ip, XFS_COW_FORK, offset,
&imap);
if (error)
@@ -1311,7 +1311,6 @@ __xfs_get_blocks(
ssize_t size;
int new = 0;
bool is_cow = false;
- bool need_alloc = false;
BUG_ON(create && !direct);
@@ -1337,9 +1336,11 @@ __xfs_get_blocks(
end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + size);
offset_fsb = XFS_B_TO_FSBT(mp, offset);
- if (create && direct && xfs_is_reflink_inode(ip))
- is_cow = xfs_reflink_find_cow_mapping(ip, offset, &imap,
- &need_alloc);
+ if (create && direct && xfs_is_reflink_inode(ip)) {
+ is_cow = xfs_reflink_find_cow_mapping(ip, offset, &imap);
+ ASSERT(!is_cow || !isnullstartblock(imap.br_startblock));
+ }
+
if (!is_cow) {
error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
&imap, &nimaps, XFS_BMAPI_ENTIRE);
@@ -1356,7 +1357,6 @@ __xfs_get_blocks(
xfs_reflink_trim_irec_to_next_cow(ip, offset_fsb,
&imap);
}
- ASSERT(!need_alloc);
if (error)
goto out_unlock;
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 35e02ce..e92355a 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -420,44 +420,31 @@ xfs_reflink_allocate_cow_range(
}
/*
- * Find the CoW reservation (and whether or not it needs block allocation)
- * for a given byte offset of a file.
+ * Find the CoW reservation for a given byte offset of a file.
*/
bool
xfs_reflink_find_cow_mapping(
struct xfs_inode *ip,
xfs_off_t offset,
- struct xfs_bmbt_irec *imap,
- bool *need_alloc)
+ struct xfs_bmbt_irec *imap)
{
- struct xfs_bmbt_irec irec;
- struct xfs_ifork *ifp;
- struct xfs_bmbt_rec_host *gotp;
- xfs_fileoff_t bno;
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+ xfs_fileoff_t offset_fsb;
+ struct xfs_bmbt_irec got;
xfs_extnum_t idx;
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
ASSERT(xfs_is_reflink_inode(ip));
- /* Find the extent in the CoW fork. */
- ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
- bno = XFS_B_TO_FSBT(ip->i_mount, offset);
- gotp = xfs_iext_bno_to_ext(ifp, bno, &idx);
- if (!gotp)
+ offset_fsb = XFS_B_TO_FSBT(ip->i_mount, offset);
+ if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
return false;
-
- xfs_bmbt_get_all(gotp, &irec);
- if (bno >= irec.br_startoff + irec.br_blockcount ||
- bno < irec.br_startoff)
+ if (got.br_startoff > offset_fsb)
return false;
trace_xfs_reflink_find_cow_mapping(ip, offset, 1, XFS_IO_OVERWRITE,
- &irec);
-
- /* If it's still delalloc, we must allocate later. */
- *imap = irec;
- *need_alloc = !!(isnullstartblock(irec.br_startblock));
-
+ &got);
+ *imap = got;
return true;
}
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 97ea9b4..cff5fc3 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -31,7 +31,7 @@ extern int xfs_reflink_reserve_cow(struct xfs_inode *ip,
extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
xfs_off_t offset, xfs_off_t count);
extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
- struct xfs_bmbt_irec *imap, bool *need_alloc);
+ struct xfs_bmbt_irec *imap);
extern int xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
xfs_fileoff_t offset_fsb, struct xfs_bmbt_irec *imap);
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (8 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks Christoph Hellwig
` (4 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
And remove the unused return value.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_reflink.c | 33 ++++++++++++---------------------
fs/xfs/xfs_reflink.h | 2 +-
2 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index e92355a..d3cfae8 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -451,43 +451,34 @@ xfs_reflink_find_cow_mapping(
/*
* Trim an extent to end at the next CoW reservation past offset_fsb.
*/
-int
+void
xfs_reflink_trim_irec_to_next_cow(
struct xfs_inode *ip,
xfs_fileoff_t offset_fsb,
struct xfs_bmbt_irec *imap)
{
- struct xfs_bmbt_irec irec;
- struct xfs_ifork *ifp;
- struct xfs_bmbt_rec_host *gotp;
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+ struct xfs_bmbt_irec got;
xfs_extnum_t idx;
if (!xfs_is_reflink_inode(ip))
- return 0;
+ return;
/* Find the extent in the CoW fork. */
- ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
- gotp = xfs_iext_bno_to_ext(ifp, offset_fsb, &idx);
- if (!gotp)
- return 0;
- xfs_bmbt_get_all(gotp, &irec);
+ if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
+ return;
/* This is the extent before; try sliding up one. */
- if (irec.br_startoff < offset_fsb) {
- idx++;
- if (idx >= xfs_iext_count(ifp))
- return 0;
- gotp = xfs_iext_get_ext(ifp, idx);
- xfs_bmbt_get_all(gotp, &irec);
+ if (got.br_startoff < offset_fsb) {
+ if (!xfs_iext_get_extent(ifp, idx + 1, &got))
+ return;
}
- if (irec.br_startoff >= imap->br_startoff + imap->br_blockcount)
- return 0;
+ if (got.br_startoff >= imap->br_startoff + imap->br_blockcount)
+ return;
- imap->br_blockcount = irec.br_startoff - imap->br_startoff;
+ imap->br_blockcount = got.br_startoff - imap->br_startoff;
trace_xfs_reflink_trim_irec(ip, imap);
-
- return 0;
}
/*
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index cff5fc3..aa6a4d6 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -32,7 +32,7 @@ extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
xfs_off_t offset, xfs_off_t count);
extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
struct xfs_bmbt_irec *imap);
-extern int xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
+extern void xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
xfs_fileoff_t offset_fsb, struct xfs_bmbt_irec *imap);
extern int xfs_reflink_cancel_cow_blocks(struct xfs_inode *ip,
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (9 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow Christoph Hellwig
` (3 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_reflink.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index d3cfae8..4e07da3 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -492,18 +492,15 @@ xfs_reflink_cancel_cow_blocks(
xfs_fileoff_t end_fsb)
{
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
- struct xfs_bmbt_irec got, prev, del;
+ struct xfs_bmbt_irec got, del;
xfs_extnum_t idx;
xfs_fsblock_t firstfsb;
struct xfs_defer_ops dfops;
- int error = 0, eof = 0;
+ int error = 0;
if (!xfs_is_reflink_inode(ip))
return 0;
-
- xfs_bmap_search_extents(ip, offset_fsb, XFS_COW_FORK, &eof, &idx,
- &got, &prev);
- if (eof)
+ if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
return 0;
while (got.br_startoff < end_fsb) {
@@ -546,9 +543,8 @@ xfs_reflink_cancel_cow_blocks(
xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
}
- if (++idx >= xfs_iext_count(ifp))
+ if (!xfs_iext_get_extent(ifp, ++idx, &got))
break;
- xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
}
/* clear tag if cow fork is emptied */
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (10 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 13/14] xfs: remove xfs_bmap_search_extents Christoph Hellwig
` (2 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_reflink.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 4e07da3..56372be 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -614,13 +614,13 @@ xfs_reflink_end_cow(
xfs_off_t count)
{
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
- struct xfs_bmbt_irec got, prev, del;
+ struct xfs_bmbt_irec got, del;
struct xfs_trans *tp;
xfs_fileoff_t offset_fsb;
xfs_fileoff_t end_fsb;
xfs_fsblock_t firstfsb;
struct xfs_defer_ops dfops;
- int error, eof = 0;
+ int error;
unsigned int resblks;
xfs_filblks_t rlen;
xfs_extnum_t idx;
@@ -644,13 +644,11 @@ xfs_reflink_end_cow(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
- xfs_bmap_search_extents(ip, end_fsb - 1, XFS_COW_FORK, &eof, &idx,
- &got, &prev);
-
/* If there is a hole at end_fsb - 1 go to the previous extent */
- if (eof || got.br_startoff > end_fsb) {
+ if (!xfs_iext_lookup_extent(ip, ifp, end_fsb - 1, &idx, &got) ||
+ got.br_startoff > end_fsb) {
ASSERT(idx > 0);
- xfs_bmbt_get_all(xfs_iext_get_ext(ifp, --idx), &got);
+ xfs_iext_get_extent(ifp, --idx, &got);
}
/* Walk backwards until we're out of the I/O range... */
@@ -698,11 +696,9 @@ xfs_reflink_end_cow(
error = xfs_defer_finish(&tp, &dfops, ip);
if (error)
goto out_defer;
-
next_extent:
- if (idx < 0)
+ if (!xfs_iext_get_extent(ifp, idx, &got))
break;
- xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
}
error = xfs_trans_commit(tp);
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 13/14] xfs: remove xfs_bmap_search_extents
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (11 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 16:38 ` [PATCH 14/14] xfs: remove NULLEXTNUM Christoph Hellwig
2016-11-22 17:46 ` new helpers to clean up extent tree lookups V2 Darrick J. Wong
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
Now that all users are gone.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 91 ------------------------------------------------
fs/xfs/libxfs/xfs_bmap.h | 4 ---
2 files changed, 95 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index f5de137..23cf045 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1370,97 +1370,6 @@ xfs_bmap_read_extents(
return -EFSCORRUPTED;
}
-
-/*
- * Search the extent records for the entry containing block bno.
- * If bno lies in a hole, point to the next entry. If bno lies
- * past eof, *eofp will be set, and *prevp will contain the last
- * entry (null if none). Else, *lastxp will be set to the index
- * of the found entry; *gotp will contain the entry.
- */
-STATIC xfs_bmbt_rec_host_t * /* pointer to found extent entry */
-xfs_bmap_search_multi_extents(
- xfs_ifork_t *ifp, /* inode fork pointer */
- xfs_fileoff_t bno, /* block number searched for */
- int *eofp, /* out: end of file found */
- xfs_extnum_t *lastxp, /* out: last extent index */
- xfs_bmbt_irec_t *gotp, /* out: extent entry found */
- xfs_bmbt_irec_t *prevp) /* out: previous extent entry found */
-{
- xfs_bmbt_rec_host_t *ep; /* extent record pointer */
- xfs_extnum_t lastx; /* last extent index */
-
- /*
- * Initialize the extent entry structure to catch access to
- * uninitialized br_startblock field.
- */
- gotp->br_startoff = 0xffa5a5a5a5a5a5a5LL;
- gotp->br_blockcount = 0xa55a5a5a5a5a5a5aLL;
- gotp->br_state = XFS_EXT_INVALID;
- gotp->br_startblock = 0xffffa5a5a5a5a5a5LL;
- prevp->br_startoff = NULLFILEOFF;
-
- ep = xfs_iext_bno_to_ext(ifp, bno, &lastx);
- if (lastx > 0) {
- xfs_bmbt_get_all(xfs_iext_get_ext(ifp, lastx - 1), prevp);
- }
- if (lastx < xfs_iext_count(ifp)) {
- xfs_bmbt_get_all(ep, gotp);
- *eofp = 0;
- } else {
- if (lastx > 0) {
- *gotp = *prevp;
- }
- *eofp = 1;
- ep = NULL;
- }
- *lastxp = lastx;
- return ep;
-}
-
-/*
- * Search the extents list for the inode, for the extent containing bno.
- * If bno lies in a hole, point to the next entry. If bno lies past eof,
- * *eofp will be set, and *prevp will contain the last entry (null if none).
- * Else, *lastxp will be set to the index of the found
- * entry; *gotp will contain the entry.
- */
-xfs_bmbt_rec_host_t * /* pointer to found extent entry */
-xfs_bmap_search_extents(
- xfs_inode_t *ip, /* incore inode pointer */
- xfs_fileoff_t bno, /* block number searched for */
- int fork, /* data or attr fork */
- int *eofp, /* out: end of file found */
- xfs_extnum_t *lastxp, /* out: last extent index */
- xfs_bmbt_irec_t *gotp, /* out: extent entry found */
- xfs_bmbt_irec_t *prevp) /* out: previous extent entry found */
-{
- xfs_ifork_t *ifp; /* inode fork pointer */
- xfs_bmbt_rec_host_t *ep; /* extent record pointer */
-
- XFS_STATS_INC(ip->i_mount, xs_look_exlist);
- ifp = XFS_IFORK_PTR(ip, fork);
-
- ep = xfs_bmap_search_multi_extents(ifp, bno, eofp, lastxp, gotp, prevp);
-
- if (unlikely(!(gotp->br_startblock) && (*lastxp != NULLEXTNUM) &&
- !(XFS_IS_REALTIME_INODE(ip) && fork == XFS_DATA_FORK))) {
- xfs_alert_tag(ip->i_mount, XFS_PTAG_FSBLOCK_ZERO,
- "Access to block zero in inode %llu "
- "start_block: %llx start_off: %llx "
- "blkcnt: %llx extent-state: %x lastx: %x",
- (unsigned long long)ip->i_ino,
- (unsigned long long)gotp->br_startblock,
- (unsigned long long)gotp->br_startoff,
- (unsigned long long)gotp->br_blockcount,
- gotp->br_state, *lastxp);
- *lastxp = NULLEXTNUM;
- *eofp = 1;
- return NULL;
- }
- return ep;
-}
-
/*
* Returns the file-relative block number of the first unused block(s)
* in the file with at least "len" logically contiguous blocks free.
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index e3c2b5a..ffed1f9 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -237,10 +237,6 @@ int xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
struct xfs_defer_ops *dfops, enum shift_direction direction,
int num_exts);
int xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
-struct xfs_bmbt_rec_host *
- xfs_bmap_search_extents(struct xfs_inode *ip, xfs_fileoff_t bno,
- int fork, int *eofp, xfs_extnum_t *lastxp,
- struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
xfs_fileoff_t aoff, xfs_filblks_t len,
struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 14/14] xfs: remove NULLEXTNUM
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (12 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 13/14] xfs: remove xfs_bmap_search_extents Christoph Hellwig
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-22 17:46 ` new helpers to clean up extent tree lookups V2 Darrick J. Wong
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-21 16:38 UTC (permalink / raw)
To: linux-xfs
We only ever set a field to this constant for an impossible to reach
error case in xfs_bmap_search_extents. That functions has been removed,
so we can remove the constant as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 2 +-
fs/xfs/libxfs/xfs_types.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 23cf045..4a5b335 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4260,7 +4260,7 @@ xfs_bmapi_allocate(
if (bma->wasdel) {
bma->length = (xfs_extlen_t)bma->got.br_blockcount;
bma->offset = bma->got.br_startoff;
- if (bma->idx != NULLEXTNUM && bma->idx) {
+ if (bma->idx) {
xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma->idx - 1),
&bma->prev);
}
diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
index cf044c0..717909f 100644
--- a/fs/xfs/libxfs/xfs_types.h
+++ b/fs/xfs/libxfs/xfs_types.h
@@ -57,7 +57,6 @@ typedef __int64_t xfs_sfiloff_t; /* signed block number in a file */
#define NULLAGBLOCK ((xfs_agblock_t)-1)
#define NULLAGNUMBER ((xfs_agnumber_t)-1)
-#define NULLEXTNUM ((xfs_extnum_t)-1)
#define NULLCOMMITLSN ((xfs_lsn_t)-1)
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 02/14] xfs: cleanup xfs_bmap_last_before
2016-11-21 16:38 ` [PATCH 02/14] xfs: cleanup xfs_bmap_last_before Christoph Hellwig
@ 2016-11-21 17:19 ` Brian Foster
0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2016-11-21 17:19 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 21, 2016 at 05:38:44PM +0100, Christoph Hellwig wrote:
> Rewrite the function using xfs_iext_lookup_extent and xfs_iext_get_extent,
> and massage the flow into something easily understandable.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_bmap.c | 64 ++++++++++++++++++++++++------------------------
> 1 file changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 5c3c4dd..98f490b 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1523,44 +1523,44 @@ xfs_bmap_first_unused(
> */
> int /* error */
> xfs_bmap_last_before(
> - xfs_trans_t *tp, /* transaction pointer */
> - xfs_inode_t *ip, /* incore inode */
> - xfs_fileoff_t *last_block, /* last block */
> - int whichfork) /* data or attr fork */
> + struct xfs_trans *tp, /* transaction pointer */
> + struct xfs_inode *ip, /* incore inode */
> + xfs_fileoff_t *last_block, /* last block */
> + int whichfork) /* data or attr fork */
> {
> - xfs_fileoff_t bno; /* input file offset */
> - int eof; /* hit end of file */
> - xfs_bmbt_rec_host_t *ep; /* pointer to last extent */
> - int error; /* error return value */
> - xfs_bmbt_irec_t got; /* current extent value */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> - xfs_extnum_t lastx; /* last extent used */
> - xfs_bmbt_irec_t prev; /* previous extent value */
> + struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> + struct xfs_bmbt_irec got;
> + xfs_extnum_t idx;
> + int error;
>
> - if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
> - XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
> - XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> - return -EIO;
> - if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
> + switch (XFS_IFORK_FORMAT(ip, whichfork)) {
> + case XFS_DINODE_FMT_LOCAL:
> *last_block = 0;
> return 0;
> + case XFS_DINODE_FMT_BTREE:
> + case XFS_DINODE_FMT_EXTENTS:
> + break;
> + default:
> + return -EIO;
> }
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> - if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> - (error = xfs_iread_extents(tp, ip, whichfork)))
> - return error;
> - bno = *last_block - 1;
> - ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
> - &prev);
> - if (eof || xfs_bmbt_get_startoff(ep) > bno) {
> - if (prev.br_startoff == NULLFILEOFF)
> - *last_block = 0;
> - else
> - *last_block = prev.br_startoff + prev.br_blockcount;
> +
> + if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> + error = xfs_iread_extents(tp, ip, whichfork);
> + if (error)
> + return error;
> }
> - /*
> - * Otherwise *last_block is already the right answer.
> - */
> +
> + if (xfs_iext_lookup_extent(ip, ifp, *last_block - 1, &idx, &got)) {
> + if (got.br_startoff <= *last_block - 1)
> + return 0;
> + }
> +
> + if (xfs_iext_get_extent(ifp, idx - 1, &got)) {
> + *last_block = got.br_startoff + got.br_blockcount;
> + return 0;
> + }
> +
> + *last_block = 0;
> return 0;
> }
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write
2016-11-21 16:38 ` [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write Christoph Hellwig
@ 2016-11-21 17:19 ` Brian Foster
0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2016-11-21 17:19 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 21, 2016 at 05:38:46PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_bmap.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 1a0fee4..9a8621d 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4561,7 +4561,7 @@ xfs_bmapi_write(
> struct xfs_ifork *ifp;
> struct xfs_bmalloca bma = { NULL }; /* args for xfs_bmap_alloc */
> xfs_fileoff_t end; /* end of mapped file region */
> - int eof; /* after the end of extents */
> + bool eof = false; /* after the end of extents */
> int error; /* error return */
> int n; /* current extent index */
> xfs_fileoff_t obno; /* old block number (offset) */
> @@ -4639,12 +4639,14 @@ xfs_bmapi_write(
> goto error0;
> }
>
> - xfs_bmap_search_extents(ip, bno, whichfork, &eof, &bma.idx, &bma.got,
> - &bma.prev);
> n = 0;
> end = bno + len;
> obno = bno;
>
> + if (!xfs_iext_lookup_extent(ip, ifp, bno, &bma.idx, &bma.got))
> + eof = true;
> + if (!xfs_iext_get_extent(ifp, bma.idx - 1, &bma.prev))
> + bma.prev.br_startoff = NULLFILEOFF;
> bma.tp = tp;
> bma.ip = ip;
> bma.total = total;
> @@ -4731,11 +4733,8 @@ xfs_bmapi_write(
>
> /* Else go on to the next record. */
> bma.prev = bma.got;
> - if (++bma.idx < xfs_iext_count(ifp)) {
> - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma.idx),
> - &bma.got);
> - } else
> - eof = 1;
> + if (!xfs_iext_get_extent(ifp, ++bma.idx, &bma.got))
> + eof = true;
> }
> *nmap = n;
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc
2016-11-21 16:38 ` [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc Christoph Hellwig
@ 2016-11-21 17:19 ` Brian Foster
0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2016-11-21 17:19 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 21, 2016 at 05:38:48PM +0100, Christoph Hellwig wrote:
> We can easily lookup the previous extent for the cases where we need it,
> which saves the callers from looking it up for us later in the series.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_bmap.c | 8 ++++++--
> fs/xfs/libxfs/xfs_bmap.h | 3 +--
> fs/xfs/xfs_iomap.c | 3 +--
> fs/xfs/xfs_reflink.c | 2 +-
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 3ed2157..f5de137 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4235,7 +4235,6 @@ xfs_bmapi_reserve_delalloc(
> xfs_fileoff_t aoff,
> xfs_filblks_t len,
> struct xfs_bmbt_irec *got,
> - struct xfs_bmbt_irec *prev,
> xfs_extnum_t *lastx,
> int eof)
> {
> @@ -4257,7 +4256,12 @@ xfs_bmapi_reserve_delalloc(
> else
> extsz = xfs_get_extsz_hint(ip);
> if (extsz) {
> - error = xfs_bmap_extsize_align(mp, got, prev, extsz, rt, eof,
> + struct xfs_bmbt_irec prev;
> +
> + if (!xfs_iext_get_extent(ifp, *lastx - 1, &prev))
> + prev.br_startoff = NULLFILEOFF;
> +
> + error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
> 1, 0, &aoff, &alen);
> ASSERT(!error);
> }
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 7cae6ec..e3c2b5a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -243,8 +243,7 @@ struct xfs_bmbt_rec_host *
> struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
> int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
> xfs_fileoff_t aoff, xfs_filblks_t len,
> - struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *prev,
> - xfs_extnum_t *lastx, int eof);
> + struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
>
> enum xfs_bmap_intent_type {
> XFS_BMAP_MAP = 1,
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 436e109..59ffcac 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -622,8 +622,7 @@ xfs_file_iomap_begin_delay(
>
> retry:
> error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
> - end_fsb - offset_fsb, &got,
> - &prev, &idx, eof);
> + end_fsb - offset_fsb, &got, &idx, eof);
> switch (error) {
> case 0:
> break;
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 0edf835..52cdfba 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -293,7 +293,7 @@ xfs_reflink_reserve_cow(
>
> retry:
> error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
> - end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
> + end_fsb - imap->br_startoff, &got, &idx, eof);
> switch (error) {
> case 0:
> break;
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: new helpers to clean up extent tree lookups V2
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
` (13 preceding siblings ...)
2016-11-21 16:38 ` [PATCH 14/14] xfs: remove NULLEXTNUM Christoph Hellwig
@ 2016-11-22 17:46 ` Darrick J. Wong
14 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2016-11-22 17:46 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 21, 2016 at 05:38:42PM +0100, Christoph Hellwig wrote:
> Now that Eric send out xfs_iext_count before I could get to it I need
> to get my other iext helpers out ASAP before running into another
> rebase :)
>
> This series adds two new helpers that make iterating the extent list
> a lot cleaner. They are the first step towards better abstracting out
> access to the extent list and thus allowing us to experiment with
> better data structures for it. The next step is the manipulations
> in xfs_bmap_{add,del}_extent*, which will be more work than these
> helpers, but I have some of that in progress.
>
> Changes since V1:
> - incorporate review feedback from Brian (various minor items)
Looks reasonable to me, so for the whole series:
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc
2016-11-18 8:19 ` Christoph Hellwig
@ 2016-11-18 13:19 ` Brian Foster
0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2016-11-18 13:19 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Fri, Nov 18, 2016 at 09:19:44AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 17, 2016 at 01:27:07PM -0500, Brian Foster wrote:
> > It just hit me that extnum_t is signed and xfs_iext_get_extent() checks
> > for < 0, so that covers here and my similar previous few comments. I
> > still think we should probably check it in context rather than bury the
> > check in the caller (I'd prefer an assert). Just my .02.
>
> There are several callers that rely on xfs_iext_get_extent handling
> negative extents with a NULL return - in fact one reason for the
> exact prototype of the function is to cover out of bound indices
> that happen during normal operation based on how we iterate over
> the extent list.
Fair enough. I'm not a fan of the approach in principle, but I'm less
worried about it given that it's not an actual bug.
Brian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc
2016-11-17 18:27 ` Brian Foster
@ 2016-11-18 8:19 ` Christoph Hellwig
2016-11-18 13:19 ` Brian Foster
0 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-18 8:19 UTC (permalink / raw)
To: Brian Foster; +Cc: Christoph Hellwig, linux-xfs
On Thu, Nov 17, 2016 at 01:27:07PM -0500, Brian Foster wrote:
> It just hit me that extnum_t is signed and xfs_iext_get_extent() checks
> for < 0, so that covers here and my similar previous few comments. I
> still think we should probably check it in context rather than bury the
> check in the caller (I'd prefer an assert). Just my .02.
There are several callers that rely on xfs_iext_get_extent handling
negative extents with a NULL return - in fact one reason for the
exact prototype of the function is to cover out of bound indices
that happen during normal operation based on how we iterate over
the extent list.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc
2016-11-14 17:12 ` [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc Christoph Hellwig
@ 2016-11-17 18:27 ` Brian Foster
2016-11-18 8:19 ` Christoph Hellwig
0 siblings, 1 reply; 23+ messages in thread
From: Brian Foster @ 2016-11-17 18:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:37PM +0100, Christoph Hellwig wrote:
> We can easily lookup the previous extent for the cases where we need it,
> which saves the callers from looking it up for us later in the series.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 8 ++++++--
> fs/xfs/libxfs/xfs_bmap.h | 3 +--
> fs/xfs/xfs_iomap.c | 3 +--
> fs/xfs/xfs_reflink.c | 2 +-
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 18de89c..4aa9c07 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4235,7 +4235,6 @@ xfs_bmapi_reserve_delalloc(
> xfs_fileoff_t aoff,
> xfs_filblks_t len,
> struct xfs_bmbt_irec *got,
> - struct xfs_bmbt_irec *prev,
> xfs_extnum_t *lastx,
> int eof)
> {
> @@ -4257,7 +4256,12 @@ xfs_bmapi_reserve_delalloc(
> else
> extsz = xfs_get_extsz_hint(ip);
> if (extsz) {
> - error = xfs_bmap_extsize_align(mp, got, prev, extsz, rt, eof,
> + struct xfs_bmbt_irec prev;
> +
> + if (!xfs_iext_get_extent(ifp, *lastx - 1, &prev))
> + prev.br_startoff = NULLFILEOFF;
It just hit me that extnum_t is signed and xfs_iext_get_extent() checks
for < 0, so that covers here and my similar previous few comments. I
still think we should probably check it in context rather than bury the
check in the caller (I'd prefer an assert). Just my .02.
Brian
> +
> + error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
> 1, 0, &aoff, &alen);
> ASSERT(!error);
> }
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 7cae6ec..e3c2b5a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -243,8 +243,7 @@ struct xfs_bmbt_rec_host *
> struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
> int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
> xfs_fileoff_t aoff, xfs_filblks_t len,
> - struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *prev,
> - xfs_extnum_t *lastx, int eof);
> + struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
>
> enum xfs_bmap_intent_type {
> XFS_BMAP_MAP = 1,
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 436e109..59ffcac 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -622,8 +622,7 @@ xfs_file_iomap_begin_delay(
>
> retry:
> error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
> - end_fsb - offset_fsb, &got,
> - &prev, &idx, eof);
> + end_fsb - offset_fsb, &got, &idx, eof);
> switch (error) {
> case 0:
> break;
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 0edf835..52cdfba 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -293,7 +293,7 @@ xfs_reflink_reserve_cow(
>
> retry:
> error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
> - end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
> + end_fsb - imap->br_startoff, &got, &idx, eof);
> switch (error) {
> case 0:
> break;
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 18:27 ` Brian Foster
0 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 UTC (permalink / raw)
To: linux-xfs
We can easily lookup the previous extent for the cases where we need it,
which saves the callers from looking it up for us later in the series.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 8 ++++++--
fs/xfs/libxfs/xfs_bmap.h | 3 +--
fs/xfs/xfs_iomap.c | 3 +--
fs/xfs/xfs_reflink.c | 2 +-
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 18de89c..4aa9c07 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4235,7 +4235,6 @@ xfs_bmapi_reserve_delalloc(
xfs_fileoff_t aoff,
xfs_filblks_t len,
struct xfs_bmbt_irec *got,
- struct xfs_bmbt_irec *prev,
xfs_extnum_t *lastx,
int eof)
{
@@ -4257,7 +4256,12 @@ xfs_bmapi_reserve_delalloc(
else
extsz = xfs_get_extsz_hint(ip);
if (extsz) {
- error = xfs_bmap_extsize_align(mp, got, prev, extsz, rt, eof,
+ struct xfs_bmbt_irec prev;
+
+ if (!xfs_iext_get_extent(ifp, *lastx - 1, &prev))
+ prev.br_startoff = NULLFILEOFF;
+
+ error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
1, 0, &aoff, &alen);
ASSERT(!error);
}
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 7cae6ec..e3c2b5a 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -243,8 +243,7 @@ struct xfs_bmbt_rec_host *
struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
xfs_fileoff_t aoff, xfs_filblks_t len,
- struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *prev,
- xfs_extnum_t *lastx, int eof);
+ struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
enum xfs_bmap_intent_type {
XFS_BMAP_MAP = 1,
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 436e109..59ffcac 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -622,8 +622,7 @@ xfs_file_iomap_begin_delay(
retry:
error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
- end_fsb - offset_fsb, &got,
- &prev, &idx, eof);
+ end_fsb - offset_fsb, &got, &idx, eof);
switch (error) {
case 0:
break;
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 0edf835..52cdfba 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -293,7 +293,7 @@ xfs_reflink_reserve_cow(
retry:
error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
- end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
+ end_fsb - imap->br_startoff, &got, &idx, eof);
switch (error) {
case 0:
break;
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-11-22 17:46 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 Christoph Hellwig
2016-11-21 16:38 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
2016-11-21 16:38 ` [PATCH 02/14] xfs: cleanup xfs_bmap_last_before Christoph Hellwig
2016-11-21 17:19 ` Brian Foster
2016-11-21 16:38 ` [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read Christoph Hellwig
2016-11-21 16:38 ` [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write Christoph Hellwig
2016-11-21 17:19 ` Brian Foster
2016-11-21 16:38 ` [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi Christoph Hellwig
2016-11-21 16:38 ` [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc Christoph Hellwig
2016-11-21 17:19 ` Brian Foster
2016-11-21 16:38 ` [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay Christoph Hellwig
2016-11-21 16:38 ` [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow Christoph Hellwig
2016-11-21 16:38 ` [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping Christoph Hellwig
2016-11-21 16:38 ` [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow Christoph Hellwig
2016-11-21 16:38 ` [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks Christoph Hellwig
2016-11-21 16:38 ` [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow Christoph Hellwig
2016-11-21 16:38 ` [PATCH 13/14] xfs: remove xfs_bmap_search_extents Christoph Hellwig
2016-11-21 16:38 ` [PATCH 14/14] xfs: remove NULLEXTNUM Christoph Hellwig
2016-11-22 17:46 ` new helpers to clean up extent tree lookups V2 Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
2016-11-14 17:12 ` [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc Christoph Hellwig
2016-11-17 18:27 ` Brian Foster
2016-11-18 8:19 ` Christoph Hellwig
2016-11-18 13:19 ` Brian Foster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.