* new helpers to clean up extent tree lookups
@ 2016-11-14 17:12 Christoph Hellwig
2016-11-14 17:12 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
` (14 more replies)
0 siblings, 15 replies; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 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.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 01/14] xfs: new inode extent list lookup helpers
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:11 ` Brian Foster
2016-11-14 17:12 ` [PATCH 02/14] xfs: cleanup xfs_bmap_last_before Christoph Hellwig
` (13 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 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>
---
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..749fbfb 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 indes 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] 38+ messages in thread
* [PATCH 02/14] xfs: cleanup xfs_bmap_last_before
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
2016-11-14 17:12 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 18:11 ` Brian Foster
2016-11-14 17:12 ` [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read Christoph Hellwig
` (12 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 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] 38+ messages in thread
* [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
2016-11-14 17:12 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
2016-11-14 17:12 ` [PATCH 02/14] xfs: cleanup xfs_bmap_last_before Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 18:11 ` Brian Foster
2016-11-14 17:12 ` [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write Christoph Hellwig
` (11 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
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] 38+ messages in thread
* [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (2 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 18:12 ` Brian Foster
2016-11-14 17:12 ` [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi Christoph Hellwig
` (10 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 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] 38+ messages in thread
* [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (3 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 18:12 ` Brian Foster
2016-11-14 17:12 ` [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc Christoph Hellwig
` (9 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 39 ++++++++++++---------------------------
1 file changed, 12 insertions(+), 27 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9a8621d..18de89c 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,11 @@ __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) {
+ xfs_bmbt_irec_t prev;
/*
* This one is already unwritten.
* It must have a written left neighbor.
@@ -5637,8 +5627,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 +5725,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] 38+ 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
` (4 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 18:27 ` Brian Foster
2016-11-14 17:12 ` [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay Christoph Hellwig
` (8 subsequent siblings)
14 siblings, 1 reply; 38+ 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] 38+ messages in thread
* [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (5 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 18:33 ` Brian Foster
2016-11-14 17:12 ` [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow Christoph Hellwig
` (7 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 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>
---
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] 38+ messages in thread
* [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (6 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping Christoph Hellwig
` (6 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
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] 38+ messages in thread
* [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (7 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow Christoph Hellwig
` (5 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 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>
---
fs/xfs/xfs_aops.c | 16 ++++++++--------
fs/xfs/xfs_reflink.c | 30 +++++++++---------------------
fs/xfs/xfs_reflink.h | 2 +-
3 files changed, 18 insertions(+), 30 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..6056fd1 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -427,37 +427,25 @@ 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] 38+ messages in thread
* [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (8 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks Christoph Hellwig
` (4 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 UTC (permalink / raw)
To: linux-xfs
And remove the unused return value.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
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 6056fd1..0668490 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -452,43 +452,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] 38+ messages in thread
* [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (9 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow Christoph Hellwig
` (3 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
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 0668490..a878d42 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -493,18 +493,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) {
@@ -547,9 +544,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] 38+ messages in thread
* [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (10 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 13/14] xfs: remove xfs_bmap_search_extents Christoph Hellwig
` (2 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
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 a878d42..4b024e7 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -615,13 +615,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;
@@ -645,13 +645,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... */
@@ -699,11 +697,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] 38+ messages in thread
* [PATCH 13/14] xfs: remove xfs_bmap_search_extents
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (11 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 19:12 ` Brian Foster
2016-11-14 17:12 ` [PATCH 14/14] xfs: remove NULLEXTNUM Christoph Hellwig
2016-11-19 0:21 ` new helpers to clean up extent tree lookups Eric Sandeen
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 UTC (permalink / raw)
To: linux-xfs
Now that all users are gone.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
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 4aa9c07..856d98d 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] 38+ messages in thread
* [PATCH 14/14] xfs: remove NULLEXTNUM
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (12 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 13/14] xfs: remove xfs_bmap_search_extents Christoph Hellwig
@ 2016-11-14 17:12 ` Christoph Hellwig
2016-11-17 19:12 ` Brian Foster
2016-11-19 0:21 ` new helpers to clean up extent tree lookups Eric Sandeen
14 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-14 17:12 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>
---
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 856d98d..3ab68db 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] 38+ messages in thread
* Re: [PATCH 01/14] xfs: new inode extent list lookup helpers
2016-11-14 17:12 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
@ 2016-11-17 18:11 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 18:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:32PM +0100, Christoph Hellwig wrote:
> 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>
> ---
> 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..749fbfb 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 indes in *idx.
Typo: index
Otherwise looks good:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> + * 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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 02/14] xfs: cleanup xfs_bmap_last_before
2016-11-14 17:12 ` [PATCH 02/14] xfs: cleanup xfs_bmap_last_before Christoph Hellwig
@ 2016-11-17 18:11 ` Brian Foster
2016-11-18 8:16 ` Christoph Hellwig
0 siblings, 1 reply; 38+ messages in thread
From: Brian Foster @ 2016-11-17 18:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:33PM +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>
> ---
> 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)) {
It may not be an issue in current usage, but I think we should check for
idx here (i.e., 'if (idx && xfs_iext_get_extent(..))').
Brian
> + *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] 38+ messages in thread
* Re: [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read
2016-11-14 17:12 ` [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read Christoph Hellwig
@ 2016-11-17 18:11 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 18:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:34PM +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 | 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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write
2016-11-14 17:12 ` [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write Christoph Hellwig
@ 2016-11-17 18:12 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 18:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:35PM +0100, Christoph Hellwig wrote:
> 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))
Similar issue here:
if (!bma.idx || !xfs_iext_get_extent(..))
...
Brian
> + 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] 38+ messages in thread
* Re: [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi
2016-11-14 17:12 ` [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi Christoph Hellwig
@ 2016-11-17 18:12 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 18:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:36PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 39 ++++++++++++---------------------------
> 1 file changed, 12 insertions(+), 27 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 9a8621d..18de89c 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,11 @@ __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);
Slightly tricky, but Ok.
> continue;
> } else if (del.br_state == XFS_EXT_UNWRITTEN) {
> + xfs_bmbt_irec_t prev;
struct xfs_bmbt_irec
Otherwise looks good:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> /*
> * This one is already unwritten.
> * It must have a written left neighbor.
> @@ -5637,8 +5627,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 +5725,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
>
> --
> 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] 38+ 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; 38+ 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] 38+ messages in thread
* Re: [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay
2016-11-14 17:12 ` [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay Christoph Hellwig
@ 2016-11-17 18:33 ` Brian Foster
2016-11-18 8:20 ` Christoph Hellwig
0 siblings, 1 reply; 38+ messages in thread
From: Brian Foster @ 2016-11-17 18:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:38PM +0100, Christoph Hellwig wrote:
> And only lookup the previous extent inside xfs_iomap_prealloc_size
> if we actually need it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> 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)
My cow prealloc series is going to end up adding this right back, fwiw.
Though at that point it's not really "prev" as used throughout the
extent manipulation code, but rather just an extent on which to base the
initial prealloc size.
That aside, looks good to me:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> {
> 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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow
2016-11-14 17:12 ` [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow Christoph Hellwig
@ 2016-11-17 19:07 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 19:07 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:39PM +0100, Christoph Hellwig wrote:
> 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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping
2016-11-14 17:12 ` [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping Christoph Hellwig
@ 2016-11-17 19:07 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 19:07 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:40PM +0100, Christoph Hellwig wrote:
> 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>
> ---
> fs/xfs/xfs_aops.c | 16 ++++++++--------
> fs/xfs/xfs_reflink.c | 30 +++++++++---------------------
> fs/xfs/xfs_reflink.h | 2 +-
> 3 files changed, 18 insertions(+), 30 deletions(-)
>
...
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 35e02ce..6056fd1 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -427,37 +427,25 @@ 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)
The comment above the function needs fixing as well. Otherwise:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> {
> - 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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow
2016-11-14 17:12 ` [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow Christoph Hellwig
@ 2016-11-17 19:07 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 19:07 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:41PM +0100, Christoph Hellwig wrote:
> 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 6056fd1..0668490 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -452,43 +452,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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks
2016-11-14 17:12 ` [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks Christoph Hellwig
@ 2016-11-17 19:07 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 19:07 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:42PM +0100, Christoph Hellwig wrote:
> 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 0668490..a878d42 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -493,18 +493,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) {
> @@ -547,9 +544,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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow
2016-11-14 17:12 ` [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow Christoph Hellwig
@ 2016-11-17 19:07 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 19:07 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:43PM +0100, Christoph Hellwig wrote:
> 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 a878d42..4b024e7 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -615,13 +615,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;
> @@ -645,13 +645,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... */
> @@ -699,11 +697,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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 13/14] xfs: remove xfs_bmap_search_extents
2016-11-14 17:12 ` [PATCH 13/14] xfs: remove xfs_bmap_search_extents Christoph Hellwig
@ 2016-11-17 19:12 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 19:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:44PM +0100, Christoph Hellwig wrote:
> 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 4aa9c07..856d98d 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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 14/14] xfs: remove NULLEXTNUM
2016-11-14 17:12 ` [PATCH 14/14] xfs: remove NULLEXTNUM Christoph Hellwig
@ 2016-11-17 19:12 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-17 19:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Nov 14, 2016 at 06:12:45PM +0100, Christoph Hellwig wrote:
> 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 856d98d..3ab68db 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
>
> --
> 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] 38+ messages in thread
* Re: [PATCH 02/14] xfs: cleanup xfs_bmap_last_before
2016-11-17 18:11 ` Brian Foster
@ 2016-11-18 8:16 ` Christoph Hellwig
0 siblings, 0 replies; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-18 8:16 UTC (permalink / raw)
To: Brian Foster; +Cc: Christoph Hellwig, linux-xfs
On Thu, Nov 17, 2016 at 01:11:54PM -0500, Brian Foster wrote:
> On Mon, Nov 14, 2016 at 06:12:33PM +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>
> > ---
> > 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)) {
>
> It may not be an issue in current usage, but I think we should check for
> idx here (i.e., 'if (idx && xfs_iext_get_extent(..))').
I've designed xfs_iext_get_extent to gracefully handle invalid indices
(including negative ones) and return NULL.
case.
^ permalink raw reply [flat|nested] 38+ 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; 38+ 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] 38+ messages in thread
* Re: [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay
2016-11-17 18:33 ` Brian Foster
@ 2016-11-18 8:20 ` Christoph Hellwig
2016-11-18 13:20 ` Brian Foster
0 siblings, 1 reply; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-18 8:20 UTC (permalink / raw)
To: Brian Foster; +Cc: Christoph Hellwig, linux-xfs
On Thu, Nov 17, 2016 at 01:33:30PM -0500, Brian Foster wrote:
> My cow prealloc series is going to end up adding this right back, fwiw.
> Though at that point it's not really "prev" as used throughout the
> extent manipulation code, but rather just an extent on which to base the
> initial prealloc size.
Oh, well. I could keep the argument for now, but doing the lookups
in the caller just for the prev value that's not even always used
seems a little silly.
^ permalink raw reply [flat|nested] 38+ 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; 38+ 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] 38+ messages in thread
* Re: [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay
2016-11-18 8:20 ` Christoph Hellwig
@ 2016-11-18 13:20 ` Brian Foster
0 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2016-11-18 13:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Fri, Nov 18, 2016 at 09:20:47AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 17, 2016 at 01:33:30PM -0500, Brian Foster wrote:
> > My cow prealloc series is going to end up adding this right back, fwiw.
> > Though at that point it's not really "prev" as used throughout the
> > extent manipulation code, but rather just an extent on which to base the
> > initial prealloc size.
>
> Oh, well. I could keep the argument for now, but doing the lookups
> in the caller just for the prev value that's not even always used
> seems a little silly.
It's probably not worth it. This looks like a good clean up to me as it
is... I just wanted to call that out since I was using/changing 'prev'.
Looking again, the cow prealloc stuff is still going to use the data
extent record. I may be able to convert 'prev' to 'base' within
*_prealloc_size() just the same without changing the function signature
back.
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] 38+ messages in thread
* Re: new helpers to clean up extent tree lookups
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
` (13 preceding siblings ...)
2016-11-14 17:12 ` [PATCH 14/14] xfs: remove NULLEXTNUM Christoph Hellwig
@ 2016-11-19 0:21 ` Eric Sandeen
2016-11-21 17:17 ` Christoph Hellwig
14 siblings, 1 reply; 38+ messages in thread
From: Eric Sandeen @ 2016-11-19 0:21 UTC (permalink / raw)
To: Christoph Hellwig, linux-xfs
On 11/14/16 11:12 AM, 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 :)
I could have deferred if you had spoken up ;)
(I did think that iext_count probably wasn't the end of it)
-Eric
> 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.
> --
> 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] 38+ messages in thread
* Re: new helpers to clean up extent tree lookups
2016-11-19 0:21 ` new helpers to clean up extent tree lookups Eric Sandeen
@ 2016-11-21 17:17 ` Christoph Hellwig
0 siblings, 0 replies; 38+ messages in thread
From: Christoph Hellwig @ 2016-11-21 17:17 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, linux-xfs
On Fri, Nov 18, 2016 at 06:21:32PM -0600, Eric Sandeen wrote:
> On 11/14/16 11:12 AM, 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 :)
>
> I could have deferred if you had spoken up ;)
>
> (I did think that iext_count probably wasn't the end of it)
Np - your patch was a good reminder that I should finally send this
series out.
^ permalink raw reply [flat|nested] 38+ 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; 38+ 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] 38+ 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
@ 2016-11-21 16:38 ` Christoph Hellwig
2016-11-21 17:19 ` Brian Foster
0 siblings, 1 reply; 38+ 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] 38+ messages in thread
end of thread, other threads:[~2016-11-21 17:19 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-14 17:12 new helpers to clean up extent tree lookups Christoph Hellwig
2016-11-14 17:12 ` [PATCH 01/14] xfs: new inode extent list lookup helpers Christoph Hellwig
2016-11-17 18:11 ` Brian Foster
2016-11-14 17:12 ` [PATCH 02/14] xfs: cleanup xfs_bmap_last_before Christoph Hellwig
2016-11-17 18:11 ` Brian Foster
2016-11-18 8:16 ` Christoph Hellwig
2016-11-14 17:12 ` [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read Christoph Hellwig
2016-11-17 18:11 ` Brian Foster
2016-11-14 17:12 ` [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write Christoph Hellwig
2016-11-17 18:12 ` Brian Foster
2016-11-14 17:12 ` [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi Christoph Hellwig
2016-11-17 18:12 ` Brian Foster
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
2016-11-14 17:12 ` [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay Christoph Hellwig
2016-11-17 18:33 ` Brian Foster
2016-11-18 8:20 ` Christoph Hellwig
2016-11-18 13:20 ` Brian Foster
2016-11-14 17:12 ` [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow Christoph Hellwig
2016-11-17 19:07 ` Brian Foster
2016-11-14 17:12 ` [PATCH 13/14] xfs: remove xfs_bmap_search_extents Christoph Hellwig
2016-11-17 19:12 ` Brian Foster
2016-11-14 17:12 ` [PATCH 14/14] xfs: remove NULLEXTNUM Christoph Hellwig
2016-11-17 19:12 ` Brian Foster
2016-11-19 0:21 ` new helpers to clean up extent tree lookups Eric Sandeen
2016-11-21 17:17 ` Christoph Hellwig
2016-11-21 16:38 new helpers to clean up extent tree lookups V2 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
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.