linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xfs: remove support for untagged lookups in xfs_icwalk*
@ 2021-08-13  8:16 Christoph Hellwig
  2021-08-13 16:26 ` Darrick J. Wong
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2021-08-13  8:16 UTC (permalink / raw)
  To: linux-xfs; +Cc: Dan Carpenter

With quotaoff not allowing disabling of accounting there is no need
for untagged lookups in this code, so remove the dead leftovers.

Repoted-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_icache.c | 39 ++++-----------------------------------
 1 file changed, 4 insertions(+), 35 deletions(-)

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index e7e69e55b7680a..f5a52ec084842d 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -43,15 +43,6 @@ enum xfs_icwalk_goal {
 	XFS_ICWALK_RECLAIM	= XFS_ICI_RECLAIM_TAG,
 };
 
-#define XFS_ICWALK_NULL_TAG	(-1U)
-
-/* Compute the inode radix tree tag for this goal. */
-static inline unsigned int
-xfs_icwalk_tag(enum xfs_icwalk_goal goal)
-{
-	return goal < 0 ? XFS_ICWALK_NULL_TAG : goal;
-}
-
 static int xfs_icwalk(struct xfs_mount *mp,
 		enum xfs_icwalk_goal goal, struct xfs_icwalk *icw);
 static int xfs_icwalk_ag(struct xfs_perag *pag,
@@ -1676,22 +1667,14 @@ xfs_icwalk_ag(
 	nr_found = 0;
 	do {
 		struct xfs_inode *batch[XFS_LOOKUP_BATCH];
-		unsigned int	tag = xfs_icwalk_tag(goal);
 		int		error = 0;
 		int		i;
 
 		rcu_read_lock();
 
-		if (tag == XFS_ICWALK_NULL_TAG)
-			nr_found = radix_tree_gang_lookup(&pag->pag_ici_root,
-					(void **)batch, first_index,
-					XFS_LOOKUP_BATCH);
-		else
-			nr_found = radix_tree_gang_lookup_tag(
-					&pag->pag_ici_root,
-					(void **) batch, first_index,
-					XFS_LOOKUP_BATCH, tag);
-
+		nr_found = radix_tree_gang_lookup_tag(&pag->pag_ici_root,
+				(void **) batch, first_index,
+				XFS_LOOKUP_BATCH, goal);
 		if (!nr_found) {
 			done = true;
 			rcu_read_unlock();
@@ -1769,20 +1752,6 @@ xfs_icwalk_ag(
 	return last_error;
 }
 
-/* Fetch the next (possibly tagged) per-AG structure. */
-static inline struct xfs_perag *
-xfs_icwalk_get_perag(
-	struct xfs_mount	*mp,
-	xfs_agnumber_t		agno,
-	enum xfs_icwalk_goal	goal)
-{
-	unsigned int		tag = xfs_icwalk_tag(goal);
-
-	if (tag == XFS_ICWALK_NULL_TAG)
-		return xfs_perag_get(mp, agno);
-	return xfs_perag_get_tag(mp, agno, tag);
-}
-
 /* Walk all incore inodes to achieve a given goal. */
 static int
 xfs_icwalk(
@@ -1795,7 +1764,7 @@ xfs_icwalk(
 	int			last_error = 0;
 	xfs_agnumber_t		agno = 0;
 
-	while ((pag = xfs_icwalk_get_perag(mp, agno, goal))) {
+	while ((pag = xfs_perag_get_tag(mp, agno, goal))) {
 		agno = pag->pag_agno + 1;
 		error = xfs_icwalk_ag(pag, goal, icw);
 		xfs_perag_put(pag);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] xfs: remove support for untagged lookups in xfs_icwalk*
  2021-08-13  8:16 [PATCH] xfs: remove support for untagged lookups in xfs_icwalk* Christoph Hellwig
@ 2021-08-13 16:26 ` Darrick J. Wong
  2021-08-14  7:22   ` Christoph Hellwig
  0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2021-08-13 16:26 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, Dan Carpenter

On Fri, Aug 13, 2021 at 10:16:23AM +0200, Christoph Hellwig wrote:
> With quotaoff not allowing disabling of accounting there is no need
> for untagged lookups in this code, so remove the dead leftovers.
> 
> Repoted-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_icache.c | 39 ++++-----------------------------------
>  1 file changed, 4 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index e7e69e55b7680a..f5a52ec084842d 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -43,15 +43,6 @@ enum xfs_icwalk_goal {

The patch itself looks fine, so
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

But you might as well convert the ag walk to use the foreach macros
like everywhere else:

--D

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index f5a52ec08484..b7ffdc03e0f7 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1762,16 +1762,16 @@ xfs_icwalk(
 	struct xfs_perag	*pag;
 	int			error = 0;
 	int			last_error = 0;
-	xfs_agnumber_t		agno = 0;
+	xfs_agnumber_t		agno;
 
-	while ((pag = xfs_perag_get_tag(mp, agno, goal))) {
-		agno = pag->pag_agno + 1;
+	for_each_perag_tag(mp, agno, pag, goal) {
 		error = xfs_icwalk_ag(pag, goal, icw);
-		xfs_perag_put(pag);
 		if (error) {
 			last_error = error;
-			if (error == -EFSCORRUPTED)
+			if (error == -EFSCORRUPTED) {
+				xfs_perag_put(pag);
 				break;
+			}
 		}
 	}
 	return last_error;
>  	XFS_ICWALK_RECLAIM	= XFS_ICI_RECLAIM_TAG,
>  };
>  
> -#define XFS_ICWALK_NULL_TAG	(-1U)
> -
> -/* Compute the inode radix tree tag for this goal. */
> -static inline unsigned int
> -xfs_icwalk_tag(enum xfs_icwalk_goal goal)
> -{
> -	return goal < 0 ? XFS_ICWALK_NULL_TAG : goal;
> -}
> -
>  static int xfs_icwalk(struct xfs_mount *mp,
>  		enum xfs_icwalk_goal goal, struct xfs_icwalk *icw);
>  static int xfs_icwalk_ag(struct xfs_perag *pag,
> @@ -1676,22 +1667,14 @@ xfs_icwalk_ag(
>  	nr_found = 0;
>  	do {
>  		struct xfs_inode *batch[XFS_LOOKUP_BATCH];
> -		unsigned int	tag = xfs_icwalk_tag(goal);
>  		int		error = 0;
>  		int		i;
>  
>  		rcu_read_lock();
>  
> -		if (tag == XFS_ICWALK_NULL_TAG)
> -			nr_found = radix_tree_gang_lookup(&pag->pag_ici_root,
> -					(void **)batch, first_index,
> -					XFS_LOOKUP_BATCH);
> -		else
> -			nr_found = radix_tree_gang_lookup_tag(
> -					&pag->pag_ici_root,
> -					(void **) batch, first_index,
> -					XFS_LOOKUP_BATCH, tag);
> -
> +		nr_found = radix_tree_gang_lookup_tag(&pag->pag_ici_root,
> +				(void **) batch, first_index,
> +				XFS_LOOKUP_BATCH, goal);
>  		if (!nr_found) {
>  			done = true;
>  			rcu_read_unlock();
> @@ -1769,20 +1752,6 @@ xfs_icwalk_ag(
>  	return last_error;
>  }
>  
> -/* Fetch the next (possibly tagged) per-AG structure. */
> -static inline struct xfs_perag *
> -xfs_icwalk_get_perag(
> -	struct xfs_mount	*mp,
> -	xfs_agnumber_t		agno,
> -	enum xfs_icwalk_goal	goal)
> -{
> -	unsigned int		tag = xfs_icwalk_tag(goal);
> -
> -	if (tag == XFS_ICWALK_NULL_TAG)
> -		return xfs_perag_get(mp, agno);
> -	return xfs_perag_get_tag(mp, agno, tag);
> -}
> -
>  /* Walk all incore inodes to achieve a given goal. */
>  static int
>  xfs_icwalk(
> @@ -1795,7 +1764,7 @@ xfs_icwalk(
>  	int			last_error = 0;
>  	xfs_agnumber_t		agno = 0;
>  
> -	while ((pag = xfs_icwalk_get_perag(mp, agno, goal))) {
> +	while ((pag = xfs_perag_get_tag(mp, agno, goal))) {
>  		agno = pag->pag_agno + 1;
>  		error = xfs_icwalk_ag(pag, goal, icw);
>  		xfs_perag_put(pag);
> -- 
> 2.30.2
> 

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] xfs: remove support for untagged lookups in xfs_icwalk*
  2021-08-13 16:26 ` Darrick J. Wong
@ 2021-08-14  7:22   ` Christoph Hellwig
  0 siblings, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2021-08-14  7:22 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs, Dan Carpenter

On Fri, Aug 13, 2021 at 09:26:36AM -0700, Darrick J. Wong wrote:
> On Fri, Aug 13, 2021 at 10:16:23AM +0200, Christoph Hellwig wrote:
> > With quotaoff not allowing disabling of accounting there is no need
> > for untagged lookups in this code, so remove the dead leftovers.
> > 
> > Repoted-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_icache.c | 39 ++++-----------------------------------
> >  1 file changed, 4 insertions(+), 35 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > index e7e69e55b7680a..f5a52ec084842d 100644
> > --- a/fs/xfs/xfs_icache.c
> > +++ b/fs/xfs/xfs_icache.c
> > @@ -43,15 +43,6 @@ enum xfs_icwalk_goal {
> 
> The patch itself looks fine, so
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> But you might as well convert the ag walk to use the foreach macros
> like everywhere else:

True, this makes sense for consistency.  That being said, comparing the
two, I find the one without the extra layer of macro sugar way easier
to read.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-14  7:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13  8:16 [PATCH] xfs: remove support for untagged lookups in xfs_icwalk* Christoph Hellwig
2021-08-13 16:26 ` Darrick J. Wong
2021-08-14  7:22   ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).